Add no-useless-spread rule (#1401)

docs/rules/no-useless-spread.md

@@ -0,0 +1,61 @@
+# Disallow useless spread
+
+Using spread syntax in the following cases is unnecessary:
+
+- 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
+
+This rule is fixable.
+
+## Fail
+
+```js
+const array = [firstElement, ...[secondElement], thirdElement];
+```
+
+```js
+const object = {firstProperty, ...{secondProperty}, thirdProperty};
+```
+
+```js
+foo(firstArgument, ...[secondArgument], thirdArgument);
+```
+
+```js
+const object = new Foo(firstArgument, ...[secondArgument], thirdArgument);
+```
+
+## Pass
+
+```js
+const array = [firstElement, secondElement, thirdElement];
+```
+
+```js
+const object = {firstProperty, secondProperty, thirdProperty};
+```
+
+```js
+foo(firstArgument, secondArgument, thirdArgument);
+```
+
+```js
+const object = new Foo(firstArgument, secondArgument, thirdArgument);
+```
+
+```js
+const array = [...foo, bar];
+```
+
+```js
+const object = {...foo, bar};
+```
+
+```js
+foo(foo, ...bar);
+```
+
+```js
+const object = new Foo(...foo, bar);
+```

docs/rules/prefer-spread.md

@@ -51,3 +51,28 @@ const tail = array.slice(1);
 ```js
 const copy = [...array];
 ```
+
+## With the `unicorn/no-useless-spread` rule
+
+Some cases are fixed using extra spread syntax. Therefore we recommend enabling the [`unicorn/no-useless-spread`](./no-useless-spread.md) rule to fix it.
+
+For example:
+
+```js
+const baz = [2];
+call(foo, ...[bar].concat(baz));
+```
+
+Will be fixed to:
+
+```js
+const baz = [2];
+call(foo, ...[bar, ...baz]);
+```
+
+`unicorn/no-useless-spread` will fix it to:
+
+```js
+const baz = [2];
+call(foo, bar, ...baz);
+```

index.js

@@ -78,6 +78,7 @@ module.exports = {
 				'unicorn/no-unsafe-regex': 'off',
 				'unicorn/no-unused-properties': 'off',
 				'unicorn/no-useless-length-check': 'error',
+				'unicorn/no-useless-spread': 'error',
 				'unicorn/no-useless-undefined': 'error',
 				'unicorn/no-zero-fractions': 'error',
 				'unicorn/number-literal-case': 'error',

readme.md

@@ -75,6 +75,7 @@ Configure it in `package.json`.
 			"unicorn/no-unsafe-regex": "off",
 			"unicorn/no-unused-properties": "off",
 			"unicorn/no-useless-length-check": "error",
+			"unicorn/no-useless-spread": "error",
 			"unicorn/no-useless-undefined": "error",
 			"unicorn/no-zero-fractions": "error",
 			"unicorn/number-literal-case": "error",
@@ -180,6 +181,7 @@ Each rule has emojis denoting:
 | [no-unsafe-regex](docs/rules/no-unsafe-regex.md) | Disallow unsafe regular expressions. |  |  |  |
 | [no-unused-properties](docs/rules/no-unused-properties.md) | Disallow unused object properties. |  |  |  |
 | [no-useless-length-check](docs/rules/no-useless-length-check.md) | Disallow useless array length check. | ✅ | 🔧 |  |
+| [no-useless-spread](docs/rules/no-useless-spread.md) | Disallow unnecessary spread. | ✅ | 🔧 |  |
 | [no-useless-undefined](docs/rules/no-useless-undefined.md) | Disallow useless `undefined`. | ✅ | 🔧 |  |
 | [no-zero-fractions](docs/rules/no-zero-fractions.md) | Disallow number literals with zero fractions or dangling dots. | ✅ | 🔧 |  |
 | [number-literal-case](docs/rules/number-literal-case.md) | Enforce proper case for numeric literals. | ✅ | 🔧 |  |

rules/no-useless-spread.js

@@ -0,0 +1,124 @@
+'use strict';
+const {isCommaToken} = require('eslint-utils');
+const {matches} = require('./selectors/index.js');
+const {getParentheses} = require('./utils/parentheses.js');
+
+const MESSAGE_ID = 'no-useless-spread';
+const messages = {
+	[MESSAGE_ID]: 'Spread an {{argumentType}} literal in {{parentDescription}} is unnecessary.'
+};
+
+const selector = matches([
+	'ArrayExpression > SpreadElement.elements > ArrayExpression.argument',
+	'ObjectExpression > SpreadElement.properties > ObjectExpression.argument',
+	'CallExpression > SpreadElement.arguments > ArrayExpression.argument',
+	'NewExpression > SpreadElement.arguments > ArrayExpression.argument'
+]);
+
+const parentDescriptions = {
+	ArrayExpression: 'array literal',
+	ObjectExpression: 'object literal',
+	CallExpression: 'arguments',
+	NewExpression: 'arguments'
+};
+
+function getCommaTokens(arrayExpression, sourceCode) {
+	let startToken = sourceCode.getFirstToken(arrayExpression);
+
+	return arrayExpression.elements.map((element, index, elements) => {
+		if (index === elements.length - 1) {
+			const penultimateToken = sourceCode.getLastToken(arrayExpression, {skip: 1});
+			if (isCommaToken(penultimateToken)) {
+				return penultimateToken;
+			}
+
+			return;
+		}
+
+		const commaToken = sourceCode.getTokenAfter(element || startToken, isCommaToken);
+		startToken = commaToken;
+		return commaToken;
+	});
+}
+
+/** @param {import('eslint').Rule.RuleContext} context */
+const create = context => {
+	const sourceCode = context.getSourceCode();
+
+	return {
+		[selector](spreadObject) {
+			const spreadElement = spreadObject.parent;
+			const spreadToken = sourceCode.getFirstToken(spreadElement);
+			const parentType = spreadElement.parent.type;
+
+			return {
+				node: spreadToken,
+				messageId: MESSAGE_ID,
+				data: {
+					argumentType: spreadObject.type === 'ArrayExpression' ? 'array' : 'object',
+					parentDescription: parentDescriptions[parentType]
+				},
+				/** @param {import('eslint').Rule.RuleFixer} fixer */
+				* fix(fixer) {
+					// `[...[foo]]`
+					//   ^^^
+					yield fixer.remove(spreadToken);
+
+					// `[...(( [foo] ))]`
+					//      ^^       ^^
+					const parentheses = getParentheses(spreadObject, sourceCode);
+					for (const parenthesis of parentheses) {
+						yield fixer.remove(parenthesis);
+					}
+
+					// `[...[foo]]`
+					//      ^
+					const firstToken = sourceCode.getFirstToken(spreadObject);
+					yield fixer.remove(firstToken);
+
+					const [
+						penultimateToken,
+						lastToken
+					] = sourceCode.getLastTokens(spreadObject, 2);
+
+					// `[...[foo]]`
+					//          ^
+					yield fixer.remove(lastToken);
+
+					// `[...[foo,]]`
+					//          ^
+					if (isCommaToken(penultimateToken)) {
+						yield fixer.remove(penultimateToken);
+					}
+
+					if (parentType !== 'CallExpression' && parentType !== 'NewExpression') {
+						return;
+					}
+
+					const commaTokens = getCommaTokens(spreadObject, sourceCode);
+					for (const [index, commaToken] of commaTokens.entries()) {
+						if (spreadObject.elements[index]) {
+							continue;
+						}
+
+						// `call([foo, , bar])`
+						//             ^ Replace holes with `undefined`
+						yield fixer.insertTextBefore(commaToken, 'undefined');
+					}
+				}
+			};
+		}
+	};
+};
+
+module.exports = {
+	create,
+	meta: {
+		type: 'suggestion',
+		docs: {
+			description: 'Disallow unnecessary spread.'
+		},
+		fixable: 'code',
+		messages
+	}
+};

test/no-useless-spread.mjs

@@ -0,0 +1,140 @@
+import outdent from 'outdent';
+import {getTester} from './utils/test.mjs';
+
+const {test} = getTester(import.meta);
+
+test.snapshot({
+	valid: [
+		'const array = [[]]',
+		'const array = [{}]',
+		'const object = ({...[]})',
+		'const array = [...[].map(x => x)]',
+		'foo([])',
+		'foo({})',
+		'new Foo([])',
+		'new Foo({})',
+		'const array = [...a]',
+		'const object = {...a}',
+		'const [first, ...rest] = []',
+		'const {foo, ...rest} = {}',
+		'function a(foo, ...rest) {}'
+	],
+	invalid: [
+		'const array = [...[a]]',
+		'const object = {...{a}}',
+		'foo(...[a])',
+		'new Foo(...[a])',
+
+		// Trailing comma
+		'const array = [...[a,]]',
+		'const object = {...{a,}}',
+		'foo(...[a,])',
+		'new Foo(...[a,])',
+
+		// Trailing comma in parent
+		'const array = [...[a,],]',
+		'const object = {...{a,},}',
+		'foo(...[a,],)',
+		'new Foo(...[a,],)',
+
+		// Parentheses
+		'const array = [...(( [a] ))]',
+		'const object = {...(( {a} ))}',
+		'foo(...(( [a] )))',
+		'new Foo(...(( [a] )))',
+
+		// Empty
+		'const array = [...[]]',
+		'const object = {...{}}',
+		'foo(...[])',
+		'new Foo(...[])',
+
+		// Hole(s)
+		'const array = [...[,]]',
+		'foo(...[,])',
+		'new Foo(...[,])',
+		'const array = [...[,,]]',
+		'foo(...[,,])',
+		'new Foo(...[,,])',
+		'const array = [...[a, , b,]]',
+		'foo(...[a, , b,])',
+		'new Foo(...[a, , b,])',
+		'const array = [...[a, , b,],]',
+		'foo(...[a, , b,],)',
+		'new Foo(...[a, , b,],)',
+		'foo(...[,, ,(( a )), ,,(0, b), ,,])',
+
+		// Extra elements/properties
+		'const array = [a, ...[a, b]]',
+		'const object = {a, ...{a, b}}',
+		'foo(a, ...[a, b])',
+		'new Foo(a, ...[a, b])',
+		'const array = [...[a, b], b,]',
+		'const object = {...{a, b}, b,}',
+		'foo(...[a, b], b,)',
+		'new Foo(...[a, b], b,)',
+		'const array = [a, ...[a, b], b,]',
+		'const object = {a, ...{a, b}, b,}',
+		'foo(a, ...[a, b], b,)',
+		'new Foo(a, ...[a, b], b,)',
+
+		// Duplicated keys
+		'({a:1, ...{a: 2}})',
+		'({...{a:1}, ...{a: 2}})',
+		outdent`
+			({
+				get a() {},
+				set a(v) {},
+				...{
+					get a() {}
+				}
+			})
+		`,
+		// Computed
+		'({[a]:1, ...{[a]: 2}})',
+
+		outdent`
+			const object = {
+				a: 1,
+
+				...{
+					testKeys() {
+						console.assert(Object.keys(this).length === 2)
+					}
+				}
+			}
+			object.testKeys();
+		`,
+
+		outdent`
+			new Foo(
+				foo(
+					a,
+					...[a, b],
+					b,
+				),
+				...[
+					a,
+					...[
+						a,
+						b,
+					],
+					b,
+				],
+				{
+					a: [...[a, b]],
+					...{
+						a,
+						b,
+					},
+				}
+			)
+		`,
+
+		// Code from example in `prefer-spread` rule docs
+		outdent`
+			const baz = [2];
+			call(foo, ...[bar, ...baz]);
+		`
+	]
+});

test/prefer-spread.mjs

@@ -279,7 +279,12 @@ test.snapshot({
 		{
 			code: 'with (foo) foo.concat(1)',
 			parserOptions: {ecmaVersion: 5, sourceType: 'script'}
-		}
+		},
+		// Code from example in docs
+		outdent`
+			const baz = [2];
+			call(foo, ...[bar].concat(baz));
+		`
 	]
 });