prefer-spread: Prefer ... over Array#concat() (#1029)

Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>

docs/rules/prefer-spread.md

@@ -1,19 +1,25 @@
-# Prefer the spread operator over `Array.from()`
+# Prefer the spread operator over `Array.from()` and `Array#concat()`
 
-Enforces the use of the spread operator over `Array.from()`. This rule adds on to the built-in [prefer-spread](https://eslint.org/docs/rules/prefer-spread) rule, which only flags uses of `.apply()`. Does not enforce for `TypedArray.from()`;
-
-This rule is fixable.
+Enforces the use of the spread operator over `Array.from()` and `Array#concat()`. This rule adds on to the built-in [prefer-spread](https://eslint.org/docs/rules/prefer-spread) rule, which only flags uses of `.apply()`. Does not enforce for `TypedArray.from()`;
 
+This rule is partly fixable.
 
 ## Fail
 
 ```js
-Array.from(set).map(() => {});
+Array.from(set).map(element => foo(element));
 ```
 
+```js
+const array = array1.concat(array2);
+```
 
 ## Pass
 
 ```js
-[...set].map(() => {});
+[...set].map(element => foo(element));
+```
+
+```js
+const array = [...array1, ...array2];
 ```

readme.md

@@ -173,7 +173,7 @@ Configure it in `package.json`.
 - [prefer-reflect-apply](docs/rules/prefer-reflect-apply.md) - Prefer `Reflect.apply()` over `Function#apply()`. *(fixable)*
 - [prefer-regexp-test](docs/rules/prefer-regexp-test.md) - Prefer `RegExp#test()` over `String#match()` and `RegExp#exec()`. *(fixable)*
 - [prefer-set-has](docs/rules/prefer-set-has.md) - Prefer `Set#has()` over `Array#includes()` when checking for existence or non-existence. *(fixable)*
-- [prefer-spread](docs/rules/prefer-spread.md) - Prefer the spread operator over `Array.from()`. *(fixable)*
+- [prefer-spread](docs/rules/prefer-spread.md) - Prefer the spread operator over `Array.from()` and `Array#concat()`. *(partly fixable)*
 - [prefer-string-replace-all](docs/rules/prefer-string-replace-all.md) - Prefer `String#replaceAll()` over regex searches with the global flag. *(fixable)*
 - [prefer-string-slice](docs/rules/prefer-string-slice.md) - Prefer `String#slice()` over `String#substr()` and `String#substring()`. *(partly fixable)*
 - [prefer-string-starts-ends-with](docs/rules/prefer-string-starts-ends-with.md) - Prefer `String#startsWith()` & `String#endsWith()` over `RegExp#test()`. *(fixable)*

rules/no-array-push-push.js

@@ -1,7 +1,8 @@
 'use strict';
-const {hasSideEffect, isCommaToken, isOpeningParenToken, isSemicolonToken} = require('eslint-utils');
+const {hasSideEffect, isCommaToken, isSemicolonToken} = require('eslint-utils');
 const getDocumentationUrl = require('./utils/get-documentation-url');
 const methodSelector = require('./utils/method-selector');
+const getCallExpressionArgumentsText = require('./utils/get-call-expression-arguments-text');
 
 const ERROR = 'error';
 const SUGGESTION = 'suggestion';
@@ -20,16 +21,6 @@ const arrayPushExpressionStatement = [
 
 const selector = `${arrayPushExpressionStatement} + ${arrayPushExpressionStatement}`;
 
-const getCallExpressionArgumentsText = (node, sourceCode) => {
-	const openingParenthesisToken = sourceCode.getTokenAfter(node.callee, isOpeningParenToken);
-	const closingParenthesisToken = sourceCode.getLastToken(node);
-
-	return sourceCode.text.slice(
-		openingParenthesisToken.range[1],
-		closingParenthesisToken.range[0]
-	);
-};
-
 function getFirstExpression(node, sourceCode) {
 	const {parent} = node;
 	const visitorKeys = sourceCode.visitorKeys[parent.type] || Object.keys(parent);

rules/no-unused-properties.js

@@ -129,7 +129,7 @@ const create = context => {
 				continue;
 			}
 
-			const nextPath = path.concat(key);
+			const nextPath = [...path, key];
 
 			const nextReferences = references
 				.map(reference => {

rules/prefer-spread.js

@@ -1,14 +1,23 @@
 'use strict';
+const {getStaticValue, isCommaToken} = require('eslint-utils');
 const getDocumentationUrl = require('./utils/get-documentation-url');
 const methodSelector = require('./utils/method-selector');
 const needsSemicolon = require('./utils/needs-semicolon');
+const getParentheses = require('./utils/get-parentheses');
+const getCallExpressionArgumentsText = require('./utils/get-call-expression-arguments-text');
 
-const MESSAGE_ID = 'prefer-spread';
+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 messages = {
-	[MESSAGE_ID]: 'Prefer the spread operator over `Array.from()`.'
+	[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]: 'Argument of `Array#concat(…)` is an `array`.',
+	[SUGGESTION_CONCAT_ARGUMENT_IS_NOT_SPREADABLE]: 'Argument of `Array#concat(…)` is not an `array`.'
 };
 
-const selector = [
+const arrayFromCallSelector = [
 	methodSelector({
 		object: 'Array',
 		name: 'from',
@@ -19,15 +28,88 @@ const selector = [
 	'[arguments.0.type!="ObjectExpression"]'
 ].join('');
 
+const arrayConcatCallSelector = [
+	methodSelector({
+		name: 'concat'
+	}),
+	`:not(${
+		[
+			'Literal',
+			'TemplateLiteral'
+		].map(type => `[callee.object.type="${type}"]`).join(', ')
+	})`
+].join('');
+
+const isArrayLiteral = node => node.type === 'ArrayExpression';
+
+function fixConcat(node, sourceCode, isSpreadable) {
+	const array = node.callee.object;
+	const [item] = node.arguments;
+
+	const getRangeAfterArray = () => {
+		const [, start] = getParenthesizedArrayRange();
+		const [, end] = node.range;
+
+		return [start, end];
+	};
+
+	const getParenthesizedArrayRange = () => {
+		const [firstToken = array, lastToken = array] = getParentheses(array, sourceCode);
+
+		const [start] = firstToken.range;
+		const [, end] = lastToken.range;
+		return [start, end];
+	};
+
+	const getFixedText = () => {
+		if (isArrayLiteral(item)) {
+			return sourceCode.getText(item, -1, -1);
+		}
+
+		const text = getCallExpressionArgumentsText(node, sourceCode);
+		return isSpreadable ? `...${text}` : text;
+	};
+
+	return function * (fixer) {
+		// Fixed code always starts with `[` or `(`
+		if (needsSemicolon(sourceCode.getTokenBefore(node), sourceCode, '[')) {
+			yield fixer.insertTextBefore(node, ';');
+		}
+
+		const rangeAfterArray = getRangeAfterArray();
+		let text = getFixedText();
+
+		if (isArrayLiteral(array)) {
+			const [penultimateToken, closingBracketToken] = sourceCode.getLastTokens(array, 2);
+
+			if (array.elements.length > 0) {
+				text = ` ${text}`;
+
+				if (!isCommaToken(penultimateToken)) {
+					text = `,${text}`;
+				}
+			}
+
+			yield fixer.insertTextBefore(closingBracketToken, text);
+		} else {
+			yield fixer.insertTextBefore(node, '[...');
+			yield fixer.insertTextAfterRange(getParenthesizedArrayRange(), `, ${text}`);
+			yield fixer.insertTextAfter(node, ']');
+		}
+
+		yield fixer.replaceTextRange(rangeAfterArray, '');
+	};
+}
+
 const create = context => {
 	const sourceCode = context.getSourceCode();
 	const getSource = node => sourceCode.getText(node);
 
 	return {
-		[selector](node) {
+		[arrayFromCallSelector](node) {
 			context.report({
 				node,
-				messageId: MESSAGE_ID,
+				messageId: ERROR_ARRAY_FROM,
 				fix: fixer => {
 					const [arrayLikeArgument, mapFn, thisArgument] = node.arguments.map(node => getSource(node));
 					let replacement = `${
@@ -42,6 +124,55 @@ const create = context => {
 					return fixer.replaceText(node, replacement);
 				}
 			});
+		},
+		[arrayConcatCallSelector](node) {
+			const scope = context.getScope();
+			const staticResult = getStaticValue(node.callee.object, scope);
+
+			if (staticResult && !Array.isArray(staticResult.value)) {
+				return;
+			}
+
+			const problem = {
+				node: node.callee.property,
+				messageId: ERROR_ARRAY_CONCAT
+			};
+
+			const [item] = node.arguments;
+			if (node.arguments.length !== 1 || item.type === 'SpreadElement') {
+				context.report(problem);
+				return;
+			}
+
+			let isItemArray;
+			if (isArrayLiteral(item)) {
+				isItemArray = true;
+			} else {
+				const result = getStaticValue(item, scope);
+
+				if (result) {
+					isItemArray = Array.isArray(result.value);
+				}
+			}
+
+			if (isItemArray === true) {
+				problem.fix = fixConcat(node, sourceCode, /* isSpreadable */ true);
+			} else if (isItemArray === false) {
+				problem.fix = fixConcat(node, sourceCode, /* isSpreadable */ false);
+			} else {
+				problem.suggest = [
+					{
+						messageId: SUGGESTION_CONCAT_ARGUMENT_IS_SPREADABLE,
+						fix: fixConcat(node, sourceCode, /* isSpreadable */ true)
+					},
+					{
+						messageId: SUGGESTION_CONCAT_ARGUMENT_IS_NOT_SPREADABLE,
+						fix: fixConcat(node, sourceCode, /* isSpreadable */ false)
+					}
+				];
+			}
+
+			context.report(problem);
 		}
 	};
 };

rules/prevent-abbreviations.js

@@ -569,7 +569,7 @@ const create = context => {
 					scope: variable.scope,
 					defs: variable.defs,
 					identifiers: variable.identifiers,
-					references: variable.references.concat(outerClassVariable.references)
+					references: [...variable.references, ...outerClassVariable.references]
 				};
 
 				// Call the common checker with the newly forged normalized class variable
@@ -640,7 +640,10 @@ const create = context => {
 			return;
 		}
 
-		const scopes = variable.references.map(reference => reference.from).concat(variable.scope);
+		const scopes = [
+			...variable.references.map(reference => reference.from),
+			variable.scope
+		];
 		variableReplacements.samples = variableReplacements.samples.map(
 			name => avoidCapture(name, scopes, ecmaVersion, isSafeName)
 		);

rules/utils/get-call-expression-arguments-text.js

@@ -0,0 +1,21 @@
+'use strict';
+const {isOpeningParenToken} = require('eslint-utils');
+
+/**
+Get the text of the arguments list of `CallExpression`.
+
+@param {Node} node - The `CallExpression` node.
+@param {SourceCode} sourceCode - The source code object.
+@returns {string}
+*/
+const getCallExpressionArgumentsText = (node, sourceCode) => {
+	const openingParenthesisToken = sourceCode.getTokenAfter(node.callee, isOpeningParenToken);
+	const closingParenthesisToken = sourceCode.getLastToken(node);
+
+	return sourceCode.text.slice(
+		openingParenthesisToken.range[1],
+		closingParenthesisToken.range[0]
+	);
+};
+
+module.exports = getCallExpressionArgumentsText;

rules/utils/get-parentheses.js

@@ -0,0 +1,25 @@
+'use strict';
+const {isOpeningParenToken, isClosingParenToken} = require('eslint-utils');
+const getParenthesizedTimes = require('./get-parenthesized-times');
+
+/*
+Get the first opening parenthesis token and the last closing parenthesis token of a parenthesized node.
+
+@param {Node} node - The node to be checked.
+@param {SourceCode} sourceCode - The source code object.
+@returns {Token[]}
+*/
+function getParentheses(node, sourceCode) {
+	const parenthesizedTimes = getParenthesizedTimes(node, sourceCode);
+
+	if (parenthesizedTimes > 0) {
+		return [
+			sourceCode.getTokenBefore(node, {skip: parenthesizedTimes - 1, filter: isOpeningParenToken}),
+			sourceCode.getTokenAfter(node, {skip: parenthesizedTimes - 1, filter: isClosingParenToken})
+		];
+	}
+
+	return [];
+}
+
+module.exports = getParentheses;

rules/utils/get-references.js

@@ -1,10 +1,9 @@
 'use strict';
-const {uniq} = require('lodash');
+const {uniq, flatten} = require('lodash');
 
-const getReferences = scope => uniq(
-	scope.references.concat(
-		...scope.childScopes.map(scope => getReferences(scope))
-	)
-);
+const getReferences = scope => uniq([
+	...scope.references,
+	...flatten(scope.childScopes.map(scope => getReferences(scope)))
+]);
 
 module.exports = getReferences;