Skip to content

Commit

Permalink
fix: ignore spacing before ] and } in comma-spacing (#16113)
Browse files Browse the repository at this point in the history 
This avoids conflicts with array-bracket-spacing and object-curly-spacing rules.

Fixes #16100
  • Loading branch information
@mdjermanovic
mdjermanovic committed Jul 8, 2022
1 parent a75d3b4 commit bfe5e88
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 72 deletions.
48 changes: 29 additions & 19 deletions docs/src/rules/comma-spacing.md
View file
Expand Up @@ -6,6 +6,7 @@ rule_type: layout
related_rules:
- array-bracket-spacing
- comma-style
- object-curly-spacing
- space-in-brackets
- space-in-parens
- space-infix-ops
Expand All @@ -30,10 +31,13 @@ var foo = 1 ,bar = 2;

This rule enforces consistent spacing before and after commas in variable declarations, array literals, object literals, function parameters, and sequences.

This rule does not apply in an `ArrayExpression` or `ArrayPattern` in either of the following cases:
This rule does not apply in either of the following cases:

* adjacent null elements
* an initial null element, to avoid conflicts with the [`array-bracket-spacing`](array-bracket-spacing) rule
* between two commas
* between opening bracket `[` and comma, to avoid conflicts with the [`array-bracket-spacing`](array-bracket-spacing) rule
* between comma and closing bracket `]`, to avoid conflicts with the [`array-bracket-spacing`](array-bracket-spacing) rule
* between comma and closing brace `}`, to avoid conflicts with the [`object-curly-spacing`](object-curly-spacing) rule
* between comma and closing parentheses `)`, to avoid conflicts with the [`space-in-parens`](space-in-parens) rule

## Options

Expand Down Expand Up @@ -84,15 +88,34 @@ a, b

:::

Example of **correct** code for this rule with initial null element for the default `{ "before": false, "after": true }` options:
Additional examples of **correct** code for this rule with the default `{ "before": false, "after": true }` options:

:::correct

```js
/*eslint comma-spacing: ["error", { "before": false, "after": true }]*/
/*eslint array-bracket-spacing: ["error", "always"]*/

var arr = [ , 2, 3 ]
// this rule does not enforce spacing between two commas
var arr = [
,,
, ,
];

// this rule does not enforce spacing after `[` and before `]`
var arr = [,];
var arr = [ , ];
var arr = [a, b,];
[,] = arr;
[ , ] = arr;
[a, b,] = arr;

// this rule does not enforce spacing before `}`
var obj = {x, y,};
var {z, q,} = obj;
import {foo, bar,} from "mod";

// this rule does not enforce spacing before `)`
foo(a, b,)
```

:::
Expand Down Expand Up @@ -136,19 +159,6 @@ a ,b

:::

Examples of **correct** code for this rule with initial null element for the `{ "before": true, "after": false }` options:

:::correct

```js
/*eslint comma-spacing: ["error", { "before": true, "after": false }]*/
/*eslint array-bracket-spacing: ["error", "never"]*/

var arr = [,2 ,3]
```

:::

## When Not To Use It

If your project will not be following a consistent comma-spacing pattern, turn this rule off.
76 changes: 35 additions & 41 deletions lib/rules/comma-spacing.js
View file
Expand Up @@ -103,38 +103,6 @@ module.exports = {
});
}

/**
* Validates the spacing around a comma token.
* @param {Object} tokens The tokens to be validated.
* @param {Token} tokens.comma The token representing the comma.
* @param {Token} [tokens.left] The last token before the comma.
* @param {Token} [tokens.right] The first token after the comma.
* @param {Token|ASTNode} reportItem The item to use when reporting an error.
* @returns {void}
* @private
*/
function validateCommaItemSpacing(tokens, reportItem) {
if (tokens.left && astUtils.isTokenOnSameLine(tokens.left, tokens.comma) &&
(options.before !== sourceCode.isSpaceBetweenTokens(tokens.left, tokens.comma))
) {
report(reportItem, "before", tokens.left);
}

if (tokens.right && astUtils.isClosingParenToken(tokens.right)) {
return;
}

if (tokens.right && !options.after && tokens.right.type === "Line") {
return;
}

if (tokens.right && astUtils.isTokenOnSameLine(tokens.comma, tokens.right) &&
(options.after !== sourceCode.isSpaceBetweenTokens(tokens.comma, tokens.right))
) {
report(reportItem, "after", tokens.right);
}
}

