From d79c37cd191a5d10ad678f2a1379c6267e914bf2 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Mon, 6 Nov 2017 16:09:35 -0800 Subject: [PATCH] Discriminate contextual types (#19733) * Discriminate contextual types * Invert conditional * Update findMatchingDiscriminantType and baselines --- src/compiler/checker.ts | 59 ++++++++++++---- .../contextuallyTypedByDiscriminableUnion.js | 42 +++++++++++ ...textuallyTypedByDiscriminableUnion.symbols | 60 ++++++++++++++++ ...ontextuallyTypedByDiscriminableUnion.types | 70 +++++++++++++++++++ .../excessPropertyCheckWithUnions.errors.txt | 18 +++-- .../excessPropertyCheckWithUnions.js | 4 +- .../excessPropertyCheckWithUnions.symbols | 2 +- .../excessPropertyCheckWithUnions.types | 6 +- .../contextuallyTypedByDiscriminableUnion.ts | 25 +++++++ .../compiler/excessPropertyCheckWithUnions.ts | 2 +- 10 files changed, 262 insertions(+), 26 deletions(-) create mode 100644 tests/baselines/reference/contextuallyTypedByDiscriminableUnion.js create mode 100644 tests/baselines/reference/contextuallyTypedByDiscriminableUnion.symbols create mode 100644 tests/baselines/reference/contextuallyTypedByDiscriminableUnion.types create mode 100644 tests/cases/compiler/contextuallyTypedByDiscriminableUnion.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 649deef7e3..7d9a32a8f9 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -9269,20 +9269,24 @@ namespace ts { return Ternary.False; } + // Keep this up-to-date with the same logic within `getApparentTypeOfContextualType`, since they should behave similarly function findMatchingDiscriminantType(source: Type, target: UnionOrIntersectionType) { let match: Type; const sourceProperties = getPropertiesOfObjectType(source); if (sourceProperties) { - const sourceProperty = findSingleDiscriminantProperty(sourceProperties, target); - if (sourceProperty) { - const sourceType = getTypeOfSymbol(sourceProperty); - for (const type of target.types) { - const targetType = getTypeOfPropertyOfType(type, sourceProperty.escapedName); - if (targetType && isRelatedTo(sourceType, targetType)) { - if (match) { - return undefined; + const sourcePropertiesFiltered = findDiscriminantProperties(sourceProperties, target); + if (sourcePropertiesFiltered) { + for (const sourceProperty of sourcePropertiesFiltered) { + const sourceType = getTypeOfSymbol(sourceProperty); + for (const type of target.types) { + const targetType = getTypeOfPropertyOfType(type, sourceProperty.escapedName); + if (targetType && isRelatedTo(sourceType, targetType)) { + if (type === match) continue; // Finding multiple fields which discriminate to the same type is fine + if (match) { + return undefined; + } + match = type; } - match = type; } } } @@ -11396,14 +11400,15 @@ namespace ts { return false; } - function findSingleDiscriminantProperty(sourceProperties: Symbol[], target: Type): Symbol | undefined { - let result: Symbol; + function findDiscriminantProperties(sourceProperties: Symbol[], target: Type): Symbol[] | undefined { + let result: Symbol[]; for (const sourceProperty of sourceProperties) { if (isDiscriminantProperty(target, sourceProperty.escapedName)) { if (result) { - return undefined; + result.push(sourceProperty); + continue; } - result = sourceProperty; + result = [sourceProperty]; } } return result; @@ -13691,8 +13696,32 @@ namespace ts { // Return the contextual type for a given expression node. During overload resolution, a contextual type may temporarily // be "pushed" onto a node using the contextualType property. function getApparentTypeOfContextualType(node: Expression): Type { - const type = getContextualType(node); - return type && mapType(type, getApparentType); + let contextualType = getContextualType(node); + contextualType = contextualType && mapType(contextualType, getApparentType); + if (!(contextualType && contextualType.flags & TypeFlags.Union && isObjectLiteralExpression(node))) { + return contextualType; + } + // Keep the below up-to-date with the work done within `isRelatedTo` by `findMatchingDiscriminantType` + let match: Type | undefined; + propLoop: for (const prop of node.properties) { + if (!prop.symbol) continue; + if (prop.kind !== SyntaxKind.PropertyAssignment) continue; + if (isDiscriminantProperty(contextualType, prop.symbol.escapedName)) { + const discriminatingType = getTypeOfNode(prop.initializer); + for (const type of (contextualType as UnionType).types) { + const targetType = getTypeOfPropertyOfType(type, prop.symbol.escapedName); + if (targetType && checkTypeAssignableTo(discriminatingType, targetType, /*errorNode*/ undefined)) { + if (match) { + if (type === match) continue; // Finding multiple fields which discriminate to the same type is fine + match = undefined; + break propLoop; + } + match = type; + } + } + } + } + return match || contextualType; } /** diff --git a/tests/baselines/reference/contextuallyTypedByDiscriminableUnion.js b/tests/baselines/reference/contextuallyTypedByDiscriminableUnion.js new file mode 100644 index 0000000000..b61c235dee --- /dev/null +++ b/tests/baselines/reference/contextuallyTypedByDiscriminableUnion.js @@ -0,0 +1,42 @@ +//// [contextuallyTypedByDiscriminableUnion.ts] +type ADT = { + kind: "a", + method(x: string): number; +} | { + kind: "b", + method(x: number): string; +}; + + +function invoke(item: ADT) { + if (item.kind === "a") { + item.method(""); + } + else { + item.method(42); + } +} + +invoke({ + kind: "a", + method(a) { + return +a; + } +}); + + +//// [contextuallyTypedByDiscriminableUnion.js] +function invoke(item) { + if (item.kind === "a") { + item.method(""); + } + else { + item.method(42); + } +} +invoke({ + kind: "a", + method: function (a) { + return +a; + } +}); diff --git a/tests/baselines/reference/contextuallyTypedByDiscriminableUnion.symbols b/tests/baselines/reference/contextuallyTypedByDiscriminableUnion.symbols new file mode 100644 index 0000000000..e4ef8ee73e --- /dev/null +++ b/tests/baselines/reference/contextuallyTypedByDiscriminableUnion.symbols @@ -0,0 +1,60 @@ +=== tests/cases/compiler/contextuallyTypedByDiscriminableUnion.ts === +type ADT = { +>ADT : Symbol(ADT, Decl(contextuallyTypedByDiscriminableUnion.ts, 0, 0)) + + kind: "a", +>kind : Symbol(kind, Decl(contextuallyTypedByDiscriminableUnion.ts, 0, 12)) + + method(x: string): number; +>method : Symbol(method, Decl(contextuallyTypedByDiscriminableUnion.ts, 1, 14)) +>x : Symbol(x, Decl(contextuallyTypedByDiscriminableUnion.ts, 2, 11)) + +} | { + kind: "b", +>kind : Symbol(kind, Decl(contextuallyTypedByDiscriminableUnion.ts, 3, 5)) + + method(x: number): string; +>method : Symbol(method, Decl(contextuallyTypedByDiscriminableUnion.ts, 4, 14)) +>x : Symbol(x, Decl(contextuallyTypedByDiscriminableUnion.ts, 5, 11)) + +}; + + +function invoke(item: ADT) { +>invoke : Symbol(invoke, Decl(contextuallyTypedByDiscriminableUnion.ts, 6, 2)) +>item : Symbol(item, Decl(contextuallyTypedByDiscriminableUnion.ts, 9, 16)) +>ADT : Symbol(ADT, Decl(contextuallyTypedByDiscriminableUnion.ts, 0, 0)) + + if (item.kind === "a") { +>item.kind : Symbol(kind, Decl(contextuallyTypedByDiscriminableUnion.ts, 0, 12), Decl(contextuallyTypedByDiscriminableUnion.ts, 3, 5)) +>item : Symbol(item, Decl(contextuallyTypedByDiscriminableUnion.ts, 9, 16)) +>kind : Symbol(kind, Decl(contextuallyTypedByDiscriminableUnion.ts, 0, 12), Decl(contextuallyTypedByDiscriminableUnion.ts, 3, 5)) + + item.method(""); +>item.method : Symbol(method, Decl(contextuallyTypedByDiscriminableUnion.ts, 1, 14)) +>item : Symbol(item, Decl(contextuallyTypedByDiscriminableUnion.ts, 9, 16)) +>method : Symbol(method, Decl(contextuallyTypedByDiscriminableUnion.ts, 1, 14)) + } + else { + item.method(42); +>item.method : Symbol(method, Decl(contextuallyTypedByDiscriminableUnion.ts, 4, 14)) +>item : Symbol(item, Decl(contextuallyTypedByDiscriminableUnion.ts, 9, 16)) +>method : Symbol(method, Decl(contextuallyTypedByDiscriminableUnion.ts, 4, 14)) + } +} + +invoke({ +>invoke : Symbol(invoke, Decl(contextuallyTypedByDiscriminableUnion.ts, 6, 2)) + + kind: "a", +>kind : Symbol(kind, Decl(contextuallyTypedByDiscriminableUnion.ts, 18, 8)) + + method(a) { +>method : Symbol(method, Decl(contextuallyTypedByDiscriminableUnion.ts, 19, 14)) +>a : Symbol(a, Decl(contextuallyTypedByDiscriminableUnion.ts, 20, 11)) + + return +a; +>a : Symbol(a, Decl(contextuallyTypedByDiscriminableUnion.ts, 20, 11)) + } +}); + diff --git a/tests/baselines/reference/contextuallyTypedByDiscriminableUnion.types b/tests/baselines/reference/contextuallyTypedByDiscriminableUnion.types new file mode 100644 index 0000000000..d62dc4b5e0 --- /dev/null +++ b/tests/baselines/reference/contextuallyTypedByDiscriminableUnion.types @@ -0,0 +1,70 @@ +=== tests/cases/compiler/contextuallyTypedByDiscriminableUnion.ts === +type ADT = { +>ADT : ADT + + kind: "a", +>kind : "a" + + method(x: string): number; +>method : (x: string) => number +>x : string + +} | { + kind: "b", +>kind : "b" + + method(x: number): string; +>method : (x: number) => string +>x : number + +}; + + +function invoke(item: ADT) { +>invoke : (item: ADT) => void +>item : ADT +>ADT : ADT + + if (item.kind === "a") { +>item.kind === "a" : boolean +>item.kind : "a" | "b" +>item : ADT +>kind : "a" | "b" +>"a" : "a" + + item.method(""); +>item.method("") : number +>item.method : (x: string) => number +>item : { kind: "a"; method(x: string): number; } +>method : (x: string) => number +>"" : "" + } + else { + item.method(42); +>item.method(42) : string +>item.method : (x: number) => string +>item : { kind: "b"; method(x: number): string; } +>method : (x: number) => string +>42 : 42 + } +} + +invoke({ +>invoke({ kind: "a", method(a) { return +a; }}) : void +>invoke : (item: ADT) => void +>{ kind: "a", method(a) { return +a; }} : { kind: "a"; method(a: string): number; } + + kind: "a", +>kind : string +>"a" : "a" + + method(a) { +>method : (a: string) => number +>a : string + + return +a; +>+a : number +>a : string + } +}); + diff --git a/tests/baselines/reference/excessPropertyCheckWithUnions.errors.txt b/tests/baselines/reference/excessPropertyCheckWithUnions.errors.txt index 9fbbb00160..d802733354 100644 --- a/tests/baselines/reference/excessPropertyCheckWithUnions.errors.txt +++ b/tests/baselines/reference/excessPropertyCheckWithUnions.errors.txt @@ -1,6 +1,6 @@ tests/cases/compiler/excessPropertyCheckWithUnions.ts(10,30): error TS2322: Type '{ tag: "T"; a1: string; }' is not assignable to type 'ADT'. Object literal may only specify known properties, and 'a1' does not exist in type '{ tag: "T"; }'. -tests/cases/compiler/excessPropertyCheckWithUnions.ts(11,21): error TS2322: Type '{ tag: "A"; d20: 12; }' is not assignable to type 'ADT'. +tests/cases/compiler/excessPropertyCheckWithUnions.ts(11,21): error TS2322: Type '{ tag: "A"; d20: number; }' is not assignable to type 'ADT'. Object literal may only specify known properties, and 'd20' does not exist in type '{ tag: "A"; a1: string; }'. tests/cases/compiler/excessPropertyCheckWithUnions.ts(12,1): error TS2322: Type '{ tag: "D"; }' is not assignable to type 'ADT'. Type '{ tag: "D"; }' is not assignable to type '{ tag: "D"; d20: 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18 | 19 | 20; }'. @@ -17,9 +17,13 @@ tests/cases/compiler/excessPropertyCheckWithUnions.ts(40,1): error TS2322: Type Type '{ tag: "A"; z: true; }' is not assignable to type '{ tag: "C"; }'. Types of property 'tag' are incompatible. Type '"A"' is not assignable to type '"C"'. +tests/cases/compiler/excessPropertyCheckWithUnions.ts(49,35): error TS2322: Type '{ a: 1; b: 1; first: string; second: string; }' is not assignable to type 'Overlapping'. + Object literal may only specify known properties, and 'second' does not exist in type '{ a: 1; b: 1; first: string; }'. +tests/cases/compiler/excessPropertyCheckWithUnions.ts(50,35): error TS2322: Type '{ a: 1; b: 1; first: string; third: string; }' is not assignable to type 'Overlapping'. + Object literal may only specify known properties, and 'third' does not exist in type '{ a: 1; b: 1; first: string; }'. -==== tests/cases/compiler/excessPropertyCheckWithUnions.ts (7 errors) ==== +==== tests/cases/compiler/excessPropertyCheckWithUnions.ts (9 errors) ==== type ADT = { tag: "A", a1: string @@ -35,7 +39,7 @@ tests/cases/compiler/excessPropertyCheckWithUnions.ts(40,1): error TS2322: Type !!! error TS2322: Object literal may only specify known properties, and 'a1' does not exist in type '{ tag: "T"; }'. wrong = { tag: "A", d20: 12 } ~~~~~~~ -!!! error TS2322: Type '{ tag: "A"; d20: 12; }' is not assignable to type 'ADT'. +!!! error TS2322: Type '{ tag: "A"; d20: number; }' is not assignable to type 'ADT'. !!! error TS2322: Object literal may only specify known properties, and 'd20' does not exist in type '{ tag: "A"; a1: string; }'. wrong = { tag: "D" } ~~~~~ @@ -93,9 +97,15 @@ tests/cases/compiler/excessPropertyCheckWithUnions.ts(40,1): error TS2322: Type | { b: 3, third: string } let over: Overlapping - // these two are not reported because there are two discriminant properties + // these two are still errors despite their doubled up discriminants over = { a: 1, b: 1, first: "ok", second: "error" } + ~~~~~~~~~~~~~~~ +!!! error TS2322: Type '{ a: 1; b: 1; first: string; second: string; }' is not assignable to type 'Overlapping'. +!!! error TS2322: Object literal may only specify known properties, and 'second' does not exist in type '{ a: 1; b: 1; first: string; }'. over = { a: 1, b: 1, first: "ok", third: "error" } + ~~~~~~~~~~~~~~ +!!! error TS2322: Type '{ a: 1; b: 1; first: string; third: string; }' is not assignable to type 'Overlapping'. +!!! error TS2322: Object literal may only specify known properties, and 'third' does not exist in type '{ a: 1; b: 1; first: string; }'. // Freshness disappears after spreading a union declare let t0: { a: any, b: any } | { d: any, e: any } diff --git a/tests/baselines/reference/excessPropertyCheckWithUnions.js b/tests/baselines/reference/excessPropertyCheckWithUnions.js index c6b45123cc..a20983e4b0 100644 --- a/tests/baselines/reference/excessPropertyCheckWithUnions.js +++ b/tests/baselines/reference/excessPropertyCheckWithUnions.js @@ -46,7 +46,7 @@ type Overlapping = | { b: 3, third: string } let over: Overlapping -// these two are not reported because there are two discriminant properties +// these two are still errors despite their doubled up discriminants over = { a: 1, b: 1, first: "ok", second: "error" } over = { a: 1, b: 1, first: "ok", third: "error" } @@ -84,7 +84,7 @@ amb = { tag: "A", y: 12, extra: 12 }; amb = { tag: "A" }; amb = { tag: "A", z: true }; var over; -// these two are not reported because there are two discriminant properties +// these two are still errors despite their doubled up discriminants over = { a: 1, b: 1, first: "ok", second: "error" }; over = { a: 1, b: 1, first: "ok", third: "error" }; var t2 = __assign({}, t1); diff --git a/tests/baselines/reference/excessPropertyCheckWithUnions.symbols b/tests/baselines/reference/excessPropertyCheckWithUnions.symbols index 7778c6bf21..381681de38 100644 --- a/tests/baselines/reference/excessPropertyCheckWithUnions.symbols +++ b/tests/baselines/reference/excessPropertyCheckWithUnions.symbols @@ -127,7 +127,7 @@ let over: Overlapping >over : Symbol(over, Decl(excessPropertyCheckWithUnions.ts, 45, 3)) >Overlapping : Symbol(Overlapping, Decl(excessPropertyCheckWithUnions.ts, 39, 27)) -// these two are not reported because there are two discriminant properties +// these two are still errors despite their doubled up discriminants over = { a: 1, b: 1, first: "ok", second: "error" } >over : Symbol(over, Decl(excessPropertyCheckWithUnions.ts, 45, 3)) >a : Symbol(a, Decl(excessPropertyCheckWithUnions.ts, 48, 8)) diff --git a/tests/baselines/reference/excessPropertyCheckWithUnions.types b/tests/baselines/reference/excessPropertyCheckWithUnions.types index 78f5c025b3..212eccbe2f 100644 --- a/tests/baselines/reference/excessPropertyCheckWithUnions.types +++ b/tests/baselines/reference/excessPropertyCheckWithUnions.types @@ -29,9 +29,9 @@ let wrong: ADT = { tag: "T", a1: "extra" } >"extra" : "extra" wrong = { tag: "A", d20: 12 } ->wrong = { tag: "A", d20: 12 } : { tag: "A"; d20: 12; } +>wrong = { tag: "A", d20: 12 } : { tag: "A"; d20: number; } >wrong : ADT ->{ tag: "A", d20: 12 } : { tag: "A"; d20: 12; } +>{ tag: "A", d20: 12 } : { tag: "A"; d20: number; } >tag : string >"A" : "A" >d20 : number @@ -167,7 +167,7 @@ let over: Overlapping >over : Overlapping >Overlapping : Overlapping -// these two are not reported because there are two discriminant properties +// these two are still errors despite their doubled up discriminants over = { a: 1, b: 1, first: "ok", second: "error" } >over = { a: 1, b: 1, first: "ok", second: "error" } : { a: 1; b: 1; first: string; second: string; } >over : Overlapping diff --git a/tests/cases/compiler/contextuallyTypedByDiscriminableUnion.ts b/tests/cases/compiler/contextuallyTypedByDiscriminableUnion.ts new file mode 100644 index 0000000000..5fbcd2dbbc --- /dev/null +++ b/tests/cases/compiler/contextuallyTypedByDiscriminableUnion.ts @@ -0,0 +1,25 @@ +// @noImplicitAny: true +type ADT = { + kind: "a", + method(x: string): number; +} | { + kind: "b", + method(x: number): string; +}; + + +function invoke(item: ADT) { + if (item.kind === "a") { + item.method(""); + } + else { + item.method(42); + } +} + +invoke({ + kind: "a", + method(a) { + return +a; + } +}); diff --git a/tests/cases/compiler/excessPropertyCheckWithUnions.ts b/tests/cases/compiler/excessPropertyCheckWithUnions.ts index d5a2327380..240af391cf 100644 --- a/tests/cases/compiler/excessPropertyCheckWithUnions.ts +++ b/tests/cases/compiler/excessPropertyCheckWithUnions.ts @@ -46,7 +46,7 @@ type Overlapping = | { b: 3, third: string } let over: Overlapping -// these two are not reported because there are two discriminant properties +// these two are still errors despite their doubled up discriminants over = { a: 1, b: 1, first: "ok", second: "error" } over = { a: 1, b: 1, first: "ok", third: "error" }