fix(hir): a nested class's own name beats an enclosing binding in new

[proggeramlug]

Refs #8040.

What breaks

A class C declared inside a nested function, constructed by new C() from one of its own method bodies, throws TypeError: undefined is not a constructor whenever some enclosing scope also declares a binding named C. Node runs it — the class's own name binding is the nearest one.

var A: any;                       // enclosing binding, never assigned
const g = () => {
  class A {
    static mk() { return new A(); }   // <- throws under Perry
    m() { return "ok"; }
  }
  return A;
};
console.log(g().mk().m());        // node: "ok"   perry: TypeError: undefined is not a constructor

Root cause

Two arms of ident lowering disagreed about the same name.

The bare-ident read arm (arm_ident.rs) already applied the JS nearest-binding rule through forward_class_names + forward_class_decl_depth, so a plain A inside the method resolved to ClassRef("A") — typeof A in that same method returns "function".

The new <Ident> arm did not. It snapshotted ctx.lookup_local("A") unconditionally, found the enclosing scope's binding, and rerouted the construct to NewDynamic { callee: LocalGet(<outer slot>) }. A method compiles to its own function, so that slot index names an unrelated, uninitialized local there; the callee evaluates to undefined and the construct throws.

Everything up to that point is silent: the class registers, its methods exist, and every reference to the name other than new resolves correctly.

The fix

Extract the rule into LoweringContext::forward_class_shadows_local and call it from both arms so they cannot drift again:

• a local declared in the CURRENT scope always wins (a sibling param/var is nearer than any class);
• otherwise the binding at the greater scope depth wins.

The depth half is what preserves the case the reroute exists for: a module-scope class e still loses to a factory-local let e, so mysql2's bundled chunk keeps constructing the local's value. That direction has its own test in this PR.

Why it matters

Next 16 ships exactly this shape in the webpack chunk that inlines @opentelemetry/api: the module IIFE declares var g,h,i,j,… and an inner factory declares

class i {
  static getInstance(){ return this._instance || (this._instance = new i), this._instance }
  active(){ return this._getContextManager().active() }
}

getInstance() threw, so the module factory aborted mid-initialization; webpack's module cache then handed the tracer a {} for @opentelemetry/api, and context never got its active().

Validation

• cargo test -p perry-hir — 312 lib tests plus every integration suite, all green, exit 0.

• Sabotage: with the new guard forced off, nested_class_shadowing_outer_var_constructs_the_class_not_the_local fails printing the exact defect:

  `new A()` inside A's own method must not construct through an enclosing-scope local slot: [
      Return(Some(NewDynamic { callee: LocalGet(0), args: [], byte_offset: 132 })),
  ]
  

  The companion test passes either way by design — it exists to catch over-triggering, not to detect this bug.

• cargo fmt --all -- --check and scripts/check_file_size.sh clean.

Summary by CodeRabbit

• Bug Fixes

  • Fixed nested class construction when a class name conflicts with an outer local variable.
  • Ensured new expressions resolve to the nearest valid class or local binding.
  • Preserved expected behavior for factory- and method-local variables that intentionally shadow class names.

• Tests

  • Added regression coverage for nested classes, repeated class names, and local shadowing scenarios.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 04b64f45-68a9-4600-843d-c62ae6470a9a

📥 Commits

Reviewing files that changed from the base of the PR and between 499e296 and 72c8b74.

📒 Files selected for processing (5)

• changelog.d/8153-nested-class-name-shadow.md
• crates/perry-hir/src/lower/context.rs
• crates/perry-hir/src/lower/expr_new.rs
• crates/perry-hir/src/lower/lower_expr/arm_ident.rs
• crates/perry-hir/src/lower/tests.rs

Included review availability: Your plan includes up to 8 reviews per rolling hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

Nested class-name resolution now compares class and local declaration depth. Identifier and new lowering use the shared predicate. Regression tests cover nested classes, valid local shadowing, repeated class names, and dynamic construction.

Changes

Nested class shadowing

Layer / File(s)	Summary
Centralized binding resolution crates/perry-hir/src/lower/context.rs, crates/perry-hir/src/lower/lower_expr/arm_ident.rs	forward_class_shadows_local centralizes class-versus-local resolution and applies lexical declaration depth. Identifier lowering uses the shared predicate.
Constructor lowering integration crates/perry-hir/src/lower/expr_new.rs	new lowering suppresses an unrelated outer local when a nearer class declaration exists. The fallback path no longer restores that local binding.
Regression coverage and changelog crates/perry-hir/src/lower/tests.rs, changelog.d/8153-nested-class-name-shadow.md	Tests cover nested classes, multiple same-named classes, factory-local precedence, and method-local dynamic construction. The changelog records the fix and validation.Estimated code review effort: 3 (Moderate)

Possibly related PRs

• PerryTS/perry#6730: Both changes adjust constructor resolution in expr_new.rs, but this PR handles nested class shadowing.
• PerryTS/perry#8077: Both changes update HIR binding resolution and expr_new.rs, but this PR handles class bindings rather than proxies.

Suggested reviewers: thehypnoo, jdalton

Merge Risk: ⚪ Minimal · up to 72c8b

This localized fix aligns nested-class construction with JavaScript name resolution and includes targeted regression tests; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary fix for nested class-name shadowing in new expressions.
Description check	✅ Passed	The description clearly explains the bug, root cause, fix, related issue, regression tests, and validation results.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/8040-nested-class-name-shadow

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[proggeramlug]

End-to-end on the #8040 fixture

Measured on tests/release/packages/next-app-route (copy of the fixture; app compiled as an executable rather than the dylib+provider split, since the provider crates are not on main).

main alone cannot start the fixture — Error: Cannot find module './chunks/2.js' — so the run below is this branch merged with #8146 (fix/8040-runtime-relative-chunk-require), which is the startup half.

	before	with this fix + #8146
startup	PERRY_NEXT_APP_ROUTE_READY	same
per request	TypeError: active is not a function, HTTP 500, empty body	no active error anywhere, HTTP 200, empty body after ~5 min
harness assertion	not reached	generated handler bypassed routeModule.handle

So the tracer failure this PR targets is gone from the real app. What is behind it is a different defect, not a variant of this one:

1. The response body is never written. GET /api/benchmark?id=… answers 200 with content-length: 0 after ~5 minutes; node's oracle answers 207 with the JSON payload immediately. (With perry-host.js's own guard left as a throw, that same request is a 500 — the 500 is the harness's catch, not the route.)
2. A property write on the required routeModule object is not observed by the generated handler. perry-host.js does routeModule.handle = async (req, ctx) => { … } over the binding it got from require("./.next/server/app/api/benchmark/route.js"), and the compiled handler never enters it — 20/20 requests. That is what its bypass guard reports.

Neither reproduces in the otel harness this PR is validated against; they want their own issue.