Skip to content

feat(eslint-plugin): [switch-exhaustiveness-check] add support for "no default" comment #10218

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions packages/eslint-plugin/docs/rules/switch-exhaustiveness-check.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,27 @@ switch (literal) {
}
```

### `defaultCaseCommentPattern`

{/* insert option description */}

Default: `/^no default$/iu`.

It can sometimes be preferable to omit the default case for only some switch statements.
For those situations, this rule can be given a pattern for a comment that's allowed to take the place of a `default:`.

Examples of additional **correct** code with `{ defaultCaseCommentPattern: "^skip\\sdefault" }`:

```ts option='{ "defaultCaseCommentPattern": "^skip default" }' showPlaygroundButton
declare const value: 'a' | 'b';

switch (value) {
case 'a':
break;
// skip default
}
```

## Examples

When the switch doesn't have exhaustive cases, either filling them all out or adding a default (if you have `considerDefaultExhaustiveForUnions` enabled) will address the rule's complaint.
Expand Down
41 changes: 37 additions & 4 deletions packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ import {
requiresQuoting,
} from '../util';

const DEFAULT_COMMENT_PATTERN = /^no default$/iu;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's a default value, please specify it in defaultOptions.

Copy link
Contributor Author

@developer-bandi developer-bandi Dec 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

�In order to put a default value in defaultOptions, the defaultCaseCommentPattern option must be able to receive a flag. Therefore, this option should be in the form of an array, with an expression at the first index and a flag at the second index.

defaultCaseCommentPattern:["no default", "iu"]

Previously, the default value was treated as a separate variable because of equivalence with eslint rules. Would there be any problem with implementing it like this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As much as I would love to keep things to the defaultOptions plumbing, I don't think this is doable. I think this PR is fine as-is.


interface SwitchMetadata {
readonly containsNonLiteralType: boolean;
readonly defaultCase: TSESTree.SwitchCase | undefined;
readonly defaultCase: TSESTree.Comment | TSESTree.SwitchCase | undefined;
readonly missingLiteralBranchTypes: ts.Type[];
readonly symbolName: string | undefined;
}
Expand All @@ -38,6 +40,11 @@ type Options = [
*/
requireDefaultForNonUnion?: boolean;

/**
* Regular expression for a comment that can indicate an intentionally omitted default case.
*/
defaultCaseCommentPattern?: string;

/**
* If `true`, the `default` clause is used to determine whether the switch statement is exhaustive for union types.
*
Expand Down Expand Up @@ -81,6 +88,10 @@ export default createRule<Options, MessageIds>({
type: 'boolean',
description: `If 'true', the 'default' clause is used to determine whether the switch statement is exhaustive for union type`,
},
defaultCaseCommentPattern: {
type: 'string',
description: `Regular expression for a comment that can indicate an intentionally omitted default case.`,
},
requireDefaultForNonUnion: {
type: 'boolean',
description: `If 'true', require a 'default' clause for switches on non-union types.`,
Expand All @@ -102,13 +113,34 @@ export default createRule<Options, MessageIds>({
{
allowDefaultCaseForExhaustiveSwitch,
considerDefaultExhaustiveForUnions,
defaultCaseCommentPattern,
requireDefaultForNonUnion,
},
],
) {
const services = getParserServices(context);
const checker = services.program.getTypeChecker();
const compilerOptions = services.program.getCompilerOptions();
const commentRegExp =
defaultCaseCommentPattern != null
? new RegExp(defaultCaseCommentPattern, 'u')
: DEFAULT_COMMENT_PATTERN;

function getCommentDefaultCase(
node: TSESTree.SwitchStatement,
): TSESTree.Comment | undefined {
const lastCase = node.cases.at(-1);
const commentsAfterLastCase = lastCase
? context.sourceCode.getCommentsAfter(lastCase)
: [];
const defaultCaseComment = commentsAfterLastCase.at(-1);

if (commentRegExp.test(defaultCaseComment?.value.trim() || '')) {
return defaultCaseComment;
}

return;
}

function getSwitchMetadata(node: TSESTree.SwitchStatement): SwitchMetadata {
const defaultCase = node.cases.find(
Expand Down Expand Up @@ -170,7 +202,7 @@ export default createRule<Options, MessageIds>({

return {
containsNonLiteralType,
defaultCase,
defaultCase: defaultCase ?? getCommentDefaultCase(node),
missingLiteralBranchTypes,
symbolName,
};
Expand Down Expand Up @@ -210,6 +242,7 @@ export default createRule<Options, MessageIds>({
fixer,
node,
missingLiteralBranchTypes,
defaultCase,
symbolName?.toString(),
);
},
Expand All @@ -223,11 +256,11 @@ export default createRule<Options, MessageIds>({
fixer: TSESLint.RuleFixer,
node: TSESTree.SwitchStatement,
missingBranchTypes: (ts.Type | null)[], // null means default branch
defaultCase: TSESTree.Comment | TSESTree.SwitchCase | undefined,
symbolName?: string,
): TSESLint.RuleFix {
const lastCase =
node.cases.length > 0 ? node.cases[node.cases.length - 1] : null;
const defaultCase = node.cases.find(caseEl => caseEl.test == null);

const caseIndent = lastCase
? ' '.repeat(lastCase.loc.start.column)
Expand Down Expand Up @@ -349,7 +382,7 @@ export default createRule<Options, MessageIds>({
{
messageId: 'addMissingCases',
fix(fixer): TSESLint.RuleFix {
return fixSwitch(fixer, node, [null]);
return fixSwitch(fixer, node, [null], defaultCase);
},
},
],
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

150 changes: 150 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 @@ -834,6 +834,7 @@ switch (literal) {
options: [
{
considerDefaultExhaustiveForUnions: true,
requireDefaultForNonUnion: true,
},
],
},
Expand Down Expand Up @@ -954,6 +955,54 @@ function foo(x: string[], y: string | undefined) {
},
},
},
{
code: `
declare const value: number;
switch (value) {
case 0:
break;
case 1:
break;
// no default
}
`,
options: [
{
requireDefaultForNonUnion: true,
},
],
},
{
code: `
declare const value: 'a' | 'b';
switch (value) {
case 'a':
break;
// no default
}
`,
options: [
{
considerDefaultExhaustiveForUnions: true,
},
],
},
{
code: `
declare const value: 'a' | 'b';
switch (value) {
case 'a':
break;
// skip default
}
`,
options: [
{
considerDefaultExhaustiveForUnions: true,
defaultCaseCommentPattern: '^skip\\sdefault',
},
],
},
],
invalid: [
{
Expand Down Expand Up @@ -2797,5 +2846,106 @@ function foo(x: string[]) {
},
},
},
{
code: `
declare const myValue: 'a' | 'b';
switch (myValue) {
case 'a':
return 'a';
case 'b':
return 'b';
// no default
}
`,
errors: [
{
messageId: 'dangerousDefaultCase',
},
],
options: [
{
allowDefaultCaseForExhaustiveSwitch: false,
},
],
},
{
code: `
declare const literal: 'a' | 'b' | 'c';

