From 01f865dee7f36f00ad79877b09661e6b8de08b21 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Sun, 7 Aug 2016 07:48:40 -0700 Subject: [PATCH 1/6] Fix instanceof operator narrowing issues --- src/compiler/checker.ts | 53 +++++++++++++++++++++++++++++++++-------- 1 file changed, 43 insertions(+), 10 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 9583fadba6230..7dbd6c1ab8bdc 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -8084,6 +8084,25 @@ namespace ts { return source.flags & TypeFlags.Union ? !forEach((source).types, t => !contains(types, t)) : contains(types, source); } + function isTypeSubsetOf(source: Type, target: Type) { + return source === target || target.flags & TypeFlags.Union && isTypeSubsetOfUnion(source, target); + } + + function isTypeSubsetOfUnion(source: Type, target: UnionType) { + if (source.flags & TypeFlags.Union) { + for (const t of (source).types) { + if (!containsType(target.types, t)) { + return false; + } + } + return true; + } + if (source.flags & TypeFlags.EnumLiteral && target.flags & TypeFlags.Enum && (source).baseType === target) { + return true; + } + return containsType(target.types, source); + } + function filterType(type: Type, f: (t: Type) => boolean): Type { return type.flags & TypeFlags.Union ? getUnionType(filter((type).types, f)) : @@ -8230,6 +8249,7 @@ namespace ts { function getTypeAtFlowBranchLabel(flow: FlowLabel): FlowType { const antecedentTypes: Type[] = []; + let subtypeReduction = false; let seenIncomplete = false; for (const antecedent of flow.antecedents) { const flowType = getTypeAtFlowNode(antecedent); @@ -8244,11 +8264,17 @@ namespace ts { if (!contains(antecedentTypes, type)) { antecedentTypes.push(type); } + // If an antecedent type is not a subset of the declared type, we need to perform + // subtype reduction. This happens when a "foreign" type is injected into the control + // flow using the instanceof operator or a user defined type predicate. + if (!isTypeSubsetOf(type, declaredType)) { + subtypeReduction = true; + } if (isIncomplete(flowType)) { seenIncomplete = true; } } - return createFlowType(getUnionType(antecedentTypes), seenIncomplete); + return createFlowType(getUnionType(antecedentTypes, subtypeReduction), seenIncomplete); } function getTypeAtFlowLoopLabel(flow: FlowLabel): FlowType { @@ -8274,6 +8300,7 @@ namespace ts { // Add the flow loop junction and reference to the in-process stack and analyze // each antecedent code path. const antecedentTypes: Type[] = []; + let subtypeReduction = false; flowLoopNodes[flowLoopCount] = flow; flowLoopKeys[flowLoopCount] = key; flowLoopTypes[flowLoopCount] = antecedentTypes; @@ -8290,6 +8317,12 @@ namespace ts { if (!contains(antecedentTypes, type)) { antecedentTypes.push(type); } + // If an antecedent type is not a subset of the declared type, we need to perform + // subtype reduction. This happens when a "foreign" type is injected into the control + // flow using the instanceof operator or a user defined type predicate. + if (!isTypeSubsetOf(type, declaredType)) { + subtypeReduction = true; + } // If the type at a particular antecedent path is the declared type there is no // reason to process more antecedents since the only possible outcome is subtypes // that will be removed in the final union type anyway. @@ -8297,7 +8330,7 @@ namespace ts { break; } } - return cache[key] = getUnionType(antecedentTypes); + return cache[key] = getUnionType(antecedentTypes, subtypeReduction); } function isMatchingPropertyAccess(expr: Expression) { @@ -8494,9 +8527,7 @@ namespace ts { function getNarrowedType(type: Type, candidate: Type, assumeTrue: boolean) { if (!assumeTrue) { - return type.flags & TypeFlags.Union ? - getUnionType(filter((type).types, t => !isTypeSubtypeOf(t, candidate))) : - type; + return filterType(type, t => !isTypeSubtypeOf(t, candidate)); } // If the current type is a union type, remove all constituents that aren't assignable to // the candidate type. If one or more constituents remain, return a union of those. @@ -8506,13 +8537,15 @@ namespace ts { return getUnionType(assignableConstituents); } } - // If the candidate type is assignable to the target type, narrow to the candidate type. - // Otherwise, if the current type is assignable to the candidate, keep the current type. - // Otherwise, the types are completely unrelated, so narrow to the empty type. + // If the candidate type is a subtype of the target type, narrow to the candidate type. + // Otherwise, narrow to whichever of the target type or the candidate type that is assignable + // to the other. Otherwise, the types are completely unrelated, so narrow to an intersection + // of the two types. const targetType = type.flags & TypeFlags.TypeParameter ? getApparentType(type) : type; - return isTypeAssignableTo(candidate, targetType) ? candidate : + return isTypeSubtypeOf(candidate, targetType) ? candidate : isTypeAssignableTo(type, candidate) ? type : - getIntersectionType([type, candidate]); + isTypeAssignableTo(candidate, targetType) ? candidate : + getIntersectionType([type, candidate]); } function narrowTypeByTypePredicate(type: Type, callExpression: CallExpression, assumeTrue: boolean): Type { From f50226b481900af204ed9f8d0c29e32a56827c54 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Sun, 7 Aug 2016 07:53:36 -0700 Subject: [PATCH 2/6] Accept new baselines --- .../baselines/reference/controlFlowBinaryOrExpression.symbols | 4 ++-- tests/baselines/reference/controlFlowBinaryOrExpression.types | 2 +- tests/baselines/reference/stringLiteralTypesAsTags01.types | 4 ++-- tests/baselines/reference/stringLiteralTypesAsTags02.types | 4 ++-- tests/baselines/reference/stringLiteralTypesAsTags03.types | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/baselines/reference/controlFlowBinaryOrExpression.symbols b/tests/baselines/reference/controlFlowBinaryOrExpression.symbols index e76cfbf4caba9..5251973005fcd 100644 --- a/tests/baselines/reference/controlFlowBinaryOrExpression.symbols +++ b/tests/baselines/reference/controlFlowBinaryOrExpression.symbols @@ -86,8 +86,8 @@ if (isNodeList(sourceObj) || isHTMLCollection(sourceObj)) { >sourceObj : Symbol(sourceObj, Decl(controlFlowBinaryOrExpression.ts, 23, 3)) sourceObj.length; ->sourceObj.length : Symbol(length, Decl(controlFlowBinaryOrExpression.ts, 10, 27), Decl(controlFlowBinaryOrExpression.ts, 14, 33)) +>sourceObj.length : Symbol(NodeList.length, Decl(controlFlowBinaryOrExpression.ts, 10, 27)) >sourceObj : Symbol(sourceObj, Decl(controlFlowBinaryOrExpression.ts, 23, 3)) ->length : Symbol(length, Decl(controlFlowBinaryOrExpression.ts, 10, 27), Decl(controlFlowBinaryOrExpression.ts, 14, 33)) +>length : Symbol(NodeList.length, Decl(controlFlowBinaryOrExpression.ts, 10, 27)) } diff --git a/tests/baselines/reference/controlFlowBinaryOrExpression.types b/tests/baselines/reference/controlFlowBinaryOrExpression.types index 0bf5ab524b906..d24462316b4da 100644 --- a/tests/baselines/reference/controlFlowBinaryOrExpression.types +++ b/tests/baselines/reference/controlFlowBinaryOrExpression.types @@ -106,7 +106,7 @@ if (isNodeList(sourceObj) || isHTMLCollection(sourceObj)) { sourceObj.length; >sourceObj.length : number ->sourceObj : NodeList | HTMLCollection | ({ a: string; } & HTMLCollection) +>sourceObj : NodeList >length : number } diff --git a/tests/baselines/reference/stringLiteralTypesAsTags01.types b/tests/baselines/reference/stringLiteralTypesAsTags01.types index e00fd18163dd6..e95a7364f9340 100644 --- a/tests/baselines/reference/stringLiteralTypesAsTags01.types +++ b/tests/baselines/reference/stringLiteralTypesAsTags01.types @@ -99,8 +99,8 @@ if (hasKind(x, "A")) { } else { let b = x; ->b : A ->x : A +>b : never +>x : never } if (!hasKind(x, "B")) { diff --git a/tests/baselines/reference/stringLiteralTypesAsTags02.types b/tests/baselines/reference/stringLiteralTypesAsTags02.types index fb1632559ea90..c984b8a78f9ba 100644 --- a/tests/baselines/reference/stringLiteralTypesAsTags02.types +++ b/tests/baselines/reference/stringLiteralTypesAsTags02.types @@ -93,8 +93,8 @@ if (hasKind(x, "A")) { } else { let b = x; ->b : A ->x : A +>b : never +>x : never } if (!hasKind(x, "B")) { diff --git a/tests/baselines/reference/stringLiteralTypesAsTags03.types b/tests/baselines/reference/stringLiteralTypesAsTags03.types index 05be633813b49..fbe71ff07c188 100644 --- a/tests/baselines/reference/stringLiteralTypesAsTags03.types +++ b/tests/baselines/reference/stringLiteralTypesAsTags03.types @@ -96,8 +96,8 @@ if (hasKind(x, "A")) { } else { let b = x; ->b : A ->x : A +>b : never +>x : never } if (!hasKind(x, "B")) { From 67b3fe58fa9b0761da93d9ea6ab2e8019b6a4b22 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Sun, 7 Aug 2016 08:53:36 -0700 Subject: [PATCH 3/6] Add regression test --- .../reference/controlFlowInstanceof.js | 156 +++++++++++++ .../reference/controlFlowInstanceof.symbols | 194 ++++++++++++++++ .../reference/controlFlowInstanceof.types | 217 ++++++++++++++++++ tests/cases/compiler/controlFlowInstanceof.ts | 80 +++++++ 4 files changed, 647 insertions(+) create mode 100644 tests/baselines/reference/controlFlowInstanceof.js create mode 100644 tests/baselines/reference/controlFlowInstanceof.symbols create mode 100644 tests/baselines/reference/controlFlowInstanceof.types create mode 100644 tests/cases/compiler/controlFlowInstanceof.ts diff --git a/tests/baselines/reference/controlFlowInstanceof.js b/tests/baselines/reference/controlFlowInstanceof.js new file mode 100644 index 0000000000000..7c7833927ca04 --- /dev/null +++ b/tests/baselines/reference/controlFlowInstanceof.js @@ -0,0 +1,156 @@ +//// [controlFlowInstanceof.ts] + +// Repros from #10167 + +function f1(s: Set | Set) { + s = new Set(); + s; // Set + if (s instanceof Set) { + s; // Set + } + s; // Set + s.add(42); +} + +function f2(s: Set | Set) { + s = new Set(); + s; // Set + if (s instanceof Promise) { + s; // Set & Promise + } + s; // Set + s.add(42); +} + +function f3(s: Set | Set) { + s; // Set | Set + if (s instanceof Set) { + s; // Set | Set + } + else { + s; // never + } +} + +function f4(s: Set | Set) { + s = new Set(); + s; // Set + if (s instanceof Set) { + s; // Set + } + else { + s; // never + } +} + +// More tests + +class A { a: string } +class B extends A { b: string } +class C extends A { c: string } + +function foo(x: A | undefined) { + x; // A | undefined + if (x instanceof B || x instanceof C) { + x; // B | C + } + x; // A | undefined + if (x instanceof B && x instanceof C) { + x; // B & C + } + x; // A | undefined + if (!x) { + return; + } + x; // A + if (x instanceof B) { + x; // B + if (x instanceof C) { + x; // B & C + } + else { + x; // B + } + x; // B + } + else { + x; // A + } + x; // A +} + +//// [controlFlowInstanceof.js] +// Repros from #10167 +function f1(s) { + s = new Set(); + s; // Set + if (s instanceof Set) { + s; // Set + } + s; // Set + s.add(42); +} +function f2(s) { + s = new Set(); + s; // Set + if (s instanceof Promise) { + s; // Set & Promise + } + s; // Set + s.add(42); +} +function f3(s) { + s; // Set | Set + if (s instanceof Set) { + s; // Set | Set + } + else { + s; // never + } +} +function f4(s) { + s = new Set(); + s; // Set + if (s instanceof Set) { + s; // Set + } + else { + s; // never + } +} +// More tests +class A { +} +class B extends A { +} +class C extends A { +} +function foo(x) { + x; // A | undefined + if (x instanceof B || x instanceof C) { + x; // B | C + } + x; // A | undefined + if (x instanceof B && x instanceof C) { + x; // B & C + } + x; // A | undefined + if (!x) { + return; + } + x; // A + if (x instanceof B) { + x; // B + if (x instanceof C) { + x; // B & C + } + else { + x; // B + } + x; // B + } + else { + x; // A + } + x; // A +} diff --git a/tests/baselines/reference/controlFlowInstanceof.symbols b/tests/baselines/reference/controlFlowInstanceof.symbols new file mode 100644 index 0000000000000..64b567c1548d4 --- /dev/null +++ b/tests/baselines/reference/controlFlowInstanceof.symbols @@ -0,0 +1,194 @@ +=== tests/cases/compiler/controlFlowInstanceof.ts === + +// Repros from #10167 + +function f1(s: Set | Set) { +>f1 : Symbol(f1, Decl(controlFlowInstanceof.ts, 0, 0)) +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 3, 12)) +>Set : Symbol(Set, Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --)) +>Set : Symbol(Set, Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --)) + + s = new Set(); +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 3, 12)) +>Set : Symbol(Set, Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --)) + + s; // Set +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 3, 12)) + + if (s instanceof Set) { +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 3, 12)) +>Set : Symbol(Set, Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --)) + + s; // Set +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 3, 12)) + } + s; // Set +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 3, 12)) + + s.add(42); +>s.add : Symbol(Set.add, Decl(lib.es2015.collection.d.ts, --, --)) +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 3, 12)) +>add : Symbol(Set.add, Decl(lib.es2015.collection.d.ts, --, --)) +} + +function f2(s: Set | Set) { +>f2 : Symbol(f2, Decl(controlFlowInstanceof.ts, 11, 1)) +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 13, 12)) +>Set : Symbol(Set, Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --)) +>Set : Symbol(Set, Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --)) + + s = new Set(); +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 13, 12)) +>Set : Symbol(Set, Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --)) + + s; // Set +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 13, 12)) + + if (s instanceof Promise) { +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 13, 12)) +>Promise : Symbol(Promise, Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2015.promise.d.ts, --, --), Decl(lib.es2015.promise.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --)) + + s; // Set & Promise +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 13, 12)) + } + s; // Set +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 13, 12)) + + s.add(42); +>s.add : Symbol(Set.add, Decl(lib.es2015.collection.d.ts, --, --)) +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 13, 12)) +>add : Symbol(Set.add, Decl(lib.es2015.collection.d.ts, --, --)) +} + +function f3(s: Set | Set) { +>f3 : Symbol(f3, Decl(controlFlowInstanceof.ts, 21, 1)) +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 23, 12)) +>Set : Symbol(Set, Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --)) +>Set : Symbol(Set, Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --)) + + s; // Set | Set +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 23, 12)) + + if (s instanceof Set) { +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 23, 12)) +>Set : Symbol(Set, Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --)) + + s; // Set | Set +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 23, 12)) + } + else { + s; // never +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 23, 12)) + } +} + +function f4(s: Set | Set) { +>f4 : Symbol(f4, Decl(controlFlowInstanceof.ts, 31, 1)) +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 33, 12)) +>Set : Symbol(Set, Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --)) +>Set : Symbol(Set, Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --)) + + s = new Set(); +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 33, 12)) +>Set : Symbol(Set, Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --)) + + s; // Set +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 33, 12)) + + if (s instanceof Set) { +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 33, 12)) +>Set : Symbol(Set, Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --)) + + s; // Set +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 33, 12)) + } + else { + s; // never +>s : Symbol(s, Decl(controlFlowInstanceof.ts, 33, 12)) + } +} + +// More tests + +class A { a: string } +>A : Symbol(A, Decl(controlFlowInstanceof.ts, 42, 1)) +>a : Symbol(A.a, Decl(controlFlowInstanceof.ts, 46, 9)) + +class B extends A { b: string } +>B : Symbol(B, Decl(controlFlowInstanceof.ts, 46, 21)) +>A : Symbol(A, Decl(controlFlowInstanceof.ts, 42, 1)) +>b : Symbol(B.b, Decl(controlFlowInstanceof.ts, 47, 19)) + +class C extends A { c: string } +>C : Symbol(C, Decl(controlFlowInstanceof.ts, 47, 31)) +>A : Symbol(A, Decl(controlFlowInstanceof.ts, 42, 1)) +>c : Symbol(C.c, Decl(controlFlowInstanceof.ts, 48, 19)) + +function foo(x: A | undefined) { +>foo : Symbol(foo, Decl(controlFlowInstanceof.ts, 48, 31)) +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 50, 13)) +>A : Symbol(A, Decl(controlFlowInstanceof.ts, 42, 1)) + + x; // A | undefined +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 50, 13)) + + if (x instanceof B || x instanceof C) { +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 50, 13)) +>B : Symbol(B, Decl(controlFlowInstanceof.ts, 46, 21)) +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 50, 13)) +>C : Symbol(C, Decl(controlFlowInstanceof.ts, 47, 31)) + + x; // B | C +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 50, 13)) + } + x; // A | undefined +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 50, 13)) + + if (x instanceof B && x instanceof C) { +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 50, 13)) +>B : Symbol(B, Decl(controlFlowInstanceof.ts, 46, 21)) +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 50, 13)) +>C : Symbol(C, Decl(controlFlowInstanceof.ts, 47, 31)) + + x; // B & C +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 50, 13)) + } + x; // A | undefined +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 50, 13)) + + if (!x) { +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 50, 13)) + + return; + } + x; // A +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 50, 13)) + + if (x instanceof B) { +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 50, 13)) +>B : Symbol(B, Decl(controlFlowInstanceof.ts, 46, 21)) + + x; // B +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 50, 13)) + + if (x instanceof C) { +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 50, 13)) +>C : Symbol(C, Decl(controlFlowInstanceof.ts, 47, 31)) + + x; // B & C +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 50, 13)) + } + else { + x; // B +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 50, 13)) + } + x; // B +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 50, 13)) + } + else { + x; // A +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 50, 13)) + } + x; // A +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 50, 13)) +} diff --git a/tests/baselines/reference/controlFlowInstanceof.types b/tests/baselines/reference/controlFlowInstanceof.types new file mode 100644 index 0000000000000..362715914f994 --- /dev/null +++ b/tests/baselines/reference/controlFlowInstanceof.types @@ -0,0 +1,217 @@ +=== tests/cases/compiler/controlFlowInstanceof.ts === + +// Repros from #10167 + +function f1(s: Set | Set) { +>f1 : (s: Set | Set) => void +>s : Set | Set +>Set : Set +>Set : Set + + s = new Set(); +>s = new Set() : Set +>s : Set | Set +>new Set() : Set +>Set : SetConstructor + + s; // Set +>s : Set + + if (s instanceof Set) { +>s instanceof Set : boolean +>s : Set +>Set : SetConstructor + + s; // Set +>s : Set + } + s; // Set +>s : Set + + s.add(42); +>s.add(42) : Set +>s.add : (value: number) => Set +>s : Set +>add : (value: number) => Set +>42 : number +} + +function f2(s: Set | Set) { +>f2 : (s: Set | Set) => void +>s : Set | Set +>Set : Set +>Set : Set + + s = new Set(); +>s = new Set() : Set +>s : Set | Set +>new Set() : Set +>Set : SetConstructor + + s; // Set +>s : Set + + if (s instanceof Promise) { +>s instanceof Promise : boolean +>s : Set +>Promise : PromiseConstructor + + s; // Set & Promise +>s : Set & Promise + } + s; // Set +>s : Set + + s.add(42); +>s.add(42) : Set +>s.add : (value: number) => Set +>s : Set +>add : (value: number) => Set +>42 : number +} + +function f3(s: Set | Set) { +>f3 : (s: Set | Set) => void +>s : Set | Set +>Set : Set +>Set : Set + + s; // Set | Set +>s : Set | Set + + if (s instanceof Set) { +>s instanceof Set : boolean +>s : Set | Set +>Set : SetConstructor + + s; // Set | Set +>s : Set | Set + } + else { + s; // never +>s : never + } +} + +function f4(s: Set | Set) { +>f4 : (s: Set | Set) => void +>s : Set | Set +>Set : Set +>Set : Set + + s = new Set(); +>s = new Set() : Set +>s : Set | Set +>new Set() : Set +>Set : SetConstructor + + s; // Set +>s : Set + + if (s instanceof Set) { +>s instanceof Set : boolean +>s : Set +>Set : SetConstructor + + s; // Set +>s : Set + } + else { + s; // never +>s : never + } +} + +// More tests + +class A { a: string } +>A : A +>a : string + +class B extends A { b: string } +>B : B +>A : A +>b : string + +class C extends A { c: string } +>C : C +>A : A +>c : string + +function foo(x: A | undefined) { +>foo : (x: A) => void +>x : A +>A : A + + x; // A | undefined +>x : A + + if (x instanceof B || x instanceof C) { +>x instanceof B || x instanceof C : boolean +>x instanceof B : boolean +>x : A +>B : typeof B +>x instanceof C : boolean +>x : A +>C : typeof C + + x; // B | C +>x : B | C + } + x; // A | undefined +>x : A + + if (x instanceof B && x instanceof C) { +>x instanceof B && x instanceof C : boolean +>x instanceof B : boolean +>x : A +>B : typeof B +>x instanceof C : boolean +>x : B +>C : typeof C + + x; // B & C +>x : B & C + } + x; // A | undefined +>x : A + + if (!x) { +>!x : boolean +>x : A + + return; + } + x; // A +>x : A + + if (x instanceof B) { +>x instanceof B : boolean +>x : A +>B : typeof B + + x; // B +>x : B + + if (x instanceof C) { +>x instanceof C : boolean +>x : B +>C : typeof C + + x; // B & C +>x : B & C + } + else { + x; // B +>x : B + } + x; // B +>x : B + } + else { + x; // A +>x : A + } + x; // A +>x : A +} diff --git a/tests/cases/compiler/controlFlowInstanceof.ts b/tests/cases/compiler/controlFlowInstanceof.ts new file mode 100644 index 0000000000000..229a00236baf2 --- /dev/null +++ b/tests/cases/compiler/controlFlowInstanceof.ts @@ -0,0 +1,80 @@ +// @target: es6 + +// Repros from #10167 + +function f1(s: Set | Set) { + s = new Set(); + s; // Set + if (s instanceof Set) { + s; // Set + } + s; // Set + s.add(42); +} + +function f2(s: Set | Set) { + s = new Set(); + s; // Set + if (s instanceof Promise) { + s; // Set & Promise + } + s; // Set + s.add(42); +} + +function f3(s: Set | Set) { + s; // Set | Set + if (s instanceof Set) { + s; // Set | Set + } + else { + s; // never + } +} + +function f4(s: Set | Set) { + s = new Set(); + s; // Set + if (s instanceof Set) { + s; // Set + } + else { + s; // never + } +} + +// More tests + +class A { a: string } +class B extends A { b: string } +class C extends A { c: string } + +function foo(x: A | undefined) { + x; // A | undefined + if (x instanceof B || x instanceof C) { + x; // B | C + } + x; // A | undefined + if (x instanceof B && x instanceof C) { + x; // B & C + } + x; // A | undefined + if (!x) { + return; + } + x; // A + if (x instanceof B) { + x; // B + if (x instanceof C) { + x; // B & C + } + else { + x; // B + } + x; // B + } + else { + x; // A + } + x; // A +} \ No newline at end of file From a0c5608770b911d357c745f4842e03b408a135fe Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Mon, 8 Aug 2016 09:44:24 -0700 Subject: [PATCH 4/6] Update comment --- src/compiler/checker.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 7dbd6c1ab8bdc..09402b9411926 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -8538,9 +8538,10 @@ namespace ts { } } // If the candidate type is a subtype of the target type, narrow to the candidate type. - // Otherwise, narrow to whichever of the target type or the candidate type that is assignable - // to the other. Otherwise, the types are completely unrelated, so narrow to an intersection - // of the two types. + // Otherwise, if the target type is assignable to the candidate type, keep the target type. + // Otherwise, if the candidate type is assignable to the target type, narrow to the candidate + // type. Otherwise, the types are completely unrelated, so narrow to an intersection of the + // two types. const targetType = type.flags & TypeFlags.TypeParameter ? getApparentType(type) : type; return isTypeSubtypeOf(candidate, targetType) ? candidate : isTypeAssignableTo(type, candidate) ? type : From ce5a3f466d281695d700fbd08dcb92a685340809 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Mon, 8 Aug 2016 09:44:43 -0700 Subject: [PATCH 5/6] Add more tests --- tests/cases/compiler/controlFlowInstanceof.ts | 19 ++++++++++++ .../discriminantsAndTypePredicates.ts | 31 +++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 tests/cases/compiler/discriminantsAndTypePredicates.ts diff --git a/tests/cases/compiler/controlFlowInstanceof.ts b/tests/cases/compiler/controlFlowInstanceof.ts index 229a00236baf2..56f3ff97e4c8e 100644 --- a/tests/cases/compiler/controlFlowInstanceof.ts +++ b/tests/cases/compiler/controlFlowInstanceof.ts @@ -77,4 +77,23 @@ function foo(x: A | undefined) { x; // A } x; // A +} + +// X is neither assignable to Y nor a subtype of Y +// Y is assignable to X, but not a subtype of X + +interface X { + x?: string; +} + +class Y { + y: string; +} + +function goo(x: X) { + x; + if (x instanceof Y) { + x.y; + } + x; } \ No newline at end of file diff --git a/tests/cases/compiler/discriminantsAndTypePredicates.ts b/tests/cases/compiler/discriminantsAndTypePredicates.ts new file mode 100644 index 0000000000000..c21ab7ec8f49a --- /dev/null +++ b/tests/cases/compiler/discriminantsAndTypePredicates.ts @@ -0,0 +1,31 @@ +// Repro from #10145 + +interface A { type: 'A' } +interface B { type: 'B' } + +function isA(x: A | B): x is A { return x.type === 'A'; } +function isB(x: A | B): x is B { return x.type === 'B'; } + +function foo1(x: A | B): any { + x; // A | B + if (isA(x)) { + return x; // A + } + x; // B + if (isB(x)) { + return x; // B + } + x; // never +} + +function foo2(x: A | B): any { + x; // A | B + if (x.type === 'A') { + return x; // A + } + x; // B + if (x.type === 'B') { + return x; // B + } + x; // never +} \ No newline at end of file From ba521de66d8f654784be4f011814a5ca17b413a2 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Mon, 8 Aug 2016 09:45:12 -0700 Subject: [PATCH 6/6] Accept new baselines --- .../reference/controlFlowInstanceof.js | 28 +++++ .../reference/controlFlowInstanceof.symbols | 38 +++++++ .../reference/controlFlowInstanceof.types | 39 +++++++ .../discriminantsAndTypePredicates.js | 59 ++++++++++ .../discriminantsAndTypePredicates.symbols | 94 ++++++++++++++++ .../discriminantsAndTypePredicates.types | 104 ++++++++++++++++++ 6 files changed, 362 insertions(+) create mode 100644 tests/baselines/reference/discriminantsAndTypePredicates.js create mode 100644 tests/baselines/reference/discriminantsAndTypePredicates.symbols create mode 100644 tests/baselines/reference/discriminantsAndTypePredicates.types diff --git a/tests/baselines/reference/controlFlowInstanceof.js b/tests/baselines/reference/controlFlowInstanceof.js index 7c7833927ca04..3f2287b46285a 100644 --- a/tests/baselines/reference/controlFlowInstanceof.js +++ b/tests/baselines/reference/controlFlowInstanceof.js @@ -77,6 +77,25 @@ function foo(x: A | undefined) { x; // A } x; // A +} + +// X is neither assignable to Y nor a subtype of Y +// Y is assignable to X, but not a subtype of X + +interface X { + x?: string; +} + +class Y { + y: string; +} + +function goo(x: X) { + x; + if (x instanceof Y) { + x.y; + } + x; } //// [controlFlowInstanceof.js] @@ -154,3 +173,12 @@ function foo(x) { } x; // A } +class Y { +} +function goo(x) { + x; + if (x instanceof Y) { + x.y; + } + x; +} diff --git a/tests/baselines/reference/controlFlowInstanceof.symbols b/tests/baselines/reference/controlFlowInstanceof.symbols index 64b567c1548d4..3189482799013 100644 --- a/tests/baselines/reference/controlFlowInstanceof.symbols +++ b/tests/baselines/reference/controlFlowInstanceof.symbols @@ -192,3 +192,41 @@ function foo(x: A | undefined) { x; // A >x : Symbol(x, Decl(controlFlowInstanceof.ts, 50, 13)) } + +// X is neither assignable to Y nor a subtype of Y +// Y is assignable to X, but not a subtype of X + +interface X { +>X : Symbol(X, Decl(controlFlowInstanceof.ts, 78, 1)) + + x?: string; +>x : Symbol(X.x, Decl(controlFlowInstanceof.ts, 83, 13)) +} + +class Y { +>Y : Symbol(Y, Decl(controlFlowInstanceof.ts, 85, 1)) + + y: string; +>y : Symbol(Y.y, Decl(controlFlowInstanceof.ts, 87, 9)) +} + +function goo(x: X) { +>goo : Symbol(goo, Decl(controlFlowInstanceof.ts, 89, 1)) +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 91, 13)) +>X : Symbol(X, Decl(controlFlowInstanceof.ts, 78, 1)) + + x; +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 91, 13)) + + if (x instanceof Y) { +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 91, 13)) +>Y : Symbol(Y, Decl(controlFlowInstanceof.ts, 85, 1)) + + x.y; +>x.y : Symbol(Y.y, Decl(controlFlowInstanceof.ts, 87, 9)) +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 91, 13)) +>y : Symbol(Y.y, Decl(controlFlowInstanceof.ts, 87, 9)) + } + x; +>x : Symbol(x, Decl(controlFlowInstanceof.ts, 91, 13)) +} diff --git a/tests/baselines/reference/controlFlowInstanceof.types b/tests/baselines/reference/controlFlowInstanceof.types index 362715914f994..e52d1a25b1d0d 100644 --- a/tests/baselines/reference/controlFlowInstanceof.types +++ b/tests/baselines/reference/controlFlowInstanceof.types @@ -215,3 +215,42 @@ function foo(x: A | undefined) { x; // A >x : A } + +// X is neither assignable to Y nor a subtype of Y +// Y is assignable to X, but not a subtype of X + +interface X { +>X : X + + x?: string; +>x : string +} + +class Y { +>Y : Y + + y: string; +>y : string +} + +function goo(x: X) { +>goo : (x: X) => void +>x : X +>X : X + + x; +>x : X + + if (x instanceof Y) { +>x instanceof Y : boolean +>x : X +>Y : typeof Y + + x.y; +>x.y : string +>x : Y +>y : string + } + x; +>x : X +} diff --git a/tests/baselines/reference/discriminantsAndTypePredicates.js b/tests/baselines/reference/discriminantsAndTypePredicates.js new file mode 100644 index 0000000000000..66cfe056ab711 --- /dev/null +++ b/tests/baselines/reference/discriminantsAndTypePredicates.js @@ -0,0 +1,59 @@ +//// [discriminantsAndTypePredicates.ts] +// Repro from #10145 + +interface A { type: 'A' } +interface B { type: 'B' } + +function isA(x: A | B): x is A { return x.type === 'A'; } +function isB(x: A | B): x is B { return x.type === 'B'; } + +function foo1(x: A | B): any { + x; // A | B + if (isA(x)) { + return x; // A + } + x; // B + if (isB(x)) { + return x; // B + } + x; // never +} + +function foo2(x: A | B): any { + x; // A | B + if (x.type === 'A') { + return x; // A + } + x; // B + if (x.type === 'B') { + return x; // B + } + x; // never +} + +//// [discriminantsAndTypePredicates.js] +// Repro from #10145 +function isA(x) { return x.type === 'A'; } +function isB(x) { return x.type === 'B'; } +function foo1(x) { + x; // A | B + if (isA(x)) { + return x; // A + } + x; // B + if (isB(x)) { + return x; // B + } + x; // never +} +function foo2(x) { + x; // A | B + if (x.type === 'A') { + return x; // A + } + x; // B + if (x.type === 'B') { + return x; // B + } + x; // never +} diff --git a/tests/baselines/reference/discriminantsAndTypePredicates.symbols b/tests/baselines/reference/discriminantsAndTypePredicates.symbols new file mode 100644 index 0000000000000..4f4128239364f --- /dev/null +++ b/tests/baselines/reference/discriminantsAndTypePredicates.symbols @@ -0,0 +1,94 @@ +=== tests/cases/compiler/discriminantsAndTypePredicates.ts === +// Repro from #10145 + +interface A { type: 'A' } +>A : Symbol(A, Decl(discriminantsAndTypePredicates.ts, 0, 0)) +>type : Symbol(A.type, Decl(discriminantsAndTypePredicates.ts, 2, 13)) + +interface B { type: 'B' } +>B : Symbol(B, Decl(discriminantsAndTypePredicates.ts, 2, 25)) +>type : Symbol(B.type, Decl(discriminantsAndTypePredicates.ts, 3, 13)) + +function isA(x: A | B): x is A { return x.type === 'A'; } +>isA : Symbol(isA, Decl(discriminantsAndTypePredicates.ts, 3, 25)) +>x : Symbol(x, Decl(discriminantsAndTypePredicates.ts, 5, 13)) +>A : Symbol(A, Decl(discriminantsAndTypePredicates.ts, 0, 0)) +>B : Symbol(B, Decl(discriminantsAndTypePredicates.ts, 2, 25)) +>x : Symbol(x, Decl(discriminantsAndTypePredicates.ts, 5, 13)) +>A : Symbol(A, Decl(discriminantsAndTypePredicates.ts, 0, 0)) +>x.type : Symbol(type, Decl(discriminantsAndTypePredicates.ts, 2, 13), Decl(discriminantsAndTypePredicates.ts, 3, 13)) +>x : Symbol(x, Decl(discriminantsAndTypePredicates.ts, 5, 13)) +>type : Symbol(type, Decl(discriminantsAndTypePredicates.ts, 2, 13), Decl(discriminantsAndTypePredicates.ts, 3, 13)) + +function isB(x: A | B): x is B { return x.type === 'B'; } +>isB : Symbol(isB, Decl(discriminantsAndTypePredicates.ts, 5, 57)) +>x : Symbol(x, Decl(discriminantsAndTypePredicates.ts, 6, 13)) +>A : Symbol(A, Decl(discriminantsAndTypePredicates.ts, 0, 0)) +>B : Symbol(B, Decl(discriminantsAndTypePredicates.ts, 2, 25)) +>x : Symbol(x, Decl(discriminantsAndTypePredicates.ts, 6, 13)) +>B : Symbol(B, Decl(discriminantsAndTypePredicates.ts, 2, 25)) +>x.type : Symbol(type, Decl(discriminantsAndTypePredicates.ts, 2, 13), Decl(discriminantsAndTypePredicates.ts, 3, 13)) +>x : Symbol(x, Decl(discriminantsAndTypePredicates.ts, 6, 13)) +>type : Symbol(type, Decl(discriminantsAndTypePredicates.ts, 2, 13), Decl(discriminantsAndTypePredicates.ts, 3, 13)) + +function foo1(x: A | B): any { +>foo1 : Symbol(foo1, Decl(discriminantsAndTypePredicates.ts, 6, 57)) +>x : Symbol(x, Decl(discriminantsAndTypePredicates.ts, 8, 14)) +>A : Symbol(A, Decl(discriminantsAndTypePredicates.ts, 0, 0)) +>B : Symbol(B, Decl(discriminantsAndTypePredicates.ts, 2, 25)) + + x; // A | B +>x : Symbol(x, Decl(discriminantsAndTypePredicates.ts, 8, 14)) + + if (isA(x)) { +>isA : Symbol(isA, Decl(discriminantsAndTypePredicates.ts, 3, 25)) +>x : Symbol(x, Decl(discriminantsAndTypePredicates.ts, 8, 14)) + + return x; // A +>x : Symbol(x, Decl(discriminantsAndTypePredicates.ts, 8, 14)) + } + x; // B +>x : Symbol(x, Decl(discriminantsAndTypePredicates.ts, 8, 14)) + + if (isB(x)) { +>isB : Symbol(isB, Decl(discriminantsAndTypePredicates.ts, 5, 57)) +>x : Symbol(x, Decl(discriminantsAndTypePredicates.ts, 8, 14)) + + return x; // B +>x : Symbol(x, Decl(discriminantsAndTypePredicates.ts, 8, 14)) + } + x; // never +>x : Symbol(x, Decl(discriminantsAndTypePredicates.ts, 8, 14)) +} + +function foo2(x: A | B): any { +>foo2 : Symbol(foo2, Decl(discriminantsAndTypePredicates.ts, 18, 1)) +>x : Symbol(x, Decl(discriminantsAndTypePredicates.ts, 20, 14)) +>A : Symbol(A, Decl(discriminantsAndTypePredicates.ts, 0, 0)) +>B : Symbol(B, Decl(discriminantsAndTypePredicates.ts, 2, 25)) + + x; // A | B +>x : Symbol(x, Decl(discriminantsAndTypePredicates.ts, 20, 14)) + + if (x.type === 'A') { +>x.type : Symbol(type, Decl(discriminantsAndTypePredicates.ts, 2, 13), Decl(discriminantsAndTypePredicates.ts, 3, 13)) +>x : Symbol(x, Decl(discriminantsAndTypePredicates.ts, 20, 14)) +>type : Symbol(type, Decl(discriminantsAndTypePredicates.ts, 2, 13), Decl(discriminantsAndTypePredicates.ts, 3, 13)) + + return x; // A +>x : Symbol(x, Decl(discriminantsAndTypePredicates.ts, 20, 14)) + } + x; // B +>x : Symbol(x, Decl(discriminantsAndTypePredicates.ts, 20, 14)) + + if (x.type === 'B') { +>x.type : Symbol(B.type, Decl(discriminantsAndTypePredicates.ts, 3, 13)) +>x : Symbol(x, Decl(discriminantsAndTypePredicates.ts, 20, 14)) +>type : Symbol(B.type, Decl(discriminantsAndTypePredicates.ts, 3, 13)) + + return x; // B +>x : Symbol(x, Decl(discriminantsAndTypePredicates.ts, 20, 14)) + } + x; // never +>x : Symbol(x, Decl(discriminantsAndTypePredicates.ts, 20, 14)) +} diff --git a/tests/baselines/reference/discriminantsAndTypePredicates.types b/tests/baselines/reference/discriminantsAndTypePredicates.types new file mode 100644 index 0000000000000..f7675b1475601 --- /dev/null +++ b/tests/baselines/reference/discriminantsAndTypePredicates.types @@ -0,0 +1,104 @@ +=== tests/cases/compiler/discriminantsAndTypePredicates.ts === +// Repro from #10145 + +interface A { type: 'A' } +>A : A +>type : "A" + +interface B { type: 'B' } +>B : B +>type : "B" + +function isA(x: A | B): x is A { return x.type === 'A'; } +>isA : (x: A | B) => x is A +>x : A | B +>A : A +>B : B +>x : any +>A : A +>x.type === 'A' : boolean +>x.type : "A" | "B" +>x : A | B +>type : "A" | "B" +>'A' : "A" + +function isB(x: A | B): x is B { return x.type === 'B'; } +>isB : (x: A | B) => x is B +>x : A | B +>A : A +>B : B +>x : any +>B : B +>x.type === 'B' : boolean +>x.type : "A" | "B" +>x : A | B +>type : "A" | "B" +>'B' : "B" + +function foo1(x: A | B): any { +>foo1 : (x: A | B) => any +>x : A | B +>A : A +>B : B + + x; // A | B +>x : A | B + + if (isA(x)) { +>isA(x) : boolean +>isA : (x: A | B) => x is A +>x : A | B + + return x; // A +>x : A + } + x; // B +>x : B + + if (isB(x)) { +>isB(x) : boolean +>isB : (x: A | B) => x is B +>x : B + + return x; // B +>x : B + } + x; // never +>x : never +} + +function foo2(x: A | B): any { +>foo2 : (x: A | B) => any +>x : A | B +>A : A +>B : B + + x; // A | B +>x : A | B + + if (x.type === 'A') { +>x.type === 'A' : boolean +>x.type : "A" | "B" +>x : A | B +>type : "A" | "B" +>'A' : "A" + + return x; // A +>x : A + } + x; // B +>x : B + + if (x.type === 'B') { +>x.type === 'B' : boolean +>x.type : "B" +>x : B +>type : "B" +>'B' : "B" + + return x; // B +>x : B + } + x; // never +>x : never +}