From 51d8d45c6f8383ea30359ef7be9959f0392bc41e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 18 Sep 2026 13:26:47 +0000 Subject: [PATCH 1/2] fix(hir): exclude inherited method names from ctor-body field detection A subclass constructor assigning this. where is a method inherited from a parent class allocated an own inline field slot for it, hiding the inherited method from the moment super() returned. Track own+inherited instance method names per class (mirroring the existing accessor-name tracking) and consult the union when deciding whether a constructor-body this. = ... assignment is a new data field or a method override. --- crates/perry-hir/src/lower/context.rs | 8 +- .../perry-hir/src/lower/lowering_context.rs | 28 +++++ crates/perry-hir/src/lower/tests.rs | 1 + .../tests/subclass_ctor_inherited_method.rs | 101 ++++++++++++++++++ crates/perry-hir/src/lower_decl/class_decl.rs | 20 ++++ ...87_subclass_ctor_hides_inherited_method.ts | 97 +++++++++++++++++ 6 files changed, 251 insertions(+), 4 deletions(-) create mode 100644 crates/perry-hir/src/lower/tests/subclass_ctor_inherited_method.rs create mode 100644 test-files/test_gap_10487_subclass_ctor_hides_inherited_method.ts diff --git a/crates/perry-hir/src/lower/context.rs b/crates/perry-hir/src/lower/context.rs index 3d4e00a380..a3de9a1cc3 100644 --- a/crates/perry-hir/src/lower/context.rs +++ b/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, diff --git a/crates/perry-hir/src/lower/lowering_context.rs b/crates/perry-hir/src/lower/lowering_context.rs index d00b76034f..c8ea9c7f52 100644 --- a/crates/perry-hir/src/lower/lowering_context.rs +++ b/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, + /// 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>, /// 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) { + 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()) + } +} diff --git a/crates/perry-hir/src/lower/tests.rs b/crates/perry-hir/src/lower/tests.rs index f127f3eff9..7e521de0f3 100644 --- a/crates/perry-hir/src/lower/tests.rs +++ b/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; diff --git a/crates/perry-hir/src/lower/tests/subclass_ctor_inherited_method.rs b/crates/perry-hir/src/lower/tests/subclass_ctor_inherited_method.rs new file mode 100644 index 0000000000..24f337b608 --- /dev/null +++ b/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::>() + ); + 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::>() + ); +} + +/// 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::>() + ); +} + +/// 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::>() + ); +} diff --git a/crates/perry-hir/src/lower_decl/class_decl.rs b/crates/perry-hir/src/lower_decl/class_decl.rs index 551137f1b2..c539100966 100644 --- a/crates/perry-hir/src/lower_decl/class_decl.rs +++ b/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 = 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 diff --git a/test-files/test_gap_10487_subclass_ctor_hides_inherited_method.ts b/test-files/test_gap_10487_subclass_ctor_hides_inherited_method.ts new file mode 100644 index 0000000000..352f585307 --- /dev/null +++ b/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. = ...` assignments to decide whether +// `` 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); +} From adf1b9e029882a754ddb124953ddf41045655791 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 18 Sep 2026 13:28:31 +0000 Subject: [PATCH 2/2] changelog: fragment for #10626 --- .../10626-subclass-ctor-inherited-method.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 changelog.d/10626-subclass-ctor-inherited-method.md diff --git a/changelog.d/10626-subclass-ctor-inherited-method.md b/changelog.d/10626-subclass-ctor-inherited-method.md new file mode 100644 index 0000000000..c51428a442 --- /dev/null +++ b/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`.