fix(hir): exclude inherited method names from ctor-body field detection

changelog.d/10626-subclass-ctor-inherited-method.md

@@ -0,0 +1,17 @@
+### Fixed
+
+- **A subclass constructor assigning `this.m = ...` over an inherited method
+  hid that method from the moment `super()` returned, not just before the
+  assignment ran.** The constructor-body field-detection pass excluded a
+  class's OWN methods from being turned into shadow data slots (so
+  `this.parse = this.parse.bind(this)` self-binding kept working), but never
+  excluded methods inherited from the `extends` chain — so
+  `this.close = () => ...` in a subclass constructor, where `close` is
+  declared only on a parent class, allocated an own `close` field that
+  existed (as `undefined`) as soon as `super()` returned and shadowed the
+  inherited method in every by-name lookup until the assignment statement
+  executed. Instance method names are now tracked as an own+inherited union
+  per class (mirroring the existing accessor-name tracking) and consulted the
+  same way. This was blocking `undici`'s `MockPool`/`MockClient`
+  (`this[kOriginalClose] = this.close.bind(this)` over `DispatcherBase`'s
+  `close`), which threw `TypeError: Bind must be called on a function`.

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

@@ -88,6 +88,7 @@ impl LoweringContext {
             class_statics: Vec::new(),
             class_field_names: HashMap::new(),
             class_accessor_names: HashMap::new(),
+            class_method_names: HashMap::new(),
             class_native_extends: Vec::new(),
             class_field_types: HashMap::new(),
             enums: Vec::new(),
@@ -591,10 +592,9 @@ impl LoweringContext {
         self.class_accessor_names.insert(class_name, accessor_names);
     }
 
-    /// Look up the accessor property names registered for a
-    /// class. The stored list includes inherited accessors (mirroring how
-    /// `class_field_names` stores the own+inherited union), so callers do
-    /// not need to walk the parent chain themselves.
+    /// Look up the accessor property names for a class. Includes inherited
+    /// accessors (mirroring `class_field_names`'s own+inherited union), so
+    /// callers do not need to walk the parent chain themselves.
     pub(crate) fn lookup_class_accessor_names(
         &self,
         class_name: &str,

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

@@ -146,6 +146,18 @@ pub struct LoweringContext {
     /// `lookup_class_accessor_names` and walked across the parent chain when
     /// processing a subclass's ctor body.
     pub(crate) class_accessor_names: HashMap<String, ClassAccessorNames>,
+    /// Issue #10487: own+inherited instance METHOD names per class (mirrors
+    /// `class_accessor_names`). Used by the "infer fields from ctor body
+    /// `this.x = ...`" pass to avoid mis-categorising an assignment that
+    /// overrides an INHERITED method (`this.close = () => …` where `close`
+    /// is declared on a parent class) as a new own data field — that
+    /// allocated an inline slot shadowing the inherited method from the
+    /// moment `super()` returns, so `this.close` read `undefined` until the
+    /// assignment ran (undici MockPool/MockClient's `this.close.bind(this)`
+    /// threw "Bind must be called on a function" for the same reason).
+    /// Own-class methods were already excluded (#665-adjacent zod fix);
+    /// this extends the exclusion across the `extends` chain.
+    pub(crate) class_method_names: HashMap<String, Vec<String>>,
     /// Issue #562: class name → `(module, class)` tuple from
     /// `native_extends`. Populated when lowering each class, consumed by
     /// `destructuring.rs` to register `let x = new SubclassOfStream()`
@@ -1119,3 +1131,19 @@ pub struct LoweringContext {
     /// bodies and module/script top-level both leave this false.
     pub(crate) in_nonarrow_fn: bool,
 }
+
+// Issue #10487: own+inherited instance method names per class (mirrors
+// `class_accessor_names`'s register/lookup pair in context.rs). Split into
+// its own `impl` block here rather than in context.rs, which sits at the
+// file-size cap.
+impl LoweringContext {
+    pub(crate) fn register_class_method_names(&mut self, class_name: String, names: Vec<String>) {
+        self.class_method_names.insert(class_name, names);
+    }
+
+    pub(crate) fn lookup_class_method_names(&self, class_name: &str) -> Option<&[String]> {
+        self.class_method_names
+            .get(class_name)
+            .map(|n| n.as_slice())
+    }
+}

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

@@ -1993,4 +1993,5 @@ mod mixin_parent_chain;
 mod native_module_sync;
 
 mod nullish_over_optional_chain;
+mod subclass_ctor_inherited_method;
 mod ui_widget_add_child;

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

@@ -0,0 +1,101 @@
+//! #10487: a subclass constructor assignment `this.m = …` must NOT become an
+//! own inline field slot when `m` is a method INHERITED from a parent class
+//! — that shadowed the inherited method with an own `undefined` field from
+//! the moment `super()` returned, until the assignment statement ran. Split
+//! from `tests.rs` for the 2000-line file cap.
+
+/// `this.close = …` in a subclass constructor, where `close` is a method
+/// declared only on the parent, must not allocate an own `close` field.
+#[test]
+fn subclass_ctor_assignment_to_inherited_method_name_is_not_a_field() {
+    let source = r#"
+        class Base {
+            close() { return "closed"; }
+        }
+        class Sub extends Base {
+            constructor() {
+                super();
+                this.seen = typeof this.close;
+                this.close = () => "replaced";
+            }
+        }
+    "#;
+    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 sub = hir
+        .classes
+        .iter()
+        .find(|c| c.name == "Sub")
+        .expect("fixture declares class Sub");
+    assert!(
+        !sub.fields.iter().any(|f| f.name == "close"),
+        "this.close = … must not become an own field on Sub when `close` is \
+         inherited from Base; fields: {:?}",
+        sub.fields.iter().map(|f| &f.name).collect::<Vec<_>>()
+    );
+    assert!(
+        sub.fields.iter().any(|f| f.name == "seen"),
+        "this.seen = … has no parent-declared counterpart and must still \
+         become an own field; fields: {:?}",
+        sub.fields.iter().map(|f| &f.name).collect::<Vec<_>>()
+    );
+}
+
+/// Same requirement across TWO levels of inheritance (the method is
+/// declared on a grandparent, not the immediate parent).
+#[test]
+fn grandparent_method_name_is_excluded_across_two_levels() {
+    let source = r#"
+        class Base {
+            close() { return "closed"; }
+        }
+        class Mid extends Base {}
+        class Grand extends Mid {
+            constructor() {
+                super();
+                this.close = () => "replaced";
+            }
+        }
+    "#;
+    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 grand = hir
+        .classes
+        .iter()
+        .find(|c| c.name == "Grand")
+        .expect("fixture declares class Grand");
+    assert!(
+        !grand.fields.iter().any(|f| f.name == "close"),
+        "this.close = … must not become an own field on Grand when `close` \
+         is inherited from Base via Mid; fields: {:?}",
+        grand.fields.iter().map(|f| &f.name).collect::<Vec<_>>()
+    );
+}
+
+/// Control: a class's OWN method being self-bound in its OWN constructor
+/// (the pre-existing #665-adjacent zod fix) must keep working — this
+/// exclusion is orthogonal to the inherited-method one added here.
+#[test]
+fn own_class_method_self_assignment_is_still_not_a_field() {
+    let source = r#"
+        class Own {
+            close() { return "closed"; }
+            constructor() {
+                this.close = this.close.bind(this);
+            }
+        }
+    "#;
+    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 own = hir
+        .classes
+        .iter()
+        .find(|c| c.name == "Own")
+        .expect("fixture declares class Own");
+    assert!(
+        !own.fields.iter().any(|f| f.name == "close"),
+        "an own-method self-assignment must not become a field either; \
+         fields: {:?}",
+        own.fields.iter().map(|f| &f.name).collect::<Vec<_>>()
+    );
+}

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

@@ -974,6 +974,20 @@ pub fn lower_class_decl(
                 _ => {}
             }
         }
+        // Issue #10487: pull in the parent chain's own+inherited method
+        // names too, mirroring the accessor union just above. A subclass
+        // constructor's `this.close = …` overriding a PARENT method (not
+        // redeclared on this class) must be recognized as a method
+        // override, not a new own data field, or the field wins the
+        // dynamic-dispatch lookup and instance reads see `undefined`
+        // until the assignment statement runs.
+        if let Some(ref parent_name) = extends_name {
+            if let Some(parent_methods) = ctx.lookup_class_method_names(parent_name) {
+                for m in parent_methods {
+                    method_names.insert(m.clone());
+                }
+            }
+        }
 
         let declared_field_names: std::collections::HashSet<String> =
             fields.iter().map(|f| f.name.clone()).collect();
@@ -1066,6 +1080,12 @@ pub fn lower_class_decl(
         // from the parent-chain lookup above.
         ctx.register_class_accessor_names(name.clone(), accessor_names);
 
+        // Issue #10487: register this class's complete (own + inherited)
+        // method-name set, mirroring the accessor registration just above,
+        // so a further subclass lowered after this one sees the full
+        // chain in one lookup.
+        ctx.register_class_method_names(name.clone(), method_names.into_iter().collect());
+
         // Issue #302: also register field TYPES so the for-of arm can
         // detect `for (... of this.someMap)` patterns. Only own fields are
         // registered here; inherited field types fall through to whichever

test-files/test_gap_10487_subclass_ctor_hides_inherited_method.ts

@@ -0,0 +1,97 @@
+// #10487: a subclass constructor that assigns `this.m = ...` where `m` is a
+// method inherited from a PARENT class hid the inherited method from the
+// moment `super()` returned, instead of only once the assignment ran.
+//
+// `lower_class_decl` (crates/perry-hir/src/lower_decl/class_decl.rs) scans
+// each constructor's `this.<name> = ...` assignments to decide whether
+// `<name>` needs a synthesized inline field slot, excluding names that are
+// declared fields, inherited fields, or accessors — and the class's OWN
+// methods (the #665-adjacent zod `this.parse.bind(this)` fix: an own-method
+// override must not get a shadowing data slot). It did not exclude METHODS
+// INHERITED FROM THE EXTENDS CHAIN, so `this.close = ...` in a subclass
+// constructor (where `close` is declared only on the parent) allocated an
+// own `close` field. That field exists (as `undefined`) as soon as `super()`
+// returns, so it shadows the inherited method in every by-name lookup until
+// the assignment statement actually runs.
+
+class Base {
+  close() {
+    return "closed";
+  }
+}
+class Sub extends Base {
+  seen: string;
+  constructor() {
+    super();
+    this.seen = typeof this.close; // inherited method, read BEFORE the own assignment below
+    this.close = () => "replaced";
+  }
+}
+class Own {
+  seen: string;
+  close() {
+    return "closed";
+  }
+  constructor() {
+    this.seen = typeof this.close;
+    this.close = () => "replaced";
+  }
+}
+class Sub2 extends Base {
+  seen: string;
+  constructor() {
+    super();
+    this.seen = typeof this.close; // control: no own assignment to `close`
+  }
+}
+class Sub3 extends Base {
+  original: any;
+  constructor() {
+    super();
+    this.original = this.close.bind(this); // undici MockPool / MockClient shape
+    this.close = () => "replaced";
+  }
+}
+// Grandparent-distance: the method is declared two levels up.
+class Mid extends Base {}
+class Grand extends Mid {
+  seen: string;
+  constructor() {
+    super();
+    this.seen = typeof this.close;
+    this.close = () => "replaced";
+  }
+}
+// Alias read: `const self = this; self.close` must see the same result.
+class SubAlias extends Base {
+  seen: string;
+  constructor() {
+    super();
+    const self = this;
+    this.seen = typeof self.close;
+    this.close = () => "replaced";
+  }
+}
+// Assignment made in a regular method (not the constructor): documented as
+// already working, must keep working.
+class SubMethodAssign extends Base {
+  seen: string = "";
+  replace() {
+    this.seen = typeof this.close;
+    this.close = () => "replaced";
+  }
+}
+
+console.log("Sub      ", new Sub().seen);
+console.log("Own      ", new Own().seen);
+console.log("Sub2     ", new Sub2().seen);
+console.log("Grand    ", new Grand().seen);
+console.log("SubAlias ", new SubAlias().seen);
+const sma = new SubMethodAssign();
+sma.replace();
+console.log("SubMethod", sma.seen);
+try {
+  console.log("Sub3", new Sub3().original());
+} catch (e: any) {
+  console.log("Sub3 threw", e.constructor.name, e.message);
+}