Skip to content
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.

Sign up for GitHub

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

Control flow analysis for destructured discriminated unions #46266

Merged
merged 5 commits into from
Nov 2, 2021
Merged

Control flow analysis for destructured discriminated unions #46266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5c6a856
CFA for dependent variables destructured from discriminated union
ahejlsberg Oct 7, 2021
c468a25
Accept new baselines
ahejlsberg Oct 7, 2021
dd07cdc
Add tests
ahejlsberg Oct 7, 2021
76fbe25
Limit calls to isSymbolAssigned
ahejlsberg Oct 8, 2021
cf546e8
Fix wrong operator
ahejlsberg Oct 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
143 changes: 106 additions & 37 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8466,12 +8466,16 @@ namespace ts {

/** Return the inferred type for a binding element */
function getTypeForBindingElement(declaration: BindingElement): Type | undefined {
const pattern = declaration.parent;
let parentType = getTypeForBindingElementParent(pattern.parent);
// If no type or an any type was inferred for parent, infer that for the binding element
if (!parentType || isTypeAny(parentType)) {
const parentType = getTypeForBindingElementParent(declaration.parent.parent);
return parentType && getBindingElementTypeFromParentType(declaration, parentType);
}

function getBindingElementTypeFromParentType(declaration: BindingElement, parentType: Type): Type {
// If an any type was inferred for parent, infer that for the binding element
if (isTypeAny(parentType)) {
return parentType;
}
const pattern = declaration.parent;
// Relax null check on ambient destructuring parameters, since the parameters have no implementation and are just documentation
if (strictNullChecks && declaration.flags & NodeFlags.Ambient && isParameterDeclaration(declaration)) {
parentType = getNonNullableType(parentType);
Expand Down Expand Up @@ -22534,30 +22538,6 @@ namespace ts {
return false;
}

function getPropertyAccess(expr: Expression) {
if (isAccessExpression(expr)) {
return expr;
}
if (isIdentifier(expr)) {
const symbol = getResolvedSymbol(expr);
if (isConstVariable(symbol)) {
const declaration = symbol.valueDeclaration!;
// Given 'const x = obj.kind', allow 'x' as an alias for 'obj.kind'
if (isVariableDeclaration(declaration) && !declaration.type && declaration.initializer && isAccessExpression(declaration.initializer)) {
return declaration.initializer;
}
// Given 'const { kind: x } = obj', allow 'x' as an alias for 'obj.kind'
if (isBindingElement(declaration) && !declaration.initializer) {
const parent = declaration.parent.parent;
if (isVariableDeclaration(parent) && !parent.type && parent.initializer && (isIdentifier(parent.initializer) || isAccessExpression(parent.initializer))) {
return declaration;
}
}
}
}
return undefined;
}

function getAccessedPropertyName(access: AccessExpression | BindingElement): __String | undefined {
let propertyName;
return access.kind === SyntaxKind.PropertyAccessExpression ? access.name.escapedText :
Expand Down Expand Up @@ -23526,19 +23506,19 @@ namespace ts {
return false;
}

function getFlowTypeOfReference(reference: Node, declaredType: Type, initialType = declaredType, flowContainer?: Node) {
function getFlowTypeOfReference(reference: Node, declaredType: Type, initialType = declaredType, flowContainer?: Node, flowNode = reference.flowNode) {
let key: string | undefined;
let isKeySet = false;
let flowDepth = 0;
if (flowAnalysisDisabled) {
return errorType;
}
if (!reference.flowNode) {
if (!flowNode) {
return declaredType;
}
flowInvocationCount++;
const sharedFlowStart = sharedFlowCount;
const evolvedType = getTypeFromFlowType(getTypeAtFlowNode(reference.flowNode));
const evolvedType = getTypeFromFlowType(getTypeAtFlowNode(flowNode));
sharedFlowCount = sharedFlowStart;
// When the reference is 'x' in an 'x.length', 'x.push(value)', 'x.unshift(value)' or x[n] = value' operation,
// we give type 'any[]' to 'x' instead of using the type determined by control flow analysis such that operations
Expand Down Expand Up @@ -23982,13 +23962,58 @@ namespace ts {
return result;
}

function getCandidateDiscriminantPropertyAccess(expr: Expression) {
if (isBindingPattern(reference)) {
// When the reference is a binding pattern, we are narrowing a pesudo-reference in getNarrowedTypeOfSymbol.
// An identifier for a destructuring variable declared in the same binding pattern is a candidate.
if (isIdentifier(expr)) {
const symbol = getResolvedSymbol(expr);
const declaration = symbol.valueDeclaration;
if (declaration && isBindingElement(declaration) && !declaration.initializer && !declaration.dotDotDotToken && reference === declaration.parent) {
return declaration;
}
}
}
else if (isAccessExpression(expr)) {
// An access expression is a candidate if the reference matches the left hand expression.
if (isMatchingReference(reference, expr.expression)) {
return expr;
}
}
else if (isIdentifier(expr)) {
const symbol = getResolvedSymbol(expr);
if (isConstVariable(symbol)) {
const declaration = symbol.valueDeclaration!;
// Given 'const x = obj.kind', allow 'x' as an alias for 'obj.kind'
if (isVariableDeclaration(declaration) && !declaration.type && declaration.initializer && isAccessExpression(declaration.initializer) &&
isMatchingReference(reference, declaration.initializer.expression)) {
return declaration.initializer;
}
// Given 'const { kind: x } = obj', allow 'x' as an alias for 'obj.kind'
if (isBindingElement(declaration) && !declaration.initializer) {
const parent = declaration.parent.parent;
if (isVariableDeclaration(parent) && !parent.type && parent.initializer && (isIdentifier(parent.initializer) || isAccessExpression(parent.initializer)) &&
isMatchingReference(reference, parent.initializer)) {
return declaration;
}
}
}
}
return undefined;
}

function getDiscriminantPropertyAccess(expr: Expression, computedType: Type) {
let access, name;
const type = declaredType.flags & TypeFlags.Union ? declaredType : computedType;
return type.flags & TypeFlags.Union && (access = getPropertyAccess(expr)) && (name = getAccessedPropertyName(access)) &&
isMatchingReference(reference, isAccessExpression(access) ? access.expression : access.parent.parent.initializer!) &&
isDiscriminantProperty(type, name) ?
access : undefined;
if (type.flags & TypeFlags.Union) {
const access = getCandidateDiscriminantPropertyAccess(expr);
if (access) {
const name = getAccessedPropertyName(access);
if (name && isDiscriminantProperty(type, name)) {
return access;
}
}
}
return undefined;
}

function narrowTypeByDiscriminant(type: Type, access: AccessExpression | BindingElement, narrowType: (t: Type) => Type): Type {
Expand Down Expand Up @@ -24801,6 +24826,50 @@ namespace ts {
}
}

function getNarrowedTypeOfSymbol(symbol: Symbol, location: Identifier) {
// If we have a non-rest binding element with no initializer declared as a const variable or a const-like
// parameter (a parameter for which there are no assignments in the function body), and if the parent type
// for the destructuring is a union type, one or more of the binding elements may represent discriminant
// properties, and we want the effects of conditional checks on such discriminants to affect the types of
// other binding elements from the same destructuring. Consider:
//
// type Action =
// | { kind: 'A', payload: number }
// | { kind: 'B', payload: string };
//
// function f1({ kind, payload }: Action) {
// if (kind === 'A') {
// payload.toFixed();
// }
// if (kind === 'B') {
// payload.toUpperCase();
// }
// }
//
// Above, we want the conditional checks on 'kind' to affect the type of 'payload'. To facilitate this, we use
// the binding pattern AST instance for '{ kind, payload }' as a pseudo-reference and narrow this reference
// as if it occurred in the specified location. We then recompute the narrowed binding element type by
// destructuring from the narrowed parent type.
const declaration = symbol.valueDeclaration;
if (declaration && isBindingElement(declaration) && !declaration.initializer && !declaration.dotDotDotToken && declaration.parent.elements.length >= 2) {
const parent = declaration.parent.parent;
if (parent.kind === SyntaxKind.VariableDeclaration && getCombinedNodeFlags(declaration) & NodeFlags.Const || parent.kind === SyntaxKind.Parameter) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small typo here:

Suggested change
if (parent.kind === SyntaxKind.VariableDeclaration && getCombinedNodeFlags(declaration) && NodeFlags.Const || parent.kind === SyntaxKind.Parameter) {
if (parent.kind === SyntaxKind.VariableDeclaration && getCombinedNodeFlags(declaration) & NodeFlags.Const || parent.kind === SyntaxKind.Parameter) {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, good catch!

const links = getNodeLinks(location);
if (!(links.flags & NodeCheckFlags.InCheckIdentifier)) {
links.flags |= NodeCheckFlags.InCheckIdentifier;
const parentType = getTypeForBindingElementParent(parent);
links.flags &= ~NodeCheckFlags.InCheckIdentifier;
if (parentType && parentType.flags & TypeFlags.Union && !(parent.kind === SyntaxKind.Parameter && isSymbolAssigned(symbol))) {
const pattern = declaration.parent;
const narrowedType = getFlowTypeOfReference(pattern, parentType, parentType, /*flowContainer*/ undefined, location.flowNode);
return getBindingElementTypeFromParentType(declaration, narrowedType);
}
}
}
}
return getTypeOfSymbol(symbol);
}

function checkIdentifier(node: Identifier, checkMode: CheckMode | undefined): Type {
const symbol = getResolvedSymbol(node);
if (symbol === unknownSymbol) {
Expand Down Expand Up @@ -24884,7 +24953,7 @@ namespace ts {

checkNestedBlockScopedBinding(node, symbol);

let type = getTypeOfSymbol(localOrExportSymbol);
let type = getNarrowedTypeOfSymbol(localOrExportSymbol, node);
const assignmentKind = getAssignmentTargetKind(node);

if (assignmentKind) {
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5073,6 +5073,7 @@ namespace ts {
ConstructorReferenceInClass = 0x02000000, // Binding to a class constructor inside of the class's body.
ContainsClassWithPrivateIdentifiers = 0x04000000, // Marked on all block-scoped containers containing a class with private identifiers.
ContainsSuperPropertyInStaticInitializer = 0x08000000, // Marked on all block-scoped containers containing a static initializer with 'super.x' or 'super[x]'.
InCheckIdentifier = 0x10000000,
}

/* @internal */
Expand Down
12 changes: 1 addition & 11 deletions tests/baselines/reference/controlFlowAliasing.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,11 @@ tests/cases/conformance/controlFlow/controlFlowAliasing.ts(232,13): error TS2322
Type 'number' is not assignable to type 'string'.
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(233,13): error TS2322: Type 'string | number' is not assignable to type 'string'.
Type 'number' is not assignable to type 'string'.
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(267,13): error TS2322: Type 'string | number' is not assignable to type 'string'.
Type 'number' is not assignable to type 'string'.
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(270,13): error TS2322: Type 'string | number' is not assignable to type 'number'.
Type 'string' is not assignable to type 'number'.
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(280,5): error TS2448: Block-scoped variable 'a' used before its declaration.
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(280,5): error TS2454: Variable 'a' is used before being assigned.


==== tests/cases/conformance/controlFlow/controlFlowAliasing.ts (19 errors) ====
==== tests/cases/conformance/controlFlow/controlFlowAliasing.ts (17 errors) ====
// Narrowing by aliased conditional expressions

function f10(x: string | number) {
Expand Down Expand Up @@ -349,15 +345,9 @@ tests/cases/conformance/controlFlow/controlFlowAliasing.ts(280,5): error TS2454:
function foo({ kind, payload }: Data) {
if (kind === 'str') {
let t: string = payload;
~
!!! error TS2322: Type 'string | number' is not assignable to type 'string'.
!!! error TS2322: Type 'number' is not assignable to type 'string'.
}
else {
let t: number = payload;
~
!!! error TS2322: Type 'string | number' is not assignable to type 'number'.
!!! error TS2322: Type 'string' is not assignable to type 'number'.
}
}

Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/controlFlowAliasing.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -888,12 +888,12 @@ function foo({ kind, payload }: Data) {

let t: string = payload;
>t : string
>payload : string | number
>payload : string
}
else {
let t: number = payload;
>t : number
>payload : string | number
>payload : number
}
}

Expand Down
Loading