Skip to content

Commit 8ea90ab

Browse files
ahejlsbergahejlsberg
authored
Merge pull request #10194 from Microsoft/fixInstanceofNarrowing
Fix instanceof narrowing
2 parents f6a850b + ba521de commit 8ea90ab

14 files changed

+1112
-19
lines changed

‎src/compiler/checker.ts‎

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8126,6 +8126,25 @@ namespace ts {
81268126
return source.flags & TypeFlags.Union ? !forEach((<UnionType>source).types, t => !contains(types, t)) : contains(types, source);
81278127
}
81288128

8129+
function isTypeSubsetOf(source: Type, target: Type) {
8130+
return source === target || target.flags & TypeFlags.Union && isTypeSubsetOfUnion(source, <UnionType>target);
8131+
}
8132+
8133+
function isTypeSubsetOfUnion(source: Type, target: UnionType) {
8134+
if (source.flags & TypeFlags.Union) {
8135+
for (const t of (<UnionType>source).types) {
8136+
if (!containsType(target.types, t)) {
8137+
return false;
8138+
}
8139+
}
8140+
return true;
8141+
}
8142+
if (source.flags & TypeFlags.EnumLiteral && target.flags & TypeFlags.Enum && (<EnumLiteralType>source).baseType === target) {
8143+
return true;
8144+
}
8145+
return containsType(target.types, source);
8146+
}
8147+
81298148
function filterType(type: Type, f: (t: Type) => boolean): Type {
81308149
return type.flags & TypeFlags.Union ?
81318150
getUnionType(filter((<UnionType>type).types, f)) :
@@ -8272,6 +8291,7 @@ namespace ts {
82728291

82738292
function getTypeAtFlowBranchLabel(flow: FlowLabel): FlowType {
82748293
const antecedentTypes: Type[] = [];
8294+
let subtypeReduction = false;
82758295
let seenIncomplete = false;
82768296
for (const antecedent of flow.antecedents) {
82778297
const flowType = getTypeAtFlowNode(antecedent);
@@ -8286,11 +8306,17 @@ namespace ts {
82868306
if (!contains(antecedentTypes, type)) {
82878307
antecedentTypes.push(type);
82888308
}
8309+
// If an antecedent type is not a subset of the declared type, we need to perform
8310+
// subtype reduction. This happens when a "foreign" type is injected into the control
8311+
// flow using the instanceof operator or a user defined type predicate.
8312+
if (!isTypeSubsetOf(type, declaredType)) {
8313+
subtypeReduction = true;
8314+
}
82898315
if (isIncomplete(flowType)) {
82908316
seenIncomplete = true;
82918317
}
82928318
}
8293-
return createFlowType(getUnionType(antecedentTypes), seenIncomplete);
8319+
return createFlowType(getUnionType(antecedentTypes, subtypeReduction), seenIncomplete);
82948320
}
82958321

82968322
function getTypeAtFlowLoopLabel(flow: FlowLabel): FlowType {
@@ -8316,6 +8342,7 @@ namespace ts {
83168342
// Add the flow loop junction and reference to the in-process stack and analyze
83178343
// each antecedent code path.
83188344
const antecedentTypes: Type[] = [];
8345+
let subtypeReduction = false;
83198346
flowLoopNodes[flowLoopCount] = flow;
83208347
flowLoopKeys[flowLoopCount] = key;
83218348
flowLoopTypes[flowLoopCount] = antecedentTypes;
@@ -8332,14 +8359,20 @@ namespace ts {
83328359
if (!contains(antecedentTypes, type)) {
83338360
antecedentTypes.push(type);
83348361
}
8362+
// If an antecedent type is not a subset of the declared type, we need to perform
8363+
// subtype reduction. This happens when a "foreign" type is injected into the control
8364+
// flow using the instanceof operator or a user defined type predicate.
8365+
if (!isTypeSubsetOf(type, declaredType)) {
8366+
subtypeReduction = true;
8367+
}
83358368
// If the type at a particular antecedent path is the declared type there is no
83368369
// reason to process more antecedents since the only possible outcome is subtypes
83378370
// that will be removed in the final union type anyway.
83388371
if (type === declaredType) {
83398372
break;
83408373
}
83418374
}
8342-
return cache[key] = getUnionType(antecedentTypes);
8375+
return cache[key] = getUnionType(antecedentTypes, subtypeReduction);
83438376
}
83448377

83458378
function isMatchingReferenceDiscriminant(expr: Expression) {
@@ -8537,9 +8570,7 @@ namespace ts {
85378570

85388571
function getNarrowedType(type: Type, candidate: Type, assumeTrue: boolean) {
85398572
if (!assumeTrue) {
8540-
return type.flags & TypeFlags.Union ?
8541-
getUnionType(filter((<UnionType>type).types, t => !isTypeSubtypeOf(t, candidate))) :
8542-
type;
8573+
return filterType(type, t => !isTypeSubtypeOf(t, candidate));
85438574
}
85448575
// If the current type is a union type, remove all constituents that aren't assignable to
85458576
// the candidate type. If one or more constituents remain, return a union of those.
@@ -8549,13 +8580,16 @@ namespace ts {
85498580
return getUnionType(assignableConstituents);
85508581
}
85518582
}
8552-
// If the candidate type is assignable to the target type, narrow to the candidate type.
8553-
// Otherwise, if the current type is assignable to the candidate, keep the current type.
8554-
// Otherwise, the types are completely unrelated, so narrow to the empty type.
8583+
// If the candidate type is a subtype of the target type, narrow to the candidate type.
8584+
// Otherwise, if the target type is assignable to the candidate type, keep the target type.
8585+
// Otherwise, if the candidate type is assignable to the target type, narrow to the candidate
8586+
// type. Otherwise, the types are completely unrelated, so narrow to an intersection of the
8587+
// two types.
85558588
const targetType = type.flags & TypeFlags.TypeParameter ? getApparentType(type) : type;
8556-
return isTypeAssignableTo(candidate, targetType) ? candidate :
8589+
return isTypeSubtypeOf(candidate, targetType) ? candidate :
85578590
isTypeAssignableTo(type, candidate) ? type :
8558-
getIntersectionType([type, candidate]);
8591+
isTypeAssignableTo(candidate, targetType) ? candidate :
8592+
getIntersectionType([type, candidate]);
85598593
}
85608594

85618595
function narrowTypeByTypePredicate(type: Type, callExpression: CallExpression, assumeTrue: boolean): Type {

‎tests/baselines/reference/controlFlowBinaryOrExpression.symbols‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@ if (isNodeList(sourceObj) || isHTMLCollection(sourceObj)) {
8686
>sourceObj : Symbol(sourceObj, Decl(controlFlowBinaryOrExpression.ts, 23, 3))
8787

8888
sourceObj.length;
89-
>sourceObj.length : Symbol(length, Decl(controlFlowBinaryOrExpression.ts, 10, 27), Decl(controlFlowBinaryOrExpression.ts, 14, 33))
89+
>sourceObj.length : Symbol(NodeList.length, Decl(controlFlowBinaryOrExpression.ts, 10, 27))
9090
>sourceObj : Symbol(sourceObj, Decl(controlFlowBinaryOrExpression.ts, 23, 3))
91-
>length : Symbol(length, Decl(controlFlowBinaryOrExpression.ts, 10, 27), Decl(controlFlowBinaryOrExpression.ts, 14, 33))
91+
>length : Symbol(NodeList.length, Decl(controlFlowBinaryOrExpression.ts, 10, 27))
9292
}
9393

‎tests/baselines/reference/controlFlowBinaryOrExpression.types‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ if (isNodeList(sourceObj) || isHTMLCollection(sourceObj)) {
106106

107107
sourceObj.length;
108108
>sourceObj.length : number
109-
>sourceObj : NodeList | HTMLCollection | ({ a: string; } & HTMLCollection)
109+
>sourceObj : NodeList
110110
>length : number
111111
}
112112

‎tests/baselines/reference/controlFlowInstanceof.js‎

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
//// [controlFlowInstanceof.ts]
2+
3+
// Repros from #10167
4+
5+
function f1(s: Set<string> | Set<number>) {
6+
s = new Set<number>();
7+
s; // Set<number>
8+
if (s instanceof Set) {
9+
s; // Set<number>
10+
}
11+
s; // Set<number>
12+
s.add(42);
13+
}
14+
15+
function f2(s: Set<string> | Set<number>) {
16+
s = new Set<number>();
17+
s; // Set<number>
18+
if (s instanceof Promise) {
19+
s; // Set<number> & Promise<any>
20+
}
21+
s; // Set<number>
22+
s.add(42);
23+
}
24+
25+
function f3(s: Set<string> | Set<number>) {
26+
s; // Set<string> | Set<number>
27+
if (s instanceof Set) {
28+
s; // Set<string> | Set<number>
29+
}
30+
else {
31+
s; // never
32+
}
33+
}
34+
35+
function f4(s: Set<string> | Set<number>) {
36+
s = new Set<number>();
37+
s; // Set<number>
38+
if (s instanceof Set) {
39+
s; // Set<number>
40+
}
41+
else {
42+
s; // never
43+
}
44+
}
45+
46+
// More tests
47+
48+
class A { a: string }
49+
class B extends A { b: string }
50+
class C extends A { c: string }
51+
52+
function foo(x: A | undefined) {
53+
x; // A | undefined
54+
if (x instanceof B || x instanceof C) {
55+
x; // B | C
56+
}
57+
x; // A | undefined
58+
if (x instanceof B && x instanceof C) {
59+
x; // B & C
60+
}
61+
x; // A | undefined
62+
if (!x) {
63+
return;
64+
}
65+
x; // A
66+
if (x instanceof B) {
67+
x; // B
68+
if (x instanceof C) {
69+
x; // B & C
70+
}
71+
else {
72+
x; // B
73+
}
74+
x; // B
75+
}
76+
else {
77+
x; // A
78+
}
79+
x; // A
80+
}
81+
82+
// X is neither assignable to Y nor a subtype of Y
83+
// Y is assignable to X, but not a subtype of X
84+
85+
interface X {
86+
x?: string;
87+
}
88+
89+
class Y {
90+
y: string;
91+
}
92+
93+
function goo(x: X) {
94+
x;
95+
if (x instanceof Y) {
96+
x.y;
97+
}
98+
x;
99+
}
100+
101+
//// [controlFlowInstanceof.js]
102+
// Repros from #10167
103+
function f1(s) {
104+
s = new Set();
105+
s; // Set<number>
106+
if (s instanceof Set) {
107+
s; // Set<number>
108+
}
109+
s; // Set<number>
110+
s.add(42);
111+
}
112+
function f2(s) {
113+
s = new Set();
114+
s; // Set<number>
115+
if (s instanceof Promise) {
116+
s; // Set<number> & Promise<any>
117+
}
118+
s; // Set<number>
119+
s.add(42);
120+
}
121+
function f3(s) {
122+
s; // Set<string> | Set<number>
123+
if (s instanceof Set) {
124+
s; // Set<string> | Set<number>
125+
}
126+
else {
127+
s; // never
128+
}
129+
}
130+
function f4(s) {
131+
s = new Set();
132+
s; // Set<number>
133+
if (s instanceof Set) {
134+
s; // Set<number>
135+
}
136+
else {
137+
s; // never
138+
}
139+
}
140+
// More tests
141+
class A {
142+
}
143+
class B extends A {
144+
}
145+
class C extends A {
146+
}
147+
function foo(x) {
148+
x; // A | undefined
149+
if (x instanceof B || x instanceof C) {
150+
x; // B | C
151+
}
152+
x; // A | undefined
153+
if (x instanceof B && x instanceof C) {
154+
x; // B & C
155+
}
156+
x; // A | undefined
157+
if (!x) {
158+
return;
159+
}
160+
x; // A
161+
if (x instanceof B) {
162+
x; // B
163+
if (x instanceof C) {
164+
x; // B & C
165+
}
166+
else {
167+
x; // B
168+
}
169+
x; // B
170+
}
171+
else {
172+
x; // A
173+
}
174+
x; // A
175+
}
176+
class Y {
177+
}
178+
function goo(x) {
179+
x;
180+
if (x instanceof Y) {
181+
x.y;
182+
}
183+
x;
184+
}

0 commit comments

Comments
 (0)