diff --git a/DESIGN.md b/DESIGN.md index 0c0f8ed..9b52a7b 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -893,7 +893,8 @@ checker already uses for built-in/in-file modules, so the existing `Field`-node cross-module reference with no new lookup logic. A module's interface is its top-level **`let` values** plus its **sum types** (since the cross-module-ADT follow-on; `ModuleExports` carries each public sum type's name, arity, and constructors) **and its records** (since the cross-module-record follow-on; -`ModuleExports` also carries each public record's name + fields). A consumer can construct (`Geometry.Circle +`ModuleExports` also carries each public record's name + fields) **and its opaque handle types** (name + +arity, so `extern type Rng` can be named across the boundary). A consumer can construct (`Geometry.Circle 2.0`) and pattern-match (`| Geometry.Circle r ->`, a qualified constructor pattern) the imported type's values, with **exhaustiveness checked across the boundary** (a missing arm reports the qualified witness `Geometry.Rect _ _`). **Records cross too** (`DESIGN.md` §8.3): construct `Geometry.Point { x = 1, y = 2 }`, @@ -904,10 +905,26 @@ an ADT variant payload or an `extern` signature. Both spellings are accepted, ba `Shapes.Placed`, and they denote the same type: an imported type registers under its bare identity name (unique across everything visible, as the import clash check enforces) plus a qualified key, and a written qualifier is validated and then folded back to that identity, so the two unify freely. Prefer the qualified -spelling where the reader benefits from knowing which module a type came from. The one thing this does not -extend to is *transitive* naming: a third module importing `Holder` without importing `Shapes` can hold and -pass its `item` around, but cannot access that value's fields or construct one, since only `Shapes` brings -`Placed`'s field registry into scope. **Externs and +spelling where the reader benefits from knowing which module a type came from. **Opaque handle types +(`extern type Rng`, §6) cross like any other type name:** the interface carries the name + arity, so a +consumer can write `Gen.Rng` (or bare `Rng`) in a record field, an ADT payload, or an `extern` signature; +values of the type always crossed through the exported schemes. **Interfaces close transitively over what +they reference:** a record or opaque type that an exported scheme, constructor, or record field mentions is +carried in the exporter's interface, tagged with the module that declares it, and the pull repeats through +each hop (a worklist walk with a seen set, so mutually referencing records terminate). A third module +importing `Holder` without importing `Shapes` can therefore hold its `item`, read that value's fields, and +name its type bare (`Placed`). A carried type crosses as an *identity*, not as a member: constructing it, +pattern-matching it, or writing the qualified spelling still requires importing the declaring module +directly. A carried name already taken here (by a local type or a direct import) is skipped silently rather +than reported, since the consumer never wrote that name: local and direct declarations win the bare name, +the same record arriving along two import paths is recognized by its declaring module and admitted once, +and a genuinely different type shadows the carried record, whose fields then feed the "unknown record +field" diagnostic (the message names the hidden record and the module that declares it). Field lookup +mirrors the same precedence: in a by-name field access (a base whose type is not yet known), records +declared here or imported directly form the deciding tier, and a carried record owns a field only when no +record in that tier declares it, so editing a dependency two hops away can never make a working access +ambiguous. Two carried records that alone declare a field still tie, and the ambiguity message names +them. **Externs and measures cross too:** an imported `extern` (`Mathx.cbrt`) is exported like a value (its scheme joins the interface) and — in the project lowering path — also **bound at top level in its own module** (`cbrt = math.cbrt`, `import math` hoisted) so a dependent module references it as `mathx.cbrt`; single-file lowering diff --git a/INTERNALS.md b/INTERNALS.md index 5743f3a..8d4a46d 100644 --- a/INTERNALS.md +++ b/INTERNALS.md @@ -430,8 +430,9 @@ returned topological order (dependencies first, entry last). `project::build_fro in the entry's directory). Cross-module *checking* and *emit* consume this `Project`. **Cross-module checking** (`types::check_module` + `project::check`). *Implementation:* the single-file `run` was generalized to take the imports map and return the module's -exported value schemes (which now include `extern` names), its exported sum types, **its exported records**, -**and its measures + measure-aliases**; `check_module(module, +`ModuleExports` interface: its exported value schemes (which include `extern` names), its exported sum +types, **its exported records**, **its opaque handle types** (`collect_exported_opaques` / +`ExportedOpaque`: name + arity, issue #72), **and its measures + measure-aliases**; `check_module(module, imports)` seeds imported values under qualified keys and imported sum types into the decls under **qualified constructor keys**. The type merge happens in **two halves**, because a local `type` declaration may name an imported type (`type Holder = { item: Shapes.Placed }`, DESIGN §6.1): `merge_imported_type_names` runs @@ -456,6 +457,30 @@ and leaked local variables into the "free in the env" set — blocking generaliz bindings, which then exported un-quantified and cascaded onward (#26). Every imported scheme is therefore alpha-renamed into the consumer's id space as it is seeded (`Infer::refresh_scheme`, values and constructors alike, in sorted order so the ids do not depend on `HashMap` iteration). +*Transitive interface closure* (issues #72/#75): before `run` hands back its `ModuleExports`, +`close_over_references` walks every exported scheme, constructor scheme, and record field for `Ty::Con` +names (`collect_con_names`) and pulls any name satisfied by an import's record/opaque tables into this +module's own exports, tagged `origin: Some(declaring module)` (own declarations carry `origin: None`); +newly pulled records are walked in turn, and a seen set terminates the walk on mutually referencing +records. Each import's interface is itself closed, so one pull per hop reaches any depth. On the consumer +side `merge_imported_type_names` registers carried entries **last** and under the **bare identity name +only** (no qualified key, no `record_aliases` entry, so constructing or pattern-matching a carried record +still requires the direct import); a taken bare name skips the carried entry silently (`record_home` +recognizes the same record arriving along two paths). By-name field lookup applies the same precedence in +tiers (`Infer::field_owner_tier`, consulted by `record_of_field` and the pending-field fallback): local +and directly imported records decide first, and a carried record owns a field only when no direct one +declares it, so a carried record can never make an existing access ambiguous; a tie between carried +records with no direct owner is still the ambiguity error, naming the tier that decided. A genuinely +shadowed record's fields land in +`Decls::field_hints`, which `record_of_field` / `record_of_field_on` render as "the record `Config` in +module `Inner` declares this field" instead of a bare unknown. Constructing or matching a carried record +bare is refused with the fix spelled out (`resolve_record_tag` via `Decls::carried_record_home`: "the +record `Config` is declared in module `Inner`; import `Inner` to construct its values here", the pattern +site says "to match"), while a name that is no record at all keeps "not a record type". The lowering side: +`ModuleExports::carried_records` feeds `project::compile`'s per-module `ImportContext::record_class_modules` +plus field data keyed by the *declaring* module's tag, so a record **update** in a consumer that never +imports the declaring module still reconstructs via the right class (`inner.Config(...)`, +`Lowerer::record_class_name` accepts those modules and hoists their imports). `project::check` threads the `ModuleExports` map through the topological order, seeding each module from only the modules it actually imports (so an unimported module's members/constructors stay invisible), and returns errors grouped by module. *Lowering* routes a qualified constructor — in expression or pattern diff --git a/src/lowering/mod.rs b/src/lowering/mod.rs index 315b6d0..2a2eac4 100644 --- a/src/lowering/mod.rs +++ b/src/lowering/mod.rs @@ -122,6 +122,12 @@ pub struct ImportContext { /// (`x` → `Geometry.Point`), so a cross-module update `{ p with x = 3 }` /// (which carries no tag) routes to the imported class. pub record_field_owners: HashMap, + /// Modules that declare a **transitively** visible record (`DESIGN.md` §6.1) + /// without being imported by this module directly. A record update on such a + /// record reconstructs via the declaring module's class (`inner.Config(...)`), + /// so its module must be recognized as a class source (and its Python import + /// hoisted) even though no `import` names it here. + pub record_class_modules: HashSet, } /// A module lowered as part of a multi-file project. @@ -149,6 +155,7 @@ pub fn lower_in_project( let mut lowerer = Lowerer::new(module); lowerer.float_literals = float_literals.clone(); lowerer.imported_modules = ctx.modules.clone(); + lowerer.record_class_modules = ctx.record_class_modules.clone(); lowerer.imported_nullary_ctors = ctx.nullary_ctors.clone(); lowerer .newtype_ctors @@ -318,6 +325,11 @@ struct Lowerer { /// A `Geometry.member` reference routes to Python `geometry.member` (vs the /// `Geometry_member` mangling used for in-file `module` declarations). imported_modules: HashSet, + /// Modules that declare a transitively visible record without being imported + /// here directly ([`ImportContext::record_class_modules`]). Consulted only by + /// [`Lowerer::record_class_name`], so nothing but a record reconstruction can + /// route through such a module. + record_class_modules: HashSet, /// Qualified names of imported nullary constructors (`Palette.Red`), referenced /// as values, which must lower to a call (`palette.Red()`) not the bare class. imported_nullary_ctors: HashSet, @@ -610,6 +622,7 @@ impl Lowerer { float_literals: HashSet::new(), cur_module: None, imported_modules: HashSet::new(), + record_class_modules: HashSet::new(), imported_nullary_ctors: HashSet::new(), use_runtime: false, project_mode: false, @@ -2850,10 +2863,13 @@ impl Lowerer { /// file module (`Geometry.Point`) becomes dotted attribute access on that module /// (`geometry.Point`, with `import geometry` hoisted) so it references the *same* /// class the module defines (the consumer never redefines it); a bare tag is the - /// record class name (mangled for the reserved `Exception`). + /// record class name (mangled for the reserved `Exception`). A tag rooted in a + /// module visible only through a transitively carried record + /// (`record_class_modules`) routes the same way — an update on such a record + /// reconstructs via the declaring module's class. fn record_class_name(&mut self, tag: &str) -> String { if let Some((base, rec)) = tag.split_once('.') - && self.imported_modules.contains(base) + && (self.imported_modules.contains(base) || self.record_class_modules.contains(base)) { let module = self.py_module_ref(base); format!("{module}.{}", py_record_class(rec)) diff --git a/src/project/mod.rs b/src/project/mod.rs index aa2c920..a988057 100644 --- a/src/project/mod.rs +++ b/src/project/mod.rs @@ -242,9 +242,11 @@ pub fn compile_targeting( // Per-module spans of integer literals that inference resolved to `float`, so // lowering emits them as `7.0`. Computed in topological order (like `check`) so // each module's imports are seeded before it — a literal's float-ness can depend - // on a cross-module call. Exports are threaded forward in the same pass. + // on a cross-module call. Exports are threaded forward in the same pass and kept: + // an import's interface also carries the records it references transitively + // (`DESIGN.md` §6.1), which the lowering context below needs. + let mut exports: HashMap = HashMap::new(); let float_spans: HashMap> = { - let mut exports: HashMap = HashMap::new(); let mut spans = HashMap::new(); for module in &project.modules { let imports: HashMap = module @@ -294,6 +296,26 @@ pub fn compile_targeting( } } } + // Records an import's interface carries transitively (declared in a + // module this one does not import directly): keyed by the *declaring* + // module's tag, so an update on such a record reconstructs via the right + // class (`inner.Config(...)`, with `import inner` hoisted). Registered + // after the direct entries, which keep precedence for shared field names. + for import in &module.imports { + let Some(exp) = exports.get(import) else { + continue; + }; + for (origin, rec, fields) in exp.carried_records() { + let tag = format!("{origin}.{rec}"); + for field in &fields { + ctx.record_field_owners + .entry(field.clone()) + .or_insert_with(|| tag.clone()); + } + ctx.record_fields.entry(tag).or_insert(fields); + ctx.record_class_modules.insert(origin); + } + } let floats = float_spans.get(&module.name).unwrap_or(&no_floats); let lowered = lowering::lower_in_project(&module.ast, &ctx, floats)?; needs_runtime |= lowered.uses_runtime; diff --git a/src/types/mod.rs b/src/types/mod.rs index c9d4705..6cbca94 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -1805,6 +1805,20 @@ struct Decls { /// name (`Point`), so a qualified construction/pattern resolves to the same /// `RecordInfo` (and `Ty::Con`) the exporting module uses. record_aliases: HashMap, + /// Bare identity name → declaring module for each record an import's + /// interface carries **transitively** and that registered here. Such a + /// record deliberately has no construction alias (constructing or matching + /// it requires importing its declaring module directly), so the tag + /// resolver uses this to say exactly that instead of the false + /// "not a record type". + carried_record_home: HashMap, + /// Field name → `(record, declaring module)` for each record an import's + /// interface carries **transitively** whose bare name is already taken here + /// (by a local type or another import), so the record itself stays out of + /// scope. Consulted only to improve the "unknown record field" / + /// "has no field" diagnostics: the field's owner is knowable even though the + /// record is not visible. + field_hints: HashMap>, /// User-declared in-file module names (`module Foo = …`), for the "X is a /// module" diagnostic on a bare reference. modules: HashSet, @@ -1817,8 +1831,7 @@ struct Decls { /// Type-check a whole module, returning every independent error found. pub fn check(module: &Module) -> Result<(), Vec> { - let (errors, _types, _schemes, _exports, _records, _measures, _holes, _ordered) = - run(module, false, &HashMap::new()); + let (errors, _types, _exports, _holes, _ordered) = run(module, false, &HashMap::new()); if errors.is_empty() { Ok(()) } else { @@ -1829,7 +1842,8 @@ pub fn check(module: &Module) -> Result<(), Vec> { /// One sum type a module exports (`DESIGN.md` §6.1): its name, type-parameter /// arity, and constructors (bare name + signature/arity). Carried in /// [`ModuleExports`] so an importing module can construct and pattern-match the -/// type's values. Records / measures / externs are not yet cross-module exported. +/// type's values. Records, measures, externs, and opaque handle types cross the +/// boundary through the other [`ModuleExports`] slots. #[derive(Clone)] struct ExportedType { name: String, @@ -1837,18 +1851,55 @@ struct ExportedType { ctors: Vec<(String, CtorInfo)>, } +/// One record a module's interface carries (`DESIGN.md` §8.3): its bare identity +/// name and its [`RecordInfo`] (params + fields). +#[derive(Clone)] +struct ExportedRecord { + name: String, + info: RecordInfo, + /// The module that **declares** the record: `None` when this module declares + /// it itself, `Some(module)` when the record is carried transitively because + /// an exported scheme or record field references it (`DESIGN.md` §6.1). + origin: Option, +} + +/// One opaque handle type (`extern type Rng`) a module's interface carries: its +/// name and type-parameter arity. There is nothing more to a handle type — no +/// constructors, no fields — so exporting the name is exporting the type. +#[derive(Clone)] +struct ExportedOpaque { + name: String, + arity: usize, + /// As on [`ExportedRecord`]: `None` for this module's own declaration, + /// `Some(module)` for one carried transitively from that module. + origin: Option, +} + /// A module's exported interface (`DESIGN.md` §6.1): its public top-level `let` /// values' schemes (keyed by **bare** name) and its public sum types. Opaque — /// the scheme/ctor representation is internal — produced by [`check_module`] and /// fed back in as a dependent module's imports. +/// +/// The record and opaque-type slots are **transitively closed**: they hold this +/// module's own declarations plus every record / opaque type its exported +/// schemes, constructors, and record fields reference out of its own imports' +/// interfaces (each tagged with the module that declares it). `Wrap` cannot +/// honestly be exported without the `Config` its field is typed with, or a +/// consumer two hops from `Config`'s module could hold a value whose fields it +/// cannot access and whose type it cannot name. #[derive(Clone, Default)] pub struct ModuleExports { schemes: Env, types: Vec, - /// Public **record** types (`DESIGN.md` §8.3): each bare name + its - /// [`RecordInfo`] (params + fields), so an importing module can construct, - /// pattern-match, update, and field-access the record via qualified tags. - records: Vec<(String, RecordInfo)>, + /// **Record** types (`DESIGN.md` §8.3) — own + transitively referenced — so + /// an importing module can construct, pattern-match, update, and + /// field-access a record via qualified tags (own records), and field-access + /// / name transitively carried ones. + records: Vec, + /// **Opaque handle types** (`extern type Rng`) — own + transitively + /// referenced — so an importing module can write the type's name in a + /// record field, an `extern` signature, or an ADT payload. + opaques: Vec, /// Public **base measure** names (`measure m`). Merged **unqualified** into a /// consumer's decls — there is no qualified unit syntax (`` is bare), so /// measures cross by name and erase at lowering (`DESIGN.md` §6.1). @@ -1858,6 +1909,27 @@ pub struct ModuleExports { measure_aliases: HashMap, } +impl ModuleExports { + /// The records this interface carries from **other** modules' declarations + /// (the transitively pulled entries): `(declaring module, record name, + /// declared field order)`. The project lowerer uses this to route a record + /// update in a consumer that never imports the declaring module directly — + /// the update must reconstruct via the declaring module's class. + pub(crate) fn carried_records(&self) -> Vec<(String, String, Vec)> { + self.records + .iter() + .filter_map(|rec| { + let origin = rec.origin.clone()?; + Some(( + origin, + rec.name.clone(), + rec.info.fields.iter().map(|(f, _)| f.clone()).collect(), + )) + }) + .collect() + } +} + /// Type-check `module` as one node of a multi-file project (`DESIGN.md` §6.1). /// /// `imports` maps each imported module's **name** to its [`ModuleExports`]; its @@ -1872,18 +1944,8 @@ pub fn check_module( module: &Module, imports: &HashMap, ) -> (Vec, ModuleExports) { - let (errors, _types, schemes, types, records, (measures, measure_aliases), _holes, _ordered) = - run(module, false, imports); - ( - errors, - ModuleExports { - schemes, - types, - records, - measures, - measure_aliases, - }, - ) + let (errors, _types, exports, _holes, _ordered) = run(module, false, imports); + (errors, exports) } /// Like [`check_module`] but also returns the span→type table (as @@ -1894,19 +1956,8 @@ pub fn check_module_collecting( module: &Module, imports: &HashMap, ) -> (Vec, Vec, ModuleExports) { - let (errors, types, schemes, tys, records, (measures, measure_aliases), _holes, _ordered) = - run(module, true, imports); - ( - errors, - types, - ModuleExports { - schemes, - types: tys, - records, - measures, - measure_aliases, - }, - ) + let (errors, types, exports, _holes, _ordered) = run(module, true, imports); + (errors, types, exports) } /// Like [`check_collecting`] but with imported modules' exports seeded @@ -1916,8 +1967,7 @@ pub fn check_collecting_with_imports( module: &Module, imports: &HashMap, ) -> (Vec, Vec, Vec) { - let (errors, types, _schemes, _exports, _records, _measures, holes, _ordered) = - run(module, true, imports); + let (errors, types, _exports, holes, _ordered) = run(module, true, imports); (errors, types, holes) } @@ -1929,8 +1979,7 @@ pub fn check_collecting_with_imports( pub fn check_collecting( module: &Module, ) -> (Vec, Vec, Vec, HashSet) { - let (errors, types, _schemes, _exports, _records, _measures, holes, ordered) = - run(module, true, &HashMap::new()); + let (errors, types, _exports, holes, ordered) = run(module, true, &HashMap::new()); (errors, types, holes, ordered) } @@ -1939,40 +1988,35 @@ pub fn check_collecting( /// node, which we resolve and render once inference is complete. `imports` pre-binds /// imported modules' interfaces (members under qualified keys, sum types under /// qualified constructor keys, for the multi-file driver); it is empty for a -/// single-file check. Returns the errors, the hover table, the module's exported -/// value schemes (top-level `let` bindings under their bare names), and its -/// exported sum types, and its exported records. +/// single-file check. Returns the errors, the hover table, and the module's +/// [`ModuleExports`] interface (value schemes, sum types, records, opaque handle +/// types, measures — transitively closed over what the interface references). type RunResult = ( Vec, Vec, - Env, - Vec, - Vec<(String, RecordInfo)>, - ExportedMeasures, + ModuleExports, Vec, // User type names the program compares (need ordering methods emitted). HashSet, ); -/// This module's own measures, for its export interface: base names and derived -/// aliases (`DESIGN.md` §6.1). -type ExportedMeasures = (HashSet, HashMap); - fn run(module: &Module, record: bool, imports: &HashMap) -> RunResult { let mut errors = Vec::new(); // `build_decls` registers imported type *names* mid-way through, so a local // `type` declaration can be written in terms of an imported type; it hands back // the names it accepted for the body merge below. let (mut decls, ctor_env, imported) = build_decls(module, imports, &mut errors); - // This module's own public sum types + records: `collect_exported_*` walk this - // module's own items, so only what it declares is exported. + // This module's own public sum types + records + opaque handle types: + // `collect_exported_*` walk this module's own items, so only what it declares + // starts here (the transitive closure below may add carried records/opaques). let exported_types = collect_exported_types(module, &decls); let exported_records = collect_exported_records(module, &decls); + let exported_opaques = collect_exported_opaques(module, &decls); // Captured before imports are merged, so we export only this module's own // measures. `decls.measures` holds base + alias names; `decls.measure_aliases` // holds the aliases' expansions. - let exported_measures: ExportedMeasures = - (decls.measures.clone(), decls.measure_aliases.clone()); + let exported_measures = decls.measures.clone(); + let exported_measure_aliases = decls.measure_aliases.clone(); // Merge imported modules' type *bodies* into the decls (qualified constructor // keys), so `Geometry.Circle` construction, qualified ctor patterns, and // exhaustiveness all resolve against the imported type. This half runs after @@ -2245,21 +2289,133 @@ fn run(module: &Module, record: bool, imports: &HashMap) .collect(); let ordered = std::mem::take(&mut inf.ordered); + let (exported_records, exported_opaques) = close_over_references( + &exports, + &exported_types, + exported_records, + exported_opaques, + imports, + ); ( errors, types, - exports, - exported_types, - exported_records, - exported_measures, + ModuleExports { + schemes: exports, + types: exported_types, + records: exported_records, + opaques: exported_opaques, + measures: exported_measures, + measure_aliases: exported_measure_aliases, + }, holes, ordered, ) } +/// Close a module's export interface over the records and opaque handle types it +/// references (`DESIGN.md` §6.1, issues #72/#75): any `Ty::Con` name mentioned by +/// an exported scheme, an exported constructor's scheme, or an exported record's +/// field type that is satisfied by an import's record/opaque tables joins this +/// module's own exports, tagged with the module that declares it. Newly pulled +/// records are walked in turn, so a chain of any depth crosses; the `seen` set +/// makes the walk terminate on cycles (mutually-referencing records are legal). +/// Each import's interface is itself closed, so one level of pulling per hop +/// reaches everything. Names not found in any import (builtins, type parameters, +/// imported sum types) are simply skipped. +fn close_over_references( + schemes: &Env, + types: &[ExportedType], + mut records: Vec, + mut opaques: Vec, + imports: &HashMap, +) -> (Vec, Vec) { + if imports.is_empty() { + return (records, opaques); + } + // Names this module's interface already provides need no pull. + let mut seen: HashSet = HashSet::new(); + seen.extend(types.iter().map(|t| t.name.clone())); + seen.extend(records.iter().map(|r| r.name.clone())); + seen.extend(opaques.iter().map(|o| o.name.clone())); + + let mut work: Vec = Vec::new(); + for scheme in schemes.values() { + collect_con_names(&scheme.ty, &mut work); + } + for ty in types { + for (_, info) in &ty.ctors { + collect_con_names(&info.scheme.ty, &mut work); + } + } + for rec in &records { + for (_, field_ty) in &rec.info.fields { + collect_con_names(field_ty, &mut work); + } + } + + // Sorted so the pull is deterministic when two imports carry the same name + // (they then carry the same record, so the choice only fixes iteration order). + let mut import_names: Vec<&String> = imports.keys().collect(); + import_names.sort(); + while let Some(name) = work.pop() { + if !seen.insert(name.clone()) { + continue; + } + for module_name in &import_names { + let exp = &imports[*module_name]; + if let Some(rec) = exp.records.iter().find(|r| r.name == name) { + let origin = rec.origin.clone().unwrap_or_else(|| (*module_name).clone()); + for (_, field_ty) in &rec.info.fields { + collect_con_names(field_ty, &mut work); + } + records.push(ExportedRecord { + name: name.clone(), + info: rec.info.clone(), + origin: Some(origin), + }); + break; + } + if let Some(op) = exp.opaques.iter().find(|o| o.name == name) { + let origin = op.origin.clone().unwrap_or_else(|| (*module_name).clone()); + opaques.push(ExportedOpaque { + name: name.clone(), + arity: op.arity, + origin: Some(origin), + }); + break; + } + } + } + (records, opaques) +} + +/// Collect the name of every applied type constructor in `ty`, at any depth +/// (through arrows, tuples, and constructor arguments). +fn collect_con_names(ty: &Ty, out: &mut Vec) { + match ty { + Ty::Con(name, args) => { + out.push(name.clone()); + for a in args { + collect_con_names(a, out); + } + } + Ty::Fun(a, b, _) => { + collect_con_names(a, out); + collect_con_names(b, out); + } + Ty::Tuple(elems) => { + for e in elems { + collect_con_names(e, out); + } + } + Ty::Int(_) | Ty::Float(_) | Ty::Bool | Ty::Str | Ty::Unit | Ty::Num(..) | Ty::Var(_) => {} + } +} + /// Capture a module's own public **sum** types from the freshly-built decls, for -/// its export interface (`DESIGN.md` §6.1). Records / measures / externs are not -/// yet cross-module exported. +/// its export interface (`DESIGN.md` §6.1). Records, opaque handle types, and +/// measures are captured by their own collectors below; externs export like +/// values (their schemes join the env). fn collect_exported_types(module: &Module, decls: &Decls) -> Vec { let mut out = Vec::new(); for item in &module.items { @@ -2293,7 +2449,7 @@ fn collect_exported_types(module: &Module, decls: &Decls) -> Vec { /// (`DESIGN.md` §8.3), for its export interface: each bare name + its [`RecordInfo`] /// (params + fields). The reserved `Exception` record is seeded, not user-declared, /// so it is never exported. -fn collect_exported_records(module: &Module, decls: &Decls) -> Vec<(String, RecordInfo)> { +fn collect_exported_records(module: &Module, decls: &Decls) -> Vec { let mut out = Vec::new(); for item in &module.items { let Item::Type(decl) = item else { continue }; @@ -2301,20 +2457,50 @@ fn collect_exported_records(module: &Module, decls: &Decls) -> Vec<(String, Reco continue; } if let Some(info) = decls.records.get(&decl.name) { - out.push((decl.name.clone(), info.clone())); + out.push(ExportedRecord { + name: decl.name.clone(), + info: info.clone(), + origin: None, + }); } } out } -/// Register imported modules' sum-type and record **names** in `decls` -/// (`DESIGN.md` §6.1 + §8.3): each under its **bare identity name** (`Point`, so an -/// imported type is the same `Ty::Con` everywhere) *and* under a qualified key -/// (`Geometry.Point`) at the same arity, which [`resolve`] accepts in a type -/// position and folds back to the identity name. A name clashing with one already -/// present is reported (the bare-name uniqueness sum types already rely on) and -/// then skipped, so the returned set of accepted `(module, name)` pairs is what -/// [`merge_imported_types`] may go on to fill in. +/// Capture a module's own **opaque handle types** (`extern type Rng`) from the +/// freshly-built decls, for its export interface (`DESIGN.md` §6.1 / issue #72): +/// each name + arity, so an importing module can write `Gen.Rng` (or bare `Rng`) +/// in a type position. Values of the type already cross through the exported +/// schemes; this makes the *name* cross with them. +fn collect_exported_opaques(module: &Module, decls: &Decls) -> Vec { + let mut out = Vec::new(); + for item in &module.items { + let Item::Type(decl) = item else { continue }; + if !matches!(decl.kind, TypeDeclKind::Opaque) { + continue; + } + if let Some(&arity) = decls.type_arity.get(&decl.name) { + out.push(ExportedOpaque { + name: decl.name.clone(), + arity, + origin: None, + }); + } + } + out +} + +/// Register imported modules' sum-type, record, and opaque-handle-type **names** +/// in `decls` (`DESIGN.md` §6.1 + §8.3): each under its **bare identity name** +/// (`Point`, so an imported type is the same `Ty::Con` everywhere) *and* under a +/// qualified key (`Geometry.Point`) at the same arity, which [`resolve`] accepts +/// in a type position and folds back to the identity name. A directly imported +/// name clashing with one already present is reported (the bare-name uniqueness +/// sum types already rely on) and then skipped, so the returned set of accepted +/// `(module, name)` pairs is what [`merge_imported_types`] may go on to fill in. +/// Records and opaque types an import carries **transitively** register last, +/// under the bare identity name only, and a taken name skips them silently — see +/// the pass comments below. /// /// This runs from [`build_decls`] **between** its two passes: after local type /// names are registered (so a local declaration still wins a clash) and before @@ -2351,8 +2537,18 @@ fn merge_imported_type_names( } } // Records after types (both share the `type_arity` namespace and clash check). + // Only an import's **own** records here (`origin: None`); the transitively + // carried ones follow below, after every directly named declaration, so a + // direct import always wins the bare name. `record_home` tracks each accepted + // record's declaring module so the same record arriving along two paths is + // recognized as one. + let mut record_home: HashMap = HashMap::new(); for module_name in &module_names { - for (name, info) in &imports[*module_name].records { + for rec in &imports[*module_name].records { + if rec.origin.is_some() { + continue; + } + let name = &rec.name; if decls.type_arity.contains_key(name) { errors.push(TypeError { message: format!( @@ -2362,11 +2558,81 @@ fn merge_imported_type_names( }); continue; } - decls.type_arity.insert(name.clone(), info.params_count); + decls.type_arity.insert(name.clone(), rec.info.params_count); decls .type_arity - .insert(format!("{module_name}.{name}"), info.params_count); + .insert(format!("{module_name}.{name}"), rec.info.params_count); accepted.insert(((*module_name).clone(), name.clone())); + record_home.insert(name.clone(), (*module_name).clone()); + } + } + // An import's own opaque handle types (`extern type Rng`): name + arity under + // the bare identity and a qualified key, exactly like a sum type — there is + // nothing more to a handle type, so this is the whole export (issue #72). + for module_name in &module_names { + for op in &imports[*module_name].opaques { + if op.origin.is_some() { + continue; + } + if decls.type_arity.contains_key(&op.name) { + errors.push(TypeError { + message: format!( + "imported type `{}` (from `{module_name}`) clashes with an existing type", + op.name + ), + span: Span::new(0, 0), + }); + continue; + } + decls.type_arity.insert(op.name.clone(), op.arity); + decls + .type_arity + .insert(format!("{module_name}.{}", op.name), op.arity); + } + } + // Transitively carried records (`origin: Some`, `DESIGN.md` §6.1): declared in + // a module this one does not import directly, referenced by an import's + // interface. They register under the **bare identity name only** — no + // qualified key, no construction alias — so the type can be named, unified, + // and field-accessed, while constructing or pattern-matching it still asks + // for a direct import of the declaring module. A bare name already taken is + // **not** an error (the consumer never asked for this name): the same record + // arriving along another path is already in, and a genuinely different type + // keeps the name — local and direct declarations win — while the hidden + // record's fields feed the unknown-field hints. + for module_name in &module_names { + for rec in &imports[*module_name].records { + let Some(origin) = &rec.origin else { continue }; + let name = &rec.name; + if decls.type_arity.contains_key(name) { + if record_home.get(name) == Some(origin) { + continue; // the same record, already visible along another path + } + for (field, _) in &rec.info.fields { + let hint = (name.clone(), origin.clone()); + let hints = decls.field_hints.entry(field.clone()).or_default(); + if !hints.contains(&hint) { + hints.push(hint); + } + } + continue; + } + decls.type_arity.insert(name.clone(), rec.info.params_count); + accepted.insert(((*module_name).clone(), name.clone())); + record_home.insert(name.clone(), origin.clone()); + decls + .carried_record_home + .insert(name.clone(), origin.clone()); + } + } + // Transitively carried opaque handle types: bare identity name only, and a + // taken name is skipped silently (there are no fields to hint about). + for module_name in &module_names { + for op in &imports[*module_name].opaques { + if op.origin.is_none() || decls.type_arity.contains_key(&op.name) { + continue; + } + decls.type_arity.insert(op.name.clone(), op.arity); } } accepted @@ -2400,23 +2666,38 @@ fn merge_imported_types( decls.type_ctors.insert(ty.name.clone(), ctor_names); } } - for module_name in &module_names { - for (name, info) in &imports[*module_name].records { - if !accepted.contains(&((*module_name).clone(), name.clone())) { - continue; - } - decls.records.insert(name.clone(), info.clone()); - decls - .record_aliases - .insert(format!("{module_name}.{name}"), name.clone()); - // Fields join the multimap under the record's bare identity name. Local - // records were registered first (in `build_decls`), so they lead. - for (field, _) in &info.fields { - decls - .field_owner - .entry(field.clone()) - .or_default() - .push(name.clone()); + // Directly imported records first, then transitively carried ones, so the + // field multimap keeps a stable order: local records (from `build_decls`), + // then direct imports by module name, then carried records. A carried record + // joins `records` and `field_owner`, but in a by-name field lookup it is a + // **fallback tier**: a field it declares resolves to it only when no local or + // directly imported record declares that field (`field_owner_tier`), so a + // carried record can never make an existing access ambiguous. It also gets + // **no** qualified construction alias: constructing or pattern-matching it + // still requires importing its declaring module directly. + for transitive in [false, true] { + for module_name in &module_names { + for rec in &imports[*module_name].records { + if rec.origin.is_some() != transitive + || !accepted.contains(&((*module_name).clone(), rec.name.clone())) + { + continue; + } + let name = &rec.name; + decls.records.insert(name.clone(), rec.info.clone()); + if rec.origin.is_none() { + decls + .record_aliases + .insert(format!("{module_name}.{name}"), name.clone()); + } + // Fields join the multimap under the record's bare identity name. + for (field, _) in &rec.info.fields { + decls + .field_owner + .entry(field.clone()) + .or_default() + .push(name.clone()); + } } } } @@ -7042,15 +7323,11 @@ impl Infer { Ok(Ty::Unit) } - /// The ambiguity error, naming both ways out. The parameter form is usually the + /// The ambiguity error, naming both ways out. `owners` is the tier that + /// decided the lookup ([`Infer::field_owner_tier`]), so the message names + /// only the records that actually competed. The parameter form is usually the /// nicer one and is easy to miss, since it arrived after the message did. - fn ambiguous_field(&self, field: &str, span: Span) -> TypeError { - let owners = self - .decls - .field_owner - .get(field) - .cloned() - .unwrap_or_default(); + fn ambiguous_field(&self, field: &str, owners: &[String], span: Span) -> TypeError { let names = owners .iter() .map(|r| format!("`{r}`")) @@ -7069,7 +7346,10 @@ impl Infer { } /// Whether this field name is declared by more than one visible record, so an - /// access on an unknown base cannot be resolved by name alone. + /// access on an unknown base cannot be resolved by name alone. Deliberately + /// counts **all** owners, carried included: deferring the access keeps the + /// chance to resolve it by the base's actual type once inference pins it, + /// which beats any by-name precedence. fn field_is_ambiguous(&self, field: &str) -> bool { self.decls .field_owner @@ -7077,27 +7357,60 @@ impl Infer { .is_some_and(|owners| owners.len() >= 2) } + /// The record names competing to own `field` in a **by-name** lookup, tiered + /// (`DESIGN.md` §6.1): records this module declares or imports directly + /// decide first, and records carried transitively are a fallback consulted + /// only when no direct one declares the field. This mirrors the bare-name + /// rule for carried type names (local and direct declarations win, since the + /// consumer never wrote the carried name): without the tier, editing a + /// dependency two hops away could make a working access ambiguous. + fn field_owner_tier(&self, field: &str) -> Vec { + let owners = self + .decls + .field_owner + .get(field) + .map(Vec::as_slice) + .unwrap_or(&[]); + let direct: Vec = owners + .iter() + .filter(|o| !self.decls.carried_record_home.contains_key(*o)) + .cloned() + .collect(); + if direct.is_empty() { + owners.to_vec() + } else { + direct + } + } + /// Settle every deferred field access, now that the enclosing binding is fully /// inferred. A base that some later statement pinned down resolves exactly as it - /// would have at the access site; one that is *still* unknown is the genuine - /// ambiguity, and is reported at the access with the ways out. + /// would have at the access site; one that is *still* unknown falls back to the + /// by-name lookup with its owner tiers ([`Infer::record_of_field`]), so a field + /// whose only competition is a carried record still resolves to the direct one, + /// and only a genuine within-tier tie is reported as the ambiguity. fn resolve_pending_fields(&mut self) -> Result<(), TypeError> { let pending = std::mem::take(&mut self.pending_fields); for p in pending { let base = self.apply(&p.base); - let Ty::Con(record, _) = &base else { - return Err(self.ambiguous_field(&p.field, p.span)); - }; - let Some(info) = self.decls.records.get(record) else { - return Err(self.ambiguous_field(&p.field, p.span)); + let owner = if let Ty::Con(record, _) = &base { + let Some(info) = self.decls.records.get(record) else { + return Err(self.ambiguous_field( + &p.field, + &self.field_owner_tier(&p.field), + p.span, + )); + }; + if !info.fields.iter().any(|(n, _)| *n == p.field) { + return Err(TypeError { + message: format!("record `{record}` has no field `{}`", p.field), + span: p.span, + }); + } + record.clone() + } else { + self.record_of_field(&p.field, p.span)? }; - if !info.fields.iter().any(|(n, _)| *n == p.field) { - return Err(TypeError { - message: format!("record `{record}` has no field `{}`", p.field), - span: p.span, - }); - } - let owner = record.clone(); let (record_ty, field_tys) = self.instantiate_record(&owner); self.unify(&record_ty, &base, p.span)?; let fty = field_tys @@ -7139,8 +7452,9 @@ impl Infer { .filter(|&(d, n)| d <= (field.chars().count().max(n.chars().count()) / 3).max(2)) .map(|(_, n)| format!(" (did you mean `{n}`?)")) .unwrap_or_default(); + let hidden = self.hidden_field_hint(field); return Err(TypeError { - message: format!("record `{rec}` has no field `{field}`{near}"), + message: format!("record `{rec}` has no field `{field}`{near}{hidden}"), span, }); } @@ -7148,15 +7462,26 @@ impl Infer { } /// The bare identity name of the record type owning `field`, **by name alone** - /// (`DESIGN.md` §8.3) — the fallback for a base whose type is not yet known: - /// **0** owners is an unknown field, **1** owner resolves, **2+** is an ambiguity - /// error *at this access site*. Ambiguity is never an error at declaration or - /// import; module isolation is preserved. + /// (`DESIGN.md` §8.3) — the fallback for a base whose type is not yet known. + /// Owners are consulted in tiers ([`Infer::field_owner_tier`]): within the + /// deciding tier, **0** owners is an unknown field, **1** owner resolves, + /// **2+** is an ambiguity error *at this access site*. Ambiguity is never an + /// error at declaration or import; module isolation is preserved. fn record_of_field(&self, field: &str, span: Span) -> Result { - match self.decls.field_owner.get(field).map(Vec::as_slice) { - Some([only]) => Ok(only.clone()), - Some(owners) if owners.len() >= 2 => Err(self.ambiguous_field(field, span)), + match self.field_owner_tier(field).as_slice() { + [only] => Ok(only.clone()), + owners if owners.len() >= 2 => Err(self.ambiguous_field(field, owners, span)), _ => { + // A record carried transitively but shadowed by a same-named type + // is out of scope, yet its interface data still says where the + // field lives — say so instead of a dead-end "unknown". + let hidden = self.hidden_field_hint(field); + if !hidden.is_empty() { + return Err(TypeError { + message: format!("unknown record field `{field}`{hidden}"), + span, + }); + } // Empty `decls.records` means records aren't in use at all. let hint = if self.decls.records.is_empty() { " (no record types are declared)" @@ -7171,13 +7496,31 @@ impl Infer { } } + /// A rendered note when `field` is declared by a record the consumer's + /// interface data knows of but that is **not in scope here** (a transitively + /// carried record whose bare name a local or directly imported type already + /// holds — `Decls::field_hints`). Empty when there is nothing to say, so the + /// plain messages stay as they are. + fn hidden_field_hint(&self, field: &str) -> String { + match self.decls.field_hints.get(field).and_then(|h| h.first()) { + Some((rec, module)) => format!( + "; the record `{rec}` in module `{module}` declares this field, but another \ + type named `{rec}` is already in scope here, so that record is hidden" + ), + None => String::new(), + } + } + /// Resolve a surface record tag (`Point` or `Geometry.Point`) at a construction /// or pattern site to its **bare identity name** (`DESIGN.md` §8.3). A qualified /// tag resolves via the imported-record alias table; a bare tag resolves only to /// a **local** record (an imported record must be tagged qualified, exactly as an - /// imported sum-type constructor must be). Anything else is "not a record type" + /// imported sum-type constructor must be). A bare tag naming a **transitively + /// carried** record is a record type, just not one this module may construct or + /// match, so the error says so and names the module to import (`verb` is the + /// site's word: "construct" or "match"). Anything else is "not a record type" /// (or "not a member of `M`" for an unknown qualified tag). - fn resolve_record_tag(&self, tag: &str, span: Span) -> Result { + fn resolve_record_tag(&self, tag: &str, span: Span, verb: &str) -> Result { if let Some((module, rec)) = tag.split_once('.') { if let Some(bare) = self.decls.record_aliases.get(tag) { return Ok(bare.clone()); @@ -7189,6 +7532,14 @@ impl Infer { } if self.decls.local_records.contains(tag) { Ok(tag.to_string()) + } else if let Some(home) = self.decls.carried_record_home.get(tag) { + Err(TypeError { + message: format!( + "the record `{tag}` is declared in module `{home}`; import `{home}` \ + to {verb} its values here" + ), + span, + }) } else { Err(TypeError { message: format!("`{tag}` is not a record type"), @@ -7244,7 +7595,7 @@ impl Infer { span: Span, env: &Env, ) -> Result { - let owner = self.resolve_record_tag(ty, ty_span)?; + let owner = self.resolve_record_tag(ty, ty_span, "construct")?; let (record_ty, field_tys) = self.instantiate_record(&owner); let mut seen: HashSet = HashSet::new(); @@ -7690,7 +8041,7 @@ impl Infer { ty_span, fields, } => { - let owner = self.resolve_record_tag(ty, ty_span.span())?; + let owner = self.resolve_record_tag(ty, ty_span.span(), "match")?; let (record_ty, field_tys) = self.instantiate_record(&owner); self.unify(&record_ty, scrut_ty, span)?; let mut seen: HashSet = HashSet::new(); diff --git a/tests/project.rs b/tests/project.rs index 1a52447..ffb376c 100644 --- a/tests/project.rs +++ b/tests/project.rs @@ -879,6 +879,513 @@ fn a_cross_module_newtype_stays_distinct_from_its_underlying() { ); } +// ---------- transitive interface closure (issues #72 / #75) ---------- + +/// The three-module shape of issue #75: `Inner` declares a record, `Middle` +/// wraps it, and the consumer imports only `Middle`. +const INNER: &str = "type Config = { cSize: int }\nlet make n = Config { cSize = n }"; +const MIDDLE: &str = "import Inner\n\ + type Wrap = { wConf: Inner.Config }\n\ + let mk n = Wrap { wConf = Inner.make n }\n\ + let size w = w.wConf.cSize"; + +#[test] +fn e2e_an_extern_type_can_be_named_across_the_module_boundary() { + // issue #72: the *name* of an `extern type` crosses with its module's + // interface, so a consumer can type a record field `Gen.Rng` (values of the + // type always crossed through the imported schemes). + let files = compile( + "Main", + &[ + ( + "Gen", + "extern type Rng\n\ + extern newRng : int -> Rng = random.Random\n\ + let make seed = newRng seed", + ), + ( + "Main", + "import Gen\n\ + type Holder = { h: Gen.Rng }\n\ + let x = Holder { h = Gen.make 1 }\n\ + print \"ok\"", + ), + ], + ); + let dir = Scratch::new("e2e_extern_type_name"); + if let Some(out) = run_project(&dir, &files, "main.py") { + assert_eq!(out.trim(), "ok"); + } +} + +#[test] +fn an_unknown_type_qualified_to_an_opaque_exporting_module_reports_once() { + // A genuinely unknown qualified type still reports cleanly — one error, no + // cascade — now that the module also exports opaque type names. + let project = build_mem( + "Main", + &[ + ( + "Gen", + "extern type Rng\nextern newRng : int -> Rng = random.Random", + ), + ("Main", "import Gen\ntype Holder = { h: Gen.Nope }"), + ], + ); + let errors = project::check(&project); + assert_eq!(errors.len(), 1); + assert_eq!(errors[0].errors.len(), 1, "got: {:?}", errors[0].errors); + assert!( + errors[0].errors[0] + .message + .contains("unknown type `Gen.Nope`"), + "got: {}", + errors[0].errors[0].message + ); +} + +#[test] +fn an_opaque_type_carried_transitively_can_be_named_bare() { + // `Middle.fresh : int -> Rng` mentions Gen's handle type, so `Rng` crosses + // with Middle's interface and the consumer can name it (bare — constructing + // or qualifying still asks for the declaring module). + let project = build_mem( + "Main", + &[ + ( + "Gen", + "extern type Rng\n\ + extern newRng : int -> Rng = random.Random\n\ + let make seed = newRng seed", + ), + ("Middle", "import Gen\nlet fresh n = Gen.make n"), + ( + "Main", + "import Middle\n\ + type H = { h: Rng }\n\ + let x = H { h = Middle.fresh 1 }", + ), + ], + ); + let errors = project::check(&project); + assert!(errors.is_empty(), "unexpected errors: {errors:?}"); +} + +#[test] +fn a_local_type_clashing_with_an_imported_opaque_type_is_reported() { + // A handle type now registers like any imported type, so a same-named local + // declaration is the ordinary import clash (it would otherwise silently + // conflate two distinct nominal types). + let project = build_mem( + "Main", + &[ + ( + "Gen", + "extern type Rng\nextern newRng : int -> Rng = random.Random", + ), + ("Main", "import Gen\ntype Rng = A | B\nlet x = A"), + ], + ); + let errors = project::check(&project); + assert_eq!(errors.len(), 1); + assert!( + errors[0].errors[0] + .message + .contains("imported type `Rng` (from `Gen`) clashes"), + "got: {}", + errors[0].errors[0].message + ); +} + +#[test] +fn e2e_a_transitive_record_field_is_accessible() { + // issue #75: `Wrap`'s field is typed `Inner.Config`, so `Config` crosses + // with Middle's interface and the consumer can read `x.wConf.cSize` without + // importing `Inner` directly. + let files = compile( + "Main", + &[ + ("Inner", INNER), + ("Middle", MIDDLE), + ( + "Main", + "import Middle\nlet x = Middle.mk 5\nprint x.wConf.cSize", + ), + ], + ); + let dir = Scratch::new("e2e_transitive_field"); + if let Some(out) = run_project(&dir, &files, "main.py") { + assert_eq!(out.trim(), "5"); + } +} + +#[test] +fn e2e_a_record_update_through_a_transitive_boundary_routes_to_the_declaring_class() { + // An update reconstructs the record, so the consumer's emitted Python must + // reference the *declaring* module's class (`inner.Config`), with its import + // hoisted, even though the consumer never writes `import Inner`. + let files = compile( + "Main", + &[ + ("Inner", INNER), + ("Middle", MIDDLE), + ( + "Main", + "import Middle\n\ + let x = Middle.mk 5\n\ + let y = { x with wConf.cSize = 9 }\n\ + let z = { x.wConf with cSize = 7 }\n\ + print y.wConf.cSize\n\ + print z.cSize", + ), + ], + ); + let main = file(&files, "main.py"); + assert!(main.contains("import inner"), "{main}"); + assert!(main.contains("inner.Config("), "{main}"); + let dir = Scratch::new("e2e_transitive_update"); + if let Some(out) = run_project(&dir, &files, "main.py") { + assert_eq!(out.replace("\r\n", "\n").trim(), "9\n7"); + } +} + +#[test] +fn e2e_a_transitive_record_crosses_any_depth() { + // The record is declared three imports away; each hop's interface carries it + // onward, so the chain works at any depth. + let files = compile( + "Main", + &[ + ( + "Deep", + "type Core = { cv: int }\nlet mkCore n = Core { cv = n }", + ), + ( + "Mid", + "import Deep\n\ + type Two = { w1: Deep.Core }\n\ + let mk2 n = Two { w1 = Deep.mkCore n }", + ), + ( + "Top", + "import Mid\n\ + type Three = { w2: Mid.Two }\n\ + let mk3 n = Three { w2 = Mid.mk2 n }", + ), + ("Main", "import Top\nlet x = Top.mk3 7\nprint x.w2.w1.cv"), + ], + ); + let dir = Scratch::new("e2e_transitive_depth"); + if let Some(out) = run_project(&dir, &files, "main.py") { + assert_eq!(out.trim(), "7"); + } +} + +#[test] +fn mutually_referencing_records_cross_transitively() { + // Two records that reference each other (via Option) are legal; the export + // closure must follow the cycle and terminate rather than recurse forever. + let project = build_mem( + "Main", + &[ + ( + "Inner", + "type A = { av: int, ab: Option B }\n\ + type B = { bv: int, ba: Option A }\n\ + let mkA n = A { av = n, ab = None }\n\ + let mkB n = B { bv = n, ba = Some (mkA n) }", + ), + ("Middle", "import Inner\nlet make n = Inner.mkB n"), + ( + "Main", + "import Middle\n\ + let b = Middle.make 3\n\ + let v = b.bv\n\ + let w = Option.map (fun a -> a.av) b.ba", + ), + ], + ); + let errors = project::check(&project); + assert!(errors.is_empty(), "unexpected errors: {errors:?}"); +} + +#[test] +fn a_direct_record_wins_a_field_name_over_a_carried_one() { + // A local record and a transitively carried one both declare `cSize`. Carried + // records are a fallback tier for by-name field lookup (mirroring the + // bare-name rule: local and direct declarations win, the consumer never + // wrote the carried name), so the access resolves to `Local` and stays + // unambiguous — a record in a module this file cannot see must not be able + // to break a working access. + let project = build_mem( + "Main", + &[ + ("Inner", INNER), + ("Middle", MIDDLE), + ( + "Main", + "import Middle\n\ + type Local = { cSize: bool, other: int }\n\ + let get w = w.cSize\n\ + let v = get (Local { cSize = true, other = 1 })", + ), + ], + ); + let errors = project::check(&project); + assert!(errors.is_empty(), "unexpected errors: {errors:?}"); +} + +#[test] +fn e2e_a_carried_record_cannot_break_a_working_field_access() { + // The full breaking shape: the consumer's program never names `Inner`, so a + // shared field name there must not reach its `p.size` — the access keeps + // resolving to the local record and the program keeps running. + let files = compile( + "Main", + &[ + ( + "Inner", + "type Config = { size: int }\nlet make n = Config { size = n }", + ), + ( + "Middle", + "import Inner\n\ + type Wrap = { wConf: Inner.Config }\n\ + let mk n = Wrap { wConf = Inner.make n }", + ), + ( + "Main", + "import Middle\n\ + type Local = { size: bool }\n\ + let f p = p.size\n\ + print (f (Local { size = true }))", + ), + ], + ); + let dir = Scratch::new("e2e_tiered_field"); + if let Some(out) = run_project(&dir, &files, "main.py") { + assert_eq!(out.trim(), "True"); + } +} + +#[test] +fn two_carried_records_sharing_a_field_are_still_ambiguous() { + // With nothing local or direct declaring `size`, the carried tier decides + // the lookup, and a tie within it is the genuine ambiguity — the message + // names the carried records. + let project = build_mem( + "Main", + &[ + ( + "Twin", + "type A = { size: int }\n\ + type B = { size: bool }\n\ + let mkA n = A { size = n }\n\ + let mkB b = B { size = b }", + ), + ( + "Carry", + "import Twin\n\ + type WrapA = { wa: Twin.A }\n\ + type WrapB = { wb: Twin.B }\n\ + let mka n = WrapA { wa = Twin.mkA n }\n\ + let mkb b = WrapB { wb = Twin.mkB b }", + ), + ("Main", "import Carry\nlet get w = w.size"), + ], + ); + let errors = project::check(&project); + assert_eq!(errors.len(), 1); + let message = &errors[0].errors[0].message; + assert!( + message.contains("ambiguous") && message.contains("`A`") && message.contains("`B`"), + "got: {message}" + ); +} + +#[test] +fn a_transitive_record_shadowed_by_a_local_name_hints_at_its_home() { + // A local type already holds the name `Config`, so the carried record stays + // out of scope (local and direct declarations win the bare name, and the + // consumer never asked for the carried one — no clash error). Its fields + // still feed the diagnostics: both the known-base and the untyped-base + // failure name the record and the module that declares it. + let with_known_base = build_mem( + "Main", + &[ + ("Inner", INNER), + ("Middle", MIDDLE), + ( + "Main", + "import Middle\n\ + type Config = { other: int }\n\ + let x = Middle.mk 5\n\ + let bad = x.wConf.cSize", + ), + ], + ); + let errors = project::check(&with_known_base); + assert_eq!(errors.len(), 1); + let message = &errors[0].errors[0].message; + assert!( + message.contains("has no field `cSize`") + && message.contains("module `Inner`") + && message.contains("hidden"), + "got: {message}" + ); + + let with_untyped_base = build_mem( + "Main", + &[ + ("Inner", INNER), + ("Middle", MIDDLE), + ( + "Main", + "import Middle\n\ + type Config = { other: int }\n\ + let get w = w.cSize", + ), + ], + ); + let errors = project::check(&with_untyped_base); + assert_eq!(errors.len(), 1); + let message = &errors[0].errors[0].message; + assert!( + message.contains("unknown record field `cSize`") && message.contains("module `Inner`"), + "got: {message}" + ); +} + +#[test] +fn a_diamond_of_carried_records_does_not_clash() { + // Two imports both carry `Inner.Config` transitively; the consumer must see + // one record, not a spurious clash for a name it never asked for. + let project = build_mem( + "Main", + &[ + ("Inner", INNER), + ( + "Left", + "import Inner\n\ + type LWrap = { lc: Inner.Config }\n\ + let mkL n = LWrap { lc = Inner.make n }", + ), + ( + "Right", + "import Inner\n\ + type RWrap = { rc: Inner.Config }\n\ + let mkR n = RWrap { rc = Inner.make n }", + ), + ( + "Main", + "import Left\n\ + import Right\n\ + let a = (Left.mkL 1).lc.cSize\n\ + let b = (Right.mkR 2).rc.cSize", + ), + ], + ); + let errors = project::check(&project); + assert!(errors.is_empty(), "unexpected errors: {errors:?}"); +} + +#[test] +fn importing_the_declaring_module_alongside_the_carrier_is_fine() { + // Direct import and transitive carry of the same record coexist: one record, + // full access, and construction through the direct import. + let project = build_mem( + "Main", + &[ + ("Inner", INNER), + ("Middle", MIDDLE), + ( + "Main", + "import Inner\n\ + import Middle\n\ + let x = Middle.mk 5\n\ + let a = x.wConf.cSize\n\ + let mine = Inner.Config { cSize = 1 }", + ), + ], + ); + let errors = project::check(&project); + assert!(errors.is_empty(), "unexpected errors: {errors:?}"); +} + +#[test] +fn a_carried_record_cannot_be_constructed_without_a_direct_import() { + // The carried record crosses as an identity (naming, unification, field + // access), not as a member: constructing it still asks for `import Inner`. + let project = build_mem( + "Main", + &[ + ("Inner", INNER), + ("Middle", MIDDLE), + ( + "Main", + "import Middle\nlet bad = Inner.Config { cSize = 1 }", + ), + ], + ); + let errors = project::check(&project); + assert_eq!(errors.len(), 1); + assert!( + errors[0].errors[0] + .message + .contains("not a member of `Inner`"), + "got: {}", + errors[0].errors[0].message + ); +} + +#[test] +fn a_bare_carried_record_construction_names_the_declaring_module() { + // `Config` *is* a record type here (its fields are readable through the same + // name), so the refusal must say what is actually missing: the direct import + // that construction requires. + let project = build_mem( + "Main", + &[ + ("Inner", INNER), + ("Middle", MIDDLE), + ("Main", "import Middle\nlet bad = Config { cSize = 1 }"), + ], + ); + let errors = project::check(&project); + assert_eq!(errors.len(), 1); + assert_eq!(errors[0].errors.len(), 1, "got: {:?}", errors[0].errors); + assert_eq!( + errors[0].errors[0].message, + "the record `Config` is declared in module `Inner`; import `Inner` to construct \ + its values here" + ); +} + +#[test] +fn a_bare_carried_record_pattern_names_the_declaring_module() { + // The pattern site gets the same treatment, worded for matching. + let project = build_mem( + "Main", + &[ + ("Inner", INNER), + ("Middle", MIDDLE), + ( + "Main", + "import Middle\n\ + let f w =\n match w:\n case Config { cSize = n }: n", + ), + ], + ); + let errors = project::check(&project); + assert_eq!(errors.len(), 1); + assert_eq!(errors[0].errors.len(), 1, "got: {:?}", errors[0].errors); + assert_eq!( + errors[0].errors[0].message, + "the record `Config` is declared in module `Inner`; import `Inner` to match \ + its values here" + ); +} + #[test] fn a_binding_colliding_with_a_module_alias_gets_a_mangled_import() { // `import Ids` + `let ids = …` used to emit `ids = …` after `import ids`,