Skip to content

Commit

Permalink
prefer-spread: Add more suggestions for .concat fix (#1054)
Browse files Browse the repository at this point in the history
  • Loading branch information
@fisker
fisker committed Jan 25, 2021
1 parent d707e83 commit 673c440
Show file tree
Hide file tree
Showing 4 changed files with 160 additions and 15 deletions.
45 changes: 39 additions & 6 deletions rules/prefer-spread.js
View file
@@ -1,5 +1,5 @@
'use strict';
const {isParenthesized, getStaticValue, isCommaToken} = require('eslint-utils');
const {isParenthesized, getStaticValue, isCommaToken, hasSideEffect} = require('eslint-utils');
const getDocumentationUrl = require('./utils/get-documentation-url');
const methodSelector = require('./utils/method-selector');
const needsSemicolon = require('./utils/needs-semicolon');
Expand All @@ -10,11 +10,15 @@ 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 SUGGESTION_CONCAT_TEST_ARGUMENT = 'test-argument';
const SUGGESTION_CONCAT_SPREAD_ALL_ARGUMENTS = 'spread-all-arguments';
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]: 'First argument is an `array`.',
[SUGGESTION_CONCAT_ARGUMENT_IS_NOT_SPREADABLE]: 'First argument is not an `array`.'
[SUGGESTION_CONCAT_ARGUMENT_IS_NOT_SPREADABLE]: 'First argument is not an `array`.',
[SUGGESTION_CONCAT_TEST_ARGUMENT]: 'Test first argument with `Array.isArray(…)`.',
[SUGGESTION_CONCAT_SPREAD_ALL_ARGUMENTS]: 'Spread all unknown arguments`.'
};

