Skip to content

Commit 28f8fda

Browse files
ahejlsbergahejlsberg
authored
Merge pull request #28784 from Microsoft/controlFlowDestructuringLoop
Fix control flow analysis of destructuring in loops
2 parents 4263285 + e968243 commit 28f8fda

File tree

5 files changed

+215
-13
lines changed

5 files changed

+215
-13
lines changed

‎src/compiler/checker.ts

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14615,15 +14615,15 @@ namespace ts {
1461514615
getAccessedPropertyName(source as PropertyAccessExpression | ElementAccessExpression) === getAccessedPropertyName(target) &&
1461614616
isMatchingReference((source as PropertyAccessExpression | ElementAccessExpression).expression, target.expression);
1461714617
case SyntaxKind.BindingElement:
14618-
if (target.kind !== SyntaxKind.PropertyAccessExpression) return false;
14619-
const t = target as PropertyAccessExpression;
14620-
if (t.name.escapedText !== getBindingElementNameText(source as BindingElement)) return false;
14621-
if (source.parent.parent.kind === SyntaxKind.BindingElement && isMatchingReference(source.parent.parent, t.expression)) {
14622-
return true;
14623-
}
14624-
if (source.parent.parent.kind === SyntaxKind.VariableDeclaration) {
14625-
const maybeId = (source.parent.parent as VariableDeclaration).initializer;
14626-
return !!maybeId && isMatchingReference(maybeId, t.expression);
14618+
if (target.kind === SyntaxKind.PropertyAccessExpression && (<PropertyAccessExpression>target).name.escapedText === getBindingElementNameText(<BindingElement>source)) {
14619+
const ancestor = source.parent.parent;
14620+
if (ancestor.kind === SyntaxKind.BindingElement) {
14621+
return isMatchingReference(ancestor, (<PropertyAccessExpression>target).expression);
14622+
}
14623+
if (ancestor.kind === SyntaxKind.VariableDeclaration) {
14624+
const initializer = (<VariableDeclaration>ancestor).initializer;
14625+
return !!initializer && isMatchingReference(initializer, (<PropertyAccessExpression>target).expression);
14626+
}
1462714627
}
1462814628
}
1462914629
return false;
@@ -14635,14 +14635,25 @@ namespace ts {
1463514635
undefined;
1463614636
}
1463714637

14638+
function getReferenceParent(source: Node) {
14639+
if (source.kind === SyntaxKind.PropertyAccessExpression) {
14640+
return (<PropertyAccessExpression>source).expression;
14641+
}
14642+
if (source.kind === SyntaxKind.BindingElement) {
14643+
const ancestor = source.parent.parent;
14644+
return ancestor.kind === SyntaxKind.VariableDeclaration ? (<VariableDeclaration>ancestor).initializer : ancestor;
14645+
}
14646+
return undefined;
14647+
}
14648+
1463814649
function containsMatchingReference(source: Node, target: Node) {
14639-
while (source.kind === SyntaxKind.PropertyAccessExpression) {
14640-
source = (<PropertyAccessExpression>source).expression;
14641-
if (isMatchingReference(source, target)) {
14650+
let parent = getReferenceParent(source);
14651+
while (parent) {
14652+
if (isMatchingReference(parent, target)) {
1464214653
return true;
1464314654
}
14655+
parent = getReferenceParent(parent);
1464414656
}
14645-
return false;
1464614657
}
1464714658

1464814659
// Return true if target is a property access xxx.yyy, source is a property access xxx.zzz, the declared

‎tests/baselines/reference/controlFlowDestructuringLoop.js

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
//// [controlFlowDestructuringLoop.ts]
2+
// Repro from #28758
3+
4+
interface NumVal { val: number; }
5+
interface StrVal { val: string; }
6+
type Val = NumVal | StrVal;
7+
8+
function isNumVal(x: Val): x is NumVal {
9+
return typeof x.val === 'number';
10+
}
11+
12+
function foo(things: Val[]): void {
13+
for (const thing of things) {
14+
if (isNumVal(thing)) {
15+
const { val } = thing;
16+
val.toFixed(2);
17+
}
18+
else {
19+
const { val } = thing;
20+
val.length;
21+
}
22+
}
23+
}
24+
25+
//// [controlFlowDestructuringLoop.js]
26+
"use strict";
27+
// Repro from #28758
28+
function isNumVal(x) {
29+
return typeof x.val === 'number';
30+
}
31+
function foo(things) {
32+
for (var _i = 0, things_1 = things; _i < things_1.length; _i++) {
33+
var thing = things_1[_i];
34+
if (isNumVal(thing)) {
35+
var val = thing.val;
36+
val.toFixed(2);
37+
}
38+
else {
39+
var val = thing.val;
40+
val.length;
41+
}
42+
}
43+
}

‎tests/baselines/reference/controlFlowDestructuringLoop.symbols

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
=== tests/cases/compiler/controlFlowDestructuringLoop.ts ===
2+
// Repro from #28758
3+
4+
interface NumVal { val: number; }
5+
>NumVal : Symbol(NumVal, Decl(controlFlowDestructuringLoop.ts, 0, 0))
6+
>val : Symbol(NumVal.val, Decl(controlFlowDestructuringLoop.ts, 2, 18))
7+
8+
interface StrVal { val: string; }
9+
>StrVal : Symbol(StrVal, Decl(controlFlowDestructuringLoop.ts, 2, 33))
10+
>val : Symbol(StrVal.val, Decl(controlFlowDestructuringLoop.ts, 3, 18))
11+
12+
type Val = NumVal | StrVal;
13+
>Val : Symbol(Val, Decl(controlFlowDestructuringLoop.ts, 3, 33))
14+
>NumVal : Symbol(NumVal, Decl(controlFlowDestructuringLoop.ts, 0, 0))
15+
>StrVal : Symbol(StrVal, Decl(controlFlowDestructuringLoop.ts, 2, 33))
16+
17+
function isNumVal(x: Val): x is NumVal {
18+
>isNumVal : Symbol(isNumVal, Decl(controlFlowDestructuringLoop.ts, 4, 27))
19+
>x : Symbol(x, Decl(controlFlowDestructuringLoop.ts, 6, 18))
20+
>Val : Symbol(Val, Decl(controlFlowDestructuringLoop.ts, 3, 33))
21+
>x : Symbol(x, Decl(controlFlowDestructuringLoop.ts, 6, 18))
22+
>NumVal : Symbol(NumVal, Decl(controlFlowDestructuringLoop.ts, 0, 0))
23+
24+
return typeof x.val === 'number';
25+
>x.val : Symbol(val, Decl(controlFlowDestructuringLoop.ts, 2, 18), Decl(controlFlowDestructuringLoop.ts, 3, 18))
26+
>x : Symbol(x, Decl(controlFlowDestructuringLoop.ts, 6, 18))
27+
>val : Symbol(val, Decl(controlFlowDestructuringLoop.ts, 2, 18), Decl(controlFlowDestructuringLoop.ts, 3, 18))
28+
}
29+
30+
function foo(things: Val[]): void {
31+
>foo : Symbol(foo, Decl(controlFlowDestructuringLoop.ts, 8, 1))
32+
>things : Symbol(things, Decl(controlFlowDestructuringLoop.ts, 10, 13))
33+
>Val : Symbol(Val, Decl(controlFlowDestructuringLoop.ts, 3, 33))
34+
35+
for (const thing of things) {
36+
>thing : Symbol(thing, Decl(controlFlowDestructuringLoop.ts, 11, 14))
37+
>things : Symbol(things, Decl(controlFlowDestructuringLoop.ts, 10, 13))
38+
39+
if (isNumVal(thing)) {
40+
>isNumVal : Symbol(isNumVal, Decl(controlFlowDestructuringLoop.ts, 4, 27))
41+
>thing : Symbol(thing, Decl(controlFlowDestructuringLoop.ts, 11, 14))
42+
43+
const { val } = thing;
44+
>val : Symbol(val, Decl(controlFlowDestructuringLoop.ts, 13, 19))
45+
>thing : Symbol(thing, Decl(controlFlowDestructuringLoop.ts, 11, 14))
46+
47+
val.toFixed(2);
48+
>val.toFixed : Symbol(Number.toFixed, Decl(lib.es5.d.ts, --, --))
49+
>val : Symbol(val, Decl(controlFlowDestructuringLoop.ts, 13, 19))
50+
>toFixed : Symbol(Number.toFixed, Decl(lib.es5.d.ts, --, --))
51+
}
52+
else {
53+
const { val } = thing;
54+
>val : Symbol(val, Decl(controlFlowDestructuringLoop.ts, 17, 19))
55+
>thing : Symbol(thing, Decl(controlFlowDestructuringLoop.ts, 11, 14))
56+
57+
val.length;
58+
>val.length : Symbol(String.length, Decl(lib.es5.d.ts, --, --))
59+
>val : Symbol(val, Decl(controlFlowDestructuringLoop.ts, 17, 19))
60+
>length : Symbol(String.length, Decl(lib.es5.d.ts, --, --))
61+
}
62+
}
63+
}

‎tests/baselines/reference/controlFlowDestructuringLoop.types

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
=== tests/cases/compiler/controlFlowDestructuringLoop.ts ===
2+
// Repro from #28758
3+
4+
interface NumVal { val: number; }
5+
>val : number
6+
7+
interface StrVal { val: string; }
8+
>val : string
9+
10+
type Val = NumVal | StrVal;
11+
>Val : Val
12+
13+
function isNumVal(x: Val): x is NumVal {
14+
>isNumVal : (x: Val) => x is NumVal
15+
>x : Val
16+
17+
return typeof x.val === 'number';
18+
>typeof x.val === 'number' : boolean
19+
>typeof x.val : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"
20+
>x.val : string | number
21+
>x : Val
22+
>val : string | number
23+
>'number' : "number"
24+
}
25+
26+
function foo(things: Val[]): void {
27+
>foo : (things: Val[]) => void
28+
>things : Val[]
29+
30+
for (const thing of things) {
31+
>thing : Val
32+
>things : Val[]
33+
34+
if (isNumVal(thing)) {
35+
>isNumVal(thing) : boolean
36+
>isNumVal : (x: Val) => x is NumVal
37+
>thing : Val
38+
39+
const { val } = thing;
40+
>val : number
41+
>thing : NumVal
42+
43+
val.toFixed(2);
44+
>val.toFixed(2) : string
45+
>val.toFixed : (fractionDigits?: number | undefined) => string
46+
>val : number
47+
>toFixed : (fractionDigits?: number | undefined) => string
48+
>2 : 2
49+
}
50+
else {
51+
const { val } = thing;
52+
>val : string
53+
>thing : StrVal
54+
55+
val.length;
56+
>val.length : number
57+
>val : string
58+
>length : number
59+
}
60+
}
61+
}

‎tests/cases/compiler/controlFlowDestructuringLoop.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// @strict: true
2+
3+
// Repro from #28758
4+
5+
interface NumVal { val: number; }
6+
interface StrVal { val: string; }
7+
type Val = NumVal | StrVal;
8+
9+
function isNumVal(x: Val): x is NumVal {
10+
return typeof x.val === 'number';
11+
}
12+
13+
function foo(things: Val[]): void {
14+
for (const thing of things) {
15+
if (isNumVal(thing)) {
16+
const { val } = thing;
17+
val.toFixed(2);
18+
}
19+
else {
20+
const { val } = thing;
21+
val.length;
22+
}
23+
}
24+
}

0 commit comments

Comments
 (0)