New: Add default-case-last rule (fixes #12665) (#12668)

eslint · Jan 17, 2020 · f9774ec · f9774ec
1 parent 9a93d9e
commit f9774ec
Show file tree

Hide file tree

Showing 5 changed files with 299 additions and 0 deletions.
diff --git a/docs/rules/default-case-last.md 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
@@ -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
@@ -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
@@ -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
@@ -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",