ReviewOS

home-lang/home

fix(ts): instantiate merged ambient generic constructors without namespace substitution

#941
Closed glennmichael123 opened this 8 days ago ·
8 days ago

Problem

After the paired-accessor transfer in #938, an explicit generic new expression for a nested ambient class merged with a namespace can substitute the accessor's setter type with the namespace object instead of the explicit type argument. An explicitly typed instance (declare const box: Nested.Box<boolean>) follows the correct read/write types, so this is a separate constructor-instantiation path, not a reason to weaken the accessor route.

Reproduction

first.d.ts:

declare module 'pkg' {
  export namespace Nested {
    export class Box<T> {
      constructor();
      get value(): string;
      set value(input: T | number);
    }
  }
}

second.d.ts:

declare module 'pkg' {
  export namespace Nested.Box { export const extra: 3; }
}
declare module 'other' {
  export namespace Nested {
    export class Box<T> {
      constructor();
      get value(): T;
      set value(input: T | boolean);
    }
  }
}

main.ts:

import { Nested } from 'pkg';
import { Nested as Other } from 'other';
const box = new Nested.Box<boolean>();
const read: string = box.value;
box.value = 42;
box.value = true;
box.value = "bad";
const wrong: number = box.value;
const extra: 3 = Nested.Box.extra;
const other = new Other.Box<number>();
other.value = true;
other.value = "bad";
const wrongOther: string = other.value;
void read; void wrong; void extra; void wrongOther;

Pinned TypeScript 5.9.3 reports four TS2322 diagnostics: invalid box.value = "bad", the wrong box.value read, invalid other.value = "bad", and the wrong other.value read. Under Home's 3,840 MB guarded Program test, the same fixture reported six TS2322 diagnostics. It rejected the valid box.value = 42 and box.value = true, saying the target type was { readonly Box: { new <T>(): Box; ... readonly extra: 3 } }—the nested namespace value, not boolean | number. The failing consumer offsets were 142, 158, 176, 201, 322, and 349. The separate declare const box: Nested.Box<boolean> fixture matches the TypeScript diagnostic codes and locations.

Acceptance

  • Preserve the exact class-generic binder and the explicit boolean argument through nested ambient class/namespace constructor invocation; no synthetic any or namespace-object substitution.
  • Match TypeScript 5.9.3 codes and locations for valid/invalid reads and writes, including another module with the same class name.
  • Add same-file, split-file, and reverse-add-order Program and CLI regressions under the memory guard.
  • Investigate the other.value read after writes separately if it remains wider than TypeScript after the constructor path is corrected; do not count matching error totals alone as parity.

Related: #938, #66. Observed while validating de0b9bd4e.

8 days ago

Progress on #941: 13cb2aa is pushed to origin/main under glennmichael123. The constructor path was misclassifying a union as a polymorphic this parameter because union flags include constituent type-parameter flags while its payload indexes the union pool. The fix distinguishes direct type-parameter payloads in the checker and interner; a Program regression using new Nested.Box<boolean>() now reports the four expected TS2322 diagnostics instead of six. Verified: Program 383/383, checker 4681/4681, targeted interner 2/2, Zig formatting clean, all under the 3,840 MB guard. Keeping this issue open: the full two-module new-expression fixture, post-write read type, CLI route, and reverse-add-order acceptance still need explicit verification. Pinned Bun WebKit is absent locally, so this is not a Bun-native suite claim.

8 days ago

Resolved on origin/main by 13cb2aa and dade2cf. The first fix stops union flags from being misread as a polymorphic this parameter during generic constructor substitution. The second bounds accessor assignment flow by the getter read type: compatible writes can still narrow, while setter-only accepted values restore the declared getter type. The full two-module fixture matches pinned TypeScript 5.9.3 with exactly four TS2322 diagnostics and no spurious post-write read error. Program regressions cover split, reverse-add-order, and same-file declarations; the tsc_main CLI-adapter regression covers the same three modes and also preserves equal read/write accessor narrowing. Verification under HOME_RUN_MAX_MB=3840: Program 383/383, checker 4681/4681, tsc_main 63/63; Zig formatting and diff checks clean. This does not claim a Bun-native corpus pass because the pinned WebKit artifact is missing locally. Related broader accessor work remains tracked in #938 and #66.

Sign in to comment on this issue.