Skip to content

Commit 95b3f37

Browse files
JeanMechemattrbeck
JeanMeche
authored and
mattrbeck
committed
feat(compiler): Exhaustive checks for switch blocks
`@switch` blocks can now enable exhaustive typechecking by adding `@default(never);` at the end of a `@switch` block.
1 parent a8aab64 commit 95b3f37

File tree

19 files changed

+443
-20
lines changed

19 files changed

+443
-20
lines changed

‎packages/compiler-cli/src/ngtsc/typecheck/src/ops/switch_block.ts‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@
88

99
import {TmplAstSwitchBlock, TmplAstSwitchBlockCaseGroup} from '@angular/compiler';
1010
import ts from 'typescript';
11+
import {markIgnoreDiagnostics} from '../comments';
1112
import {TcbOp} from './base';
12-
import type {Scope} from './scope';
1313
import type {Context} from './context';
1414
import {tcbExpression} from './expression';
15-
import {markIgnoreDiagnostics} from '../comments';
15+
import type {Scope} from './scope';
1616

1717
/**
1818
* A `TcbOp` which renders a `switch` block as a TypeScript `switch` statement.
@@ -56,6 +56,28 @@ export class TcbSwitchOp extends TcbOp {
5656
});
5757
});
5858

59+
if (this.block.exhaustiveCheck) {
60+
const switchValue = tcbExpression(this.block.expression, this.tcb, this.scope);
61+
clauses.push(
62+
ts.factory.createDefaultClause([
63+
ts.factory.createVariableStatement(
64+
undefined,
65+
ts.factory.createVariableDeclarationList(
66+
[
67+
ts.factory.createVariableDeclaration(
68+
ts.factory.createUniqueName('tcbExhaustive'),
69+
undefined,
70+
ts.factory.createKeywordTypeNode(ts.SyntaxKind.NeverKeyword),
71+
switchValue,
72+
),
73+
],
74+
ts.NodeFlags.Const,
75+
),
76+
),
77+
]),
78+
);
79+
}
80+
5981
this.scope.addStatement(
6082
ts.factory.createSwitchStatement(switchExpression, ts.factory.createCaseBlock(clauses)),
6183
);

‎packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1372,6 +1372,65 @@ class TestComponent {
13721372
});
13731373
});
13741374

1375+
describe('switch exhaustiveness', () => {
1376+
it('should report an error when a case is missing in a switch block', () => {
1377+
const messages = diagnose(
1378+
`
1379+
@switch (value) {
1380+
@case ('a') {}
1381+
@default never;
1382+
}
1383+
`,
1384+
`
1385+
export class TestComponent {
1386+
value: 'a' | 'b';
1387+
}
1388+
`,
1389+
);
1390+
1391+
expect(messages).toEqual([
1392+
`TestComponent.html(2, 20): Type '"b"' is not assignable to type 'never'.`,
1393+
]);
1394+
});
1395+
1396+
it('should not report an error when all cases are handled', () => {
1397+
const messages = diagnose(
1398+
`
1399+
@switch (value) {
1400+
@case ('a') {}
1401+
@case ('b') {}
1402+
@default never;
1403+
}
1404+
`,
1405+
`
1406+
export class TestComponent {
1407+
value: 'a' | 'b';
1408+
}
1409+
`,
1410+
);
1411+
1412+
expect(messages).toEqual([]);
1413+
});
1414+
1415+
it('should not report an error when a default case is provided', () => {
1416+
const messages = diagnose(
1417+
`
1418+
@switch (value) {
1419+
@case ('a') {}
1420+
@default {}
1421+
}
1422+
`,
1423+
`
1424+
export class TestComponent {
1425+
value: 'a' | 'b';
1426+
}
1427+
`,
1428+
);
1429+
1430+
expect(messages).toEqual([]);
1431+
});
1432+
});
1433+
13751434
// https://github.com/angular/angular/issues/43970
13761435
describe('template parse failures', () => {
13771436
afterEach(resetParseTemplateAsSourceFileForTest);

‎packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts‎

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {absoluteFrom, getSourceFileOrError} from '../../file_system';
1212
import {initMockFileSystem} from '../../file_system/testing';
1313
import {Reference} from '../../imports';
1414
import {OptimizeFor, TypeCheckingConfig} from '../api';
15-
import {ALL_ENABLED_CONFIG, setup, tcb, TestDeclaration, TestDirective} from '../testing';
15+
import {ALL_ENABLED_CONFIG, diagnose, setup, tcb, TestDeclaration, TestDirective} from '../testing';
1616

1717
describe('type check blocks', () => {
1818
beforeEach(() => initMockFileSystem('Native'));
@@ -2141,6 +2141,89 @@ describe('type check blocks', () => {
21412141
'switch (((this).expr)) { ' + 'case 1: break; ' + 'case 2: break; ' + 'default: break; }',
21422142
);
21432143
});
2144+
2145+
it('should generate a switch block with exhaustiveness checking', () => {
2146+
const TEMPLATE = `
2147+
@switch (expr) {
2148+
@case (1) {
2149+
{{one()}}
2150+
}
2151+
@case (2) {
2152+
{{two()}}
2153+
}
2154+
@default never;
2155+
}
2156+
`;
2157+
2158+
expect(tcb(TEMPLATE)).toContain(
2159+
'switch (((this).expr)) { ' +
2160+
'case 1: "" + ((this).one()); break; ' +
2161+
'case 2: "" + ((this).two()); break; ' +
2162+
'default: const tcbExhaustive_1: never = ((this).expr);',
2163+
);
2164+
});
2165+
2166+
it('should not report unused locals for exhaustiveness check variable', () => {
2167+
const TEMPLATE = `
2168+
@switch (expr) {
2169+
@case (1) {}
2170+
@default never;
2171+
}
2172+
`;
2173+
const SOURCE = `
2174+
export class TestComponent {
2175+
expr!: 1|2;
2176+
}
2177+
`;
2178+
expect(diagnose(TEMPLATE, SOURCE, undefined, [], undefined, {noUnusedLocals: true})).toEqual([
2179+
`TestComponent.html(2, 18): Type '2' is not assignable to type 'never'.`,
2180+
]);
2181+
});
2182+
2183+
it('should not generate exhaustiveness checking when there is a consecutive default case', () => {
2184+
const TEMPLATE = `
2185+
@switch (expr) {
2186+
@case (1) {
2187+
{{one()}}
2188+
}
2189+
@case (2)
2190+
@default {
2191+
{{default()}}
2192+
}
2193+
}
2194+
`;
2195+
2196+
expect(tcb(TEMPLATE)).toContain(
2197+
'switch (((this).expr)) { ' +
2198+
'case 1: "" + ((this).one()); break; ' +
2199+
'case 2: ' +
2200+
'default: "" + ((this).default()); break; }',
2201+
);
2202+
});
2203+
2204+
it('should generate the right TCB if default is not the last case', () => {
2205+
const TEMPLATE = `
2206+
@switch (expr) {
2207+
@case (1) {
2208+
{{one()}}
2209+
}
2210+
@default
2211+
@case (3) {
2212+
{{default()}}
2213+
}
2214+
@case (2) {
2215+
{{two()}}
2216+
}
2217+
}
2218+
`;
2219+
expect(tcb(TEMPLATE)).toContain(
2220+
'switch (((this).expr)) { ' +
2221+
'case 1: "" + ((this).one()); break; ' +
2222+
'default: ' +
2223+
'case 3: "" + ((this).default()); break; ' +
2224+
'case 2: "" + ((this).two()); break; }',
2225+
);
2226+
});
21442227
});
21452228

21462229
describe('for loop blocks', () => {

‎packages/compiler/src/combined_visitor.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,8 @@ export class CombinedRecursiveAstVisitor extends RecursiveAstVisitor implements
101101
this.visitAllTemplateNodes(block.children);
102102
}
103103

104+
visitSwitchExhaustiveCheck(block: t.SwitchExhaustiveCheck): void {}
105+
104106
visitForLoopBlock(block: t.ForLoopBlock): void {
105107
block.item.visit(this);
106108
this.visitAllTemplateNodes(block.contextVariables);

‎packages/compiler/src/compiler.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ export {
169169
SwitchBlock as TmplAstSwitchBlock,
170170
SwitchBlockCase as TmplAstSwitchBlockCase,
171171
SwitchBlockCaseGroup as TmplAstSwitchBlockCaseGroup,
172+
SwitchExhaustiveCheck as TmplAstSwitchExhaustiveCheck,
172173
Template as TmplAstTemplate,
173174
Text as TmplAstText,
174175
TextAttribute as TmplAstTextAttribute,

‎packages/compiler/src/ml_parser/lexer.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,14 @@ class _Tokenizer {
299299
this._beginToken(TokenType.BLOCK_OPEN_START, start);
300300
const startToken = this._endToken([this._getBlockName()]);
301301

302+
if (startToken.parts[0] === 'default never' && this._attemptCharCode(chars.$SEMICOLON)) {
303+
this._beginToken(TokenType.BLOCK_OPEN_END);
304+
this._endToken([]);
305+
this._beginToken(TokenType.BLOCK_CLOSE);
306+
this._endToken([]);
307+
return;
308+
}
309+
302310
if (this._cursor.peek() === chars.$LPAREN) {
303311
// Advance past the opening paren.
304312
this._cursor.advance();

‎packages/compiler/src/render3/r3_ast.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,7 @@ export class SwitchBlock extends BlockNode implements Node {
410410
* aren't meant to be processed in any other way.
411411
*/
412412
public unknownBlocks: UnknownBlock[],
413+
public exhaustiveCheck: SwitchExhaustiveCheck | null,
413414
sourceSpan: ParseSourceSpan,
414415
startSourceSpan: ParseSourceSpan,
415416
endSourceSpan: ParseSourceSpan | null,
@@ -457,6 +458,21 @@ export class SwitchBlockCaseGroup extends BlockNode implements Node {
457458
}
458459
}
459460

