Fix33448 (#35513)

* Filter out discriminants of type 'never'. * Add tests * Accept new baselines * Remove unnecessary '!' assertion
microsoft · Dec 12, 2019 · 71a9176 · 71a9176
1 parent b98cb06
commit 71a9176
Show file tree

Hide file tree

Showing 6 changed files with 430 additions and 9 deletions.
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -18311,19 +18311,14 @@ namespace ts {
             return undefined;
         }
 
-        function isDiscriminantType(type: Type): boolean {
-            return !!(type.flags & TypeFlags.Union &&
-                (type.flags & (TypeFlags.Boolean | TypeFlags.EnumLiteral) || !isGenericIndexType(type)));
-        }
-
         function isDiscriminantProperty(type: Type | undefined, name: __String) {
             if (type && type.flags & TypeFlags.Union) {
                 const prop = getUnionOrIntersectionProperty(<UnionType>type, name);
                 if (prop && getCheckFlags(prop) & CheckFlags.SyntheticProperty) {
                     if ((<TransientSymbol>prop).isDiscriminantProperty === undefined) {
                         (<TransientSymbol>prop).isDiscriminantProperty =
                             ((<TransientSymbol>prop).checkFlags & CheckFlags.Discriminant) === CheckFlags.Discriminant &&
-                            isDiscriminantType(getTypeOfSymbol(prop));
+                            !maybeTypeOfKind(getTypeOfSymbol(prop), TypeFlags.Instantiable);
                     }
                     return !!(<TransientSymbol>prop).isDiscriminantProperty;
                 }
@@ -19512,8 +19507,14 @@ namespace ts {
                     return type;
                 }
                 const propType = getTypeOfPropertyOfType(type, propName);
-                const narrowedPropType = propType && narrowType(propType);
-                return propType === narrowedPropType ? type : filterType(type, t => isTypeComparableTo(getTypeOfPropertyOrIndexSignature(t, propName), narrowedPropType!));
+                if (!propType) {
+                    return type;
+                }
+                const narrowedPropType = narrowType(propType);
+                return filterType(type, t => {
+                    const discriminantType = getTypeOfPropertyOrIndexSignature(t, propName);
+                    return !(discriminantType.flags & TypeFlags.Never) && isTypeComparableTo(discriminantType, narrowedPropType);
+                });
             }
 
             function narrowTypeByTruthiness(type: Type, expr: Expression, assumeTrue: boolean): Type {

diff --git a/tests/baselines/reference/discriminatedUnionTypes2.errors.txt b/tests/baselines/reference/discriminatedUnionTypes2.errors.txt
@@ -2,9 +2,10 @@ tests/cases/conformance/types/union/discriminatedUnionTypes2.ts(27,30): error TS
   Object literal may only specify known properties, and 'c' does not exist in type '{ a: null; b: string; }'.
 tests/cases/conformance/types/union/discriminatedUnionTypes2.ts(32,11): error TS2339: Property 'b' does not exist on type '{ a: 0; b: string; } | { a: T; c: number; }'.
   Property 'b' does not exist on type '{ a: T; c: number; }'.
+tests/cases/conformance/types/union/discriminatedUnionTypes2.ts(132,11): error TS2339: Property 'value' does not exist on type 'never'.
 
 
-==== tests/cases/conformance/types/union/discriminatedUnionTypes2.ts (2 errors) ====
+==== tests/cases/conformance/types/union/discriminatedUnionTypes2.ts (3 errors) ====
     function f10(x : { kind: false, a: string } | { kind: true, b: string } | { kind: string, c: string }) {
         if (x.kind === false) {
             x.a;
@@ -105,4 +106,55 @@ tests/cases/conformance/types/union/discriminatedUnionTypes2.ts(32,11): error TS
             foo;
         }
     }
+
+    // Repro from #33448
+
+    type a = {
+        type: 'a',
+        data: string
+    }
+    type b = {
+        type: 'b',
+        name: string
+    }
+    type c = {
+        type: 'c',
+        other: string
+    }
+
+    type abc = a | b | c;
+
+    function f(problem: abc & (b | c)) {
+        if (problem.type === 'b') {
+            problem.name;
+        }
+        else {
+            problem.other;
+        }
+    }
+
+    type RuntimeValue =
+        | { type: 'number', value: number }
+        | { type: 'string', value: string }
+        | { type: 'boolean', value: boolean };
+
+    function foo1(x: RuntimeValue & { type: 'number' }) {
+        if (x.type === 'number') {
+            x.value;  // number
+        }
+        else {
+            x.value;  // Error, x is never
+              ~~~~~
+!!! error TS2339: Property 'value' does not exist on type 'never'.
+        }
+    }
+
+    function foo2(x: RuntimeValue & ({ type: 'number' } | { type: 'string' })) {
+        if (x.type === 'number') {
+            x.value;  // number
+        }
+        else {
+            x.value;  // string
+        }
+    }
 
diff --git a/tests/baselines/reference/discriminatedUnionTypes2.js b/tests/baselines/reference/discriminatedUnionTypes2.js
@@ -93,6 +93,55 @@ function f31(foo: Foo) {
         foo;
     }
 }
+
+// Repro from #33448
+
+type a = {
+    type: 'a',
+    data: string
+}
+type b = {
+    type: 'b',
+    name: string
+}
+type c = {
+    type: 'c',
+    other: string
+}
+
+type abc = a | b | c;
+
+function f(problem: abc & (b | c)) {
+    if (problem.type === 'b') {
+        problem.name;
+    }
+    else {
+        problem.other;
+    }
+}
+
+type RuntimeValue =
+    | { type: 'number', value: number }
+    | { type: 'string', value: string }
+    | { type: 'boolean', value: boolean };
+
+function foo1(x: RuntimeValue & { type: 'number' }) {
+    if (x.type === 'number') {
+        x.value;  // number
+    }
+    else {
+        x.value;  // Error, x is never
+    }
+}
+
+function foo2(x: RuntimeValue & ({ type: 'number' } | { type: 'string' })) {
+    if (x.type === 'number') {
+        x.value;  // number
+    }
+    else {
+        x.value;  // string
+    }
+}
 
 
 //// [discriminatedUnionTypes2.js]
@@ -164,3 +213,27 @@ function f31(foo) {
         foo;
     }
 }
+function f(problem) {
+    if (problem.type === 'b') {
+        problem.name;
+    }
+    else {
+        problem.other;
+    }
+}
+function foo1(x) {
+    if (x.type === 'number') {
+        x.value; // number
+    }
+    else {
+        x.value; // Error, x is never
+    }
+}
+function foo2(x) {
+    if (x.type === 'number') {
+        x.value; // number
+    }
+    else {
+        x.value; // string
+    }
+}
diff --git a/tests/baselines/reference/discriminatedUnionTypes2.symbols b/tests/baselines/reference/discriminatedUnionTypes2.symbols
@@ -273,3 +273,126 @@ function f31(foo: Foo) {
     }
 }
 