switch (literal) {
case 'a':
break;
// no default
}
`,
errors: [
{
column: 9,
line: 4,
messageId: 'switchIsNotExhaustive',
suggestions: [
{
messageId: 'addMissingCases',
output: `
declare const literal: 'a' | 'b' | 'c';

switch (literal) {
case 'a':
break;
case "b": { throw new Error('Not implemented yet: "b" case') }
case "c": { throw new Error('Not implemented yet: "c" case') }
// no default
}
`,
},
],
},
],
options: [
{
considerDefaultExhaustiveForUnions: false,
},
],
},
{
code: `
declare const literal: 'a' | 'b' | 'c';

switch (literal) {
case 'a':
break;
// skip default
}
`,
errors: [
{
column: 9,
line: 4,
messageId: 'switchIsNotExhaustive',
suggestions: [
{
messageId: 'addMissingCases',
output: `
declare const literal: 'a' | 'b' | 'c';

switch (literal) {
case 'a':
break;
case "b": { throw new Error('Not implemented yet: "b" case') }
case "c": { throw new Error('Not implemented yet: "c" case') }
// skip default
}
`,
},
],
},
],
options: [
{
considerDefaultExhaustiveForUnions: false,
defaultCaseCommentPattern: '^skip\\sdefault',
},
],
},
],
});

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading