Fix: allow fallthrough comment inside block (fixes eslint#14701)

bakkot · Jun 14, 2021 · 6454106 · 6454106
1 parent 757c495
commit 6454106
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 6 deletions.
diff --git a/docs/rules/no-fallthrough.md b/docs/rules/no-fallthrough.md
@@ -54,6 +54,17 @@ switch(foo) {
     case 2:
         doSomethingElse();
 }
+
+switch(foo) {
+    case 1: {
+        doSomething();
+        // falls through
+    }
+
+    case 2: {
+        doSomethingElse();
+    }
+}
 ```
 
 In this example, there is no confusion as to the expected behavior. It is clear that the first case is meant to fall through to the second case.

diff --git a/lib/rules/no-fallthrough.js b/lib/rules/no-fallthrough.js
@@ -11,15 +11,26 @@
 const DEFAULT_FALLTHROUGH_COMMENT = /falls?\s?through/iu;
 
 /**
- * Checks whether or not a given node has a fallthrough comment.
- * @param {ASTNode} node A SwitchCase node to get comments.
+ * Checks whether or not a given case has a fallthrough comment.
+ * @param {ASTNode} caseWhichFallsThrough SwitchCase node which falls through.
+ * @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 node has a valid fallthrough comment.
+ * @returns {boolean} `true` if the case has a valid fallthrough comment.
  */
-function hasFallthroughComment(node, context, fallthroughCommentPattern) {
+function hasFallthroughComment(caseWhichFallsThrough, subsequentCase, context, fallthroughCommentPattern) {
     const sourceCode = context.getSourceCode();
-    const comment = sourceCode.getCommentsBefore(node).pop();
+
+    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 && fallthroughCommentPattern.test(commentInBlock.value)) {
+            return true;
+        }
+    }
+
+    const comment = sourceCode.getCommentsBefore(subsequentCase).pop();
 
     return Boolean(comment && fallthroughCommentPattern.test(comment.value));
 }
@@ -108,7 +119,7 @@ module.exports = {
                  * Checks whether or not there is a fallthrough comment.
                  * And reports the previous fallthrough node if that does not exist.
                  */
-                if (fallthroughCase && !hasFallthroughComment(node, context, fallthroughCommentPattern)) {
+                if (fallthroughCase && !hasFallthroughComment(fallthroughCase, node, context, fallthroughCommentPattern)) {
                     context.report({
                         messageId: node.test ? "case" : "default",
                         node

diff --git a/tests/lib/rules/no-fallthrough.js b/tests/lib/rules/no-fallthrough.js
@@ -30,6 +30,14 @@ ruleTester.run("no-fallthrough", rule, {
         "switch(foo) { case 0: a(); /* fall through */ case 1: b(); }",
         "switch(foo) { case 0: a(); /* fallthrough */ case 1: b(); }",
         "switch(foo) { case 0: a(); /* FALLS THROUGH */ case 1: b(); }",
+        "switch(foo) { case 0: { a(); /* falls through */ } case 1: b(); }",
+        "switch(foo) { case 0: { a()\n /* falls through */ } case 1: b(); }",
+        "switch(foo) { case 0: { a(); /* fall through */ } case 1: b(); }",
+        "switch(foo) { case 0: { a(); /* fallthrough */ } case 1: b(); }",
+        "switch(foo) { case 0: { a(); /* FALLS THROUGH */ } case 1: b(); }",
+        "switch(foo) { case 0: { a(); } /* falls through */ case 1: b(); }",
+        "switch(foo) { case 0: { a(); /* falls through */ } /* comment */ case 1: b(); }",
+        "switch(foo) { case 0: { /* falls through */ } case 1: b(); }",
         "function foo() { switch(foo) { case 0: a(); return; case 1: b(); }; }",
         "switch(foo) { case 0: a(); throw 'foo'; case 1: b(); }",
         "while (a) { switch(foo) { case 0: a(); continue; case 1: b(); } }",
@@ -133,6 +141,14 @@ ruleTester.run("no-fallthrough", rule, {
             code: "switch(foo) { case 0:\n\n default: b() }",
             errors: errorsDefault
         },
+        {
+            code: "switch(foo) { case 0: {} default: b() }",
+            errors: errorsDefault
+        },
+        {
+            code: "switch(foo) { case 0: { /* comment */ } default: b() }",
+            errors: errorsDefault
+        },
         {
             code: "switch(foo) { case 0:\n // comment\n default: b() }",
             errors: errorsDefault
@@ -168,6 +184,20 @@ ruleTester.run("no-fallthrough", rule, {
                     column: 1
                 }
             ]
+        },
+        {
+            code: "switch(foo) { case 0: { a();\n/* no break */\n/* todo: fix readability */ }\ndefault: b() }",
+            options: [{
+                commentPattern: "no break"
+            }],
+            errors: [
+                {
+                    messageId: "default",
+                    type: "SwitchCase",
+                    line: 4,
+                    column: 1
+                }
+            ]
         }
     ]
 });