Repository navigation
Conversation
…pe has an instantiated type argument The fixing mapper fixed a type parameter that the contextual parameter type did not mention, so the return type of the arrow function never contributed a candidate and the inference was unknown. Two causes. Instantiating a type with the symbol of an object literal and no outer type parameters created a new type whose property mappers carried an identity pair for the type parameter. It is now returned as is, as getObjectTypeInstantiation does. And params_mentioned_in counted every type parameter as mentioned when it reached a synthesized type with call signatures. It now traverses the signatures.
|
Updated 1:55 AM PT - Oct 23rd, 2026
❌ @robobun, your commit 7eea0a9 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 46564That installs a local version of the PR into your bun-46564 --bun |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe checker now marks only inference parameters found during type traversal and returns certain synthesized types unchanged during instantiation. Regression tests cover context-sensitive callback return-type inference with instantiated types and nested instantiations near the depth limit. ChangesCallback inference
Suggested reviewers: Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to
|
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline notes, I also checked the other callers of params_mentioned_in (non_fixing_mapper_comparing, read_type_arguments_for, the conditional-type extends path in mapped.rs, and instantiate_with_outer_return_mapper in call.rs): each only loses over-marking, and any_type_in(_, true, ..) is the same documented exact mode instantiate.rs already relies on. I also confirmed every site that sets Shape::symbol_declared_at (spread, widened literal, rest type) sets shape.mapper from the declaring scope's full identity mapper, so IDENTITY does mean no outer type parameters for the new early return. The repeat helper in check.test.ts is Buffer.alloc-based, so the snapshot normalization follows the test convention.
Extended reasoning...
The change is a two-site fix in the Rust type checker behind bun check (inference fixing mapper and object-type instantiation) plus one multi-fixture test; it touches no security-sensitive surface. The inference semantics are subtle with several callers affected, and the only findings are test-coverage nits, so a human maintainer look is still worthwhile rather than an outright approve.
There was a problem hiding this comment.
I re-reviewed the latest push and found no bugs; both points from my earlier review are now covered (member.ts for the object-literal-member selector, over.ts pinning Deep<48> past the limit). Because this changes core inference fixing and object-type instantiation in the checker, a human look is still worthwhile.
What was reviewed:
- The new
Synthearly return insrc/sema/check/instantiate.rsagainst every site that setssymbol_declared_at: each also setsShape::mapperfrom the declaration scope, soIDENTITYdoes mean no outer type parameters, matching the documented invariant and tsgo'slen(typeParameters) == 0return. - Dropping the
may_be_anyfallback inparams_mentioned_in: the exactall = truetraversal is already relied on byderive_instantiation_of_type_parameters, and the only remaining inexactmentions()caller is inprint.rs, not an inference site. - Worth a human weighing: the PR's own numbers show the adversarial intersection-of-800-generic-functions case going from 5.1 s to 11.7 s (debug), and the
Deep<47>/Deep<48>boundary is pinned by snapshot but its agreement with tsc rests on the author's local run.
Extended reasoning...
The diff touches two hunks in the Rust type checker (src/sema/check/infer.rs and src/sema/check/instantiate.rs) plus one new test in test/cli/check/check.test.ts with six fixtures; it touches no security-sensitive surface. The Rust changes are small but sit in inference fixing and object type instantiation, where every caller of params_mentioned_in and every instantiation of an object-literal-symbol'd type changes behavior, and correctness depends on tsc equivalence that cannot be run in this session. The author addressed both prior inline nits in later commits, and no CODEOWNERS entry covers these paths, but the PR's own perf note (an adversarial case roughly doubling in time) is a tradeoff a maintainer should weigh.
Fixes #46470
Problem
bun checkreportsTS2322: Type 'unknown' is not assignable to type 'boolean'foruseSelector((state) => state.isLoading)when the contextual parameter type isState<typeof spreadRow>andspreadRowis a spread of a supabase select result. tsc 7.0.2 infersboolean. The same happens for TanStack Router'suseRouterState({ select }).TSelectedwith no candidates before the arrow's return type was inferred from. Two causes:instantiate_uncached(src/sema/check/instantiate.rs:1092) instantiated every member of the spread type, socomposeput the identity pairTSelected -> TSelectedinto a property mapper, andparams_mentioned_in(src/sema/check/infer.rs:3362) then countedTSelectedas mentioned. It also counted every parameter as mentioned when it reached a synthesized type with call signatures.Fix
instantiate, as tsgo'sgetObjectTypeInstantiationdoes whenlen(typeParameters) == 0. The Anon, Cond and Substitution branches already return a declared type unchanged under an identity mapper.params_mentioned_intraverses signatures (any_type_inwithall = true, the exact mode) instead of marking every parameter. tsgo'sInferenceTypeMapper.Mapfixes only the parameters thatinstantiateTypereaches, so the set must be exact.test/cli/check/check.test.ts, "infers the return type of a context sensitive function whose parameter type has an instantiated type argument", four programs (spread, call signature as type argument, instantiated callable interface, depth limit), output verified against tsc 7.0.2. Also all 2406 programs ofdifferential-cases.jsonagainst tsc 7.0.2, and the TypeScript conformance sample plus one of eight shards (1646 tests), byte for byte.Background
(type parameter, type)pairs. An identity pairT -> Tis kept on purpose: it records that a declared type depends onT.compose(first, second)adds every pair ofsecondthatfirstdoes not map, so an identity pair insecondspreads into the result.Shape::mapperof a synthesized object type with an object literal symbol maps the literal's outer type parameters to themselves.IDENTITYmeans there are none, and tsgo does not instantiate such a type at all.fixing_mapperis the mapper tsgo callscontext.mapper. Fixing a type parameter freezes its inference, so fixing one the type does not mention loses every candidate found later. Here the arrow's return type was that candidate.any_type_inor incompose: both change what a declared type's mapper means for every reader. Each half on its own fixes only one of the two fixtures.Downsides
params_mentioned_innow walks the signatures of a synthesized type it reaches instead of returning early. On a file with 2000useSelectorcalls whose parameter type holds one such type: 1240 ms vs 1265 ms checked (debug build, 3 interleaved runs, noise about 25 ms). On an adversarial file with 1000 calls whose parameter type is an intersection of 800 generic function types: 5.1 s vs 11.7 s (debug build, tsc 7.0.2 takes 0.5 s).any_type_invisits from 1291 to 1289. A literal no longer charges one instantiation per member towards the depth limit, so a false TS2589 near the limit disappears (thedeep.tsfixture).Notes
Self-reviewed: 2 concerns raised, 2 addressed (a fixture for the instantiated callable interface path, and a fixture plus this note for the TS2589 change).
The 8-line reduction without a dependency.
infer Resttwice makesRestcount as possibly referenced (more than one declaration), so the type literal in the false branch keeps an identity pair in its mapper. That gives the spread typeCOULD_CONTAIN_TYPE_VARIABLES, which is what postgrest-js's select parser also produces.The trace of the original:
types_assigned_from_contextual_signature->sig_params->instantiate_parameterinstantiatesState<typeof spreadRow>with[TSelected -> TSelected]. The Synth branch composes that into theidproperty's mapper (aPropSource::Mappedproperty whose mapper maps to a mapped type, so it is not closed).instantiate_signature_in_inference_context->fixing_mapper->params_mentioned_infindsTSelectedamong the mapper values.get_inferred_typeruns with zero candidates and returnsunknown.infer_from_signatureinfersnumberfrom the return type afterwards, too late.Counters (temporary, not in the diff) on the four test programs:
any_type_invisits 31 -> 31, synthesized types 25 -> 17.Other suites run:
test/cli/check/check.test.tsin full (the only non-timeout failure, "reports TS2589 for a conditional type that recurs through a type node that isn't deferred", fails the same way on main in this container: it runs out of stack). The timeouts are the 5 s default underdescribe.concurrentin a debug build.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