eslint#18182 (no-fallthrough) Report unused fallthrough comments

docs/src/rules/no-fallthrough.md

@@ -178,6 +178,8 @@ This rule has an object option:
 
 * Set the `allowEmptyCase` option to `true` to allow empty cases regardless of the layout. By default, this rule does not require a fallthrough comment after an empty `case` only if the empty `case` and the next `case` are on the same line or on consecutive lines.
 
+* Set the `reportUnusedFallthroughComment` option to `true` to prohibit a fallthrough comment from being present if the case cannot fallthrough due to being unreachable. This is mostly intended to help avoid misleading comments occurring as a result of refactoring.
+
 ### commentPattern
 
 Examples of **correct** code for the `{ "commentPattern": "break[\\s\\w]*omitted" }` option:
@@ -235,6 +237,42 @@ switch(foo){
 
 :::
 
+### reportUnusedFallthroughComment
+
+Examples of **incorrect** code for the `{ "reportUnusedFallthroughComment": true}` option:
+
+::: incorrect
+
+```js
+/* eslint no-fallthrough: ["error", { "reportUnusedFallthroughComment": true }] */
+
+switch(foo){
+    case 1:
+        doSomething();
+        break;
+        // falls through
+    case 2: doSomething();
+}
+
+function f() {
+    switch(foo){
+        case 1:
+            if (a) {
+                throw new Error();
+            } else if (b) {
+                break;
+            } else {
+                return;
+            }
+            // falls through
+        case 2:
+            break;
+    }
+}
+```
+
+:::
+
 ## When Not To Use It
 
 If you don't want to enforce that each `case` statement should end with a `throw`, `return`, `break`, or comment, then you can safely turn this rule off.

lib/rules/no-fallthrough.js

@@ -48,23 +48,27 @@ function isFallThroughComment(comment, fallthroughCommentPattern) {
  * @param {ASTNode} subsequentCase The case after caseWhichFallsThrough.
  * @param {RuleContext} context A rule context which stores comments.
  * @param {RegExp} fallthroughCommentPattern A pattern to match comment to.
- * @returns {boolean} `true` if the case has a valid fallthrough comment.
+ * @returns {null | object} the comment if the case has a valid fallthrough comment, otherwise null
  */
-function hasFallthroughComment(caseWhichFallsThrough, subsequentCase, context, fallthroughCommentPattern) {
+function getFallthroughComment(caseWhichFallsThrough, subsequentCase, context, fallthroughCommentPattern) {
     const sourceCode = context.sourceCode;
 
     if (caseWhichFallsThrough.consequent.length === 1 && caseWhichFallsThrough.consequent[0].type === "BlockStatement") {
         const trailingCloseBrace = sourceCode.getLastToken(caseWhichFallsThrough.consequent[0]);
         const commentInBlock = sourceCode.getCommentsBefore(trailingCloseBrace).pop();
 
         if (commentInBlock && isFallThroughComment(commentInBlock.value, fallthroughCommentPattern)) {
-            return true;
+            return commentInBlock;
         }
     }
 
     const comment = sourceCode.getCommentsBefore(subsequentCase).pop();
 
-    return Boolean(comment && isFallThroughComment(comment.value, fallthroughCommentPattern));
+    if (comment && isFallThroughComment(comment.value, fallthroughCommentPattern)) {
+        return comment;
+    }
+
+    return null;
 }
 
 /**
@@ -103,12 +107,17 @@ module.exports = {
                     allowEmptyCase: {
                         type: "boolean",
                         default: false
+                    },
+                    reportUnusedFallthroughComment: {
+                        type: "boolean",
+                        default: false
                     }
                 },
                 additionalProperties: false
             }
         ],
         messages: {
+            unusedFallthroughComment: "Found a comment that would permit fallthrough, but case cannot fall through.",
             case: "Expected a 'break' statement before 'case'.",
             default: "Expected a 'break' statement before 'default'."
         }
@@ -120,12 +129,13 @@ module.exports = {
         let currentCodePathSegments = new Set();
         const sourceCode = context.sourceCode;
         const allowEmptyCase = options.allowEmptyCase || false;
+        const reportUnusedFallthroughComment = options.reportUnusedFallthroughComment || false;
 
         /*
          * We need to use leading comments of the next SwitchCase node because
          * trailing comments is wrong if semicolons are omitted.
          */
-        let fallthroughCase = null;
+        let previousCase = null;
         let fallthroughCommentPattern = null;
 
         if (options.commentPattern) {
@@ -168,13 +178,24 @@ module.exports = {
                  * And reports the previous fallthrough node if that does not exist.
                  */
 
-                if (fallthroughCase && (!hasFallthroughComment(fallthroughCase, node, context, fallthroughCommentPattern))) {
-                    context.report({
-                        messageId: node.test ? "case" : "default",
-                        node
-                    });
+                if (previousCase) {
+                    const previousCaseFallthroughComment = getFallthroughComment(previousCase.node, node, context, fallthroughCommentPattern);
+
+                    if (previousCase.isFallthrough && !(previousCaseFallthroughComment)) {
+                        context.report({
+                            messageId: node.test ? "case" : "default",
+                            node
+                        });
+                    } else if (reportUnusedFallthroughComment && !previousCase.isSwitchExitReachable && previousCaseFallthroughComment) {
+                        context.report({
+                            messageId: "unusedFallthroughComment",
+                            node: previousCaseFallthroughComment
+                        });
+                    }
+
+
                 }
-                fallthroughCase = null;
+                previousCase = null;
             },
 
             "SwitchCase:exit"(node) {
@@ -185,11 +206,16 @@ module.exports = {
                  * `break`, `return`, or `throw` are unreachable.
                  * And allows empty cases and the last case.
                  */
-                if (isAnySegmentReachable(currentCodePathSegments) &&
-                    (node.consequent.length > 0 || (!allowEmptyCase && hasBlankLinesBetween(node, nextToken))) &&
-                    node.parent.cases.at(-1) !== node) {
-                    fallthroughCase = node;
-                }
+                const isSwitchExitReachable = isAnySegmentReachable(currentCodePathSegments);
+                const isFallthrough = isSwitchExitReachable && (node.consequent.length > 0 || (!allowEmptyCase && hasBlankLinesBetween(node, nextToken))) &&
+                    node.parent.cases.at(-1) !== node;
+
+                previousCase = {
+                    node,
+                    isSwitchExitReachable,
+                    isFallthrough
+                };
+
             }
         };
     }

tests/lib/rules/no-fallthrough.js

@@ -64,6 +64,27 @@ ruleTester.run("no-fallthrough", rule, {
         "switch (foo) { case 0: try { throw 0; } catch (err) { break; } default: b(); }",
         "switch (foo) { case 0: do { throw 0; } while(a); default: b(); }",
         "switch (foo) { case 0: a(); \n// eslint-disable-next-line rule-to-test/no-fallthrough\n case 1: }",
+        `
+            switch (foo) {
+                case 0:
+                    a();
+                    break;
+                    /* falls through */
+                case 1:
+                    b();
+            }
+        `,
+        `
+            switch (foo) {
+                case 0:
+                    a();
+                    break;
+                    // eslint-disable-next-line rule-to-test/no-fallthrough
+                    /* falls through */
+                case 1:
+                    b();
+            }
+        `,
         {
             code: "switch(foo) { case 0: a(); /* no break */ case 1: b(); }",
             options: [{
@@ -111,6 +132,7 @@ ruleTester.run("no-fallthrough", rule, {
             options: [{ allowEmptyCase: false }]
         }
     ],
+
     invalid: [
         {
             code: "switch(foo) { case 0: a();\ncase 1: b() }",
@@ -310,6 +332,83 @@ ruleTester.run("no-fallthrough", rule, {
                     column: 2
                 }
             ]
+        },
+        {
+            code: `
+switch (foo) {
+    case 0:
+        a();
+        break;
+        /* falls through */
+    case 1:
+        b();
+}`,
+            options: [{ reportUnusedFallthroughComment: true }],
+            errors: [
+                {
+                    line: 6,
+                    messageId: "unusedFallthroughComment"
+                }
+            ]
+        },
+        {
+            code: `
+switch (foo) {
+    default:
+        a();
+        break;
+        /* falls through */
+    case 1:
+        b();
+}`,
+            options: [{ reportUnusedFallthroughComment: true }],
+            errors: [
+                {
+                    line: 6,
+                    messageId: "unusedFallthroughComment"
+                }
+            ]
+        },
+        {
+            code: `
+switch(foo){
+    case 1:
+        doSomething();
+        break;
+        // falls through
+    case 2: doSomething();
+}`,
+            options: [{ reportUnusedFallthroughComment: true }],
+            errors: [
+                {
+                    line: 6,
+                    messageId: "unusedFallthroughComment"
+                }
+            ]
+        }, {
+            code: `
+function f() {
+    switch(foo){
+        case 1:
+            if (a) {
+                throw new Error();
+            } else if (b) {
+                break;
+            } else {
+                return;
+            }
+            // falls through
+        case 2:
+            break;
+    }
+}`,
+            options: [{ reportUnusedFallthroughComment: true }],
+            errors: [
+                {
+                    line: 12,
+                    messageId: "unusedFallthroughComment"
+                }
+            ]
         }
     ]
 });