From 0929a2fc9934168f479d75aaa1446fd0b07477a6 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Wed, 4 Dec 2019 16:50:37 -0800 Subject: [PATCH 1/4] Filter out discriminants of type 'never'. --- src/compiler/checker.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index a9b07dd145466..784e32b318949 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -18304,11 +18304,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(type, name); @@ -18316,7 +18311,7 @@ namespace ts { if ((prop).isDiscriminantProperty === undefined) { (prop).isDiscriminantProperty = ((prop).checkFlags & CheckFlags.Discriminant) === CheckFlags.Discriminant && - isDiscriminantType(getTypeOfSymbol(prop)); + !maybeTypeOfKind(getTypeOfSymbol(prop), TypeFlags.Instantiable); } return !!(prop).isDiscriminantProperty; } @@ -19505,8 +19500,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 { From 220034d9318cc22702a3037264c1fb53097c2d42 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Wed, 4 Dec 2019 17:02:20 -0800 Subject: [PATCH 2/4] Add tests --- .../types/union/discriminatedUnionTypes2.ts | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/cases/conformance/types/union/discriminatedUnionTypes2.ts b/tests/cases/conformance/types/union/discriminatedUnionTypes2.ts index a11e1447780ee..b73fdb8f0ab83 100644 --- 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 + } +} From 6bc8b122d845c3ae8dbf67e553fc012fa8c7b0c7 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Wed, 4 Dec 2019 17:02:27 -0800 Subject: [PATCH 3/4] Accept new baselines --- .../discriminatedUnionTypes2.errors.txt | 54 +++++++- .../reference/discriminatedUnionTypes2.js | 73 +++++++++++ .../discriminatedUnionTypes2.symbols | 123 ++++++++++++++++++ .../reference/discriminatedUnionTypes2.types | 123 ++++++++++++++++++ 4 files changed, 372 insertions(+), 1 deletion(-) diff --git a/tests/baselines/reference/discriminatedUnionTypes2.errors.txt b/tests/baselines/reference/discriminatedUnionTypes2.errors.txt index 15ed6968aeac2..0cb87c1aa5931 100644 --- 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 + } + } \ No newline at end of file diff --git a/tests/baselines/reference/discriminatedUnionTypes2.js b/tests/baselines/reference/discriminatedUnionTypes2.js index ec5b2a6b5f13f..584678ee7fa53 100644 --- 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 index 682c62b9be545..d338561cb535c 100644 --- 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)) + } +} + diff --git a/tests/baselines/reference/discriminatedUnionTypes2.types b/tests/baselines/reference/discriminatedUnionTypes2.types index 58819d0d3495a..96759a759cb0a 100644 --- 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 + } +} + From c14b160328788a2ebb36f64e3cb02463fd2bdab2 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Thu, 5 Dec 2019 06:47:47 -0800 Subject: [PATCH 4/4] Remove unnecessary '!' assertion --- src/compiler/checker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 784e32b318949..1f1966a13db6e 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -19506,7 +19506,7 @@ namespace ts { const narrowedPropType = narrowType(propType); return filterType(type, t => { const discriminantType = getTypeOfPropertyOrIndexSignature(t, propName); - return !(discriminantType.flags & TypeFlags.Never) && isTypeComparableTo(discriminantType, narrowedPropType!); + return !(discriminantType.flags & TypeFlags.Never) && isTypeComparableTo(discriminantType, narrowedPropType); }); }