Skip to content

bun check: walk a type under the restrictive and permissive mappers as tsgo does, and never a non-generic top-level union alias - #46868

Open
robobun wants to merge 2 commits into
mainfrom
robobun/60faf1ff/check-deep-instantiation
Open

robobun wants to merge 2 commits into
mainfrom
robobun/60faf1ff/check-deep-instantiation

Conversation

@robobun

@robobun robobun commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator

Part of #46793

Problem

  • bun check reports TS2589: Type instantiation is excessively deep and possibly infinite where tsc accepts: a TanStack Table ColumnDef whose TData is a mapped type instance, such as a supabase-js row, or a top-level alias around one (type Row = Item & { id: string }). bun lint --type-aware says "The type checker ran out of stack".
  • derive_instantiation_of_type_parameters (src/sema/check/instantiate.rs:543) returns the identity mapper, a no-op, when every type parameter maps to itself. tsgo's function mappers always walk. And a union or intersection named by a non-generic top-level alias keeps COULD_CONTAIN_TYPE_VARIABLES from its members. tsgo's isNonGenericTopLevelType makes it false.
  • Either way UnionToIntersection's extends check compares a mapped type instance under one alias key with the same instance under its own key. The relation goes structural and enters the same conditional type again until the depth limit.

Fix

  • function_mapper: an empty pair list becomes a mapper that maps nothing if the type has HAS_OTHER_INSTANTIATION, so instantiate walks and finds the instance under its own key. The helper is the one bun check: fix more differences from tsc #44685 added for the inference mappers. The outer return mapper in call.rs uses it too.
  • Provenance::is_non_generic_top_level: such a union or intersection loses COULD_CONTAIN_TYPE_VARIABLES when it is created, so instantiate returns it unchanged (checker.go:22117).
  • Verified: test/cli/check/check.test.ts, "walks a type with no type parameters under the restrictive and permissive mappers" (five programs) and "computes the constraint of a distributive conditional type behind an alias". Also all 13101 conformance cases, byte for byte, in both modes.
  • The file in the issue also spreads the query result. bun check: infer from a context sensitive function whose parameter type has an instantiated type argument #46564 fixes that. With both PRs the file is clean. Neither alone closes the issue.
  • Self-reviewed: 5 concerns raised, 5 addressed.

Background

  • A mapper is an interned list of (type parameter, type) pairs. The empty list is tsgo's nil mapper: instantiate returns the type unchanged.
  • tsgo caches a mapped type instance under its type arguments and alias, so Copy<Item> first reached through type S = Copy<Item> is stored under S's key. Rust marks a type that contains such an instance with HAS_OTHER_INSTANTIATION.
  • Considered canonicalizing before is_assignable_restrictive only: the permissive check (c.ts) keeps the stale instance. Considered one identity at creation: tsgo prints Copy<Item> | Copy<Item> for S | Copy<Item>.

Downsides

Notes
  • The file in the issue passes on 1.4.3 and fails from canary 9bcbf1e (2026-10-13). The mechanisms here predate that: Readonly<{ id: number }> as TData of a published @tanstack/react-table 9.2.3 ColumnDef fails on 1.4.3 too.
  • Control experiment: (x: infer R extends unknown) => any makes the pair list non-empty and the 23-line file passes on main.
  • The test's d.ts is an infinite type over the same instance. It still reports TS2589 at the position tsc reports. Across the 13101 conformance cases the walking mapper is returned 3 times, for one type, and the walk returns it unchanged. The changed-type path is covered by the new tests and by a local corpus of 1010 reductions: 112 outputs differ from main, all of them TS2589 removals, except one spread input that still needs bun check: infer from a context sensitive function whose parameter type has an instantiated type argument #46564.
  • The aliased conditional type Bar<U> in the second test: its restrictive instantiation is now another instance without the alias, as in tsgo, so constraint_of_distributive_conditional computes a constraint and a false TS2322 goes away.
  • Under bun lint --type-aware the 5,000,000 instantiation limit is hit inside a rule query that has no node to report at, so the driver prints "ran out of stack". Limit hits are not cached, so every later rule query recomputes the cycle. That reporting is unchanged here.
  • Pre-existing in this container, not changed by this PR: check.test.ts "reports TS2589 for a conditional type that recurs through a type node that isn't deferred" reports 5 of 7 TS2589 on the debug build of main (7 on release and tsc), and the Eden Treaty test needs test/node_modules.
  • The self-review raised the top-level alias residual, the distributive constraint test, the corpus note, one constructor for the mapper, and the scope of the body. All five are in.

no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/cli/check/check.test.ts

… permissive and unique literal mappers

