Repository navigation
Various order dependence fixes - #65292
Jake Bailey (jakebailey) wants to merge 13 commits into
Conversation
142dd55 to
b8870a5
Compare
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
2 open issues
What changed in this PR
This PR addresses order-dependent behavior in the type checker by making comparisons and caches stable across traversal order, and updates/adds compiler baselines to lock in determinism improvements.
Changes:
- Makes type/symbol ordering more content-based (less dependent on creation order / IDs), including new ordering rules for object literals, tuple labels, indexed access accessFlags, and type-parameter origins.
- Adds/interns additional caches (object literals, spreads, binding-pattern types, intersection content) to avoid generating distinct-but-identical types.
- Adds new regression tests and updates many baselines to reflect deterministic ordering/diagnostics changes.
| File | Description |
|---|---|
| tsc/testdata/tests/cases/compiler/excessivelyLargeTupleSpread.ts | Adds @checkOrder coverage to validate deterministic behavior across check orders. |
| tsc/testdata/tests/cases/compiler/assertionOperandInLoopAssertionCallNoHang.ts | New regression test for assertion operand + loop behavior without hangs across check orders. |
| tsc/testdata/baselines/reference/tsc/generateTrace/generateTrace-with-multiple-files-and-complex-types.js | Baseline update reflecting new stable ordering in generated trace entities. |
| tsc/testdata/baselines/reference/fourslash/findAllReferences/findAllRefsObjectBindingElementPropertyName05.baseline.jsonc | Baseline update for reference-marking / binding-element naming output. |
| tsc/testdata/baselines/reference/conformance/spreadUnion.types | Baseline update reflecting stable union constituent ordering. |
| tsc/testdata/baselines/reference/conformance/destructuringWithLiteralInitializers(checkorder=typesfirst).types | Baseline update reflecting new inferred types for destructuring defaults. |
| tsc/testdata/baselines/reference/conformance/destructuringWithLiteralInitializers(checkorder=typesfirst).errors.txt | Baseline update reflecting changed error set after checker fixes. |
| tsc/testdata/baselines/reference/compiler/spreadUnionPropOverride.types | Baseline update reflecting stable ordering of spread-derived union object types. |
| tsc/testdata/baselines/reference/compiler/returnTypeInferenceNotTooBroad(checkorder=typesfirst).types | Baseline update reflecting improved literal/narrowed return inference stability. |
| tsc/testdata/baselines/reference/compiler/relationTruncatedCacheCheckOrder(checkorder=typesfirst).errors.txt | Baseline update reflecting stability fixes to truncation-caching behavior. |
| tsc/testdata/baselines/reference/compiler/relationTruncatedCacheCheckOrder(checkorder=default).errors.txt | New baseline for default check order to validate truncation-caching independence. |
| tsc/testdata/baselines/reference/compiler/relationMaybeCheckOrder.types | Baseline update reflecting stable relation results independent of check order. |
| tsc/testdata/baselines/reference/compiler/relationMaybeCheckOrder.errors.txt | Baseline update reflecting changed error set after relation caching fixes. |
| tsc/testdata/baselines/reference/compiler/relationDeeplyNestedCheckOrder.errors.txt | New baseline validating deeply-nested relation results are order-independent. |
| tsc/testdata/baselines/reference/compiler/recursiveConditionalTypes.types | Baseline update reflecting stable resolution of recursive conditional type outputs. |
| tsc/testdata/baselines/reference/compiler/overloadresolutionWithConstraintCheckingDeferred(checkorder=typesfirst).types | Baseline update reflecting improved contextual typing / overload resolution stability. |
| tsc/testdata/baselines/reference/compiler/overloadresolutionWithConstraintCheckingDeferred(checkorder=typesfirst).errors.txt | Baseline update reflecting changed error ordering/set post-fixes. |
| tsc/testdata/baselines/reference/compiler/objectRestUnionInstantiationOrder(checkorder=typesfirst).types | Baseline update validating deterministic object-rest union instantiation ordering. |
| tsc/testdata/baselines/reference/compiler/objectLiteralRegularAndWidenedCopyOrder(checkorder=default).types | Baseline update reflecting stable ordering between regular and widened copies. |
| tsc/testdata/baselines/reference/compiler/objectInstantiationFromUnionSpread.types | Baseline update reflecting stable spread-instantiation ordering in unions. |
| tsc/testdata/baselines/reference/compiler/noImplicitAnyUnionNormalizedObjectLiteral1.types | Baseline update reflecting stable object-literal normalization ordering. |
| tsc/testdata/baselines/reference/compiler/noFlakyDiagnosticsSpellingSuggestionOrder.types | Baseline update reflecting stable type display impacting diagnostics content. |
| tsc/testdata/baselines/reference/compiler/noFlakyDiagnosticsSpellingSuggestionOrder.errors.txt | Baseline update reflecting stable diagnostics output ordering/content. |
| tsc/testdata/baselines/reference/compiler/noFlakyDiagnosticsNamespaceMembersResolutionOrderReexportedTypes.types | Baseline update reflecting namespace resolution stability changes. |
| tsc/testdata/baselines/reference/compiler/noFlakyDiagnosticsNamespaceMembersResolutionOrder.types | Baseline update reflecting namespace resolution stability changes. |
| tsc/testdata/baselines/reference/compiler/noFlakyDeclarationEmitSharedCheckerOrder.types | Baseline update reflecting stable declaration emit behavior. |
| tsc/testdata/baselines/reference/compiler/narrowedNeverAfterLoopWithAndCondition.errors.txt | Baseline update reflecting changed narrowing/flow behavior. |
| tsc/testdata/baselines/reference/compiler/namespaceMembersResolutionOrderExportStar.types | Baseline update reflecting export-star namespace resolution stability. |
| tsc/testdata/baselines/reference/compiler/jsdocTypeTagOnContextuallyTypedFunction.types | Baseline update reflecting contextual typing stability changes for JSDoc. |
| tsc/testdata/baselines/reference/compiler/jsdocTypeTagOnContextuallyTypedFunction.symbols | Baseline update reflecting symbol resolution stability changes for JSDoc. |
| tsc/testdata/baselines/reference/compiler/jsdocTypeTagOnContextuallyTypedFunction.errors.txt | Baseline update reflecting changed error set due to contextual typing changes. |
| tsc/testdata/baselines/reference/compiler/jsFunctionWithPrototypeNoErrorTruncationNoCrash.types | Baseline update reflecting stable prototype typing output. |
| tsc/testdata/baselines/reference/compiler/inferenceDeeplyNestedCheckOrder.types | Baseline update for deterministic inference under deep nesting. |
| tsc/testdata/baselines/reference/compiler/inferenceDeeplyNestedCheckOrder.errors.txt | Baseline update reflecting changed inferred types/errors under deep nesting. |
| tsc/testdata/baselines/reference/compiler/indexedAccessAccessFlagsUnionOrder(checkorder=default).types | Baseline update reflecting indexed-access ordering stabilized by accessFlags. |
| tsc/testdata/baselines/reference/compiler/indexedAccessAccessFlagsUnionOrder(checkorder=default).errors.txt | Baseline update reflecting removed errors after indexed-access fixes. |
| tsc/testdata/baselines/reference/compiler/genericCallInferenceUsingThisTypeNoInvalidCacheReuseAfterMappedTypeApplication1.types | Baseline update validating cache reuse fixes in generic-call inference. |
| tsc/testdata/baselines/reference/compiler/fixingTypeParametersRepeatedly2(checkorder=typesfirst).types | Baseline update reflecting stable type-parameter fixing behavior. |
| tsc/testdata/baselines/reference/compiler/fixingTypeParametersRepeatedly2(checkorder=typesfirst).errors.txt | Baseline update reflecting changed error set post-fixes. |
| tsc/testdata/baselines/reference/compiler/fallbackToBindingPatternForTypeInference(checkorder=typesfirst).errors.txt | Baseline update reflecting stability changes in binding-pattern inference. |
| tsc/testdata/baselines/reference/compiler/exhaustiveSwitchCheckCircularity(checkorder=typesfirst).types | Baseline update reflecting changed narrowing/circularity behavior. |
| tsc/testdata/baselines/reference/compiler/excessivelyLargeTupleSpread(checkorder=typesfirst).symbols | New/updated baseline validating stable symbol output for new @checkOrder. |
| tsc/testdata/baselines/reference/compiler/excessivelyLargeTupleSpread(checkorder=typesfirst).js | New baseline emission output for typesfirst check order. |
| tsc/testdata/baselines/reference/compiler/excessivelyLargeTupleSpread(checkorder=typesfirst).errors.txt | New baseline errors for typesfirst check order. |
| tsc/testdata/baselines/reference/compiler/excessivelyLargeTupleSpread(checkorder=default).symbols | New baseline symbol output for default check order. |
| tsc/testdata/baselines/reference/compiler/excessivelyLargeTupleSpread(checkorder=default).js | New baseline emission output for default check order. |
| tsc/testdata/baselines/reference/compiler/excessivelyLargeTupleSpread(checkorder=default).errors.txt | New baseline errors for default check order. |
| tsc/testdata/baselines/reference/compiler/deleteExpressionMustBeOptional(checkorder=typesfirst,strict=true).errors.txt | Baseline update reflecting error-set stabilization changes. |
| tsc/testdata/baselines/reference/compiler/declarationEmitUnknownImport2(target=es2015).errors.txt | Baseline update reflecting deferred diagnostics / circularity stability changes. |
| tsc/testdata/baselines/reference/compiler/declarationEmitUnknownImport(target=es2015).errors.txt | Baseline update reflecting deferred diagnostics / circularity stability changes. |
| tsc/testdata/baselines/reference/compiler/contextualReturnArrayPatternCallback(checkorder=typesfirst).types | Baseline update reflecting improved contextual typing for callbacks. |
| tsc/testdata/baselines/reference/compiler/contextualReturnArrayPatternCallback(checkorder=typesfirst).errors.txt | Baseline update reflecting removed error after contextual typing fix. |
| tsc/testdata/baselines/reference/compiler/contextualOuterTypeParameters(checkorder=typesfirst).types | Baseline update reflecting stable representation of conditional types with outer TPs. |
| tsc/testdata/baselines/reference/compiler/clonedTypeParameterUnionOrder(checkorder=typesfirst).types | Baseline update reflecting stable ordering of cloned type-parameter-derived unions. |
| tsc/testdata/baselines/reference/compiler/clonedTypeParameterUnionOrder(checkorder=default).types | Baseline update reflecting stable ordering of cloned type-parameter-derived unions. |
| tsc/testdata/baselines/reference/compiler/circularInlineMappedGenericTupleTypeNoCrash(checkorder=typesfirst).errors.txt | Baseline update reflecting correct error positioning and stability. |
| tsc/testdata/baselines/reference/compiler/checkOrderSpreadCycle(checkorder=typesfirst).types | Baseline update reflecting spread-cycle resolution stability. |
| tsc/testdata/baselines/reference/compiler/checkOrderSpreadCycle(checkorder=reverse).types | Baseline update reflecting spread-cycle resolution stability across check orders. |
| tsc/testdata/baselines/reference/compiler/checkOrderSpreadCycle(checkorder=isolated).types | Baseline update reflecting spread-cycle resolution stability across check orders. |
| tsc/testdata/baselines/reference/compiler/checkOrderSpreadCycle(checkorder=default).types | Baseline update reflecting spread-cycle resolution stability across check orders. |
| tsc/testdata/baselines/reference/compiler/checkOrderParameterInTypeQuery(checkorder=typesfirst).types | Baseline update reflecting stable contextual typing/type-query interactions. |
| tsc/testdata/baselines/reference/compiler/checkOrderParameterInTypeQuery(checkorder=isolated).types | Baseline update reflecting stable contextual typing/type-query interactions. |
| tsc/testdata/baselines/reference/compiler/checkOrderEnumComputedMember(checkorder=isolated).types | Baseline update reflecting stable enum computed member checking behavior. |
| tsc/testdata/baselines/reference/compiler/checkIndexConstraintOfJavascriptClassExpression(checkorder=typesfirst).errors.txt | Baseline update reflecting added implied-any parameter error stability. |
| tsc/testdata/baselines/reference/compiler/blockScopedBindingUsedBeforeDef.errors.txt | Baseline update reflecting changed index-type diagnostics output. |
| tsc/testdata/baselines/reference/compiler/assertionOperandInLoopAssertionCallNoHang(checkorder=typesfirst).js | New baseline emission output for new regression test (typesfirst). |
| tsc/testdata/baselines/reference/compiler/assertionOperandInLoopAssertionCallNoHang(checkorder=reverse).js | New baseline emission output for new regression test (reverse). |
| tsc/testdata/baselines/reference/compiler/assertionOperandInLoopAssertionCallNoHang(checkorder=isolated).js | New baseline emission output for new regression test (isolated). |
| tsc/testdata/baselines/reference/compiler/assertionOperandInLoopAssertionCallNoHang(checkorder=default).js | New baseline emission output for new regression test (default). |
| tsc/testdata/baselines/reference/compiler/argumentsReferenceInObjectLiteral_Js(alwaysstrict=true).types | Baseline update reflecting stable typing of arguments property in object literal. |
| tsc/internal/checker/utilities.go | Implements stable ordering comparisons for symbols/types (tuple labels, object literals, TPs, accessFlags). |
| tsc/internal/checker/types_test.go | Updates checker construction for new intersection-content cache usage in tests/benchmarks. |
| tsc/internal/checker/types.go | Extends core type structures (generation, type-parameter origin) to support stable ordering and recursion heuristics. |
| tsc/internal/checker/relater.go | Makes relation caching/truncation behavior order-independent; adds type generation computation for deep nesting. |
| tsc/internal/checker/pseudotypenodebuilder.go | Adjusts pseudo type serialization to avoid mismatches around widened property assignment types. |
| tsc/internal/checker/nodecopy.go | Improves symbol equivalence checks by comparing target symbols for instantiated parameters. |
| tsc/internal/checker/flow.go | Fixes shared flow-cache reuse across loop frames to avoid order-dependent/incomplete type reuse. |
| tsc/internal/checker/checker.go | Adds caches/interning for object literals/spreads/binding patterns; stabilizes call/signature resolution and assertion operand checks. |
Review effort: Lite
b8870a5 to
a224515
Compare
a224515 to
9a960ce
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Critical issues allow contextual types to override explicit JSDoc annotations, discard diagnostics queued during deferred processing, and cause infinite recursion when comparing tuple element parameters.
3 open issues
2 resolved since last review
Review effort: Balanced
| c.addDeferredDiagnostic(func() { | ||
| c.reportFailedToResolveSymbol(errorLocation, name, meaning, nameNotFoundMessage) | ||
| }) |
| if signature == nil { | ||
| return | ||
| } | ||
| signature := c.getSignatureFromDeclarationUncontextual(node) |
| if c := CompareTypes(p1.origin, p2.origin); c != 0 { | ||
| return c | ||
| } |
A spread that produces a tuple too large to represent reports the error at the node being checked. When the type is first requested through the API, before checking has begun, there is no such node and reporting the error crashed instead. (seen in excessivelyLargeTupleSpread)
checkAssertion runs in several check modes, such as when a call is first checked without its context sensitive arguments, and each run overwrote the operand type kept for the deferred comparability check. That check used whichever was stored last, so its result depended on the order in which the assertion was checked. The deferred check now uses the cached normal type of the operand. The assertion itself only needs the operand type for `as const`, and the operand of a non-const assertion is contextually typed by the asserted type alone, so in any check mode the assertion computes the cached type the deferred check will read, with inference for an enclosing call hidden from the operand. This is the result the deferred check would produce on its own, and checks each operand once rather than again with fresh flow analysis in the deferred check.
The type of a destructured parameter's binding elements depended on
whether the parameter's type happened to be cached already. For
example, in non-strict mode
(({ [a()?.d]: c = "" }) => {})();
reported TS2537 when 'c' was queried before the IIFE call resolved
the parameter (its declared type is the undefined widening type),
but not otherwise. Conversely, in
f7({ a: {} });
function f7({ a: { x = 0, y = 0 } } = { a: {} }) {}
calling f7 first made 'x' and 'y' errors of type any.
Make the choice depend only on the declaration. Parameters of
context-sensitive functions take their type from contextual
assignment, so ask for the parameter's type. Everything else uses
the type derived from the declaration, as it did when nothing was
cached. Asking for the parameter's type unconditionally instead
introduces circularities in patterns whose initializers reference
sibling elements.
A context-sensitive function is typed in parts: the enclosing call
assigns its parameters their contextual types, completes its signature
with the type parameters of a generic contextual signature, and checks
it. Queries that reached the function before or during those steps
saw it incomplete and cached what they derived from it.
The first query of such a parameter returned the type it computed
itself from the final signature, while every later query returned the
type the enclosing call assigned. In
declare function fun<T>(g: (x: T) => T, h: (y: T) => T, x: T): T;
fun((Math.random() < 0.5 ? (x => x) : (x => undefined)), x => x, 10);
the last 'x' was number when the call was checked first, but any when
'x' was queried first. The assigned type is now returned whenever one
exists, as the original comment intended.
Type aliases in the body read the signature's type parameters as outer
type parameters, so querying such an alias first resolved its
conditional types eagerly. In
declare function f(fun: <T>(t: T) => void): void
f(function self(t) {
const s = self;
type P = Parameters<typeof s>;
});
P was [t: unknown] when the call was checked first, but [t: T] when P
was asked for first. Assigning the contextual parameter types, which
completes the signature, is now split from the rest of the contextual
check, and every way to obtain the signature requests it: the type of
the function's symbol, before anything resolves its members, and the
signature of its declaration. Contextual typing can reach the
enclosing call while the signature is being created, so the signature
created there is kept rather than replacing the one that was
contextually typed.
When resolving a function expression's members reached the function
again through its enclosing call, the contextual check saw an empty
signature list and gave up, yet left the node marked as checked, so
implicit any errors could be lost depending on which query got there
first. The deferred function check now requests the contextual check,
so it is performed at least once.
Generic calls nested in the arguments of other generic calls, or in context-sensitive functions passed to them, depend on the enclosing call for their contextual type and parameter types, and the enclosing call infers from them in turn. When the inner call was queried first, it was already marked as resolving by the time its contextual type resolved the enclosing call, so the enclosing inference saw a placeholder and both calls cached results that a check in source order never produces. Computing the contextual type of a call nested in an argument of an enclosing call before resolving it resolves the enclosing call first, as source order does. Other contexts are not resolved ahead of time, since computing them may evaluate expressions that checking does not, such as the target of a destructuring assignment. Resolving a call's signature can also be requested while checking one of its arguments. Errors that resolution reports at the current node, such as an instantiation depth error, were located at whichever node asked first, so checking an argument ahead of its call moved the error onto the argument. Resolution is now located at the call.
Asking for the symbol of a property access that hits an index signature stored the index symbol as the access's resolved symbol. Checking a delete expression reads that link to decide whether the operand must be optional, so a prior symbol query turned `delete a.x` on an index signature into an error that a plain check does not report. The type of the index symbol at such an access, which hovers show, now looks the index symbol up again rather than reading it from links. A single signature type produced by higher order inference is instantiated with the full mapper combined in, since its signature can reference type parameters other than the outer type parameters of its declaration. The instantiation cache was keyed only by the mapped outer type parameters, so two instantiations differing in another type parameter (here the this type) shared whichever result was computed first, and checking `outer.log` gave it the type of `inner.log`. Such instantiations are no longer cached.
Explaining why a name failed to resolve looks up other symbols, for example to find a spelling suggestion, and that can resolve aliases. When the failed name was the target of an alias, the suggestion lookup re-entered the alias being resolved, or skipped it, depending on which aliases were already in progress. In declarationEmitUnknownImport that decided whether two "Circular definition of import alias" errors were reported: they appeared when the import was checked first, and not when the export specifier was resolved first. Defer the explanation until no resolution is in progress, so it only observes finished results. The circularity errors that were artifacts of the suggestion lookup are no longer reported.
When one reference hit the flow analysis depth limit, flow analysis was disabled for every later reference in the same function or module body, so the type of a reference depended on whether another reference in the body had been analyzed before it. Each reference now gives up only when its own analysis hits the limit. Shared flow node results are cached for the duration of one flow analysis, including incomplete results computed under a loop back edge. Once that loop's back edge has been exited and the loop label has a complete type, they are stale. Reusing them made a reference's type depend on which reference was analyzed first: in exhaustiveSwitchCheckCircularity, querying the reference inside the loop before the switch expression produced `never` instead of "bbb". An incomplete shared result is now reused only while the loop that was innermost when it was computed is still being analyzed.
A computed property name stored a placeholder type while its
expression was checked, and returned the placeholder when re-entered.
That kept the cycle off the resolution stack, so the computations in
between did not fail, and the placeholder `{}` escaped into messages
when checking entered through the property name, while `any` did when
it entered through the variable it references. In
blockScopedBindingUsedBeforeDef, the error read "Type '{}' cannot be
used as an index type" or "Type 'any' ..." depending on what was
checked first.
Resolve computed property names on the resolution stack, failing with
the error type on re-entry like other circular resolutions.
CompareTypes orders the constituents of unions, and so printed types,
union deduplication, and everything derived from them. Many types it
cannot tell apart fell through to the type id, which follows creation
order, so these results depended on what had been checked before.
Most such ties were copies: checking the same expression again
(inference passes, overload resolution, contextual re-checks) created
a new type with identical content every time.
Types that are the same by content are now interned, so the same
inputs yield the same type whenever and however often they are
computed: object literal types, spread results and the property
symbols they synthesize, widened, regular, and padded copies of object
literals, object rest types, types implied by binding patterns,
intersections with the same constituents, flags, and alias, and
deferred indexed accesses, which are keyed by the flags they record
rather than by transient ones. Interned object types are keyed by
their members in table order, so spreads such as { ...a, ...b } and
{ ...b, ...a } stay distinct and each keeps its own property order.
Types that remain distinct are ordered by how they were derived:
- type parameters sharing a symbol (tuple elements, clones made for
signature and mapped type instantiation, distributive and
restrictive copies) by the kind of copy, then by their origin and
the instantiation they were made for;
- fresh and regular literal types, array literal types after the
references they were cloned from, and object literal copies by the
flags they were created with;
- object literal, rest, and binding pattern types by their members,
compared by declaration and name rather than by symbol id;
- tuple targets by their label declarations.
isDeeplyNestedType counts only occurrences of a recursion identity
newer than the previous one, using type ids for "newer", so whether a
comparison gave up as deeply nested depended on what had been checked
before. It now uses a type's generation, one more than that of the
types it was built from, which captures the same ordering wherever it
follows from how the types are built, and nothing else.
The pseudo-type printer reused the declared type of an inferred
property assignment when the expression's widened type matched, but
that type is widened without the context of its object literal and
can differ from the real one. It is now reused only when equivalent.
A relation that succeeds while an outer comparison is still in progress has only been shown to hold under the outer comparison's assumption. Caching it globally made later lookups depend on which comparison happened to run first. The same was true for results cut short by depth limits measured against outer resolutions. Each result now tracks the lowest resolution it depends on and whether it was truncated. Results are cached globally only when the comparison that introduced their assumptions completes, and truncated results only at their own head. Failures remain cached immediately, since assumptions only ever make a relation more likely to succeed. A comparison that only relates by assuming comparisons within it is reported as related once it completes, whether it is computed or read from the cache. Comparisons of types referencing constrained type parameters are considered related while the same comparison with unconstrained type parameters is in progress, since constraints only make more types related. A completed comparison found to succeed now counts the same way. That only went unnoticed because completed comparisons used to stay in progress until the outermost one completed; without it, relating structurally identical generic interfaces creates ten times the types.
Instantiating a signature copies a parameter unless the parameter's type has already been resolved, so whether the parameter in a signature's printed scope is the declared one depends on what was checked before. Both name the same declaration, so treating only the declared one as the same reference made printing a reused annotation depend on check order.
Computing the values of an enum's members type-checked each computed initializer to report one that is not a number. The initializer may refer to the enum itself, whose type then reads member values that are still being computed, so the enum's type depended on whether its values were first computed by checking it or by asking for its type. The values do not depend on the initializer's type, so the check now happens when the member is checked, as for any other error in an initializer.
9a960ce to
d8bd2ec
Compare


This is the second part of #65064, ordering fixes that can apply to main
The first chunk of the PR are fixing places where code peeked at caches that "happened to be filled" instead of calling the actual function to pull on them.
Then, all of the ID-using code gets removed. At the end everything now has an ID independent ordering based on content, flags, the works. This caught a bunch of places where we were not caching things and ending up with types that were by all rights identical but had different IDs and then were unstable.