/**
* Adds null elements of the given ArrayExpression or ArrayPattern node to the ignore list.
* @param {ASTNode} node An ArrayExpression or ArrayPattern node.
Expand Down Expand Up @@ -172,18 +140,44 @@ module.exports = {
return;
}

if (token && token.type === "JSXText") {
return;
}

const previousToken = tokensAndComments[i - 1];
const nextToken = tokensAndComments[i + 1];

validateCommaItemSpacing({
comma: token,
left: astUtils.isCommaToken(previousToken) || commaTokensToIgnore.includes(token) ? null : previousToken,
right: astUtils.isCommaToken(nextToken) ? null : nextToken
}, token);
if (
previousToken &&
!astUtils.isCommaToken(previousToken) && // ignore spacing between two commas

/*
* `commaTokensToIgnore` are ending commas of `null` elements (array holes/elisions).
* In addition to spacing between two commas, this can also ignore:
*
* - Spacing after `[` (controlled by array-bracket-spacing)
* Example: [ , ]
* ^
* - Spacing after a comment (for backwards compatibility, this was possibly unintentional)
* Example: [a, /* * / ,]
* ^
*/
!commaTokensToIgnore.includes(token) &&

astUtils.isTokenOnSameLine(previousToken, token) &&
options.before !== sourceCode.isSpaceBetweenTokens(previousToken, token)
) {
report(token, "before", previousToken);
}

