Skip to content

Commit 7ba16fe

Browse files
fiskerfisker
committed
prefer-spread: Add a useful suggestion for .concat fix
1 parent 3765807 commit 7ba16fe

File tree

4 files changed

+88
-15
lines changed

4 files changed

+88
-15
lines changed

‎rules/prefer-spread.js

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
'use strict';
2-
const {isParenthesized, getStaticValue, isCommaToken} = require('eslint-utils');
2+
const {isParenthesized, getStaticValue, isCommaToken, hasSideEffect} = 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');
@@ -10,11 +10,13 @@ const ERROR_ARRAY_FROM = 'array-from';
1010
const ERROR_ARRAY_CONCAT = 'array-concat';
1111
const SUGGESTION_CONCAT_ARGUMENT_IS_SPREADABLE = 'argument-is-spreadable';
1212
const SUGGESTION_CONCAT_ARGUMENT_IS_NOT_SPREADABLE = 'argument-is-not-spreadable';
13+
const SUGGESTION_CONCAT_TEST_ARGUMENT = 'test-argument';
1314
const messages = {
1415
[ERROR_ARRAY_FROM]: 'Prefer the spread operator over `Array.from(…)`.',
1516
[ERROR_ARRAY_CONCAT]: 'Prefer the spread operator over `Array#concat(…)`.',
1617
[SUGGESTION_CONCAT_ARGUMENT_IS_SPREADABLE]: 'First argument is an `array`.',
17-
[SUGGESTION_CONCAT_ARGUMENT_IS_NOT_SPREADABLE]: 'First argument is not an `array`.'
18+
[SUGGESTION_CONCAT_ARGUMENT_IS_NOT_SPREADABLE]: 'First argument is not an `array`.',
19+
[SUGGESTION_CONCAT_TEST_ARGUMENT]: 'Use `Array.isArray(…)` test first argument.'
1820
};
1921

