Skip to content

Commit 0949ad1

Browse files
jack-williamsRyanCavanaugh
jack-williams
authored and
RyanCavanaugh
committed
Fix microsoft#24991: Weaken narrowing for == (microsoft#29840)
Spelling
1 parent 95413f0 commit 0949ad1

6 files changed

Lines changed: 506 additions & 1 deletion

File tree

‎src/compiler/checker.ts‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14227,6 +14227,32 @@ namespace ts {
1422714227
return strictNullChecks ? getGlobalNonNullableTypeInstantiation(type) : type;
1422814228
}
1422914229

14230+
14231+
/**
14232+
* Is source potentially coercible to target type under `==`.
14233+
* Assumes that `source` is a constituent of a union, hence
14234+
* the boolean literal flag on the LHS, but not on the RHS.
14235+
*
14236+
* This does not fully replicate the semantics of `==`. The
14237+
* intention is to catch cases that are clearly not right.
14238+
*
14239+
* Comparing (string | number) to number should not remove the
14240+
* string element.
14241+
*
14242+
* Comparing (string | number) to 1 will remove the string
14243+
* element, though this is not sound. This is a pragmatic
14244+
* choice.
14245+
*
14246+
* @see narrowTypeByEquality
14247+
*
14248+
* @param source
14249+
* @param target
14250+
*/
14251+
function isCoercibleUnderDoubleEquals(source: Type, target: Type): boolean {
14252+
return ((source.flags & (TypeFlags.Number | TypeFlags.String | TypeFlags.BooleanLiteral)) !== 0)
14253+
&& ((target.flags & (TypeFlags.Number | TypeFlags.String | TypeFlags.Boolean)) !== 0);
14254+
}
14255+
1423014256
/**
1423114257
* Return true if type was inferred from an object literal, written as an object type literal, or is the shape of a module
1423214258
* with no call or construct signatures.
@@ -16570,7 +16596,10 @@ namespace ts {
1657016596
return type;
1657116597
}
1657216598
if (assumeTrue) {
16573-
const narrowedType = filterType(type, t => areTypesComparable(t, valueType));
16599+
const filterFn: (t: Type) => boolean = operator === SyntaxKind.EqualsEqualsToken ?
16600+
(t => areTypesComparable(t, valueType) || isCoercibleUnderDoubleEquals(t, valueType)) :
16601+
t => areTypesComparable(t, valueType);
16602+
const narrowedType = filterType(type, filterFn);
1657416603
return narrowedType.flags & TypeFlags.Never ? type : replacePrimitivesWithLiterals(narrowedType, valueType);
1657516604
}
1657616605
if (isUnitType(valueType)) {

‎tests/baselines/reference/narrowByEquality.errors.txt‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
tests/cases/compiler/narrowByEquality.ts(53,15): error TS2322: Type 'string | number' is not assignable to type 'number'.
2+
Type 'string' is not assignable to type 'number'.
3+
tests/cases/compiler/narrowByEquality.ts(54,9): error TS2322: Type 'string | number' is not assignable to type 'number'.
4+
Type 'string' is not assignable to type 'number'.
5+
6+
7+
==== tests/cases/compiler/narrowByEquality.ts (2 errors) ====
8+
declare let x: number | string | boolean
9+
declare let n: number;
10+
declare let s: string;
11+
declare let b: boolean;
12+
13+
if (x == n) {
14+
x;
15+
}
16+
17+
if (x == s) {
18+
x;
19+
}
20+
21+
if (x == b) {
22+
x;
23+
}
24+
25+
if (x == 1) {
26+
x;
27+
}
28+
29+
if (x == "") {
30+
x;
31+
}
32+
33+
if (x == "foo") {
34+
x;
35+
}
36+
37+
if (x == true) {
38+
x;
39+
}
40+
41+
if (x == false) {
42+
x;
43+
}
44+
45+
declare let xAndObj: number | string | boolean | object
46+
47+
if (xAndObj == {}) {
48+
xAndObj;
49+
}
50+
51+
if (x == xAndObj) {
52+
x;
53+
xAndObj;
54+
}
55+
56+
// Repro from #24991
57+
58+
function test(level: number | string):number {
59+
if (level == +level) {
60+
const q2: number = level; // error
61+
~~
62+
!!! error TS2322: Type 'string | number' is not assignable to type 'number'.
63+
!!! error TS2322: Type 'string' is not assignable to type 'number'.
64+
return level;
65+
~~~~~~~~~~~~~
66+
!!! error TS2322: Type 'string | number' is not assignable to type 'number'.
67+
!!! error TS2322: Type 'string' is not assignable to type 'number'.
68+
}
69+
return 0;
70+
}
71+

‎tests/baselines/reference/narrowByEquality.js‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
//// [narrowByEquality.ts]
2+
declare let x: number | string | boolean
3+
declare let n: number;
4+
declare let s: string;
5+
declare let b: boolean;
6+
7+
if (x == n) {
8+
x;
9+
}
10+
11+
if (x == s) {
12+
x;
13+
}
14+
15+
if (x == b) {
16+
x;
17+
}
18+
19+
if (x == 1) {
20+
x;
21+
}
22+
23+
if (x == "") {
24+
x;
25+
}
26+
27+
if (x == "foo") {
28+
x;
29+
}
30+
31+
if (x == true) {
32+
x;
33+
}
34+
35+
if (x == false) {
36+
x;
37+
}
38+
39+
declare let xAndObj: number | string | boolean | object
40+
41+
if (xAndObj == {}) {
42+
xAndObj;
43+
}
44+
45+
if (x == xAndObj) {
46+
x;
47+
xAndObj;
48+
}
49+
50+
// Repro from #24991
51+
52+
function test(level: number | string):number {
53+
if (level == +level) {
54+
const q2: number = level; // error
55+
return level;
56+
}
57+
return 0;
58+
}
59+
60+
61+
//// [narrowByEquality.js]
62+
"use strict";
63+
if (x == n) {
64+
x;
65+
}
66+
if (x == s) {
67+
x;
68+
}
69+
if (x == b) {
70+
x;
71+
}
72+
if (x == 1) {
73+
x;
74+
}
75+
if (x == "") {
76+
x;
77+
}
78+
if (x == "foo") {
79+
x;
80+
}
81+
if (x == true) {
82+
x;
83+
}
84+
if (x == false) {
85+
x;
86+
}
87+
if (xAndObj == {}) {
88+
xAndObj;
89+
}
90+
if (x == xAndObj) {
91+
x;
92+
xAndObj;
93+
}
94+
// Repro from #24991
95+
function test(level) {
96+
if (level == +level) {
97+
var q2 = level; // error
98+
return level;
99+
}
100+
return 0;
101+
}

‎tests/baselines/reference/narrowByEquality.symbols‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
=== tests/cases/compiler/narrowByEquality.ts ===
2+
declare let x: number | string | boolean
3+
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
4+
5+
declare let n: number;
6+
>n : Symbol(n, Decl(narrowByEquality.ts, 1, 11))
7+
8+
declare let s: string;
9+
>s : Symbol(s, Decl(narrowByEquality.ts, 2, 11))
10+
11+
declare let b: boolean;
12+
>b : Symbol(b, Decl(narrowByEquality.ts, 3, 11))
13+
14+
if (x == n) {
15+
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
16+
>n : Symbol(n, Decl(narrowByEquality.ts, 1, 11))
17+
18+
x;
19+
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
20+
}
21+
22+
if (x == s) {
23+
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
24+
>s : Symbol(s, Decl(narrowByEquality.ts, 2, 11))
25+
26+
x;
27+
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
28+
}
29+
30+
if (x == b) {
31+
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
32+
>b : Symbol(b, Decl(narrowByEquality.ts, 3, 11))
33+
34+
x;
35+
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
36+
}
37+
38+
if (x == 1) {
39+
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
40+
41+
x;
42+
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
43+
}
44+
45+
if (x == "") {
46+
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
47+
48+
x;
49+
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
50+
}
51+
52+
if (x == "foo") {
53+
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
54+
55+
x;
56+
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
57+
}
58+
59+
if (x == true) {
60+
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
61+
62+
x;
63+
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
64+
}
65+
66+
if (x == false) {
67+
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
68+
69+
x;
70+
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
71+
}
72+
73+
declare let xAndObj: number | string | boolean | object
74+
>xAndObj : Symbol(xAndObj, Decl(narrowByEquality.ts, 37, 11))
75+
76+
if (xAndObj == {}) {
77+
>xAndObj : Symbol(xAndObj, Decl(narrowByEquality.ts, 37, 11))
78+
79+
xAndObj;
80+
>xAndObj : Symbol(xAndObj, Decl(narrowByEquality.ts, 37, 11))
81+
}
82+
83+
if (x == xAndObj) {
84+
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
85+
>xAndObj : Symbol(xAndObj, Decl(narrowByEquality.ts, 37, 11))
86+
87+
x;
88+
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
89+
90+
xAndObj;
91+
>xAndObj : Symbol(xAndObj, Decl(narrowByEquality.ts, 37, 11))
92+
}
93+
94+
// Repro from #24991
95+
96+
function test(level: number | string):number {
97+
>test : Symbol(test, Decl(narrowByEquality.ts, 46, 1))
98+
>level : Symbol(level, Decl(narrowByEquality.ts, 50, 14))
99+
100+
if (level == +level) {
101+
>level : Symbol(level, Decl(narrowByEquality.ts, 50, 14))
102+
>level : Symbol(level, Decl(narrowByEquality.ts, 50, 14))
103+
104+
const q2: number = level; // error
105+
>q2 : Symbol(q2, Decl(narrowByEquality.ts, 52, 13))
106+
>level : Symbol(level, Decl(narrowByEquality.ts, 50, 14))
107+
108+
return level;
109+
>level : Symbol(level, Decl(narrowByEquality.ts, 50, 14))
110+
}
111+
return 0;
112+
}
113+

0 commit comments

Comments
 (0)