Skip to content

Commit c3c7ba3

Browse files
fiskersindresorhus
fisker
and
sindresorhus
authored
prefer-spread: Fix more .concat cases (#1042)
Co-authored-by: Sindre Sorhus <[email protected]>
1 parent f5496c7 commit c3c7ba3

File tree

5 files changed

+678
-94
lines changed

5 files changed

+678
-94
lines changed

‎rules/prefer-spread.js‎

Lines changed: 182 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
'use strict';
2-
const {getStaticValue, isCommaToken} = require('eslint-utils');
2+
const {isParenthesized, getStaticValue, isCommaToken} = require('eslint-utils');
33
const getDocumentationUrl = require('./utils/get-documentation-url');
44
const methodSelector = require('./utils/method-selector');
55
const needsSemicolon = require('./utils/needs-semicolon');
66
const getParentheses = require('./utils/get-parentheses');
7-
const getCallExpressionArgumentsText = require('./utils/get-call-expression-arguments-text');
7+
const shouldAddParenthesesToSpreadElementArgument = require('./utils/should-add-parentheses-to-spread-element-argument');
88

99
const ERROR_ARRAY_FROM = 'array-from';
1010
const ERROR_ARRAY_CONCAT = 'array-concat';
@@ -13,8 +13,8 @@ const SUGGESTION_CONCAT_ARGUMENT_IS_NOT_SPREADABLE = 'argument-is-not-spreadable
1313
const messages = {
1414
[ERROR_ARRAY_FROM]: 'Prefer the spread operator over `Array.from(…)`.',
1515
[ERROR_ARRAY_CONCAT]: 'Prefer the spread operator over `Array#concat(…)`.',
16-
[SUGGESTION_CONCAT_ARGUMENT_IS_SPREADABLE]: 'Argument of `Array#concat(…)` is an `array`.',
17-
[SUGGESTION_CONCAT_ARGUMENT_IS_NOT_SPREADABLE]: 'Argument of `Array#concat(…)` is not an `array`.'
16+
[SUGGESTION_CONCAT_ARGUMENT_IS_SPREADABLE]: 'First argument is an `array`.',
17+
[SUGGESTION_CONCAT_ARGUMENT_IS_NOT_SPREADABLE]: 'First argument is not an `array`.'
1818
};
1919

2020
const arrayFromCallSelector = [
@@ -41,66 +41,182 @@ const arrayConcatCallSelector = [
4141
].join('');
4242

4343
const isArrayLiteral = node => node.type === 'ArrayExpression';
44+
const isArrayLiteralHasTrailingComma = (node, sourceCode) => {
45+
if (node.elements.length === 0) {
46+
return false;
47+
}
48+
49+
return isCommaToken(sourceCode.getLastToken(node, 1));
50+
};
51+
52+
const getParenthesizedRange = (node, sourceCode) => {
53+
const [firstToken = node, lastToken = node] = getParentheses(node, sourceCode);
4454

45-
function fixConcat(node, sourceCode, isSpreadable) {
55+
const [start] = firstToken.range;
56+
const [, end] = lastToken.range;
57+
return [start, end];
58+
};
59+
60+
function fixConcat(node, sourceCode, fixableArguments) {
4661
const array = node.callee.object;
47-
const [item] = node.arguments;
62+
const concatCallArguments = node.arguments;
63+
const arrayParenthesizedRange = getParenthesizedRange(array, sourceCode);
64+
const arrayIsArrayLiteral = isArrayLiteral(array);
65+
const arrayHasTrailingComma = arrayIsArrayLiteral && isArrayLiteralHasTrailingComma(array, sourceCode);
4866

4967
const getRangeAfterArray = () => {
50-
const [, start] = getParenthesizedArrayRange();
68+
const [, start] = arrayParenthesizedRange;
5169
const [, end] = node.range;
5270

5371
return [start, end];
5472
};
5573

56-
const getParenthesizedArrayRange = () => {
57-
const [firstToken = array, lastToken = array] = getParentheses(array, sourceCode);
74+
const getArrayLiteralElementsText = (node, keepTrailingComma) => {
75+
if (
76+
!keepTrailingComma &&
77+
isArrayLiteralHasTrailingComma(node, sourceCode)
78+
) {
79+
const start = node.range[0] + 1;
80+
const end = sourceCode.getLastToken(node, 1).range[0];
81+
return sourceCode.text.slice(start, end);
82+
}
5883

59-
const [start] = firstToken.range;
60-
const [, end] = lastToken.range;
61-
return [start, end];
84+
return sourceCode.getText(node, -1, -1);
6285
};
6386

6487
const getFixedText = () => {
65-
if (isArrayLiteral(item)) {
66-
return sourceCode.getText(item, -1, -1);
67-
}
88+
const nonEmptyArguments = fixableArguments
89+
.filter(({node, isArrayLiteral}) => (!isArrayLiteral || node.elements.length > 0));
90+
const lastArgument = nonEmptyArguments[nonEmptyArguments.length - 1];
6891

69-
const text = getCallExpressionArgumentsText(node, sourceCode);
70-
return isSpreadable ? `...${text}` : text;
71-
};
92+
let text = nonEmptyArguments
93+
.map(({node, isArrayLiteral, isSpreadable}) => {
94+
if (isArrayLiteral) {
95+
return getArrayLiteralElementsText(node, node === lastArgument.node);
96+
}
7297

73-
return function * (fixer) {
74-
// Fixed code always starts with `[` or `(`
75-
if (needsSemicolon(sourceCode.getTokenBefore(node), sourceCode, '[')) {
76-
yield fixer.insertTextBefore(node, ';');
77-
}
98+
const [start, end] = getParenthesizedRange(node, sourceCode);
99+
let text = sourceCode.text.slice(start, end);
100+
if (isSpreadable) {
101+
if (
102+
!isParenthesized(node, sourceCode) &&
103+
shouldAddParenthesesToSpreadElementArgument(node)
104+
) {
105+
text = `(${text})`;
106+
}
78107

79-
const rangeAfterArray = getRangeAfterArray();
80-
let text = getFixedText();
108+
text = `...${text}`;
109+
}
81110

82-
if (isArrayLiteral(array)) {
83-
const [penultimateToken, closingBracketToken] = sourceCode.getLastTokens(array, 2);
111+
return text || ' ';
112+
})
113+
.join(', ');
84114

115+
if (!text) {
116+
return '';
117+
}
118+
119+
if (arrayIsArrayLiteral) {
85120
if (array.elements.length > 0) {
86121
text = ` ${text}`;
87122

88-
if (!isCommaToken(penultimateToken)) {
123+
if (!arrayHasTrailingComma) {
89124
text = `,${text}`;
90125
}
126+
127+
if (
128+
arrayHasTrailingComma &&
129+
(!lastArgument.isArrayLiteral || !isArrayLiteralHasTrailingComma(lastArgument.node, sourceCode))
130+
) {
131+
text = `${text},`;
132+
}
91133
}
134+
} else {
135+
text = `, ${text}`;
136+
}
137+
138+
return text;
139+
};
140+
141+
function removeArguments(fixer) {
142+
const [firstArgument] = concatCallArguments;
143+
const lastArgument = concatCallArguments[fixableArguments.length - 1];
144+
145+
const [start] = getParenthesizedRange(firstArgument, sourceCode);
146+
let [, end] = sourceCode.getTokenAfter(lastArgument, isCommaToken).range;
147+
148+
const textAfter = sourceCode.text.slice(end);
149+
const [leadingSpaces] = textAfter.match(/^\s*/);
150+
end += leadingSpaces.length;
151+
152+
return fixer.replaceTextRange([start, end], '');
153+
}
154+
155+
return function * (fixer) {
156+
// Fixed code always starts with `[`
157+
if (
158+
!arrayIsArrayLiteral &&
159+
needsSemicolon(sourceCode.getTokenBefore(node), sourceCode, '[')
160+
) {
161+
yield fixer.insertTextBefore(node, ';');
162+
}
163+
164+
yield (
165+
concatCallArguments.length - fixableArguments.length === 0 ?
166+
fixer.replaceTextRange(getRangeAfterArray(), '') :
167+
removeArguments(fixer)
168+
);
169+
170+
const text = getFixedText();
92171

172+
if (arrayIsArrayLiteral) {
173+
const closingBracketToken = sourceCode.getLastToken(array);
93174
yield fixer.insertTextBefore(closingBracketToken, text);
94175
} else {
95-
yield fixer.insertTextBefore(node, '[...');
96-
yield fixer.insertTextAfterRange(getParenthesizedArrayRange(), `, ${text}`);
97-
yield fixer.insertTextAfter(node, ']');
176+
// The array is already accessing `.concat`, there should not any case need add extra `()`
177+
yield fixer.insertTextBeforeRange(arrayParenthesizedRange, '[...');
178+
yield fixer.insertTextAfterRange(arrayParenthesizedRange, text);
179+
yield fixer.insertTextAfterRange(arrayParenthesizedRange, ']');
98180
}
99-
100-
yield fixer.replaceTextRange(rangeAfterArray, '');
101181
};
102182
}
103183

184+
const getConcatArgumentSpreadable = (node, scope) => {
185+
if (node.type === 'SpreadElement') {
186+
return;
187+
}
188+
189+
if (isArrayLiteral(node)) {
190+
return {node, isArrayLiteral: true};
191+
}
192+
193+
const result = getStaticValue(node, scope);
194+
195+
if (!result) {
196+
return;
197+
}
198+
199+
const isSpreadable = Array.isArray(result.value);
200+
201+
return {node, isSpreadable};
202+
};
203+
204+
function getConcatFixableArguments(argumentsList, scope) {
205+
const fixableArguments = [];
206+
207+
for (const node of argumentsList) {
208+
const result = getConcatArgumentSpreadable(node, scope);
209+
210+
if (result) {
211+
fixableArguments.push(result);
212+
} else {
213+
break;
214+
}
215+
}
216+
217+
return fixableArguments;
218+
}
219+
104220
const create = context => {
105221
const sourceCode = context.getSourceCode();
106222
const getSource = node => sourceCode.getText(node);
@@ -138,39 +254,45 @@ const create = context => {
138254
messageId: ERROR_ARRAY_CONCAT
139255
};
140256

141-
const [item] = node.arguments;
142-
if (node.arguments.length !== 1 || item.type === 'SpreadElement') {
257+
const fixableArguments = getConcatFixableArguments(node.arguments, scope);
258+
259+
if (fixableArguments.length > 0 || node.arguments.length === 0) {
260+
problem.fix = fixConcat(node, sourceCode, fixableArguments);
143261
context.report(problem);
144262
return;
145263
}
146264

147-
let isItemArray;
148-
if (isArrayLiteral(item)) {
149-
isItemArray = true;
150-
} else {
151-
const result = getStaticValue(item, scope);
152-
153-
if (result) {
154-
isItemArray = Array.isArray(result.value);
155-
}
265+
const [firstArgument, ...restArguments] = node.arguments;
266+
if (firstArgument.type === 'SpreadElement') {
267+
context.report(problem);
268+
return;
156269
}
157270

158-
if (isItemArray === true) {
159-
problem.fix = fixConcat(node, sourceCode, /* isSpreadable */ true);
160-
} else if (isItemArray === false) {
161-
problem.fix = fixConcat(node, sourceCode, /* isSpreadable */ false);
162-
} else {
163-
problem.suggest = [
164-
{
165-
messageId: SUGGESTION_CONCAT_ARGUMENT_IS_SPREADABLE,
166-
fix: fixConcat(node, sourceCode, /* isSpreadable */ true)
167-
},
168-
{
169-
messageId: SUGGESTION_CONCAT_ARGUMENT_IS_NOT_SPREADABLE,
170-
fix: fixConcat(node, sourceCode, /* isSpreadable */ false)
171-
}
172-
];
173-
}
271+
const fixableArgumentsAfterFirstArgument = getConcatFixableArguments(restArguments, scope);
272+
problem.suggest = [
273+
{
274+
messageId: SUGGESTION_CONCAT_ARGUMENT_IS_SPREADABLE,
275+
isSpreadable: true
276+
},
277+
{
278+
messageId: SUGGESTION_CONCAT_ARGUMENT_IS_NOT_SPREADABLE,
279+
isSpreadable: false
280+
}
281+
].map(({messageId, isSpreadable}) => ({
282+
messageId,
283+
fix: fixConcat(
284+
node,
285+
sourceCode,
286+
// When apply suggestion, we also merge fixable arguments after the first one
287+
[
288+
{
289+
node: firstArgument,
290+
isSpreadable
291+
},
292+
...fixableArgumentsAfterFirstArgument
293+
]
294+
)
295+
}));
174296

175297
context.report(problem);
176298
}

‎rules/utils/should-add-parentheses-to-spread-element-argument.js‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
'use strict';
2+
3+
const nodeTypesDoNotNeedParentheses = new Set([
4+
'CallExpression',
5+
'Identifier',
6+
'Literal',
7+
'MemberExpression',
8+
'NewExpression',
9+
'TemplateLiteral',
10+
'ThisExpression'
11+
]);
12+
13+
/**
14+
Check if parentheses should be added to a `node` when it's used as `argument` of `SpreadElement`.
15+
16+
@param {Node} node - The AST node to check.
17+
@returns {boolean}
18+
*/
19+
const shouldAddParenthesesToSpreadElementArgument = node =>
20+
!nodeTypesDoNotNeedParentheses.has(node.type);
21+
22+
module.exports = shouldAddParenthesesToSpreadElementArgument;

0 commit comments

Comments
 (0)