Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(eslint-plugin): [switch-exhaustiveness-check] fix new allowDefaultCaseForExhaustiveSwitch option #8176

Merged
merged 9 commits into from
Jan 8, 2024
41 changes: 38 additions & 3 deletions packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Zamiell marked this conversation as resolved.
Show resolved Hide resolved
*
* @default true
*/
Expand Down Expand Up @@ -104,12 +106,16 @@ export default createRule<Options, MessageIds>({
| string
| undefined;

const containsNonLiteralType =
doesTypeContainNonLiteralType(discriminantType);

if (!discriminantType.isUnion()) {
return {
symbolName,
missingBranchTypes: [],
defaultCase,
isUnion: false,
containsNonLiteralType,
};
}

Expand Down Expand Up @@ -138,9 +144,29 @@ export default createRule<Options, MessageIds>({
missingBranchTypes,
defaultCase,
isUnion: true,
containsNonLiteralType,
};
}

/**
* For example:
*
* - `"foo" | "bar"` is a type with all literal types.
* - `"foo" | number` is a type that contains 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);
return types.some(
type =>
!isFlagSet(
type.getFlags(),
ts.TypeFlags.Literal | ts.TypeFlags.Undefined | ts.TypeFlags.Null,
),
);
}

function checkSwitchExhaustive(
node: TSESTree.SwitchStatement,
switchMetadata: SwitchMetadata,
Expand Down Expand Up @@ -272,9 +298,14 @@ export default createRule<Options, MessageIds>({
return;
}

const { missingBranchTypes, defaultCase } = switchMetadata;
const { missingBranchTypes, defaultCase, containsNonLiteralType } =
switchMetadata;

if (missingBranchTypes.length === 0 && defaultCase !== undefined) {
if (
missingBranchTypes.length === 0 &&
defaultCase !== undefined &&
!containsNonLiteralType
) {
context.report({
node: defaultCase,
messageId: 'dangerousDefaultCase',
Expand Down Expand Up @@ -322,3 +353,7 @@ export default createRule<Options, MessageIds>({
};
},
});

function isFlagSet(flags: number, flag: number): boolean {
return (flags & flag) !== 0;
}
285 changes: 285 additions & 0 deletions packages/eslint-plugin/tests/rules/switch-exhaustiveness-check.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,110 @@ switch (value) {
},
],
},
// 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;
case 1:
return 1;
default:
return -1;
}
`,
options: [
{
allowDefaultCaseForExhaustiveSwitch: false,
requireDefaultForNonUnion: false,
},
],
},
],
invalid: [
{
Expand Down Expand Up @@ -735,6 +839,7 @@ switch (value) {
],
},
{
// superfluous switch with a string-based union
code: `
type MyUnion = 'foo' | 'bar' | 'baz';

Expand All @@ -746,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 (myEnum) {
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 (myEnum) {
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;
}
Expand Down