diff --git a/rules/prefer-spread.js b/rules/prefer-spread.js index bca7942c49..554a1bc915 100644 --- a/rules/prefer-spread.js +++ b/rules/prefer-spread.js @@ -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'); @@ -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 = [ @@ -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) && @@ -269,7 +278,7 @@ const create = context => { } const fixableArgumentsAfterFirstArgument = getConcatFixableArguments(restArguments, scope); - problem.suggest = [ + const suggestions = [ { messageId: SUGGESTION_CONCAT_ARGUMENT_IS_SPREADABLE, isSpreadable: true @@ -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, @@ -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); } }; diff --git a/test/prefer-spread.js b/test/prefer-spread.js index 7e7b662a40..cffbd95b06 100644 --- a/test/prefer-spread.js +++ b/test/prefer-spread.js @@ -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)' ] }); diff --git a/test/snapshots/prefer-spread.js.md b/test/snapshots/prefer-spread.js.md index 8101a29648..eec2c36fb9 100644 --- a/test/snapshots/prefer-spread.js.md +++ b/test/snapshots/prefer-spread.js.md @@ -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 @@ -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 @@ -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 @@ -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 @@ -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)␊ + ` diff --git a/test/snapshots/prefer-spread.js.snap b/test/snapshots/prefer-spread.js.snap index 0eebd4b5a4..42167ccdb7 100644 Binary files a/test/snapshots/prefer-spread.js.snap and b/test/snapshots/prefer-spread.js.snap differ