Skip to content

Commit 6955142

Browse files
weswighamweswigham
authored
Fix #17023 (#17180)
* Fix #17023 * Be more general when handling matching references through binding elements * Better cache key, PR feedback * Deeper tests, better cache key handling
1 parent 7d7a06d commit 6955142

File tree

6 files changed

+447
-6
lines changed

6 files changed

+447
-6
lines changed

‎src/compiler/binder.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2061,8 +2061,10 @@ namespace ts {
20612061
case SyntaxKind.Parameter:
20622062
return bindParameter(<ParameterDeclaration>node);
20632063
case SyntaxKind.VariableDeclaration:
2064+
return bindVariableDeclarationOrBindingElement(<VariableDeclaration>node);
20642065
case SyntaxKind.BindingElement:
2065-
return bindVariableDeclarationOrBindingElement(<VariableDeclaration | BindingElement>node);
2066+
node.flowNode = currentFlow;
2067+
return bindVariableDeclarationOrBindingElement(<BindingElement>node);
20662068
case SyntaxKind.PropertyDeclaration:
20672069
case SyntaxKind.PropertySignature:
20682070
return bindPropertyWorker(node as PropertyDeclaration | PropertySignature);

‎src/compiler/checker.ts

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4089,8 +4089,8 @@ namespace ts {
40894089

40904090
/** Return the inferred type for a binding element */
40914091
function getTypeForBindingElement(declaration: BindingElement): Type {
4092-
const pattern = <BindingPattern>declaration.parent;
4093-
const parentType = getTypeForBindingElementParent(<VariableLikeDeclaration>pattern.parent);
4092+
const pattern = declaration.parent;
4093+
const parentType = getTypeForBindingElementParent(pattern.parent);
40944094
// If parent has the unknown (error) type, then so does this binding element
40954095
if (parentType === unknownType) {
40964096
return unknownType;
@@ -4135,7 +4135,8 @@ namespace ts {
41354135
// or otherwise the type of the string index signature.
41364136
const text = getTextOfPropertyName(name);
41374137

4138-
type = getTypeOfPropertyOfType(parentType, text) ||
4138+
const declaredType = getTypeOfPropertyOfType(parentType, text);
4139+
type = declaredType && getFlowTypeOfReference(declaration, declaredType) ||
41394140
isNumericLiteralName(text) && getIndexTypeOfType(parentType, IndexKind.Number) ||
41404141
getIndexTypeOfType(parentType, IndexKind.String);
41414142
if (!type) {
@@ -10671,7 +10672,7 @@ namespace ts {
1067110672
// The result is undefined if the reference isn't a dotted name. We prefix nodes
1067210673
// occurring in an apparent type position with '@' because the control flow type
1067310674
// of such nodes may be based on the apparent type instead of the declared type.
10674-
function getFlowCacheKey(node: Node): string {
10675+
function getFlowCacheKey(node: Node): string | undefined {
1067510676
if (node.kind === SyntaxKind.Identifier) {
1067610677
const symbol = getResolvedSymbol(<Identifier>node);
1067710678
return symbol !== unknownSymbol ? (isApparentTypePosition(node) ? "@" : "") + getSymbolId(symbol) : undefined;
@@ -10681,7 +10682,14 @@ namespace ts {
1068110682
}
1068210683
if (node.kind === SyntaxKind.PropertyAccessExpression) {
1068310684
const key = getFlowCacheKey((<PropertyAccessExpression>node).expression);
10684-
return key && key + "." + (<PropertyAccessExpression>node).name.text;
10685+
return key && key + "." + unescapeLeadingUnderscores((<PropertyAccessExpression>node).name.text);
10686+
}
10687+
if (node.kind === SyntaxKind.BindingElement) {
10688+
const container = (node as BindingElement).parent.parent;
10689+
const key = container.kind === SyntaxKind.BindingElement ? getFlowCacheKey(container) : (container.initializer && getFlowCacheKey(container.initializer));
10690+
const text = getBindingElementNameText(node as BindingElement);
10691+
const result = key && text && (key + "." + text);
10692+
return result;
1068510693
}
1068610694
return undefined;
1068710695
}
@@ -10697,6 +10705,28 @@ namespace ts {
1069710705
return undefined;
1069810706
}
1069910707

10708+
function getBindingElementNameText(element: BindingElement): string | undefined {
10709+
if (element.parent.kind === SyntaxKind.ObjectBindingPattern) {
10710+
const name = element.propertyName || element.name;
10711+
switch (name.kind) {
10712+
case SyntaxKind.Identifier:
10713+
return unescapeLeadingUnderscores(name.text);
10714+
case SyntaxKind.ComputedPropertyName:
10715+
if (isComputedNonLiteralName(name as PropertyName)) return undefined;
10716+
return (name.expression as LiteralExpression).text;
10717+
case SyntaxKind.StringLiteral:
10718+
case SyntaxKind.NumericLiteral:
10719+
return name.text;
10720+
default:
10721+
// Per types, array and object binding patterns remain, however they should never be present if propertyName is not defined
10722+
Debug.fail("Unexpected name kind for binding element name");
10723+
}
10724+
}
10725+
else {
10726+
return "" + element.parent.elements.indexOf(element);
10727+
}
10728+
}
10729+
1070010730
function isMatchingReference(source: Node, target: Node): boolean {
1070110731
switch (source.kind) {
1070210732
case SyntaxKind.Identifier:
@@ -10711,6 +10741,17 @@ namespace ts {
1071110741
return target.kind === SyntaxKind.PropertyAccessExpression &&
1071210742
(<PropertyAccessExpression>source).name.text === (<PropertyAccessExpression>target).name.text &&
1071310743
isMatchingReference((<PropertyAccessExpression>source).expression, (<PropertyAccessExpression>target).expression);
10744+
case SyntaxKind.BindingElement:
10745+
if (target.kind !== SyntaxKind.PropertyAccessExpression) return false;
10746+
const t = target as PropertyAccessExpression;
10747+
if (t.name.text !== getBindingElementNameText(source as BindingElement)) return false;
10748+
if (source.parent.parent.kind === SyntaxKind.BindingElement && isMatchingReference(source.parent.parent, t.expression)) {
10749+
return true;
10750+
}
10751+
if (source.parent.parent.kind === SyntaxKind.VariableDeclaration) {
10752+
const maybeId = (source.parent.parent as VariableDeclaration).initializer;
10753+
return maybeId && isMatchingReference(maybeId, t.expression);
10754+
}
1071410755
}
1071510756
return false;
1071610757
}
@@ -11501,6 +11542,10 @@ namespace ts {
1150111542
const cache = flowLoopCaches[id] || (flowLoopCaches[id] = createMap<Type>());
1150211543
if (!key) {
1150311544
key = getFlowCacheKey(reference);
11545+
// No cache key is generated when binding patterns are in unnarrowable situations
11546+
if (!key) {
11547+
return declaredType;
11548+
}
1150411549
}
1150511550
const cached = cache.get(key);
1150611551
if (cached) {

‎tests/baselines/reference/destructuringTypeGuardFlow.js

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
//// [destructuringTypeGuardFlow.ts]
2+
type foo = {
3+
bar: number | null;
4+
baz: string;
5+
nested: {
6+
a: number;
7+
b: string | null;
8+
}
9+
};
10+
11+
const aFoo: foo = { bar: 3, baz: "b", nested: { a: 1, b: "y" } };
12+
13+
if (aFoo.bar && aFoo.nested.b) {
14+
const { bar, baz, nested: {a, b: text} } = aFoo;
15+
const right: number = aFoo.bar;
16+
const wrong: number = bar;
17+
const another: string = baz;
18+
const aAgain: number = a;
19+
const bAgain: string = text;
20+
}
21+
22+
type bar = {
23+
elem1: number | null;
24+
elem2: foo | null;
25+
};
26+
27+
const bBar = { elem1: 7, elem2: aFoo };
28+
29+
if (bBar.elem2 && bBar.elem2.bar && bBar.elem2.nested.b) {
30+
const { bar, baz, nested: {a, b: text} } = bBar.elem2;
31+
const right: number = bBar.elem2.bar;
32+
const wrong: number = bar;
33+
const another: string = baz;
34+
const aAgain: number = a;
35+
const bAgain: string = text;
36+
}
37+
38+
39+
//// [destructuringTypeGuardFlow.js]
40+
var aFoo = { bar: 3, baz: "b", nested: { a: 1, b: "y" } };
41+
if (aFoo.bar && aFoo.nested.b) {
42+
var bar = aFoo.bar, baz = aFoo.baz, _a = aFoo.nested, a = _a.a, text = _a.b;
43+
var right = aFoo.bar;
44+
var wrong = bar;
45+
var another = baz;
46+
var aAgain = a;
47+
var bAgain = text;
48+
}
49+
var bBar = { elem1: 7, elem2: aFoo };
50+
if (bBar.elem2 && bBar.elem2.bar && bBar.elem2.nested.b) {
51+
var _b = bBar.elem2, bar = _b.bar, baz = _b.baz, _c = _b.nested, a = _c.a, text = _c.b;
52+
var right = bBar.elem2.bar;
53+
var wrong = bar;
54+
var another = baz;
55+
var aAgain = a;
56+
var bAgain = text;
57+
}

‎tests/baselines/reference/destructuringTypeGuardFlow.symbols

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
=== tests/cases/compiler/destructuringTypeGuardFlow.ts ===
2+
type foo = {
3+
>foo : Symbol(foo, Decl(destructuringTypeGuardFlow.ts, 0, 0))
4+
5+
bar: number | null;
6+
>bar : Symbol(bar, Decl(destructuringTypeGuardFlow.ts, 0, 12))
7+
8+
baz: string;
9+
>baz : Symbol(baz, Decl(destructuringTypeGuardFlow.ts, 1, 21))
10+
11+
nested: {
12+
>nested : Symbol(nested, Decl(destructuringTypeGuardFlow.ts, 2, 14))
13+
14+
a: number;
15+
>a : Symbol(a, Decl(destructuringTypeGuardFlow.ts, 3, 11))
16+
17+
b: string | null;
18+
>b : Symbol(b, Decl(destructuringTypeGuardFlow.ts, 4, 14))
19+
}
20+
};
21+
22+
const aFoo: foo = { bar: 3, baz: "b", nested: { a: 1, b: "y" } };
23+
>aFoo : Symbol(aFoo, Decl(destructuringTypeGuardFlow.ts, 9, 5))
24+
>foo : Symbol(foo, Decl(destructuringTypeGuardFlow.ts, 0, 0))
25+
>bar : Symbol(bar, Decl(destructuringTypeGuardFlow.ts, 9, 19))
26+
>baz : Symbol(baz, Decl(destructuringTypeGuardFlow.ts, 9, 27))
27+
>nested : Symbol(nested, Decl(destructuringTypeGuardFlow.ts, 9, 37))
28+
>a : Symbol(a, Decl(destructuringTypeGuardFlow.ts, 9, 47))
29+
>b : Symbol(b, Decl(destructuringTypeGuardFlow.ts, 9, 53))
30+
31+
if (aFoo.bar && aFoo.nested.b) {
32+
>aFoo.bar : Symbol(bar, Decl(destructuringTypeGuardFlow.ts, 0, 12))
33+
>aFoo : Symbol(aFoo, Decl(destructuringTypeGuardFlow.ts, 9, 5))
34+
>bar : Symbol(bar, Decl(destructuringTypeGuardFlow.ts, 0, 12))
35+
>aFoo.nested.b : Symbol(b, Decl(destructuringTypeGuardFlow.ts, 4, 14))
36+
>aFoo.nested : Symbol(nested, Decl(destructuringTypeGuardFlow.ts, 2, 14))
37+
>aFoo : Symbol(aFoo, Decl(destructuringTypeGuardFlow.ts, 9, 5))
38+
>nested : Symbol(nested, Decl(destructuringTypeGuardFlow.ts, 2, 14))
39+
>b : Symbol(b, Decl(destructuringTypeGuardFlow.ts, 4, 14))
40+
41+
const { bar, baz, nested: {a, b: text} } = aFoo;
42+
>bar : Symbol(bar, Decl(destructuringTypeGuardFlow.ts, 12, 9))
43+
>baz : Symbol(baz, Decl(destructuringTypeGuardFlow.ts, 12, 14))
44+
>nested : Symbol(nested, Decl(destructuringTypeGuardFlow.ts, 2, 14))
45+
>a : Symbol(a, Decl(destructuringTypeGuardFlow.ts, 12, 29))
46+
>b : Symbol(b, Decl(destructuringTypeGuardFlow.ts, 4, 14))
47+
>text : Symbol(text, Decl(destructuringTypeGuardFlow.ts, 12, 31))
48+
>aFoo : Symbol(aFoo, Decl(destructuringTypeGuardFlow.ts, 9, 5))
49+
50+
const right: number = aFoo.bar;
51+
>right : Symbol(right, Decl(destructuringTypeGuardFlow.ts, 13, 7))
52+
>aFoo.bar : Symbol(bar, Decl(destructuringTypeGuardFlow.ts, 0, 12))
53+
>aFoo : Symbol(aFoo, Decl(destructuringTypeGuardFlow.ts, 9, 5))
54+
>bar : Symbol(bar, Decl(destructuringTypeGuardFlow.ts, 0, 12))
55+
56+
const wrong: number = bar;
57+
>wrong : Symbol(wrong, Decl(destructuringTypeGuardFlow.ts, 14, 7))
58+
>bar : Symbol(bar, Decl(destructuringTypeGuardFlow.ts, 12, 9))
59+
60+
const another: string = baz;
61+
>another : Symbol(another, Decl(destructuringTypeGuardFlow.ts, 15, 7))
62+
>baz : Symbol(baz, Decl(destructuringTypeGuardFlow.ts, 12, 14))
63+
64+
const aAgain: number = a;
65+
>aAgain : Symbol(aAgain, Decl(destructuringTypeGuardFlow.ts, 16, 7))
66+
>a : Symbol(a, Decl(destructuringTypeGuardFlow.ts, 12, 29))
67+
68+
const bAgain: string = text;
69+
>bAgain : Symbol(bAgain, Decl(destructuringTypeGuardFlow.ts, 17, 7))
70+
>text : Symbol(text, Decl(destructuringTypeGuardFlow.ts, 12, 31))
71+
}
72+
73+
type bar = {
74+
>bar : Symbol(bar, Decl(destructuringTypeGuardFlow.ts, 18, 1))
75+
76+
elem1: number | null;
77+
>elem1 : Symbol(elem1, Decl(destructuringTypeGuardFlow.ts, 20, 12))
78+
79+
elem2: foo | null;
80+
>elem2 : Symbol(elem2, Decl(destructuringTypeGuardFlow.ts, 21, 23))
81+
>foo : Symbol(foo, Decl(destructuringTypeGuardFlow.ts, 0, 0))
82+
83+
};
84+
85+
const bBar = { elem1: 7, elem2: aFoo };
86+
>bBar : Symbol(bBar, Decl(destructuringTypeGuardFlow.ts, 25, 5))
87+
>elem1 : Symbol(elem1, Decl(destructuringTypeGuardFlow.ts, 25, 14))
88+
>elem2 : Symbol(elem2, Decl(destructuringTypeGuardFlow.ts, 25, 24))
89+
>aFoo : Symbol(aFoo, Decl(destructuringTypeGuardFlow.ts, 9, 5))
90+
91+
if (bBar.elem2 && bBar.elem2.bar && bBar.elem2.nested.b) {
92+
>bBar.elem2 : Symbol(elem2, Decl(destructuringTypeGuardFlow.ts, 25, 24))
93+
>bBar : Symbol(bBar, Decl(destructuringTypeGuardFlow.ts, 25, 5))
94+
>elem2 : Symbol(elem2, Decl(destructuringTypeGuardFlow.ts, 25, 24))
95+
>bBar.elem2.bar : Symbol(bar, Decl(destructuringTypeGuardFlow.ts, 0, 12))
96+
>bBar.elem2 : Symbol(elem2, Decl(destructuringTypeGuardFlow.ts, 25, 24))
97+
>bBar : Symbol(bBar, Decl(destructuringTypeGuardFlow.ts, 25, 5))
98+
>elem2 : Symbol(elem2, Decl(destructuringTypeGuardFlow.ts, 25, 24))
99+
>bar : Symbol(bar, Decl(destructuringTypeGuardFlow.ts, 0, 12))
100+
>bBar.elem2.nested.b : Symbol(b, Decl(destructuringTypeGuardFlow.ts, 4, 14))
101+
>bBar.elem2.nested : Symbol(nested, Decl(destructuringTypeGuardFlow.ts, 2, 14))
102+
>bBar.elem2 : Symbol(elem2, Decl(destructuringTypeGuardFlow.ts, 25, 24))
103+
>bBar : Symbol(bBar, Decl(destructuringTypeGuardFlow.ts, 25, 5))
104+
>elem2 : Symbol(elem2, Decl(destructuringTypeGuardFlow.ts, 25, 24))
105+
>nested : Symbol(nested, Decl(destructuringTypeGuardFlow.ts, 2, 14))
106+
>b : Symbol(b, Decl(destructuringTypeGuardFlow.ts, 4, 14))
107+
108+
const { bar, baz, nested: {a, b: text} } = bBar.elem2;
109+
>bar : Symbol(bar, Decl(destructuringTypeGuardFlow.ts, 28, 9))
110+
>baz : Symbol(baz, Decl(destructuringTypeGuardFlow.ts, 28, 14))
111+
>nested : Symbol(nested, Decl(destructuringTypeGuardFlow.ts, 2, 14))
112+
>a : Symbol(a, Decl(destructuringTypeGuardFlow.ts, 28, 29))
113+
>b : Symbol(b, Decl(destructuringTypeGuardFlow.ts, 4, 14))
114+
>text : Symbol(text, Decl(destructuringTypeGuardFlow.ts, 28, 31))
115+
>bBar.elem2 : Symbol(elem2, Decl(destructuringTypeGuardFlow.ts, 25, 24))
116+
>bBar : Symbol(bBar, Decl(destructuringTypeGuardFlow.ts, 25, 5))
117+
>elem2 : Symbol(elem2, Decl(destructuringTypeGuardFlow.ts, 25, 24))
118+
119+
const right: number = bBar.elem2.bar;
120+
>right : Symbol(right, Decl(destructuringTypeGuardFlow.ts, 29, 7))
121+
>bBar.elem2.bar : Symbol(bar, Decl(destructuringTypeGuardFlow.ts, 0, 12))
122+
>bBar.elem2 : Symbol(elem2, Decl(destructuringTypeGuardFlow.ts, 25, 24))
123+
>bBar : Symbol(bBar, Decl(destructuringTypeGuardFlow.ts, 25, 5))
124+
>elem2 : Symbol(elem2, Decl(destructuringTypeGuardFlow.ts, 25, 24))
125+
>bar : Symbol(bar, Decl(destructuringTypeGuardFlow.ts, 0, 12))
126+
127+
const wrong: number = bar;
128+
>wrong : Symbol(wrong, Decl(destructuringTypeGuardFlow.ts, 30, 7))
129+
>bar : Symbol(bar, Decl(destructuringTypeGuardFlow.ts, 28, 9))
130+
131+
const another: string = baz;
132+
>another : Symbol(another, Decl(destructuringTypeGuardFlow.ts, 31, 7))
133+
>baz : Symbol(baz, Decl(destructuringTypeGuardFlow.ts, 28, 14))
134+
135+
const aAgain: number = a;
136+
>aAgain : Symbol(aAgain, Decl(destructuringTypeGuardFlow.ts, 32, 7))
137+
>a : Symbol(a, Decl(destructuringTypeGuardFlow.ts, 28, 29))
138+
139+
const bAgain: string = text;
140+
>bAgain : Symbol(bAgain, Decl(destructuringTypeGuardFlow.ts, 33, 7))
141+
>text : Symbol(text, Decl(destructuringTypeGuardFlow.ts, 28, 31))
142+
}
143+

0 commit comments

Comments
 (0)