fix(hir): aliased ESM import of a native class keeps its native-class identity#5472
Conversation
… identity
`import { BlockList as Wj4 } from "net"; new Wj4()` threw 'addSubnet is not a
function' while the un-aliased `import { BlockList }` worked. Native-class
resolution keyed on the LOCAL binding name instead of the imported export name
at two sites:
- expr_new.rs (bare-ident `new <Ident>()`): `new Wj4()` lowered to
`Expr::New { class_name: "Wj4" }`; codegen recognizes builtins by literal
name ("BlockList", "Socket", ...), so "Wj4" missed every arm and built an
empty placeholder object.
- destructuring/var_decl.rs: `register_native_instance` tagged the binding
`("net","Wj4")`, missing the `("net","BlockList")` dispatch rows.
`lookup_native_module` was already alias-aware; both sites now resolve the local
binding to its imported export name before matching/registering (guarded so user
classes/locals shadow and non-native user imports don't over-trigger). Aliased
HIR is now byte-identical to the un-aliased form. Fixes all aliased native
classes (net BlockList/Socket, http Server, url URL, ...). Adds e2e test.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds alias-aware resolution for native built-in classes across the Perry HIR pipeline. When a native class is imported under a local alias (e.g., ChangesAlias-aware native class resolution
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@crates/perry/tests/aliased_native_class_import.rs`:
- Around line 70-79: The `print_hir` function is returning stdout without
verifying that the `perry compile --print-hir` command executed successfully.
Add a status check after the `.output()` call to assert the command succeeded,
and if it fails, surface the stderr output for better diagnostics. Mirror the
status assertion pattern used in the `compile_and_run` function to ensure
compiler errors are visible when the HIR compilation fails, rather than silently
returning incomplete stdout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 56e3e352-50ec-4f63-b52a-ddf7d0e4bfb4
📒 Files selected for processing (3)
crates/perry-hir/src/destructuring/var_decl.rscrates/perry-hir/src/lower/expr_new.rscrates/perry/tests/aliased_native_class_import.rs
| let out = Command::new(perry_bin()) | ||
| .current_dir(dir) | ||
| .arg("compile") | ||
| .arg(&entry) | ||
| .arg("--print-hir") | ||
| .arg("-o") | ||
| .arg(&output) | ||
| .output() | ||
| .expect("run perry compile --print-hir"); | ||
| String::from_utf8_lossy(&out.stdout).into_owned() |
There was a problem hiding this comment.
Assert --print-hir compile success before returning output.
print_hir currently returns stdout even if perry compile --print-hir fails, which weakens failure diagnostics in the HIR assertions. Mirror the status/assert pattern used in compile_and_run so failures surface the compiler stderr directly.
Suggested patch
fn print_hir(dir: &Path, source: &str) -> String {
@@
let out = Command::new(perry_bin())
.current_dir(dir)
.arg("compile")
.arg(&entry)
.arg("--print-hir")
.arg("-o")
.arg(&output)
.output()
.expect("run perry compile --print-hir");
+ assert!(
+ out.status.success(),
+ "perry compile --print-hir failed\nstdout:\n{}\nstderr:\n{}",
+ String::from_utf8_lossy(&out.stdout),
+ String::from_utf8_lossy(&out.stderr)
+ );
String::from_utf8_lossy(&out.stdout).into_owned()
}🤖 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/tests/aliased_native_class_import.rs` around lines 70 - 79, The
`print_hir` function is returning stdout without verifying that the `perry
compile --print-hir` command executed successfully. Add a status check after the
`.output()` call to assert the command succeeded, and if it fails, surface the
stderr output for better diagnostics. Mirror the status assertion pattern used
in the `compile_and_run` function to ensure compiler errors are visible when the
HIR compilation fails, rather than silently returning incomplete stdout.
Symptom
import { BlockList as Wj4 } from "net"; const q = new Wj4(); q.addSubnet(...)threwTypeError: addSubnet is not a function, while the un-aliasedimport { BlockList } from "net"worked. Sonew <alias>()wasn't recognized as the native class — it built a generic object whose native methods don't exist.Root cause
Native-class resolution keyed on the local binding name instead of the imported export name at two HIR sites:
crates/perry-hir/src/lower/expr_new.rs(bare-identnew <Ident>()):new Wj4()lowered toExpr::New { class_name: "Wj4" }; codegen's builtin-Newdispatch recognizes classes by literal name ("BlockList","Socket","SocketAddress", …), so"Wj4"missed every arm and produced an empty placeholder.crates/perry-hir/src/destructuring/var_decl.rs:register_native_instancetagged the binding("net","Wj4"), soq.<method>()missed the("net","BlockList")dispatch rows.lookup_native_modulewas already alias-aware (the named-import lowering registerslocal → (module, imported)), but both call sites discarded the imported name.Fix
Both sites resolve the local binding to its imported export name (via
lookup_native_module) before matching/registering — guarded so user classes/locals shadow correctly and non-native user imports don't over-trigger. The aliased form now produces HIR byte-identical to the un-aliased form. General across all aliased native classes (netBlockList/Socket, httpServer, urlURL, …).Validation
crates/perry/tests/aliased_native_class_import.rs(5 tests): URL/Socket/BlockList aliases resolve, aliased==un-aliased HIR, user-import not over-triggered. 5 passed.cargo test -p perry-hir -p perry-codegen: 642 passed, 0 failed.URL as U(.host/.pathname),Socket as Sk,BlockList as Wj4(addSubnet/check).Part of bringing up
@anthropic-ai/claude-codenatively (the bundle imports{ BlockList as Wj4 } from "net").Summary by CodeRabbit
Release Notes
Bug Fixes
Tests
print-hirlowering and runtime behavior for aliased native classes, including positive and negative scenarios.