461+
export class SwitchExhaustiveCheck extends BlockNode implements Node {
462+
constructor(
463+
sourceSpan: ParseSourceSpan,
464+
startSourceSpan: ParseSourceSpan,
465+
endSourceSpan: ParseSourceSpan | null,
466+
nameSpan: ParseSourceSpan,
467+
) {
468+
super(nameSpan, sourceSpan, startSourceSpan, endSourceSpan);
469+
}
470+
471+
visit<Result>(visitor: Visitor<Result>): Result {
472+
return visitor.visitSwitchExhaustiveCheck(this);
473+
}
474+
}
475+
460476
export class ForLoopBlock extends BlockNode implements Node {
461477
constructor(
462478
public item: Variable,
@@ -725,6 +741,7 @@ export interface Visitor<Result = any> {
725741
visitSwitchBlock(block: SwitchBlock): Result;
726742
visitSwitchBlockCase(block: SwitchBlockCase): Result;
727743
visitSwitchBlockCaseGroup(block: SwitchBlockCaseGroup): Result;
744+
visitSwitchExhaustiveCheck(block: SwitchExhaustiveCheck): Result;
728745
visitForLoopBlock(block: ForLoopBlock): Result;
729746
visitForLoopBlockEmpty(block: ForLoopBlockEmpty): Result;
730747
visitIfBlock(block: IfBlock): Result;
@@ -773,6 +790,7 @@ export class RecursiveVisitor implements Visitor<void> {
773790
visitAll(this, block.cases);
774791
visitAll(this, block.children);
775792
}
793+
visitSwitchExhaustiveCheck(block: SwitchExhaustiveCheck): void {}
776794
visitForLoopBlock(block: ForLoopBlock): void {
777795
const blockItems = [block.item, ...block.contextVariables, ...block.children];
778796
block.empty && blockItems.push(block.empty);

‎packages/compiler/src/render3/r3_control_flow.ts‎

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -232,23 +232,67 @@ export function createSwitchBlock(
232232
const unknownBlocks: t.UnknownBlock[] = [];
233233
let collectedCases: t.SwitchBlockCase[] = [];
234234
let firstCaseStart: ParseSourceSpan | null = null;
235+
let exhaustiveCheck: t.SwitchExhaustiveCheck | null = null;
235236

236237
// Here we assume that all the blocks are valid given that we validated them above.
237238
for (const node of ast.children) {
238239
if (!(node instanceof html.Block)) {
239240
continue;
240241
}
241242

242-
if ((node.name !== 'case' || node.parameters.length === 0) && node.name !== 'default') {
243+
if (
244+
(node.name !== 'case' || node.parameters.length === 0) &&
245+
node.name !== 'default' &&
246+
node.name !== 'default never'
247+
) {
243248
unknownBlocks.push(new t.UnknownBlock(node.name, node.sourceSpan, node.nameSpan));
244249
continue;
245250
}
246251

252+
if (exhaustiveCheck !== null) {
253+
errors.push(
254+
new ParseError(
255+
node.sourceSpan,
256+
'@default block with "never" parameter must be the last case in a switch',
257+
),
258+
);
259+
}
260+
247261
const isCase = node.name === 'case';
248262
let expression: AST | null = null;
249263

250264
if (isCase) {
251265
expression = parseBlockParameterToBinding(node.parameters[0], bindingParser);
266+
} else if (node.name === 'default never') {
267+
if (
268+
node.children.length > 0 ||
269+
(node.endSourceSpan !== null &&
270+
node.endSourceSpan.start.offset !== node.endSourceSpan.end.offset)
271+
) {
272+
errors.push(
273+
new ParseError(
274+
node.sourceSpan,
275+
'@default block with "never" parameter cannot have a body',
276+
),
277+
);
278+
}
279+
280+
if (collectedCases.length > 0) {
281+
errors.push(
282+
new ParseError(
283+
node.sourceSpan,
284+
'A @case block with no body cannot be followed by a @default block with "never" parameter',
285+
),
286+
);
287+
}
288+
289+
exhaustiveCheck = new t.SwitchExhaustiveCheck(
290+
node.sourceSpan,
291+
node.startSourceSpan,
292+
node.endSourceSpan,
293+
node.nameSpan,
294+
);
295+
continue;
252296
}
253297

254298
const switchCase = new t.SwitchBlockCase(
@@ -300,6 +344,7 @@ export function createSwitchBlock(
300344
primaryExpression,
301345
groups,
302346
unknownBlocks,
347+
exhaustiveCheck,
303348
ast.sourceSpan,
304349
ast.startSourceSpan,
305350
ast.endSourceSpan,
@@ -570,14 +615,24 @@ function validateSwitchBlock(ast: html.Block): ParseError[] {
570615
continue;
571616
}
572617

573-
if (!(node instanceof html.Block) || (node.name !== 'case' && node.name !== 'default')) {
618+
if (
619+
!(node instanceof html.Block) ||
620+
(node.name !== 'case' && node.name !== 'default' && node.name !== 'default never')
621+
) {
574622
errors.push(
575623
new ParseError(node.sourceSpan, '@switch block can only contain @case and @default blocks'),
576624
);
577625
continue;
578626
}
579627

580-
if (node.name === 'default') {
628+
if (node.name === 'default never') {
629+
if (hasDefault) {
630+
errors.push(
631+
new ParseError(node.startSourceSpan, '@switch block can only have one @default block'),
632+
);
633+
}
634+
hasDefault = true;
635+
} else if (node.name === 'default') {
581636
if (hasDefault) {
582637
errors.push(
583638
new ParseError(node.startSourceSpan, '@switch block can only have one @default block'),

0 commit comments

Comments
 (0)