diff --git a/changelog.d/11316-export-star-private-name-collision.md b/changelog.d/11316-export-star-private-name-collision.md new file mode 100644 index 00000000000..9041f9ad178 --- /dev/null +++ b/changelog.d/11316-export-star-private-name-collision.md @@ -0,0 +1,21 @@ +Fixed a link failure for re-exports that travel through an `export *` barrel +when another star source has a private binding with the same name +(`Undefined symbols: _perry_fn___`). `export *` forwards only a +source module's exports, but the binding-origin walk in +`crates/perry-hir/src/dynamic_import/binding_origin.rs` treated any same-named +function, class, global, enum, `let`, or import as the module's definition of +the exported name. A private `foo` in one star source and an exported `foo` in +another then looked ambiguous, the walk gave up, and the namespace entry named +the pure barrel, which never emits that getter. On export-name hops (import, +re-export, and star) a same-named binding now resolves only when the module +exports it under that name, which also stops a re-export from silently binding +to a private function of a star forwarder. + +Real-world trigger: zod 4.6.5, where `core/schemas.ts` has a private +`validateAsync` next to the exported one in `core/parse.ts`, and +`classic/parse.ts` re-exports it from `core/index.js`. + +Coverage: `test-parity/node-suite/module/imports/export-star-private-name-collision.ts` +(with fixtures under `imports/fixtures/export-star-private/`), two +`perry-hir` unit tests, and a tier-3 package fixture pinning zod 4.6.5 +(`tests/release/packages/zod-4-6/`). diff --git a/crates/perry-hir/src/dynamic_import.rs b/crates/perry-hir/src/dynamic_import.rs index 6f51021421c..e398fdc5e5a 100644 --- a/crates/perry-hir/src/dynamic_import.rs +++ b/crates/perry-hir/src/dynamic_import.rs @@ -171,13 +171,11 @@ fn flatten_into<'a, F>( // Resolving the binding to its defining module instead lets the // classifier see the real function/class/global and emit the // proper closure/global reference. - let origin = - resolve_binding_origin(module_name, local, lookup).unwrap_or_else(|| { - BindingOrigin { - source_module: module_name.to_string(), - source_local: local.clone(), - namespace_of: None, - } + let origin = resolve_binding_origin(module_name, local, false, lookup) + .unwrap_or_else(|| BindingOrigin { + source_module: module_name.to_string(), + source_local: local.clone(), + namespace_of: None, }); out.push(FlatExport { name: exported.clone(), @@ -200,7 +198,7 @@ fn flatten_into<'a, F>( // fall back to naming the directly-importing source — the // long-standing one-hop behaviour. let origin = - resolve_binding_origin(source, imported, lookup).unwrap_or_else(|| { + resolve_binding_origin(source, imported, true, lookup).unwrap_or_else(|| { BindingOrigin { source_module: source.clone(), source_local: imported.clone(), diff --git a/crates/perry-hir/src/dynamic_import/binding_origin.rs b/crates/perry-hir/src/dynamic_import/binding_origin.rs index 1ca3b89ee00..d595bb3138b 100644 --- a/crates/perry-hir/src/dynamic_import/binding_origin.rs +++ b/crates/perry-hir/src/dynamic_import/binding_origin.rs @@ -30,6 +30,15 @@ fn defines_local_binding(module: &Module, name: &str) -> bool { }) } +/// True when `module` exports its own binding `name` under that same name. +/// Only such a binding answers an export-name lookup: a private `function foo` +/// is not the export `foo`, which may instead arrive through `export *`. +fn exports_own_binding(module: &Module, name: &str) -> bool { + module.exports.iter().any(|export| { + matches!(export, Export::Named { local, exported } if local == name && exported == name) + }) +} + /// The import binding, if any, that `name` refers to in `module`. /// Native imports are excluded because their source has no compiled HIR owner. fn find_import_binding(module: &Module, name: &str) -> Option<(String, ImportBindingKind)> { @@ -69,16 +78,19 @@ enum ImportBindingKind { /// Resolve `(module_name, local)` to the module that actually defines the /// binding. Returns `None` when the chain does not move or leaves the compiled /// module graph, preserving the caller's existing one-hop fallback. +/// `as_export` says `start_local` is an export name of `start_module` rather +/// than a binding in its scope. pub(super) fn resolve_binding_origin<'a, F>( start_module: &str, start_local: &str, + as_export: bool, lookup: &F, ) -> Option where F: Fn(&str) -> Option<&'a Module>, { let mut seen = HashSet::new(); - let origin = resolve_exported_binding(start_module, start_local, lookup, &mut seen)?; + let origin = resolve_exported_binding(start_module, start_local, as_export, lookup, &mut seen)?; (origin.source_module != start_module || origin.source_local != start_local || origin.namespace_of.is_some()) @@ -91,9 +103,15 @@ where /// `bridge: export { v } from barrel` must resolve to `leaf`, because the pure /// barrel does not emit a `perry_fn_barrel__v` getter. Each star branch gets its /// own cycle set; distinct successful owners are ambiguous and do not resolve. +/// +/// `as_export` marks `local` as an export name (import, re-export, and star +/// hops). A same-named binding then only resolves when the module exports it, +/// so a private `foo` in one star source cannot shadow or conflict with the +/// exported `foo` of another. fn resolve_exported_binding<'a, F>( module_name: &str, local: &str, + as_export: bool, lookup: &F, seen: &mut HashSet<(String, String)>, ) -> Option @@ -104,8 +122,9 @@ where return None; } let module = lookup(module_name)?; + let own_binding = !as_export || exports_own_binding(module, local); - if defines_local_binding(module, local) { + if own_binding && defines_local_binding(module, local) { return Some(BindingOrigin { source_module: module_name.to_string(), source_local: local.to_string(), @@ -113,11 +132,16 @@ where }); } - if let Some((source, kind)) = find_import_binding(module, local) { + let import = if own_binding { + find_import_binding(module, local) + } else { + None + }; + if let Some((source, kind)) = import { if lookup(&source).is_some() { return match kind { ImportBindingKind::Value(imported) => { - resolve_exported_binding(&source, &imported, lookup, seen) + resolve_exported_binding(&source, &imported, true, lookup, seen) } ImportBindingKind::Namespace => Some(BindingOrigin { source_module: source.clone(), @@ -138,7 +162,7 @@ where } = export { if exported == local && source != local { - return resolve_exported_binding(module_name, source, lookup, seen); + return resolve_exported_binding(module_name, source, false, lookup, seen); } } } @@ -151,7 +175,7 @@ where imported, exported, } if exported == local && lookup(source).is_some() => { - return resolve_exported_binding(source, imported, lookup, seen); + return resolve_exported_binding(source, imported, true, lookup, seen); } Export::NamespaceReExport { source, name } if name == local && lookup(source).is_some() => @@ -176,7 +200,8 @@ where continue; }; let mut branch_seen = seen.clone(); - let Some(candidate) = resolve_exported_binding(source, local, lookup, &mut branch_seen) + let Some(candidate) = + resolve_exported_binding(source, local, true, lookup, &mut branch_seen) else { continue; }; diff --git a/crates/perry-hir/src/dynamic_import/tests.rs b/crates/perry-hir/src/dynamic_import/tests.rs index 4b30dc74208..d64edd55377 100644 --- a/crates/perry-hir/src/dynamic_import/tests.rs +++ b/crates/perry-hir/src/dynamic_import/tests.rs @@ -1228,6 +1228,67 @@ fn flatten_explicit_reexport_through_export_all_reaches_ultimate_owner() { assert_eq!(flat[1].source_local, "_null"); } +#[test] +fn flatten_export_all_ignores_private_same_named_binding() { + // `export *` forwards only a source's exports. A private `foo` in one star + // source must not make the exported `foo` of another star source ambiguous; + // pre-fix the walk returned `None` and named the pure barrel as owner, whose + // `perry_fn_barrel__foo` getter is never emitted (link error, zod 4.6.5). + let real = module_defining_fn("real", "foo"); + let mut shadow = module_defining_fn("shadow", "foo"); + shadow.exports.clear(); + let mut barrel = Module::new("barrel"); + for source in ["real", "shadow"] { + barrel.exports.push(Export::ExportAll { + source: source.into(), + }); + } + let mut bridge = Module::new("bridge"); + bridge.exports.push(Export::ReExport { + source: "barrel".into(), + imported: "foo".into(), + exported: "foo".into(), + }); + let map = std::collections::HashMap::from([ + ("real".to_string(), real), + ("shadow".to_string(), shadow), + ("barrel".to_string(), barrel), + ("bridge".to_string(), bridge), + ]); + let lookup = |s: &str| map.get(s); + let flat = flatten_exports("bridge", &lookup); + assert_eq!(flat.len(), 1); + assert_eq!(flat[0].source_module, "real"); + assert_eq!(flat[0].source_local, "foo"); +} + +#[test] +fn flatten_reexport_skips_private_binding_of_star_forwarder() { + // `shadow` has a private `foo` and `export * from "./real"`, so its export + // `foo` is real's. The private body must not answer the export lookup. + let real = module_defining_fn("real", "foo"); + let mut shadow = module_defining_fn("shadow", "foo"); + shadow.exports = vec![Export::ExportAll { + source: "real".into(), + }]; + let mut bridge = Module::new("bridge"); + bridge.exports.push(Export::ReExport { + source: "shadow".into(), + imported: "foo".into(), + exported: "foo".into(), + }); + let map = std::collections::HashMap::from([ + ("real".to_string(), real), + ("shadow".to_string(), shadow), + ("bridge".to_string(), bridge), + ]); + let lookup = |s: &str| map.get(s); + let flat = flatten_exports("bridge", &lookup); + assert_eq!(flat.len(), 1); + assert_eq!(flat[0].source_module, "real"); + assert_eq!(flat[0].source_local, "foo"); +} + #[test] fn flatten_local_definition_still_wins_over_same_named_import() { // A module that DEFINES the name it exports must keep pointing at itself — diff --git a/test-parity/node-suite/module/imports/export-star-private-name-collision.ts b/test-parity/node-suite/module/imports/export-star-private-name-collision.ts new file mode 100644 index 00000000000..9d42347c1ec --- /dev/null +++ b/test-parity/node-suite/module/imports/export-star-private-name-collision.ts @@ -0,0 +1,16 @@ +// `export *` forwards only a source module's exports. A private function in +// one star source must not collide with the same-named export of another star +// source, whether the name is read through the barrel namespace, a named +// re-export, or the private function's own caller. +import * as z from "./fixtures/export-star-private/bridge.ts"; +import { foo, useFoo } from "./fixtures/export-star-private/barrel.ts"; + +// Re-exporting the namespace forces every entry to be materialized. +export { z }; + +console.log("namespace:", typeof z, Object.keys(z).sort().join(",")); +console.log("named re-export:", z.foo()); +console.log("barrel namespace:", z.coreNamespace().foo()); +console.log("barrel keys:", Object.keys(z.coreNamespace()).sort().join(",")); +console.log("direct import:", foo()); +console.log("private caller:", useFoo()); diff --git a/test-parity/node-suite/module/imports/fixtures/export-star-private/barrel.ts b/test-parity/node-suite/module/imports/fixtures/export-star-private/barrel.ts new file mode 100644 index 00000000000..3e78fb7b1c7 --- /dev/null +++ b/test-parity/node-suite/module/imports/fixtures/export-star-private/barrel.ts @@ -0,0 +1,2 @@ +export * from "./real.ts"; +export * from "./shadow.ts"; diff --git a/test-parity/node-suite/module/imports/fixtures/export-star-private/bridge.ts b/test-parity/node-suite/module/imports/fixtures/export-star-private/bridge.ts new file mode 100644 index 00000000000..5d5b81f5322 --- /dev/null +++ b/test-parity/node-suite/module/imports/fixtures/export-star-private/bridge.ts @@ -0,0 +1,7 @@ +import * as core from "./barrel.ts"; + +export function coreNamespace(): typeof core { + return core; +} + +export { foo } from "./barrel.ts"; diff --git a/test-parity/node-suite/module/imports/fixtures/export-star-private/real.ts b/test-parity/node-suite/module/imports/fixtures/export-star-private/real.ts new file mode 100644 index 00000000000..882e22e521a --- /dev/null +++ b/test-parity/node-suite/module/imports/fixtures/export-star-private/real.ts @@ -0,0 +1,3 @@ +export function foo(): string { + return "real"; +} diff --git a/test-parity/node-suite/module/imports/fixtures/export-star-private/shadow.ts b/test-parity/node-suite/module/imports/fixtures/export-star-private/shadow.ts new file mode 100644 index 00000000000..7c30a80ad8b --- /dev/null +++ b/test-parity/node-suite/module/imports/fixtures/export-star-private/shadow.ts @@ -0,0 +1,8 @@ +// Private `foo` must NOT be forwarded by `export *` (only exports are). +function foo(): string { + return "fake"; +} + +export function useFoo(): string { + return foo(); +} diff --git a/tests/release/packages/zod-4-6/entry.ts b/tests/release/packages/zod-4-6/entry.ts new file mode 100644 index 00000000000..40fc7977522 --- /dev/null +++ b/tests/release/packages/zod-4-6/entry.ts @@ -0,0 +1,31 @@ +// zod 4.6.5 re-exports `validate`/`validateAsync` from core/index.ts, whose +// `export *` sources also include a private `validateAsync` (core/schemas.ts). +import { z } from "zod"; + +const User = z.object({ name: z.string(), age: z.number().int().min(0) }); + +const ok = User.safeParse({ name: "Ada", age: 36 }); +console.log("safeParse ok:", ok.success, JSON.stringify(ok.success ? ok.data : null)); + +const bad = User.safeParse({ name: 1, age: -1 }); +console.log("safeParse error:", bad.success); +if (!bad.success) { + for (const issue of bad.error.issues) { + console.log(" ", issue.code, issue.path.join(".")); + } +} + +console.log("z.core:", typeof z.core, typeof z.core.$constructor); +console.log("validate:", typeof z.validate, typeof z.validateAsync); + +async function main(): Promise { + const parsed = await User.parseAsync({ name: "Grace", age: 85 }); + console.log("parseAsync:", JSON.stringify(parsed)); + try { + await User.parseAsync({ name: "Linus" }); + } catch (err) { + console.log("parseAsync error:", err instanceof z.ZodError); + } +} + +main(); diff --git a/tests/release/packages/zod-4-6/expected.txt b/tests/release/packages/zod-4-6/expected.txt new file mode 100644 index 00000000000..40ecbf108d5 --- /dev/null +++ b/tests/release/packages/zod-4-6/expected.txt @@ -0,0 +1,8 @@ +safeParse ok: true {"name":"Ada","age":36} +safeParse error: false + invalid_type name + too_small age +z.core: object function +validate: function function +parseAsync: {"name":"Grace","age":85} +parseAsync error: true diff --git a/tests/release/packages/zod-4-6/fixture.sh b/tests/release/packages/zod-4-6/fixture.sh new file mode 100755 index 00000000000..97ed05e5540 --- /dev/null +++ b/tests/release/packages/zod-4-6/fixture.sh @@ -0,0 +1,17 @@ +#!/usr/bin/env bash +set -uo pipefail +cd "$(dirname "$0")" +. "$(dirname "$0")/../_fixture_lib.sh" + +NAME="zod-4-6" + +if [[ "${1:-}" == "--__did-skip-marker" ]]; then + exit 1 +fi + +# zod 4.6.5 has a private `validateAsync` in core/schemas.ts next to the exported +# one in core/parse.ts; both reach core/index.ts through `export *`. This +# fixture pins it to guard the export-star private-name collision fix. + +fixture_setup "$NAME" || exit 1 +fixture_compile_run_diff "$NAME" diff --git a/tests/release/packages/zod-4-6/package-lock.json b/tests/release/packages/zod-4-6/package-lock.json new file mode 100644 index 00000000000..0bc8e3b663c --- /dev/null +++ b/tests/release/packages/zod-4-6/package-lock.json @@ -0,0 +1,24 @@ +{ + "name": "perry-release-fixture-zod-4-6", + "version": "0.0.0", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { + "name": "perry-release-fixture-zod-4-6", + "version": "0.0.0", + "dependencies": { + "zod": "4.6.5" + } + }, + "node_modules/zod": { + "version": "4.6.5", + "resolved": "https://registry.npmjs.org/zod/-/zod-4.6.5.tgz", + "integrity": "sha512-v5l/aFXZQeai4awLbOpSoHecE9UiMrnfx75tEXLjNonXVARxQ5mOeipTjROUchszUNCqnE+hqAMujRsRHsut2Q==", + "license": "MIT", + "funding": { + "url": "https://github.com/sponsors/colinhacks" + } + } + } +} diff --git a/tests/release/packages/zod-4-6/package.json b/tests/release/packages/zod-4-6/package.json new file mode 100644 index 00000000000..efac7798f8c --- /dev/null +++ b/tests/release/packages/zod-4-6/package.json @@ -0,0 +1,16 @@ +{ + "name": "perry-release-fixture-zod-4-6", + "version": "0.0.0", + "private": true, + "type": "module", + "description": "Tier-3 fixture: zod 4.6.5 compile/run smoke (export-star private-name collision).", + "dependencies": { + "zod": "4.6.5" + }, + "perry": { + "compilePackages": ["zod"], + "allow": { + "compilePackages": ["zod"] + } + } +}