Fix stack overflow caused by circular destructuring

[ahejlsberg]

Fixes crash caused by this code in --strict mode:

const { c, f }: string | number = { c: 0, f };

Supersedes #2528, which didn't seem to go anywhere.

[Copilot]

Pull request overview

This PR fixes a stack overflow that occurred when type-checking circular destructuring patterns in strict mode. The issue was caused by improper management of a re-entrancy guard flag that should prevent infinite recursion during type narrowing operations.

Changes:

• Added a test case for circular destructuring that previously caused a crash
• Fixed the re-entrancy guard in getNarrowedTypeOfSymbol by ensuring the flag is cleared only after all recursive operations complete
• Refactored two if-statements to switch-case statements for better code structure

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
testdata/tests/cases/compiler/circularDestructuring.ts	New test case that reproduces the circular destructuring bug
testdata/baselines/reference/compiler/circularDestructuring.types	Expected type output showing both variables typed as any
testdata/baselines/reference/compiler/circularDestructuring.symbols	Expected symbol output for the destructuring pattern
testdata/baselines/reference/compiler/circularDestructuring.errors.txt	Expected errors including circular reference and type mismatch errors
internal/checker/checker.go	Core fix that moves the re-entrancy guard flag cleanup to after all recursive operations, preventing stack overflow

[DanielRosenwasser]

What was the stack trace? Even the other PR doesn't link back to anything.

[DanielRosenwasser]

Is there a reason you didn't unset the flag before the if like in the original code?

[ahejlsberg]

I think it was simply that it wasn't clear we might recurse through getBindingElementTypeFromParentType.

[DanielRosenwasser]

Okay, this is the original issue: microsoft/TypeScript#62993

[DanielRosenwasser]

And a recurrence we're running into is

    at checkObjectLiteral (c:\Users\drosen\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-6.0.20260118\node_modules\typescript\lib\typescript.js:78613:28)
    at checkExpressionWorker (c:\Users\drosen\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-6.0.20260118\node_modules\typescript\lib\typescript.js:85518:16)
    at checkExpression (c:\Users\drosen\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-6.0.20260118\node_modules\typescript\lib\typescript.js:85429:32)
    at getTypeOfExpression (c:\Users\drosen\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-6.0.20260118\node_modules\typescript\lib\typescript.js:85370:18)
    at getTypeOfInitializer (c:\Users\drosen\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-6.0.20260118\node_modules\typescript\lib\typescript.js:74361:34)
    at getBindingElementTypeFromParentType (c:\Users\drosen\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-6.0.20260118\node_modules\typescript\lib\typescript.js:60446:80)
    at getNarrowedTypeOfSymbol (c:\Users\drosen\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-6.0.20260118\node_modules\typescript\lib\typescript.js:76499:22)
    at checkIdentifier (c:\Users\drosen\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-6.0.20260118\node_modules\typescript\lib\typescript.js:76624:16)
    at checkExpressionWorker (c:\Users\drosen\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-6.0.20260118\node_modules\typescript\lib\typescript.js:85486:16)
    at checkExpression (c:\Users\drosen\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-6.0.20260118\node_modules\typescript\lib\typescript.js:85429:32)
    at checkExpressionForMutableLocation (c:\Users\drosen\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-6.0.20260118\node_modules\typescript\lib\typescript.js:85188:18)
    at checkObjectLiteral (c:\Users\drosen\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-6.0.20260118\node_modules\typescript\lib\typescript.js:78643:71)

So the answer to my other question is basically "it's different from the original code because this is a fix that also needs to happen in the other codebase." (I misread and thought this was a porting bug)

[ahejlsberg]

it's different from the original code because this is a fix that also needs to happen in the other codebase.

Right, we should consider back-porting this to 6.0.

[Andarist]

FWIW, I already had this PR open in Strada. I used a different approach and once I saw this PR here, I ran an experiment to see if that would allow me to remove the NodeCheckFlags.InCheckIdentifier completely and it turns out that yes, it would: microsoft/TypeScript@ba0d77c