Control flow analysis for destructured rests #62350
Conversation
typescript-bot
added
the
For Uncommitted Bug
|
The TypeScript team hasn't accepted the linked issue #46680. If you can get it accepted, this PR will have a better chance of being reviewed. |
1 similar comment
|
The TypeScript team hasn't accepted the linked issue #46680. If you can get it accepted, this PR will have a better chance of being reviewed. |
There was a problem hiding this comment.
Pull Request Overview
This PR implements control flow analysis for destructured rest variables, addressing two specific TypeScript issues (#46680 and #53947). The changes enable the TypeScript compiler to properly narrow types in destructuring assignments that include rest elements, improving type inference when discriminated unions are involved.
Key Changes
- Enhanced control flow analysis to handle destructured rest variables by tracking narrowed types through rest patterns
- Modified the constraint substitution logic to be more selective, improving type narrowing for binding elements
- Extended support for both object and array destructuring with rest elements
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/cases/conformance/controlFlow/dependentDestructuredRestVariables1.tsx | New test cases demonstrating control flow analysis for destructured rest variables in various scenarios |
| tests/baselines/reference/dependentDestructuredRestVariables1.symbols | Generated symbol baseline for the new test cases |
| tests/baselines/reference/arrayDestructuringInSwitch1.types | Updated type baseline showing improved type inference for array destructuring |
| tests/baselines/reference/arrayDestructuringInSwitch1.symbols | Updated symbol baseline with refined symbol references |
| src/compiler/checker.ts | Core implementation changes to enable control flow analysis for rest variables |
Comments suppressed due to low confidence (1)
src/compiler/checker.ts:1
- [nitpick] The long conditional expression on line 30851 reduces readability. Consider extracting the check mode determination into a variable:
const checkMode = shouldSubstituteConstraints(type, location) ? CheckMode.Normal : CheckMode.SkipConstraintsSubstitution;
import {
| const parent = declaration.parent.parent; | ||
| const rootDeclaration = getRootDeclaration(parent); | ||
| if (rootDeclaration.kind === SyntaxKind.VariableDeclaration && getCombinedNodeFlagsCached(rootDeclaration) & NodeFlags.Constant || rootDeclaration.kind === SyntaxKind.Parameter) { | ||
| const links = getNodeLinks(parent); | ||
| if (!(links.flags & NodeCheckFlags.InCheckIdentifier)) { | ||
| links.flags |= NodeCheckFlags.InCheckIdentifier; | ||
| const parentType = getTypeForBindingElementParent(parent, CheckMode.Normal); | ||
| const parentTypeConstraint = parentType && mapType(parentType, getBaseConstraintOrType); | ||
| const parentType = getTypeForBindingElementParent(parent, shouldSubstituteConstraints(type, location) ? CheckMode.Normal : CheckMode.SkipConstraintsSubstitution); |
There was a problem hiding this comment.
When obtaining the parent type we don't want to get its constraints substituted based on its contextual type or it being in a constraint position or something. This would lead to returning its constraint and then using that for getBindingElementTypeFromParentType even when the current location isn't at the constraint position.
|
Huh, I'll have to investigate what's happening with the self-check |
|
wowwww ❤️ |
| === Performance Stats === | ||
| Assignability cache: 2,500 | ||
| Type Count: 10,000 | ||
| Instantiation count: 100,000 | ||
| Symbol count: 50,000 |
There was a problem hiding this comment.
fwiw, those numbers are the same on main
fixes #46680
fixes #53947