A tsgo function mapper is never nil, so instantiateType walks the type and
getObjectTypeInstantiation looks a mapped type instance up under its own
key. The pair list that stands in for those mappers was empty when every
type parameter mapped to itself, and an empty list is the identity mapper,
which instantiate treats as a no-op. A mapped type instance first stored
under another alias's key then kept that identity on one side of a
conditional type's extends check while the inferred type held the instance
under its own key. The check went structural and entered the same
conditional type again with the same arguments until the depth limit.

The inference context's mappers already returned a mapper that maps
nothing for such a type. The helper now serves the restrictive, permissive
and unique literal mappers and the outer return mapper too.
…lias could contain no type variables

isNonGenericTopLevelType: tsgo returns such a type unchanged from
instantiateType, whatever its members. Here the flag was folded from the
members, so the type was walked under a mapper, a mapped type instance in
it was looked up under its own key, and the result was a second identity of
the alias's type. A conditional type's extends check then compared the two
identities structurally and entered itself again until the depth limit.

Also one constructor for a mapper that tsgo has as a function.
@robobun

robobun commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 4:55 AM PT - Oct 25th, 2026

✅ @robobun, your commit b24b15b07011c8df914f22bfd50596c079a7471d passed in Build #132540! 🎉


🧪   To try this PR locally:

bunx bun-pr 46868

That installs a local version of the PR into your bun-46868 executable, so you can run:

bun-46868 --bun

@coderabbitai

coderabbitai Bot commented Oct 25, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 179664e and b24b15b.

Files selected for processing: 11
  • src/sema/check/alias.rs
  • src/sema/check/call.rs
  • src/sema/check/context.rs
  • src/sema/check/expr.rs
  • src/sema/check/infer.rs
  • src/sema/check/instantiate.rs
  • src/sema/check/mapped.rs
  • src/sema/check/task_tests.rs
  • src/sema/check/unions.rs
  • src/sema/types.rs
  • test/cli/check/check.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

Walkthrough

The checker now records whether union and intersection provenance comes from a non-generic top-level alias. Type construction uses that metadata when setting type-variable flags. Mapper creation handles empty parameter mappings through a shared helper, and CLI tests cover recursive mapped types and an aliased distributive conditional.

Changes

Type Checker Provenance and Mapper Behavior

Layer / File(s) Summary
Record non-generic top-level alias provenance
src/sema/types.rs, src/sema/check/alias.rs, src/sema/check/{context.rs, expr.rs, mapped.rs, unions.rs, task_tests.rs}
Provenance now carries the non-generic top-level alias flag. Alias construction sets the flag, while other provenance construction sites initialize it to false. Union and intersection records with the flag clear COULD_CONTAIN_TYPE_VARIABLES.
Select mappers and validate checker behavior
src/sema/check/{instantiate.rs, infer.rs, call.rs}, test/cli/check/check.test.ts
Mapper creation uses a shared helper for empty parameter mappings. The tests cover recursive mapped types and an aliased distributive conditional.

Possibly related issues

  • Issue 46793 — The recursive ColumnDef regression test covers a related bun check TS2589 pattern. The issue also reports a bun lint --type-aware stack overflow, which this change does not address.

Suggested reviewers: jarred-sumner

Priority: Normal

Merge Risk: Minimal · up to b24b1

No identified issue blocks merging after normal checks.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title accurately identifies the main changes: walking types under restrictive and permissive mappers and handling non-generic top-level aliases. It is long and somewhat awkward, but it is specific…
Description check Passed The description clearly explains the problem, the fix, verification steps, test coverage, performance results, limitations, and related issue context. It provides the required information even though …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title accurately identifies the main changes: walking types under restrictive and permissive mappers and handling non-generic top-level aliases. It is long and somewhat awkward, but it is specific and related to the changeset.

Full details: Description check

Explanation

The description clearly explains the problem, the fix, verification steps, test coverage, performance results, limitations, and related issue context. It provides the required information even though it uses different headings from the repository template.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked the new Provenance::is_non_generic_top_level field end to end: every ProvenanceKey constructor (alias.rs, unions.rs, expr.rs, mapped.rs, context.rs, task_tests.rs, the types.rs unit test) sets it, and is_default, is, to_owned, follow_struct!, clone_in_struct! and visit_type_as_content hashing all include it, so interning identity stays consistent. The has_other_instantiation reorder in src/sema/check/alias.rs is equivalent to the old expression (is_non_generic_top_level_alias is a pure scope lookup, so evaluating it earlier has no side effect).

Extended reasoning...

The change touches the bun check type checker internals in src/sema/ (type interning keys, mapper construction for restrictive/permissive/inference instantiation) plus a new test block in test/cli/check/check.test.ts; it touches no security-sensitive surface. The two inline findings (shared sentinel mapper interacting with active_mappers dedup, and the untested call.rs caller) are what keep this from an approve; the ruled-out note above records the struct-field audit and refactor-equivalence check that were done in addition.

Comment thread src/sema/check/instantiate.rs
Comment thread src/sema/check/call.rs

This branch has not been deployed

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant