diff --git a/changelog.d/11833-namespace-member-shadows-global.md b/changelog.d/11833-namespace-member-shadows-global.md new file mode 100644 index 0000000000..0540baf52f --- /dev/null +++ b/changelog.d/11833-namespace-member-shadows-global.md @@ -0,0 +1 @@ +- **perry (compile)**: a namespace import no longer lets a member named like a global intrinsic shadow that global in the importer. `import * as Arr from "./Array.ts"` registered `Arr`'s `Array` member as a bare-name fallback, so the importer's own `new Array(n)` constructed the member instead of the builtin — `undefined` results, and a link failure in the OpenCode v1.18.30 build, whose effect modules all do this (#10945). Bare-name fallback entries now skip global-intrinsic names, as #10356 does for classes; `Arr.Array` still resolves. diff --git a/crates/perry/src/commands/compile/run_pipeline.rs b/crates/perry/src/commands/compile/run_pipeline.rs index 934c9dfa8a..8f4ce14d55 100644 --- a/crates/perry/src/commands/compile/run_pipeline.rs +++ b/crates/perry/src/commands/compile/run_pipeline.rs @@ -3984,6 +3984,17 @@ pub fn run_with_parse_cache( for (export_name, origin_path) in exports { let origin_prefix = compute_module_prefix(origin_path, &ctx.project_root); + // #10945: a namespace member enters the FLAT maps below only as a + // best-effort fallback for a bare-name call (#5927). A bare name that is a + // global intrinsic (`Array`, `Map`, `Request`, ...) never means the member in + // an importer that did not import it by name, and an entry here makes the + // importer's `new Array(n)` construct the member instead (`lower_new`'s + // `user_owns_construction`) -- effect's `Array.ts` exports + // `const Array = globalThis.Array`. Same rule #10356 applies to implicitly + // registered classes. The per-namespace entries are kept, so `ns.Array` + // still resolves. + let flat = + !perry_hir::analysis::is_global_intrinsic_value_name(export_name); // Issue #5927: namespace members are a // best-effort fallback in the flat // `import_function_prefixes` map — the @@ -4009,9 +4020,11 @@ pub fn run_with_parse_cache( // remeda call) resolved against // `Context.ts`'s prefix instead of // remeda's chunk. - import_function_prefixes - .entry(export_name.clone()) - .or_insert_with(|| origin_prefix.clone()); + if flat { + import_function_prefixes + .entry(export_name.clone()) + .or_insert_with(|| origin_prefix.clone()); + } // Issue #678: surface origin-name overrides // for namespace-imported members too. A // member reached via a re-export rename @@ -4023,7 +4036,7 @@ pub fn run_with_parse_cache( .and_then(|m| m.get(export_name)) .cloned(); if let Some(ref origin_name) = resolved_origin_name { - if origin_name != export_name { + if flat && origin_name != export_name { // Issue #5927: same `or_insert` // rationale as `import_function_prefixes` // above — never let a namespace @@ -4066,15 +4079,21 @@ pub fn run_with_parse_cache( let scoped_func_key = perry_codegen::namespace_member_func_key(local, export_name); if let Some(¶m_count) = exported_func_param_counts.get(&key) { - imported_param_counts.insert(export_name.clone(), param_count); + if flat { + imported_param_counts.insert(export_name.clone(), param_count); + } imported_param_counts.insert(scoped_func_key.clone(), param_count); } if exported_func_has_rest.get(&key).copied().unwrap_or(false) { - imported_has_rest.insert(export_name.clone()); + if flat { + imported_has_rest.insert(export_name.clone()); + } imported_has_rest.insert(scoped_func_key.clone()); } if exported_func_synthetic_arguments.contains(&key) { - imported_synthetic_arguments.insert(export_name.clone()); + if flat { + imported_synthetic_arguments.insert(export_name.clone()); + } imported_synthetic_arguments.insert(scoped_func_key); } // Issue #636: namespace-imported vars must @@ -4457,6 +4476,18 @@ pub fn run_with_parse_cache( for (export_name, origin_path) in target_exports { let origin_prefix = compute_module_prefix(origin_path, &ctx.project_root); + // #10945: a namespace member enters the FLAT maps below only as a + // best-effort fallback for a bare-name call (#5927). A bare name that is a + // global intrinsic (`Array`, `Map`, `Request`, ...) never means the member in + // an importer that did not import it by name, and an entry here makes the + // importer's `new Array(n)` construct the member instead (`lower_new`'s + // `user_owns_construction`) -- effect's `Array.ts` exports + // `const Array = globalThis.Array`. Same rule #10356 applies to implicitly + // registered classes. The per-namespace entries are kept, so `ns.Array` + // still resolves. + let flat = !perry_hir::analysis::is_global_intrinsic_value_name( + export_name, + ); // Issue #5927: `or_insert` — see the // matching rationale on the // `namespace_like_local` branch above. @@ -4466,9 +4497,11 @@ pub fn run_with_parse_cache( // has no other resolution path and // must always win, regardless of // import-statement order. - import_function_prefixes - .entry(export_name.clone()) - .or_insert_with(|| origin_prefix.clone()); + if flat { + import_function_prefixes + .entry(export_name.clone()) + .or_insert_with(|| origin_prefix.clone()); + } // Issue #5922 (companion to #680): also // register under the per-namespace key so // `Context.foo` and `Option.foo` resolve to @@ -4493,7 +4526,7 @@ pub fn run_with_parse_cache( .and_then(|m| m.get(export_name)) .cloned(); if let Some(ref origin_name) = resolved_origin_name { - if origin_name != export_name { + if flat && origin_name != export_name { // Issue #5927: `or_insert` — see // the matching rationale above. import_function_origin_names @@ -4544,16 +4577,23 @@ pub fn run_with_parse_cache( export_name, ); if let Some(¶m_count) = exported_func_param_counts.get(&key) { - imported_param_counts.insert(export_name.clone(), param_count); + if flat { + imported_param_counts + .insert(export_name.clone(), param_count); + } imported_param_counts .insert(scoped_func_key.clone(), param_count); } if exported_func_has_rest.get(&key).copied().unwrap_or(false) { - imported_has_rest.insert(export_name.clone()); + if flat { + imported_has_rest.insert(export_name.clone()); + } imported_has_rest.insert(scoped_func_key.clone()); } if exported_func_synthetic_arguments.contains(&key) { - imported_synthetic_arguments.insert(export_name.clone()); + if flat { + imported_synthetic_arguments.insert(export_name.clone()); + } imported_synthetic_arguments.insert(scoped_func_key); } // Issue #321: NamespaceReExport members diff --git a/crates/perry/tests/issue_10945_namespace_member_shadows_global.rs b/crates/perry/tests/issue_10945_namespace_member_shadows_global.rs new file mode 100644 index 0000000000..8b4e43574d --- /dev/null +++ b/crates/perry/tests/issue_10945_namespace_member_shadows_global.rs @@ -0,0 +1,129 @@ +//! Regression test for #10945: a NAMESPACE import registered every member of +//! the imported module in the flat `import_function_prefixes` map as a +//! best-effort fallback for bare-name calls (#5927). For a member named like a +//! global intrinsic, that entry hijacked the importer's own bare global: +//! `lower_new` found the name in the map and constructed the member instead of +//! the builtin, so `new Array(n).fill(0).length` came back `undefined`. +//! +//! effect's `src/Array.ts` exports `const Array = globalThis.Array`, and its +//! `Chunk.ts`, `Cron.ts` and `internal/effect.ts` do `import * as Arr from +//! "./Array.ts"` and then write `new Array(n)`. In the full OpenCode v1.18.30 +//! build that referenced Array.ts's wrapper under a name the owning module +//! never emits, and the link failed. +//! +//! A namespace import binds exactly ONE name, the namespace, so a bare `Array` +//! in the importer is the global. Members still resolve through the namespace +//! (`Arr.Array`, `Arr.bump`), and an EXPLICIT named import of a +//! global-shadowing name (`import { Map }`) must still win — #10356's control. + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn runtime_dir() -> PathBuf { + std::env::var_os("PERRY_RUNTIME_DIR") + .map(PathBuf::from) + .unwrap_or_else(|| { + perry_bin() + .parent() + .expect("compiler directory") + .to_path_buf() + }) +} + +/// Mirrors effect's `src/Array.ts`: a member named like the global it aliases, +/// next to an ordinary function member. +const ARRAY_MOD_SOURCE: &str = r#" +export const Array = globalThis.Array +export function bump(x: number) { return x + 1 } +"#; + +/// The explicit-import control: a module whose `Map` IS imported by name. +const FAKE_MAP_SOURCE: &str = r#" +export class Map { + readonly kind = "explicitly-imported-map" +} +"#; + +const SHADOW_SOURCE: &str = r#" +import { Map } from "./fake_map.js" +export const explicitKind = new Map().kind +"#; + +/// Uses the bare globals inside a function and a closure, as effect does. +const MAIN_SOURCE: &str = r#" +import * as Arr from "./array_mod.js" +import { explicitKind } from "./shadow.js" + +function sized(n: number) { + return new Array(n).fill(0).length +} +const pair = (a: number, b: number) => Array.of(a, b).join("+") + +console.log("1 new Array(n):", sized(3)) +console.log("2 Array.isArray:", Array.isArray([1]), Array.isArray("x")) +console.log("3 Array.from:", Array.from("abc").length) +console.log("4 Array.of in a closure:", pair(4, 5)) +console.log("5 Arr.Array is the global:", Arr.Array === globalThis.Array) +console.log("6 new Arr.Array:", new Arr.Array(2).length) +console.log("7 Arr.bump:", Arr.bump(41)) +console.log("8 explicit-import:", explicitKind) +"#; + +/// Byte-for-byte what node 26.5.1 prints. +const EXPECTED: &str = "\ +1 new Array(n): 3 +2 Array.isArray: true false +3 Array.from: 3 +4 Array.of in a closure: 4+5 +5 Arr.Array is the global: true +6 new Arr.Array: 2 +7 Arr.bump: 42 +8 explicit-import: explicitly-imported-map +"; + +#[test] +fn namespace_member_does_not_shadow_a_global_intrinsic() { + let dir = tempfile::tempdir().expect("tempdir"); + let root = dir.path(); + std::fs::write(root.join("array_mod.ts"), ARRAY_MOD_SOURCE).unwrap(); + std::fs::write(root.join("fake_map.ts"), FAKE_MAP_SOURCE).unwrap(); + std::fs::write(root.join("shadow.ts"), SHADOW_SOURCE).unwrap(); + std::fs::write(root.join("main.ts"), MAIN_SOURCE).unwrap(); + + let output = root.join("main_bin"); + let out = Command::new(perry_bin()) + .current_dir(root) + .arg("compile") + .arg(root.join("main.ts")) + .arg("-o") + .arg(&output) + .arg("--no-cache") + .env("PERRY_NO_AUTO_OPTIMIZE", "1") + .env("PERRY_RUNTIME_DIR", runtime_dir()) + .output() + .expect("run perry compile"); + assert!( + out.status.success(), + "namespace-shadowing probe must compile; stdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); + + let run = Command::new(&output).output().expect("run compiled binary"); + assert!( + run.status.success(), + "compiled binary must run; stdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + let stdout = String::from_utf8(run.stdout).expect("UTF-8 stdout"); + assert_eq!( + stdout, EXPECTED, + "a namespace import binds only the namespace: a member named like a \ + global must not shadow that global in the importer" + ); +}