Skip to content
Permalink
Browse files
Fix: allow fallthrough comment inside block (fixes #14701) (#14702)
* Fix: allow fallthrough comment inside block (fixes #14701)

* address comments
  • Loading branch information
bakkot committed Jun 18, 2021
1 parent 97d9bd2 commit 6a1c7a0dac050ea5876972c50563a7eb867b38d3
Showing with 85 additions and 6 deletions.
  1. +22 −0 docs/rules/no-fallthrough.md
  2. +17 −6 lib/rules/no-fallthrough.js
  3. +46 −0 tests/lib/rules/no-fallthrough.js
@@ -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.
@@ -124,6 +135,17 @@ switch(foo) {
case 2:
doSomething();
}
switch(foo) {
case 1: {
doSomething();
// falls through
}
case 2: {
doSomethingElse();
}
}
```

Note that the last `case` statement in these examples does not cause a warning because there is nothing to fall through into.
@@ -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
@@ -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,30 @@ 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: a(); { /* falls through */ } default: b() }",
errors: errorsDefault
},
{
code: "switch(foo) { case 0: { /* falls through */ } a(); default: b() }",
errors: errorsDefault
},
{
code: "switch(foo) { case 0: if (a) { /* falls through */ } default: b() }",
errors: errorsDefault
},
{
code: "switch(foo) { case 0: { { /* falls through */ } } 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 +200,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
}
]
}
]
});

0 comments on commit 6a1c7a0

Please sign in to comment.