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

changelog.d/8153-nested-class-name-shadow.md

@@ -0,0 +1,71 @@
+### Fixed
+
+- **`new C()` inside `C`'s own method constructed an unrelated local when an
+  enclosing scope had a same-named binding.** A `class C` declared inside a
+  nested function, referenced by `new C()` from one of its own method bodies,
+  while some enclosing scope also declares `var C` / `let C`, threw
+  `TypeError: undefined is not a constructor` at runtime. Node runs it fine —
+  the class's own name binding is the nearest one.
+
+  Two arms of the lowering disagreed about the same identifier. The bare-ident
+  read arm (`arm_ident.rs`) already applied the JS nearest-binding rule via
+  `forward_class_names` + `forward_class_decl_depth`, so a plain `C` inside the
+  method correctly resolved to `ClassRef("C")` — `typeof C` returned
+  `"function"`. The `new <Ident>` arm did not: it snapshotted
+  `ctx.lookup_local("C")` 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 evaluated to `undefined` and the construct threw.
+
+  The failure is silent up to that point: the class registers, its methods
+  exist, and every reference to the name *other than* `new` resolves correctly.
+
+  Affected files:
+
+  - `crates/perry-hir/src/lower/context.rs` — new
+    `LoweringContext::forward_class_shadows_local`, the nearest-binding rule as
+    one predicate: a local in the CURRENT scope always wins; otherwise the
+    binding at the greater scope depth wins.
+  - `crates/perry-hir/src/lower/lower_expr/arm_ident.rs` — the read arm now
+    calls that predicate instead of carrying its own copy, so the two arms
+    cannot drift again.
+  - `crates/perry-hir/src/lower/expr_new.rs` — suppress the local-callee
+    snapshot (and the later re-lookup that could resurrect it) when the class
+    binding is the nearer one.
+
+  The depth rule is what keeps the case the reroute exists for: a module-scope
+  `class e` does **not** beat a factory-local `let e`, so mysql2's bundled
+  chunk still constructs the local's value.
+
+  Found while triaging #8040 (a production Next.js App Route serving empty
+  bodies with `TypeError: active is not a function`). Next 16 ships this shape
+  in the webpack chunk that inlines `@opentelemetry/api`: the module IIFE
+  declares `var g,h,i,j,…` and later assigns each of them a module-exports
+  object, while an inner factory declares
+  `class i { static getInstance(){ return this._instance || (this._instance = new i), this._instance } active(){ … } }`.
+  `new i` constructed the outer `i` — a plain exports object — so
+  `js_new_function_construct` fell back to a `class_id = 0` empty object.
+  `context` was therefore a non-null object with no prototype at all, which is
+  why the tracer's `(context == null ? void 0 : context.active())` guard let it
+  through and the request died on `active`.
+
+  The same file shows why the symptom looked like a prototype bug and moved
+  under unrelated edits: the collision rename accidentally immunised every
+  DUPLICATE single-letter class (`i$0`, `i$1`, … match no local), so only the
+  first `class <letter>` of each name was affected. In that bundle
+  `ContextAPI` (`class i`) and `PropagationAPI` (`class l`) were broken while
+  `TraceAPI` (`class j`, renamed) worked.
+
+  Validation: `cargo test -p perry-hir` — 314 lib tests plus every integration
+  suite, exit 0. Sabotage: with the guard forced off, both regression tests
+  fail with the exact defect (`NewDynamic { callee: LocalGet(…) }` in the
+  static method's body); the two over-trigger guards pass either way, which is
+  their job.
+
+  End-to-end: a harness built from the fixture's own
+  `.next/server/chunks/2.js` reproduces `TypeError: active is not a function`
+  and, with only the compiler swapped and the runtime archives held fixed,
+  flips to node's output — `context`/`propagation`/`trace`/`diag`/`metrics` all
+  carry their prototypes, `getTracePropagationData()`, `isOpenTelemetryEnabled()`
+  and `trace()` all return node's values.

crates/perry-hir/src/lower/context.rs

@@ -927,6 +927,41 @@ impl LoweringContext {
         Some(depth)
     }
 
+    /// Does a `class <name>` declared in (or lexically enclosing) the body
+    /// being lowered SHADOW every same-named local binding currently visible?
+    ///
+    /// This is the JS nearest-binding rule for the three-way race between a
+    /// class declaration, an outer-scope local of the same name, and a
+    /// sibling-scope class whose name lingers in the inherited
+    /// `forward_class_names` set:
+    ///
+    ///   * a local declared in the CURRENT scope (a param/`var`/`let` next to
+    ///     the reference) always wins — the class cannot be nearer than that;
+    ///   * otherwise the binding at the GREATER scope depth wins, so a class
+    ///     declared inside a nested factory beats a module-scope `var` of the
+    ///     same name, while a module-scope class loses to a factory-local.
+    ///
+    /// Single source of truth for the ident-read arm (`arm_ident.rs`) and the
+    /// `new <Ident>` arm (`expr_new.rs`), which disagreed before #8040: the
+    /// read resolved to the class while `new` still rerouted through the outer
+    /// local's slot.
+    pub(crate) fn forward_class_shadows_local(&self, name: &str) -> bool {
+        if !self.forward_class_names.contains(name) {
+            return false;
+        }
+        if self.lookup_local_in_current_scope(name).is_some() {
+            return false;
+        }
+        match (
+            self.local_decl_scope_depth(name),
+            self.forward_class_decl_depth.get(name).copied(),
+        ) {
+            (None, _) => true,       // no local at all: the class wins
+            (Some(_), None) => true, // depth unknown: keep prior behavior
+            (Some(local_depth), Some(class_depth)) => class_depth > local_depth,
+        }
+    }
+
     /// #5216: drop the most-recently-bound local named `name` (if any), e.g. a
     /// module-var the top-level pre-scan registered for `const ns =
     /// require("<native>")`. After this, a bare read of `name` resolves to its

crates/perry-hir/src/lower/expr_new.rs

@@ -248,8 +248,29 @@ pub(super) fn lower_new(ctx: &mut LoweringContext, new_expr: &ast::NewExpr) -> R
             // intrinsic — a lexical `class Set {}` / `const Set = …` must NOT
             // capture it. Suppress the local snapshot (and the shadow flag
             // below) so the built-in arms fire.
+            // #8040: …but a `class <name>` declared NEARER the reference than
+            // the local wins over it, exactly as the bare-ident read arm
+            // already resolved it (`arm_ident.rs`). The two disagreed: inside a
+            // method body of `class A` declared in a nested factory, a
+            // module-scope `var A` was still visible to `lookup_local`, so
+            // `new A()` rerouted to `NewDynamic { LocalGet(<outer slot>) }` —
+            // and a method compiles to its own function, where that slot index
+            // names an unrelated (undefined) local, so the construct threw
+            // "undefined is not a constructor". Meanwhile a plain `A` read in
+            // the same body correctly resolved to the class. Next 16's webpack
+            // chunk hits this on the bundled `@opentelemetry/api`: the minified
+            // module IIFE declares `var …,i,…` and its inner factory declares
+            // `class i { static getInstance(){ return this._instance ||
+            // (this._instance = new i), this._instance } active(){…} }`, so
+            // `context.active()` was unreachable at request time (#8040).
+            // The depth rule keeps the mysql2 case working: a module-scope
+            // `class e` does NOT beat a factory-local `let e`.
+            let class_shadows_callee_local =
+                !force_global_intrinsic && ctx.forward_class_shadows_local(ident.sym.as_str());
             let callee_local_at_entry: Option<LocalId> = if force_global_intrinsic {
                 None
+            } else if class_shadows_callee_local {
+                None
             } else {
                 ctx.lookup_local(&class_name)
             };
@@ -1288,9 +1309,16 @@ pub(super) fn lower_new(ctx: &mut LoweringContext, new_expr: &ast::NewExpr) -> R
             if ctx.lookup_class(&class_name).is_none()
                 && ctx.resolve_class_alias(&class_name).is_none()
             {
-                if let Some(local_id) =
-                    callee_local_at_entry.or_else(|| ctx.lookup_local(&class_name))
-                {
+                if let Some(local_id) = callee_local_at_entry.or_else(|| {
+                    // #8040: the same nearer-class rule as the snapshot above —
+                    // a re-lookup here must not resurrect the outer local the
+                    // class shadows.
+                    if class_shadows_callee_local {
+                        None
+                    } else {
+                        ctx.lookup_local(&class_name)
+                    }
+                }) {
                     return Ok(Expr::NewDynamic {
                         callee: Box::new(Expr::LocalGet(local_id)),
                         args,

crates/perry-hir/src/lower/lower_expr/arm_ident.rs

@@ -49,32 +49,23 @@ pub(crate) fn lower_ident_expr(ctx: &mut LoweringContext, ident: &ast::Ident) ->
     // of the class, so `new r.TimeoutError` threw "undefined is not a
     // constructor". Gate on there being NO current-scope local of that
     // name (a sibling param/var/let still wins).
-    if ctx.forward_class_names.contains(&name) && ctx.lookup_local_in_current_scope(&name).is_none()
-    {
-        // A `class <name>` in `forward_class_names` shadows a SAME-named local
-        // only when JS lexical scoping says the class binding is the nearest:
-        // i.e. there is no local of that name at all, OR the class was declared
-        // at a scope depth deeper than (nearer the reference than) the nearest
-        // enclosing local. The class binding lives in whatever function body
-        // declared it; a captured local declared in a NEARER scope (a deeper
-        // or sibling function whose local lingered in the inherited set) must
-        // win. Without this depth check, a `class <name>` in a SIBLING factory
-        // (its name still present in the inherited `forward_class_names`)
-        // wrongly shadowed a legitimate captured local — Next.js
-        // app-page-turbo's route-render closure read its captured params local
-        // `ej` as the `class ej` (= NextURL) reference, which then flowed into
-        // a WeakMap key and threw "Invalid value used as weak map key".
-        let class_wins = match (
-            ctx.local_decl_scope_depth(&name),
-            ctx.forward_class_decl_depth.get(&name).copied(),
-        ) {
-            (None, _) => true,       // no local: class wins (TimeoutError etc.)
-            (Some(_), None) => true, // depth unknown: keep prior behavior
-            (Some(local_depth), Some(class_depth)) => class_depth > local_depth,
-        };
-        if class_wins {
-            return Ok(Expr::ClassRef(ctx.resolve_class_name(&name)));
-        }
+    // A `class <name>` in `forward_class_names` shadows a SAME-named local
+    // only when JS lexical scoping says the class binding is the nearest: no
+    // local of that name at all, or the class declared at a scope depth deeper
+    // than (nearer the reference than) the nearest enclosing local, and no
+    // sibling param/var of that name in the CURRENT scope. The class binding
+    // lives in whatever function body declared it; a captured local declared in
+    // a NEARER scope (a deeper or sibling function whose local lingered in the
+    // inherited set) must win. Without the depth check, a `class <name>` in a
+    // SIBLING factory (its name still present in the inherited
+    // `forward_class_names`) wrongly shadowed a legitimate captured local —
+    // Next.js app-page-turbo's route-render closure read its captured params
+    // local `ej` as the `class ej` (= NextURL) reference, which then flowed
+    // into a WeakMap key and threw "Invalid value used as weak map key".
+    // (`forward_class_shadows_local` is shared with the `new <Ident>` arm; see
+    // #8040 for what happened while the two disagreed.)
+    if ctx.forward_class_shadows_local(&name) {
+        return Ok(Expr::ClassRef(ctx.resolve_class_name(&name)));
     }
     // Chained-assignment class self-alias referenced from inside one of the
     // class's own method/getter/setter bodies. tsc's decorator-capture form