From b252f837af799cfa3630e73959cd1b1e050254d4 Mon Sep 17 00:00:00 2001 From: Brummel Date: Sat, 9 May 2026 18:13:49 +0200 Subject: [PATCH] =?UTF-8?q?iter=2022b.2.6:=20fix=20=E2=80=94=20structural?= =?UTF-8?q?=20Origin=20enum,=20fn-fn=20collision=20delegation,=20test=20pa?= =?UTF-8?q?th=20helper?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address quality review of a9aa248: - Replace string-prefix discriminator (`prior.starts_with("class ")`) with a structural `Origin { Class | Fn }` enum; `kind` is now a `match` on variants, formatting is reserved for error construction. - Fix fn-fn collision misreport: in the `Def::Fn` arm, only fire `MethodNameCollision` when the prior origin is a class. Two `Def::Fn` with the same name are `CheckError::DuplicateDef`'s job in `ailang-check`, not this pre-pass. - Tests use `examples_dir()` like the rest of the module; path construction is no longer ad-hoc. - Tests assert `first_origin` / `second_origin` so an origin-format regression cannot silently break the `kind` discriminator. - Drop forward-looking 22b.4 prelude reservation note from the variant doc — keep only what's load-bearing for 22b.2. --- crates/ailang-core/src/workspace.rs | 121 +++++++++++++++++++--------- 1 file changed, 85 insertions(+), 36 deletions(-) diff --git a/crates/ailang-core/src/workspace.rs b/crates/ailang-core/src/workspace.rs index 0219566..6eaf627 100644 --- a/crates/ailang-core/src/workspace.rs +++ b/crates/ailang-core/src/workspace.rs @@ -258,9 +258,8 @@ pub enum WorkspaceLoadError { }, /// Iter 22b.2: a class-method name collides with another - /// class-method or with a top-level fn. The Prelude reserves - /// names `show`, `eq`, `ne`, `lt`, `le`, `gt`, `ge` once 22b.4 - /// lands. `kind` is `"class-class"` or `"class-fn"`. + /// class-method or with a top-level fn. `kind` is + /// `"class-class"` or `"class-fn"`. #[error( "method name `{method}` collides ({kind}): defined in `{first_origin}` and `{second_origin}`" )] @@ -386,47 +385,73 @@ fn build_registry( // class-method ↔ top-level-fn; fn-fn collisions are a separate // concern surfaced by `CheckError::DuplicateDef` in `ailang-check`, // not here. - let mut method_origins: BTreeMap = BTreeMap::new(); + // + // Origins are kept structural (the `Origin` enum below) so the + // `kind` discriminator is a `match` on variants rather than a + // string-prefix check on a display form. + enum Origin { + Class { class_name: String, module: String }, + Fn { name: String, module: String }, + } + impl Origin { + fn format(&self) -> String { + match self { + Origin::Class { class_name, module } => { + format!("class {class_name} (in {module})") + } + Origin::Fn { name, module } => format!("fn {name} (in {module})"), + } + } + } + + let mut method_origins: BTreeMap = BTreeMap::new(); for (mod_name, m) in modules { for def in &m.defs { match def { Def::Class(c) => { for method in &c.methods { - let origin = format!("class {} (in {mod_name})", c.name); + let origin = Origin::Class { + class_name: c.name.clone(), + module: mod_name.clone(), + }; if let Some(prior) = method_origins.get(&method.name) { + let kind = match prior { + Origin::Class { .. } => "class-class", + Origin::Fn { .. } => "class-fn", + }; return Err(WorkspaceLoadError::MethodNameCollision { method: method.name.clone(), - kind: if prior.starts_with("class ") { - "class-class" - } else { - "class-fn" - }, - first_origin: prior.clone(), - second_origin: origin, + kind, + first_origin: prior.format(), + second_origin: origin.format(), }); } method_origins.insert(method.name.clone(), origin); } } Def::Fn(f) => { - // fn-fn collisions (two `Def::Fn` with the same - // name) are a separate concern: `ailang-check` - // surfaces them as `CheckError::DuplicateDef`. The - // `kind` literal here is therefore `"class-fn"` - // unconditionally — by construction, the only - // collision this pass cares about is a class - // method shadowing (or being shadowed by) a free - // fn. - let origin = format!("fn {} (in {mod_name})", f.name); + let origin = Origin::Fn { + name: f.name.clone(), + module: mod_name.clone(), + }; if let Some(prior) = method_origins.get(&f.name) { - return Err(WorkspaceLoadError::MethodNameCollision { - method: f.name.clone(), - kind: "class-fn", - first_origin: prior.clone(), - second_origin: origin, - }); + // Only fire on class-fn collisions here. fn-fn + // collisions (two `Def::Fn` with the same name) + // are `CheckError::DuplicateDef`'s job in + // `ailang-check`; firing here with `kind: + // "class-fn"` would misreport. + if matches!(prior, Origin::Class { .. }) { + return Err(WorkspaceLoadError::MethodNameCollision { + method: f.name.clone(), + kind: "class-fn", + first_origin: prior.format(), + second_origin: origin.format(), + }); + } + // prior is a fn: skip; do not overwrite. + } else { + method_origins.insert(f.name.clone(), origin); } - method_origins.insert(f.name.clone(), origin); } _ => {} } @@ -1050,15 +1075,27 @@ mod tests { /// no way to choose between them at a use site. #[test] fn class_class_method_name_collision_fires() { - let entry = std::path::PathBuf::from( - "../../examples/test_22b2_method_name_collision_class_class.ail.json", - ); + let entry = examples_dir() + .join("test_22b2_method_name_collision_class_class.ail.json"); let err = load_workspace(&entry) .expect_err("must fire method-name-collision"); match err { - WorkspaceLoadError::MethodNameCollision { method, kind, .. } => { + WorkspaceLoadError::MethodNameCollision { + method, + kind, + first_origin, + second_origin, + } => { assert_eq!(method, "foo"); assert_eq!(kind, "class-class"); + assert!( + first_origin.starts_with("class A"), + "first_origin = {first_origin:?}", + ); + assert!( + second_origin.starts_with("class B"), + "second_origin = {second_origin:?}", + ); } other => panic!("expected MethodNameCollision, got {other:?}"), } @@ -1072,15 +1109,27 @@ mod tests { /// the worst possible failure mode. #[test] fn class_fn_method_name_collision_fires() { - let entry = std::path::PathBuf::from( - "../../examples/test_22b2_method_name_collision_class_fn.ail.json", - ); + let entry = examples_dir() + .join("test_22b2_method_name_collision_class_fn.ail.json"); let err = load_workspace(&entry) .expect_err("must fire method-name-collision"); match err { - WorkspaceLoadError::MethodNameCollision { method, kind, .. } => { + WorkspaceLoadError::MethodNameCollision { + method, + kind, + first_origin, + second_origin, + } => { assert_eq!(method, "greet"); assert_eq!(kind, "class-fn"); + assert!( + first_origin.starts_with("class Greet"), + "first_origin = {first_origin:?}", + ); + assert!( + second_origin.starts_with("fn greet"), + "second_origin = {second_origin:?}", + ); } other => panic!("expected MethodNameCollision, got {other:?}"), }