Merge pull request #28112 from Microsoft/fixInstanceofControlFlow

ahejlsberg · web-flow · commit 6e8e5c10dde5 · 2018-10-25T06:16:39.000-07:00
Fix instanceof control flow analysis
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -14340,6 +14340,11 @@ namespace ts {
             return false;
         }
 
+        function hasNarrowableDeclaredType(expr: Node) {
+            const type = getDeclaredTypeOfReference(expr);
+            return !!(type && type.flags & TypeFlags.Union);
+        }
+
         function findDiscriminantProperties(sourceProperties: Symbol[], target: Type): Symbol[] | undefined {
             let result: Symbol[] | undefined;
             for (const sourceProperty of sourceProperties) {
@@ -15373,9 +15378,9 @@ namespace ts {
                 // We have '==', '!=', '====', or !==' operator with 'typeof xxx' and string literal operands
                 const target = getReferenceCandidate(typeOfExpr.expression);
                 if (!isMatchingReference(reference, target)) {
-                    // For a reference of the form 'x.y', a 'typeof x === ...' type guard resets the
-                    // narrowed type of 'y' to its declared type.
-                    if (containsMatchingReference(reference, target)) {
+                    // For a reference of the form 'x.y', where 'x' has a narrowable declared type, a
+                    // 'typeof x === ...' type guard resets the narrowed type of 'y' to its declared type.
+                    if (containsMatchingReference(reference, target) && hasNarrowableDeclaredType(target)) {
                         return declaredType;
                     }
                     return type;
@@ -15516,9 +15521,9 @@ namespace ts {
             function narrowTypeByInstanceof(type: Type, expr: BinaryExpression, assumeTrue: boolean): Type {
                 const left = getReferenceCandidate(expr.left);
                 if (!isMatchingReference(reference, left)) {
-                    // For a reference of the form 'x.y', an 'x instanceof T' type guard resets the
-                    // narrowed type of 'y' to its declared type.
-                    if (containsMatchingReference(reference, left)) {
+                    // For a reference of the form 'x.y', where 'x' has a narrowable declared type, an
+                    // 'x instanceof T' type guard resets the narrowed type of 'y' to its declared type.
+                    if (containsMatchingReference(reference, left) && hasNarrowableDeclaredType(left)) {
                         return declaredType;
                     }
                     return type;
diff --git a/tests/baselines/reference/narrowingOfDottedNames.errors.txt b/tests/baselines/reference/narrowingOfDottedNames.errors.txt
@@ -0,0 +1,67 @@
+tests/cases/compiler/narrowingOfDottedNames.ts(45,5): error TS2564: Property 'x' has no initializer and is not definitely assigned in the constructor.
+tests/cases/compiler/narrowingOfDottedNames.ts(54,5): error TS2564: Property 'x' has no initializer and is not definitely assigned in the constructor.
+
+
+==== tests/cases/compiler/narrowingOfDottedNames.ts (2 errors) ====
+    // Repro from #8383
+    
+    class A {
+        prop!: { a: string; };
+    }
+    
+    class B {
+        prop!: { b: string; }
+    }
+    
+    function isA(x: any): x is A {
+        return x instanceof A;
+    }
+    
+    function isB(x: any): x is B {
+        return x instanceof B;
+    }
+    
+    function f1(x: A | B) {
+        while (true) {
+            if (x instanceof A) {
+                x.prop.a;
+            }
+            else if (x instanceof B) {
+                x.prop.b;
+            }
+        }
+    }
+    
+    function f2(x: A | B) {
+        while (true) {
+            if (isA(x)) {
+                x.prop.a;
+            }
+            else if (isB(x)) {
+                x.prop.b;
+            }
+        }
+    }
+    
+    // Repro from #28100
+    
+    class Foo1
+    {
+        x: number;  // Error
+        ~
+!!! error TS2564: Property 'x' has no initializer and is not definitely assigned in the constructor.
+        constructor() {
+            if (this instanceof Boolean) {
+            }
+        }
+    }
+    
+    class Foo2
+    {
+        x: number;  // Error
+        ~
+!!! error TS2564: Property 'x' has no initializer and is not definitely assigned in the constructor.
+        constructor() {
+        }
+    }
+    
diff --git a/tests/baselines/reference/narrowingOfDottedNames.js b/tests/baselines/reference/narrowingOfDottedNames.js
@@ -2,11 +2,11 @@
 // Repro from #8383
 
 class A {
-    prop: { a: string; };
+    prop!: { a: string; };
 }
 
 class B {
-    prop: { b: string; }
+    prop!: { b: string; }
 }
 
 function isA(x: any): x is A {
@@ -38,9 +38,28 @@ function f2(x: A | B) {
         }
     }
 }
+
+// Repro from #28100
+
+class Foo1
+{
+    x: number;  // Error
+    constructor() {
+        if (this instanceof Boolean) {
+        }
+    }
+}
+
+class Foo2
+{
+    x: number;  // Error
+    constructor() {
+    }
+}
 
 
 //// [narrowingOfDottedNames.js]
+"use strict";
 // Repro from #8383
 var A = /** @class */ (function () {
     function A() {
@@ -78,3 +97,16 @@ function f2(x) {
         }
     }
 }
+// Repro from #28100
+var Foo1 = /** @class */ (function () {
+    function Foo1() {
+        if (this instanceof Boolean) {
+        }
+    }
+    return Foo1;
+}());
+var Foo2 = /** @class */ (function () {
+    function Foo2() {
+    }
+    return Foo2;
+}());
diff --git a/tests/baselines/reference/narrowingOfDottedNames.symbols b/tests/baselines/reference/narrowingOfDottedNames.symbols
@@ -4,17 +4,17 @@
 class A {
 >A : Symbol(A, Decl(narrowingOfDottedNames.ts, 0, 0))
 
-    prop: { a: string; };
+    prop!: { a: string; };
 >prop : Symbol(A.prop, Decl(narrowingOfDottedNames.ts, 2, 9))
->a : Symbol(a, Decl(narrowingOfDottedNames.ts, 3, 11))
+>a : Symbol(a, Decl(narrowingOfDottedNames.ts, 3, 12))
 }
 
 class B {
 >B : Symbol(B, Decl(narrowingOfDottedNames.ts, 4, 1))
 
-    prop: { b: string; }
+    prop!: { b: string; }
 >prop : Symbol(B.prop, Decl(narrowingOfDottedNames.ts, 6, 9))
->b : Symbol(b, Decl(narrowingOfDottedNames.ts, 7, 11))
+>b : Symbol(b, Decl(narrowingOfDottedNames.ts, 7, 12))
 }
 
 function isA(x: any): x is A {
@@ -51,22 +51,22 @@ function f1(x: A | B) {
 >A : Symbol(A, Decl(narrowingOfDottedNames.ts, 0, 0))
 
             x.prop.a;
->x.prop.a : Symbol(a, Decl(narrowingOfDottedNames.ts, 3, 11))
+>x.prop.a : Symbol(a, Decl(narrowingOfDottedNames.ts, 3, 12))
 >x.prop : Symbol(A.prop, Decl(narrowingOfDottedNames.ts, 2, 9))
 >x : Symbol(x, Decl(narrowingOfDottedNames.ts, 18, 12))
 >prop : Symbol(A.prop, Decl(narrowingOfDottedNames.ts, 2, 9))
->a : Symbol(a, Decl(narrowingOfDottedNames.ts, 3, 11))
+>a : Symbol(a, Decl(narrowingOfDottedNames.ts, 3, 12))
         }
         else if (x instanceof B) {
 >x : Symbol(x, Decl(narrowingOfDottedNames.ts, 18, 12))
 >B : Symbol(B, Decl(narrowingOfDottedNames.ts, 4, 1))
 
             x.prop.b;
->x.prop.b : Symbol(b, Decl(narrowingOfDottedNames.ts, 7, 11))
+>x.prop.b : Symbol(b, Decl(narrowingOfDottedNames.ts, 7, 12))
 >x.prop : Symbol(B.prop, Decl(narrowingOfDottedNames.ts, 6, 9))
 >x : Symbol(x, Decl(narrowingOfDottedNames.ts, 18, 12))
 >prop : Symbol(B.prop, Decl(narrowingOfDottedNames.ts, 6, 9))
->b : Symbol(b, Decl(narrowingOfDottedNames.ts, 7, 11))
+>b : Symbol(b, Decl(narrowingOfDottedNames.ts, 7, 12))
         }
     }
 }
@@ -83,23 +83,49 @@ function f2(x: A | B) {
 >x : Symbol(x, Decl(narrowingOfDottedNames.ts, 29, 12))
 
             x.prop.a;
->x.prop.a : Symbol(a, Decl(narrowingOfDottedNames.ts, 3, 11))
+>x.prop.a : Symbol(a, Decl(narrowingOfDottedNames.ts, 3, 12))
 >x.prop : Symbol(A.prop, Decl(narrowingOfDottedNames.ts, 2, 9))
 >x : Symbol(x, Decl(narrowingOfDottedNames.ts, 29, 12))
 >prop : Symbol(A.prop, Decl(narrowingOfDottedNames.ts, 2, 9))
->a : Symbol(a, Decl(narrowingOfDottedNames.ts, 3, 11))
+>a : Symbol(a, Decl(narrowingOfDottedNames.ts, 3, 12))
         }
         else if (isB(x)) {
 >isB : Symbol(isB, Decl(narrowingOfDottedNames.ts, 12, 1))
 >x : Symbol(x, Decl(narrowingOfDottedNames.ts, 29, 12))
 
             x.prop.b;
->x.prop.b : Symbol(b, Decl(narrowingOfDottedNames.ts, 7, 11))
+>x.prop.b : Symbol(b, Decl(narrowingOfDottedNames.ts, 7, 12))
 >x.prop : Symbol(B.prop, Decl(narrowingOfDottedNames.ts, 6, 9))
 >x : Symbol(x, Decl(narrowingOfDottedNames.ts, 29, 12))
 >prop : Symbol(B.prop, Decl(narrowingOfDottedNames.ts, 6, 9))
->b : Symbol(b, Decl(narrowingOfDottedNames.ts, 7, 11))
+>b : Symbol(b, Decl(narrowingOfDottedNames.ts, 7, 12))
         }
     }
 }
 
+// Repro from #28100
+
+class Foo1
+>Foo1 : Symbol(Foo1, Decl(narrowingOfDottedNames.ts, 38, 1))
+{
+    x: number;  // Error
+>x : Symbol(Foo1.x, Decl(narrowingOfDottedNames.ts, 43, 1))
+
+    constructor() {
+        if (this instanceof Boolean) {
+>this : Symbol(Foo1, Decl(narrowingOfDottedNames.ts, 38, 1))
+>Boolean : Symbol(Boolean, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
+        }
+    }
+}
+
+class Foo2
+>Foo2 : Symbol(Foo2, Decl(narrowingOfDottedNames.ts, 49, 1))
+{
+    x: number;  // Error
+>x : Symbol(Foo2.x, Decl(narrowingOfDottedNames.ts, 52, 1))
+
+    constructor() {
+    }
+}
+
diff --git a/tests/baselines/reference/narrowingOfDottedNames.types b/tests/baselines/reference/narrowingOfDottedNames.types
@@ -4,15 +4,15 @@
 class A {
 >A : A
 
-    prop: { a: string; };
+    prop!: { a: string; };
 >prop : { a: string; }
 >a : string
 }
 
 class B {
 >B : B
 
-    prop: { b: string; }
+    prop!: { b: string; }
 >prop : { b: string; }
 >b : string
 }
@@ -105,3 +105,30 @@ function f2(x: A | B) {
     }
 }
 
+// Repro from #28100
+
+class Foo1
+>Foo1 : Foo1
+{
+    x: number;  // Error
+>x : number
+
+    constructor() {
+        if (this instanceof Boolean) {
+>this instanceof Boolean : boolean
+>this : this
+>Boolean : BooleanConstructor
+        }
+    }
+}
+
+class Foo2
+>Foo2 : Foo2
+{
+    x: number;  // Error
+>x : number
+
+    constructor() {
+    }
+}
+
diff --git a/tests/cases/compiler/narrowingOfDottedNames.ts b/tests/cases/compiler/narrowingOfDottedNames.ts
@@ -1,11 +1,13 @@
+// @strict: true
+
 // Repro from #8383
 
 class A {
-    prop: { a: string; };
+    prop!: { a: string; };
 }
 
 class B {
-    prop: { b: string; }
+    prop!: { b: string; }
 }
 
 function isA(x: any): x is A {
@@ -37,3 +39,21 @@ function f2(x: A | B) {
         }
     }
 }
+
+// Repro from #28100
+
+class Foo1
+{
+    x: number;  // Error
+    constructor() {
+        if (this instanceof Boolean) {
+        }
+    }
+}
+
+class Foo2
+{
+    x: number;  // Error
+    constructor() {
+    }
+}
