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. Fixes #62180. Refs #64192.
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 comes from the same place. There the symptom is
TS2741: Property 'out' is missingon an inherited member. Both are fixed here; fixing either alone makes the other worse.Cause
Before it walks base types,
resolveObjectTypeMemberspublishes the self-declared member table as a recursion guard. Inside that window the inherited members are missing. Two defects follow and need opposite answers.Inference. Constraint verification of an inferred candidate walks every source property. An un-annotated getter infers its type from its body, so walking it re-enters a declaration that is still being resolved. That circularity is an artifact of the comparison. It still gets reported and cached.
Publication. A lookup that lands in the window reads an inherited member as absent and acts on that.
The change
On entry,
compareProvisionallymarkstypeResolutions. WhenpushTypeResolutionfinds a cycle that starts below that mark it raises a private sentinel. The forcing site,tryGetTypeOfMember, catches it, 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, no cache write to journal and nothing to retract. An earlier draft did all of that and ran 513 lines.
Two things follow from the unwind, each with a test that fails without it.
MembersResolvedon a type that holds only its own members, so the flags are cleared on the way out, inresolveStructuredTypeMembers.For the publication defect,
resolveObjectTypeMemberspushes the bases it is about to inherit, and a miss in the window resolves against those, returning the member the table is about to hold. Re-entry through the type in flight finds its record marked consulting and falls back to the published table, the same view the inheritance loop has.The provisional-depth gate
A miss in the window has two correct answers depending on the caller. 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. Inside a provisional region the narrower suppression still applies. AgetUnmatchedPropertymiss means "not yet", but only for a name some base declares, and that check (mayInheritProperty) reads declaration tables without forcing a member type. It is not entirely inert, since resolving a class's base list can force its heritage expression, but that is the resolution the window is already inside and the resolution stack guards it.On a corpus of probe files reducing the shapes this affects, counting the errors that demand an annotation the user cannot write (
TS7022,TS7023,TS2502):mainThe publication fix alone is a small net negative on inference. Ungated, it reintroduces the collapse on plain mutual recursion (
get posts() { return array(post); }) in three files, because completing the lookup forcespostsmid-inference. Dropping the suppression instead costs fifteen.Tests
Fifteen conformance cases with baselines cover 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. The fourslash fixture is #62180's code verbatim.
Five behave identically to
mainand pin that. 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.The flow-loop case,
recursiveTypeThroughObjectLiteralGetterFlowLoop.ts, pins the rollback. A getter body that runs a loop and assigns 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 that crosses it has to restore the saved stack rather than re-slice the replacement. Get that wrong and the checker crashes.The mutual case,
recursiveTypeThroughObjectLiteralGetterMutual.ts, pins 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. Thirteen hand-written cases and the corpus all missed this; the case is reduced from a real failure.Every case asserting a resolved type also names an absent key behind a
@ts-expect-error, which reports as unused onmain.Five fourslash cases. Two are #62181, including a check that diagnostics are unchanged whether or not the hover was requested first. Three pin postponed-constraint reporting across open orders, one without opening the file that carries the error.
Cost
All 63 packages under
go test ./internal/...pass, and no reference baseline moves beyond the cases added here.Code that does not hit the pattern is unchanged. The
--extendedDiagnosticscounters are reproducible run to run.maintsconfig.json— Symbols / Types / Instantiationspackages/coreOn the probe corpus, which does hit it:
mainMostly types that collapsed to
anynow resolving.Wall-clock differences are below what this hardware resolves. Interleaved runs on the inference-heavy projects give effects that change sign between runs, and same-binary control arms move as far as the effect.
498 projects compiled on both compilers and compared on full diagnostic text, each compiled twice on
mainfirst because three or four are nondeterministic there.mainThe one difference is the probe corpus, where the declarations that demanded an annotation on
mainnow infer.Effect on a library
Zod substitutes a deliberately vague constraint for the real one at 296 sites, because the real constraint does not survive inference through a getter and the schema collapses to
any.This commit deletes all 296 and restores the intended constraint at every site. Under
mainthe library then reports 189 errors: 167 are declarations demanding an annotation, getters demanding a return type, and the constraints that fail once those collapse. With this change all 167 are gone. The 22 left are unused symbols and a missing@types/nodein that checkout. Its recursive schemas cover recursion through a union, a union of one and a discriminated union, mutual recursion, recursive tuples, cyclic data andz.lazy.Scope
The unwind is reachable only from a provisional comparison. The 498-project result and the unchanged counters above bear that out.
One shape is not fixed; a case pins 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. The reverse-mapped case,recursiveTypeThroughObjectLiteralGetterReverseMapped.ts, asserts 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, measured against the conformance and fourslash suites, 498 projects, and Zod 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.