Merge pull request #12251 from Microsoft/fixTypePredicateStructuralMatch

ahejlsberg · web-flow · commit 73ada7a78db1 · 2016-11-15T13:20:35.000-08:00
Fix type predicates with structurally identical types
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -9741,20 +9741,20 @@ namespace ts {
                 }
 
                 if (targetType) {
-                    return getNarrowedType(type, targetType, assumeTrue);
+                    return getNarrowedType(type, targetType, assumeTrue, isTypeInstanceOf);
                 }
 
                 return type;
             }
 
-            function getNarrowedType(type: Type, candidate: Type, assumeTrue: boolean) {
+            function getNarrowedType(type: Type, candidate: Type, assumeTrue: boolean, isRelated: (source: Type, target: Type) => boolean) {
                 if (!assumeTrue) {
-                    return filterType(type, t => !isTypeInstanceOf(t, candidate));
+                    return filterType(type, t => !isRelated(t, candidate));
                 }
                 // If the current type is a union type, remove all constituents that couldn't be instances of
                 // the candidate type. If one or more constituents remain, return a union of those.
                 if (type.flags & TypeFlags.Union) {
-                    const assignableType = filterType(type, t => isTypeInstanceOf(t, candidate));
+                    const assignableType = filterType(type, t => isRelated(t, candidate));
                     if (!(assignableType.flags & TypeFlags.Never)) {
                         return assignableType;
                     }
@@ -9790,7 +9790,7 @@ namespace ts {
                     const predicateArgument = callExpression.arguments[predicate.parameterIndex];
                     if (predicateArgument) {
                         if (isMatchingReference(reference, predicateArgument)) {
-                            return getNarrowedType(type, predicate.type, assumeTrue);
+                            return getNarrowedType(type, predicate.type, assumeTrue, isTypeSubtypeOf);
                         }
                         if (containsMatchingReference(reference, predicateArgument)) {
                             return declaredType;
@@ -9803,7 +9803,7 @@ namespace ts {
                         const accessExpression = invokedExpression as ElementAccessExpression | PropertyAccessExpression;
                         const possibleReference = skipParentheses(accessExpression.expression);
                         if (isMatchingReference(reference, possibleReference)) {
-                            return getNarrowedType(type, predicate.type, assumeTrue);
+                            return getNarrowedType(type, predicate.type, assumeTrue, isTypeSubtypeOf);
                         }
                         if (containsMatchingReference(reference, possibleReference)) {
                             return declaredType;
diff --git a/tests/baselines/reference/controlFlowBinaryOrExpression.symbols b/tests/baselines/reference/controlFlowBinaryOrExpression.symbols
@@ -64,19 +64,19 @@ if (isNodeList(sourceObj)) {
 >sourceObj : Symbol(sourceObj, Decl(controlFlowBinaryOrExpression.ts, 23, 3))
 
     sourceObj.length;
->sourceObj.length : Symbol(NodeList.length, Decl(controlFlowBinaryOrExpression.ts, 10, 27))
+>sourceObj.length : Symbol(length, Decl(controlFlowBinaryOrExpression.ts, 10, 27), Decl(controlFlowBinaryOrExpression.ts, 14, 33))
 >sourceObj : Symbol(sourceObj, Decl(controlFlowBinaryOrExpression.ts, 23, 3))
->length : Symbol(NodeList.length, Decl(controlFlowBinaryOrExpression.ts, 10, 27))
+>length : Symbol(length, Decl(controlFlowBinaryOrExpression.ts, 10, 27), Decl(controlFlowBinaryOrExpression.ts, 14, 33))
 }
 
 if (isHTMLCollection(sourceObj)) {
 >isHTMLCollection : Symbol(isHTMLCollection, Decl(controlFlowBinaryOrExpression.ts, 18, 67))
 >sourceObj : Symbol(sourceObj, Decl(controlFlowBinaryOrExpression.ts, 23, 3))
 
     sourceObj.length;
->sourceObj.length : Symbol(HTMLCollection.length, Decl(controlFlowBinaryOrExpression.ts, 14, 33))
+>sourceObj.length : Symbol(length, Decl(controlFlowBinaryOrExpression.ts, 10, 27), Decl(controlFlowBinaryOrExpression.ts, 14, 33))
 >sourceObj : Symbol(sourceObj, Decl(controlFlowBinaryOrExpression.ts, 23, 3))
->length : Symbol(HTMLCollection.length, Decl(controlFlowBinaryOrExpression.ts, 14, 33))
+>length : Symbol(length, Decl(controlFlowBinaryOrExpression.ts, 10, 27), Decl(controlFlowBinaryOrExpression.ts, 14, 33))
 }
 
 if (isNodeList(sourceObj) || isHTMLCollection(sourceObj)) {
@@ -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
@@ -80,7 +80,7 @@ if (isNodeList(sourceObj)) {
 
     sourceObj.length;
 >sourceObj.length : number
->sourceObj : NodeList
+>sourceObj : NodeList | HTMLCollection
 >length : number
 }
 
@@ -91,7 +91,7 @@ if (isHTMLCollection(sourceObj)) {
 
     sourceObj.length;
 >sourceObj.length : number
->sourceObj : HTMLCollection
+>sourceObj : NodeList | HTMLCollection
 >length : number
 }
 
@@ -102,11 +102,11 @@ if (isNodeList(sourceObj) || isHTMLCollection(sourceObj)) {
 >sourceObj : NodeList | HTMLCollection | { a: string; }
 >isHTMLCollection(sourceObj) : boolean
 >isHTMLCollection : (sourceObj: any) => sourceObj is HTMLCollection
->sourceObj : HTMLCollection | { a: string; }
+>sourceObj : { a: string; }
 
     sourceObj.length;
 >sourceObj.length : number
->sourceObj : NodeList | HTMLCollection
+>sourceObj : NodeList
 >length : number
 }
 
diff --git a/tests/baselines/reference/instanceofWithStructurallyIdenticalTypes.errors.txt b/tests/baselines/reference/instanceofWithStructurallyIdenticalTypes.errors.txt
@@ -0,0 +1,76 @@
+tests/cases/compiler/instanceofWithStructurallyIdenticalTypes.ts(32,18): error TS2339: Property 'item' does not exist on type 'never'.
+
+
+==== tests/cases/compiler/instanceofWithStructurallyIdenticalTypes.ts (1 errors) ====
+    // Repro from #7271
+    
+    class C1 { item: string }
+    class C2 { item: string[] }
+    class C3 { item: string }
+    
+    function foo1(x: C1 | C2 | C3): string {
+        if (x instanceof C1) {
+            return x.item;
+        }
+        else if (x instanceof C2) {
+            return x.item[0];
+        }
+        else if (x instanceof C3) {
+            return x.item;
+        }
+        return "error";
+    }
+    
+    function isC1(c: C1 | C2 | C3): c is C1 { return c instanceof C1 }
+    function isC2(c: C1 | C2 | C3): c is C2 { return c instanceof C2 }
+    function isC3(c: C1 | C2 | C3): c is C3 { return c instanceof C3 }
+    
+    function foo2(x: C1 | C2 | C3): string {
+        if (isC1(x)) {
+            return x.item;
+        }
+        else if (isC2(x)) {
+            return x.item[0];
+        }
+        else if (isC3(x)) {
+            return x.item;
+                     ~~~~
+!!! error TS2339: Property 'item' does not exist on type 'never'.
+        }
+        return "error";
+    }
+    
+    // More tests
+    
+    class A { a: string }
+    class A1 extends A { }
+    class A2 { a: string }
+    class B extends A { b: string }
+    
+    function goo(x: A) {
+        if (x instanceof A) {
+            x;  // A
+        }
+        else {
+            x;  // never
+        }
+        if (x instanceof A1) {
+            x;  // A1
+        }
+        else {
+            x;  // A
+        }
+        if (x instanceof A2) {
+            x;  // A2
+        }
+        else {
+            x;  // A
+        }
+        if (x instanceof B) {
+            x;  // B
+        }
+        else {
+            x;  // A
+        }
+    }
+    
diff --git a/tests/baselines/reference/typePredicateStructuralMatch.js b/tests/baselines/reference/typePredicateStructuralMatch.js
@@ -0,0 +1,46 @@
+//// [typePredicateStructuralMatch.ts]
+// Repro from #12235
+
+getResults1([]);
+getResults1({data: []});
+
+getResults2([]);
+getResults2({data: []});
+
+type Result = { value: string };
+type Results = Result[];
+
+function isResponseInData<T>(value: T | { data: T}): value is { data: T } {
+    return value.hasOwnProperty('data');
+}
+
+function getResults1(value: Results | { data: Results }): Results {
+    return isResponseInData(value) ? value.data : value;
+}
+
+function isPlainResponse<T>(value: T | { data: T}): value is T {
+    return !value.hasOwnProperty('data');
+}
+
+function getResults2(value: Results | { data: Results }): Results {
+    return isPlainResponse(value) ? value : value.data;
+}
+
+//// [typePredicateStructuralMatch.js]
+// Repro from #12235
+getResults1([]);
+getResults1({ data: [] });
+getResults2([]);
+getResults2({ data: [] });
+function isResponseInData(value) {
+    return value.hasOwnProperty('data');
+}
+function getResults1(value) {
+    return isResponseInData(value) ? value.data : value;
+}
+function isPlainResponse(value) {
+    return !value.hasOwnProperty('data');
+}
+function getResults2(value) {
+    return isPlainResponse(value) ? value : value.data;
+}
diff --git a/tests/baselines/reference/typePredicateStructuralMatch.symbols b/tests/baselines/reference/typePredicateStructuralMatch.symbols
@@ -0,0 +1,91 @@
+=== tests/cases/compiler/typePredicateStructuralMatch.ts ===
+// Repro from #12235
+
+getResults1([]);
+>getResults1 : Symbol(getResults1, Decl(typePredicateStructuralMatch.ts, 13, 1))
+
+getResults1({data: []});
+>getResults1 : Symbol(getResults1, Decl(typePredicateStructuralMatch.ts, 13, 1))
+>data : Symbol(data, Decl(typePredicateStructuralMatch.ts, 3, 13))
+
+getResults2([]);
+>getResults2 : Symbol(getResults2, Decl(typePredicateStructuralMatch.ts, 21, 1))
+
+getResults2({data: []});
+>getResults2 : Symbol(getResults2, Decl(typePredicateStructuralMatch.ts, 21, 1))
+>data : Symbol(data, Decl(typePredicateStructuralMatch.ts, 6, 13))
+
+type Result = { value: string };
+>Result : Symbol(Result, Decl(typePredicateStructuralMatch.ts, 6, 24))
+>value : Symbol(value, Decl(typePredicateStructuralMatch.ts, 8, 15))
+
+type Results = Result[];
+>Results : Symbol(Results, Decl(typePredicateStructuralMatch.ts, 8, 32))
+>Result : Symbol(Result, Decl(typePredicateStructuralMatch.ts, 6, 24))
+
+function isResponseInData<T>(value: T | { data: T}): value is { data: T } {
+>isResponseInData : Symbol(isResponseInData, Decl(typePredicateStructuralMatch.ts, 9, 24))
+>T : Symbol(T, Decl(typePredicateStructuralMatch.ts, 11, 26))
+>value : Symbol(value, Decl(typePredicateStructuralMatch.ts, 11, 29))
+>T : Symbol(T, Decl(typePredicateStructuralMatch.ts, 11, 26))
+>data : Symbol(data, Decl(typePredicateStructuralMatch.ts, 11, 41))
+>T : Symbol(T, Decl(typePredicateStructuralMatch.ts, 11, 26))
+>value : Symbol(value, Decl(typePredicateStructuralMatch.ts, 11, 29))
+>data : Symbol(data, Decl(typePredicateStructuralMatch.ts, 11, 63))
+>T : Symbol(T, Decl(typePredicateStructuralMatch.ts, 11, 26))
+
+    return value.hasOwnProperty('data');
+>value.hasOwnProperty : Symbol(Object.hasOwnProperty, Decl(lib.d.ts, --, --))
+>value : Symbol(value, Decl(typePredicateStructuralMatch.ts, 11, 29))
+>hasOwnProperty : Symbol(Object.hasOwnProperty, Decl(lib.d.ts, --, --))
+}
+
+function getResults1(value: Results | { data: Results }): Results {
+>getResults1 : Symbol(getResults1, Decl(typePredicateStructuralMatch.ts, 13, 1))
+>value : Symbol(value, Decl(typePredicateStructuralMatch.ts, 15, 21))
+>Results : Symbol(Results, Decl(typePredicateStructuralMatch.ts, 8, 32))
+>data : Symbol(data, Decl(typePredicateStructuralMatch.ts, 15, 39))
+>Results : Symbol(Results, Decl(typePredicateStructuralMatch.ts, 8, 32))
+>Results : Symbol(Results, Decl(typePredicateStructuralMatch.ts, 8, 32))
+
+    return isResponseInData(value) ? value.data : value;
+>isResponseInData : Symbol(isResponseInData, Decl(typePredicateStructuralMatch.ts, 9, 24))
+>value : Symbol(value, Decl(typePredicateStructuralMatch.ts, 15, 21))
+>value.data : Symbol(data, Decl(typePredicateStructuralMatch.ts, 15, 39))
+>value : Symbol(value, Decl(typePredicateStructuralMatch.ts, 15, 21))
+>data : Symbol(data, Decl(typePredicateStructuralMatch.ts, 15, 39))
+>value : Symbol(value, Decl(typePredicateStructuralMatch.ts, 15, 21))
+}
+
+function isPlainResponse<T>(value: T | { data: T}): value is T {
+>isPlainResponse : Symbol(isPlainResponse, Decl(typePredicateStructuralMatch.ts, 17, 1))
+>T : Symbol(T, Decl(typePredicateStructuralMatch.ts, 19, 25))
+>value : Symbol(value, Decl(typePredicateStructuralMatch.ts, 19, 28))
+>T : Symbol(T, Decl(typePredicateStructuralMatch.ts, 19, 25))
+>data : Symbol(data, Decl(typePredicateStructuralMatch.ts, 19, 40))
+>T : Symbol(T, Decl(typePredicateStructuralMatch.ts, 19, 25))
+>value : Symbol(value, Decl(typePredicateStructuralMatch.ts, 19, 28))
+>T : Symbol(T, Decl(typePredicateStructuralMatch.ts, 19, 25))
+
+    return !value.hasOwnProperty('data');
+>value.hasOwnProperty : Symbol(Object.hasOwnProperty, Decl(lib.d.ts, --, --))
+>value : Symbol(value, Decl(typePredicateStructuralMatch.ts, 19, 28))
+>hasOwnProperty : Symbol(Object.hasOwnProperty, Decl(lib.d.ts, --, --))
+}
+
+function getResults2(value: Results | { data: Results }): Results {
+>getResults2 : Symbol(getResults2, Decl(typePredicateStructuralMatch.ts, 21, 1))
+>value : Symbol(value, Decl(typePredicateStructuralMatch.ts, 23, 21))
+>Results : Symbol(Results, Decl(typePredicateStructuralMatch.ts, 8, 32))
+>data : Symbol(data, Decl(typePredicateStructuralMatch.ts, 23, 39))
+>Results : Symbol(Results, Decl(typePredicateStructuralMatch.ts, 8, 32))
+>Results : Symbol(Results, Decl(typePredicateStructuralMatch.ts, 8, 32))
+
+    return isPlainResponse(value) ? value : value.data;
+>isPlainResponse : Symbol(isPlainResponse, Decl(typePredicateStructuralMatch.ts, 17, 1))
+>value : Symbol(value, Decl(typePredicateStructuralMatch.ts, 23, 21))
+>value : Symbol(value, Decl(typePredicateStructuralMatch.ts, 23, 21))
+>value.data : Symbol(data, Decl(typePredicateStructuralMatch.ts, 23, 39))
+>value : Symbol(value, Decl(typePredicateStructuralMatch.ts, 23, 21))
+>data : Symbol(data, Decl(typePredicateStructuralMatch.ts, 23, 39))
+}
diff --git a/tests/baselines/reference/typePredicateStructuralMatch.types b/tests/baselines/reference/typePredicateStructuralMatch.types
diff --git a/tests/cases/compiler/typePredicateStructuralMatch.ts b/tests/cases/compiler/typePredicateStructuralMatch.ts

Original file line number	Diff line number	Diff line change
`@@ -9741,20 +9741,20 @@ namespace ts {`
`9741`	`9741`	`}`
`9742`	`9742`
`9743`	`9743`	`if (targetType) {`
`9744`		`- return getNarrowedType(type, targetType, assumeTrue);`
	`9744`	`+ return getNarrowedType(type, targetType, assumeTrue, isTypeInstanceOf);`
`9745`	`9745`	`}`
`9746`	`9746`
`9747`	`9747`	`return type;`
`9748`	`9748`	`}`
`9749`	`9749`
`9750`		`- function getNarrowedType(type: Type, candidate: Type, assumeTrue: boolean) {`
	`9750`	`+ function getNarrowedType(type: Type, candidate: Type, assumeTrue: boolean, isRelated: (source: Type, target: Type) => boolean) {`
`9751`	`9751`	`if (!assumeTrue) {`
`9752`		`- return filterType(type, t => !isTypeInstanceOf(t, candidate));`
	`9752`	`+ return filterType(type, t => !isRelated(t, candidate));`
`9753`	`9753`	`}`
`9754`	`9754`	`// If the current type is a union type, remove all constituents that couldn't be instances of`
`9755`	`9755`	`// the candidate type. If one or more constituents remain, return a union of those.`
`9756`	`9756`	`if (type.flags & TypeFlags.Union) {`
`9757`		`- const assignableType = filterType(type, t => isTypeInstanceOf(t, candidate));`
	`9757`	`+ const assignableType = filterType(type, t => isRelated(t, candidate));`
`9758`	`9758`	`if (!(assignableType.flags & TypeFlags.Never)) {`
`9759`	`9759`	`return assignableType;`
`9760`	`9760`	`}`
`@@ -9790,7 +9790,7 @@ namespace ts {`
`9790`	`9790`	`const predicateArgument = callExpression.arguments[predicate.parameterIndex];`
`9791`	`9791`	`if (predicateArgument) {`
`9792`	`9792`	`if (isMatchingReference(reference, predicateArgument)) {`
`9793`		`- return getNarrowedType(type, predicate.type, assumeTrue);`
	`9793`	`+ return getNarrowedType(type, predicate.type, assumeTrue, isTypeSubtypeOf);`
`9794`	`9794`	`}`
`9795`	`9795`	`if (containsMatchingReference(reference, predicateArgument)) {`
`9796`	`9796`	`return declaredType;`
`@@ -9803,7 +9803,7 @@ namespace ts {`
`9803`	`9803`	`const accessExpression = invokedExpression as ElementAccessExpression \| PropertyAccessExpression;`
`9804`	`9804`	`const possibleReference = skipParentheses(accessExpression.expression);`
`9805`	`9805`	`if (isMatchingReference(reference, possibleReference)) {`
`9806`		`- return getNarrowedType(type, predicate.type, assumeTrue);`
	`9806`	`+ return getNarrowedType(type, predicate.type, assumeTrue, isTypeSubtypeOf);`
`9807`	`9807`	`}`
`9808`	`9808`	`if (containsMatchingReference(reference, possibleReference)) {`
`9809`	`9809`	`return declaredType;`