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

prefer-spread: Prefer ... over Array#concat() #1029

Merged
merged 15 commits into from
Jan 19, 2021
18 changes: 12 additions & 6 deletions docs/rules/prefer-spread.md
Original file line number Diff line number Diff line change
@@ -1,19 +1,25 @@
# Prefer the spread operator over `Array.from()`
# Prefer the spread operator over `Array.from()` and `Array#concat()`

Enforces the use of the spread operator over `Array.from()`. This rule adds on to the built-in [prefer-spread](https://eslint.org/docs/rules/prefer-spread) rule, which only flags uses of `.apply()`. Does not enforce for `TypedArray.from()`;

This rule is fixable.
Enforces the use of the spread operator over `Array.from()` and `Array#concat()`. This rule adds on to the built-in [prefer-spread](https://eslint.org/docs/rules/prefer-spread) rule, which only flags uses of `.apply()`. Does not enforce for `TypedArray.from()`;

This rule is partly fixable.

## Fail

```js
Array.from(set).map(() => {});
Array.from(set).map(element => foo(element));
```

```js
const array = array1.concat(array2);
```

## Pass

```js
[...set].map(() => {});
[...set].map(element => foo(element));
```

```js
const array = [...array1, ...array2];
```
2 changes: 1 addition & 1 deletion readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ Configure it in `package.json`.
- [prefer-reflect-apply](docs/rules/prefer-reflect-apply.md) - Prefer `Reflect.apply()` over `Function#apply()`. *(fixable)*
- [prefer-regexp-test](docs/rules/prefer-regexp-test.md) - Prefer `RegExp#test()` over `String#match()` and `RegExp#exec()`. *(fixable)*
- [prefer-set-has](docs/rules/prefer-set-has.md) - Prefer `Set#has()` over `Array#includes()` when checking for existence or non-existence. *(fixable)*
- [prefer-spread](docs/rules/prefer-spread.md) - Prefer the spread operator over `Array.from()`. *(fixable)*
- [prefer-spread](docs/rules/prefer-spread.md) - Prefer the spread operator over `Array.from()` and `Array#concat()`. *(partly fixable)*
- [prefer-string-replace-all](docs/rules/prefer-string-replace-all.md) - Prefer `String#replaceAll()` over regex searches with the global flag. *(fixable)*
- [prefer-string-slice](docs/rules/prefer-string-slice.md) - Prefer `String#slice()` over `String#substr()` and `String#substring()`. *(partly fixable)*
- [prefer-string-starts-ends-with](docs/rules/prefer-string-starts-ends-with.md) - Prefer `String#startsWith()` & `String#endsWith()` over `RegExp#test()`. *(fixable)*
Expand Down
13 changes: 2 additions & 11 deletions rules/no-array-push-push.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
'use strict';
const {hasSideEffect, isCommaToken, isOpeningParenToken, isSemicolonToken} = require('eslint-utils');
const {hasSideEffect, isCommaToken, isSemicolonToken} = require('eslint-utils');
const getDocumentationUrl = require('./utils/get-documentation-url');
const methodSelector = require('./utils/method-selector');
const getCallExpressionArgumentsText = require('./utils/get-call-expression-arguments-text');

const ERROR = 'error';
const SUGGESTION = 'suggestion';
Expand All @@ -20,16 +21,6 @@ const arrayPushExpressionStatement = [

const selector = `${arrayPushExpressionStatement} + ${arrayPushExpressionStatement}`;

const getCallExpressionArgumentsText = (node, sourceCode) => {
const openingParenthesisToken = sourceCode.getTokenAfter(node.callee, isOpeningParenToken);
const closingParenthesisToken = sourceCode.getLastToken(node);

return sourceCode.text.slice(
openingParenthesisToken.range[1],
closingParenthesisToken.range[0]
);
};

function getFirstExpression(node, sourceCode) {
const {parent} = node;
const visitorKeys = sourceCode.visitorKeys[parent.type] || Object.keys(parent);
Expand Down
2 changes: 1 addition & 1 deletion rules/no-unused-properties.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ const create = context => {
return;
}

const nextPath = path.concat(key);
const nextPath = [...path, key];

const nextReferences = references
.map(reference => {
Expand Down
141 changes: 136 additions & 5 deletions rules/prefer-spread.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,23 @@
'use strict';
const {getStaticValue, isCommaToken} = require('eslint-utils');
const getDocumentationUrl = require('./utils/get-documentation-url');
const methodSelector = require('./utils/method-selector');
const needsSemicolon = require('./utils/needs-semicolon');
const getParentheses = require('./utils/get-parentheses');
const getCallExpressionArgumentsText = require('./utils/get-call-expression-arguments-text');

const MESSAGE_ID = 'prefer-spread';
const ERROR_ARRAY_FROM = 'array-from';
const ERROR_ARRAY_CONCAT = 'array-concat';
const SUGGESTION_CONCAT_ARGUMENT_IS_SPREADABLE = 'argument-is-spreadable';
const SUGGESTION_CONCAT_ARGUMENT_IS_NOT_SPREADABLE = 'argument-is-not-spreadable';
const messages = {
[MESSAGE_ID]: 'Prefer the spread operator over `Array.from()`.'
[ERROR_ARRAY_FROM]: 'Prefer the spread operator over `Array.from(…)`.',
[ERROR_ARRAY_CONCAT]: 'Prefer the spread operator over `Array#concat(…)`.',
[SUGGESTION_CONCAT_ARGUMENT_IS_SPREADABLE]: 'Argument of `Array#concat(…)` is an `array`.',
[SUGGESTION_CONCAT_ARGUMENT_IS_NOT_SPREADABLE]: 'Argument of `Array#concat(…)` is not an `array`.'
};

const selector = [
const arrayFromCallSelector = [
methodSelector({
object: 'Array',
name: 'from',
Expand All @@ -19,15 +28,88 @@ const selector = [
'[arguments.0.type!="ObjectExpression"]'
].join('');

const arrayConcatCallSelector = [
methodSelector({
name: 'concat'
}),
`:not(${
[
'Literal',
'TemplateLiteral'
].map(type => `[callee.object.type="${type}"]`).join(', ')
})`
].join('');

const isArrayLiteral = node => node.type === 'ArrayExpression';

function fixConcat(node, sourceCode, isSpreadable) {
const array = node.callee.object;
const [item] = node.arguments;

const getRangeAfterArray = () => {
const [, start] = getParenthesizedArrayRange();
const [, end] = node.range;

return [start, end];
};

const getParenthesizedArrayRange = () => {
const [firstToken = array, lastToken = array] = getParentheses(array, sourceCode);

const [start] = firstToken.range;
const [, end] = lastToken.range;
return [start, end];
};

const getFixedText = () => {
if (isArrayLiteral(item)) {
return sourceCode.getText(item, -1, -1);
}

const text = getCallExpressionArgumentsText(node, sourceCode);
return isSpreadable ? `...${text}` : text;
};

return function * (fixer) {
// Fixed code always starts with `[` or `(`
if (needsSemicolon(sourceCode.getTokenBefore(node), sourceCode, '[')) {
yield fixer.insertTextBefore(node, ';');
}

const rangeAfterArray = getRangeAfterArray();
let text = getFixedText();

if (isArrayLiteral(array)) {
const [penultimateToken, closingBracketToken] = sourceCode.getLastTokens(array, 2);

if (array.elements.length > 0) {
text = ` ${text}`;

if (!isCommaToken(penultimateToken)) {
text = `,${text}`;
}
}

yield fixer.insertTextBefore(closingBracketToken, text);
} else {
yield fixer.insertTextBefore(node, '[...');
yield fixer.insertTextAfterRange(getParenthesizedArrayRange(), `, ${text}`);
yield fixer.insertTextAfter(node, ']');
}

yield fixer.replaceTextRange(rangeAfterArray, '');
};
}

const create = context => {
const sourceCode = context.getSourceCode();
const getSource = node => sourceCode.getText(node);

return {
[selector](node) {
[arrayFromCallSelector](node) {
context.report({
node,
messageId: MESSAGE_ID,
messageId: ERROR_ARRAY_FROM,
fix: fixer => {
const [arrayLikeArgument, mapFn, thisArgument] = node.arguments.map(node => getSource(node));
let replacement = `${
Expand All @@ -42,6 +124,55 @@ const create = context => {
return fixer.replaceText(node, replacement);
}
});
},
[arrayConcatCallSelector](node) {
const scope = context.getScope();
const staticResult = getStaticValue(node.callee.object, scope);

if (staticResult && !Array.isArray(staticResult.value)) {
return;
}

const problem = {
node: node.callee.property,
messageId: ERROR_ARRAY_CONCAT
};

const [item] = node.arguments;
if (node.arguments.length !== 1 || item.type === 'SpreadElement') {
context.report(problem);
return;
}

let isItemArray;
if (isArrayLiteral(item)) {
isItemArray = true;
} else {
const result = getStaticValue(item, scope);

if (result) {
isItemArray = Array.isArray(result.value);
}
}

if (isItemArray === true) {
problem.fix = fixConcat(node, sourceCode, /* isSpreadable */ true);
} else if (isItemArray === false) {
problem.fix = fixConcat(node, sourceCode, /* isSpreadable */ false);
} else {
problem.suggest = [
{
messageId: SUGGESTION_CONCAT_ARGUMENT_IS_SPREADABLE,
fix: fixConcat(node, sourceCode, /* isSpreadable */ true)
},
{
messageId: SUGGESTION_CONCAT_ARGUMENT_IS_NOT_SPREADABLE,
fix: fixConcat(node, sourceCode, /* isSpreadable */ false)
}
];
}

context.report(problem);
}
};
};
Expand Down
7 changes: 5 additions & 2 deletions rules/prevent-abbreviations.js
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ const create = context => {
scope: variable.scope,
defs: variable.defs,
identifiers: variable.identifiers,
references: variable.references.concat(outerClassVariable.references)
references: [...variable.references, ...outerClassVariable.references]
};

// Call the common checker with the newly forged normalized class variable
Expand Down Expand Up @@ -640,7 +640,10 @@ const create = context => {
return;
}

const scopes = variable.references.map(reference => reference.from).concat(variable.scope);
const scopes = [
...variable.references.map(reference => reference.from),
variable.scope
];
variableReplacements.samples = variableReplacements.samples.map(
name => avoidCapture(name, scopes, ecmaVersion, isSafeName)
);
Expand Down
21 changes: 21 additions & 0 deletions rules/utils/get-call-expression-arguments-text.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
'use strict';
const {isOpeningParenToken} = require('eslint-utils');

/**
Get the text of the arguments list of `CallExpression`.

@param {Node} node - The `CallExpression` node.
@param {SourceCode} sourceCode - The source code object.
@returns {string}
*/
const getCallExpressionArgumentsText = (node, sourceCode) => {
const openingParenthesisToken = sourceCode.getTokenAfter(node.callee, isOpeningParenToken);
const closingParenthesisToken = sourceCode.getLastToken(node);

return sourceCode.text.slice(
openingParenthesisToken.range[1],
closingParenthesisToken.range[0]
);
};

module.exports = getCallExpressionArgumentsText;
25 changes: 25 additions & 0 deletions rules/utils/get-parentheses.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
'use strict';
const {isOpeningParenToken, isClosingParenToken} = require('eslint-utils');
const getParenthesizedTimes = require('./get-parenthesized-times');

/*
Get the first opening parenthesis token and the last closing parenthesis token of a parenthesized node.

@param {Node} node - The node to be checked.
@param {SourceCode} sourceCode - The source code object.
@returns {Token[]}
*/
function getParentheses(node, sourceCode) {
const parenthesizedTimes = getParenthesizedTimes(node, sourceCode);

if (parenthesizedTimes > 0) {
return [
sourceCode.getTokenBefore(node, {skip: parenthesizedTimes - 1, filter: isOpeningParenToken}),
sourceCode.getTokenAfter(node, {skip: parenthesizedTimes - 1, filter: isClosingParenToken})
];
}

return [];
}

module.exports = getParentheses;
11 changes: 5 additions & 6 deletions rules/utils/get-references.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
'use strict';
const {uniq} = require('lodash');
const {uniq, flatten} = require('lodash');

const getReferences = scope => uniq(
scope.references.concat(
...scope.childScopes.map(scope => getReferences(scope))
)
);
const getReferences = scope => uniq([
...scope.references,
...flatten(scope.childScopes.map(scope => getReferences(scope)))
]);

module.exports = getReferences;