From 8fa47a199b36370f40525452e7f7bd8676a8fd91 Mon Sep 17 00:00:00 2001 From: Zamiell <5511220+Zamiell@users.noreply.github.com> Date: Tue, 2 Jan 2024 17:30:37 -0500 Subject: [PATCH 1/7] fix(allowDefaultCaseForExhaustiveSwitch): rule --- .../src/rules/switch-exhaustiveness-check.ts | 2 +- .../rules/switch-exhaustiveness-check.test.ts | 21 +++++++++++++++++++ 2 files changed, 22 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 64ab5ef6b2e2..605dc1c6bb9e 100644 --- a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts +++ b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts @@ -268,7 +268,7 @@ export default createRule({ function checkSwitchUnnecessaryDefaultCase( switchMetadata: SwitchMetadata, ): void { - if (allowDefaultCaseForExhaustiveSwitch) { + if (allowDefaultCaseForExhaustiveSwitch || !switchMetadata.isUnion) { return; } 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 9a06ace1e0a9..c2cbd4b071b0 100644 --- a/packages/eslint-plugin/tests/rules/switch-exhaustiveness-check.test.ts +++ b/packages/eslint-plugin/tests/rules/switch-exhaustiveness-check.test.ts @@ -229,6 +229,27 @@ switch (value) { }, ], }, + // switch with default clause on non-union type + + // "allowDefaultCaseForExhaustiveSwitch" option + { + code: ` +declare const value: number; +switch (value) { + case 0: + return 0; + case 1: + return 1; + default: + return -1; +} + `, + options: [ + { + allowDefaultCaseForExhaustiveSwitch: false, + requireDefaultForNonUnion: false, + }, + ], + }, ], invalid: [ { From 8a48533c28333154e65d50db4ed0b396d60e9569 Mon Sep 17 00:00:00 2001 From: Zamiell <5511220+Zamiell@users.noreply.github.com> Date: Tue, 2 Jan 2024 17:33:51 -0500 Subject: [PATCH 2/7] chore: format --- .../src/rules/switch-exhaustiveness-check.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts index 605dc1c6bb9e..4c4789291c38 100644 --- a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts +++ b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts @@ -268,13 +268,17 @@ export default createRule({ function checkSwitchUnnecessaryDefaultCase( switchMetadata: SwitchMetadata, ): void { - if (allowDefaultCaseForExhaustiveSwitch || !switchMetadata.isUnion) { + if (allowDefaultCaseForExhaustiveSwitch) { return; } - const { missingBranchTypes, defaultCase } = switchMetadata; + const { missingBranchTypes, defaultCase, isUnion } = switchMetadata; - if (missingBranchTypes.length === 0 && defaultCase !== undefined) { + if ( + missingBranchTypes.length === 0 && + defaultCase !== undefined && + !isUnion + ) { context.report({ node: defaultCase, messageId: 'dangerousDefaultCase', From e018e9dbbfd2ffe25c1140c96e48232ee31cd735 Mon Sep 17 00:00:00 2001 From: Zamiell <5511220+Zamiell@users.noreply.github.com> Date: Thu, 4 Jan 2024 10:12:39 -0500 Subject: [PATCH 3/7] fix: add containsNonLiteralType --- .../src/rules/switch-exhaustiveness-check.ts | 67 ++++- .../rules/switch-exhaustiveness-check.test.ts | 266 +++++++++++++++++- 2 files changed, 329 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts index 4c4789291c38..2f6b331b183e 100644 --- a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts +++ b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts @@ -17,12 +17,14 @@ interface SwitchMetadata { readonly missingBranchTypes: ts.Type[]; readonly defaultCase: TSESTree.SwitchCase | undefined; readonly isUnion: boolean; + readonly containsNonLiteralType: boolean; } type Options = [ { /** - * If `true`, allow `default` cases on switch statements with exhaustive cases. + * If `true`, allow `default` cases on switch statements with exhaustive + * cases. * * @default true */ @@ -104,12 +106,16 @@ export default createRule({ | string | undefined; + const containsNonLiteralType = + doesTypeContainNonLiteralType(discriminantType); + if (!discriminantType.isUnion()) { return { symbolName, missingBranchTypes: [], defaultCase, isUnion: false, + containsNonLiteralType, }; } @@ -138,9 +144,57 @@ export default createRule({ missingBranchTypes, defaultCase, isUnion: true, + containsNonLiteralType, }; } + /** + * For example: + * + * - `"foo" | "bar"` is a type with all literal types. + * - `string` and `"foo" | number` are types that contain non-literal types. + * + * Default cases are never superfluous in switches with non-literal types. + */ + function doesTypeContainNonLiteralType(type: ts.Type): boolean { + const types = tsutils.unionTypeParts(type); + const typeNames = types.map(type => getTypeNameSpecific(type)); + + return typeNames.some( + typeName => + typeName === 'string' || + typeName === 'number' || + typeName === 'bigint' || + typeName === 'symbol', + ); + } + + /** + * Similar to the `getTypeName` function, but returns a more specific name. + * This is useful in differentiating between `string` and `"foo"`. + */ + function getTypeNameSpecific(type: ts.Type): string | undefined { + const escapedName = type.getSymbol()?.escapedName as string | undefined; + if (escapedName !== undefined && escapedName !== '__type') { + return escapedName; + } + + const aliasSymbolName = type.aliasSymbol?.getName(); + if (aliasSymbolName !== undefined) { + return aliasSymbolName; + } + + // The above checks do not work with boolean values. + if ('intrinsicName' in type) { + const { intrinsicName } = type; + if (typeof intrinsicName === 'string' && intrinsicName !== '') { + return intrinsicName; + } + } + + return undefined; + } + function checkSwitchExhaustive( node: TSESTree.SwitchStatement, switchMetadata: SwitchMetadata, @@ -272,12 +326,19 @@ export default createRule({ return; } - const { missingBranchTypes, defaultCase, isUnion } = switchMetadata; + const { missingBranchTypes, defaultCase, containsNonLiteralType } = + switchMetadata; + + /* + console.log('1:', missingBranchTypes.length === 0); + console.log('2:', defaultCase !== undefined); + console.log('3:', !containsNonLiteralType); + */ if ( missingBranchTypes.length === 0 && defaultCase !== undefined && - !isUnion + !containsNonLiteralType ) { context.report({ node: defaultCase, 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 c2cbd4b071b0..90affada064d 100644 --- a/packages/eslint-plugin/tests/rules/switch-exhaustiveness-check.test.ts +++ b/packages/eslint-plugin/tests/rules/switch-exhaustiveness-check.test.ts @@ -229,11 +229,94 @@ switch (value) { }, ], }, - // switch with default clause on non-union type + + // switch with default clause on string type + + // "allowDefaultCaseForExhaustiveSwitch" option + { + code: ` +declare const value: string; +switch (value) { + case 'foo': + return 0; + case 'bar': + return 1; + default: + return -1; +} + `, + options: [ + { + allowDefaultCaseForExhaustiveSwitch: false, + requireDefaultForNonUnion: false, + }, + ], + }, + // switch with default clause on number type + // "allowDefaultCaseForExhaustiveSwitch" option { code: ` declare const value: number; +switch (value) { + case 0: + return 0; + case 1: + return 1; + default: + return -1; +} + `, + options: [ + { + allowDefaultCaseForExhaustiveSwitch: false, + requireDefaultForNonUnion: false, + }, + ], + }, + // switch with default clause on bigint type + + // "allowDefaultCaseForExhaustiveSwitch" option + { + code: ` +declare const value: bigint; +switch (value) { + case 0: + return 0; + case 1: + return 1; + default: + return -1; +} + `, + options: [ + { + allowDefaultCaseForExhaustiveSwitch: false, + requireDefaultForNonUnion: false, + }, + ], + }, + // switch with default clause on symbol type + + // "allowDefaultCaseForExhaustiveSwitch" option + { + code: ` +declare const value: symbol; +const foo = Symbol('foo'); +switch (value) { + case foo: + return 0; + default: + return -1; +} + `, + options: [ + { + allowDefaultCaseForExhaustiveSwitch: false, + requireDefaultForNonUnion: false, + }, + ], + }, + // switch with default clause on union with number + + // "allowDefaultCaseForExhaustiveSwitch" option + { + code: ` +declare const value: 0 | 1 | number; switch (value) { case 0: return 0; @@ -756,6 +839,7 @@ switch (value) { ], }, { + // superfluous switch with a string-based union code: ` type MyUnion = 'foo' | 'bar' | 'baz'; @@ -767,6 +851,186 @@ switch (myUnion) { case 'baz': { break; } + default: { + break; + } +} + `, + options: [ + { + allowDefaultCaseForExhaustiveSwitch: false, + requireDefaultForNonUnion: false, + }, + ], + errors: [ + { + messageId: 'dangerousDefaultCase', + }, + ], + }, + { + // superfluous switch with a string-based enum + code: ` +enum MyEnum { + Foo = 'Foo', + Bar = 'Bar', + Baz = 'Baz', +} + +declare const myEnum: MyEnum; + +switch (myUnion) { + case MyEnum.Foo: + case MyEnum.Bar: + case MyEnum.Baz: { + break; + } + default: { + break; + } +} + `, + options: [ + { + allowDefaultCaseForExhaustiveSwitch: false, + requireDefaultForNonUnion: false, + }, + ], + errors: [ + { + messageId: 'dangerousDefaultCase', + }, + ], + }, + { + // superfluous switch with a number-based enum + code: ` +enum MyEnum { + Foo, + Bar, + Baz, +} + +declare const myEnum: MyEnum; + +switch (myUnion) { + case MyEnum.Foo: + case MyEnum.Bar: + case MyEnum.Baz: { + break; + } + default: { + break; + } +} + `, + options: [ + { + allowDefaultCaseForExhaustiveSwitch: false, + requireDefaultForNonUnion: false, + }, + ], + errors: [ + { + messageId: 'dangerousDefaultCase', + }, + ], + }, + { + // superfluous switch with a boolean + code: ` +declare const myBoolean: boolean; + +switch (myBoolean) { + case true: + case false: { + break; + } + default: { + break; + } +} + `, + options: [ + { + allowDefaultCaseForExhaustiveSwitch: false, + requireDefaultForNonUnion: false, + }, + ], + errors: [ + { + messageId: 'dangerousDefaultCase', + }, + ], + }, + { + // superfluous switch with undefined + code: ` +declare const myValue: undefined; + +switch (myValue) { + case undefined: { + break; + } + + default: { + break; + } +} + `, + options: [ + { + allowDefaultCaseForExhaustiveSwitch: false, + requireDefaultForNonUnion: false, + }, + ], + errors: [ + { + messageId: 'dangerousDefaultCase', + }, + ], + }, + { + // superfluous switch with null + code: ` +declare const myValue: null; + +switch (myValue) { + case null: { + break; + } + + default: { + break; + } +} + `, + options: [ + { + allowDefaultCaseForExhaustiveSwitch: false, + requireDefaultForNonUnion: false, + }, + ], + errors: [ + { + messageId: 'dangerousDefaultCase', + }, + ], + }, + { + // superfluous switch with union of various types + code: ` +declare const myValue: 'foo' | boolean | undefined | null; + +switch (myValue) { + case 'foo': + case true: + case false: + case undefined: + case null: { + break; + } + default: { break; } From 56c77117e227a061ed9c27a26f8e94ba56453834 Mon Sep 17 00:00:00 2001 From: Zamiell <5511220+Zamiell@users.noreply.github.com> Date: Thu, 4 Jan 2024 10:15:13 -0500 Subject: [PATCH 4/7] chore: cleanup --- .../eslint-plugin/src/rules/switch-exhaustiveness-check.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts index 2f6b331b183e..3adcf12c6f7a 100644 --- a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts +++ b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts @@ -329,12 +329,6 @@ export default createRule({ const { missingBranchTypes, defaultCase, containsNonLiteralType } = switchMetadata; - /* - console.log('1:', missingBranchTypes.length === 0); - console.log('2:', defaultCase !== undefined); - console.log('3:', !containsNonLiteralType); - */ - if ( missingBranchTypes.length === 0 && defaultCase !== undefined && From 8b29eb7cd9b78a27b9ea4f52f5570e058dc0a37c Mon Sep 17 00:00:00 2001 From: Zamiell <5511220+Zamiell@users.noreply.github.com> Date: Thu, 4 Jan 2024 17:15:23 -0500 Subject: [PATCH 5/7] refactor: use type flags instead of strings --- .../src/rules/switch-exhaustiveness-check.ts | 46 +++++-------------- .../rules/switch-exhaustiveness-check.test.ts | 4 +- 2 files changed, 13 insertions(+), 37 deletions(-) diff --git a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts index 3adcf12c6f7a..72d36a7b07c6 100644 --- a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts +++ b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts @@ -152,49 +152,21 @@ export default createRule({ * For example: * * - `"foo" | "bar"` is a type with all literal types. - * - `string` and `"foo" | number` are types that contain non-literal types. + * - `"foo" | number` is a types that contain non-literal types. * * Default cases are never superfluous in switches with non-literal types. */ function doesTypeContainNonLiteralType(type: ts.Type): boolean { const types = tsutils.unionTypeParts(type); - const typeNames = types.map(type => getTypeNameSpecific(type)); - - return typeNames.some( - typeName => - typeName === 'string' || - typeName === 'number' || - typeName === 'bigint' || - typeName === 'symbol', + return types.some( + type => + !isFlagSet( + type.getFlags(), + ts.TypeFlags.Literal | ts.TypeFlags.Undefined | ts.TypeFlags.Null, + ), ); } - /** - * Similar to the `getTypeName` function, but returns a more specific name. - * This is useful in differentiating between `string` and `"foo"`. - */ - function getTypeNameSpecific(type: ts.Type): string | undefined { - const escapedName = type.getSymbol()?.escapedName as string | undefined; - if (escapedName !== undefined && escapedName !== '__type') { - return escapedName; - } - - const aliasSymbolName = type.aliasSymbol?.getName(); - if (aliasSymbolName !== undefined) { - return aliasSymbolName; - } - - // The above checks do not work with boolean values. - if ('intrinsicName' in type) { - const { intrinsicName } = type; - if (typeof intrinsicName === 'string' && intrinsicName !== '') { - return intrinsicName; - } - } - - return undefined; - } - function checkSwitchExhaustive( node: TSESTree.SwitchStatement, switchMetadata: SwitchMetadata, @@ -381,3 +353,7 @@ export default createRule({ }; }, }); + +function isFlagSet(flags: number, flag: number): boolean { + return (flags & flag) !== 0; +} 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 90affada064d..5d58d0576052 100644 --- a/packages/eslint-plugin/tests/rules/switch-exhaustiveness-check.test.ts +++ b/packages/eslint-plugin/tests/rules/switch-exhaustiveness-check.test.ts @@ -879,7 +879,7 @@ enum MyEnum { declare const myEnum: MyEnum; -switch (myUnion) { +switch (myEnum) { case MyEnum.Foo: case MyEnum.Bar: case MyEnum.Baz: { @@ -913,7 +913,7 @@ enum MyEnum { declare const myEnum: MyEnum; -switch (myUnion) { +switch (myEnum) { case MyEnum.Foo: case MyEnum.Bar: case MyEnum.Baz: { From 208434ca79fd3dbaeca02c4d3e0e6a07b07b4854 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Mon, 8 Jan 2024 11:28:05 -0500 Subject: [PATCH 6/7] Update packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts --- packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts | 2 +- 1 file changed, 1 insertion(+), 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 72d36a7b07c6..5c85f9b0c410 100644 --- a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts +++ b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts @@ -152,7 +152,7 @@ export default createRule({ * For example: * * - `"foo" | "bar"` is a type with all literal types. - * - `"foo" | number` is a types that contain non-literal types. + * - `"foo" | number` is a type that contain non-literal types. * * Default cases are never superfluous in switches with non-literal types. */ From 1ad227a086d14f53ff79743f89f95b6049973d32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Mon, 8 Jan 2024 11:28:18 -0500 Subject: [PATCH 7/7] Update packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts --- packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts | 2 +- 1 file changed, 1 insertion(+), 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 5c85f9b0c410..c6ed86463ca0 100644 --- a/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts +++ b/packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts @@ -152,7 +152,7 @@ export default createRule({ * For example: * * - `"foo" | "bar"` is a type with all literal types. - * - `"foo" | number` is a type that contain non-literal types. + * - `"foo" | number` is a type that contains non-literal types. * * Default cases are never superfluous in switches with non-literal types. */