Skip to content

Commit af1d91d

Browse files
ericbfericbf
authored
Fix missed errors in switch when using union of literal and non-literal types (#38686) (#51373)
* Fix missed errors in switch when using union of literal and non-literal types (#38686) This commit makes it so we don’t use the base type of literals when checking comparability in switch. The comparability checks handle that case already, is my understanding, so we don’t need to clobber the type before actually doing the check, causing missed errors. When comparing the types in switch, if a union with a literal and a non-literal was used, the compiler in `checker.ts` would automatically get the base type of all parts of the union, resulting in missed errors. For example, if the union of the non-literal `number` and literal `"hello"` was compared to the literal `"world"` in a switch case, the compiler would miss that they’re actually not comparable. Maybe someone can tell me why we were getting the base type before checking comparability, rather than relying on the logic within the comparability checks to handle literal/base type comparability? * Fix lint (whitespace) issue in checker.ts by running lint with fix flag.
1 parent bb42b5c commit af1d91d

8 files changed

+105
-21
lines changed

‎src/compiler/checker.ts

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -40861,7 +40861,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
4086140861
let hasDuplicateDefaultClause = false;
4086240862

4086340863
const expressionType = checkExpression(node.expression);
40864-
const expressionIsLiteral = isLiteralType(expressionType);
40864+
4086540865
forEach(node.caseBlock.clauses, clause => {
4086640866
// Grammar check for duplicate default clauses, skip if we already report duplicate default clause
4086740867
if (clause.kind === SyntaxKind.DefaultClause && !hasDuplicateDefaultClause) {
@@ -40887,16 +40887,11 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
4088740887
// TypeScript 1.0 spec (April 2014): 5.9
4088840888
// In a 'switch' statement, each 'case' expression must be of a type that is comparable
4088940889
// to or from the type of the 'switch' expression.
40890-
let caseType = checkExpression(clause.expression);
40891-
const caseIsLiteral = isLiteralType(caseType);
40892-
let comparedExpressionType = expressionType;
40893-
if (!caseIsLiteral || !expressionIsLiteral) {
40894-
caseType = caseIsLiteral ? getBaseTypeOfLiteralType(caseType) : caseType;
40895-
comparedExpressionType = getBaseTypeOfLiteralType(expressionType);
40896-
}
40897-
if (!isTypeEqualityComparableTo(comparedExpressionType, caseType)) {
40890+
const caseType = checkExpression(clause.expression);
40891+
40892+
if (!isTypeEqualityComparableTo(expressionType, caseType)) {
4089840893
// expressionType is not comparable to caseType, try the reversed check and report errors if it fails
40899-
checkTypeComparableTo(caseType, comparedExpressionType, clause.expression, /*headMessage*/ undefined);
40894+
checkTypeComparableTo(caseType, expressionType, clause.expression, /*headMessage*/ undefined);
4090040895
}
4090140896
};
4090240897
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
tests/cases/compiler/switchAssignmentCompat.ts(4,10): error TS2678: Type 'typeof Foo' is not comparable to type 'number'.
1+
tests/cases/compiler/switchAssignmentCompat.ts(4,10): error TS2678: Type 'typeof Foo' is not comparable to type '0'.
22

33

44
==== tests/cases/compiler/switchAssignmentCompat.ts (1 errors) ====
@@ -7,6 +7,6 @@ tests/cases/compiler/switchAssignmentCompat.ts(4,10): error TS2678: Type 'typeof
77
switch (0) {
88
case Foo: break; // Error expected
99
~~~
10-
!!! error TS2678: Type 'typeof Foo' is not comparable to type 'number'.
10+
!!! error TS2678: Type 'typeof Foo' is not comparable to type '0'.
1111
}
1212

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
tests/cases/compiler/switchCaseCircularRefeference.ts(5,10): error TS2678: Type '{ a: "A"; b: any; } | { a: "C"; e: any; }' is not comparable to type 'string'.
2-
Type '{ a: "C"; e: any; }' is not comparable to type 'string'.
1+
tests/cases/compiler/switchCaseCircularRefeference.ts(5,10): error TS2678: Type '{ a: "A"; b: any; } | { a: "C"; e: any; }' is not comparable to type '"A" | "C"'.
2+
Type '{ a: "C"; e: any; }' is not comparable to type '"A" | "C"'.
33

44

55
==== tests/cases/compiler/switchCaseCircularRefeference.ts (1 errors) ====
@@ -9,8 +9,8 @@ tests/cases/compiler/switchCaseCircularRefeference.ts(5,10): error TS2678: Type
99
switch (x.a) {
1010
case x:
1111
~
12-
!!! error TS2678: Type '{ a: "A"; b: any; } | { a: "C"; e: any; }' is not comparable to type 'string'.
13-
!!! error TS2678: Type '{ a: "C"; e: any; }' is not comparable to type 'string'.
12+
!!! error TS2678: Type '{ a: "A"; b: any; } | { a: "C"; e: any; }' is not comparable to type '"A" | "C"'.
13+
!!! error TS2678: Type '{ a: "C"; e: any; }' is not comparable to type '"A" | "C"'.
1414
break;
1515
}
1616
}

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

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,18 @@
1-
tests/cases/compiler/switchCasesExpressionTypeMismatch.ts(4,10): error TS2678: Type 'typeof Foo' is not comparable to type 'number'.
1+
tests/cases/compiler/switchCasesExpressionTypeMismatch.ts(4,10): error TS2678: Type 'typeof Foo' is not comparable to type '0'.
22
tests/cases/compiler/switchCasesExpressionTypeMismatch.ts(5,10): error TS2678: Type '"sss"' is not comparable to type '0'.
33
tests/cases/compiler/switchCasesExpressionTypeMismatch.ts(6,10): error TS2678: Type '123' is not comparable to type '0'.
44
tests/cases/compiler/switchCasesExpressionTypeMismatch.ts(7,10): error TS2678: Type 'true' is not comparable to type '0'.
5+
tests/cases/compiler/switchCasesExpressionTypeMismatch.ts(16,10): error TS2678: Type 'true' is not comparable to type 'number | "hello"'.
6+
tests/cases/compiler/switchCasesExpressionTypeMismatch.ts(18,10): error TS2678: Type '"world"' is not comparable to type 'number | "hello"'.
57

68

7-
==== tests/cases/compiler/switchCasesExpressionTypeMismatch.ts (4 errors) ====
9+
==== tests/cases/compiler/switchCasesExpressionTypeMismatch.ts (6 errors) ====
810
class Foo { }
911

1012
switch (0) {
1113
case Foo: break; // Error
1214
~~~
13-
!!! error TS2678: Type 'typeof Foo' is not comparable to type 'number'.
15+
!!! error TS2678: Type 'typeof Foo' is not comparable to type '0'.
1416
case "sss": break; // Error
1517
~~~~~
1618
!!! error TS2678: Type '"sss"' is not comparable to type '0'.
@@ -22,6 +24,21 @@ tests/cases/compiler/switchCasesExpressionTypeMismatch.ts(7,10): error TS2678: T
2224
!!! error TS2678: Type 'true' is not comparable to type '0'.
2325
}
2426

27+
declare var q: string
28+
declare var r: number | "hello"
29+
30+
switch (r) {
31+
case q: break
32+
case 42: break
33+
case true: break // Error
34+
~~~~
35+
!!! error TS2678: Type 'true' is not comparable to type 'number | "hello"'.
36+
case "hello": break
37+
case "world": break // Error
38+
~~~~~~~
39+
!!! error TS2678: Type '"world"' is not comparable to type 'number | "hello"'.
40+
}
41+
2542
var s: any = 0;
2643

2744
// No error for all

‎tests/baselines/reference/switchCasesExpressionTypeMismatch.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,17 @@ switch (0) {
88
case true: break; // Error
99
}
1010

11+
declare var q: string
12+
declare var r: number | "hello"
13+
14+
switch (r) {
15+
case q: break
16+
case 42: break
17+
case true: break // Error
18+
case "hello": break
19+
case "world": break // Error
20+
}
21+
1122
var s: any = 0;
1223

1324
// No error for all
@@ -31,6 +42,13 @@ switch (0) {
3142
case 123: break; // Error
3243
case true: break; // Error
3344
}
45+
switch (r) {
46+
case q: break;
47+
case 42: break;
48+
case true: break; // Error
49+
case "hello": break;
50+
case "world": break; // Error
51+
}
3452
var s = 0;
3553
// No error for all
3654
switch (s) {

‎tests/baselines/reference/switchCasesExpressionTypeMismatch.symbols

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,30 @@ switch (0) {
1111
case true: break; // Error
1212
}
1313

14+
declare var q: string
15+
>q : Symbol(q, Decl(switchCasesExpressionTypeMismatch.ts, 9, 11))
16+
17+
declare var r: number | "hello"
18+
>r : Symbol(r, Decl(switchCasesExpressionTypeMismatch.ts, 10, 11))
19+
20+
switch (r) {
21+
>r : Symbol(r, Decl(switchCasesExpressionTypeMismatch.ts, 10, 11))
22+
23+
case q: break
24+
>q : Symbol(q, Decl(switchCasesExpressionTypeMismatch.ts, 9, 11))
25+
26+
case 42: break
27+
case true: break // Error
28+
case "hello": break
29+
case "world": break // Error
30+
}
31+
1432
var s: any = 0;
15-
>s : Symbol(s, Decl(switchCasesExpressionTypeMismatch.ts, 9, 3))
33+
>s : Symbol(s, Decl(switchCasesExpressionTypeMismatch.ts, 20, 3))
1634

1735
// No error for all
1836
switch (s) {
19-
>s : Symbol(s, Decl(switchCasesExpressionTypeMismatch.ts, 9, 3))
37+
>s : Symbol(s, Decl(switchCasesExpressionTypeMismatch.ts, 20, 3))
2038

2139
case Foo: break;
2240
>Foo : Symbol(Foo, Decl(switchCasesExpressionTypeMismatch.ts, 0, 0))

‎tests/baselines/reference/switchCasesExpressionTypeMismatch.types

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,31 @@ switch (0) {
1818
>true : true
1919
}
2020

21+
declare var q: string
22+
>q : string
23+
24+
declare var r: number | "hello"
25+
>r : number | "hello"
26+
27+
switch (r) {
28+
>r : number | "hello"
29+
30+
case q: break
31+
>q : string
32+
33+
case 42: break
34+
>42 : 42
35+
36+
case true: break // Error
37+
>true : true
38+
39+
case "hello": break
40+
>"hello" : "hello"
41+
42+
case "world": break // Error
43+
>"world" : "world"
44+
}
45+
2146
var s: any = 0;
2247
>s : any
2348
>0 : 0

‎tests/cases/compiler/switchCasesExpressionTypeMismatch.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,17 @@ switch (0) {
77
case true: break; // Error
88
}
99

10+
declare var q: string
11+
declare var r: number | "hello"
12+
13+
switch (r) {
14+
case q: break
15+
case 42: break
16+
case true: break // Error
17+
case "hello": break
18+
case "world": break // Error
19+
}
20+
1021
var s: any = 0;
1122

1223
// No error for all

0 commit comments

Comments
 (0)