From 9f922d77ae64abfc3c0f1ac6729abf186670a258 Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Mon, 16 Dec 2019 02:21:42 +0100 Subject: [PATCH] New: Add default-case-last rule (fixes #12665) --- docs/rules/default-case-last.md | 125 ++++++++++++++++++++++++++ lib/rules/default-case-last.js | 44 +++++++++ lib/rules/index.js | 1 + tests/lib/rules/default-case-last.js | 128 +++++++++++++++++++++++++++ tools/rule-types.json | 1 + 5 files changed, 299 insertions(+) create mode 100644 docs/rules/default-case-last.md create mode 100644 lib/rules/default-case-last.js create mode 100644 tests/lib/rules/default-case-last.js diff --git a/docs/rules/default-case-last.md b/docs/rules/default-case-last.md new file mode 100644 index 00000000000..c9eedbeec65 --- /dev/null +++ b/docs/rules/default-case-last.md @@ -0,0 +1,125 @@ +# Enforce default clauses in switch statements to be last (default-case-last) + +A `switch` statement can optionally have a `default` clause. + +If present, it's usually the last clause, but it doesn't need to be. It is also allowed to put the `default` clause before all `case` clauses, or anywhere between. The behavior is mostly the same as if it was the last clause. The `default` block will be still executed only if there is no match in the `case` clauses (including those defined after the `default`), but there is also the ability to "fall through" from the `default` clause to the following clause in the list. However, such flow is not common and it would be confusing to the readers. + +Even if there is no "fall through" logic, it's still unexpected to see the `default` clause before or between the `case` clauses. By convention, it is expected to be the last clause. + +If a `switch` statement should have a `default` clause, it's considered a best practice to define it as the last clause. + +## Rule Details + +This rule enforces `default` clauses in `switch` statements to be last. + +It applies only to `switch` statements that already have a `default` clause. + +This rule does not enforce the existence of `default` clauses. See [default-case](default-case.md) if you also want to enforce the existence of `default` clauses in `switch` statements. + +Examples of **incorrect** code for this rule: + +```js +/*eslint default-case-last: "error"*/ + +switch (foo) { + default: + bar(); + break; + case "a": + baz(); + break; +} + +switch (foo) { + case 1: + bar(); + break; + default: + baz(); + break; + case 2: + quux(); + break; +} + +switch (foo) { + case "x": + bar(); + break; + default: + case "y": + baz(); + break; +} + +switch (foo) { + default: + break; + case -1: + bar(); + break; +} + +switch (foo) { + default: + doSomethingIfNotZero(); + case 0: + doSomethingAnyway(); +} +``` + +Examples of **correct** code for this rule: + +```js +/*eslint default-case-last: "error"*/ + +switch (foo) { + case "a": + baz(); + break; + default: + bar(); + break; +} + +switch (foo) { + case 1: + bar(); + break; + case 2: + quux(); + break; + default: + baz(); + break; +} + +switch (foo) { + case "x": + bar(); + break; + case "y": + default: + baz(); + break; +} + +switch (foo) { + case -1: + bar(); + break; +} + +if (foo !== 0) { + doSomethingIfNotZero(); +} +doSomethingAnyway(); +``` + +## Further Reading + +* [MDN switch statement](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/switch) + +## Related Rules + +* [default-case](default-case.md) diff --git a/lib/rules/default-case-last.js b/lib/rules/default-case-last.js new file mode 100644 index 00000000000..80c5d6bda73 --- /dev/null +++ b/lib/rules/default-case-last.js @@ -0,0 +1,44 @@ +/** + * @fileoverview Rule to enforce default clauses in switch statements to be last + * @author Milos Djermanovic + */ + +"use strict"; + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +module.exports = { + meta: { + type: "suggestion", + + docs: { + description: "enforce default clauses in switch statements to be last", + category: "Best Practices", + recommended: false, + url: "https://eslint.org/docs/rules/default-case-last" + }, + + schema: [], + + messages: { + notLast: "Default clause should be the last clause." + } + }, + + create(context) { + return { + SwitchStatement(node) { + const cases = node.cases, + indexOfDefault = cases.findIndex(c => c.test === null); + + if (indexOfDefault !== -1 && indexOfDefault !== cases.length - 1) { + const defaultClause = cases[indexOfDefault]; + + context.report({ node: defaultClause, messageId: "notLast" }); + } + } + }; + } +}; diff --git a/lib/rules/index.js b/lib/rules/index.js index d3fbe412080..26d66c045a9 100644 --- a/lib/rules/index.js +++ b/lib/rules/index.js @@ -37,6 +37,7 @@ module.exports = new LazyLoadingRuleMap(Object.entries({ "constructor-super": () => require("./constructor-super"), curly: () => require("./curly"), "default-case": () => require("./default-case"), + "default-case-last": () => require("./default-case-last"), "default-param-last": () => require("./default-param-last"), "dot-location": () => require("./dot-location"), "dot-notation": () => require("./dot-notation"), diff --git a/tests/lib/rules/default-case-last.js b/tests/lib/rules/default-case-last.js new file mode 100644 index 00000000000..9f75329abd6 --- /dev/null +++ b/tests/lib/rules/default-case-last.js @@ -0,0 +1,128 @@ +/** + * @fileoverview Tests for the default-case-last rule + * @author Milos Djermanovic + */ + +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const rule = require("../../../lib/rules/default-case-last"); +const { RuleTester } = require("../../../lib/rule-tester"); + +//------------------------------------------------------------------------------ +// Helpers +//------------------------------------------------------------------------------ + +/** + * Creates an error object. + * @param {number} [column] Reported column. + * @returns {Object} The error object. + */ +function error(column) { + const errorObject = { + messageId: "notLast", + type: "SwitchCase" + }; + + if (column) { + errorObject.column = column; + } + + return errorObject; +} + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +const ruleTester = new RuleTester(); + +ruleTester.run("default-case-last", rule, { + valid: [ + "switch (foo) {}", + "switch (foo) { case 1: bar(); break; }", + "switch (foo) { case 1: break; }", + "switch (foo) { case 1: }", + "switch (foo) { case 1: bar(); break; case 2: baz(); break; }", + "switch (foo) { case 1: break; case 2: break; }", + "switch (foo) { case 1: case 2: break; }", + "switch (foo) { case 1: case 2: }", + "switch (foo) { default: bar(); break; }", + "switch (foo) { default: bar(); }", + "switch (foo) { default: break; }", + "switch (foo) { default: }", + "switch (foo) { case 1: break; default: break; }", + "switch (foo) { case 1: break; default: }", + "switch (foo) { case 1: default: break; }", + "switch (foo) { case 1: default: }", + "switch (foo) { case 1: baz(); break; case 2: quux(); break; default: quuux(); break; }", + "switch (foo) { case 1: break; case 2: break; default: break; }", + "switch (foo) { case 1: break; case 2: break; default: }", + "switch (foo) { case 1: case 2: break; default: break; }", + "switch (foo) { case 1: break; case 2: default: break; }", + "switch (foo) { case 1: break; case 2: default: }", + "switch (foo) { case 1: case 2: default: }" + ], + + invalid: [ + { + code: "switch (foo) { default: bar(); break; case 1: baz(); break; }", + errors: [error(16)] + }, + { + code: "switch (foo) { default: break; case 1: break; }", + errors: [error(16)] + }, + { + code: "switch (foo) { default: break; case 1: }", + errors: [error(16)] + }, + { + code: "switch (foo) { default: case 1: break; }", + errors: [error(16)] + }, + { + code: "switch (foo) { default: case 1: }", + errors: [error(16)] + }, + { + code: "switch (foo) { default: break; case 1: break; case 2: break; }", + errors: [error(16)] + }, + { + code: "switch (foo) { default: case 1: break; case 2: break; }", + errors: [error(16)] + }, + { + code: "switch (foo) { default: case 1: case 2: break; }", + errors: [error(16)] + }, + { + code: "switch (foo) { default: case 1: case 2: }", + errors: [error(16)] + }, + { + code: "switch (foo) { case 1: break; default: break; case 2: break; }", + errors: [error(31)] + }, + { + code: "switch (foo) { case 1: default: break; case 2: break; }", + errors: [error(24)] + }, + { + code: "switch (foo) { case 1: break; default: case 2: break; }", + errors: [error(31)] + }, + { + code: "switch (foo) { case 1: default: case 2: break; }", + errors: [error(24)] + }, + { + code: "switch (foo) { case 1: default: case 2: }", + errors: [error(24)] + } + ] +}); diff --git a/tools/rule-types.json b/tools/rule-types.json index 7b5789fbe8d..2ab896458ae 100644 --- a/tools/rule-types.json +++ b/tools/rule-types.json @@ -24,6 +24,7 @@ "constructor-super": "problem", "curly": "suggestion", "default-case": "suggestion", + "default-case-last": "suggestion", "default-param-last": "suggestion", "dot-location": "layout", "dot-notation": "suggestion",