diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 0ba91204ab0..48bc4300eec 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -148,6 +148,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/restrict-plus-operands`](./docs/rules/restrict-plus-operands.md) | When adding two variables, operands must both be of type number or of type string | | | :thought_balloon: | | [`@typescript-eslint/restrict-template-expressions`](./docs/rules/restrict-template-expressions.md) | Enforce template literal expressions to be of string type | | | :thought_balloon: | | [`@typescript-eslint/strict-boolean-expressions`](./docs/rules/strict-boolean-expressions.md) | Restricts the types allowed in boolean expressions | | | :thought_balloon: | +| [`@typescript-eslint/switch-exhaustiveness-check`](./docs/rules/switch-exhaustiveness-check.md) | Exhaustiveness checking in switch with union type | | | :thought_balloon: | | [`@typescript-eslint/triple-slash-reference`](./docs/rules/triple-slash-reference.md) | Sets preference level for triple slash directives versus ES6-style import declarations | :heavy_check_mark: | | | | [`@typescript-eslint/type-annotation-spacing`](./docs/rules/type-annotation-spacing.md) | Require consistent spacing around type annotations | :heavy_check_mark: | :wrench: | | | [`@typescript-eslint/typedef`](./docs/rules/typedef.md) | Requires type annotations to exist | | | | diff --git a/packages/eslint-plugin/docs/rules/switch-exhaustiveness-check.md b/packages/eslint-plugin/docs/rules/switch-exhaustiveness-check.md new file mode 100644 index 00000000000..a26c6e6dff2 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/switch-exhaustiveness-check.md @@ -0,0 +1,103 @@ +# Exhaustiveness checking in switch with union type (`switch-exhaustiveness-check`) + +Union type may have a lot of parts. It's easy to forget to consider all cases in switch. This rule reminds which parts are missing. If domain of the problem requires to have only a partial switch, developer may _explicitly_ add a default clause. + +Examples of **incorrect** code for this rule: + +```ts +type Day = + | 'Monday' + | 'Tuesday' + | 'Wednesday' + | 'Thursday' + | 'Friday' + | 'Saturday' + | 'Sunday'; + +const day = 'Monday' as Day; +let result = 0; + +switch (day) { + case 'Monday': { + result = 1; + break; + } +} +``` + +Examples of **correct** code for this rule: + +```ts +type Day = + | 'Monday' + | 'Tuesday' + | 'Wednesday' + | 'Thursday' + | 'Friday' + | 'Saturday' + | 'Sunday'; + +const day = 'Monday' as Day; +let result = 0; + +switch (day) { + case 'Monday': { + result = 1; + break; + } + case 'Tuesday': { + result = 2; + break; + } + case 'Wednesday': { + result = 3; + break; + } + case 'Thursday': { + result = 4; + break; + } + case 'Friday': { + result = 5; + break; + } + case 'Saturday': { + result = 6; + break; + } + case 'Sunday': { + result = 7; + break; + } +} +``` + +or + +```ts +type Day = + | 'Monday' + | 'Tuesday' + | 'Wednesday' + | 'Thursday' + | 'Friday' + | 'Saturday' + | 'Sunday'; + +const day = 'Monday' as Day; +let result = 0; + +switch (day) { + case 'Monday': { + result = 1; + break; + } + default: { + result = 42; + } +} +``` + +## When Not To Use It + +If program doesn't have union types with many parts. Downside of this rule is the need for type information, so it's slower than regular rules. diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index 69797b4f5b6..23e9db2d974 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -95,6 +95,7 @@ "space-before-function-paren": "off", "@typescript-eslint/space-before-function-paren": "error", "@typescript-eslint/strict-boolean-expressions": "error", + "@typescript-eslint/switch-exhaustiveness-check": "error", "@typescript-eslint/triple-slash-reference": "error", "@typescript-eslint/type-annotation-spacing": "error", "@typescript-eslint/typedef": "error", diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index fa32bec6986..16a4c705ca5 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -79,6 +79,7 @@ import returnAwait from './return-await'; import semi from './semi'; import spaceBeforeFunctionParen from './space-before-function-paren'; import strictBooleanExpressions from './strict-boolean-expressions'; +import switchExhaustivenessCheck from './switch-exhaustiveness-check'; import tripleSlashReference from './triple-slash-reference'; import typeAnnotationSpacing from './type-annotation-spacing'; import typedef from './typedef'; @@ -167,6 +168,7 @@ export default { semi: semi, 'space-before-function-paren': spaceBeforeFunctionParen, 'strict-boolean-expressions': strictBooleanExpressions, + 'switch-exhaustiveness-check': switchExhaustivenessCheck, 'triple-slash-reference': tripleSlashReference, 'type-annotation-spacing': typeAnnotationSpacing, typedef: typedef, diff --git a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts new file mode 100644 index 00000000000..2cf394d2026 --- /dev/null +++ b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts @@ -0,0 +1,152 @@ +import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; +import * as ts from 'typescript'; +import { + createRule, + getParserServices, + getConstrainedTypeAtLocation, +} from '../util'; +import { isTypeFlagSet, unionTypeParts } from 'tsutils'; +import { isClosingBraceToken, isOpeningBraceToken } from 'eslint-utils'; + +export default createRule({ + name: 'switch-exhaustiveness-check', + meta: { + type: 'suggestion', + docs: { + description: 'Exhaustiveness checking in switch with union type', + category: 'Best Practices', + recommended: false, + requiresTypeChecking: true, + }, + schema: [], + messages: { + switchIsNotExhaustive: + 'Switch is not exhaustive. Cases not matched: {{missingBranches}}', + addMissingCases: 'Add branches for missing cases', + }, + }, + defaultOptions: [], + create(context) { + const sourceCode = context.getSourceCode(); + const service = getParserServices(context); + const checker = service.program.getTypeChecker(); + + function getNodeType(node: TSESTree.Node): ts.Type { + const tsNode = service.esTreeNodeToTSNodeMap.get(node); + return getConstrainedTypeAtLocation(checker, tsNode); + } + + function fixSwitch( + fixer: TSESLint.RuleFixer, + node: TSESTree.SwitchStatement, + missingBranchTypes: Array, + ): TSESLint.RuleFix | null { + const lastCase = + node.cases.length > 0 ? node.cases[node.cases.length - 1] : null; + const caseIndent = lastCase + ? ' '.repeat(lastCase.loc.start.column) + : // if there are no cases, use indentation of the switch statement + // and leave it to user to format it correctly + ' '.repeat(node.loc.start.column); + + const missingCases = []; + for (const missingBranchType of missingBranchTypes) { + // While running this rule on checker.ts of TypeScript project + // the fix introduced a compiler error due to: + // + // type __String = (string & { + // __escapedIdentifier: void; + // }) | (void & { + // __escapedIdentifier: void; + // }) | InternalSymbolName; + // + // The following check fixes it. + if (missingBranchType.isIntersection()) { + continue; + } + + const caseTest = checker.typeToString(missingBranchType); + const errorMessage = `Not implemented yet: ${caseTest} case`; + + missingCases.push( + `case ${caseTest}: { throw new Error('${errorMessage}') }`, + ); + } + + const fixString = missingCases + .map(code => `${caseIndent}${code}`) + .join('\n'); + + if (lastCase) { + return fixer.insertTextAfter(lastCase, `\n${fixString}`); + } + + // there were no existing cases + const openingBrace = sourceCode.getTokenAfter( + node.discriminant, + isOpeningBraceToken, + )!; + const closingBrace = sourceCode.getTokenAfter( + node.discriminant, + isClosingBraceToken, + )!; + + return fixer.replaceTextRange( + [openingBrace.range[0], closingBrace.range[1]], + ['{', fixString, `${caseIndent}}`].join('\n'), + ); + } + + function checkSwitchExhaustive(node: TSESTree.SwitchStatement): void { + const discriminantType = getNodeType(node.discriminant); + + if (discriminantType.isUnion()) { + const unionTypes = unionTypeParts(discriminantType); + const caseTypes: Set = new Set(); + for (const switchCase of node.cases) { + if (switchCase.test === null) { + // Switch has 'default' branch - do nothing. + return; + } + + caseTypes.add(getNodeType(switchCase.test)); + } + + const missingBranchTypes = unionTypes.filter( + unionType => !caseTypes.has(unionType), + ); + + if (missingBranchTypes.length === 0) { + // All cases matched - do nothing. + return; + } + + context.report({ + node: node.discriminant, + messageId: 'switchIsNotExhaustive', + data: { + missingBranches: missingBranchTypes + .map(missingType => + isTypeFlagSet(missingType, ts.TypeFlags.ESSymbolLike) + ? `typeof ${missingType.symbol.escapedName}` + : checker.typeToString(missingType), + ) + .join(' | '), + }, + suggest: [ + { + messageId: 'addMissingCases', + fix(fixer): TSESLint.RuleFix | null { + return fixSwitch(fixer, node, missingBranchTypes); + }, + }, + ], + }); + } + } + + return { + SwitchStatement: checkSwitchExhaustive, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/switch-exhaustiveness-check.test.ts b/packages/eslint-plugin/tests/rules/switch-exhaustiveness-check.test.ts new file mode 100644 index 00000000000..fd9462ddb1e --- /dev/null +++ b/packages/eslint-plugin/tests/rules/switch-exhaustiveness-check.test.ts @@ -0,0 +1,434 @@ +import path from 'path'; +import switchExhaustivenessCheck from '../../src/rules/switch-exhaustiveness-check'; +import { RuleTester } from '../RuleTester'; + +const rootPath = path.join(process.cwd(), 'tests/fixtures/'); + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + tsconfigRootDir: rootPath, + project: './tsconfig.json', + }, +}); + +ruleTester.run('switch-exhaustiveness-check', switchExhaustivenessCheck, { + valid: [ + // All branches matched + ` +type Day = 'Monday' | 'Tuesday' | 'Wednesday' | 'Thursday' | 'Friday' | 'Saturday' | 'Sunday' + +const day = 'Monday' as Day +let result = 0 + +switch (day) { + case 'Monday': { + result = 1 + break + } + case 'Tuesday': { + result = 2 + break + } + case 'Wednesday': { + result = 3 + break + } + case 'Thursday': { + result = 4 + break + } + case 'Friday': { + result = 5 + break + } + case 'Saturday': { + result = 6 + break + } + case 'Sunday': { + result = 7 + break + } +} +`, + // Other primitive literals work too + ` +type Num = 0 | 1 | 2 + +function test(value: Num): number { + switch (value) { + case 0: return 0 + case 1: return 1 + case 2: return 2 + } +} +`, + ` +type Bool = true | false + +function test(value: Bool): number { + switch (value) { + case true: return 1 + case false: return 0 + } +} +`, + ` +type Mix = 0 | 1 | 'two' | 'three' | true + +function test(value: Mix): number { + switch (value) { + case 0: return 0 + case 1: return 1 + case 'two': return 2 + case 'three': return 3 + case true: return 4 + } +} +`, + // Works with type references + ` +type A = 'a' +type B = 'b' +type C = 'c' +type Union = A | B | C + +function test(value: Union): number { + switch (value) { + case 'a': return 1 + case 'b': return 2 + case 'c': return 3 + } +} +`, + // Works with `typeof` + ` +const A = 'a' +const B = 1 +const C = true + +type Union = typeof A | typeof B | typeof C + +function test(value: Union): number { + switch (value) { + case 'a': return 1 + case 1: return 2 + case true: return 3 + } +} +`, + // Switch contains default clause. + ` +type Day = 'Monday' | 'Tuesday' | 'Wednesday' | 'Thursday' | 'Friday' | 'Saturday' | 'Sunday' + +const day = 'Monday' as Day +let result = 0 + +switch (day) { + case 'Monday': { + result = 1 + break + } + default: { + result = 42 + } +} + `, + // Exhaustiveness check only works for union types... + ` +const day = 'Monday' as string +let result = 0 + +switch (day) { + case 'Monday': { + result = 1 + break + } + case 'Tuesday': { + result = 2 + break + } +} + `, + // ... and enums (at least for now). + ` +enum Enum { A, B } + +function test(value: Enum): number { + switch (value) { + case Enum.A: return 1 + case Enum.B: return 2 + } +} +`, + // Object union types won't work either, unless it's a discriminated union + ` +type ObjectUnion = { a: 1 } | { b: 2 } + +function test(value: ObjectUnion): number { + switch (value.a) { + case 1: return 1 + } +} +`, + ], + invalid: [ + { + // Matched only one branch out of seven. + code: ` +type Day = 'Monday' | 'Tuesday' | 'Wednesday' | 'Thursday' | 'Friday' | 'Saturday' | 'Sunday' + +const day = 'Monday' as Day +let result = 0 + +switch (day) { + case 'Monday': { + result = 1 + break + } +} +`, + errors: [ + { + messageId: 'switchIsNotExhaustive', + line: 7, + column: 9, + data: { + missingBranches: + '"Tuesday" | "Wednesday" | "Thursday" | "Friday" | "Saturday" | "Sunday"', + }, + }, + ], + }, + { + // Didn't match all enum variants + code: ` +enum Enum { A, B } + +function test(value: Enum): number { + switch (value) { + case Enum.A: return 1 + } +} +`, + errors: [ + { + messageId: 'switchIsNotExhaustive', + line: 5, + column: 11, + data: { + missingBranches: 'Enum.B', + }, + }, + ], + }, + { + code: ` +type A = 'a' +type B = 'b' +type C = 'c' +type Union = A | B | C + +function test(value: Union): number { + switch (value) { + case 'a': return 1 + } +} +`, + errors: [ + { + messageId: 'switchIsNotExhaustive', + line: 8, + column: 11, + data: { + missingBranches: '"b" | "c"', + }, + }, + ], + }, + { + code: ` +const A = 'a' +const B = 1 +const C = true + +type Union = typeof A | typeof B | typeof C + +function test(value: Union): number { + switch (value) { + case 'a': return 1 + } +} +`, + errors: [ + { + messageId: 'switchIsNotExhaustive', + line: 9, + column: 11, + data: { + missingBranches: 'true | 1', + }, + }, + ], + }, + { + code: ` +type DiscriminatedUnion = { type: 'A', a: 1 } | { type: 'B', b: 2 } + +function test(value: DiscriminatedUnion): number { + switch (value.type) { + case 'A': return 1 + } +} +`, + errors: [ + { + messageId: 'switchIsNotExhaustive', + line: 5, + column: 11, + data: { + missingBranches: '"B"', + }, + }, + ], + }, + { + // Still complains with empty switch + code: ` +type Day = 'Monday' | 'Tuesday' | 'Wednesday' | 'Thursday' | 'Friday' | 'Saturday' | 'Sunday' + +const day = 'Monday' as Day + +switch (day) { +} +`, + errors: [ + { + messageId: 'switchIsNotExhaustive', + line: 6, + column: 9, + data: { + missingBranches: + '"Monday" | "Tuesday" | "Wednesday" | "Thursday" | "Friday" | "Saturday" | "Sunday"', + }, + }, + ], + }, + { + // Still complains with union intersection part + code: ` +type FooBar = string & { foo: void } | 'bar' + +const foobar = 'bar' as FooBar +let result = 0 + +switch (foobar) { + case 'bar': { + result = 42 + break + } +} + `, + errors: [ + { + messageId: 'switchIsNotExhaustive', + line: 7, + column: 9, + data: { + missingBranches: 'string & { foo: void; }', + }, + }, + ], + }, + { + code: ` +const a = Symbol('a') +const b = Symbol('b') +const c = Symbol('c') + +type T = typeof a | typeof b | typeof c + +function test(value: T): number { + switch (value) { + case a: return 1; + } +} + `, + errors: [ + { + messageId: 'switchIsNotExhaustive', + line: 9, + column: 11, + data: { + missingBranches: 'typeof b | typeof c', + }, + }, + ], + }, + // Provides suggestions to add missing cases + { + // with existing cases present + code: ` +type T = 1 | 2 + +function test(value: T): number { + switch (value) { + case 1: return 1; + } +} + `, + errors: [ + { + messageId: 'switchIsNotExhaustive', + suggestions: [ + { + messageId: 'addMissingCases', + output: ` +type T = 1 | 2 + +function test(value: T): number { + switch (value) { + case 1: return 1; + case 2: { throw new Error('Not implemented yet: 2 case') } + } +} + `, + }, + ], + }, + ], + }, + { + // without existing cases + code: ` +type T = 1 | 2 + +function test(value: T): number { + switch (value) { + } +} + `, + errors: [ + { + messageId: 'switchIsNotExhaustive', + suggestions: [ + { + messageId: 'addMissingCases', + output: ` +type T = 1 | 2 + +function test(value: T): number { + switch (value) { + case 1: { throw new Error('Not implemented yet: 1 case') } + case 2: { throw new Error('Not implemented yet: 2 case') } + } +} + `, + }, + ], + }, + ], + }, + ], +});