lowering: close the local-binder module-alias shadowing residual

ROADMAP.md

@@ -30,14 +30,13 @@ Keep this a *forward-looking* backlog — do not let it grow back into a changel
   measured on a decode-dominated workload; dynamic shapes (`andThen`, decoder-as-value) keep the
   interpreter.)
 
-- **Module-alias shadowing, local-binder residual** (S, demand-driven) — the top-level case is
-  **fixed** (2026-07-27): `import Ids` + `let ids = …` now emits `import ids as _pf_ids` and routes
-  every qualified reference through the alias (`lowering::py_module_ref`; un-collided programs keep
-  the readable plain `import ids`). Residual: a *local* binder with the colliding name (a parameter
-  or block `let` named `ids` in a function that also calls `Ids.…` qualified) still shadows within
-  that one function — the alias decision is per-module and keys off top-level `user_defs` only.
-  Fixing it means a whole-tree binder scan feeding the same alias switch; pick it up if a real
-  program hits it.
+- ~~Module-alias shadowing~~ **CLOSED 2026-07-27** — `import Ids` + any same-named binder (top-level
+  `let`, parameter, block `let` anywhere in the function, lambda parameter, match-pattern capture at
+  any level, native-CE binder) now emits `import ids as _pf_ids` at the affected sites
+  (`lowering::py_module_ref` consults `user_defs` + `module_binders` + the `fn_local_stack` scope
+  frames; plain and aliased imports coexist, so un-collided sites keep readable output). No known
+  residual; the per-shape regression test is
+  `tests/project.rs::local_binders_colliding_with_a_module_alias_also_get_the_mangled_import`.
 
 ## Performance beyond CPython (scoped 2026-07-18)
 

src/lowering/mod.rs

