Skip to content

Commit

Permalink
Add no-useless-switch-case rule (#1779)
Browse files Browse the repository at this point in the history
  • Loading branch information
fisker authored Apr 1, 2022
1 parent 6dfdeb0 commit a8fb966
Show file tree
Hide file tree
Showing 11 changed files with 733 additions and 8 deletions.
1 change: 1 addition & 0 deletions configs/recommended.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ module.exports = {
'unicorn/no-useless-length-check': 'error',
'unicorn/no-useless-promise-resolve-reject': 'error',
'unicorn/no-useless-spread': 'error',
'unicorn/no-useless-switch-case': 'error',
'unicorn/no-useless-undefined': 'error',
'unicorn/no-zero-fractions': 'error',
'unicorn/number-literal-case': 'error',
Expand Down
67 changes: 67 additions & 0 deletions docs/rules/no-useless-switch-case.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# Disallow useless case in switch statements

<!-- Do not manually modify RULE_NOTICE part. Run: `npm run generate-rule-notices` -->
<!-- RULE_NOTICE -->
*This rule is part of the [recommended](https://github.com/sindresorhus/eslint-plugin-unicorn#recommended-config) config.*

💡 *This rule provides [suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions).*
<!-- /RULE_NOTICE -->

An empty case before the last default case is useless.

## Fail

```js
switch (foo) {
case 1:
default:
handleDefaultCase();
break;
}
```

## Pass

```js
switch (foo) {
case 1:
case 2:
handleCase1And2();
break;
}
```

```js
switch (foo) {
case 1:
handleCase1();
break;
default:
handleDefaultCase();
break;
}
```

```js
switch (foo) {
case 1:
handleCase1();
// Fallthrough
default:
handleDefaultCase();
break;
}
```

```js
switch (foo) {
// This is actually useless, but we only check cases where the last case is the `default` case
case 1:
default:
handleDefaultCase();
break;
case 2:
handleCase2();
break;
}
```
1 change: 1 addition & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ Each rule has emojis denoting:
| [no-useless-length-check](docs/rules/no-useless-length-check.md) | Disallow useless array length check. || 🔧 | |
| [no-useless-promise-resolve-reject](docs/rules/no-useless-promise-resolve-reject.md) | Disallow returning/yielding `Promise.resolve/reject()` in async functions or promise callbacks || 🔧 | |
| [no-useless-spread](docs/rules/no-useless-spread.md) | Disallow unnecessary spread. || 🔧 | |
| [no-useless-switch-case](docs/rules/no-useless-switch-case.md) | Disallow useless case in switch statements. || | 💡 |
| [no-useless-undefined](docs/rules/no-useless-undefined.md) | Disallow useless `undefined`. || 🔧 | |
| [no-zero-fractions](docs/rules/no-zero-fractions.md) | Disallow number literals with zero fractions or dangling dots. || 🔧 | |
| [number-literal-case](docs/rules/number-literal-case.md) | Enforce proper case for numeric literals. || 🔧 | |
Expand Down
4 changes: 4 additions & 0 deletions rules/ast/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
'use strict';
module.exports = {
isEmptyNode: require('./is-empty-node.js'),
};
20 changes: 20 additions & 0 deletions rules/ast/is-empty-node.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
'use strict';
function isEmptyNode(node, additionalEmpty) {
const {type} = node;

if (type === 'BlockStatement') {
return node.body.every(currentNode => isEmptyNode(currentNode, additionalEmpty));
}

if (type === 'EmptyStatement') {
return true;
}

if (additionalEmpty && additionalEmpty(node)) {
return true;
}

return false;
}

module.exports = isEmptyNode;
12 changes: 4 additions & 8 deletions rules/no-empty-file.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,13 @@
'use strict';
const {isEmptyNode} = require('./ast/index.js');

const MESSAGE_ID = 'no-empty-file';
const messages = {
[MESSAGE_ID]: 'Empty files are not allowed.',
};

const isEmpty = node =>
(
(node.type === 'Program' || node.type === 'BlockStatement')
&& node.body.every(currentNode => isEmpty(currentNode))
)
|| node.type === 'EmptyStatement'
|| (node.type === 'ExpressionStatement' && 'directive' in node);
const isDirective = node => node.type === 'ExpressionStatement' && 'directive' in node;
const isEmpty = node => isEmptyNode(node, isDirective);

const isTripleSlashDirective = node =>
node.type === 'Line' && node.value.startsWith('/');
Expand All @@ -29,7 +25,7 @@ const create = context => {

return {
Program(node) {
if (!isEmpty(node)) {
if (node.body.some(node => !isEmpty(node))) {
return;
}

Expand Down
71 changes: 71 additions & 0 deletions rules/no-useless-switch-case.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
'use strict';
const {isEmptyNode} = require('./ast/index.js');
const getSwitchCaseHeadLocation = require('./utils/get-switch-case-head-location.js');

const MESSAGE_ID_ERROR = 'no-useless-switch-case/error';
const MESSAGE_ID_SUGGESTION = 'no-useless-switch-case/suggestion';
const messages = {
[MESSAGE_ID_ERROR]: 'Useless case in switch statement.',
[MESSAGE_ID_SUGGESTION]: 'Remove this case.',
};

const isEmptySwitchCase = node => node.consequent.every(node => isEmptyNode(node));

/** @param {import('eslint').Rule.RuleContext} context */
const create = context => ({
* 'SwitchStatement[cases.length>1]'(switchStatement) {
const {cases} = switchStatement;

// TypeScript allows multiple `default` cases
const defaultCases = cases.filter(switchCase => switchCase.test === null);
if (defaultCases.length !== 1) {
return;
}

const [defaultCase] = defaultCases;

// We only check cases where the last case is the `default` case
if (defaultCase !== cases[cases.length - 1]) {
return;
}

const uselessCases = [];

for (let index = cases.length - 2; index >= 0; index--) {
const node = cases[index];
if (isEmptySwitchCase(node)) {
uselessCases.unshift(node);
} else {
break;
}
}

for (const node of uselessCases) {
yield {
node,
loc: getSwitchCaseHeadLocation(node, context.getSourceCode()),
messageId: MESSAGE_ID_ERROR,
suggest: [
{
messageId: MESSAGE_ID_SUGGESTION,
/** @param {import('eslint').Rule.RuleFixer} fixer */
fix: fixer => fixer.remove(node),
},
],
};
}
},
});

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
create,
meta: {
type: 'suggestion',
docs: {
description: 'Disallow useless case in switch statements.',
},
hasSuggestions: true,
messages,
},
};
21 changes: 21 additions & 0 deletions rules/utils/get-switch-case-head-location.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
'use strict';

const {isColonToken} = require('eslint-utils');

/**
@typedef {line: number, column: number} Position
Get the location of the given `SwitchCase` node for reporting.
@param {Node} node - The `SwitchCase` node to get.
@param {SourceCode} sourceCode - The source code object to get tokens from.
@returns {{start: Position, end: Position}} The location of the class node for reporting.
*/
function getSwitchCaseHeadLocation(node, sourceCode) {
const startToken = node.test || sourceCode.getFirstToken(node);
const colonToken = sourceCode.getTokenAfter(startToken, isColonToken);

return {start: node.loc.start, end: colonToken.loc.end};
}

module.exports = getSwitchCaseHeadLocation;
Loading

0 comments on commit a8fb966

Please sign in to comment.