fix(hir): forward inherited captures through a locally-shadowed class parent

changelog.d/10628-class-expr-subclass-captures.md

@@ -0,0 +1,21 @@
+### Fixed
+
+- **An inherited method of a capture-bearing class expression could read
+  `undefined` captures when called on a subclass instance.** A subclass
+  with no explicit constructor, extending a capture-bearing class
+  EXPRESSION bound to a local (`const Base = class { m() { return cap; } };
+  const Sub = class extends Base { n() {...} };`), never forwarded `Base`'s
+  captured enclosing-scope locals to the synthesized subclass constructor —
+  the lowering deliberately drops the static `extends_name` for such a
+  lexically-local heritage identifier (avoiding a same-named-class
+  collision, #5437), and capture propagation was keyed off that same name.
+  Capture forwarding now resolves the heritage identifier through the same
+  let/const class-alias table `new X()` construction already uses, scoped
+  to subclasses that have their own member and no explicit constructor of
+  their own (an explicit constructor already forwarded captures correctly
+  through a separate mechanism). This was blocking `typescript`'s CJS
+  `transpileModule` output (`IdentifierNameMultiMap extends
+  IdentifierNameMap`, both class expressions with their own methods, in the
+  bundled `typescript.js`). A subclass with a completely empty body, or
+  whose base is a class DECLARATION rather than expression, hits a
+  separate, still-open codegen field-layout gap (#10486).

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

@@ -1992,5 +1992,6 @@ mod function_ctor_runtime_routing;
 mod mixin_parent_chain;
 mod native_module_sync;
 
+mod class_expr_subclass_captures;
 mod nullish_over_optional_chain;
 mod ui_widget_add_child;

crates/perry-hir/src/lower/tests/class_expr_subclass_captures.rs

@@ -0,0 +1,80 @@
+//! #10486: a class extending a capture-bearing class EXPRESSION held in a
+//! local (`const Base = class { m() { return cap; } }; class Sub extends
+//! Base {}`) must forward the base's captured locals to the synthesized
+//! subclass constructor, even though `extends_name` is deliberately left
+//! `None` for this lexically-local heritage shape (see the #5437 PQueue
+//! fix). Split from `tests.rs` for the 2000-line file cap.
+
+/// The minimal repro from #10486: a subclass EXPRESSION with no own
+/// constructor extending a base class EXPRESSION, both capturing distinct
+/// enclosing-function locals. The `new Sub()` construction site must
+/// forward BOTH captured ids (the base's and the subclass's own), not just
+/// the subclass's own capture.
+#[test]
+fn subclass_of_local_class_expr_forwards_base_captures() {
+    let source = r#"
+        function outer() {
+            const baseCap = "base-capture";
+            const subCap = "sub-capture";
+            const Base = class { m() { return baseCap; } };
+            const Sub = class extends Base { n() { return subCap; } };
+            return new Sub().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 outer = hir
+        .functions
+        .iter()
+        .find(|f| f.name == "outer")
+        .expect("fixture declares function outer");
+    let compact: String = format!("{:?}", outer.body)
+        .chars()
+        .filter(|ch| !ch.is_whitespace())
+        .collect();
+    // The `new Sub()` site must append TWO forwarded capture args (the
+    // subclass's own `subCap` plus the inherited `baseCap`), not one.
+    assert!(
+        compact.contains("cap_args_appended:2"),
+        "expected new Sub() to forward both the base and subclass captures \
+         (cap_args_appended: 2); got body: {compact}"
+    );
+}
+
+// NOTE: a subclass DECLARATION (as opposed to expression) extending a local
+// class expression is a separate lowering path (`Expr::NewDynamic` with a
+// `RegisterClassCaptures`/`RefreshClassExprCaptures`-based shared-box
+// capture mechanism, not `Expr::New{cap_args_appended}`) that this fix does
+// not cover — left as a known gap, see the PR body for #10486.
+
+/// A subclass that captures nothing of its own, extending a capture-bearing
+/// local class expression, must still forward the base's capture (a
+/// regression the naive "skip if the child's own union is empty" shape
+/// would have reintroduced).
+#[test]
+fn subclass_with_no_own_captures_still_forwards_base_captures() {
+    let source = r#"
+        function outer() {
+            const cap = "only-base-cap";
+            const Base = class { m() { return cap; } };
+            const Sub = class extends Base {};
+            return new Sub().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 outer = hir
+        .functions
+        .iter()
+        .find(|f| f.name == "outer")
+        .expect("fixture declares function outer");
+    let compact: String = format!("{:?}", outer.body)
+        .chars()
+        .filter(|ch| !ch.is_whitespace())
+        .collect();
+    assert!(
+        compact.contains("cap_args_appended:1"),
+        "expected new Sub() to forward the base's capture even though Sub \
+         itself captures nothing; got body: {compact}"
+    );
+}

crates/perry-hir/src/lower_decl/class_decl.rs

@@ -414,6 +414,61 @@ pub fn lower_class_decl(
         (None, None, None, None)
     };
 
+    // Issue #10486: the branches above deliberately leave `extends_name`
+    // None when the heritage identifier resolves to a lexically-scoped
+    // local (`locally_shadowed`) or a fully dynamic expression, to avoid
+    // corrupting the static class-registry walks (instanceof / method
+    // dispatch / field layout — see the `locally_shadowed` comment above,
+    // #5437's PQueue regression). Capture forwarding is narrower and
+    // already tolerates a wrong/missing match (`lookup_class_captures`
+    // returns `None` and `synthesize_class_captures` is then a no-op for
+    // that source), so resolve the heritage identifier through
+    // `resolve_class_alias` — the SAME table `Expr::New`'s own capture
+    // lookup already uses (`expr_new.rs`) for `let X = class {...}; new
+    // X()`, populated when `const X = class {...}`/`let X = Y` is lowered
+    // (`register_let_class_alias`) — rather than a raw name match: a class
+    // extending a capture-bearing class EXPRESSION held in a local
+    // (`const Base = class { m() { return cap; } }; class Sub extends
+    // Base {}`) still finds and forwards `Base`'s captures at
+    // construction. Without this, every inherited method read `undefined`
+    // for the base's captures because the synthesized subclass
+    // constructor never received them as params. A raw-text match (or
+    // `resolve_class_name`, which only disambiguates same-named class
+    // DECLARATIONS) would reintroduce exactly the #5437 same-named-local
+    // collision for a minified bundle where two unrelated functions each
+    // declare their own `const Base = class {...}`.
+    // Only fall back for a subclass with NO explicit constructor of its
+    // own: an explicit constructor's own `super(...)` call already forwards
+    // whatever the parent needs via a SEPARATE, already-correct mechanism
+    // (the issue's own "Works" list: "an explicit `constructor() {
+    // super(); }` in the subclass" — confirmed by probing that case against
+    // a build without this fallback). Widening the union unconditionally
+    // regressed it: the parent capture then also lands in THIS class's own
+    // `captures_vec`, and the auto-stash machinery below expects to own
+    // forwarding an inherited cap into `super(...)` only for the
+    // SYNTHESIZED default constructor shape, not a user-written one.
+    let has_own_constructor = class_decl
+        .class
+        .body
+        .iter()
+        .any(|m| matches!(m, ast::ClassMember::Constructor(_)));
+    let capture_parent_name: Option<String> = extends_name.clone().or_else(|| {
+        if has_own_constructor {
+            return None;
+        }
+        class_decl
+            .class
+            .super_class
+            .as_deref()
+            .and_then(|sc| match sc {
+                ast::Expr::Ident(ident) => {
+                    let raw = ident.sym.to_string();
+                    Some(ctx.resolve_class_alias(&raw).unwrap_or(raw))
+                }
+                _ => None,
+            })
+    });
+
     // First pass: collect static field/method names for early registration
     // This allows static method bodies to reference static fields
     let mut static_field_names = Vec::new();
@@ -1120,7 +1175,7 @@ pub fn lower_class_decl(
     synthesize_class_captures(
         ctx,
         &name,
-        extends_name.as_deref(),
+        capture_parent_name.as_deref(),
         extends.is_some()
             || extends_name.is_some()
             || native_extends.is_some()
@@ -1435,6 +1490,33 @@ pub fn lower_class_from_ast(
         (None, None, None, None)
     };
 
+    // Issue #10486: mirrors the capture-forwarding fallback in
+    // `lower_class_decl` above (see its comment for the full rationale) —
+    // a class EXPRESSION extending a lexically-local capture-bearing class
+    // EXPRESSION (`const Base = class {…}; const Sub = class extends Base
+    // {…}`) needs the alias-resolved heritage identifier for capture
+    // lookup even when `extends_name` was deliberately left None for
+    // class-registry resolution.
+    // See the matching guard in `lower_class_decl` above: skip the
+    // fallback when this class expression has its own explicit
+    // constructor (its `super(...)` already forwards correctly).
+    let has_own_constructor = class
+        .body
+        .iter()
+        .any(|m| matches!(m, ast::ClassMember::Constructor(_)));
+    let capture_parent_name: Option<String> = extends_name.clone().or_else(|| {
+        if has_own_constructor {
+            return None;
+        }
+        class.super_class.as_deref().and_then(|sc| match sc {
+            ast::Expr::Ident(ident) => {
+                let raw = ident.sym.to_string();
+                Some(ctx.resolve_class_alias(&raw).unwrap_or(raw))
+            }
+            _ => None,
+        })
+    });
+
     let mut static_field_names = Vec::new();
     let mut static_method_names = Vec::new();
     for member in &class.body {
@@ -1829,7 +1911,7 @@ pub fn lower_class_from_ast(
     synthesize_class_captures(
         ctx,
         name,
-        extends_name.as_deref(),
+        capture_parent_name.as_deref(),
         extends.is_some()
             || extends_name.is_some()
             || native_extends.is_some()

test-files/test_gap_10486_class_expr_subclass_captures.ts

@@ -0,0 +1,93 @@
+// #10486: an inherited method of a capture-bearing class EXPRESSION saw
+// `undefined` captures when called on an instance of a capture-bearing
+// `class extends` subclass with no explicit constructor.
+//
+// `lower_class_decl`/`lower_class_from_ast` (crates/perry-hir/src/lower_decl/
+// class_decl.rs) deliberately leave `extends_name` at `None` when the
+// heritage identifier resolves to a lexically-scoped local (`locally_
+// shadowed`) rather than a statically-registered class declaration — the
+// #5437 PQueue fix, avoiding a retained name being re-resolved by the
+// static parent-chain walks to an unrelated same-named class.
+// `synthesize_class_captures` (lower_decl/class_captures.rs) unions a
+// parent's registered captures into the child's synthesized-constructor
+// params keyed off that SAME `extends_name`; when it's `None`, the union
+// never ran, so the subclass constructor never received the base's
+// captured locals and every inherited method read `undefined` for them.
+//
+// This is exactly the shape `const Base = class {...}; const Sub = class
+// extends Base {...}` produces (both class EXPRESSIONS assigned to
+// locals) — esbuild/tsc's typical bundled-class emit, and what broke
+// typescript 5.8.2's CJS `transpileModule` (`IdentifierNameMultiMap
+// extends IdentifierNameMap`, both class expressions in `typescript.js`'s
+// module wrapper, the subclass with its own `add`/`remove` methods).
+//
+// NOTE: this fix is scoped to a subclass that has at least one member of
+// its own (a method, in this file) and NO explicit constructor of its own
+// — see `explicitCtor` below, the pre-existing "already works" control
+// this fix must not regress. A subclass with a completely empty body
+// (`const Sub = class extends Base {};`), or a base class DECLARATION
+// (rather than expression) as the extends target, hits a SEPARATE, unfixed
+// codegen field-layout bug — see the PR body for #10486.
+//
+// Each function below uses its own distinct Base/Sub identifier names
+// (Base1/Sub1, Base2/Sub2, ...): re-using the same literal name across
+// sibling functions hits an unrelated, pre-existing collision in the
+// class-capture registries (confirmed present on a build with NONE of this
+// PR's changes) that is out of this PR's scope.
+
+function minimal(): void {
+  const baseCap = "base-capture";
+  const subCap = "sub-capture";
+  const Base1 = class {
+    m() {
+      return baseCap;
+    }
+  };
+  const Sub1 = class extends Base1 {
+    n() {
+      return subCap;
+    }
+  };
+  console.log("minimal", new Base1().m(), new Sub1().m(), new Sub1().n());
+}
+minimal();
+
+// Control: an explicit `constructor() { super(); }` on the subclass must
+// keep working (pre-existing "Works" case; a naive union of parent
+// captures into every subclass regressed exactly this shape during
+// development of this fix — kept here as the regression guard).
+function explicitCtor(): void {
+  const cap2 = "explicit-super-cap";
+  const Base2 = class {
+    m() {
+      return cap2;
+    }
+  };
+  const Sub2 = class extends Base2 {
+    constructor() {
+      super();
+    }
+  };
+  console.log("explicitCtor", new Sub2().m());
+}
+explicitCtor();
+
+// A captured HELPER function (not just a string) read from an inherited
+// method on a subclass instance; subclass has its own (uncaptured) member.
+function capturedHelper(): void {
+  function helper3(x: string): string {
+    return "[" + x + "]";
+  }
+  const Base3 = class {
+    m() {
+      return helper3("base");
+    }
+  };
+  const Sub3 = class extends Base3 {
+    own() {
+      return "own";
+    }
+  };
+  console.log("capturedHelper", new Sub3().m(), new Sub3().own());
+}
+capturedHelper();