Skip to content

Commit 6a1c7a0

Browse files
bakkotbakkot
authored
Fix: allow fallthrough comment inside block (fixes #14701) (#14702)
* Fix: allow fallthrough comment inside block (fixes #14701) * address comments
1 parent 97d9bd2 commit 6a1c7a0

3 files changed

Lines changed: 85 additions & 6 deletions

File tree

‎docs/rules/no-fallthrough.md‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,17 @@ switch(foo) {
5454
case 2:
5555
doSomethingElse();
5656
}
57+
58+
switch(foo) {
59+
case 1: {
60+
doSomething();
61+
// falls through
62+
}
63+
64+
case 2: {
65+
doSomethingElse();
66+
}
67+
}
5768
```
5869

5970
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) {
124135
case 2:
125136
doSomething();
126137
}
138+
139+
switch(foo) {
140+
case 1: {
141+
doSomething();
142+
// falls through
143+
}
144+
145+
case 2: {
146+
doSomethingElse();
147+
}
148+
}
127149
```
128150

129151
Note that the last `case` statement in these examples does not cause a warning because there is nothing to fall through into.

‎lib/rules/no-fallthrough.js‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,26 @@
1111
const DEFAULT_FALLTHROUGH_COMMENT = /falls?\s?through/iu;
1212

1313
/**
14-
* Checks whether or not a given node has a fallthrough comment.
15-
* @param {ASTNode} node A SwitchCase node to get comments.
14+
* Checks whether or not a given case has a fallthrough comment.
15+
* @param {ASTNode} caseWhichFallsThrough SwitchCase node which falls through.
16+
* @param {ASTNode} subsequentCase The case after caseWhichFallsThrough.
1617
* @param {RuleContext} context A rule context which stores comments.
1718
* @param {RegExp} fallthroughCommentPattern A pattern to match comment to.
18-
* @returns {boolean} `true` if the node has a valid fallthrough comment.
19+
* @returns {boolean} `true` if the case has a valid fallthrough comment.
1920
*/
20-
function hasFallthroughComment(node, context, fallthroughCommentPattern) {
21+
function hasFallthroughComment(caseWhichFallsThrough, subsequentCase, context, fallthroughCommentPattern) {
2122
const sourceCode = context.getSourceCode();
22-
const comment = sourceCode.getCommentsBefore(node).pop();
23+
24+
if (caseWhichFallsThrough.consequent.length === 1 && caseWhichFallsThrough.consequent[0].type === "BlockStatement") {
25+
const trailingCloseBrace = sourceCode.getLastToken(caseWhichFallsThrough.consequent[0]);
26+
const commentInBlock = sourceCode.getCommentsBefore(trailingCloseBrace).pop();
27+
28+
if (commentInBlock && fallthroughCommentPattern.test(commentInBlock.value)) {
29+
return true;
30+
}
31+
}
32+
33+
const comment = sourceCode.getCommentsBefore(subsequentCase).pop();
2334

2435
return Boolean(comment && fallthroughCommentPattern.test(comment.value));
2536
}
@@ -108,7 +119,7 @@ module.exports = {
108119
* Checks whether or not there is a fallthrough comment.
109120
* And reports the previous fallthrough node if that does not exist.
110121
*/
111-
if (fallthroughCase && !hasFallthroughComment(node, context, fallthroughCommentPattern)) {
122+
if (fallthroughCase && !hasFallthroughComment(fallthroughCase, node, context, fallthroughCommentPattern)) {
112123
context.report({
113124
messageId: node.test ? "case" : "default",
114125
node

‎tests/lib/rules/no-fallthrough.js‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,14 @@ ruleTester.run("no-fallthrough", rule, {
3030
"switch(foo) { case 0: a(); /* fall through */ case 1: b(); }",
3131
"switch(foo) { case 0: a(); /* fallthrough */ case 1: b(); }",
3232
"switch(foo) { case 0: a(); /* FALLS THROUGH */ case 1: b(); }",
33+
"switch(foo) { case 0: { a(); /* falls through */ } case 1: b(); }",
34+
"switch(foo) { case 0: { a()\n /* falls through */ } case 1: b(); }",
35+
"switch(foo) { case 0: { a(); /* fall through */ } case 1: b(); }",
36+
"switch(foo) { case 0: { a(); /* fallthrough */ } case 1: b(); }",
37+
"switch(foo) { case 0: { a(); /* FALLS THROUGH */ } case 1: b(); }",
38+
"switch(foo) { case 0: { a(); } /* falls through */ case 1: b(); }",
39+
"switch(foo) { case 0: { a(); /* falls through */ } /* comment */ case 1: b(); }",
40+
"switch(foo) { case 0: { /* falls through */ } case 1: b(); }",
3341
"function foo() { switch(foo) { case 0: a(); return; case 1: b(); }; }",
3442
"switch(foo) { case 0: a(); throw 'foo'; case 1: b(); }",
3543
"while (a) { switch(foo) { case 0: a(); continue; case 1: b(); } }",
@@ -133,6 +141,30 @@ ruleTester.run("no-fallthrough", rule, {
133141
code: "switch(foo) { case 0:\n\n default: b() }",
134142
errors: errorsDefault
135143
},
144+
{
145+
code: "switch(foo) { case 0: {} default: b() }",
146+
errors: errorsDefault
147+
},
148+
{
149+
code: "switch(foo) { case 0: a(); { /* falls through */ } default: b() }",
150+
errors: errorsDefault
151+
},
152+
{
153+
code: "switch(foo) { case 0: { /* falls through */ } a(); default: b() }",
154+
errors: errorsDefault
155+
},
156+
{
157+
code: "switch(foo) { case 0: if (a) { /* falls through */ } default: b() }",
158+
errors: errorsDefault
159+
},
160+
{
161+
code: "switch(foo) { case 0: { { /* falls through */ } } default: b() }",
162+
errors: errorsDefault
163+
},
164+
{
165+
code: "switch(foo) { case 0: { /* comment */ } default: b() }",
166+
errors: errorsDefault
167+
},
136168
{
137169
code: "switch(foo) { case 0:\n // comment\n default: b() }",
138170
errors: errorsDefault
@@ -168,6 +200,20 @@ ruleTester.run("no-fallthrough", rule, {
168200
column: 1
169201
}
170202
]
203+
},
204+
{
205+
code: "switch(foo) { case 0: { a();\n/* no break */\n/* todo: fix readability */ }\ndefault: b() }",
206+
options: [{
207+
commentPattern: "no break"
208+
}],
209+
errors: [
210+
{
211+
messageId: "default",
212+
type: "SwitchCase",
213+
line: 4,
214+
column: 1
215+
}
216+
]
171217
}
172218
]
173219
});

0 commit comments

Comments
 (0)