diff --git a/docs/rules/no-useless-spread.md b/docs/rules/no-useless-spread.md index d314de2d88..73b1099ad7 100644 --- a/docs/rules/no-useless-spread.md +++ b/docs/rules/no-useless-spread.md @@ -12,6 +12,7 @@ - Spread an array literal as elements of an array literal - Spread an array literal as arguments of a call or a `new` call - Spread an object literal as properties of an object literal + - Use spread syntax to clone an array created inline - The following builtins accept an iterable, so it's unnecessary to convert the iterable to an array: @@ -65,6 +66,14 @@ function * foo() { } ``` +```js +function foo(bar) { + return [ + ...bar.map(x => x * 2), + ]; +} +``` + ## Pass ```js @@ -116,3 +125,9 @@ function * foo() { yield * anotherGenerator(); } ``` + +```js +function foo(bar) { + return bar.map(x => x * 2); +} +``` diff --git a/rules/no-useless-spread.js b/rules/no-useless-spread.js index 570b1dfc3c..ad363b3e96 100644 --- a/rules/no-useless-spread.js +++ b/rules/no-useless-spread.js @@ -6,17 +6,27 @@ const { methodCallSelector, } = require('./selectors/index.js'); const typedArray = require('./shared/typed-array.js'); -const {removeParentheses, fixSpaceAroundKeyword} = require('./fix/index.js'); +const { + removeParentheses, + fixSpaceAroundKeyword, + addParenthesizesToReturnOrThrowExpression, +} = require('./fix/index.js'); +const isOnSameLine = require('./utils/is-on-same-line.js'); +const { + isParenthesized, +} = require('./utils/parentheses.js'); const SPREAD_IN_LIST = 'spread-in-list'; const ITERABLE_TO_ARRAY = 'iterable-to-array'; const ITERABLE_TO_ARRAY_IN_FOR_OF = 'iterable-to-array-in-for-of'; const ITERABLE_TO_ARRAY_IN_YIELD_STAR = 'iterable-to-array-in-yield-star'; +const CLONE_ARRAY = 'clone-array'; const messages = { [SPREAD_IN_LIST]: 'Spread an {{argumentType}} literal in {{parentDescription}} is unnecessary.', [ITERABLE_TO_ARRAY]: '`{{parentDescription}}` accepts iterable as argument, it\'s unnecessary to convert to an array.', [ITERABLE_TO_ARRAY_IN_FOR_OF]: '`for…of` can iterate over iterable, it\'s unnecessary to convert to an array.', [ITERABLE_TO_ARRAY_IN_YIELD_STAR]: '`yield*` can delegate iterable, it\'s unnecessary to convert to an array.', + [CLONE_ARRAY]: 'Unnecessarily cloning an array.', }; const uselessSpreadInListSelector = matches([ @@ -26,7 +36,7 @@ const uselessSpreadInListSelector = matches([ 'NewExpression > SpreadElement.arguments > ArrayExpression.argument', ]); -const iterableToArraySelector = [ +const singleArraySpreadSelector = [ 'ArrayExpression', '[elements.length=1]', '[elements.0.type="SpreadElement"]', @@ -49,12 +59,47 @@ const uselessIterableToArraySelector = matches([ methodCallSelector({object: 'Object', method: 'fromEntries', argumentsLength: 1}), ]), ' > ', - `${iterableToArraySelector}.arguments:first-child`, + `${singleArraySpreadSelector}.arguments:first-child`, ].join(''), - `ForOfStatement > ${iterableToArraySelector}.right`, - `YieldExpression[delegate=true] > ${iterableToArraySelector}.argument`, + `ForOfStatement > ${singleArraySpreadSelector}.right`, + `YieldExpression[delegate=true] > ${singleArraySpreadSelector}.argument`, ]); +const uselessArrayCloneSelector = [ + `${singleArraySpreadSelector} > .elements:first-child > .argument`, + matches([ + // Array methods returns a new array + methodCallSelector([ + 'concat', + 'copyWithin', + 'filter', + 'flat', + 'flatMap', + 'map', + 'slice', + 'splice', + ]), + // `String#split()` + methodCallSelector('split'), + // `Object.keys()` and `Object.values()` + methodCallSelector({object: 'Object', methods: ['keys', 'values'], argumentsLength: 1}), + // `await Promise.all()` and `await Promise.allSettled` + [ + 'AwaitExpression', + methodCallSelector({ + object: 'Promise', + methods: ['all', 'allSettled'], + argumentsLength: 1, + path: 'argument', + }), + ].join(''), + // `Array.from()`, `Array.of()` + methodCallSelector({object: 'Array', methods: ['from', 'of']}), + // `new Array()` + newExpressionSelector('Array'), + ]), +].join(''); + const parentDescriptions = { ArrayExpression: 'array literal', ObjectExpression: 'object literal', @@ -81,6 +126,59 @@ function getCommaTokens(arrayExpression, sourceCode) { }); } +function * unwrapSingleArraySpread(fixer, arrayExpression, sourceCode) { + const [ + openingBracketToken, + spreadToken, + thirdToken, + ] = sourceCode.getFirstTokens(arrayExpression, 3); + + // `[...value]` + // ^ + yield fixer.remove(openingBracketToken); + + // `[...value]` + // ^^^ + yield fixer.remove(spreadToken); + + const [ + commaToken, + closingBracketToken, + ] = sourceCode.getLastTokens(arrayExpression, 2); + + // `[...value]` + // ^ + yield fixer.remove(closingBracketToken); + + // `[...value,]` + // ^ + if (isCommaToken(commaToken)) { + yield fixer.remove(commaToken); + } + + /* + ```js + function foo() { + return [ + ...value, + ]; + } + ``` + */ + const {parent} = arrayExpression; + if ( + (parent.type === 'ReturnStatement' || parent.type === 'ThrowStatement') + && parent.argument === arrayExpression + && !isOnSameLine(openingBracketToken, thirdToken) + && !isParenthesized(arrayExpression, sourceCode) + ) { + yield * addParenthesizesToReturnOrThrowExpression(fixer, parent, sourceCode); + return; + } + + yield * fixSpaceAroundKeyword(fixer, arrayExpression, sourceCode); +} + /** @param {import('eslint').Rule.RuleContext} context */ const create = context => { const sourceCode = context.getSourceCode(); @@ -138,15 +236,15 @@ const create = context => { continue; } - // `call([foo, , bar])` - // ^ Replace holes with `undefined` + // `call(...[foo, , bar])` + // ^ Replace holes with `undefined` yield fixer.insertTextBefore(commaToken, 'undefined'); } }, }; }, - [uselessIterableToArraySelector](array) { - const {parent} = array; + [uselessIterableToArraySelector](arrayExpression) { + const {parent} = arrayExpression; let parentDescription = ''; let messageId = ITERABLE_TO_ARRAY; switch (parent.type) { @@ -173,42 +271,18 @@ const create = context => { } return { - node: array, + node: arrayExpression, messageId, data: {parentDescription}, - * fix(fixer) { - if (parent.type === 'ForOfStatement') { - yield * fixSpaceAroundKeyword(fixer, array, sourceCode); - } - - const [ - openingBracketToken, - spreadToken, - ] = sourceCode.getFirstTokens(array, 2); - - // `[...iterable]` - // ^ - yield fixer.remove(openingBracketToken); - - // `[...iterable]` - // ^^^ - yield fixer.remove(spreadToken); - - const [ - commaToken, - closingBracketToken, - ] = sourceCode.getLastTokens(array, 2); - - // `[...iterable]` - // ^ - yield fixer.remove(closingBracketToken); - - // `[...iterable,]` - // ^ - if (isCommaToken(commaToken)) { - yield fixer.remove(commaToken); - } - }, + fix: fixer => unwrapSingleArraySpread(fixer, arrayExpression, sourceCode), + }; + }, + [uselessArrayCloneSelector](node) { + const arrayExpression = node.parent.parent; + return { + node: arrayExpression, + messageId: CLONE_ARRAY, + fix: fixer => unwrapSingleArraySpread(fixer, arrayExpression, sourceCode), }; }, }; diff --git a/test/no-useless-spread.mjs b/test/no-useless-spread.mjs index adb975c1be..981ea530a1 100644 --- a/test/no-useless-spread.mjs +++ b/test/no-useless-spread.mjs @@ -9,7 +9,6 @@ test.snapshot({ 'const array = [[]]', 'const array = [{}]', 'const object = ({...[]})', - 'const array = [...[].map(x => x)]', 'foo([])', 'foo({})', 'new Foo([])', @@ -241,6 +240,69 @@ test.snapshot({ ], }); +// Cloning an immediate array +test.snapshot({ + valid: [ + '[...not.array]', + '[...not.array()]', + '[...array.unknown()]', + '[...Object.notReturningArray(foo)]', + '[...NotObject.keys(foo)]', + '[...Int8Array.from(foo)]', + '[...Int8Array.of()]', + '[...new Int8Array(3)]', + '[...Promise.all(foo)]', + '[...Promise.allSettled(foo)]', + '[...await Promise.all(foo, extraArgument)]', + ], + invalid: [ + '[...foo.concat(bar)]', + '[...foo.copyWithin(-2)]', + '[...foo.filter(bar)]', + '[...foo.flat()]', + '[...foo.flatMap(bar)]', + '[...foo.map(bar)]', + '[...foo.slice(1)]', + '[...foo.splice(1)]', + '[...foo.split("|")]', + '[...Object.keys(foo)]', + '[...Object.values(foo)]', + '[...Array.from(foo)]', + '[...Array.of()]', + '[...new Array(3)]', + '[...await Promise.all(foo)]', + '[...await Promise.allSettled(foo)]', + outdent` + function foo(bar) { + return[...Object.keys(bar)]; + } + `, + outdent` + function foo(bar) { + return[ + ...Object.keys(bar) + ]; + } + `, + outdent` + function foo(bar) { + return[ + ...( + Object.keys(bar) + ) + ]; + } + `, + outdent` + function foo(bar) { + return([ + ...Object.keys(bar) + ]); + } + `, + ], +}); + test.babel({ valid: [], invalid: [ diff --git a/test/snapshots/no-useless-spread.mjs.md b/test/snapshots/no-useless-spread.mjs.md index 56475ab77f..672be64118 100644 --- a/test/snapshots/no-useless-spread.mjs.md +++ b/test/snapshots/no-useless-spread.mjs.md @@ -1553,3 +1553,379 @@ Generated by [AVA](https://avajs.dev). | ^^^^^^^^^^^^^^^^^^^ \`yield*\` can delegate iterable, it's unnecessary to convert to an array.␊ 3 | }␊ ` + +## Invalid #1 + 1 | [...foo.concat(bar)] + +> Output + + `␊ + 1 | foo.concat(bar)␊ + ` + +> Error 1/1 + + `␊ + > 1 | [...foo.concat(bar)]␊ + | ^^^^^^^^^^^^^^^^^^^^ Unnecessarily cloning an array.␊ + ` + +## Invalid #2 + 1 | [...foo.copyWithin(-2)] + +> Output + + `␊ + 1 | foo.copyWithin(-2)␊ + ` + +> Error 1/1 + + `␊ + > 1 | [...foo.copyWithin(-2)]␊ + | ^^^^^^^^^^^^^^^^^^^^^^^ Unnecessarily cloning an array.␊ + ` + +## Invalid #3 + 1 | [...foo.filter(bar)] + +> Output + + `␊ + 1 | foo.filter(bar)␊ + ` + +> Error 1/1 + + `␊ + > 1 | [...foo.filter(bar)]␊ + | ^^^^^^^^^^^^^^^^^^^^ Unnecessarily cloning an array.␊ + ` + +## Invalid #4 + 1 | [...foo.flat()] + +> Output + + `␊ + 1 | foo.flat()␊ + ` + +> Error 1/1 + + `␊ + > 1 | [...foo.flat()]␊ + | ^^^^^^^^^^^^^^^ Unnecessarily cloning an array.␊ + ` + +## Invalid #5 + 1 | [...foo.flatMap(bar)] + +> Output + + `␊ + 1 | foo.flatMap(bar)␊ + ` + +> Error 1/1 + + `␊ + > 1 | [...foo.flatMap(bar)]␊ + | ^^^^^^^^^^^^^^^^^^^^^ Unnecessarily cloning an array.␊ + ` + +## Invalid #6 + 1 | [...foo.map(bar)] + +> Output + + `␊ + 1 | foo.map(bar)␊ + ` + +> Error 1/1 + + `␊ + > 1 | [...foo.map(bar)]␊ + | ^^^^^^^^^^^^^^^^^ Unnecessarily cloning an array.␊ + ` + +## Invalid #7 + 1 | [...foo.slice(1)] + +> Output + + `␊ + 1 | foo.slice(1)␊ + ` + +> Error 1/1 + + `␊ + > 1 | [...foo.slice(1)]␊ + | ^^^^^^^^^^^^^^^^^ Unnecessarily cloning an array.␊ + ` + +## Invalid #8 + 1 | [...foo.splice(1)] + +> Output + + `␊ + 1 | foo.splice(1)␊ + ` + +> Error 1/1 + + `␊ + > 1 | [...foo.splice(1)]␊ + | ^^^^^^^^^^^^^^^^^^ Unnecessarily cloning an array.␊ + ` + +## Invalid #9 + 1 | [...foo.split("|")] + +> Output + + `␊ + 1 | foo.split("|")␊ + ` + +> Error 1/1 + + `␊ + > 1 | [...foo.split("|")]␊ + | ^^^^^^^^^^^^^^^^^^^ Unnecessarily cloning an array.␊ + ` + +## Invalid #10 + 1 | [...Object.keys(foo)] + +> Output + + `␊ + 1 | Object.keys(foo)␊ + ` + +> Error 1/1 + + `␊ + > 1 | [...Object.keys(foo)]␊ + | ^^^^^^^^^^^^^^^^^^^^^ Unnecessarily cloning an array.␊ + ` + +## Invalid #11 + 1 | [...Object.values(foo)] + +> Output + + `␊ + 1 | Object.values(foo)␊ + ` + +> Error 1/1 + + `␊ + > 1 | [...Object.values(foo)]␊ + | ^^^^^^^^^^^^^^^^^^^^^^^ Unnecessarily cloning an array.␊ + ` + +## Invalid #12 + 1 | [...Array.from(foo)] + +> Output + + `␊ + 1 | Array.from(foo)␊ + ` + +> Error 1/1 + + `␊ + > 1 | [...Array.from(foo)]␊ + | ^^^^^^^^^^^^^^^^^^^^ Unnecessarily cloning an array.␊ + ` + +## Invalid #13 + 1 | [...Array.of()] + +> Output + + `␊ + 1 | Array.of()␊ + ` + +> Error 1/1 + + `␊ + > 1 | [...Array.of()]␊ + | ^^^^^^^^^^^^^^^ Unnecessarily cloning an array.␊ + ` + +## Invalid #14 + 1 | [...new Array(3)] + +> Output + + `␊ + 1 | new Array(3)␊ + ` + +> Error 1/1 + + `␊ + > 1 | [...new Array(3)]␊ + | ^^^^^^^^^^^^^^^^^ Unnecessarily cloning an array.␊ + ` + +## Invalid #15 + 1 | [...await Promise.all(foo)] + +> Output + + `␊ + 1 | await Promise.all(foo)␊ + ` + +> Error 1/1 + + `␊ + > 1 | [...await Promise.all(foo)]␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Unnecessarily cloning an array.␊ + ` + +## Invalid #16 + 1 | [...await Promise.allSettled(foo)] + +> Output + + `␊ + 1 | await Promise.allSettled(foo)␊ + ` + +> Error 1/1 + + `␊ + > 1 | [...await Promise.allSettled(foo)]␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Unnecessarily cloning an array.␊ + ` + +## Invalid #17 + 1 | function foo(bar) { + 2 | return[...Object.keys(bar)]; + 3 | } + +> Output + + `␊ + 1 | function foo(bar) {␊ + 2 | return Object.keys(bar);␊ + 3 | }␊ + ` + +> Error 1/1 + + `␊ + 1 | function foo(bar) {␊ + > 2 | return[...Object.keys(bar)];␊ + | ^^^^^^^^^^^^^^^^^^^^^ Unnecessarily cloning an array.␊ + 3 | }␊ + ` + +## Invalid #18 + 1 | function foo(bar) { + 2 | return[ + 3 | ...Object.keys(bar) + 4 | ]; + 5 | } + +> Output + + `␊ + 1 | function foo(bar) {␊ + 2 | return (␊ + 3 | Object.keys(bar)␊ + 4 | );␊ + 5 | }␊ + ` + +> Error 1/1 + + `␊ + 1 | function foo(bar) {␊ + > 2 | return[␊ + | ^␊ + > 3 | ...Object.keys(bar)␊ + | ^^^^^^^^^^^^^^^^^^^^^␊ + > 4 | ];␊ + | ^^^ Unnecessarily cloning an array.␊ + 5 | }␊ + ` + +## Invalid #19 + 1 | function foo(bar) { + 2 | return[ + 3 | ...( + 4 | Object.keys(bar) + 5 | ) + 6 | ]; + 7 | } + +> Output + + `␊ + 1 | function foo(bar) {␊ + 2 | return (␊ + 3 | (␊ + 4 | Object.keys(bar)␊ + 5 | )␊ + 6 | );␊ + 7 | }␊ + ` + +> Error 1/1 + + `␊ + 1 | function foo(bar) {␊ + > 2 | return[␊ + | ^␊ + > 3 | ...(␊ + | ^^^^^^␊ + > 4 | Object.keys(bar)␊ + | ^^^^^^␊ + > 5 | )␊ + | ^^^^^^␊ + > 6 | ];␊ + | ^^^ Unnecessarily cloning an array.␊ + 7 | }␊ + ` + +## Invalid #20 + 1 | function foo(bar) { + 2 | return([ + 3 | ...Object.keys(bar) + 4 | ]); + 5 | } + +> Output + + `␊ + 1 | function foo(bar) {␊ + 2 | return (␊ + 3 | Object.keys(bar)␊ + 4 | );␊ + 5 | }␊ + ` + +> Error 1/1 + + `␊ + 1 | function foo(bar) {␊ + > 2 | return([␊ + | ^␊ + > 3 | ...Object.keys(bar)␊ + | ^^^^^^^^^^^^^^^^^^^^^␊ + > 4 | ]);␊ + | ^^^ Unnecessarily cloning an array.␊ + 5 | }␊ + ` diff --git a/test/snapshots/no-useless-spread.mjs.snap b/test/snapshots/no-useless-spread.mjs.snap index 8d2ab2b2c8..76b405a9b8 100644 Binary files a/test/snapshots/no-useless-spread.mjs.snap and b/test/snapshots/no-useless-spread.mjs.snap differ