Skip to content

Commit

Permalink
prefer-spread: Fix more .concat cases (#1042)
Browse files Browse the repository at this point in the history 
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
  • Loading branch information
@fisker @sindresorhus
fisker and sindresorhus committed Jan 22, 2021
1 parent f5496c7 commit c3c7ba3
Show file tree
Hide file tree
Showing 5 changed files with 678 additions and 94 deletions.
242 changes: 182 additions & 60 deletions rules/prefer-spread.js
View file
@@ -1,10 +1,10 @@
'use strict';
const {getStaticValue, isCommaToken} = require('eslint-utils');
const {isParenthesized, 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 shouldAddParenthesesToSpreadElementArgument = require('./utils/should-add-parentheses-to-spread-element-argument');

const ERROR_ARRAY_FROM = 'array-from';
const ERROR_ARRAY_CONCAT = 'array-concat';
Expand All @@ -13,8 +13,8 @@ const SUGGESTION_CONCAT_ARGUMENT_IS_NOT_SPREADABLE = 'argument-is-not-spreadable
const messages = {
[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`.'
[SUGGESTION_CONCAT_ARGUMENT_IS_SPREADABLE]: 'First argument is an `array`.',
[SUGGESTION_CONCAT_ARGUMENT_IS_NOT_SPREADABLE]: 'First argument is not an `array`.'
};

const arrayFromCallSelector = [
Expand All @@ -41,66 +41,182 @@ const arrayConcatCallSelector = [
].join('');

const isArrayLiteral = node => node.type === 'ArrayExpression';
const isArrayLiteralHasTrailingComma = (node, sourceCode) => {
if (node.elements.length === 0) {
return false;
}

return isCommaToken(sourceCode.getLastToken(node, 1));
};

const getParenthesizedRange = (node, sourceCode) => {
const [firstToken = node, lastToken = node] = getParentheses(node, sourceCode);

function fixConcat(node, sourceCode, isSpreadable) {
const [start] = firstToken.range;
const [, end] = lastToken.range;
return [start, end];
};

function fixConcat(node, sourceCode, fixableArguments) {
const array = node.callee.object;
const [item] = node.arguments;
const concatCallArguments = node.arguments;
const arrayParenthesizedRange = getParenthesizedRange(array, sourceCode);
const arrayIsArrayLiteral = isArrayLiteral(array);
const arrayHasTrailingComma = arrayIsArrayLiteral && isArrayLiteralHasTrailingComma(array, sourceCode);

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

return [start, end];
};

const getParenthesizedArrayRange = () => {
const [firstToken = array, lastToken = array] = getParentheses(array, sourceCode);
const getArrayLiteralElementsText = (node, keepTrailingComma) => {
if (
!keepTrailingComma &&
isArrayLiteralHasTrailingComma(node, sourceCode)
) {
const start = node.range[0] + 1;
const end = sourceCode.getLastToken(node, 1).range[0];
return sourceCode.text.slice(start, end);
}

const [start] = firstToken.range;
const [, end] = lastToken.range;
return [start, end];
return sourceCode.getText(node, -1, -1);
};

const getFixedText = () => {
if (isArrayLiteral(item)) {
return sourceCode.getText(item, -1, -1);
}
const nonEmptyArguments = fixableArguments
.filter(({node, isArrayLiteral}) => (!isArrayLiteral || node.elements.length > 0));
const lastArgument = nonEmptyArguments[nonEmptyArguments.length - 1];

const text = getCallExpressionArgumentsText(node, sourceCode);
return isSpreadable ? `...${text}` : text;
};
let text = nonEmptyArguments
.map(({node, isArrayLiteral, isSpreadable}) => {
if (isArrayLiteral) {
return getArrayLiteralElementsText(node, node === lastArgument.node);
}

return function * (fixer) {
// Fixed code always starts with `[` or `(`
if (needsSemicolon(sourceCode.getTokenBefore(node), sourceCode, '[')) {
yield fixer.insertTextBefore(node, ';');
}
const [start, end] = getParenthesizedRange(node, sourceCode);
let text = sourceCode.text.slice(start, end);
if (isSpreadable) {
if (
!isParenthesized(node, sourceCode) &&
shouldAddParenthesesToSpreadElementArgument(node)
) {
text = `(${text})`;
}

const rangeAfterArray = getRangeAfterArray();
let text = getFixedText();
text = `...${text}`;
}

if (isArrayLiteral(array)) {
const [penultimateToken, closingBracketToken] = sourceCode.getLastTokens(array, 2);
return text || ' ';
})
.join(', ');

if (!text) {
return '';
}

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

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

if (
arrayHasTrailingComma &&
(!lastArgument.isArrayLiteral || !isArrayLiteralHasTrailingComma(lastArgument.node, sourceCode))
) {
text = `${text},`;
}
}
} else {
text = `, ${text}`;
}

return text;
};

function removeArguments(fixer) {
const [firstArgument] = concatCallArguments;
const lastArgument = concatCallArguments[fixableArguments.length - 1];

const [start] = getParenthesizedRange(firstArgument, sourceCode);
let [, end] = sourceCode.getTokenAfter(lastArgument, isCommaToken).range;

const textAfter = sourceCode.text.slice(end);
const [leadingSpaces] = textAfter.match(/^\s*/);
end += leadingSpaces.length;

return fixer.replaceTextRange([start, end], '');
}

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

yield (
concatCallArguments.length - fixableArguments.length === 0 ?
fixer.replaceTextRange(getRangeAfterArray(), '') :
removeArguments(fixer)
);

const text = getFixedText();

if (arrayIsArrayLiteral) {
const closingBracketToken = sourceCode.getLastToken(array);
yield fixer.insertTextBefore(closingBracketToken, text);
} else {
yield fixer.insertTextBefore(node, '[...');
yield fixer.insertTextAfterRange(getParenthesizedArrayRange(), `, ${text}`);
yield fixer.insertTextAfter(node, ']');
// The array is already accessing `.concat`, there should not any case need add extra `()`
yield fixer.insertTextBeforeRange(arrayParenthesizedRange, '[...');
yield fixer.insertTextAfterRange(arrayParenthesizedRange, text);
yield fixer.insertTextAfterRange(arrayParenthesizedRange, ']');
}

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

const getConcatArgumentSpreadable = (node, scope) => {
if (node.type === 'SpreadElement') {
return;
}

if (isArrayLiteral(node)) {
return {node, isArrayLiteral: true};
}

const result = getStaticValue(node, scope);

if (!result) {
return;
}

const isSpreadable = Array.isArray(result.value);

return {node, isSpreadable};
};

function getConcatFixableArguments(argumentsList, scope) {
const fixableArguments = [];

for (const node of argumentsList) {
const result = getConcatArgumentSpreadable(node, scope);

if (result) {
fixableArguments.push(result);
} else {
break;
}
}

return fixableArguments;
}

const create = context => {
const sourceCode = context.getSourceCode();
const getSource = node => sourceCode.getText(node);
Expand Down Expand Up @@ -138,39 +254,45 @@ const create = context => {
messageId: ERROR_ARRAY_CONCAT
};

const [item] = node.arguments;
if (node.arguments.length !== 1 || item.type === 'SpreadElement') {
const fixableArguments = getConcatFixableArguments(node.arguments, scope);

if (fixableArguments.length > 0 || node.arguments.length === 0) {
problem.fix = fixConcat(node, sourceCode, fixableArguments);
context.report(problem);
return;
}

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

if (result) {
isItemArray = Array.isArray(result.value);
}
const [firstArgument, ...restArguments] = node.arguments;
if (firstArgument.type === 'SpreadElement') {
context.report(problem);
return;
}

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)
}
];
}
const fixableArgumentsAfterFirstArgument = getConcatFixableArguments(restArguments, scope);
problem.suggest = [
{
messageId: SUGGESTION_CONCAT_ARGUMENT_IS_SPREADABLE,
isSpreadable: true
},
{
messageId: SUGGESTION_CONCAT_ARGUMENT_IS_NOT_SPREADABLE,
isSpreadable: false
}
].map(({messageId, isSpreadable}) => ({
messageId,
fix: fixConcat(
node,
sourceCode,
// When apply suggestion, we also merge fixable arguments after the first one
[
{
node: firstArgument,
isSpreadable
},
...fixableArgumentsAfterFirstArgument
]
)
}));

context.report(problem);
}
Expand Down
22 changes: 22 additions & 0 deletions rules/utils/should-add-parentheses-to-spread-element-argument.js
View file
@@ -0,0 +1,22 @@
'use strict';

const nodeTypesDoNotNeedParentheses = new Set([
'CallExpression',
'Identifier',
'Literal',
'MemberExpression',
'NewExpression',
'TemplateLiteral',
'ThisExpression'
]);

/**
Check if parentheses should be added to a `node` when it's used as `argument` of `SpreadElement`.
@param {Node} node - The AST node to check.
@returns {boolean}
*/
const shouldAddParenthesesToSpreadElementArgument = node =>
!nodeTypesDoNotNeedParentheses.has(node.type);

module.exports = shouldAddParenthesesToSpreadElementArgument;

0 comments on commit c3c7ba3

Please sign in to comment.