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

feat(eslint-plugin): add comma-spacing #1495

Merged
merged 3 commits into from Jan 23, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Expand Up @@ -169,6 +169,7 @@ In these cases, we create what we call an extension rule; a rule within our plug
| Name | Description | :heavy_check_mark: | :wrench: | :thought_balloon: |
| --------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------ | -------- | ----------------- |
| [`@typescript-eslint/brace-style`](./docs/rules/brace-style.md) | Enforce consistent brace style for blocks | | :wrench: | |
| [`@typescript-eslint/comma-spacing`](./docs/rules/comma-spacing.md) | Enforces consistent spacing before and after commas | | :wrench: | |
| [`@typescript-eslint/default-param-last`](./docs/rules/default-param-last.md) | Enforce default parameters to be last | | | |
| [`@typescript-eslint/func-call-spacing`](./docs/rules/func-call-spacing.md) | Require or disallow spacing between function identifiers and their invocations | | :wrench: | |
| [`@typescript-eslint/indent`](./docs/rules/indent.md) | Enforce consistent indentation | | :wrench: | |
Expand Down
135 changes: 135 additions & 0 deletions packages/eslint-plugin/docs/rules/comma-spacing.md
@@ -0,0 +1,135 @@
# Enforces consistent spacing before and after commas (`comma-spacing`)

Spacing around commas improves readability of a list of items. Although most of the style guidelines for languages prescribe adding a space after a comma and not before it, it is subjective to the preferences of a project.
a-tarasyuk marked this conversation as resolved.
Show resolved Hide resolved

<!-- prettier-ignore -->
```ts
const foo = 1, bar = 2;
const foo = 1 ,bar = 2;
```

## Rule Details

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:

