Skip to content

fix: prevent false assignability for types with methods like Map and Set - #2514

Open
puckey wants to merge 2 commits into
vega:nextfrom
puckey:fix/conditional-type-assignability
Open

fix: prevent false assignability for types with methods like Map and Set#2514
puckey wants to merge 2 commits into
vega:nextfrom
puckey:fix/conditional-type-assignability

Conversation

@puckey

@puckey puckey commented Mar 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Interfaces like Map and Set have method signatures that aren't included in ObjectType properties, causing isAssignableTo to consider any object with a size property assignable to Map
  • This produced wrong conditional type branches (e.g. Immutable<Bar> with T extends Map<infer K, infer V> incorrectly matching when Bar has a size property)
  • Track method names on ObjectType, collected in InterfaceAndClassNodeParser
  • Check method names in isAssignableTo to reject false structural matches

Test plan

  • type-awaited-return-type-breakImmutable<T> with Map/Set branches correctly resolves all properties
  • type-self-referencing-method — interfaces with this-returning methods don't cause stack overflow
  • All existing tests pass (250/250)

@puckey
puckey force-pushed the fix/conditional-type-assignability branch 2 times, most recently from fe96889 to cedd993 Compare March 30, 2026 14:28
@arthurfiorette
arthurfiorette enabled auto-merge (squash) March 30, 2026 19:39
auto-merge was automatically disabled March 31, 2026 07:12

Head branch was pushed to by a user without write access

@puckey
puckey force-pushed the fix/conditional-type-assignability branch from cedd993 to 50d6a06 Compare March 31, 2026 07:12
@puckey

puckey commented Mar 31, 2026

Copy link
Copy Markdown
Contributor Author

Fixed the formatting in the latest push.

Interfaces like Map and Set have method signatures that aren't included
in ObjectType properties, causing isAssignableTo to consider any object
with a `size` property assignable to Map. This produced wrong conditional
type branches (e.g. Immutable<T> with `T extends Map<infer K, infer V>`).

- Track method names on ObjectType, collected in InterfaceAndClassNodeParser
- Check method names in isAssignableTo to reject false structural matches
@puckey
puckey force-pushed the fix/conditional-type-assignability branch from 50d6a06 to cbfa79c Compare March 31, 2026 07:17
@puckey

puckey commented Mar 31, 2026

Copy link
Copy Markdown
Contributor Author

Rebased on next and ran tests.

@domoritz
domoritz enabled auto-merge (squash) April 7, 2026 15:10
@arthurfiorette

Copy link
Copy Markdown
Collaborator

@puckey CI is failing

@arthurfiorette arthurfiorette added the feedback Waiting for feedback resolution label Jun 18, 2026
TS6 flags moduleResolution=node (node10) as deprecated (TS5107), which
getPreEmitDiagnostics surfaces and createProgram throws on, failing CI.
auto-merge was automatically disabled June 19, 2026 07:24

Head branch was pushed to by a user without write access

@puckey

puckey commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the heads up! The failure wasn't the assignability logic — it was the two new test fixtures' tsconfig.json using "moduleResolution": "node", which TS6 now flags as deprecated (TS5107). Since createProgram throws on any getPreEmitDiagnostics output, that deprecation warning failed the type check. Switched both fixtures to "moduleResolution": "bundler" in 0dba093 — passes locally against TS 6.0.2 now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feedback Waiting for feedback resolution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants