Fix33448 (#35513)

* Filter out discriminants of type 'never'. * Add tests * Accept new baselines * Remove unnecessary '!' assertion
2019-12-12 06:45:46 -08:00 · 2019-12-12 06:45:46 -08:00 · 71a91763f4
parent b98cb0645d
commit 71a91763f4
6 changed files with 430 additions and 9 deletions
--- a/src/compiler/checker.ts
+++ b/src/compiler/checker.ts
@ -18311,11 +18311,6 @@ 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);
@ -18323,7 +18318,7 @@ namespace ts {
                    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 {
--- 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
+        }
+    }
    
--- 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
+    }
+}
--- 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))
+    }
+}
+
--- a/tests/baselines/reference/discriminatedUnionTypes2.types
+++ b/tests/baselines/reference/discriminatedUnionTypes2.types
@ -289,3 +289,126 @@ function f31(foo: Foo) {
    }
 }

+// Repro from #33448
+
+type a = {
+>a : a
+
+    type: 'a',
+>type : "a"
+
+    data: string
+>data : string
+}
+type b = {
+>b : b
+
+    type: 'b',
+>type : "b"
+
+    name: string
+>name : string
+}
+type c = {
+>c : c
+
+    type: 'c',
+>type : "c"
+
+    other: string
+>other : string
+}
+
+type abc = a | b | c;
+>abc : abc
+
+function f(problem: abc & (b | c)) {
+>f : (problem: b | c | (a & b) | (a & c) | (b & c) | (c & b)) => void
+>problem : b | c | (a & b) | (a & c) | (b & c) | (c & b)
+
+    if (problem.type === 'b') {
+>problem.type === 'b' : boolean
+>problem.type : "b" | "c"
+>problem : b | c | (a & b) | (a & c) | (b & c) | (c & b)
+>type : "b" | "c"
+>'b' : "b"
+
+        problem.name;
+>problem.name : string
+>problem : b
+>name : string
+    }
+    else {
+        problem.other;
+>problem.other : string
+>problem : c
+>other : string
+    }
+}
+
+type RuntimeValue =
+>RuntimeValue : RuntimeValue
+
+    | { type: 'number', value: number }
+>type : "number"
+>value : number
+
+    | { type: 'string', value: string }
+>type : "string"
+>value : string
+
+    | { type: 'boolean', value: boolean };
+>type : "boolean"
+>value : boolean
+
+function foo1(x: RuntimeValue & { type: 'number' }) {
+>foo1 : (x: ({ type: "number"; value: number; } & { type: "number"; }) | ({ type: "string"; value: string; } & { type: "number"; }) | ({ type: "boolean"; value: boolean; } & { type: "number"; })) => void
+>x : ({ type: "number"; value: number; } & { type: "number"; }) | ({ type: "string"; value: string; } & { type: "number"; }) | ({ type: "boolean"; value: boolean; } & { type: "number"; })
+>type : "number"
+
+    if (x.type === 'number') {
+>x.type === 'number' : boolean
+>x.type : "number"
+>x : ({ type: "number"; value: number; } & { type: "number"; }) | ({ type: "string"; value: string; } & { type: "number"; }) | ({ type: "boolean"; value: boolean; } & { type: "number"; })
+>type : "number"
+>'number' : "number"
+
+        x.value;  // number
+>x.value : number
+>x : { type: "number"; value: number; } & { type: "number"; }
+>value : number
+    }
+    else {
+        x.value;  // Error, x is never
+>x.value : any
+>x : never
+>value : any
+    }
+}
+
+function foo2(x: RuntimeValue & ({ type: 'number' } | { type: 'string' })) {
+>foo2 : (x: ({ type: "number"; value: number; } & { type: "number"; }) | ({ type: "number"; value: number; } & { type: "string"; }) | ({ type: "string"; value: string; } & { type: "number"; }) | ({ type: "string"; value: string; } & { type: "string"; }) | ({ type: "boolean"; value: boolean; } & { type: "number"; }) | ({ type: "boolean"; value: boolean; } & { type: "string"; })) => void
+>x : ({ type: "number"; value: number; } & { type: "number"; }) | ({ type: "number"; value: number; } & { type: "string"; }) | ({ type: "string"; value: string; } & { type: "number"; }) | ({ type: "string"; value: string; } & { type: "string"; }) | ({ type: "boolean"; value: boolean; } & { type: "number"; }) | ({ type: "boolean"; value: boolean; } & { type: "string"; })
+>type : "number"
+>type : "string"
+
+    if (x.type === 'number') {
+>x.type === 'number' : boolean
+>x.type : "string" | "number"
+>x : ({ type: "number"; value: number; } & { type: "number"; }) | ({ type: "number"; value: number; } & { type: "string"; }) | ({ type: "string"; value: string; } & { type: "number"; }) | ({ type: "string"; value: string; } & { type: "string"; }) | ({ type: "boolean"; value: boolean; } & { type: "number"; }) | ({ type: "boolean"; value: boolean; } & { type: "string"; })
+>type : "string" | "number"
+>'number' : "number"
+
+        x.value;  // number
+>x.value : number
+>x : { type: "number"; value: number; } & { type: "number"; }
+>value : number
+    }
+    else {
+        x.value;  // string
+>x.value : string
+>x : { type: "string"; value: string; } & { type: "string"; }
+>value : string
+    }
+}
+
--- a/tests/cases/conformance/types/union/discriminatedUnionTypes2.ts
+++ b/tests/cases/conformance/types/union/discriminatedUnionTypes2.ts
@ -94,3 +94,52 @@ 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
+    }
+}