Skip to content

Commit

Permalink
Update: space-before-blocks ignore after switch colons (fixes #15082) (
Browse files Browse the repository at this point in the history
…#15093)

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

* remove extra space in docs
  • Loading branch information
mdjermanovic committed Sep 24, 2021
1 parent c9efb5f commit cf34e5c
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 17 deletions.
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]
}
]
});

0 comments on commit cf34e5c

Please sign in to comment.