2022
const arrayFromCallSelector = [
@@ -90,13 +92,18 @@ function fixConcat(node, sourceCode, fixableArguments) {
9092
const lastArgument = nonEmptyArguments[nonEmptyArguments.length - 1];
9193

9294
let text = nonEmptyArguments
93-
.map(({node, isArrayLiteral, isSpreadable}) => {
95+
.map(({node, isArrayLiteral, isSpreadable, testArgument}) => {
9496
if (isArrayLiteral) {
9597
return getArrayLiteralElementsText(node, node === lastArgument.node);
9698
}
9799

98100
const [start, end] = getParenthesizedRange(node, sourceCode);
99101
let text = sourceCode.text.slice(start, end);
102+
103+
if (testArgument) {
104+
return `...(Array.isArray(${text}) ? ${text} : [text])`;
105+
}
106+
100107
if (isSpreadable) {
101108
if (
102109
!isParenthesized(node, sourceCode) &&
@@ -269,7 +276,7 @@ const create = context => {
269276
}
270277

271278
const fixableArgumentsAfterFirstArgument = getConcatFixableArguments(restArguments, scope);
272-
problem.suggest = [
279+
const suggestions = [
273280
{
274281
messageId: SUGGESTION_CONCAT_ARGUMENT_IS_SPREADABLE,
275282
isSpreadable: true
@@ -278,7 +285,14 @@ const create = context => {
278285
messageId: SUGGESTION_CONCAT_ARGUMENT_IS_NOT_SPREADABLE,
279286
isSpreadable: false
280287
}
281-
].map(({messageId, isSpreadable}) => ({
288+
];
289+
if (!hasSideEffect(firstArgument, sourceCode)) {
290+
suggestions.push({
291+
messageId: SUGGESTION_CONCAT_TEST_ARGUMENT,
292+
testArgument: true
293+
})
294+
}
295+
problem.suggest = suggestions.map(({messageId, isSpreadable, testArgument}) => ({
282296
messageId,
283297
fix: fixConcat(
284298
node,
@@ -287,7 +301,8 @@ const create = context => {
287301
[
288302
{
289303
node: firstArgument,
290-
isSpreadable
304+
isSpreadable,
305+
testArgument
291306
},
292307
...fixableArgumentsAfterFirstArgument
293308
]

‎test/prefer-spread.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,8 @@ test.snapshot({
225225
[EMPTY_STRING_IN_ARRAY],
226226
[[EMPTY_STRING_IN_ARRAY_OF_ARRAY]]
227227
)
228-
`
228+
`,
229+
'[].concat((a.b.c), 2)',
230+
'[].concat(a.b(), 2)'
229231
]
230232
});

‎test/snapshots/prefer-spread.js.md

Lines changed: 64 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -832,12 +832,16 @@ Generated by [AVA](https://avajs.dev).
832832
| ^^^^^^ Prefer the spread operator over `Array#concat(…)`.␊
833833
␊
834834
--------------------------------------------------------------------------------␊
835-
Suggestion 1/2: First argument is an `array`.␊
835+
Suggestion 1/3: First argument is an `array`.␊
836836
1 | [...foo, ...bar]␊
837837
␊
838838
--------------------------------------------------------------------------------␊
839-
Suggestion 2/2: First argument is not an `array`.␊
839+
Suggestion 2/3: First argument is not an `array`.␊
840840
1 | [...foo, bar]␊
841+
␊
842+
--------------------------------------------------------------------------------␊
843+
Suggestion 3/3: Use `Array.isArray(…)` test first argument.␊
844+
1 | [...foo, ...(Array.isArray(bar) ? bar : [text])]␊
841845
`
842846

843847
## Invalid #24
@@ -944,12 +948,16 @@ Generated by [AVA](https://avajs.dev).
944948
| ^^^^^^ Prefer the spread operator over `Array#concat(…)`.␊
945949
␊
946950
--------------------------------------------------------------------------------␊
947-
Suggestion 1/2: First argument is an `array`.␊
951+
Suggestion 1/3: First argument is an `array`.␊
948952
1 | [...foo, 2, ...bar]␊
949953
␊
950954
--------------------------------------------------------------------------------␊
951-
Suggestion 2/2: First argument is not an `array`.␊
955+
Suggestion 2/3: First argument is not an `array`.␊
952956
1 | [...foo, 2, bar]␊
957+
␊
958+
--------------------------------------------------------------------------------␊
959+
Suggestion 3/3: Use `Array.isArray(…)` test first argument.␊
960+
1 | [...foo, 2, ...(Array.isArray(bar) ? bar : [text])]␊
953961
`
954962

955963
## Invalid #30
@@ -978,12 +986,16 @@ Generated by [AVA](https://avajs.dev).
978986
| ^^^^^^ Prefer the spread operator over `Array#concat(…)`.␊
979987
␊
980988
--------------------------------------------------------------------------------␊
981-
Suggestion 1/2: First argument is an `array`.␊
989+
Suggestion 1/3: First argument is an `array`.␊
982990
1 | [...foo, ...bar, 2, 3]␊
983991
␊
984992
--------------------------------------------------------------------------------␊
985-
Suggestion 2/2: First argument is not an `array`.␊
993+
Suggestion 2/3: First argument is not an `array`.␊
986994
1 | [...foo, bar, 2, 3]␊
995+
␊
996+
--------------------------------------------------------------------------------␊
997+
Suggestion 3/3: Use `Array.isArray(…)` test first argument.␊
998+
1 | [...foo, ...(Array.isArray(bar) ? bar : [text]), 2, 3]␊
987999
`
9881000

9891001
## Invalid #32
@@ -996,12 +1008,16 @@ Generated by [AVA](https://avajs.dev).
9961008
| ^^^^^^ Prefer the spread operator over `Array#concat(…)`.␊
9971009
␊
9981010
--------------------------------------------------------------------------------␊
999-
Suggestion 1/2: First argument is an `array`.␊
1011+
Suggestion 1/3: First argument is an `array`.␊
10001012
1 | [...foo, ...bar, 2, 3].concat(baz)␊
10011013
␊
10021014
--------------------------------------------------------------------------------␊
1003-
Suggestion 2/2: First argument is not an `array`.␊
1015+
Suggestion 2/3: First argument is not an `array`.␊
10041016
1 | [...foo, bar, 2, 3].concat(baz)␊
1017+
␊
1018+
--------------------------------------------------------------------------------␊
1019+
Suggestion 3/3: Use `Array.isArray(…)` test first argument.␊
1020+
1 | [...foo, ...(Array.isArray(bar) ? bar : [text]), 2, 3].concat(baz)␊
10051021
`
10061022

10071023
## Invalid #33
@@ -1266,3 +1282,43 @@ Generated by [AVA](https://avajs.dev).
12661282
11 | [[EMPTY_STRING_IN_ARRAY_OF_ARRAY]]␊
12671283
12 | )␊
12681284
`
1285+
1286+
## Invalid #48
1287+
1 | [].concat((a.b.c), 2)
1288+
1289+
> Error 1/1
1290+
1291+
`␊
1292+
> 1 | [].concat((a.b.c), 2)␊
1293+
| ^^^^^^ Prefer the spread operator over `Array#concat(…)`.␊
1294+
␊
1295+
--------------------------------------------------------------------------------␊
1296+
Suggestion 1/3: First argument is an `array`.␊
1297+
1 | [...(a.b.c), 2]␊
1298+
␊
1299+
--------------------------------------------------------------------------------␊
1300+
Suggestion 2/3: First argument is not an `array`.␊
1301+
1 | [(a.b.c), 2]␊
1302+
␊
1303+
--------------------------------------------------------------------------------␊
1304+
Suggestion 3/3: Use `Array.isArray(…)` test first argument.␊
1305+
1 | [...(Array.isArray((a.b.c)) ? (a.b.c) : [text]), 2]␊
1306+
`
1307+
1308+
## Invalid #49
1309+
1 | [].concat(a.b(), 2)
1310+
1311+
> Error 1/1
1312+
1313+
`␊
1314+
> 1 | [].concat(a.b(), 2)␊
1315+
| ^^^^^^ Prefer the spread operator over `Array#concat(…)`.␊
1316+
␊
1317+
--------------------------------------------------------------------------------␊
1318+
Suggestion 1/2: First argument is an `array`.␊
1319+
1 | [...a.b(), 2]␊
1320+
␊
1321+
--------------------------------------------------------------------------------␊
1322+
Suggestion 2/2: First argument is not an `array`.␊
1323+
1 | [a.b(), 2]␊
1324+
`

‎test/snapshots/prefer-spread.js.snap

200 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)