@@ -203,6 +203,13 @@ struct Lowerer {
     /// a fully-applied wrap is the bare argument, a first-class reference is
     /// `_pf_id`, and the single-case pattern recurses into its payload.
     newtype_ctors: HashSet<String>,
+    /// Every binder name appearing ANYWHERE in the module — top-level bindings,
+    /// parameters, block `let`s, lambda parameters, match-pattern captures, CE
+    /// binders ([`collect_binders`]). Drives the module-alias shadow check
+    /// ([`Lowerer::py_module_ref`]): a whole-module overapproximation, so the
+    /// check never has to model Python's function-wide local hoisting and stays
+    /// independent of the fold pass's and closure capture's scope machinery.
+    binder_names: HashSet<String>,
     /// Declared field order of each record type, so literals and updates emit a
     /// positional constructor call in the class's `__init__` order.
     record_fields: std::collections::HashMap<String, Vec<String>>,
@@ -354,6 +361,35 @@ impl Lowerer {
         let mut top_val_defs: HashMap<String, Expr> = HashMap::new();
         let mut ap_uses = std::collections::HashMap::new();
         let mut newtype_ctors = HashSet::new();
+        // Every binder name in the module, for the module-alias shadow check
+        // (`py_module_ref`) — see `collect_binders`.
+        let mut binder_names = HashSet::new();
+        for item in &module.items {
+            match item {
+                Item::Let(binding) => {
+                    binder_names.insert(binding.name.clone());
+                    binder_names.extend(param_names(&binding.params));
+                    collect_binders(&binding.value, &mut binder_names);
+                }
+                Item::Expr(e) => collect_binders(e, &mut binder_names),
+                Item::Module { items, .. } => {
+                    for member in items {
+                        binder_names.insert(member.name.clone());
+                        binder_names.extend(param_names(&member.params));
+                        collect_binders(&member.value, &mut binder_names);
+                    }
+                }
+                Item::ActivePattern(decl) => {
+                    binder_names.extend(param_names(&decl.params));
+                    collect_binders(&decl.value, &mut binder_names);
+                }
+                Item::Type(_)
+                | Item::Measure { .. }
+                | Item::Import { .. }
+                | Item::ExternImport { .. }
+                | Item::Extern(_) => {}
+            }
+        }
         for item in &module.items {
             match item {
                 Item::Extern(decl) => {
@@ -518,6 +554,7 @@ impl Lowerer {
             ap_uses,
             ctor_arity,
             newtype_ctors,
+            binder_names,
             record_fields,
             field_to_record,
             extern_targets,
@@ -1513,23 +1550,20 @@ impl Lowerer {
         Ok((stmts, values))
     }
 
-    /// The import spec a used extern target needs — `"datetime"` or
-    /// `"numpy as np"` — or `None` for a bare builtin. Declared `extern import`s
-    /// are consulted first (`DESIGN.md` §6): an *aliased* declaration matches a
-    /// target rooted at its alias name; an unaliased one matches the longest
-    /// declared path that strictly prefixes the target. Only when no declaration
-    /// matches does the lowercase-prefix heuristic ([`extern_import`]) decide.
     /// The Python name this module's emitted code uses for an imported file
     /// module — normally the lowercase module name (`Geometry` → `geometry`,
-    /// matching `import geometry`). When a top-level binding already claims that
-    /// name (`import Ids` + `let ids = …`), the module-level assignment would
-    /// clobber the module object, so the import is aliased (`import ids as
-    /// _pf_ids`) and every qualified reference routes through the alias. (A
-    /// *local* binder of the same name still shadows within its own function —
-    /// tracked in `ROADMAP.md` as residual.)
+    /// matching `import geometry`). When ANY binder anywhere in the module
+    /// claims that name (`binder_names`: top-level bindings, parameters, block
+    /// `let`s, lambda parameters, match-pattern captures, CE binders — Python
+    /// hoists locals, so even a binder *after* the reference shadows it for its
+    /// whole function), the import is aliased (`import ids as _pf_ids`) and
+    /// every qualified reference routes through the alias. The decision is
+    /// deliberately whole-module: one collision anywhere costs only the alias
+    /// spelling, and keeps this check independent of the scope machinery the
+    /// fold pass and closure capture rely on.
     fn py_module_ref(&mut self, base: &str) -> String {
         let module = base.to_lowercase();
-        if self.user_defs.contains(&module) {
+        if self.binder_names.contains(&module) {
             let alias = format!("_pf_{module}");
             self.needed_imports.insert(format!("{module} as {alias}"));
             alias
@@ -1539,6 +1573,12 @@ impl Lowerer {
         }
     }
 
+    /// The import spec a used extern target needs — `"datetime"` or
+    /// `"numpy as np"` — or `None` for a bare builtin. Declared `extern import`s
+    /// are consulted first (`DESIGN.md` §6): an *aliased* declaration matches a
+    /// target rooted at its alias name; an unaliased one matches the longest
+    /// declared path that strictly prefixes the target. Only when no declaration
+    /// matches does the lowercase-prefix heuristic ([`extern_import`]) decide.
     fn extern_import_spec(&self, target: &[String]) -> Option<String> {
         let mut best: Option<&(Vec<String>, Option<String>)> = None;
         for decl in &self.extern_module_imports {
@@ -4706,6 +4746,119 @@ fn scan_scope(expr: &Expr, assigned: &mut HashSet<String>, bound: &mut HashSet<S
     }
 }
 
+/// Collect every binder name in an expression tree, ENTERING nested scopes
+/// (functions, lambdas, CE bodies) — unlike [`scan_scope`], which stops at
+/// scope boundaries because it feeds per-scope machinery. This feeds the
+/// module-alias shadow check ([`Lowerer::py_module_ref`]), where a deliberate
+/// whole-module overapproximation is what keeps the check simple and total:
+/// block `let`s, match-pattern captures, lambda/function parameters, and CE
+/// binders all lower to Python assignments or parameters that shadow a plain
+/// `import <name>` somewhere.
+fn collect_binders(expr: &Expr, out: &mut HashSet<String>) {
+    match &expr.kind {
+        ExprKind::Assign { value, .. } => collect_binders(value, out),
+        ExprKind::Block { stmts } => {
+            for stmt in stmts {
+                match stmt {
+                    BlockStmt::Let(b) => {
+                        out.insert(b.name.clone());
+                        out.extend(param_names(&b.params));
+                        collect_binders(&b.value, out);
+                    }
+                    BlockStmt::Expr(e) => collect_binders(e, out),
+                }
+            }
+        }
+        ExprKind::If { cond, then, else_ } => {
+            collect_binders(cond, out);
+            collect_binders(then, out);
+            collect_binders(else_, out);
+        }
+        ExprKind::Match { scrutinee, arms } => {
+            collect_binders(scrutinee, out);
+            for arm in arms {
+                out.extend(pattern_bindings(&arm.pattern));
+                if let Some(guard) = &arm.guard {
+                    collect_binders(guard, out);
+                }
+                collect_binders(&arm.body, out);
+            }
+        }
+        ExprKind::App { func, arg } => {
+            collect_binders(func, out);
+            collect_binders(arg, out);
+        }
+        ExprKind::Pipe { lhs, rhs, .. } | ExprKind::Compose { lhs, rhs, .. } => {
+            collect_binders(lhs, out);
+            collect_binders(rhs, out);
+        }
+        ExprKind::Binary { lhs, rhs, .. } => {
+            collect_binders(lhs, out);
+            collect_binders(rhs, out);
+        }
+        ExprKind::Unary { expr, .. } => collect_binders(expr, out),
+        ExprKind::Compare { first, rest } => {
+            collect_binders(first, out);
+            for (_, operand) in rest {
+                collect_binders(operand, out);
+            }
+        }
+        ExprKind::Try { body } => collect_binders(body, out),
+        ExprKind::Annot { value, .. } => collect_binders(value, out),
+        ExprKind::List { elems } | ExprKind::Tuple { elems } => {
+            for e in elems {
+                collect_binders(e, out);
+            }
+        }
+        ExprKind::Interp { parts } => {
+            for part in parts {
+                if let InterpPart::Expr(e) = part {
+                    collect_binders(e, out);
+                }
+            }
+        }
+        ExprKind::Record { fields, .. } => {
+            for f in fields {
+                collect_binders(&f.value, out);
+            }
+        }
+        ExprKind::RecordUpdate { base, fields } => {
+            collect_binders(base, out);
+            for f in fields {
+                collect_binders(&f.value, out);
+            }
+        }
+        ExprKind::Field { base, .. } => collect_binders(base, out),
+        ExprKind::Fn { params, body } => {
+            out.extend(param_names(params));
+            collect_binders(body, out);
+        }
+        ExprKind::Ce { items, .. } => {
+            for item in items {
+                match item {
+                    CeItem::LetBang { name, value, .. } | CeItem::Let { name, value, .. } => {
+                        out.insert(name.clone());
+                        collect_binders(value, out);
+                    }
+                    CeItem::DoBang(e)
+                    | CeItem::Return(e)
+                    | CeItem::ReturnBang(e)
+                    | CeItem::Yield(e)
+                    | CeItem::YieldBang(e) => collect_binders(e, out),
+                }
+            }
+        }
+        ExprKind::Int(_)
+        | ExprKind::Float(_)
+        | ExprKind::Str(_)
+        | ExprKind::Bool(_)
+        | ExprKind::Unit
+        | ExprKind::OpFunc(_)
+        | ExprKind::Hole { .. }
+        | ExprKind::Var(_) => {}
+    }
+}
+
 fn non_exhaustive_guard() -> PyCase {
     PyCase {
         pattern: PyPattern::Wildcard,

tests/project.rs

@@ -830,6 +830,47 @@ fn a_binding_colliding_with_a_module_alias_gets_a_mangled_import() {
     }
 }
 
+#[test]
+fn local_binders_colliding_with_a_module_alias_also_get_the_mangled_import() {
+    // Python hoists locals, so ANY binder in a function scope named like the
+    // import alias shadows it for the whole function: parameters, block `let`s
+    // (even after the qualified call), lambda parameters, match-pattern
+    // captures, top-level pattern captures, and CE binders (which become
+    // captures inside the generated def). Each shape used to break at runtime.
+    let files = compile(
+        "Main",
+        &[
+            ("Ids", "let greet s = String.concat \"hi \" s"),
+            (
+                "Main",
+                "import Ids\n\
+                 let viaParam ids = List.map (fun n -> Ids.greet n) ids\n\
+                 let hoisted u =\n    let g = Ids.greet u\n    let ids = [1, 2]\n    List.len ids + String.len g\n\
+                 let viaLambda xs = List.map (fun ids -> Ids.greet ids) xs\n\
+                 let fromMatch xs =\n  match xs:\n    case [ids]: Ids.greet ids\n    case _: \"none\"\n\
+                 let fromCe =\n  result {\n    let! ids = Ok [1]\n    return Ids.greet \"g\"\n  }\n\
+                 let topLevel =\n  match [\"f\"]:\n    case [ids]: Ids.greet ids\n    case _: \"none\"\n\
+                 print (List.len (viaParam [\"a\", \"b\"]))\n\
+                 print (hoisted \"c\")\n\
+                 print (List.len (viaLambda [\"d\"]))\n\
+                 print (fromMatch [\"e\"])\n\
+                 print (Result.withDefault \"err\" fromCe)\n\
+                 print topLevel",
+            ),
+        ],
+    );
+    let main = file(&files, "main.py");
+    assert!(main.contains("import ids as _pf_ids"), "{main}");
+    assert!(!main.contains("return ids.greet"), "{main}");
+    let dir = Scratch::new("alias_local_collision");
+    if let Some(out) = run_project(&dir, &files, "main.py") {
+        assert_eq!(
+            out.replace("\r\n", "\n").trim(),
+            "2\n6\n1\nhi e\nhi g\nhi f"
+        );
+    }
+}
+
 #[test]
 fn an_uncollided_module_import_stays_unmangled() {
     // No collision → the readable plain `import ids` is preserved.