Infer recursive types through object literal getters - #64172
Open
Colin McDonnell (colinhacks) wants to merge 9 commits into
Open
Infer recursive types through object literal getters#64172Colin McDonnell (colinhacks) wants to merge 9 commits into
Colin McDonnell (colinhacks) wants to merge 9 commits into
Conversation
Inferring a type argument verifies the candidate against its constraint. That check reports nothing and only decides whether to keep the candidate, but it walks every source property, and for an un-annotated getter that means inferring from its body -- which re-enters the declaration being resolved. Mark the resolution stack when the constraint check opens. A cycle that reaches below the mark was caused by the question rather than by the program, so the attempt is abandoned at the point that forced the member and the member is reported as unanswerable. Nothing computed from the circular value ever completes, so there is no diagnostic to suppress, no placeholder to hand out, no cache write to journal and nothing to retract. Two things the unwind exposes. A comparison that passed over an unanswerable member may keep the candidate but must not reject one, and its success is not written to the global relation cache. And resolveObjectTypeMembers publishes a partial member table as its own recursion guard: abandoning inside that window would leave the type marked resolved while holding only its self-declared members, permanently, for every later reader -- so the flags are cleared on the way out.
Twelve compiler cases and five fourslash cases, carried over from the earlier implementation of this fix so the two are held to the same evidence. One baseline differs from that implementation and the difference is deliberate. A postponed constraint is now reported at the member that violates it -- `TS2741: Property 'out' is missing` -- rather than at the enclosing shape through a three-level assignability chain. All three file orderings agree, so the order-independence the fourslash cases exist to pin is unaffected.
A provisional comparison explores further than an ordinary check does, so it can walk into a circular base constraint that main never reaches -- main collapses the getter first and stops. Reporting that circularity, or caching it as the type parameter's resolved constraint, adds a TS2313 to a program main accepts. The region reports nothing and decides nothing by construction, so the circularity is the question's own: it neither reports nor sticks, and the next ask outside any region is free to reach and report a real one. The new case pins that this variant, which neither main nor this change resolves, reports the same thing on both.
resolveObjectTypeMembers publishes the self-declared member table before it walks the base types, so every inherited member reads as absent until the loop adds it. A miss in that window has two correct answers, and the previous commit merged them into one. Inside a provisional comparison the asker is a speculative check whose whole job is to find out whether a member can answer yet, so completing the lookup there forces the exact type the question is about. Outside one there is no question in flight and a miss is just a lookup that arrived early, so finishing it against the bases still to be inherited is what keeps the window unobservable. Gate the completed lookup on provisionalDepth == 0 and restore the narrower suppression for the inside-a-region case. Measured on a schema-library corpus, counting the getter-collapsed-to-any diagnostic this series exists to remove: one answer everywhere gives 5, restoring the suppression without the gate gives 3, dropping the suppression and gating gives 12, and both together give 0 -- which is what the inference fix gives on its own, so microsoft#62180 now closes at no cost to microsoft#62181. Add recursiveTypeThroughObjectLiteralGetterMutual.ts, which is the only case in the suite that reaches this. It needs two schemas naming each other through getters and two separate key remappings over the same shape, one per variance side. With a single remapping every variant above passes, which is why thirteen existing cases and a 498-project corpus all missed the regression.
saveStacks recorded the length of each checker stack and restoreStacks re-sliced whatever slice was current. That holds for a stack only ever appended to and truncated, which is eleven of the fourteen. Three are replaced wholesale by a caller that puts the original back only on a normal return: checkExpressionCachedEx swaps in a nil flow-loop stack, getVariancesWorker does the same to the variance stack, and checkSourceFile clears the renamed-binding-elements stack per file. An unwind that crosses one of those leaves the replacement in place, so restoring by length re-slices the wrong slice. Every getter body with a return goes through checkExpressionCachedEx, so a getter forced from inside a loop back-edge -- where the outer flow-loop stack is non-empty -- reached nil[:N]. That is a runtime panic, raised inside the deferred recover, so it is not the sentinel and every outer recover re-panics it: the compiler dies on input it should merely report on. Hold those three as slice headers instead, which is what the flow type cache in the same struct already does. The other eleven keep their lengths, and the tail-clearing that goes with them.
Its header still said "This case is NOT fixed" and "The recursive one does not resolve", which was true of an earlier draft and has not been true since the lookup started completing itself against the bases still to be inherited. The `any` printed against `parent` in the .types baseline is the printer eliding a recursive reference, not an unresolved member, so nothing in the baseline contradicted the stale prose and it survived. Add the assertions that would have caught it: the name three levels down is a string, a number annotation on it is an error, and a key the shape does not declare is absent at depth. All three are permitted on `any`, so a collapse turns them into unused-directive errors rather than passing quietly.
Comments and test prose only. The compiler diff here is comments alone, and no behaviour changes. The explanations had grown into restating themselves, with asides that do not earn their line and constructions that name what a sentence is doing instead of saying it. Cut those back to what a reader of this code needs. The conformance case headers had the same problem, several of them explaining the type system rather than the case at hand. Baselines move only where a comment shifted a line number.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #62181 and #62180.
The problem
On
main,TS7022onnodeandTS7023onchildren. With this change, neither, andsample.children[0].children[0].nameisstring. The hover in #62181 goes from(accessor) parent: anyto the recursive type.Issue #62180 is the same window seen from the other side —
TS2741: Property 'out' is missingfor an inherited member — so both are fixed here. Fixing either alone makes the other worse; table below.Cause
resolveObjectTypeMemberspublishes the self-declared member table as a recursion guard before walking base types, so inside that window the inherited members are missing and nothing else is.Two defects start there and need opposite answers.
Inference. Constraint verification of an inferred candidate walks every source property. For an un-annotated getter that infers from the getter body, re-entering the declaration under resolution. The circularity is reported and cached, though it belongs to the question rather than the program.
Publication. A lookup landing in the window reads an inherited member as absent and acts on it.
The change
compareProvisionallymarkstypeResolutionson entry.pushTypeResolutionraises a private sentinel when the cycle it finds starts below that mark.tryGetTypeOfMemberrecovers it at the forcing site, restores the checker stacks, and reports the member unanswerable.Nothing computed from the circular value completes, so there is no diagnostic to suppress, no placeholder to hand out, no cache write to journal, nothing to retract. An earlier draft carried all four and ran 513 lines.
Two properties the unwind forces, each with a test that fails without it:
MembersResolvedon a type holding only its own members. Flags are cleared on the way out at theresolveStructuredTypeMembersfunnel.For the publication defect,
resolveObjectTypeMemberspushes the bases it is about to inherit, and a miss in the window resolves against those instead, returning the member the table is about to hold. Re-entry through the type in flight finds the record marked consulting and falls back to the published table, matching what the inheritance loop already sees.Two mechanisms, not one
A miss in the window has two correct answers depending on the caller, which is the part most worth attacking here.
Inside a provisional comparison, completing the lookup forces the type under question. Outside one, a miss is just early. So the completed lookup is gated on
provisionalDepth == 0, and inside a region the narrower suppression still applies: agetUnmatchedPropertymiss means "not yet", but only for a name some base declares.mayInheritPropertyanswers from declaration tables without forcing a member type. Not entirely inert — resolving a class's base list can force its heritage expression — but that is the resolution the window is already inside, guarded by the resolution stack.120-file schema-library corpus, counting
TS7023:TS7023mainThe publication fix alone is a small net negative on inference. Ungated it reintroduces the collapse on plain mutual recursion —
get posts() { return array(post); }— across three files, since completing the lookup forcespostsmid-inference. Dropping the suppression instead costs twelveTS7023.Tests
Fifteen conformance cases with baselines: the recursive type and the shapes it is written in, mutual recursion, the postponed constraint and the same constraint split across two files, declaration emit, union recursion in both member forms, reverse mapped inference, and a stricter variant of #62180's shape. (#62180's code verbatim is the fourslash fixture.)
Five behave identically to
mainand are in the diff to stay that way — overload resolution across an unresolved accessor in generic and plain signatures, a skipped accessor's constraint still reported, a genuinely absent property still absent, and the same on the speculative path. Each caught a real defect during development.Two are worth naming.
recursiveTypeThroughObjectLiteralGetterFlowLoop.tspins the rollback. A getter body running a loop and assigning to a union-typed variable forces the inner getter from a loop back-edge. SincecheckExpressionCachedExswaps in a nilflowLoopStackand restores it only on normal return, an unwind crossing it has to restore the saved stack rather than re-slice the replacement. Getting that wrong is a crash, not a wrong diagnostic.recursiveTypeThroughObjectLiteralGetterMutual.tspins the gate, and nothing else in the suite reaches it. It needs two schemas naming each other through getters and two key remappings over the same shape, one per variance side, each keyed on a different member of the internals. With a single remapping every candidate in the table passes. It came from reducing a real failure after thirteen hand-written cases and the corpus all missed it.Every case asserting a resolved type also names an absent key behind a
@ts-expect-error, which reports unused onmain.Five fourslash cases: two are #62181, including that diagnostics are unchanged whether or not the hover was requested first; three pin postponed-constraint reporting across open orders, one of them never opening the file carrying the error.
Cost
go test ./internal/...passes 63 packages, and no reference baselines move beyond the cases added here.Nothing changes on code that does not hit the pattern.
--extendedDiagnosticscounters, reproducible run to run:maintsconfig.json— Symbols / Types / Instantiationspackages/zod/tsconfig.jsonOn code that does hit it, on the schema library going from 122 errors to 45:
mainMostly types that collapsed to
anynow resolving.Wall clock is below what this hardware resolves — interleaved runs give the inference-heavy projects effects that change sign between runs, with same-binary control arms moving as far as the effect.
498 projects compiled on both compilers, compared on full diagnostic text, each compiled twice on
mainfirst because three or four are nondeterministic there:mainThe one difference is the schema library, 122 to 45.
Effect on a library
Zod writes a deliberately vague constraint in place of the real one at 296 places to avoid this. One fixed revision, with and without those workarounds:
tscLeft is its five recursion test files, right is all of
src/v4. The1and22are Zod's own unused-variable and missing-@types/nodediagnostics, so that is the floor. Top-left and bottom-right are identical: deleting all 296 workarounds under this compiler reports what shipping Zod reports under stock today.Those five files include recursion through a union, a union of one, and a discriminated union, plus mutual recursion, recursive tuples, cyclic data and
z.lazy.Scope
The unwind is reachable only from a provisional comparison, which is what the corpus result and the unchanged counters reflect.
One shape is knowingly not fixed, with a case pinning it. Where the parameter is a mapped type over the inferred one, so inference runs through a reverse mapped type, neither this change nor
mainresolves the declaration.recursiveTypeThroughObjectLiteralGetterReverseMapped.tsasserts both report the same thing, since a provisional comparison explores further than an ordinary check and must not turn that into an extra diagnostic.Beyond that I know of no remaining failure here, measured against the conformance and fourslash suites, 498 projects, and a schema library with every workaround removed.
Disclosure: this patch was authored with AI assistance (Claude Code). I have read and understood the result and will discuss and revise it in review.