Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/perry-codegen/src/codegen/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ pub(super) fn compile_closure(
imported_func_return_types: &cross_module.imported_func_return_types,
ffi_signatures: &cross_module.ffi_signatures,
imported_class_sources: &cross_module.imported_class_sources,
imported_class_original_names: &cross_module.imported_class_original_names,
interfaces: &cross_module.interfaces,
try_depth: 0,
pending_declares: Vec::new(),
Expand Down
2 changes: 2 additions & 0 deletions crates/perry-codegen/src/codegen/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,7 @@ pub(super) fn compile_module_entry(
imported_func_return_types: &cross_module.imported_func_return_types,
ffi_signatures: &cross_module.ffi_signatures,
imported_class_sources: &cross_module.imported_class_sources,
imported_class_original_names: &cross_module.imported_class_original_names,
interfaces: &cross_module.interfaces,
try_depth: 0,
pending_declares: Vec::new(),
Expand Down Expand Up @@ -998,6 +999,7 @@ pub(super) fn compile_module_entry(
imported_func_return_types: &cross_module.imported_func_return_types,
ffi_signatures: &cross_module.ffi_signatures,
imported_class_sources: &cross_module.imported_class_sources,
imported_class_original_names: &cross_module.imported_class_original_names,
interfaces: &cross_module.interfaces,
try_depth: 0,
pending_declares: Vec::new(),
Expand Down
1 change: 1 addition & 0 deletions crates/perry-codegen/src/codegen/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ pub(super) fn compile_function(
imported_func_return_types: &cross_module.imported_func_return_types,
ffi_signatures: &cross_module.ffi_signatures,
imported_class_sources: &cross_module.imported_class_sources,
imported_class_original_names: &cross_module.imported_class_original_names,
interfaces: &cross_module.interfaces,
try_depth: 0,
pending_declares: Vec::new(),
Expand Down
2 changes: 2 additions & 0 deletions crates/perry-codegen/src/codegen/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ pub(super) fn compile_method(
imported_func_return_types: &cross_module.imported_func_return_types,
ffi_signatures: &cross_module.ffi_signatures,
imported_class_sources: &cross_module.imported_class_sources,
imported_class_original_names: &cross_module.imported_class_original_names,
interfaces: &cross_module.interfaces,
try_depth: 0,
pending_declares: Vec::new(),
Expand Down Expand Up @@ -820,6 +821,7 @@ pub(super) fn compile_static_method(
imported_func_return_types: &cross_module.imported_func_return_types,
ffi_signatures: &cross_module.ffi_signatures,
imported_class_sources: &cross_module.imported_class_sources,
imported_class_original_names: &cross_module.imported_class_original_names,
interfaces: &cross_module.interfaces,
try_depth: 0,
pending_declares: Vec::new(),
Expand Down
19 changes: 19 additions & 0 deletions crates/perry-codegen/src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1336,6 +1336,25 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result<Vec<u8>>
}
map
},
// Per-module alias → original imported export name. Only renamed named
// imports (`local != imported`) are recorded; this lets `lower_new`
// recover the canonical built-in constructor name when a bundle aliases
// the import (e.g. `import { AsyncLocalStorage as xQ5 }`). See the
// field doc on `CompileOptions::imported_class_original_names`.
imported_class_original_names: {
let mut map: std::collections::HashMap<String, String> =
std::collections::HashMap::new();
for import in &hir.imports {
for spec in &import.specifiers {
if let perry_hir::ImportSpecifier::Named { imported, local } = spec {
if local != imported {
map.insert(local.clone(), imported.clone());
}
}
}
}
map
},
interfaces: hir
.interfaces
.iter()
Expand Down
10 changes: 10 additions & 0 deletions crates/perry-codegen/src/codegen/opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -737,6 +737,16 @@ pub(crate) struct CrossModuleCtx {
/// "Client" arm only fires when the local `Client` was imported from
/// "pg" (named or default). See issue #602.
pub imported_class_sources: std::collections::HashMap<String, String>,
/// Per-module mapping: local alias → original imported export name, for
/// named imports where the binding was renamed (`import { AsyncLocalStorage
/// as xQ5 } from "async_hooks"` records `xQ5 -> "AsyncLocalStorage"`). Built
/// once in `compile_module` from `hir.imports`. Lets `lower_new` recover the
/// real export name so the built-in constructor arms in `lower_builtin_new`
/// (keyed on the canonical name like `"AsyncLocalStorage"`) still fire when
/// a minified bundle aliases the import. Without this, `new xQ5()` fell
/// through to the empty-object placeholder and the instance had no
/// `.getStore`/`.run` methods (`TypeError: getStore is not a function`).
pub imported_class_original_names: std::collections::HashMap<String, String>,
/// Issue #655: map from interface name → HIR Interface definition.
/// Lets `static_type_of` resolve `obj.field` when `obj` is typed
/// against a TS `interface` (not a `class`). The `class_table`
Expand Down
5 changes: 5 additions & 0 deletions crates/perry-codegen/src/expr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,11 @@ pub(crate) struct FnCtx<'a> {
/// Used by `lower_builtin_new` to disambiguate ambiguously-named
/// built-in constructors. See issue #602.
pub imported_class_sources: &'a std::collections::HashMap<String, String>,
/// Per-module alias → original imported export name (renamed named imports
/// only). Used by `lower_new` to recover the canonical built-in constructor
/// name when a bundle aliases the import (`import { AsyncLocalStorage as xQ5
/// }`). See `CompileOptions::imported_class_original_names`.
pub imported_class_original_names: &'a std::collections::HashMap<String, String>,
/// Number of currently-open `try { ... }` blocks at the current
/// lowering position. Incremented before lowering a try body,
/// decremented after. `Stmt::Return` emits `js_try_end()` this many
Expand Down
18 changes: 18 additions & 0 deletions crates/perry-codegen/src/lower_call/new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,24 @@ pub(crate) fn lower_new(ctx: &mut FnCtx<'_>, class_name: &str, args: &[Expr]) ->
if let Some(val) = lower_builtin_new(ctx, class_name, args)? {
return Ok(val);
}
// Aliased built-in import: a minified bundle renames a node built-in
// constructor (`import { AsyncLocalStorage as xQ5 } from "async_hooks";
// new xQ5()`). The syntactic callee is the alias `xQ5`, so the
// canonical-name arms in `lower_builtin_new` (keyed on
// `"AsyncLocalStorage"`) never fired and `new xQ5()` fell through to the
// empty-object placeholder — the instance had no `.run`/`.getStore`, so
// `xQ5().getStore()` threw `TypeError: getStore is not a function`.
// Recover the original export name and retry. The alias is only present
// here when it was NOT already a user-defined class (the enclosing
// `!ctx.classes.contains_key(class_name)` guard), so a renamed import
// can't shadow a real local class.
if let Some(original) = ctx.imported_class_original_names.get(class_name).cloned() {
if original != class_name {
if let Some(val) = lower_builtin_new(ctx, &original, args)? {
return Ok(val);
Comment on lines +374 to +377

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Alias fallback should be source-qualified to avoid wrong constructor dispatch.

On Line 374, retrying by alias name alone can misroute non-builtin aliased imports that happen to share a builtin export name, causing new to bypass the real imported constructor path.

Suggested fix
-        if let Some(original) = ctx.imported_class_original_names.get(class_name).cloned() {
-            if original != class_name {
-                if let Some(val) = lower_builtin_new(ctx, &original, args)? {
-                    return Ok(val);
-                }
-            }
-        }
+        if let Some(original) = ctx.imported_class_original_names.get(class_name) {
+            if original != class_name {
+                let alias_is_builtin_import = ctx
+                    .import_function_node_submodule
+                    .get(class_name)
+                    .is_some_and(|(_, exported_name)| exported_name == original);
+                if alias_is_builtin_import {
+                    if let Some(val) = lower_builtin_new(ctx, original, args)? {
+                        return Ok(val);
+                    }
+                }
+            }
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if let Some(original) = ctx.imported_class_original_names.get(class_name).cloned() {
if original != class_name {
if let Some(val) = lower_builtin_new(ctx, &original, args)? {
return Ok(val);
if let Some(original) = ctx.imported_class_original_names.get(class_name) {
if original != class_name {
let alias_is_builtin_import = ctx
.import_function_node_submodule
.get(class_name)
.is_some_and(|(_, exported_name)| exported_name == original);
if alias_is_builtin_import {
if let Some(val) = lower_builtin_new(ctx, original, args)? {
return Ok(val);
}
}
}
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-codegen/src/lower_call/new.rs` around lines 374 - 377, The
lower_builtin_new call at line 375 uses the alias name from
imported_class_original_names without verifying it's actually a builtin, which
can cause non-builtin aliased imports that share a builtin export name to be
incorrectly dispatched to the builtin constructor path. Instead of directly
passing the original alias name to lower_builtin_new, first verify that the
original name is actually a builtin constructor (or use a source-qualified
lookup) before attempting the builtin path, otherwise fall through to handle it
as a regular imported constructor.

}
}
}
}

// Local class alias rerouting: `let C = SomeClass; new C()` lowers
Expand Down
63 changes: 63 additions & 0 deletions crates/perry/tests/aliased_native_class_import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,69 @@ console.log(typeof s.connect, typeof s.write, typeof s.end);
assert_eq!(aliased, "function function function\n");
}

/// Aliased `async_hooks.AsyncLocalStorage`: a minified bundle renames the
/// import (`import { AsyncLocalStorage as xQ5 }`) and later does `new xQ5()`.
/// The syntactic `new`-callee is the alias `xQ5`, so the codegen built-in
/// constructor arm (keyed on the canonical name `"AsyncLocalStorage"`) did not
/// fire and the instance fell through to the empty-object placeholder with no
/// methods — `xQ5().getStore()` threw `TypeError: getStore is not a function`.
/// Exercises the exact `run(store, cb)` / `getStore()` round-trip that a CLI's
/// `doctor`-style context plumbing relies on.
#[test]
fn aliased_async_local_storage_run_getstore_round_trip() {
let dir = tempfile::tempdir().expect("tempdir");
let stdout = compile_and_run(
dir.path(),
r#"
import { AsyncLocalStorage as xQ5 } from "async_hooks";
const als: any = new xQ5();
console.log("getStore type:", typeof als.getStore);
console.log("outside:", als.getStore());
const inside = als.run({ cwd: "/work" }, () => als.getStore().cwd);
console.log("inside:", inside);
console.log("after:", als.getStore());
"#,
);
assert_eq!(
stdout,
"getStore type: function\n\
outside: undefined\n\
inside: /work\n\
after: undefined\n"
);
}

/// The aliased `async_hooks.AsyncLocalStorage` path must match the un-aliased
/// path byte-for-byte (alias resolution == canonical lowering).
#[test]
fn aliased_async_local_storage_matches_unaliased() {
let prog = |import_line: &str, ctor: &str| {
format!(
"{import_line}\nconst als: any = new {ctor}();\n\
const r = als.run({{ id: 7 }}, () => als.getStore().id);\n\
console.log(typeof als.getStore, typeof als.run, r, als.getStore());\n"
)
};
let dir = tempfile::tempdir().expect("tempdir");
let aliased = compile_and_run(
dir.path(),
&prog(
"import { AsyncLocalStorage as Q9 } from \"node:async_hooks\";",
"Q9",
),
);
let dir2 = tempfile::tempdir().expect("tempdir");
let unaliased = compile_and_run(
dir2.path(),
&prog(
"import { AsyncLocalStorage } from \"node:async_hooks\";",
"AsyncLocalStorage",
),
);
assert_eq!(aliased, unaliased);
assert_eq!(aliased, "function function 7 undefined\n");
}

/// A non-native user import alias must NOT be treated as a native class — the
/// fix must not over-trigger native handling on ordinary user modules.
#[test]
Expand Down
Loading