prefer-spread: Ignore Array#join().concat() and (a + b).concat() (#1859)

Author: fisker

rules/prefer-spread.js

@@ -1,6 +1,6 @@
 'use strict';
 const {isParenthesized, getStaticValue, isCommaToken, hasSideEffect} = require('eslint-utils');
-const {methodCallSelector, not} = require('./selectors/index.js');
+const {methodCallSelector} = require('./selectors/index.js');
 const needsSemicolon = require('./utils/needs-semicolon.js');
 const {getParenthesizedRange, getParenthesizedText} = require('./utils/parentheses.js');
 const shouldAddParenthesesToSpreadElementArgument = require('./utils/should-add-parentheses-to-spread-element-argument.js');
@@ -11,6 +11,7 @@ const {
 	removeMethodCall,
 } = require('./fix/index.js');
 const {isLiteral} = require('./ast/index.js');
+const isMethodNamed = require('./utils/is-method-named.js');
 
 const ERROR_ARRAY_FROM = 'array-from';
 const ERROR_ARRAY_CONCAT = 'array-concat';
@@ -44,15 +45,7 @@ const arrayFromCallSelector = [
 	'[arguments.0.type!="ObjectExpression"]',
 ].join('');
 
-const arrayConcatCallSelector = [
-	methodCallSelector('concat'),
-	not(
-		[
-			'Literal',
-			'TemplateLiteral',
-		].map(type => `[callee.object.type="${type}"]`),
-	),
-].join('');
+const arrayConcatCallSelector = methodCallSelector('concat');
 
 const arraySliceCallSelector = [
 	methodCallSelector({
@@ -320,6 +313,26 @@ function isClassName(node) {
 	return /^[A-Z]./.test(name) && name.toUpperCase() !== name;
 }
 
+function isNotArray(node, scope) {
+	if (
+		node.type === 'TemplateLiteral'
+		|| node.type === 'Literal'
+		|| node.type === 'BinaryExpression'
+		|| isClassName(node)
+		// `foo.join()`
+		|| (isMethodNamed(node, 'join') && node.arguments.length <= 1)
+	) {
+		return true;
+	}
+
+	const staticValue = getStaticValue(node, scope);
+	if (staticValue && !Array.isArray(staticValue.value)) {
+		return true;
+	}
+
+	return false;
+}
+
 /** @param {import('eslint').Rule.RuleContext} context */
 const create = context => {
 	const sourceCode = context.getSourceCode();
@@ -335,7 +348,7 @@ const create = context => {
 		[arrayConcatCallSelector](node) {
 			const {object} = node.callee;
 
-			if (isClassName(object)) {
+			if (isNotArray(object, context.getScope())) {
 				return;
 			}
 

test/prefer-spread.mjs

@@ -180,6 +180,11 @@ test.snapshot({
 		'Foo.concat(1)',
 		'FooBar.concat(1)',
 		'global.Buffer.concat([])',
+		// #1809
+		'["1", "2"].join(",").concat("...")',
+		'foo.join(",").concat("...")',
+		'foo.join().concat(bar)',
+		'(a + b).concat(c)',
 	],
 	invalid: [
 		'[1].concat(2)',
@@ -288,6 +293,8 @@ test.snapshot({
 			const baz = [2];
 			call(foo, ...[bar].concat(baz));
 		`,
+		// This not considered `Array#join()` since there are more than one argument
+		'foo.join(foo, bar).concat("...")',
 	],
 });
 

test/snapshots/prefer-spread.mjs.md

@@ -2034,6 +2034,22 @@ Generated by [AVA](https://avajs.dev).
         |                    ^^^^^^ Prefer the spread operator over \`Array#concat(…)\`.␊
     `
 
+## Invalid #68
+      1 | foo.join(foo, bar).concat("...")
+
+> Output
+
+    `␊
+      1 | [...foo.join(foo, bar), "..."]␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | foo.join(foo, bar).concat("...")␊
+        |                    ^^^^^^ Prefer the spread operator over \`Array#concat(…)\`.␊
+    `
+
 ## Invalid #1
       1 | array.slice()