+// Repro from #33448
+
+type a = {
+>a : Symbol(a, Decl(discriminatedUnionTypes2.ts, 93, 1))
+
+    type: 'a',
+>type : Symbol(type, Decl(discriminatedUnionTypes2.ts, 97, 10))
+
+    data: string
+>data : Symbol(data, Decl(discriminatedUnionTypes2.ts, 98, 14))
+}
+type b = {
+>b : Symbol(b, Decl(discriminatedUnionTypes2.ts, 100, 1))
+
+    type: 'b',
+>type : Symbol(type, Decl(discriminatedUnionTypes2.ts, 101, 10))
+
+    name: string
+>name : Symbol(name, Decl(discriminatedUnionTypes2.ts, 102, 14))
+}
+type c = {
+>c : Symbol(c, Decl(discriminatedUnionTypes2.ts, 104, 1))
+
+    type: 'c',
+>type : Symbol(type, Decl(discriminatedUnionTypes2.ts, 105, 10))
+
+    other: string
+>other : Symbol(other, Decl(discriminatedUnionTypes2.ts, 106, 14))
+}
+
+type abc = a | b | c;
+>abc : Symbol(abc, Decl(discriminatedUnionTypes2.ts, 108, 1))
+>a : Symbol(a, Decl(discriminatedUnionTypes2.ts, 93, 1))
+>b : Symbol(b, Decl(discriminatedUnionTypes2.ts, 100, 1))
+>c : Symbol(c, Decl(discriminatedUnionTypes2.ts, 104, 1))
+
+function f(problem: abc & (b | c)) {
+>f : Symbol(f, Decl(discriminatedUnionTypes2.ts, 110, 21))
+>problem : Symbol(problem, Decl(discriminatedUnionTypes2.ts, 112, 11))
+>abc : Symbol(abc, Decl(discriminatedUnionTypes2.ts, 108, 1))
+>b : Symbol(b, Decl(discriminatedUnionTypes2.ts, 100, 1))
+>c : Symbol(c, Decl(discriminatedUnionTypes2.ts, 104, 1))
+
+    if (problem.type === 'b') {
+>problem.type : Symbol(type, Decl(discriminatedUnionTypes2.ts, 101, 10), Decl(discriminatedUnionTypes2.ts, 105, 10), Decl(discriminatedUnionTypes2.ts, 97, 10), Decl(discriminatedUnionTypes2.ts, 101, 10), Decl(discriminatedUnionTypes2.ts, 97, 10) ... and 5 more)
+>problem : Symbol(problem, Decl(discriminatedUnionTypes2.ts, 112, 11))
+>type : Symbol(type, Decl(discriminatedUnionTypes2.ts, 101, 10), Decl(discriminatedUnionTypes2.ts, 105, 10), Decl(discriminatedUnionTypes2.ts, 97, 10), Decl(discriminatedUnionTypes2.ts, 101, 10), Decl(discriminatedUnionTypes2.ts, 97, 10) ... and 5 more)
+
+        problem.name;
+>problem.name : Symbol(name, Decl(discriminatedUnionTypes2.ts, 102, 14))
+>problem : Symbol(problem, Decl(discriminatedUnionTypes2.ts, 112, 11))
+>name : Symbol(name, Decl(discriminatedUnionTypes2.ts, 102, 14))
+    }
+    else {
+        problem.other;
+>problem.other : Symbol(other, Decl(discriminatedUnionTypes2.ts, 106, 14))
+>problem : Symbol(problem, Decl(discriminatedUnionTypes2.ts, 112, 11))
+>other : Symbol(other, Decl(discriminatedUnionTypes2.ts, 106, 14))
+    }
+}
+
+type RuntimeValue =
+>RuntimeValue : Symbol(RuntimeValue, Decl(discriminatedUnionTypes2.ts, 119, 1))
+
+    | { type: 'number', value: number }
+>type : Symbol(type, Decl(discriminatedUnionTypes2.ts, 122, 7))
+>value : Symbol(value, Decl(discriminatedUnionTypes2.ts, 122, 23))
+
+    | { type: 'string', value: string }
+>type : Symbol(type, Decl(discriminatedUnionTypes2.ts, 123, 7))
+>value : Symbol(value, Decl(discriminatedUnionTypes2.ts, 123, 23))
+
+    | { type: 'boolean', value: boolean };
+>type : Symbol(type, Decl(discriminatedUnionTypes2.ts, 124, 7))
+>value : Symbol(value, Decl(discriminatedUnionTypes2.ts, 124, 24))
+
+function foo1(x: RuntimeValue & { type: 'number' }) {
+>foo1 : Symbol(foo1, Decl(discriminatedUnionTypes2.ts, 124, 42))
+>x : Symbol(x, Decl(discriminatedUnionTypes2.ts, 126, 14))
+>RuntimeValue : Symbol(RuntimeValue, Decl(discriminatedUnionTypes2.ts, 119, 1))
+>type : Symbol(type, Decl(discriminatedUnionTypes2.ts, 126, 33))
+
+    if (x.type === 'number') {
+>x.type : Symbol(type, Decl(discriminatedUnionTypes2.ts, 122, 7), Decl(discriminatedUnionTypes2.ts, 126, 33), Decl(discriminatedUnionTypes2.ts, 123, 7), Decl(discriminatedUnionTypes2.ts, 126, 33), Decl(discriminatedUnionTypes2.ts, 124, 7) ... and 1 more)
+>x : Symbol(x, Decl(discriminatedUnionTypes2.ts, 126, 14))
+>type : Symbol(type, Decl(discriminatedUnionTypes2.ts, 122, 7), Decl(discriminatedUnionTypes2.ts, 126, 33), Decl(discriminatedUnionTypes2.ts, 123, 7), Decl(discriminatedUnionTypes2.ts, 126, 33), Decl(discriminatedUnionTypes2.ts, 124, 7) ... and 1 more)
+
+        x.value;  // number
+>x.value : Symbol(value, Decl(discriminatedUnionTypes2.ts, 122, 23))
+>x : Symbol(x, Decl(discriminatedUnionTypes2.ts, 126, 14))
+>value : Symbol(value, Decl(discriminatedUnionTypes2.ts, 122, 23))
+    }
+    else {
+        x.value;  // Error, x is never
+>x : Symbol(x, Decl(discriminatedUnionTypes2.ts, 126, 14))
+    }
+}
+
+function foo2(x: RuntimeValue & ({ type: 'number' } | { type: 'string' })) {
+>foo2 : Symbol(foo2, Decl(discriminatedUnionTypes2.ts, 133, 1))
+>x : Symbol(x, Decl(discriminatedUnionTypes2.ts, 135, 14))
+>RuntimeValue : Symbol(RuntimeValue, Decl(discriminatedUnionTypes2.ts, 119, 1))
+>type : Symbol(type, Decl(discriminatedUnionTypes2.ts, 135, 34))
+>type : Symbol(type, Decl(discriminatedUnionTypes2.ts, 135, 55))
+
+    if (x.type === 'number') {
+>x.type : Symbol(type, Decl(discriminatedUnionTypes2.ts, 122, 7), Decl(discriminatedUnionTypes2.ts, 135, 34), Decl(discriminatedUnionTypes2.ts, 122, 7), Decl(discriminatedUnionTypes2.ts, 135, 55), Decl(discriminatedUnionTypes2.ts, 123, 7) ... and 7 more)
+>x : Symbol(x, Decl(discriminatedUnionTypes2.ts, 135, 14))
+>type : Symbol(type, Decl(discriminatedUnionTypes2.ts, 122, 7), Decl(discriminatedUnionTypes2.ts, 135, 34), Decl(discriminatedUnionTypes2.ts, 122, 7), Decl(discriminatedUnionTypes2.ts, 135, 55), Decl(discriminatedUnionTypes2.ts, 123, 7) ... and 7 more)
+
+        x.value;  // number
+>x.value : Symbol(value, Decl(discriminatedUnionTypes2.ts, 122, 23))
+>x : Symbol(x, Decl(discriminatedUnionTypes2.ts, 135, 14))
+>value : Symbol(value, Decl(discriminatedUnionTypes2.ts, 122, 23))
+    }
+    else {
+        x.value;  // string
+>x.value : Symbol(value, Decl(discriminatedUnionTypes2.ts, 123, 23))
+>x : Symbol(x, Decl(discriminatedUnionTypes2.ts, 135, 14))
+>value : Symbol(value, Decl(discriminatedUnionTypes2.ts, 123, 23))
+    }
+}
+