Add switch-case-break-position rule#2910
Conversation
|
A few things:
switch (foo) {
case 1: {
foo(); // keep with foo
}
break;
}autofixes to: switch (foo) {
case 1: {
foo();
break; // keep with foo
}
}That changes comment attachment and potentially meaning, so I do not think this fixer is safe as is. It should probably either insert before the closing brace or bail when the last statement has trailing comments.
switch (foo) { case 1: { foo(); } break; }autofixes to: switch (foo) { case 1: { foo();
break; } }So even in cases without comments, the fix currently produces malformed or at least pretty rough output. |
| } | ||
| `, | ||
| ], | ||
| }); |
There was a problem hiding this comment.
| }); | |
| }); | |
| test({ | |
| valid: [], | |
| invalid: [ | |
| // Inline comment | |
| { | |
| code: outdent` | |
| switch(foo) { | |
| case 1: { | |
| doStuff(); // Keep this comment with the statement | |
| } | |
| break; | |
| } | |
| `, | |
| output: outdent` | |
| switch(foo) { | |
| case 1: { | |
| doStuff(); // Keep this comment with the statement | |
| break; | |
| } | |
| } | |
| `, | |
| errors: [error], | |
| }, | |
| // Block comment | |
| { | |
| code: outdent` | |
| switch(foo) { | |
| case 1: { | |
| doStuff(); /* Keep this block comment with the statement */ | |
| } | |
| break; | |
| } | |
| `, | |
| output: outdent` | |
| switch(foo) { | |
| case 1: { | |
| doStuff(); /* Keep this block comment with the statement */ | |
| break; | |
| } | |
| } | |
| `, | |
| errors: [error], | |
| }, | |
| // Before closing brace | |
| { | |
| code: outdent` | |
| switch(foo) { | |
| case 1: { | |
| doStuff(); | |
| // Keep this comment before the inserted break | |
| } | |
| break; | |
| } | |
| `, | |
| output: outdent` | |
| switch(foo) { | |
| case 1: { | |
| doStuff(); | |
| // Keep this comment before the inserted break | |
| break; | |
| } | |
| } | |
| `, | |
| errors: [error], | |
| }, | |
| // ESLint directive | |
| { | |
| code: outdent` | |
| switch(foo) { | |
| case 1: { | |
| console.log(foo); // eslint-disable-line no-console | |
| } | |
| break; | |
| } | |
| `, | |
| output: outdent` | |
| switch(foo) { | |
| case 1: { | |
| console.log(foo); // eslint-disable-line no-console | |
| break; | |
| } | |
| } | |
| `, | |
| errors: [error], | |
| }, | |
| ], | |
| }); |
| import outdent from 'outdent'; | ||
| import {getTester} from './utils/test.js'; | ||
|
|
||
| const {test} = getTester(import.meta); |
There was a problem hiding this comment.
| const {test} = getTester(import.meta); | |
| const {test} = getTester(import.meta); | |
| const error = {messageId: 'switch-case-break-position'}; |
|
It's unclear if you have handled all the feedback. |
- Insert before closing brace (via getTokenBefore with includeComments) instead of after lastBodyStatement to preserve comment attachment - Skip fix for single-line blocks to avoid malformed output - Use removeRange instead of replaceNodeOrTokenAndSpacesBefore to eliminate extra blank lines in fixed output - Add comment-handling test cases and single-line block guard test
c1ce162 to
9467d5d
Compare
|
All four items from the feedback have been addressed:
|
|
I rechecked this manualy and the fixer is still unsafe here. With: switch(foo) {
case 1: {
doStuff();
}
break; // keep with break
}it fixes to: switch(foo) {
case 1: {
doStuff();
break;
} // keep with break
}So the inline comment still gets detached from the terminator and ends up after the closing brace instead. The fixer only removes through the end of the |
|
I would add these tests too:
switch(foo) {
case 1: {
doStuff();
}
break; // keep with break
}This is the clearest regression case.
switch(foo) {
case 1: {
doStuff();
}
break; /* keep with break */
}
function foo() {
switch(bar) {
case 1: {
doStuff();
}
return value; // keep with return
}
}That proves the bug is not
Less important, but still decent:
|
Skip the autofix when break/return/continue/throw has a trailing same-line comment, to avoid detaching it from the terminator. Add test cases for line comment, block comment, and return.
Closes #2850
Adds a new rule that enforces terminating statements (
break,return,continue,throw) appear inside the block statement of acaseclause, not after it.This can happen when refactoring — for example, removing an
ifwrapper but leaving thebreakoutside the braces.What the rule catches
Details
break,return,continue, andthrowafter block statements in case/default clauses--fix) — moves the statement inside the blockbreak/continuecorrectlyrecommendedconfig, disabled inunopinionatedTest coverage
18 test cases (8 valid, 10 invalid) covering:
caseanddefaultclauses