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.

Sign up for GitHub

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

Jump to bottom

fix: ignore spacing before ] and } in comma-spacing #16113

Merged
merged 1 commit into from Jul 8, 2022
Merged

fix: ignore spacing before ] and } in comma-spacing #16113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
48 changes: 29 additions & 19 deletions docs/src/rules/comma-spacing.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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"
}
]
},
Comment on lines -201 to -211
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was conflicting with array-bracket-spacing option "always" on the right side of ,.

Playground link

{
code: "var arr = [1 ,2];",
output: "var arr = [1, 2];",
Expand Down