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 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
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
22 changes: 22 additions & 0 deletions packages/eslint-plugin/docs/rules/comma-spacing.md
@@ -0,0 +1,22 @@
# Enforces consistent spacing before and after commas (`comma-spacing`)

## Rule Details

This rule extends the base [`eslint/comma-spacing`](https://eslint.org/docs/rules/comma-spacing) rule.
It adds support for trailing comma in a types parameters list.

## 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"]
}
```

## Options

See [`eslint/comma-spacing` options](https://eslint.org/docs/rules/comma-spacing#options).

<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