From e2b378913a1a272ce7c1214dc4b9b916655e936e Mon Sep 17 00:00:00 2001 From: StyleShit Date: Fri, 20 Oct 2023 15:54:56 +0300 Subject: [PATCH 1/6] feat(eslint-plugin): [switch-exhaustiveness-check] members with new line or single quotes are not being fixed correctly Closes #7768 --- .../docs/rules/switch-exhaustiveness-check.md | 78 +++++++++++++++++-- .../src/rules/switch-exhaustiveness-check.ts | 15 +++- .../rules/switch-exhaustiveness-check.test.ts | 48 +++++++++++- 3 files changed, 130 insertions(+), 11 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/switch-exhaustiveness-check.md b/packages/eslint-plugin/docs/rules/switch-exhaustiveness-check.md index 9320624924b9..e2719cbee690 100644 --- a/packages/eslint-plugin/docs/rules/switch-exhaustiveness-check.md +++ b/packages/eslint-plugin/docs/rules/switch-exhaustiveness-check.md @@ -1,15 +1,15 @@ --- -description: 'Require switch-case statements to be exhaustive with union type.' +description: 'Require switch-case statements to be exhaustive with union type or enum.' --- > 🛑 This file is source code, not the primary documentation location! 🛑 > > See **https://typescript-eslint.io/rules/switch-exhaustiveness-check** for documentation. -When working with union types in TypeScript, it's common to want to write a `switch` statement intended to contain a `case` for each constituent (possible type in the union). -However, if the union type changes, it's easy to forget to modify the cases to account for any new types. +When working with union types or enums in TypeScript, it's common to want to write a `switch` statement intended to contain a `case` for each constituent (possible type in the union or the enum). +However, if the union type or the enum changes, it's easy to forget to modify the cases to account for any new types. -This rule reports when a `switch` statement over a value typed as a union of literals is missing a case for any of those literal types and does not have a `default` clause. +This rule reports when a `switch` statement over a value typed as a union of literals or as an enum is missing a case for any of those literal types and does not have a `default` clause. ## Examples @@ -101,6 +101,74 @@ switch (day) { } ``` + + +### ❌ Incorrect + +```ts +enum Fruit { + Apple, + Banana, + Cherry, +} + +const fruit = Fruit.Cherry as Fruit; + +switch (fruit) { + case Fruit.Apple: + console.log('an apple'); + break; +} +``` + +### ✅ Correct + +```ts +enum Fruit { + Apple, + Banana, + Cherry, +} + +const fruit = Fruit.Cherry as Fruit; + +switch (fruit) { + case Fruit.Apple: + console.log('an apple'); + break; + + case Fruit.Banana: + console.log('a banana'); + break; + + case Fruit.Cherry: + console.log('a cherry'); + break; +} +``` + +### ✅ Correct + +```ts +enum Fruit { + Apple, + Banana, + Cherry, +} + +const fruit = Fruit.Cherry as Fruit; + +switch (fruit) { + case Fruit.Apple: + console.log('an apple'); + break; + + default: + console.log('a fruit'); + break; +} +``` + ## When Not To Use It -If you don't frequently `switch` over union types with many parts, or intentionally wish to leave out some parts. +If you don't frequently `switch` over union types or enums with many parts, or intentionally wish to leave out some parts. diff --git a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts index 6abdbf27fc6e..a35579586a57 100644 --- a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts +++ b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts @@ -17,7 +17,7 @@ export default createRule({ type: 'suggestion', docs: { description: - 'Require switch-case statements to be exhaustive with union type', + 'Require switch-case statements to be exhaustive with union type or enum', requiresTypeChecking: true, }, hasSuggestions: true, @@ -66,6 +66,7 @@ export default createRule({ } const missingBranchName = missingBranchType.getSymbol()?.escapedName; + const requiresBackticks = missingBranchName?.match(/[\r\n]/g); let caseTest = checker.typeToString(missingBranchType); if ( @@ -73,10 +74,18 @@ export default createRule({ (missingBranchName || missingBranchName === '') && requiresQuoting(missingBranchName.toString(), compilerOptions.target) ) { - caseTest = `${symbolName}['${missingBranchName}']`; + caseTest = requiresBackticks + ? `${symbolName}[\`${missingBranchName}\`]` + : `${symbolName}['${missingBranchName}']`; } - const errorMessage = `Not implemented yet: ${caseTest} case`; + // escape single quotes and newlines, so that the error message is a readable and valid code. + const escapedCaseTest = caseTest + .replace(/'/g, "\\'") + .replace(/\n/g, '\\n') + .replace(/\r/g, '\\r'); + + const errorMessage = `Not implemented yet: ${escapedCaseTest} case`; missingCases.push( `case ${caseTest}: { throw new Error('${errorMessage}') }`, diff --git a/packages/eslint-plugin/tests/rules/switch-exhaustiveness-check.test.ts b/packages/eslint-plugin/tests/rules/switch-exhaustiveness-check.test.ts index b250a09e2bb2..cbda3cf0b091 100644 --- a/packages/eslint-plugin/tests/rules/switch-exhaustiveness-check.test.ts +++ b/packages/eslint-plugin/tests/rules/switch-exhaustiveness-check.test.ts @@ -518,7 +518,7 @@ export enum Enum { function test(arg: Enum): string { switch (arg) { - case Enum['test-test']: { throw new Error('Not implemented yet: Enum['test-test'] case') } + case Enum['test-test']: { throw new Error('Not implemented yet: Enum[\\'test-test\\'] case') } case Enum.test: { throw new Error('Not implemented yet: Enum.test case') } } } @@ -555,7 +555,7 @@ export enum Enum { function test(arg: Enum): string { switch (arg) { - case Enum['']: { throw new Error('Not implemented yet: Enum[''] case') } + case Enum['']: { throw new Error('Not implemented yet: Enum[\\'\\'] case') } case Enum.test: { throw new Error('Not implemented yet: Enum.test case') } } } @@ -592,7 +592,7 @@ export enum Enum { function test(arg: Enum): string { switch (arg) { - case Enum['9test']: { throw new Error('Not implemented yet: Enum['9test'] case') } + case Enum['9test']: { throw new Error('Not implemented yet: Enum[\\'9test\\'] case') } case Enum.test: { throw new Error('Not implemented yet: Enum.test case') } } } @@ -602,5 +602,47 @@ function test(arg: Enum): string { }, ], }, + { + code: ` + enum Enum { + 'a' = 1, + [\`key-with + + new-line\`] = 2, + } + + declare const a: Enum; + + switch (a) { + } + `, + errors: [ + { + messageId: 'switchIsNotExhaustive', + suggestions: [ + { + messageId: 'addMissingCases', + output: ` + enum Enum { + 'a' = 1, + [\`key-with + + new-line\`] = 2, + } + + declare const a: Enum; + + switch (a) { + case Enum.a: { throw new Error('Not implemented yet: Enum.a case') } + case Enum[\`key-with + + new-line\`]: { throw new Error('Not implemented yet: Enum[\`key-with\\n\\n new-line\`] case') } + } + `, + }, + ], + }, + ], + }, ], }); From dd729d1ed56c54b346fcb6039298d6f0843285d4 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Fri, 20 Oct 2023 16:21:21 +0300 Subject: [PATCH 2/6] cleanup --- .../eslint-plugin/src/rules/switch-exhaustiveness-check.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts index a35579586a57..c5a6a99ee25c 100644 --- a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts +++ b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts @@ -70,10 +70,11 @@ export default createRule({ let caseTest = checker.typeToString(missingBranchType); if ( - symbolName && (missingBranchName || missingBranchName === '') && requiresQuoting(missingBranchName.toString(), compilerOptions.target) ) { + const requiresBackticks = missingBranchName?.match(/[\r\n]/g); + caseTest = requiresBackticks ? `${symbolName}[\`${missingBranchName}\`]` : `${symbolName}['${missingBranchName}']`; From fd21aa4f3ba71d674f53d5998f21ec377774a405 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Fri, 20 Oct 2023 16:22:15 +0300 Subject: [PATCH 3/6] oops --- packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts index c5a6a99ee25c..071e307dac39 100644 --- a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts +++ b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts @@ -66,7 +66,6 @@ export default createRule({ } const missingBranchName = missingBranchType.getSymbol()?.escapedName; - const requiresBackticks = missingBranchName?.match(/[\r\n]/g); let caseTest = checker.typeToString(missingBranchType); if ( From 1acfe46fae07d35ec1676abbd73efc248fcd5e2e Mon Sep 17 00:00:00 2001 From: StyleShit Date: Mon, 23 Oct 2023 19:46:15 +0300 Subject: [PATCH 4/6] wip wip --- .../docs/rules/switch-exhaustiveness-check.md | 24 +++++++++++++------ .../src/rules/switch-exhaustiveness-check.ts | 4 ++-- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/switch-exhaustiveness-check.md b/packages/eslint-plugin/docs/rules/switch-exhaustiveness-check.md index e2719cbee690..11998bfc5f23 100644 --- a/packages/eslint-plugin/docs/rules/switch-exhaustiveness-check.md +++ b/packages/eslint-plugin/docs/rules/switch-exhaustiveness-check.md @@ -1,5 +1,5 @@ --- -description: 'Require switch-case statements to be exhaustive with union type or enum.' +description: 'Require switch-case statements to be exhaustive with union types and enums.' --- > 🛑 This file is source code, not the primary documentation location! 🛑 @@ -13,6 +13,10 @@ This rule reports when a `switch` statement over a value typed as a union of lit ## Examples +When the switch doesn't have exhaustive cases, either filling them all out or adding a default will correct the rule's complaint. + +Here are some examples of code working with a union of literals: + ### ❌ Incorrect @@ -27,7 +31,7 @@ type Day = | 'Saturday' | 'Sunday'; -const day = 'Monday' as Day; +declare const day: Day; let result = 0; switch (day) { @@ -49,7 +53,7 @@ type Day = | 'Saturday' | 'Sunday'; -const day = 'Monday' as Day; +declare const day: Day; let result = 0; switch (day) { @@ -89,7 +93,7 @@ type Day = | 'Saturday' | 'Sunday'; -const day = 'Monday' as Day; +declare const day: Day; let result = 0; switch (day) { @@ -101,6 +105,10 @@ switch (day) { } ``` + + +Likewise, here are some examples of code working with an enum: + ### ❌ Incorrect @@ -112,7 +120,7 @@ enum Fruit { Cherry, } -const fruit = Fruit.Cherry as Fruit; +declare const fruit: Fruit; switch (fruit) { case Fruit.Apple: @@ -130,7 +138,7 @@ enum Fruit { Cherry, } -const fruit = Fruit.Cherry as Fruit; +declare const fruit: Fruit; switch (fruit) { case Fruit.Apple: @@ -156,7 +164,7 @@ enum Fruit { Cherry, } -const fruit = Fruit.Cherry as Fruit; +declare const fruit: Fruit; switch (fruit) { case Fruit.Apple: @@ -169,6 +177,8 @@ switch (fruit) { } ``` + + ## When Not To Use It If you don't frequently `switch` over union types or enums with many parts, or intentionally wish to leave out some parts. diff --git a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts index 071e307dac39..a96e01a444c1 100644 --- a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts +++ b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts @@ -17,7 +17,7 @@ export default createRule({ type: 'suggestion', docs: { description: - 'Require switch-case statements to be exhaustive with union type or enum', + 'Require switch-case statements to be exhaustive with union types and enums', requiresTypeChecking: true, }, hasSuggestions: true, @@ -79,7 +79,7 @@ export default createRule({ : `${symbolName}['${missingBranchName}']`; } - // escape single quotes and newlines, so that the error message is a readable and valid code. + // escape single quotes and newlines, so that the error message is readable and valid code. const escapedCaseTest = caseTest .replace(/'/g, "\\'") .replace(/\n/g, '\\n') From c63f866d29df0f0bd5998674ec943589108c1bfa Mon Sep 17 00:00:00 2001 From: StyleShit Date: Mon, 23 Oct 2023 22:42:52 +0300 Subject: [PATCH 5/6] maybe? --- .../src/rules/switch-exhaustiveness-check.ts | 20 ++++------ .../rules/switch-exhaustiveness-check.test.ts | 39 +++++++++++++++++-- 2 files changed, 44 insertions(+), 15 deletions(-) diff --git a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts index a96e01a444c1..add9c1171bc7 100644 --- a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts +++ b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts @@ -72,23 +72,19 @@ export default createRule({ (missingBranchName || missingBranchName === '') && requiresQuoting(missingBranchName.toString(), compilerOptions.target) ) { - const requiresBackticks = missingBranchName?.match(/[\r\n]/g); + const escapedBranchName = missingBranchName + .replace(/'/g, "\\'") + .replace(/\n/g, '\\n') + .replace(/\r/g, '\\r'); - caseTest = requiresBackticks - ? `${symbolName}[\`${missingBranchName}\`]` - : `${symbolName}['${missingBranchName}']`; + caseTest = `${symbolName}['${escapedBranchName}']`; } - // escape single quotes and newlines, so that the error message is readable and valid code. - const escapedCaseTest = caseTest - .replace(/'/g, "\\'") - .replace(/\n/g, '\\n') - .replace(/\r/g, '\\r'); - - const errorMessage = `Not implemented yet: ${escapedCaseTest} case`; + const errorMessage = `Not implemented yet: ${caseTest} case`; + const escapedErrorMessage = errorMessage.replace(/'/g, "\\'"); missingCases.push( - `case ${caseTest}: { throw new Error('${errorMessage}') }`, + `case ${caseTest}: { throw new Error('${escapedErrorMessage}') }`, ); } diff --git a/packages/eslint-plugin/tests/rules/switch-exhaustiveness-check.test.ts b/packages/eslint-plugin/tests/rules/switch-exhaustiveness-check.test.ts index cbda3cf0b091..9852e0d543df 100644 --- a/packages/eslint-plugin/tests/rules/switch-exhaustiveness-check.test.ts +++ b/packages/eslint-plugin/tests/rules/switch-exhaustiveness-check.test.ts @@ -1,4 +1,4 @@ -import { RuleTester } from '@typescript-eslint/rule-tester'; +import { noFormat, RuleTester } from '@typescript-eslint/rule-tester'; import path from 'path'; import switchExhaustivenessCheck from '../../src/rules/switch-exhaustiveness-check'; @@ -634,9 +634,42 @@ function test(arg: Enum): string { switch (a) { case Enum.a: { throw new Error('Not implemented yet: Enum.a case') } - case Enum[\`key-with + case Enum['key-with\\n\\n new-line']: { throw new Error('Not implemented yet: Enum[\\'key-with\\n\\n new-line\\'] case') } + } + `, + }, + ], + }, + ], + }, + { + code: noFormat` + enum Enum { + 'a' = 1, + "'a' \`b\` \\"c\\"" = 2, + } - new-line\`]: { throw new Error('Not implemented yet: Enum[\`key-with\\n\\n new-line\`] case') } + declare const a: Enum; + + switch (a) {} + `, + errors: [ + { + messageId: 'switchIsNotExhaustive', + suggestions: [ + { + messageId: 'addMissingCases', + output: ` + enum Enum { + 'a' = 1, + "'a' \`b\` \\"c\\"" = 2, + } + + declare const a: Enum; + + switch (a) { + case Enum.a: { throw new Error('Not implemented yet: Enum.a case') } + case Enum['\\'a\\' \`b\` "c"']: { throw new Error('Not implemented yet: Enum[\\'\\\\'a\\\\' \`b\` "c"\\'] case') } } `, }, From 82d5b37c9364a6ba07072318317e591bd0dde005 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Mon, 23 Oct 2023 22:45:10 +0300 Subject: [PATCH 6/6] idk why it was missing --- packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts index add9c1171bc7..fc3f30753ed8 100644 --- a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts +++ b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts @@ -69,6 +69,7 @@ export default createRule({ let caseTest = checker.typeToString(missingBranchType); if ( + symbolName && (missingBranchName || missingBranchName === '') && requiresQuoting(missingBranchName.toString(), compilerOptions.target) ) {