-
Notifications
You must be signed in to change notification settings - Fork 12.9k
Fixed control flow Analysis of aliased discriminants with parenthesized initializers #61788
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Fixed control flow Analysis of aliased discriminants with parenthesized initializers #61788
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes an issue where discriminant properties aliased through parenthesized or destructured initializers weren't correctly recognized in control-flow analysis.
- Added comprehensive tests covering both direct and parenthesized/destructured alias patterns.
- Refactored the alias resolution logic in the type checker to use a helper that skips parentheses.
- Introduced
getCandidateVariableDeclarationInitializer
to centralize initializer extraction.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
tests/cases/conformance/controlFlow/controlFlowAliasing2.ts | New tests for aliased discriminants with various initializer forms |
tests/baselines/reference/controlFlowAliasing2.symbols | Baseline update for the new conformance tests |
src/compiler/checker.ts | Extracted initializer logic into a helper and updated alias-matching branches |
Comments suppressed due to low confidence (1)
src/compiler/checker.ts:29266
- [nitpick] Consider renaming this helper to something shorter like
getInitializerSkippingParens
orextractInitializer
for improved readability.
function getCandidateVariableDeclarationInitializer(node: Node) {
return declaration; | ||
} | ||
} | ||
} | ||
} | ||
return undefined; | ||
|
||
function getCandidateVariableDeclarationInitializer(node: Node) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a JSDoc comment above this helper to explain that it skips parentheses and returns an initializer only for untyped variable declarations.
Copilot uses AI. Check for mistakes.
isVariableDeclaration(parent) && !parent.type && parent.initializer && (isIdentifier(parent.initializer) || isAccessExpression(parent.initializer)) && | ||
isMatchingReference(reference, parent.initializer) | ||
) { | ||
initializer = getCandidateVariableDeclarationInitializer(declaration.parent.parent); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid potential misuse, you might explicitly check that declaration.parent.parent
is indeed a VariableDeclaration
before passing it to the helper.
Copilot uses AI. Check for mistakes.
isMatchingReference(reference, declaration.initializer.expression) | ||
) { | ||
return declaration.initializer; | ||
if (initializer && isAccessExpression(initializer) && isMatchingReference(reference, initializer.expression)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Instead of reusing initializer
for both branches, consider using separate variables (e.g., directInit
and destructuredInit
) to clarify the two distinct alias checks.
Copilot uses AI. Check for mistakes.
fixes #61784