const arrayFromCallSelector = [
Expand Down Expand Up @@ -90,13 +94,18 @@ function fixConcat(node, sourceCode, fixableArguments) {
const lastArgument = nonEmptyArguments[nonEmptyArguments.length - 1];

let text = nonEmptyArguments
.map(({node, isArrayLiteral, isSpreadable}) => {
.map(({node, isArrayLiteral, isSpreadable, testArgument}) => {
if (isArrayLiteral) {
return getArrayLiteralElementsText(node, node === lastArgument.node);
}

const [start, end] = getParenthesizedRange(node, sourceCode);
let text = sourceCode.text.slice(start, end);

if (testArgument) {
return `...(Array.isArray(${text}) ? ${text} : [${text}])`;
}

if (isSpreadable) {
if (
!isParenthesized(node, sourceCode) &&
Expand Down Expand Up @@ -269,7 +278,7 @@ const create = context => {
}

const fixableArgumentsAfterFirstArgument = getConcatFixableArguments(restArguments, scope);
problem.suggest = [
const suggestions = [
{
messageId: SUGGESTION_CONCAT_ARGUMENT_IS_SPREADABLE,
isSpreadable: true
Expand All @@ -278,7 +287,16 @@ const create = context => {
messageId: SUGGESTION_CONCAT_ARGUMENT_IS_NOT_SPREADABLE,
isSpreadable: false
}
].map(({messageId, isSpreadable}) => ({
];

if (!hasSideEffect(firstArgument, sourceCode)) {
suggestions.push({
messageId: SUGGESTION_CONCAT_TEST_ARGUMENT,
testArgument: true
});
}

problem.suggest = suggestions.map(({messageId, isSpreadable, testArgument}) => ({
messageId,
fix: fixConcat(
node,
Expand All @@ -287,13 +305,28 @@ const create = context => {
[
{
node: firstArgument,
isSpreadable
isSpreadable,
testArgument
},
...fixableArgumentsAfterFirstArgument
]
)
}));

if (
fixableArgumentsAfterFirstArgument.length < restArguments.length &&
restArguments.every(({type}) => type !== 'SpreadElement')
) {
problem.suggest.push({
messageId: SUGGESTION_CONCAT_SPREAD_ALL_ARGUMENTS,
fix: fixConcat(
node,
sourceCode,
node.arguments.map(node => getConcatArgumentSpreadable(node, scope) || {node, isSpreadable: true})
)
});
}

context.report(problem);
}
};
Expand Down
6 changes: 5 additions & 1 deletion test/prefer-spread.js
View file
Expand Up @@ -225,6 +225,10 @@ test.snapshot({
[EMPTY_STRING_IN_ARRAY],
[[EMPTY_STRING_IN_ARRAY_OF_ARRAY]]
)
`
`,
'[].concat((a.b.c), 2)',
'[].concat(a.b(), 2)',
'foo.concat(bar, 2, [3, 4], baz, 5, [6, 7])',
'foo.concat(bar, 2, 3, ...baz)'
]
});
124 changes: 116 additions & 8 deletions test/snapshots/prefer-spread.js.md
View file
Expand Up @@ -832,12 +832,16 @@ Generated by [AVA](https://avajs.dev).
| ^^^^^^ Prefer the spread operator over `Array#concat(…)`.␊
␊
--------------------------------------------------------------------------------␊
Suggestion 1/2: First argument is an `array`.␊
Suggestion 1/3: First argument is an `array`.␊
1 | [...foo, ...bar]␊
␊
--------------------------------------------------------------------------------␊
Suggestion 2/2: First argument is not an `array`.␊
Suggestion 2/3: First argument is not an `array`.␊
1 | [...foo, bar]␊
␊
--------------------------------------------------------------------------------␊
Suggestion 3/3: Test first argument with `Array.isArray(…)`.␊
1 | [...foo, ...(Array.isArray(bar) ? bar : [bar])]␊
`

## Invalid #24
Expand Down Expand Up @@ -944,12 +948,16 @@ Generated by [AVA](https://avajs.dev).
| ^^^^^^ Prefer the spread operator over `Array#concat(…)`.␊
␊
--------------------------------------------------------------------------------␊
Suggestion 1/2: First argument is an `array`.␊
Suggestion 1/3: First argument is an `array`.␊
1 | [...foo, 2, ...bar]␊
␊
--------------------------------------------------------------------------------␊
Suggestion 2/2: First argument is not an `array`.␊
Suggestion 2/3: First argument is not an `array`.␊
1 | [...foo, 2, bar]␊
␊
--------------------------------------------------------------------------------␊
Suggestion 3/3: Test first argument with `Array.isArray(…)`.␊
1 | [...foo, 2, ...(Array.isArray(bar) ? bar : [bar])]␊
`

## Invalid #30
Expand Down Expand Up @@ -978,12 +986,16 @@ Generated by [AVA](https://avajs.dev).
| ^^^^^^ Prefer the spread operator over `Array#concat(…)`.␊
␊
--------------------------------------------------------------------------------␊
Suggestion 1/2: First argument is an `array`.␊
Suggestion 1/3: First argument is an `array`.␊
1 | [...foo, ...bar, 2, 3]␊
␊
--------------------------------------------------------------------------------␊
Suggestion 2/2: First argument is not an `array`.␊
Suggestion 2/3: First argument is not an `array`.␊
1 | [...foo, bar, 2, 3]␊
␊
--------------------------------------------------------------------------------␊
Suggestion 3/3: Test first argument with `Array.isArray(…)`.␊
1 | [...foo, ...(Array.isArray(bar) ? bar : [bar]), 2, 3]␊
`

## Invalid #32
Expand All @@ -996,12 +1008,20 @@ Generated by [AVA](https://avajs.dev).
| ^^^^^^ Prefer the spread operator over `Array#concat(…)`.␊
␊
--------------------------------------------------------------------------------␊
Suggestion 1/2: First argument is an `array`.␊
Suggestion 1/4: First argument is an `array`.␊
1 | [...foo, ...bar, 2, 3].concat(baz)␊
␊
--------------------------------------------------------------------------------␊
Suggestion 2/2: First argument is not an `array`.␊
Suggestion 2/4: First argument is not an `array`.␊
1 | [...foo, bar, 2, 3].concat(baz)␊
␊
--------------------------------------------------------------------------------␊
Suggestion 3/4: Test first argument with `Array.isArray(…)`.␊
1 | [...foo, ...(Array.isArray(bar) ? bar : [bar]), 2, 3].concat(baz)␊
␊
--------------------------------------------------------------------------------␊
Suggestion 4/4: Spread all unknown arguments`.␊
1 | [...foo, ...bar, 2, 3, ...baz]␊
`

## Invalid #33
Expand Down Expand Up @@ -1266,3 +1286,91 @@ Generated by [AVA](https://avajs.dev).
11 | [[EMPTY_STRING_IN_ARRAY_OF_ARRAY]]␊
12 | )␊
`

## Invalid #48
1 | [].concat((a.b.c), 2)

> Error 1/1
`␊
> 1 | [].concat((a.b.c), 2)␊
| ^^^^^^ Prefer the spread operator over `Array#concat(…)`.␊
␊
--------------------------------------------------------------------------------␊
Suggestion 1/3: First argument is an `array`.␊
1 | [...(a.b.c), 2]␊
␊
--------------------------------------------------------------------------------␊
Suggestion 2/3: First argument is not an `array`.␊
1 | [(a.b.c), 2]␊
␊
--------------------------------------------------------------------------------␊
Suggestion 3/3: Test first argument with `Array.isArray(…)`.␊
1 | [...(Array.isArray((a.b.c)) ? (a.b.c) : [(a.b.c)]), 2]␊
`

## Invalid #49
1 | [].concat(a.b(), 2)

> Error 1/1
`␊
> 1 | [].concat(a.b(), 2)␊
| ^^^^^^ Prefer the spread operator over `Array#concat(…)`.␊
␊
--------------------------------------------------------------------------------␊
Suggestion 1/2: First argument is an `array`.␊
1 | [...a.b(), 2]␊
␊
--------------------------------------------------------------------------------␊
Suggestion 2/2: First argument is not an `array`.␊
1 | [a.b(), 2]␊
`

## Invalid #50
1 | foo.concat(bar, 2, [3, 4], baz, 5, [6, 7])

> Error 1/1
`␊
> 1 | foo.concat(bar, 2, [3, 4], baz, 5, [6, 7])␊
| ^^^^^^ Prefer the spread operator over `Array#concat(…)`.␊
␊
--------------------------------------------------------------------------------␊
Suggestion 1/4: First argument is an `array`.␊
1 | [...foo, ...bar, 2, 3, 4].concat(baz, 5, [6, 7])␊
␊
--------------------------------------------------------------------------------␊
Suggestion 2/4: First argument is not an `array`.␊
1 | [...foo, bar, 2, 3, 4].concat(baz, 5, [6, 7])␊
␊
--------------------------------------------------------------------------------␊
Suggestion 3/4: Test first argument with `Array.isArray(…)`.␊
1 | [...foo, ...(Array.isArray(bar) ? bar : [bar]), 2, 3, 4].concat(baz, 5, [6, 7])␊
␊
--------------------------------------------------------------------------------␊
Suggestion 4/4: Spread all unknown arguments`.␊
1 | [...foo, ...bar, 2, 3, 4, ...baz, 5, 6, 7]␊
`

## Invalid #51
1 | foo.concat(bar, 2, 3, ...baz)

> Error 1/1
`␊
> 1 | foo.concat(bar, 2, 3, ...baz)␊
| ^^^^^^ Prefer the spread operator over `Array#concat(…)`.␊
␊
--------------------------------------------------------------------------------␊
Suggestion 1/3: First argument is an `array`.␊
1 | [...foo, ...bar, 2, 3].concat(...baz)␊
␊
--------------------------------------------------------------------------------␊
Suggestion 2/3: First argument is not an `array`.␊
1 | [...foo, bar, 2, 3].concat(...baz)␊
␊
--------------------------------------------------------------------------------␊
Suggestion 3/3: Test first argument with `Array.isArray(…)`.␊
1 | [...foo, ...(Array.isArray(bar) ? bar : [bar]), 2, 3].concat(...baz)␊
`
Binary file modified test/snapshots/prefer-spread.js.snap
View file
Binary file not shown.

0 comments on commit 673c440

Please sign in to comment.