if (
nextToken &&
!astUtils.isCommaToken(nextToken) && // ignore spacing between two commas
!astUtils.isClosingParenToken(nextToken) && // controlled by space-in-parens
!astUtils.isClosingBracketToken(nextToken) && // controlled by array-bracket-spacing
!astUtils.isClosingBraceToken(nextToken) && // controlled by object-curly-spacing
!(!options.after && nextToken.type === "Line") && // special case, allow space before line comment
astUtils.isTokenOnSameLine(token, nextToken) &&
options.after !== sourceCode.isSpaceBetweenTokens(token, nextToken)
) {
report(token, "after", nextToken);
}
});
},
ArrayExpression: addNullElementsToIgnoreList,
Expand Down
55 changes: 43 additions & 12 deletions tests/lib/rules/comma-spacing.js
View file
Expand Up @@ -27,19 +27,36 @@ ruleTester.run("comma-spacing", rule, {
"myfunc(404, // comment\n true, /* bla bla bla */ 'hello');",
{ code: "myfunc(404, // comment\n true,/* bla bla bla */ 'hello');", options: [{ before: false, after: false }] },
"var a = 1, b = 2;",
"var arr = [,];",
"var arr = [, ];",
"var arr = [ ,];",
"var arr = [ , ];",
"var arr = [1,];",
"var arr = [1, ];",
"var arr = [, 2];",
"var arr = [ , 2];",
"var arr = [1, 2];",
"var arr = [,,];",
"var arr = [ ,,];",
"var arr = [, ,];",
"var arr = [,, ];",
"var arr = [ , ,];",
"var arr = [ ,, ];",
"var arr = [, , ];",
"var arr = [ , , ];",
"var arr = [1, , ];",
"var arr = [, 2, ];",
"var arr = [, , 3];",
"var arr = [,, 3];",
"var arr = [1, 2, ];",
"var arr = [, 2, 3];",
"var arr = [1, , 3];",
"var arr = [1, 2, 3];",
"var arr = [1, 2, 3,];",
"var arr = [1, 2, 3, ];",
"var obj = {'foo':'bar', 'baz':'qur'};",
"var obj = {'foo':'bar', 'baz':'qur', };",
"var obj = {'foo':'bar', 'baz':'qur',};",
"var obj = {'foo':'bar', 'baz':\n'qur'};",
"var obj = {'foo':\n'bar', 'baz':\n'qur'};",
"function foo(a, b){}",
Expand Down Expand Up @@ -74,20 +91,34 @@ ruleTester.run("comma-spacing", rule, {
{ code: "var a = 1 ,b = 2;", options: [{ before: true, after: false }] },
{ code: "function foo(a ,b){}", options: [{ before: true, after: false }] },
{ code: "var arr = [,];", options: [{ before: true, after: false }] },
{ code: "var arr = [ ,];", options: [{ before: true, after: false }] },
{ code: "var arr = [, ];", options: [{ before: true, after: false }] },
{ code: "var arr = [ , ];", options: [{ before: true, after: false }] },
{ code: "var arr = [1 ,];", options: [{ before: true, after: false }] },
{ code: "var arr = [1 , ];", options: [{ before: true, after: false }] },
{ code: "var arr = [ ,2];", options: [{ before: true, after: false }] },
{ code: "var arr = [1 ,2];", options: [{ before: true, after: false }] },
{ code: "var arr = [,,];", options: [{ before: true, after: false }] },
{ code: "var arr = [ ,,];", options: [{ before: true, after: false }] },
{ code: "var arr = [, ,];", options: [{ before: true, after: false }] },
{ code: "var arr = [,, ];", options: [{ before: true, after: false }] },
{ code: "var arr = [ , ,];", options: [{ before: true, after: false }] },
{ code: "var arr = [ ,, ];", options: [{ before: true, after: false }] },
{ code: "var arr = [, , ];", options: [{ before: true, after: false }] },
{ code: "var arr = [ , , ];", options: [{ before: true, after: false }] },
{ code: "var arr = [1 , ,];", options: [{ before: true, after: false }] },
{ code: "var arr = [ ,2 ,];", options: [{ before: true, after: false }] },
{ code: "var arr = [,2 , ];", options: [{ before: true, after: false }] },
{ code: "var arr = [ , ,3];", options: [{ before: true, after: false }] },
{ code: "var arr = [1 ,2 ,];", options: [{ before: true, after: false }] },
{ code: "var arr = [ ,2 ,3];", options: [{ before: true, after: false }] },
{ code: "var arr = [1 , ,3];", options: [{ before: true, after: false }] },
{ code: "var arr = [1 ,2 ,3];", options: [{ before: true, after: false }] },
{ code: "var obj = {'foo':'bar' , 'baz':'qur'};", options: [{ before: true, after: true }] },
{ code: "var obj = {'foo':'bar' ,'baz':'qur' , };", options: [{ before: true, after: false }] },
{ code: "var a = 1 , b = 2;", options: [{ before: true, after: true }] },
{ code: "var arr = [, ];", options: [{ before: true, after: true }] },
{ code: "var arr = [,,];", options: [{ before: true, after: true }] },
{ code: "var arr = [1 , ];", options: [{ before: true, after: true }] },
{ code: "var arr = [ , 2];", options: [{ before: true, after: true }] },
{ code: "var arr = [1 , 2];", options: [{ before: true, after: true }] },
Expand All @@ -107,6 +138,7 @@ ruleTester.run("comma-spacing", rule, {
{ code: "var arr = [ ,2];", options: [{ before: false, after: false }] },
{ code: "var arr = [1,2];", options: [{ before: false, after: false }] },
{ code: "var arr = [,,];", options: [{ before: false, after: false }] },
{ code: "var arr = [ , , ];", options: [{ before: false, after: false }] },
{ code: "var arr = [ ,,];", options: [{ before: false, after: false }] },
{ code: "var arr = [1,,];", options: [{ before: false, after: false }] },
{ code: "var arr = [,2,];", options: [{ before: false, after: false }] },
Expand All @@ -120,12 +152,22 @@ ruleTester.run("comma-spacing", rule, {
{ code: "var a; console.log(`${a}`, \"a\");", parserOptions: { ecmaVersion: 6 } },
{ code: "var [a, b] = [1, 2];", parserOptions: { ecmaVersion: 6 } },
{ code: "var [a, b, ] = [1, 2];", parserOptions: { ecmaVersion: 6 } },
{ code: "var [a, b,] = [1, 2];", parserOptions: { ecmaVersion: 6 } },
{ code: "var [a, , b] = [1, 2, 3];", parserOptions: { ecmaVersion: 6 } },
{ code: "var [a,, b] = [1, 2, 3];", parserOptions: { ecmaVersion: 6 } },
{ code: "var [ , b] = a;", parserOptions: { ecmaVersion: 6 } },
{ code: "var [, b] = a;", parserOptions: { ecmaVersion: 6 } },
{ code: "var { a,} = a;", parserOptions: { ecmaVersion: 6 } },
{ code: "import { a,} from 'mod';", parserOptions: { ecmaVersion: 6, sourceType: "module" } },
{ code: "<a>,</a>", parserOptions: { ecmaVersion: 6, ecmaFeatures: { jsx: true } } },
{ code: "<a> , </a>", parserOptions: { ecmaVersion: 6, ecmaFeatures: { jsx: true } } },
{ code: "<a>Hello, world</a>", options: [{ before: true, after: false }], parserOptions: { ecmaVersion: 6, ecmaFeatures: { jsx: true } } }
{ code: "<a>Hello, world</a>", options: [{ before: true, after: false }], parserOptions: { ecmaVersion: 6, ecmaFeatures: { jsx: true } } },

// For backwards compatibility. Ignoring spacing between a comment and comma of a null element was possibly unintentional.
{ code: "[a, /**/ , ]", options: [{ before: false, after: true }] },
{ code: "[a , /**/, ]", options: [{ before: true, after: true }] },
{ code: "[a, /**/ , ] = foo", options: [{ before: false, after: true }], parserOptions: { ecmaVersion: 6 } },
{ code: "[a , /**/, ] = foo", options: [{ before: true, after: true }], parserOptions: { ecmaVersion: 6 } }
],

invalid: [
Expand Down Expand Up @@ -198,17 +240,6 @@ ruleTester.run("comma-spacing", rule, {
}
]
},
{
code: "var arr = [1 , ];",
output: "var arr = [1 ,];",
options: [{ before: true, after: false }],
errors: [
{
message: "There should be no space after ','.",
type: "Punctuator"
}
]
},
{
code: "var arr = [1 ,2];",
output: "var arr = [1, 2];",
Expand Down

0 comments on commit bfe5e88

Please sign in to comment.