Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update: space-before-blocks ignore after switch colons (fixes #15082) #15093

Merged
merged 2 commits into from Sep 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/rules/space-before-blocks.md
Expand Up @@ -11,6 +11,7 @@ This rule will enforce consistency of spacing before blocks. It is only applied

* This rule ignores spacing which is between `=>` and a block. The spacing is handled by the `arrow-spacing` rule.
* This rule ignores spacing which is between a keyword and a block. The spacing is handled by the `keyword-spacing` rule.
* This rule ignores spacing which is between `:` of a switch case and a block. The spacing is handled by the `switch-colon-spacing` rule.

## Options

Expand Down Expand Up @@ -210,4 +211,5 @@ You can turn this rule off if you are not concerned with the consistency of spac

* [keyword-spacing](keyword-spacing.md)
* [arrow-spacing](arrow-spacing.md)
* [switch-colon-spacing](switch-colon-spacing.md)
* [brace-style](brace-style.md)
16 changes: 14 additions & 2 deletions lib/rules/space-before-blocks.js
Expand Up @@ -107,13 +107,25 @@ module.exports = {
* Checks whether the spacing before the given block is already controlled by another rule:
* - `arrow-spacing` checks spaces after `=>`.
* - `keyword-spacing` checks spaces after keywords in certain contexts.
* - `switch-colon-spacing` checks spaces after `:` of switch cases.
* @param {Token} precedingToken first token before the block.
* @param {ASTNode|Token} node `BlockStatement` node or `{` token of a `SwitchStatement` node.
* @returns {boolean} `true` if requiring or disallowing spaces before the given block could produce conflicts with other rules.
*/
function isConflicted(precedingToken, node) {
return astUtils.isArrowToken(precedingToken) ||
astUtils.isKeywordToken(precedingToken) && !isFunctionBody(node);
return (
astUtils.isArrowToken(precedingToken) ||
(
astUtils.isKeywordToken(precedingToken) &&
!isFunctionBody(node)
) ||
(
astUtils.isColonToken(precedingToken) &&
node.parent &&
node.parent.type === "SwitchCase" &&
precedingToken === astUtils.getSwitchCaseColonToken(node.parent, sourceCode)
)
);
}

/**
Expand Down
14 changes: 1 addition & 13 deletions lib/rules/switch-colon-spacing.js
Expand Up @@ -50,18 +50,6 @@ module.exports = {
const beforeSpacing = options.before === true; // false by default
const afterSpacing = options.after !== false; // true by default

/**
* Get the colon token of the given SwitchCase node.
* @param {ASTNode} node The SwitchCase node to get.
* @returns {Token} The colon token of the node.
*/
function getColonToken(node) {
if (node.test) {
return sourceCode.getTokenAfter(node.test, astUtils.isColonToken);
}
return sourceCode.getFirstToken(node, 1);
}

/**
* Check whether the spacing between the given 2 tokens is valid or not.
* @param {Token} left The left token to check.
Expand Down Expand Up @@ -114,7 +102,7 @@ module.exports = {

return {
SwitchCase(node) {
const colonToken = getColonToken(node);
const colonToken = astUtils.getSwitchCaseColonToken(node, sourceCode);
const beforeToken = sourceCode.getTokenBefore(colonToken);
const afterToken = sourceCode.getTokenAfter(colonToken);

Expand Down
16 changes: 15 additions & 1 deletion lib/rules/utils/ast-utils.js
Expand Up @@ -756,6 +756,19 @@ function isLogicalAssignmentOperator(operator) {
return LOGICAL_ASSIGNMENT_OPERATORS.has(operator);
}

/**
* Get the colon token of the given SwitchCase node.
* @param {ASTNode} node The SwitchCase node to get.
* @param {SourceCode} sourceCode The source code object to get tokens.
* @returns {Token} The colon token of the node.
*/
function getSwitchCaseColonToken(node, sourceCode) {
if (node.test) {
return sourceCode.getTokenAfter(node.test, isColonToken);
}
return sourceCode.getFirstToken(node, 1);
}

//------------------------------------------------------------------------------
// Public Interface
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -1872,5 +1885,6 @@ module.exports = {
isSpecificMemberAccess,
equalLiteralValue,
isSameReference,
isLogicalAssignmentOperator
isLogicalAssignmentOperator,
getSwitchCaseColonToken
};
73 changes: 72 additions & 1 deletion tests/lib/rules/space-before-blocks.js
Expand Up @@ -18,6 +18,7 @@ const rule = require("../../../lib/rules/space-before-blocks"),
//------------------------------------------------------------------------------

const ruleTester = new RuleTester(),
alwaysArgs = ["always"],
neverArgs = ["never"],
functionsOnlyArgs = [{ functions: "always", keywords: "never", classes: "never" }],
keywordOnlyArgs = [{ functions: "never", keywords: "always", classes: "never" }],
Expand Down Expand Up @@ -193,7 +194,15 @@ ruleTester.run("space-before-blocks", rule, {
"if(a) {}else{}",
{ code: "if(a){}else {}", options: neverArgs },
{ code: "try {}catch(a){}", options: functionsOnlyArgs },
{ code: "export default class{}", options: classesOnlyArgs, parserOptions: { ecmaVersion: 6, sourceType: "module" } }
{ code: "export default class{}", options: classesOnlyArgs, parserOptions: { ecmaVersion: 6, sourceType: "module" } },

// https://github.com/eslint/eslint/issues/15082
{ code: "switch(x) { case 9:{ break; } }", options: alwaysArgs },
{ code: "switch(x){ case 9: { break; } }", options: neverArgs },
{ code: "switch(x) { case (9):{ break; } }", options: alwaysArgs },
{ code: "switch(x){ case (9): { break; } }", options: neverArgs },
{ code: "switch(x) { default:{ break; } }", options: alwaysArgs },
{ code: "switch(x){ default: { break; } }", options: neverArgs }
],
invalid: [
{
Expand Down Expand Up @@ -571,6 +580,68 @@ ruleTester.run("space-before-blocks", rule, {
options: neverArgs,
parser: fixtureParser("space-before-blocks", "return-type-keyword-2"),
errors: [expectedNoSpacingError]
},

// https://github.com/eslint/eslint/issues/15082 regression tests (only blocks after switch case colons should be excluded)
{
code: "label:{}",
output: "label: {}",
options: alwaysArgs,
errors: [expectedSpacingError]
},
{
code: "label: {}",
output: "label:{}",
options: neverArgs,
errors: [expectedNoSpacingError]
},
{
code: "switch(x) { case 9: label:{ break; } }",
output: "switch(x) { case 9: label: { break; } }",
options: alwaysArgs,
errors: [expectedSpacingError]
},
{
code: "switch(x){ case 9: label: { break; } }",
output: "switch(x){ case 9: label:{ break; } }",
options: neverArgs,
errors: [expectedNoSpacingError]
},
{
code: "switch(x) { case 9: if(y){ break; } }",
output: "switch(x) { case 9: if(y) { break; } }",
options: alwaysArgs,
errors: [expectedSpacingError]
},
{
code: "switch(x){ case 9: if(y) { break; } }",
output: "switch(x){ case 9: if(y){ break; } }",
options: neverArgs,
errors: [expectedNoSpacingError]
},
{
code: "switch(x) { case 9: y;{ break; } }",
output: "switch(x) { case 9: y; { break; } }",
options: alwaysArgs,
errors: [expectedSpacingError]
},
{
code: "switch(x){ case 9: y; { break; } }",
output: "switch(x){ case 9: y;{ break; } }",
options: neverArgs,
errors: [expectedNoSpacingError]
},
{
code: "switch(x) { case 9: switch(y){} }",
output: "switch(x) { case 9: switch(y) {} }",
options: alwaysArgs,
errors: [expectedSpacingError]
},
{
code: "switch(x){ case 9: switch(y) {} }",
output: "switch(x){ case 9: switch(y){} }",
options: neverArgs,
errors: [expectedNoSpacingError]
}
]
});