From e86568a8d82d1956eaf19a93ec7dada20235b2c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 15 Aug 2026 11:25:28 +0200 Subject: [PATCH 1/2] fix(hir): a nested class's own name beats an enclosing binding in `new` `class C` declared inside a nested function, constructed by `new C()` from one of its own method bodies, threw `TypeError: undefined is not a constructor` whenever an enclosing scope also declared `var C` / `let C`. The bare-ident read arm already applied the JS nearest-binding rule, so a plain `C` in the same method resolved to the class. The `new ` arm did not: it snapshotted `lookup_local("C")` unconditionally and rerouted the construct to `NewDynamic { LocalGet() }`. A method compiles to its own function, where that slot index names an unrelated uninitialized local, so the callee evaluated to `undefined`. Extract the rule into `LoweringContext::forward_class_shadows_local` and use it from both arms so they cannot drift again. The depth half of the rule keeps the case the reroute exists for: a module-scope `class e` still loses to a factory-local `let e`. Next 16's webpack chunk for the bundled `@opentelemetry/api` is this shape, which is why a production App Route could not serve a request (#8040). --- changelog.d/8153-nested-class-name-shadow.md | 55 +++++++++++ crates/perry-hir/src/lower/context.rs | 35 +++++++ crates/perry-hir/src/lower/expr_new.rs | 34 ++++++- .../src/lower/lower_expr/arm_ident.rs | 43 ++++----- crates/perry-hir/src/lower/tests.rs | 93 +++++++++++++++++++ 5 files changed, 231 insertions(+), 29 deletions(-) create mode 100644 changelog.d/8153-nested-class-name-shadow.md diff --git a/changelog.d/8153-nested-class-name-shadow.md b/changelog.d/8153-nested-class-name-shadow.md new file mode 100644 index 0000000000..6aa8fa64d6 --- /dev/null +++ b/changelog.d/8153-nested-class-name-shadow.md @@ -0,0 +1,55 @@ +### 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 ` arm did not: it snapshotted + `ctx.lookup_local("C")` unconditionally, found the *enclosing* scope's + binding, and rerouted the construct to + `NewDynamic { callee: LocalGet() }`. 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). Next 16 ships 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(){ … } }`. + `getInstance()` threw, the module factory aborted mid-initialization, and the + webpack module cache then handed the tracer a `{}` for + `@opentelemetry/api` — so `context` never got its `active()`. + + Validation: `cargo test -p perry-hir` — 312 lib tests + every integration + suite green. Sabotage: with the new guard forced off, the regression test + fails with the exact defect, `NewDynamic { callee: LocalGet(0) }` in the + static method's body. The companion test (`factory_local_still_shadows_…`) + passes either way by design — it exists to catch over-triggering, not to + detect this bug. diff --git a/crates/perry-hir/src/lower/context.rs b/crates/perry-hir/src/lower/context.rs index 1c034a0d7f..0454d87a7c 100644 --- a/crates/perry-hir/src/lower/context.rs +++ b/crates/perry-hir/src/lower/context.rs @@ -927,6 +927,41 @@ impl LoweringContext { Some(depth) } + /// Does a `class ` 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 ` 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("")`. After this, a bare read of `name` resolves to its diff --git a/crates/perry-hir/src/lower/expr_new.rs b/crates/perry-hir/src/lower/expr_new.rs index f7b13f5892..ad3897f44c 100644 --- a/crates/perry-hir/src/lower/expr_new.rs +++ b/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 ` 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() }` — + // 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 = 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, diff --git a/crates/perry-hir/src/lower/lower_expr/arm_ident.rs b/crates/perry-hir/src/lower/lower_expr/arm_ident.rs index cb427ff9c7..a63fdc7ded 100644 --- a/crates/perry-hir/src/lower/lower_expr/arm_ident.rs +++ b/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 ` 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 ` 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 ` 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 ` 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 ` 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 diff --git a/crates/perry-hir/src/lower/tests.rs b/crates/perry-hir/src/lower/tests.rs index 1103a3a791..b89315c8bf 100644 --- a/crates/perry-hir/src/lower/tests.rs +++ b/crates/perry-hir/src/lower/tests.rs @@ -759,3 +759,96 @@ fn test_named_class_expression_var_decl_reports_explicit_name() { "anonymous class expression uses the inferred binding name, no override" ); } + +/// #8040: a `class A` declared inside a nested factory, referenced by `new A()` +/// from one of its OWN method bodies, while a same-named binding (`var A`) +/// exists in an enclosing scope. +/// +/// `expr_new.rs` snapshotted `ctx.lookup_local("A")` unconditionally and, when +/// it hit, rerouted the construct to `NewDynamic { callee: LocalGet() }`. A method compiles to its own function, so that slot index names +/// an unrelated (undefined) local there and the construct threw `TypeError: +/// undefined is not a constructor` at runtime. The bare-ident read arm already +/// resolved the same name to the class via `forward_class_shadows_local`; this +/// makes `new` agree. +/// +/// Next 16's webpack chunk for the bundled `@opentelemetry/api` is exactly this +/// shape — `var …,i,…` in the module IIFE and `class i { static getInstance(){ +/// return this._instance || (this._instance = new i), this._instance } }` in an +/// inner factory — so `context.active()` was unreachable at request time. +#[test] +fn nested_class_shadowing_outer_var_constructs_the_class_not_the_local() { + let source = r#" + var A: any; + const g = () => { + class A { + static mk(): any { + return new A(); + } + m(): string { + return "ok"; + } + } + return A; + }; + const out: any = g().mk().m(); + "#; + let module = perry_parser::parse_typescript(source, "t.ts").expect("source parses"); + let hir = super::lower_module(&module, "t", "t.ts").expect("source lowers"); + + let mk = hir + .classes + .iter() + .find(|c| c.name == "A") + .expect("class A is lowered") + .static_methods + .iter() + .find(|m| m.name == "mk") + .expect("static method mk is lowered"); + let body = format!("{:#?}", mk.body); + + assert!( + !body.contains("NewDynamic"), + "`new A()` inside A's own method must not construct through an \ + enclosing-scope local slot: {body}" + ); + assert!( + body.contains("class_name: \"A\""), + "`new A()` inside A's own method must construct class A: {body}" + ); +} + +/// Companion (the case the depth rule must NOT break): a module-scope `class e` +/// and a factory-local `let e` holding a different constructor. JS says the +/// nearer local wins, so `new e()` inside the factory must still construct the +/// LOCAL's value — mysql2's bundled chunk shape, where taking the class instead +/// silently ran the wrong constructor. +#[test] +fn factory_local_still_shadows_module_scope_class_in_new() { + let source = r#" + class e { + tag(): string { return "class-e"; } + } + function make(): any { + const e: any = function () { return undefined; }; + return new e(); + } + const keep: any = e; + const out: any = make(); + "#; + let module = perry_parser::parse_typescript(source, "t.ts").expect("source parses"); + let hir = super::lower_module(&module, "t", "t.ts").expect("source lowers"); + + let make = hir + .functions + .iter() + .find(|f| f.name == "make") + .expect("function make is lowered"); + let body = format!("{:#?}", make.body); + + assert!( + body.contains("NewDynamic"), + "a factory-local binding must keep shadowing a module-scope class of \ + the same name for `new`: {body}" + ); +} From 72c8b7413cf0bd07ad056dd09130982f99cc1eca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 15 Aug 2026 12:39:54 +0200 Subject: [PATCH 2/2] test(hir): cover the many-same-named-classes shape from the otel bundle The collision rename accidentally immunises every duplicate single-letter class, so only the first `class ` of a name reaches the reroute. That asymmetry is why the bundled @opentelemetry/api lost `context` and `propagation` but kept `trace`, and why the symptom moves when unrelated code is added to the file. Add that shape plus an over-trigger guard for a method-scope local named after its own class. --- changelog.d/8153-nested-class-name-shadow.md | 42 ++++--- crates/perry-hir/src/lower/tests.rs | 109 +++++++++++++++++++ 2 files changed, 138 insertions(+), 13 deletions(-) diff --git a/changelog.d/8153-nested-class-name-shadow.md b/changelog.d/8153-nested-class-name-shadow.md index 6aa8fa64d6..5f2741d959 100644 --- a/changelog.d/8153-nested-class-name-shadow.md +++ b/changelog.d/8153-nested-class-name-shadow.md @@ -39,17 +39,33 @@ chunk still constructs the local's value. Found while triaging #8040 (a production Next.js App Route serving empty - bodies). Next 16 ships this shape in the webpack chunk that inlines - `@opentelemetry/api`: the module IIFE declares `var g,h,i,j,…` and an inner - factory declares + 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(){ … } }`. - `getInstance()` threw, the module factory aborted mid-initialization, and the - webpack module cache then handed the tracer a `{}` for - `@opentelemetry/api` — so `context` never got its `active()`. - - Validation: `cargo test -p perry-hir` — 312 lib tests + every integration - suite green. Sabotage: with the new guard forced off, the regression test - fails with the exact defect, `NewDynamic { callee: LocalGet(0) }` in the - static method's body. The companion test (`factory_local_still_shadows_…`) - passes either way by design — it exists to catch over-triggering, not to - detect this bug. + `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 ` 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. diff --git a/crates/perry-hir/src/lower/tests.rs b/crates/perry-hir/src/lower/tests.rs index b89315c8bf..3c3f69c9d1 100644 --- a/crates/perry-hir/src/lower/tests.rs +++ b/crates/perry-hir/src/lower/tests.rs @@ -852,3 +852,112 @@ fn factory_local_still_shadows_module_scope_class_in_new() { the same name for `new`: {body}" ); } + +/// #8040, the shape the minified `@opentelemetry/api` bundle actually has: a +/// file with MANY same-named single-letter classes over one outer `var i`. +/// +/// The collision rename accidentally immunised every duplicate — `i$0`, `i$1`, +/// … match no local, so `lookup_local` missed and the reroute never fired for +/// them. Only the FIRST `class i`, the one that keeps the bare name, was +/// broken. That asymmetry is why the bundle's `trace` API worked while its +/// `context` and `propagation` APIs did not, and why a symptom that looks like +/// "prototype methods are missing" moves when unrelated code is added to the +/// file. All three must construct their own class. +#[test] +fn first_of_several_same_named_nested_classes_constructs_itself() { + let source = r#" + function t(n: string, f: () => any): void { + try { console.log(n + ": " + String(f())); } catch (e) { console.log(String(e)); } + } + var i: any; + const f1 = () => { + class i { + static mk(): any { return new i(); } + m(): string { return "one"; } + } + return i; + }; + const f2 = () => { + class i { + static mk(): any { return new i(); } + m(): string { return "two"; } + } + return i; + }; + const f3 = () => { + class i { + static mk(): any { return new i(); } + m(): string { return "three"; } + } + return i; + }; + t("f1", () => f1().mk().m()); + t("f2", () => f2().mk().m()); + t("f3", () => f3().mk().m()); + "#; + let module = perry_parser::parse_typescript(source, "t.ts").expect("source parses"); + let hir = super::lower_module(&module, "t", "t.ts").expect("source lowers"); + + // The first `class i` keeps the bare name; the duplicate is renamed. + let first = hir + .classes + .iter() + .find(|c| c.name == "i") + .expect("the first class keeps the bare name `i`"); + let mk = first + .static_methods + .iter() + .find(|m| m.name == "mk") + .expect("static method mk is lowered"); + let body = format!("{:#?}", mk.body); + + assert!( + !body.contains("NewDynamic"), + "`new i()` inside i's own method must not construct through the \ + enclosing binding's slot: {body}" + ); + assert!( + body.contains("class_name: \"i\""), + "`new i()` inside i's own method must construct class i: {body}" + ); +} + +/// Over-trigger guard: a binding declared in the METHOD's own scope still wins. +/// `m() { const C = Other; return new C(); }` constructs `Other`, not the +/// enclosing class — `lookup_local_in_current_scope` is what keeps that true. +#[test] +fn method_local_shadowing_the_class_name_still_wins_in_new() { + let source = r#" + class Other { + tag(): string { return "other"; } + } + const g = () => { + class C { + static mk(): any { + const C: any = Other; + return new C(); + } + } + return C; + }; + const out: any = g().mk(); + "#; + let module = perry_parser::parse_typescript(source, "t.ts").expect("source parses"); + let hir = super::lower_module(&module, "t", "t.ts").expect("source lowers"); + + let mk = hir + .classes + .iter() + .find(|c| c.name == "C") + .expect("class C is lowered") + .static_methods + .iter() + .find(|m| m.name == "mk") + .expect("static method mk is lowered"); + let body = format!("{:#?}", mk.body); + + assert!( + body.contains("NewDynamic"), + "a method-scope local named after the class must still win for `new`: {body}" + ); +}