Merge pull request #28784 from Microsoft/controlFlowDestructuringLoop

Fix control flow analysis of destructuring in loops

src/compiler/checker.ts

@@ -14615,15 +14615,15 @@ namespace ts {
                         getAccessedPropertyName(source as PropertyAccessExpression | ElementAccessExpression) === getAccessedPropertyName(target) &&
                         isMatchingReference((source as PropertyAccessExpression | ElementAccessExpression).expression, target.expression);
                 case SyntaxKind.BindingElement:
-                    if (target.kind !== SyntaxKind.PropertyAccessExpression) return false;
-                    const t = target as PropertyAccessExpression;
-                    if (t.name.escapedText !== getBindingElementNameText(source as BindingElement)) return false;
-                    if (source.parent.parent.kind === SyntaxKind.BindingElement && isMatchingReference(source.parent.parent, t.expression)) {
-                            return true;
-                    }
-                    if (source.parent.parent.kind === SyntaxKind.VariableDeclaration) {
-                        const maybeId = (source.parent.parent as VariableDeclaration).initializer;
-                        return !!maybeId && isMatchingReference(maybeId, t.expression);
+                    if (target.kind === SyntaxKind.PropertyAccessExpression && (<PropertyAccessExpression>target).name.escapedText === getBindingElementNameText(<BindingElement>source)) {
+                        const ancestor = source.parent.parent;
+                        if (ancestor.kind === SyntaxKind.BindingElement) {
+                            return isMatchingReference(ancestor, (<PropertyAccessExpression>target).expression);
+                        }
+                        if (ancestor.kind === SyntaxKind.VariableDeclaration) {
+                            const initializer = (<VariableDeclaration>ancestor).initializer;
+                            return !!initializer && isMatchingReference(initializer, (<PropertyAccessExpression>target).expression);
+                        }
                     }
             }
             return false;
@@ -14635,14 +14635,25 @@ namespace ts {
                 undefined;
         }
 
+        function getReferenceParent(source: Node) {
+            if (source.kind === SyntaxKind.PropertyAccessExpression) {
+                return (<PropertyAccessExpression>source).expression;
+            }
+            if (source.kind === SyntaxKind.BindingElement) {
+                const ancestor = source.parent.parent;
+                return ancestor.kind === SyntaxKind.VariableDeclaration ? (<VariableDeclaration>ancestor).initializer : ancestor;
+            }
+            return undefined;
+        }
+
         function containsMatchingReference(source: Node, target: Node) {
-            while (source.kind === SyntaxKind.PropertyAccessExpression) {
-                source = (<PropertyAccessExpression>source).expression;
-                if (isMatchingReference(source, target)) {
+            let parent = getReferenceParent(source);
+            while (parent) {
+                if (isMatchingReference(parent, target)) {
                     return true;
                 }
+                parent = getReferenceParent(parent);
             }
-            return false;
         }
 
         // Return true if target is a property access xxx.yyy, source is a property access xxx.zzz, the declared

tests/baselines/reference/controlFlowDestructuringLoop.js

@@ -0,0 +1,43 @@
+//// [controlFlowDestructuringLoop.ts]
+// Repro from #28758
+
+interface NumVal { val: number; }
+interface StrVal { val: string; }
+type Val = NumVal | StrVal;
+
+function isNumVal(x: Val): x is NumVal {
+    return typeof x.val === 'number';
+}
+
+function foo(things: Val[]): void {
+    for (const thing of things) {
+        if (isNumVal(thing)) {
+            const { val } = thing;
+            val.toFixed(2);
+        }
+        else {
+            const { val } = thing;
+            val.length;
+        }
+    }
+}
+
+//// [controlFlowDestructuringLoop.js]
+"use strict";
+// Repro from #28758
+function isNumVal(x) {
+    return typeof x.val === 'number';
+}
+function foo(things) {
+    for (var _i = 0, things_1 = things; _i < things_1.length; _i++) {
+        var thing = things_1[_i];
+        if (isNumVal(thing)) {
+            var val = thing.val;
+            val.toFixed(2);
+        }
+        else {
+            var val = thing.val;
+            val.length;
+        }
+    }
+}

tests/baselines/reference/controlFlowDestructuringLoop.symbols

@@ -0,0 +1,63 @@
+=== tests/cases/compiler/controlFlowDestructuringLoop.ts ===
+// Repro from #28758
+
+interface NumVal { val: number; }
+>NumVal : Symbol(NumVal, Decl(controlFlowDestructuringLoop.ts, 0, 0))
+>val : Symbol(NumVal.val, Decl(controlFlowDestructuringLoop.ts, 2, 18))
+
+interface StrVal { val: string; }
+>StrVal : Symbol(StrVal, Decl(controlFlowDestructuringLoop.ts, 2, 33))
+>val : Symbol(StrVal.val, Decl(controlFlowDestructuringLoop.ts, 3, 18))
+
+type Val = NumVal | StrVal;
+>Val : Symbol(Val, Decl(controlFlowDestructuringLoop.ts, 3, 33))
+>NumVal : Symbol(NumVal, Decl(controlFlowDestructuringLoop.ts, 0, 0))
+>StrVal : Symbol(StrVal, Decl(controlFlowDestructuringLoop.ts, 2, 33))
+
+function isNumVal(x: Val): x is NumVal {
+>isNumVal : Symbol(isNumVal, Decl(controlFlowDestructuringLoop.ts, 4, 27))
+>x : Symbol(x, Decl(controlFlowDestructuringLoop.ts, 6, 18))
+>Val : Symbol(Val, Decl(controlFlowDestructuringLoop.ts, 3, 33))
+>x : Symbol(x, Decl(controlFlowDestructuringLoop.ts, 6, 18))
+>NumVal : Symbol(NumVal, Decl(controlFlowDestructuringLoop.ts, 0, 0))
+
+    return typeof x.val === 'number';
+>x.val : Symbol(val, Decl(controlFlowDestructuringLoop.ts, 2, 18), Decl(controlFlowDestructuringLoop.ts, 3, 18))
+>x : Symbol(x, Decl(controlFlowDestructuringLoop.ts, 6, 18))
+>val : Symbol(val, Decl(controlFlowDestructuringLoop.ts, 2, 18), Decl(controlFlowDestructuringLoop.ts, 3, 18))
+}
+
+function foo(things: Val[]): void {
+>foo : Symbol(foo, Decl(controlFlowDestructuringLoop.ts, 8, 1))
+>things : Symbol(things, Decl(controlFlowDestructuringLoop.ts, 10, 13))
+>Val : Symbol(Val, Decl(controlFlowDestructuringLoop.ts, 3, 33))
+
+    for (const thing of things) {
+>thing : Symbol(thing, Decl(controlFlowDestructuringLoop.ts, 11, 14))
+>things : Symbol(things, Decl(controlFlowDestructuringLoop.ts, 10, 13))
+
+        if (isNumVal(thing)) {
+>isNumVal : Symbol(isNumVal, Decl(controlFlowDestructuringLoop.ts, 4, 27))
+>thing : Symbol(thing, Decl(controlFlowDestructuringLoop.ts, 11, 14))
+
+            const { val } = thing;
+>val : Symbol(val, Decl(controlFlowDestructuringLoop.ts, 13, 19))
+>thing : Symbol(thing, Decl(controlFlowDestructuringLoop.ts, 11, 14))
+
+            val.toFixed(2);
+>val.toFixed : Symbol(Number.toFixed, Decl(lib.es5.d.ts, --, --))
+>val : Symbol(val, Decl(controlFlowDestructuringLoop.ts, 13, 19))
+>toFixed : Symbol(Number.toFixed, Decl(lib.es5.d.ts, --, --))
+        }
+        else {
+            const { val } = thing;
+>val : Symbol(val, Decl(controlFlowDestructuringLoop.ts, 17, 19))
+>thing : Symbol(thing, Decl(controlFlowDestructuringLoop.ts, 11, 14))
+
+            val.length;
+>val.length : Symbol(String.length, Decl(lib.es5.d.ts, --, --))
+>val : Symbol(val, Decl(controlFlowDestructuringLoop.ts, 17, 19))
+>length : Symbol(String.length, Decl(lib.es5.d.ts, --, --))
+        }
+    }
+}

tests/baselines/reference/controlFlowDestructuringLoop.types

@@ -0,0 +1,61 @@
+=== tests/cases/compiler/controlFlowDestructuringLoop.ts ===
+// Repro from #28758
+
+interface NumVal { val: number; }
+>val : number
+
+interface StrVal { val: string; }
+>val : string
+
+type Val = NumVal | StrVal;
+>Val : Val
+
+function isNumVal(x: Val): x is NumVal {
+>isNumVal : (x: Val) => x is NumVal
+>x : Val
+
+    return typeof x.val === 'number';
+>typeof x.val === 'number' : boolean
+>typeof x.val : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"
+>x.val : string | number
+>x : Val
+>val : string | number
+>'number' : "number"
+}
+
+function foo(things: Val[]): void {
+>foo : (things: Val[]) => void
+>things : Val[]
+
+    for (const thing of things) {
+>thing : Val
+>things : Val[]
+
+        if (isNumVal(thing)) {
+>isNumVal(thing) : boolean
+>isNumVal : (x: Val) => x is NumVal
+>thing : Val
+
+            const { val } = thing;
+>val : number
+>thing : NumVal
+
+            val.toFixed(2);
+>val.toFixed(2) : string
+>val.toFixed : (fractionDigits?: number | undefined) => string
+>val : number
+>toFixed : (fractionDigits?: number | undefined) => string
+>2 : 2
+        }
+        else {
+            const { val } = thing;
+>val : string
+>thing : StrVal
+
+            val.length;
+>val.length : number
+>val : string
+>length : number
+        }
+    }
+}

tests/cases/compiler/controlFlowDestructuringLoop.ts

@@ -0,0 +1,24 @@
+// @strict: true
+
+// Repro from #28758
+
+interface NumVal { val: number; }
+interface StrVal { val: string; }
+type Val = NumVal | StrVal;
+
+function isNumVal(x: Val): x is NumVal {
+    return typeof x.val === 'number';
+}
+
+function foo(things: Val[]): void {
+    for (const thing of things) {
+        if (isNumVal(thing)) {
+            const { val } = thing;
+            val.toFixed(2);
+        }
+        else {
+            const { val } = thing;
+            val.length;
+        }
+    }
+}