From a8fb966786eb31044d2a81578540490a1d6d987f Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Fri, 1 Apr 2022 14:41:04 +0800 Subject: [PATCH] Add `no-useless-switch-case` rule (#1779) --- configs/recommended.js | 1 + docs/rules/no-useless-switch-case.md | 67 ++++ readme.md | 1 + rules/ast/index.js | 4 + rules/ast/is-empty-node.js | 20 + rules/no-empty-file.js | 12 +- rules/no-useless-switch-case.js | 71 ++++ rules/utils/get-switch-case-head-location.js | 21 + test/no-useless-switch-case.mjs | 179 +++++++++ test/snapshots/no-useless-switch-case.mjs.md | 365 ++++++++++++++++++ .../snapshots/no-useless-switch-case.mjs.snap | Bin 0 -> 784 bytes 11 files changed, 733 insertions(+), 8 deletions(-) create mode 100644 docs/rules/no-useless-switch-case.md create mode 100644 rules/ast/index.js create mode 100644 rules/ast/is-empty-node.js create mode 100644 rules/no-useless-switch-case.js create mode 100644 rules/utils/get-switch-case-head-location.js create mode 100644 test/no-useless-switch-case.mjs create mode 100644 test/snapshots/no-useless-switch-case.mjs.md create mode 100644 test/snapshots/no-useless-switch-case.mjs.snap diff --git a/configs/recommended.js b/configs/recommended.js index d076d0418e..1c83f44a2c 100644 --- a/configs/recommended.js +++ b/configs/recommended.js @@ -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', diff --git a/docs/rules/no-useless-switch-case.md b/docs/rules/no-useless-switch-case.md new file mode 100644 index 0000000000..c2fdc9959c --- /dev/null +++ b/docs/rules/no-useless-switch-case.md @@ -0,0 +1,67 @@ +# Disallow useless case in switch statements + + + +βœ… *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).* + + +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; +} +``` diff --git a/readme.md b/readme.md index e52943b639..bb77e05536 100644 --- a/readme.md +++ b/readme.md @@ -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. | βœ… | πŸ”§ | | diff --git a/rules/ast/index.js b/rules/ast/index.js new file mode 100644 index 0000000000..10cb02152d --- /dev/null +++ b/rules/ast/index.js @@ -0,0 +1,4 @@ +'use strict'; +module.exports = { + isEmptyNode: require('./is-empty-node.js'), +}; diff --git a/rules/ast/is-empty-node.js b/rules/ast/is-empty-node.js new file mode 100644 index 0000000000..833b8dfd60 --- /dev/null +++ b/rules/ast/is-empty-node.js @@ -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; diff --git a/rules/no-empty-file.js b/rules/no-empty-file.js index bc3c1bafaf..20fbf5f065 100644 --- a/rules/no-empty-file.js +++ b/rules/no-empty-file.js @@ -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('/'); @@ -29,7 +25,7 @@ const create = context => { return { Program(node) { - if (!isEmpty(node)) { + if (node.body.some(node => !isEmpty(node))) { return; } diff --git a/rules/no-useless-switch-case.js b/rules/no-useless-switch-case.js new file mode 100644 index 0000000000..bc1db10f76 --- /dev/null +++ b/rules/no-useless-switch-case.js @@ -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, + }, +}; diff --git a/rules/utils/get-switch-case-head-location.js b/rules/utils/get-switch-case-head-location.js new file mode 100644 index 0000000000..52fb1f0bad --- /dev/null +++ b/rules/utils/get-switch-case-head-location.js @@ -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; diff --git a/test/no-useless-switch-case.mjs b/test/no-useless-switch-case.mjs new file mode 100644 index 0000000000..e3d87a4fe2 --- /dev/null +++ b/test/no-useless-switch-case.mjs @@ -0,0 +1,179 @@ +import outdent from 'outdent'; +import {getTester} from './utils/test.mjs'; + +const {test} = getTester(import.meta); + +test.snapshot({ + valid: [ + outdent` + switch (foo) { + case a: + case b: + handleDefaultCase(); + break; + } + `, + outdent` + switch (foo) { + case a: + handleCaseA(); + break; + default: + handleDefaultCase(); + break; + } + `, + outdent` + switch (foo) { + case a: + handleCaseA(); + default: + handleDefaultCase(); + break; + } + `, + outdent` + switch (foo) { + case a: + break; + default: + handleDefaultCase(); + break; + } + `, + outdent` + switch (foo) { + case a: + handleCaseA(); + // Fallthrough + default: + handleDefaultCase(); + break; + } + `, + outdent` + switch (foo) { + case a: + default: + handleDefaultCase(); + break; + case b: + handleCaseB(); + break; + } + `, + outdent` + switch (1) { + // This is not useless + case 1: + default: + console.log('1') + case 1: + console.log('2') + } + `, + ], + invalid: [ + outdent` + switch (foo) { + case a: + default: + handleDefaultCase(); + break; + } + `, + outdent` + switch (foo) { + case a: { + } + default: + handleDefaultCase(); + break; + } + `, + outdent` + switch (foo) { + case a: { + ;; + { + ;; + { + ;; + } + } + } + default: + handleDefaultCase(); + break; + } + `, + outdent` + switch (foo) { + case a: + case (( b )) : + default: + handleDefaultCase(); + break; + } + `, + outdent` + switch (foo) { + case a: + case b: + handleCaseAB(); + break; + case d: + case d: + default: + handleDefaultCase(); + break; + } + `, + outdent` + switch (foo) { + case a: + case b: + default: + handleDefaultCase(); + break; + } + `, + outdent` + switch (foo) { + // eslint-disable-next-line + case a: + case b: + default: + handleDefaultCase(); + break; + } + `, + outdent` + switch (foo) { + case a: + // eslint-disable-next-line + case b: + default: + handleDefaultCase(); + break; + } + `, + ], +}); + +test.typescript({ + valid: [ + outdent` + switch (1) { + default: + handleDefaultCase1(); + break; + case 1: + default: + handleDefaultCase2(); + break; + } + `, + ], + invalid: [], +}); diff --git a/test/snapshots/no-useless-switch-case.mjs.md b/test/snapshots/no-useless-switch-case.mjs.md new file mode 100644 index 0000000000..7a9e243a13 --- /dev/null +++ b/test/snapshots/no-useless-switch-case.mjs.md @@ -0,0 +1,365 @@ +# Snapshot report for `test/no-useless-switch-case.mjs` + +The actual snapshot is saved in `no-useless-switch-case.mjs.snap`. + +Generated by [AVA](https://avajs.dev). + +## Invalid #1 + 1 | switch (foo) { + 2 | case a: + 3 | default: + 4 | handleDefaultCase(); + 5 | break; + 6 | } + +> Error 1/1 + + `␊ + 1 | switch (foo) {␊ + > 2 | case a:␊ + | ^^^^^^^ Useless case in switch statement.␊ + 3 | default:␊ + 4 | handleDefaultCase();␊ + 5 | break;␊ + 6 | }␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove this case.␊ + 1 | switch (foo) {␊ + 2 | ␊ + 3 | default:␊ + 4 | handleDefaultCase();␊ + 5 | break;␊ + 6 | }␊ + ` + +## Invalid #2 + 1 | switch (foo) { + 2 | case a: { + 3 | } + 4 | default: + 5 | handleDefaultCase(); + 6 | break; + 7 | } + +> Error 1/1 + + `␊ + 1 | switch (foo) {␊ + > 2 | case a: {␊ + | ^^^^^^^ Useless case in switch statement.␊ + 3 | }␊ + 4 | default:␊ + 5 | handleDefaultCase();␊ + 6 | break;␊ + 7 | }␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove this case.␊ + 1 | switch (foo) {␊ + 2 | ␊ + 3 | default:␊ + 4 | handleDefaultCase();␊ + 5 | break;␊ + 6 | }␊ + ` + +## Invalid #3 + 1 | switch (foo) { + 2 | case a: { + 3 | ;; + 4 | { + 5 | ;; + 6 | { + 7 | ;; + 8 | } + 9 | } + 10 | } + 11 | default: + 12 | handleDefaultCase(); + 13 | break; + 14 | } + +> Error 1/1 + + `␊ + 1 | switch (foo) {␊ + > 2 | case a: {␊ + | ^^^^^^^ Useless case in switch statement.␊ + 3 | ;;␊ + 4 | {␊ + 5 | ;;␊ + 6 | {␊ + 7 | ;;␊ + 8 | }␊ + 9 | }␊ + 10 | }␊ + 11 | default:␊ + 12 | handleDefaultCase();␊ + 13 | break;␊ + 14 | }␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove this case.␊ + 1 | switch (foo) {␊ + 2 | ␊ + 3 | default:␊ + 4 | handleDefaultCase();␊ + 5 | break;␊ + 6 | }␊ + ` + +## Invalid #4 + 1 | switch (foo) { + 2 | case a: + 3 | case (( b )) : + 4 | default: + 5 | handleDefaultCase(); + 6 | break; + 7 | } + +> Error 1/2 + + `␊ + 1 | switch (foo) {␊ + > 2 | case a:␊ + | ^^^^^^^ Useless case in switch statement.␊ + 3 | case (( b )) :␊ + 4 | default:␊ + 5 | handleDefaultCase();␊ + 6 | break;␊ + 7 | }␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove this case.␊ + 1 | switch (foo) {␊ + 2 | ␊ + 3 | case (( b )) :␊ + 4 | default:␊ + 5 | handleDefaultCase();␊ + 6 | break;␊ + 7 | }␊ + ` + +> Error 2/2 + + `␊ + 1 | switch (foo) {␊ + 2 | case a:␊ + > 3 | case (( b )) :␊ + | ^^^^^^^^^^^^^^^^^^^^^^ Useless case in switch statement.␊ + 4 | default:␊ + 5 | handleDefaultCase();␊ + 6 | break;␊ + 7 | }␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove this case.␊ + 1 | switch (foo) {␊ + 2 | case a:␊ + 3 | ␊ + 4 | default:␊ + 5 | handleDefaultCase();␊ + 6 | break;␊ + 7 | }␊ + ` + +## Invalid #5 + 1 | switch (foo) { + 2 | case a: + 3 | case b: + 4 | handleCaseAB(); + 5 | break; + 6 | case d: + 7 | case d: + 8 | default: + 9 | handleDefaultCase(); + 10 | break; + 11 | } + +> Error 1/2 + + `␊ + 1 | switch (foo) {␊ + 2 | case a:␊ + 3 | case b:␊ + 4 | handleCaseAB();␊ + 5 | break;␊ + > 6 | case d:␊ + | ^^^^^^^ Useless case in switch statement.␊ + 7 | case d:␊ + 8 | default:␊ + 9 | handleDefaultCase();␊ + 10 | break;␊ + 11 | }␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove this case.␊ + 1 | switch (foo) {␊ + 2 | case a:␊ + 3 | case b:␊ + 4 | handleCaseAB();␊ + 5 | break;␊ + 6 | ␊ + 7 | case d:␊ + 8 | default:␊ + 9 | handleDefaultCase();␊ + 10 | break;␊ + 11 | }␊ + ` + +> Error 2/2 + + `␊ + 1 | switch (foo) {␊ + 2 | case a:␊ + 3 | case b:␊ + 4 | handleCaseAB();␊ + 5 | break;␊ + 6 | case d:␊ + > 7 | case d:␊ + | ^^^^^^^ Useless case in switch statement.␊ + 8 | default:␊ + 9 | handleDefaultCase();␊ + 10 | break;␊ + 11 | }␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove this case.␊ + 1 | switch (foo) {␊ + 2 | case a:␊ + 3 | case b:␊ + 4 | handleCaseAB();␊ + 5 | break;␊ + 6 | case d:␊ + 7 | ␊ + 8 | default:␊ + 9 | handleDefaultCase();␊ + 10 | break;␊ + 11 | }␊ + ` + +## Invalid #6 + 1 | switch (foo) { + 2 | case a: + 3 | case b: + 4 | default: + 5 | handleDefaultCase(); + 6 | break; + 7 | } + +> Error 1/2 + + `␊ + 1 | switch (foo) {␊ + > 2 | case a:␊ + | ^^^^^^^ Useless case in switch statement.␊ + 3 | case b:␊ + 4 | default:␊ + 5 | handleDefaultCase();␊ + 6 | break;␊ + 7 | }␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove this case.␊ + 1 | switch (foo) {␊ + 2 | ␊ + 3 | case b:␊ + 4 | default:␊ + 5 | handleDefaultCase();␊ + 6 | break;␊ + 7 | }␊ + ` + +> Error 2/2 + + `␊ + 1 | switch (foo) {␊ + 2 | case a:␊ + > 3 | case b:␊ + | ^^^^^^^ Useless case in switch statement.␊ + 4 | default:␊ + 5 | handleDefaultCase();␊ + 6 | break;␊ + 7 | }␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove this case.␊ + 1 | switch (foo) {␊ + 2 | case a:␊ + 3 | ␊ + 4 | default:␊ + 5 | handleDefaultCase();␊ + 6 | break;␊ + 7 | }␊ + ` + +## Invalid #7 + 1 | switch (foo) { + 2 | // eslint-disable-next-line + 3 | case a: + 4 | case b: + 5 | default: + 6 | handleDefaultCase(); + 7 | break; + 8 | } + +> Error 1/1 + + `␊ + 1 | switch (foo) {␊ + 2 | // eslint-disable-next-line␊ + 3 | case a:␊ + > 4 | case b:␊ + | ^^^^^^^ Useless case in switch statement.␊ + 5 | default:␊ + 6 | handleDefaultCase();␊ + 7 | break;␊ + 8 | }␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove this case.␊ + 1 | switch (foo) {␊ + 2 | // eslint-disable-next-line␊ + 3 | case a:␊ + 4 | ␊ + 5 | default:␊ + 6 | handleDefaultCase();␊ + 7 | break;␊ + 8 | }␊ + ` + +## Invalid #8 + 1 | switch (foo) { + 2 | case a: + 3 | // eslint-disable-next-line + 4 | case b: + 5 | default: + 6 | handleDefaultCase(); + 7 | break; + 8 | } + +> Error 1/1 + + `␊ + 1 | switch (foo) {␊ + > 2 | case a:␊ + | ^^^^^^^ Useless case in switch statement.␊ + 3 | // eslint-disable-next-line␊ + 4 | case b:␊ + 5 | default:␊ + 6 | handleDefaultCase();␊ + 7 | break;␊ + 8 | }␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove this case.␊ + 1 | switch (foo) {␊ + 2 | ␊ + 3 | // eslint-disable-next-line␊ + 4 | case b:␊ + 5 | default:␊ + 6 | handleDefaultCase();␊ + 7 | break;␊ + 8 | }␊ + ` diff --git a/test/snapshots/no-useless-switch-case.mjs.snap b/test/snapshots/no-useless-switch-case.mjs.snap new file mode 100644 index 0000000000000000000000000000000000000000..bedd6572d431388c3fc43278cc406b154c0d7e5d GIT binary patch literal 784 zcmV+r1MmDnRzV4 zFdUefwl;L;lneJS9lp1 z9vSc9&N;YM`P0w#La%jEcNoEJl?K4WBM5M=6Qt|9X?l8UaY<%=o`Rvip_M{V zYHog6szOOdCd6xc*uzEv5;nB(BT;dqfEqV;=*}QHs)k?OAkwWdG^(f^H~F;9H%JaO z!RJs@REL@k%E+LKr%*!wGX9*Qudk3=oRgVXqMMRgoS2l8s+*TuQKAbJNkz%D;LKw} zVdgPK^O_kxubHEI&0O3u0S>*&wE&0f-4^8KQ-Tp%K2|Ff>LD4?~kdnJ%cGk5|*7 z^`W7mkfflgsQ?2bpfN%lFB27Wc-kh|T!kF3XkfJELYtNguDX*-c@BT&fmYcLzq)gP zf&nFltB8tWXw6AZ9M=xZoJalYO^*pjBSrxuvt#t(kgW#U>F5Np71dfhXxM@^rdW~J z7BWXO!2&H5piLZHjTZy7#*5(q#x6|*8`dPGS(wo|@hG4r9y?5Lk)D`F6Ay8L4NF5* O3o`(mEeAry6aWAqYF#t{ literal 0 HcmV?d00001