iter 22b.2.6: fix — structural Origin enum, fn-fn collision delegation, test path helper
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.
This commit is contained in:
@@ -258,9 +258,8 @@ pub enum WorkspaceLoadError {
|
|||||||
},
|
},
|
||||||
|
|
||||||
/// Iter 22b.2: a class-method name collides with another
|
/// Iter 22b.2: a class-method name collides with another
|
||||||
/// class-method or with a top-level fn. The Prelude reserves
|
/// class-method or with a top-level fn. `kind` is
|
||||||
/// names `show`, `eq`, `ne`, `lt`, `le`, `gt`, `ge` once 22b.4
|
/// `"class-class"` or `"class-fn"`.
|
||||||
/// lands. `kind` is `"class-class"` or `"class-fn"`.
|
|
||||||
#[error(
|
#[error(
|
||||||
"method name `{method}` collides ({kind}): defined in `{first_origin}` and `{second_origin}`"
|
"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
|
// class-method ↔ top-level-fn; fn-fn collisions are a separate
|
||||||
// concern surfaced by `CheckError::DuplicateDef` in `ailang-check`,
|
// concern surfaced by `CheckError::DuplicateDef` in `ailang-check`,
|
||||||
// not here.
|
// not here.
|
||||||
let mut method_origins: BTreeMap<String, String> = 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<String, Origin> = BTreeMap::new();
|
||||||
for (mod_name, m) in modules {
|
for (mod_name, m) in modules {
|
||||||
for def in &m.defs {
|
for def in &m.defs {
|
||||||
match def {
|
match def {
|
||||||
Def::Class(c) => {
|
Def::Class(c) => {
|
||||||
for method in &c.methods {
|
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) {
|
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 {
|
return Err(WorkspaceLoadError::MethodNameCollision {
|
||||||
method: method.name.clone(),
|
method: method.name.clone(),
|
||||||
kind: if prior.starts_with("class ") {
|
kind,
|
||||||
"class-class"
|
first_origin: prior.format(),
|
||||||
} else {
|
second_origin: origin.format(),
|
||||||
"class-fn"
|
|
||||||
},
|
|
||||||
first_origin: prior.clone(),
|
|
||||||
second_origin: origin,
|
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
method_origins.insert(method.name.clone(), origin);
|
method_origins.insert(method.name.clone(), origin);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
Def::Fn(f) => {
|
Def::Fn(f) => {
|
||||||
// fn-fn collisions (two `Def::Fn` with the same
|
let origin = Origin::Fn {
|
||||||
// name) are a separate concern: `ailang-check`
|
name: f.name.clone(),
|
||||||
// surfaces them as `CheckError::DuplicateDef`. The
|
module: mod_name.clone(),
|
||||||
// `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);
|
|
||||||
if let Some(prior) = method_origins.get(&f.name) {
|
if let Some(prior) = method_origins.get(&f.name) {
|
||||||
return Err(WorkspaceLoadError::MethodNameCollision {
|
// Only fire on class-fn collisions here. fn-fn
|
||||||
method: f.name.clone(),
|
// collisions (two `Def::Fn` with the same name)
|
||||||
kind: "class-fn",
|
// are `CheckError::DuplicateDef`'s job in
|
||||||
first_origin: prior.clone(),
|
// `ailang-check`; firing here with `kind:
|
||||||
second_origin: origin,
|
// "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.
|
/// no way to choose between them at a use site.
|
||||||
#[test]
|
#[test]
|
||||||
fn class_class_method_name_collision_fires() {
|
fn class_class_method_name_collision_fires() {
|
||||||
let entry = std::path::PathBuf::from(
|
let entry = examples_dir()
|
||||||
"../../examples/test_22b2_method_name_collision_class_class.ail.json",
|
.join("test_22b2_method_name_collision_class_class.ail.json");
|
||||||
);
|
|
||||||
let err = load_workspace(&entry)
|
let err = load_workspace(&entry)
|
||||||
.expect_err("must fire method-name-collision");
|
.expect_err("must fire method-name-collision");
|
||||||
match err {
|
match err {
|
||||||
WorkspaceLoadError::MethodNameCollision { method, kind, .. } => {
|
WorkspaceLoadError::MethodNameCollision {
|
||||||
|
method,
|
||||||
|
kind,
|
||||||
|
first_origin,
|
||||||
|
second_origin,
|
||||||
|
} => {
|
||||||
assert_eq!(method, "foo");
|
assert_eq!(method, "foo");
|
||||||
assert_eq!(kind, "class-class");
|
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:?}"),
|
other => panic!("expected MethodNameCollision, got {other:?}"),
|
||||||
}
|
}
|
||||||
@@ -1072,15 +1109,27 @@ mod tests {
|
|||||||
/// the worst possible failure mode.
|
/// the worst possible failure mode.
|
||||||
#[test]
|
#[test]
|
||||||
fn class_fn_method_name_collision_fires() {
|
fn class_fn_method_name_collision_fires() {
|
||||||
let entry = std::path::PathBuf::from(
|
let entry = examples_dir()
|
||||||
"../../examples/test_22b2_method_name_collision_class_fn.ail.json",
|
.join("test_22b2_method_name_collision_class_fn.ail.json");
|
||||||
);
|
|
||||||
let err = load_workspace(&entry)
|
let err = load_workspace(&entry)
|
||||||
.expect_err("must fire method-name-collision");
|
.expect_err("must fire method-name-collision");
|
||||||
match err {
|
match err {
|
||||||
WorkspaceLoadError::MethodNameCollision { method, kind, .. } => {
|
WorkspaceLoadError::MethodNameCollision {
|
||||||
|
method,
|
||||||
|
kind,
|
||||||
|
first_origin,
|
||||||
|
second_origin,
|
||||||
|
} => {
|
||||||
assert_eq!(method, "greet");
|
assert_eq!(method, "greet");
|
||||||
assert_eq!(kind, "class-fn");
|
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:?}"),
|
other => panic!("expected MethodNameCollision, got {other:?}"),
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user