From 98d0de222acfd41c33d023fbf50ab2e5a951036f Mon Sep 17 00:00:00 2001 From: Brian Thorne Date: Tue, 2 Jun 2026 12:01:51 +1200 Subject: [PATCH 1/3] fix(codegen/python): disambiguate namespace/root leaf-name collisions (#161) A root type and a same-leaf type under a sub-namespace each emitted a class plus a bare ` = ` alias that shadowed the `from .._types import ` the module also imported, so `_types.` and `.` resolved to two distinct classes and field annotations bound to the wrong one. Resolve each logical type to a single class: keep the short name bound to the imported root type and reference the namespace-local type by its disambiguated flat name. No-op for schemas without such collisions. --- CHANGELOG.md | 1 + docs/src/clients/README.md | 7 + reflectapi-demo/src/tests/namespace.rs | 121 ++++++++++++++ reflectapi/src/codegen/python.rs | 218 +++++++++++++++++++++++-- 4 files changed, 333 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1cdf4acf..eae24c05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Python client generation improvements: - Multi-field Rust tuple structs now generate valid Python `RootModel[tuple[...]]` models instead of invalid numeric field names. - Generated Python clients with nested namespaces now import cleanly when models refer to parent or sibling namespaces, including cases like `offer_rules.InsurerCategory`. - Namespace names containing characters that are not valid in Python identifiers, such as dashes or leading digits, now generate valid Python classes. +- A root type and a same-leaf type under a sub-namespace (e.g. `IfConflictOnUpdate` and `nomatches::IfConflictOnUpdate`) no longer collide on a bare public name. Previously each namespace re-emitted a ` = ` alias that shadowed the `from .._types import ` it also imported, so `_types.` and `.` resolved to two different classes and field annotations silently bound to the wrong one (pydantic then rejected values with a confusingly self-identical error). The namespace-local type now keeps its disambiguated name (`NomatchesIfConflictOnUpdate`) and references resolve to a single class per logical type. - Schemas with a root namespace named `sys` no longer conflict with Python's standard `sys` module. - Re-running `reflectapi codegen` into an existing output directory now removes generated files from older schemas while preserving hand-written files. diff --git a/docs/src/clients/README.md b/docs/src/clients/README.md index 4b8f9bc6..74a9fb03 100644 --- a/docs/src/clients/README.md +++ b/docs/src/clients/README.md @@ -86,6 +86,13 @@ The demo repository includes extra project scaffolding around some generated cli - Adds a sync client only when `--python-sync` is passed. - Emits reflected Rust namespaces as real Python packages. `generated.py` is kept as a temporary compatibility facade inside the package. +- Each namespace exposes its types under short, ergonomic names (`order.Item`). + When a namespace defines a type whose short name clashes with a top-level type + of the same name (e.g. both a root `IfConflictOnUpdate` and a + `nomatches::IfConflictOnUpdate`), the namespace keeps the short name bound to + the imported top-level type and exposes its own type under a disambiguated, + namespace-prefixed name (`nomatches.NomatchesIfConflictOnUpdate`). This keeps + one Python class per logical type so `model.` resolves consistently. - Uses `reflectapi_runtime` for client base classes and runtime helpers. ### Rust diff --git a/reflectapi-demo/src/tests/namespace.rs b/reflectapi-demo/src/tests/namespace.rs index a100802f..f344f4cc 100644 --- a/reflectapi-demo/src/tests/namespace.rs +++ b/reflectapi-demo/src/tests/namespace.rs @@ -122,6 +122,127 @@ fn test_python_destutter_collision_falls_back_to_original_name() { assert!(python.contains("OfferRequestPartIdentity = OfferRequestOfferRequestPartIdentity")); } +/// Regression for #161: in the multi-file (package) Python client a root +/// type and a same-leaf type under a sub-namespace must not collide on a +/// bare public name. Previously each namespace emitted a ` = +/// ` rebind that shadowed the `from .._types import +/// ` it also emitted, so `_types.` and `.` resolved to +/// two distinct classes and field annotations silently bound to the wrong +/// one (pydantic then rejected values with a self-identical error message). +#[test] +fn test_python_namespace_leaf_collision_no_shadowing() { + #[derive( + Debug, serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output, + )] + struct SomeMatch { + id: String, + } + + #[derive( + Debug, serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output, + )] + struct RootConflict { + not_exists: Option, + matches: Option, + } + + #[derive( + Debug, serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output, + )] + struct NsConflict { + not_exists: Option, + matches: Option, + } + + #[derive( + Debug, serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output, + )] + struct TopWrapper { + conflict: RootConflict, + ns_conflict: NsConflict, + } + + #[derive( + Debug, serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output, + )] + struct NomatchesUpdateRequest { + conflict: NsConflict, + } + + async fn top_get(_s: S, _: reflectapi::Empty, _: reflectapi::Empty) -> TopWrapper { + unimplemented!() + } + async fn nomatches_update( + _s: S, + _: NomatchesUpdateRequest, + _: reflectapi::Empty, + ) -> reflectapi::Empty { + reflectapi::Empty {} + } + + let (schema, _) = reflectapi::Builder::<()>::new() + .route(top_get, |b| b.name("top.get")) + .route(nomatches_update, |b| b.name("nomatches.update")) + .rename_types( + "reflectapi_demo::tests::namespace::TopWrapper", + "TopWrapper", + ) + .rename_types( + "reflectapi_demo::tests::namespace::NomatchesUpdateRequest", + "nomatches::UpdateRequest", + ) + .rename_types("reflectapi_demo::tests::namespace::SomeMatch", "SomeMatch") + .rename_types( + "reflectapi_demo::tests::namespace::RootConflict", + "IfConflictOnUpdate", + ) + .rename_types( + "reflectapi_demo::tests::namespace::NsConflict", + "nomatches::IfConflictOnUpdate", + ) + .build() + .unwrap(); + + let files = reflectapi::codegen::python::generate_files( + schema, + &reflectapi::codegen::python::Config::default(), + ) + .unwrap(); + + let types_py = files.get("_types.py").unwrap(); + let nomatches_py = files.get("nomatches/__init__.py").unwrap(); + + // The root type is defined once (in _types.py); the namespace type keeps + // its globally-unique, namespace-prefixed flat name. + assert!(types_py.contains("class IfConflictOnUpdate(BaseModel")); + assert!(nomatches_py.contains("class NomatchesIfConflictOnUpdate(BaseModel")); + assert!(nomatches_py.contains("from .._types import")); + + // The namespace must NOT rebind the bare `IfConflictOnUpdate` it imports + // from `_types` to a *different* local class — that shadowing is the #161 + // bug (`_types.X is not nomatches.X`). + assert!( + !nomatches_py.contains("IfConflictOnUpdate = NomatchesIfConflictOnUpdate"), + "namespace must not shadow the imported root `IfConflictOnUpdate`:\n{nomatches_py}" + ); + + // The namespace wrapper's field must bind to the disambiguated local + // class, not the bare (formerly shadowed) leaf. + assert!( + nomatches_py.contains("conflict: NomatchesIfConflictOnUpdate["), + "namespace field must reference the disambiguated local class:\n{nomatches_py}" + ); + + // A root type that references the namespace-local type must qualify it + // with the disambiguated name; `nomatches.IfConflictOnUpdate` now resolves + // to the imported root class, so it would otherwise be the wrong type. + assert!( + types_py.contains("nomatches.NomatchesIfConflictOnUpdate["), + "root cross-namespace ref must use the disambiguated name:\n{types_py}" + ); + assert!(!types_py.contains("nomatches.IfConflictOnUpdate[")); +} + #[test] fn test_python_init_exports_client() { #[derive( diff --git a/reflectapi/src/codegen/python.rs b/reflectapi/src/codegen/python.rs index d7379a72..3bd875e5 100644 --- a/reflectapi/src/codegen/python.rs +++ b/reflectapi/src/codegen/python.rs @@ -2101,6 +2101,136 @@ fn flat_namespace_prefix(ns_path: &[String]) -> String { .join("") } +/// Disambiguated flat class names for *colliding* leaf names, keyed by the +/// owning namespace module path. +/// +/// A leaf "collides" when a sub-namespace defines a type whose short +/// (prefix-stripped) name equals a *root* type name. Every namespace module +/// imports all root types via `from .._types import ...`, so exposing the +/// short name as a bare alias to a different local class would shadow that +/// import — and that shadowing is the root of #161: `_types.` and +/// `.` then resolve to two distinct classes, and field annotations +/// silently bind to the wrong one. +#[derive(Default)] +struct CollisionRegistry { + by_module: BTreeMap, BTreeMap>, +} + +impl CollisionRegistry { + /// Colliding `leaf -> flat name` entries owned by `ns_path`. + fn for_module(&self, ns_path: &[String]) -> BTreeMap { + self.by_module.get(ns_path).cloned().unwrap_or_default() + } +} + +fn collect_collisions( + module: &templates::Module, + ns_path: &[String], + root_type_names: &BTreeSet, + by_module: &mut BTreeMap, BTreeMap>, +) { + if !ns_path.is_empty() { + let flat_prefix = flat_namespace_prefix(ns_path); + let mut collisions = BTreeMap::new(); + for mt in &module.types { + for flat_name in extract_defined_python_names(&mt.rendered) { + let Some(stripped) = flat_name.strip_prefix(&flat_prefix) else { + continue; + }; + if !stripped.is_empty() + && stripped != flat_name + && root_type_names.contains(stripped) + { + collisions.insert(stripped.to_string(), flat_name.clone()); + } + } + } + if !collisions.is_empty() { + by_module.insert(ns_path.to_vec(), collisions); + } + } + + for sub in module.submodules.values() { + if sub.is_empty() { + continue; + } + let mut child_path = ns_path.to_vec(); + child_path.push(sub.name.clone()); + collect_collisions(sub, &child_path, root_type_names, by_module); + } +} + +fn collision_registry( + module: &templates::Module, + root_type_names: &BTreeSet, +) -> CollisionRegistry { + let mut by_module = BTreeMap::new(); + collect_collisions(module, &[], root_type_names, &mut by_module); + CollisionRegistry { by_module } +} + +/// Replace every boundary-delimited occurrence of `token` with `replacement`. +/// Both ends must fall on a non-identifier boundary so that, e.g., replacing +/// `ns.Foo` leaves `ns.FooBar` untouched. +fn replace_token(code: &str, token: &str, replacement: &str) -> String { + let is_ident = |c: char| c.is_ascii_alphanumeric() || c == '_'; + let mut output = String::with_capacity(code.len()); + let mut cursor = 0; + + for (index, _) in code.match_indices(token) { + if index < cursor { + continue; + } + let before_is_boundary = index == 0 + || code[..index] + .chars() + .next_back() + .is_none_or(|c| !is_ident(c)); + let after_index = index + token.len(); + let after_is_boundary = code[after_index..] + .chars() + .next() + .is_none_or(|c| !is_ident(c)); + if before_is_boundary && after_is_boundary { + output.push_str(&code[cursor..index]); + output.push_str(replacement); + cursor = after_index; + } + } + + output.push_str(&code[cursor..]); + output +} + +/// Rewrite references to colliding types so they resolve to a single class. +/// +/// Cross-namespace references are qualified with the disambiguated flat name +/// (`nomatches.IfConflictOnUpdate` -> `nomatches.NomatchesIfConflictOnUpdate`); +/// references to the current module's own colliding types are localized to the +/// bare flat name. A no-op when no collisions exist, so non-colliding schemas +/// generate byte-identical output. +fn apply_collision_rewrites( + code: &str, + ns_path: &[String], + registry: &CollisionRegistry, +) -> String { + let mut result = code.to_string(); + for (owner_ns, collisions) in ®istry.by_module { + if owner_ns.as_slice() == ns_path { + continue; + } + let owner_qualified = module_qualified_name(owner_ns); + for (leaf, flat) in collisions { + let from = format!("{owner_qualified}.{leaf}"); + if result.contains(&from) { + let to = format!("{owner_qualified}.{flat}"); + result = replace_token(&result, &from, &to); + } + } + } + localize_current_module_refs(&result, ns_path, ®istry.for_module(ns_path)) +} + fn module_aliases(module: &templates::Module, ns_path: &[String]) -> BTreeMap { let flat_prefix = flat_namespace_prefix(ns_path); let mut aliases = BTreeMap::new(); @@ -2216,26 +2346,47 @@ fn contains_qualified_name(code: &str, name: &str) -> bool { }) } -fn strip_qualified_prefix(code: &str, prefix: &str) -> String { +/// Strip the current module's `.` from references to its own types so +/// they read as bare local names. A colliding leaf is instead rewritten to its +/// disambiguated flat name (passed in `collisions`) rather than the bare leaf, +/// because the bare leaf is owned by the same-named root type imported via +/// `from .._types import ...`. With an empty `collisions` map this is a plain +/// prefix strip. +fn rewrite_module_prefix( + code: &str, + prefix: &str, + collisions: &BTreeMap, +) -> String { let needle = format!("{prefix}."); let mut output = String::with_capacity(code.len()); let mut cursor = 0; for (index, _) in code.match_indices(&needle) { + if index < cursor { + continue; + } let before_is_boundary = index == 0 || code[..index] .chars() .next_back() .is_none_or(|c| !(c.is_ascii_alphanumeric() || c == '_')); let after_index = index + needle.len(); - let after_is_boundary = code[after_index..] + let ident: String = code[after_index..] + .chars() + .take_while(|c| c.is_ascii_alphanumeric() || *c == '_') + .collect(); + let starts_identifier = ident .chars() .next() - .is_none_or(|c| c.is_ascii_alphabetic() || c == '_'); + .is_some_and(|c| c.is_ascii_alphabetic() || c == '_'); - if before_is_boundary && after_is_boundary { + if before_is_boundary && starts_identifier { output.push_str(&code[cursor..index]); - cursor = after_index; + match collisions.get(&ident) { + Some(flat) => output.push_str(flat), + None => output.push_str(&ident), + } + cursor = after_index + ident.len(); } } @@ -2243,12 +2394,16 @@ fn strip_qualified_prefix(code: &str, prefix: &str) -> String { output } -fn localize_current_module_refs(code: &str, ns_path: &[String]) -> String { +fn localize_current_module_refs( + code: &str, + ns_path: &[String], + collisions: &BTreeMap, +) -> String { if ns_path.is_empty() { return code.to_string(); } - strip_qualified_prefix(code, &module_qualified_name(ns_path)) + rewrite_module_prefix(code, &module_qualified_name(ns_path), collisions) } fn module_rendered_code(module: &templates::Module) -> String { @@ -2495,6 +2650,7 @@ fn render_module_file( module: &templates::Module, ns_path: &[String], root_type_names: &BTreeSet, + registry: &CollisionRegistry, config: &Config, ) -> anyhow::Result { let mut parts = vec![generation.preamble.clone()]; @@ -2524,13 +2680,20 @@ fn render_module_file( parts.push(deferred_namespace_placeholders); } + let current_collisions = registry.for_module(ns_path); + let mut body = String::new(); for mt in &module.types { - body.push_str(&localize_current_module_refs(&mt.rendered, ns_path)); + body.push_str(&apply_collision_rewrites(&mt.rendered, ns_path, registry)); body.push('\n'); } - let aliases = module_aliases(module, ns_path); + let mut aliases = module_aliases(module, ns_path); + // Drop bare aliases whose name collides with a same-named root type + // imported into this module: binding the short name to a different local + // class would shadow that import (#161). The local class stays reachable + // under its disambiguated flat name. + aliases.retain(|leaf, _| !current_collisions.contains_key(leaf)); if !aliases.is_empty() { if !body.trim().is_empty() { body.push('\n'); @@ -2591,6 +2754,7 @@ fn render_package_module_files( module: &templates::Module, ns_path: &[String], root_type_names: &BTreeSet, + registry: &CollisionRegistry, config: &Config, files: &mut BTreeMap, ) -> anyhow::Result<()> { @@ -2598,7 +2762,14 @@ fn render_package_module_files( let path = module_file_path(ns_path); let previous = files.insert( path.clone(), - render_module_file(generation, module, ns_path, root_type_names, config)?, + render_module_file( + generation, + module, + ns_path, + root_type_names, + registry, + config, + )?, ); anyhow::ensure!( previous.is_none(), @@ -2612,7 +2783,15 @@ fn render_package_module_files( } let mut child_path = ns_path.to_vec(); child_path.push(sub.name.clone()); - render_package_module_files(generation, sub, &child_path, root_type_names, config, files)?; + render_package_module_files( + generation, + sub, + &child_path, + root_type_names, + registry, + config, + files, + )?; } Ok(()) @@ -2733,6 +2912,7 @@ fn render_rebuild_file( fn render_client_file( generation: &PythonGeneration, root_type_names: &BTreeSet, + registry: &CollisionRegistry, config: &Config, ) -> anyhow::Result { let mut parts = vec![generation.preamble.clone()]; @@ -2761,7 +2941,11 @@ fn render_client_file( parts.push( "from ._rebuild import rebuild_models as _rebuild_models\n\n_rebuild_models()".to_string(), ); - parts.push(generation.client_code.clone()); + parts.push(apply_collision_rewrites( + &generation.client_code, + &[], + registry, + )); render_python_file(config, parts.join("\n\n")) } @@ -2865,6 +3049,11 @@ fn render_package_files( let root_key = Vec::::new(); let root_type_names = flat_imports.remove(&root_key).unwrap_or_default(); + // Resolve namespace/root leaf-name collisions to a single class per logical + // type (#161). Empty for schemas without such collisions, leaving output + // byte-identical. + let registry = collision_registry(&generation.module_tree, &root_type_names); + if !root_type_names.is_empty() { let root_types_code = generation .module_tree @@ -2880,7 +3069,7 @@ fn render_package_files( root_type_parts.push(root_namespace_bindings); } root_type_parts.push(render_external_type_aliases()); - root_type_parts.push(root_types_code); + root_type_parts.push(apply_collision_rewrites(&root_types_code, &[], ®istry)); files.insert( "_types.py".to_string(), render_python_file(config, root_type_parts.join("\n\n"))?, @@ -2892,13 +3081,14 @@ fn render_package_files( &generation.module_tree, &[], &root_type_names, + ®istry, config, &mut files, )?; files.insert( "_client.py".to_string(), - render_client_file(&generation, &root_type_names, config)?, + render_client_file(&generation, &root_type_names, ®istry, config)?, ); files.insert( "_rebuild.py".to_string(), From e5a7db8f7470b4ad153f08a0c11f7589a7dfc602 Mon Sep 17 00:00:00 2001 From: Brian Thorne Date: Tue, 2 Jun 2026 12:07:49 +1200 Subject: [PATCH 2/3] fix(codegen): report formatter exit status instead of broken-pipe error A formatter (ruff/prettier/rustfmt) that rejects its input can exit before draining stdin, making `write_all` fail with `BrokenPipe` and masking the real exit status. This made `python_format_reports_ruff_failures` flaky under CI load. Swallow only `BrokenPipe` on the stdin write so `wait_with_output` reports the actual failure. --- reflectapi/src/codegen/format.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/reflectapi/src/codegen/format.rs b/reflectapi/src/codegen/format.rs index f587b631..39aad817 100644 --- a/reflectapi/src/codegen/format.rs +++ b/reflectapi/src/codegen/format.rs @@ -23,7 +23,15 @@ pub fn format_with<'a>( let stdin = child.stdin.as_mut().unwrap(); - stdin.write_all(src.as_bytes())?; + // A formatter that rejects its input can exit before draining stdin, + // which turns this write into a `BrokenPipe` error. Swallow only that + // case so the real failure (exit status + stderr) is reported by + // `wait_with_output` below, instead of being masked by "broken pipe". + if let Err(err) = stdin.write_all(src.as_bytes()) { + if err.kind() != io::ErrorKind::BrokenPipe { + return Err(err); + } + } let output = child.wait_with_output()?; if !output.status.success() { return Err(io::Error::other(format!( From 17ac4828d05f344cbfca1a5d213d888d18acabea Mon Sep 17 00:00:00 2001 From: Brian Thorne Date: Tue, 2 Jun 2026 12:28:57 +1200 Subject: [PATCH 3/3] fix(codegen/python): detect collisions in the Rust-leaf alias too (#161) module_aliases() emits both a prefix-stripped alias and a Rust-leaf alias, and either can shadow a same-named imported root type. The collision registry only inspected the stripped form, so a namespaced `OrderInsertData` whose flat name de-stutters to `MyapiOrderInsertData` slipped through: its stripped alias is the harmless `InsertData`, but the Rust-leaf alias `OrderInsertData = MyapiOrderInsertData` still shadowed the imported root. Build the registry from the module's actual aliases so both forms are covered, and add a regression test. Reported by Codex review on #162. --- reflectapi-demo/src/tests/namespace.rs | 98 ++++++++++++++++++++++++++ reflectapi/src/codegen/python.rs | 26 +++---- 2 files changed, 109 insertions(+), 15 deletions(-) diff --git a/reflectapi-demo/src/tests/namespace.rs b/reflectapi-demo/src/tests/namespace.rs index f344f4cc..a92b2abd 100644 --- a/reflectapi-demo/src/tests/namespace.rs +++ b/reflectapi-demo/src/tests/namespace.rs @@ -243,6 +243,104 @@ fn test_python_namespace_leaf_collision_no_shadowing() { assert!(!types_py.contains("nomatches.IfConflictOnUpdate[")); } +/// Regression for #161 (Codex review follow-up): the collision can be hidden in +/// the *Rust-leaf* alias rather than the prefix-stripped one. A namespaced +/// `myapi::order::OrderInsertData` de-stutters to flat `MyapiOrderInsertData`, +/// so the stripped alias is the harmless `InsertData` while the Rust-leaf alias +/// `OrderInsertData = MyapiOrderInsertData` is what shadows the imported root +/// `OrderInsertData`. Collision detection must cover both alias forms. +#[test] +fn test_python_namespace_destutter_leaf_collision_no_shadowing() { + #[derive( + Debug, serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output, + )] + struct RootOrderInsertData { + id: String, + } + + #[derive( + Debug, serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output, + )] + struct NsOrderInsertData { + name: String, + } + + #[derive( + Debug, serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output, + )] + struct TopWrapper { + root: RootOrderInsertData, + ns: NsOrderInsertData, + } + + #[derive( + Debug, serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output, + )] + struct OrderUpdateRequest { + data: NsOrderInsertData, + } + + async fn top_get(_s: S, _: reflectapi::Empty, _: reflectapi::Empty) -> TopWrapper { + unimplemented!() + } + async fn order_update( + _s: S, + _: OrderUpdateRequest, + _: reflectapi::Empty, + ) -> reflectapi::Empty { + reflectapi::Empty {} + } + + let (schema, _) = reflectapi::Builder::<()>::new() + .route(top_get, |b| b.name("top.get")) + .route(order_update, |b| b.name("myapi.order.update")) + .rename_types( + "reflectapi_demo::tests::namespace::TopWrapper", + "TopWrapper", + ) + .rename_types( + "reflectapi_demo::tests::namespace::OrderUpdateRequest", + "myapi::order::UpdateRequest", + ) + .rename_types( + "reflectapi_demo::tests::namespace::RootOrderInsertData", + "OrderInsertData", + ) + .rename_types( + "reflectapi_demo::tests::namespace::NsOrderInsertData", + "myapi::order::OrderInsertData", + ) + .build() + .unwrap(); + + let files = reflectapi::codegen::python::generate_files( + schema, + &reflectapi::codegen::python::Config::default(), + ) + .unwrap(); + + let types_py = files.get("_types.py").unwrap(); + let order_py = files.get("myapi/order/__init__.py").unwrap(); + + assert!(order_py.contains("class MyapiOrderInsertData(BaseModel")); + // The Rust-leaf alias must not rebind (shadow) the imported root type. + assert!( + !order_py.contains("OrderInsertData = MyapiOrderInsertData"), + "Rust-leaf alias must not shadow the imported root `OrderInsertData`:\n{order_py}" + ); + // The same-module field reference resolves to the disambiguated local class. + assert!( + order_py.contains("data: MyapiOrderInsertData"), + "same-module ref must use the disambiguated local class:\n{order_py}" + ); + // The cross-namespace reference from a root type is disambiguated too. + assert!( + types_py.contains("ns: myapi.order.MyapiOrderInsertData"), + "root cross-namespace ref must use the disambiguated name:\n{types_py}" + ); + assert!(!types_py.contains("ns: myapi.order.OrderInsertData")); +} + #[test] fn test_python_init_exports_client() { #[derive( diff --git a/reflectapi/src/codegen/python.rs b/reflectapi/src/codegen/python.rs index 3bd875e5..d01f6cc0 100644 --- a/reflectapi/src/codegen/python.rs +++ b/reflectapi/src/codegen/python.rs @@ -2130,21 +2130,17 @@ fn collect_collisions( by_module: &mut BTreeMap, BTreeMap>, ) { if !ns_path.is_empty() { - let flat_prefix = flat_namespace_prefix(ns_path); - let mut collisions = BTreeMap::new(); - for mt in &module.types { - for flat_name in extract_defined_python_names(&mt.rendered) { - let Some(stripped) = flat_name.strip_prefix(&flat_prefix) else { - continue; - }; - if !stripped.is_empty() - && stripped != flat_name - && root_type_names.contains(stripped) - { - collisions.insert(stripped.to_string(), flat_name.clone()); - } - } - } + // Derive collisions from the module's *actual* public aliases. + // `module_aliases` emits both a prefix-stripped alias and a Rust-leaf + // alias, and either can shadow a same-named root type imported via + // `from .._types import ...`. Checking only the stripped form would + // miss e.g. a namespaced `OrderInsertData` whose flat name de-stutters + // to `MyapiOrderInsertData`: there the *Rust-leaf* alias + // (`OrderInsertData = MyapiOrderInsertData`) is the colliding one. + let collisions: BTreeMap = module_aliases(module, ns_path) + .into_iter() + .filter(|(alias, flat)| alias != flat && root_type_names.contains(alias)) + .collect(); if !collisions.is_empty() { by_module.insert(ns_path.to_vec(), collisions); }