From 8dae7a7f80af01dea9ed67d801803de0fa766f32 Mon Sep 17 00:00:00 2001 From: James Lal Date: Fri, 7 Aug 2026 09:35:59 -0600 Subject: [PATCH] fix: disambiguate colliding schema type names --- src/analysis.rs | 371 +++++++++++++++++++++++++++- src/generator.rs | 37 +-- tests/schema_name_collision_test.rs | 82 ++++++ 3 files changed, 471 insertions(+), 19 deletions(-) create mode 100644 tests/schema_name_collision_test.rs diff --git a/src/analysis.rs b/src/analysis.rs index 5eb7a3b..f23d5b8 100644 --- a/src/analysis.rs +++ b/src/analysis.rs @@ -1090,7 +1090,8 @@ impl SchemaAnalyzer { /// Construct an analyzer with a caller-supplied [`TypeMapper`] /// (built from `GeneratorConfig.types`). The CLI / library entry /// points use this so user TOML config drives type generation. - pub fn with_type_mapper(openapi_spec: Value, type_mapper: TypeMapper) -> Result { + pub fn with_type_mapper(mut openapi_spec: Value, type_mapper: TypeMapper) -> Result { + disambiguate_component_schema_names(&mut openapi_spec); let spec: OpenApiSpec = serde_json::from_value(openapi_spec.clone()).map_err(GeneratorError::ParseError)?; let schemas = Self::extract_schemas(&spec)?; @@ -1342,6 +1343,8 @@ impl SchemaAnalyzer { } } + disambiguate_analyzed_schema_names(&mut analysis, &self.schemas); + // Snapshot the type-mapper's used-features set so the // generator can decide which helper modules to emit // (e.g. base64_serde for `format: byte`). @@ -5870,3 +5873,369 @@ impl SchemaAnalyzer { } } } + +fn disambiguate_component_schema_names(openapi_spec: &mut Value) { + let Some(schemas) = openapi_spec + .pointer_mut("/components/schemas") + .and_then(Value::as_object_mut) + else { + return; + }; + + let mut names_by_rust_name = BTreeMap::>::new(); + for name in schemas.keys() { + names_by_rust_name + .entry(crate::generator::rust_type_name(name)) + .or_default() + .push(name.clone()); + } + + // Reserve every identifier already represented by the document so a + // suffix never steals another component's canonical Rust name. + let mut claimed_rust_names = names_by_rust_name.keys().cloned().collect::>(); + let mut aliases = BTreeMap::::new(); + + for (rust_name, mut names) in names_by_rust_name { + if names.len() < 2 { + continue; + } + + // Prefer an already-canonical component key (for example `Alert` + // over `alert`), then use lexical order for deterministic results. + names.sort_by_key(|name| (name != &rust_name, name.clone())); + for source_name in names.into_iter().skip(1) { + let mut suffix = 2; + let replacement = loop { + let candidate = format!("{rust_name}{suffix}"); + if claimed_rust_names.insert(candidate.clone()) { + break candidate; + } + suffix += 1; + }; + + eprintln!( + "⚠️ schema `{source_name}` maps to the existing Rust type `{rust_name}` — disambiguated to `{replacement}`" + ); + aliases.insert(source_name, replacement); + } + } + + if aliases.is_empty() { + return; + } + + let original_schemas = std::mem::take(schemas); + for (name, schema) in original_schemas { + schemas.insert(aliases.get(&name).cloned().unwrap_or(name), schema); + } + + rewrite_component_schema_references(openapi_spec, &aliases); +} + +fn disambiguate_analyzed_schema_names( + analysis: &mut SchemaAnalysis, + component_schemas: &BTreeMap, +) { + let mut names_by_rust_name = BTreeMap::>::new(); + for name in analysis.schemas.keys() { + names_by_rust_name + .entry(crate::generator::rust_type_name(name)) + .or_default() + .push(name.clone()); + } + + let mut claimed_rust_names = names_by_rust_name.keys().cloned().collect::>(); + let mut aliases = BTreeMap::::new(); + + for (rust_name, mut names) in names_by_rust_name { + if names.len() < 2 { + continue; + } + names.sort_by_key(|name| { + ( + !component_schemas.contains_key(name), + name != &rust_name, + name.clone(), + ) + }); + + for source_name in names.into_iter().skip(1) { + let mut suffix = 2; + let replacement = loop { + let candidate = format!("{rust_name}{suffix}"); + if claimed_rust_names.insert(candidate.clone()) { + break candidate; + } + suffix += 1; + }; + eprintln!( + "⚠️ generated schema `{source_name}` maps to the existing Rust type `{rust_name}` — disambiguated to `{replacement}`" + ); + aliases.insert(source_name, replacement); + } + } + + if aliases.is_empty() { + return; + } + + let original_schemas = std::mem::take(&mut analysis.schemas); + for (name, mut schema) in original_schemas { + schema.name = renamed_schema_name(&schema.name, &aliases); + schema.dependencies = schema + .dependencies + .into_iter() + .map(|name| renamed_schema_name(&name, &aliases)) + .collect(); + rewrite_schema_type_names(&mut schema.schema_type, &aliases); + analysis + .schemas + .insert(renamed_schema_name(&name, &aliases), schema); + } + + let original_edges = std::mem::take(&mut analysis.dependencies.edges); + for (name, dependencies) in original_edges { + analysis.dependencies.edges.insert( + renamed_schema_name(&name, &aliases), + dependencies + .into_iter() + .map(|name| renamed_schema_name(&name, &aliases)) + .collect(), + ); + } + analysis.dependencies.recursive_schemas = analysis + .dependencies + .recursive_schemas + .iter() + .map(|name| renamed_schema_name(name, &aliases)) + .collect(); + + analysis.patterns.tagged_enum_schemas = analysis + .patterns + .tagged_enum_schemas + .iter() + .map(|name| renamed_schema_name(name, &aliases)) + .collect(); + analysis.patterns.untagged_enum_schemas = analysis + .patterns + .untagged_enum_schemas + .iter() + .map(|name| renamed_schema_name(name, &aliases)) + .collect(); + analysis.patterns.type_mappings = std::mem::take(&mut analysis.patterns.type_mappings) + .into_iter() + .map(|(name, mappings)| { + ( + renamed_schema_name(&name, &aliases), + mappings + .into_iter() + .map(|(value, schema_name)| { + (value, renamed_schema_name(&schema_name, &aliases)) + }) + .collect(), + ) + }) + .collect(); + + for operation in analysis.operations.values_mut() { + if let Some(request_body) = &mut operation.request_body { + rewrite_request_body_schema_name(request_body, &aliases); + } + for schema_name in operation.response_schemas.values_mut() { + *schema_name = renamed_schema_name(schema_name, &aliases); + } + for parameter in &mut operation.parameters { + if let Some(schema_name) = &mut parameter.schema_ref { + *schema_name = renamed_schema_name(schema_name, &aliases); + } + if let Some(serialization) = &mut parameter.query_serialization { + rewrite_query_serialization_schema_names(serialization, &aliases); + } + } + } + + for responses in analysis.operation_responses.values_mut() { + for response in responses.values_mut() { + if let Some(schema_name) = &mut response.schema_name { + *schema_name = renamed_schema_name(schema_name, &aliases); + } + if let Some(OperationResponseBody::Json { schema_name, .. }) = &mut response.body { + *schema_name = renamed_schema_name(schema_name, &aliases); + } + } + } +} + +fn renamed_schema_name(name: &str, aliases: &BTreeMap) -> String { + aliases + .get(name) + .cloned() + .unwrap_or_else(|| name.to_string()) +} + +fn rewrite_schema_type_names(schema_type: &mut SchemaType, aliases: &BTreeMap) { + match schema_type { + SchemaType::Object { + properties, + additional_properties, + .. + } => { + for property in properties.values_mut() { + rewrite_schema_type_names(&mut property.schema_type, aliases); + } + if let ObjectAdditionalProperties::Typed { value_type } = additional_properties { + rewrite_schema_type_names(value_type, aliases); + } + } + SchemaType::DiscriminatedUnion { variants, .. } => { + for variant in variants { + variant.type_name = renamed_schema_name(&variant.type_name, aliases); + variant.schema_ref = renamed_schema_name(&variant.schema_ref, aliases); + } + } + SchemaType::Union { variants } | SchemaType::Composition { schemas: variants } => { + for variant in variants { + variant.target = renamed_schema_name(&variant.target, aliases); + } + } + SchemaType::Array { item_type } => rewrite_schema_type_names(item_type, aliases), + SchemaType::Reference { target } => { + *target = renamed_schema_name(target, aliases); + } + SchemaType::Primitive { .. } + | SchemaType::StringEnum { .. } + | SchemaType::ExtensibleEnum { .. } => {} + } +} + +fn rewrite_request_body_schema_name( + request_body: &mut RequestBodyContent, + aliases: &BTreeMap, +) { + match request_body { + RequestBodyContent::Json { schema_name, .. } + | RequestBodyContent::FormUrlEncoded { schema_name, .. } + | RequestBodyContent::Multipart { schema_name, .. } => { + *schema_name = renamed_schema_name(schema_name, aliases); + } + _ => {} + } +} + +fn rewrite_query_serialization_schema_names( + serialization: &mut QuerySerialization, + aliases: &BTreeMap, +) { + match serialization { + QuerySerialization::FormExplodedArray { item_type } + | QuerySerialization::FormArray { item_type } + | QuerySerialization::SimpleHeaderArray { item_type } => { + rewrite_array_item_type_schema_names(item_type, aliases); + } + QuerySerialization::FormExplodedNestedObject { properties } => { + for property in properties { + rewrite_query_property_type_schema_names(&mut property.value_type, aliases); + } + } + _ => {} + } +} + +fn rewrite_array_item_type_schema_names( + item_type: &mut ArrayItemType, + aliases: &BTreeMap, +) { + match item_type { + ArrayItemType::SchemaRef(name) => *name = renamed_schema_name(name, aliases), + ArrayItemType::FlatStructRef { + schema_name, + properties, + } + | ArrayItemType::NestedStructRef { + schema_name, + properties, + } => { + *schema_name = renamed_schema_name(schema_name, aliases); + for property in properties { + rewrite_query_property_type_schema_names(&mut property.value_type, aliases); + } + } + ArrayItemType::Scalar(_) => {} + } +} + +fn rewrite_query_property_type_schema_names( + property_type: &mut QueryStructPropertyType, + aliases: &BTreeMap, +) { + match property_type { + QueryStructPropertyType::Array { item_type } => { + rewrite_array_item_type_schema_names(item_type, aliases) + } + QueryStructPropertyType::Object { properties } => { + for property in properties { + rewrite_query_property_type_schema_names(&mut property.value_type, aliases); + } + } + QueryStructPropertyType::Scalar(_) => {} + } +} + +fn rewrite_component_schema_references(value: &mut Value, aliases: &BTreeMap) { + match value { + Value::Array(values) => { + for value in values { + rewrite_component_schema_references(value, aliases); + } + } + Value::Object(object) => { + if let Some(Value::String(reference)) = object.get_mut("$ref") { + rewrite_component_schema_reference(reference, aliases); + } + + if let Some(Value::Object(mapping)) = object.get_mut("mapping") { + for target_value in mapping.values_mut() { + let Some(target) = target_value.as_str() else { + continue; + }; + let replacement = aliases.get(target).cloned().or_else(|| { + let mut target = target.to_string(); + rewrite_component_schema_reference(&mut target, aliases).then_some(target) + }); + if let Some(replacement) = replacement { + *target_value = Value::String(replacement); + } + } + } + + for value in object.values_mut() { + rewrite_component_schema_references(value, aliases); + } + } + _ => {} + } +} + +fn rewrite_component_schema_reference( + reference: &mut String, + aliases: &BTreeMap, +) -> bool { + const PREFIX: &str = "#/components/schemas/"; + let Some(encoded_name) = reference.strip_prefix(PREFIX) else { + return false; + }; + let encoded_name = encoded_name.split('/').next().unwrap_or(encoded_name); + + for (source, replacement) in aliases { + let encoded_source = source.replace('~', "~0").replace('/', "~1"); + if encoded_name == encoded_source { + reference.replace_range( + PREFIX.len()..PREFIX.len() + encoded_source.len(), + replacement, + ); + return true; + } + } + + false +} diff --git a/src/generator.rs b/src/generator.rs index 4bc1655..566290c 100644 --- a/src/generator.rs +++ b/src/generator.rs @@ -508,6 +508,8 @@ impl CodeGenerator { /// Generate the types.rs file content fn generate_types(&self, analysis: &mut SchemaAnalysis) -> Result { + self.validate_schema_type_names(analysis)?; + let provenance_attribute = self.provenance_attribute(); let mut type_definitions = TokenStream::new(); @@ -562,25 +564,11 @@ impl CodeGenerator { // Generate types based on dependency order let generation_order = analysis.dependencies.topological_sort()?; - // Defensive layer: track emitted Rust type names so that two - // analyzed schemas which sanitize to the same Rust ident don't - // produce two definitions (E0119 conflicting impls / E0428 name - // defined multiple times). The first occurrence wins; later - // occurrences are silently dropped. Schema-name uniqueness at the - // analysis layer is a follow-up; this stops the generated file from - // failing to compile. - let mut emitted_rust_names: std::collections::HashSet = - std::collections::HashSet::new(); let mut processed = std::collections::HashSet::new(); // First, generate schemas in dependency order for schema_name in generation_order { if let Some(schema) = analysis.schemas.get(&schema_name) { - let rust_name = self.to_rust_type_name(&schema.name); - if !emitted_rust_names.insert(rust_name) { - processed.insert(schema_name); - continue; - } let type_def = self.generate_type_definition(schema, analysis, &type_context)?; if !type_def.is_empty() { type_definitions.extend(type_def); @@ -598,10 +586,6 @@ impl CodeGenerator { remaining_schemas.sort_by_key(|(name, _)| name.as_str()); for (_schema_name, schema) in remaining_schemas { - let rust_name = self.to_rust_type_name(&schema.name); - if !emitted_rust_names.insert(rust_name) { - continue; - } let type_def = self.generate_type_definition(schema, analysis, &type_context)?; if !type_def.is_empty() { type_definitions.extend(type_def); @@ -833,6 +817,23 @@ impl CodeGenerator { Ok(prettyplease::unparse(&syntax_tree)) } + fn validate_schema_type_names(&self, analysis: &SchemaAnalysis) -> Result<()> { + let mut source_by_rust_name = BTreeMap::::new(); + + for schema in analysis.schemas.values() { + let rust_name = self.to_rust_type_name(&schema.name); + if let Some(first) = source_by_rust_name.insert(rust_name.clone(), schema.name.clone()) + { + return Err(GeneratorError::InvalidSchema(format!( + "schema names `{first}` and `{}` both map to Rust type `{rust_name}`", + schema.name + ))); + } + } + + Ok(()) + } + /// Generate HTTP client code for regular (non-streaming) requests. /// /// This standalone entry point honors `[client].operations` but does not diff --git a/tests/schema_name_collision_test.rs b/tests/schema_name_collision_test.rs new file mode 100644 index 0000000..dbbf13c --- /dev/null +++ b/tests/schema_name_collision_test.rs @@ -0,0 +1,82 @@ +use openapi_to_rust::{CodeGenerator, GeneratorConfig, SchemaAnalyzer}; +use serde_json::json; + +#[test] +fn distinct_component_keys_that_map_to_the_same_rust_type_are_disambiguated() { + let spec = json!({ + "openapi": "3.1.0", + "info": { "title": "name-collision-poc", "version": "1.0.0" }, + "paths": { + "/event": { + "get": { + "operationId": "getEvent", + "responses": { + "200": { + "description": "one event", + "content": { + "application/json": { + "schema": { "$ref": "#/components/schemas/session.status" } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "SessionStatus": { + "anyOf": [ + { + "type": "object", + "properties": { + "type": { "type": "string", "enum": ["idle"] } + }, + "required": ["type"] + }, + { + "type": "object", + "properties": { + "type": { "type": "string", "enum": ["busy"] }, + "detail": { "type": "string" } + }, + "required": ["type"] + } + ] + }, + "session.status": { + "type": "object", + "properties": { + "id": { "type": "string" }, + "data": { + "type": "object", + "properties": { + "sessionID": { "type": "string" }, + "status": { "$ref": "#/components/schemas/SessionStatus" } + }, + "required": ["sessionID", "status"] + } + }, + "required": ["id", "data"] + } + } + } + }); + + let mut analyzer = SchemaAnalyzer::new(spec).expect("spec should parse"); + let mut analysis = analyzer.analyze().expect("spec should analyze"); + let generated = CodeGenerator::new(GeneratorConfig::default()) + .generate(&mut analysis) + .expect("colliding component schema names should be disambiguated"); + + assert!(generated.contains("pub enum SessionStatus")); + assert!(generated.contains("pub struct SessionStatus2")); + assert!(generated.contains("pub status: SessionStatus")); + assert!(analysis.schemas.contains_key("SessionStatus")); + assert!(analysis.schemas.contains_key("SessionStatus2")); + assert!(!analysis.schemas.contains_key("session.status")); + assert_eq!( + analysis.operations["getEvent"].response_schemas["200"], + "SessionStatus2" + ); +}