- adjacent null elements
- an initial null element, to avoid conflicts with the [`array-bracket-spacing`](https://github.com/eslint/eslint/blob/master/docs/rules/array-bracket-spacing.md) rule

## Options

This rule has an object option:

- `"before": false` (default) disallows spaces before commas
- `"before": true` requires one or more spaces before commas
- `"after": true` (default) requires one or more spaces after commas
- `"after": false` disallows spaces after commas

### after

Examples of **incorrect** code for this rule with the default `{ "before": false, "after": true }` options:

<!-- prettier-ignore -->
```ts
/*eslint @typescript-eslint/comma-spacing: ["error", { "before": false, "after": true }]*/

const foo = 1 ,bar = 2;
const arr = [1 , 2];
const obj = {"foo": "bar" ,"baz": "qur"};
foo(a ,b);
new Foo(a ,b);
function foo(a ,b){}
a ,b;
function foo<T ,T1>() {}
```

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

<!-- prettier-ignore -->
```ts
/*eslint @typescript-eslint/comma-spacing: ["error", { "before": false, "after": true }] */

const foo = 1, bar = 2
, baz = 3;
const arr = [1, 2];
const arr = [1,, 3]
const obj = {"foo": "bar", "baz": "qur"};
foo(a, b);
new Foo(a, b);
function foo(a, b){}
a, b;
function foo<T, T1>() {}
function foo<T,>() {}
```

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

<!-- prettier-ignore -->
```ts
/*eslint @typescript-eslint/comma-spacing: ["error", { "before": false, "after": true }]*/
/*eslint array-bracket-spacing: ["error", "always"]*/

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

### before

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

<!-- prettier-ignore -->
```ts
/*eslint @typescript-eslint/comma-spacing: ["error", { "before": true, "after": false }]*/

const foo = 1, bar = 2;
const arr = [1 , 2];
const obj = {"foo": "bar", "baz": "qur"};
new Foo(a,b);
function foo(a,b){}
a, b;
function foo<T,T1>() {}
```

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

<!-- prettier-ignore -->
```ts
/*eslint @typescript-eslint/comma-spacing: ["error", { "before": true, "after": false }]*/

const foo = 1 ,bar = 2 ,
baz = true;
const arr = [1 ,2];
const arr = [1 ,,3]
const obj = {"foo": "bar" ,"baz": "qur"};
foo(a ,b);
new Foo(a ,b);
function foo(a ,b){}
a ,b;
function foo<T ,T1>() {}
```

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

<!-- prettier-ignore -->
```ts
/*eslint @typescript-eslint/comma-spacing: ["error", { "before": true, "after": false }]*/
/*eslint array-bracket-spacing: ["error", "never"]*/

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

## How to use

```cjson
{
// note you must disable the base rule as it can report incorrect errors
"comma-spacing": "off",
"@typescript-eslint/comma-spacing": ["error"]
}
```

## When Not To Use It

If your project will not be following a consistent comma-spacing pattern, turn this rule off.

<sup>Taken with ❤️ [from ESLint core](https://github.com/eslint/eslint/blob/master/docs/rules/comma-spacing.md)</sup>
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/configs/all.json
Expand Up @@ -8,6 +8,8 @@
"@typescript-eslint/ban-types": "error",
"brace-style": "off",
"@typescript-eslint/brace-style": "error",
"comma-spacing": "off",
"@typescript-eslint/comma-spacing": "error",
"@typescript-eslint/consistent-type-assertions": "error",
"@typescript-eslint/consistent-type-definitions": "error",
"default-param-last": "off",
Expand Down
190 changes: 190 additions & 0 deletions packages/eslint-plugin/src/rules/comma-spacing.ts
@@ -0,0 +1,190 @@
import {
TSESTree,
AST_TOKEN_TYPES,
} from '@typescript-eslint/experimental-utils';
import { isClosingParenToken, isCommaToken } from 'eslint-utils';
armano2 marked this conversation as resolved.
Show resolved Hide resolved
import { isTokenOnSameLine, createRule } from '../util';

type Options = [
{
before: boolean;
after: boolean;
},
];
type MessageIds = 'unexpected' | 'missing';

export default createRule<Options, MessageIds>({
name: 'comma-spacing',
meta: {
type: 'suggestion',
docs: {
description: 'Enforces consistent spacing before and after commas',
category: 'Stylistic Issues',
recommended: false,
extendsBaseRule: true,
},
fixable: 'whitespace',
schema: [
{
type: 'object',
properties: {
before: {
type: 'boolean',
default: false,
},
after: {
type: 'boolean',
default: true,
},
},
additionalProperties: false,
},
],
messages: {
unexpected: `There should be no space {{loc}} ','.`,
missing: `A space is required {{loc}} ','.`,
},
},
defaultOptions: [
{
before: false,
after: true,
},
],
create(context, [{ before: spaceBefore, after: spaceAfter }]) {
const sourceCode = context.getSourceCode();
const tokensAndComments = sourceCode.tokensAndComments;
const ignoredTokens = new Set<TSESTree.Token>();

/**
* Adds null elements of the ArrayExpression or ArrayPattern node to the ignore list
* @param node node to evaluate
*/
function addNullElementsToIgnoreList(
node: TSESTree.ArrayExpression | TSESTree.ArrayPattern,
): void {
let previousToken = sourceCode.getFirstToken(node);
for (const element of node.elements) {
let token: TSESTree.Token | null;
if (element === null) {
token = sourceCode.getTokenAfter(previousToken as TSESTree.Token);
if (token && isCommaToken(token)) {
ignoredTokens.add(token);
}
} else {
token = sourceCode.getTokenAfter(element);
}

previousToken = token;
}
}

/**
* Adds type parameters trailing comma token to the ignore list
* @param node node to evaluate
*/
function addTypeParametersTrailingCommaToIgnoreList(
node: TSESTree.TSTypeParameterDeclaration,
): void {
const param = node.params[node.params.length - 1];
const afterToken = sourceCode.getTokenAfter(param);
if (afterToken && isCommaToken(afterToken)) {
ignoredTokens.add(afterToken);
}
}

/**
* Validates the spacing around a comma token.
* @param commaToken The token representing the comma
* @param prevToken The last token before the comma
* @param nextToken The first token after the comma
*/
function validateCommaSpacing(
commaToken: TSESTree.Token,
prevToken: TSESTree.Token | null,
nextToken: TSESTree.Token | null,
): void {
if (
prevToken &&
isTokenOnSameLine(prevToken, commaToken) &&
spaceBefore !== sourceCode.isSpaceBetween(prevToken, commaToken)
) {
context.report({
node: commaToken,
data: {
loc: 'before',
},
messageId: spaceBefore ? 'missing' : 'unexpected',
fix: fixer =>
spaceBefore
? fixer.insertTextBefore(commaToken, ' ')
: fixer.replaceTextRange(
[prevToken.range[1], commaToken.range[0]],
'',
),
});
}

if (nextToken && isClosingParenToken(nextToken)) {
return;
}

if (!spaceAfter && nextToken && nextToken.type === AST_TOKEN_TYPES.Line) {
return;
}

if (
nextToken &&
isTokenOnSameLine(commaToken, nextToken) &&
spaceAfter !== sourceCode.isSpaceBetween(commaToken, nextToken)
) {
context.report({
node: commaToken,
data: {
loc: 'after',
},
messageId: spaceAfter ? 'missing' : 'unexpected',
fix: fixer =>
spaceAfter
? fixer.insertTextAfter(commaToken, ' ')
: fixer.replaceTextRange(
[commaToken.range[1], nextToken.range[0]],
'',
),
});
}
}

return {
TSTypeParameterDeclaration: addTypeParametersTrailingCommaToIgnoreList,
ArrayExpression: addNullElementsToIgnoreList,
ArrayPattern: addNullElementsToIgnoreList,

'Program:exit'(): void {
tokensAndComments.forEach((token, i) => {
if (!isCommaToken(token)) {
return;
}

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

const commaToken = token as TSESTree.Token;
const prevToken = tokensAndComments[i - 1] as TSESTree.Token;
const nextToken = tokensAndComments[i + 1] as TSESTree.Token;

validateCommaSpacing(
commaToken,
isCommaToken(prevToken) || ignoredTokens.has(commaToken)
? null
: prevToken,
isCommaToken(nextToken) || ignoredTokens.has(commaToken)
? null
: nextToken,
);
});
},
};
},
});
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
Expand Up @@ -7,6 +7,7 @@ import banTypes from './ban-types';
import braceStyle from './brace-style';
import camelcase from './camelcase';
import classNameCasing from './class-name-casing';
import commaSpacing from './comma-spacing';
import consistentTypeAssertions from './consistent-type-assertions';
import consistentTypeDefinitions from './consistent-type-definitions';
import defaultParamLast from './default-param-last';
Expand Down Expand Up @@ -92,6 +93,7 @@ export default {
'brace-style': braceStyle,
camelcase: camelcase,
'class-name-casing': classNameCasing,
'comma-spacing': commaSpacing,
'consistent-type-assertions': consistentTypeAssertions,
'consistent-type-definitions': consistentTypeDefinitions,
'default-param-last': defaultParamLast,
Expand Down
4 changes: 2 additions & 2 deletions packages/eslint-plugin/src/util/astUtils.ts
Expand Up @@ -55,8 +55,8 @@ function isLogicalOrOperator(node: TSESTree.Node): boolean {
* Determines whether two adjacent tokens are on the same line
*/
function isTokenOnSameLine(
left: TSESTree.Token,
right: TSESTree.Token,
left: TSESTree.Token | TSESTree.Comment,
right: TSESTree.Token | TSESTree.Comment,
): boolean {
return left.loc.end.line === right.loc.start.line;
}
Expand Down