Add isMethodCall to replace selectors (#2094)

rules/ast/index.js

@@ -24,4 +24,5 @@ module.exports = {
 	isNewExpression: require('./call-or-new-expression.js').isNewExpression,
 	isCallExpression: require('./call-or-new-expression.js').isCallExpression,
 	isMemberExpression: require('./is-member-expression.js'),
+	isMethodCall: require('./is-method-call.js'),
 };

rules/ast/is-member-expression.js

@@ -34,7 +34,6 @@ function isMemberExpression(node, options) {
 		optional,
 		computed,
 	} = {
-		path: '',
 		property: '',
 		properties: [],
 		object: '',

rules/ast/is-method-call.js

@@ -0,0 +1,61 @@
+'use strict';
+const {pick} = require('lodash');
+const isMemberExpression = require('./is-member-expression.js');
+const {isCallExpression} = require('./call-or-new-expression.js');
+
+/**
+@param {
+	{
+		// `isCallExpression` options
+		argumentsLength?: number,
+		minimumArguments?: number,
+		maximumArguments?: number,
+		optionalCall?: boolean,
+		allowSpreadElement?: boolean,
+
+		// `isMemberExpression` options
+		method?: string,
+		methods?: string[],
+		object?: string,
+		objects?: string[],
+		optionalMember?: boolean,
+		computed?: boolean
+	} | string | string[]
+} [options]
+@returns {string}
+*/
+function isMethodCall(node, options) {
+	if (typeof options === 'string') {
+		options = {methods: [options]};
+	}
+
+	if (Array.isArray(options)) {
+		options = {methods: options};
+	}
+
+	const {
+		optionalCall,
+		optionalMember,
+		method,
+		methods,
+	} = {
+		method: '',
+		methods: [],
+		...options,
+	};
+
+	return (
+		isCallExpression(node, {
+			...pick(options, ['argumentsLength', 'minimumArguments', 'maximumArguments', 'allowSpreadElement']),
+			optional: optionalCall,
+		})
+		&& isMemberExpression(node.callee, {
+			...pick(options, ['object', 'objects', 'computed']),
+			property: method,
+			properties: methods,
+			optional: optionalMember,
+		})
+	);
+}
+
+module.exports = isMethodCall;

rules/no-array-for-each.js

@@ -7,7 +7,7 @@ const {
 	findVariable,
 	hasSideEffect,
 } = require('@eslint-community/eslint-utils');
-const {methodCallSelector, referenceIdentifierSelector} = require('./selectors/index.js');
+const {referenceIdentifierSelector} = require('./selectors/index.js');
 const {extendFixRange} = require('./fix/index.js');
 const needsSemicolon = require('./utils/needs-semicolon.js');
 const shouldAddParenthesesToExpressionStatementExpression = require('./utils/should-add-parentheses-to-expression-statement-expression.js');
@@ -17,7 +17,7 @@ const isFunctionSelfUsedInside = require('./utils/is-function-self-used-inside.j
 const {isNodeMatches} = require('./utils/is-node-matches.js');
 const assertToken = require('./utils/assert-token.js');
 const {fixSpaceAroundKeyword, removeParentheses} = require('./fix/index.js');
-const {isArrowFunctionBody} = require('./ast/index.js');
+const {isArrowFunctionBody, isMethodCall} = require('./ast/index.js');
 
 const MESSAGE_ID_ERROR = 'no-array-for-each/error';
 const MESSAGE_ID_SUGGESTION = 'no-array-for-each/suggestion';
@@ -26,12 +26,6 @@ const messages = {
 	[MESSAGE_ID_SUGGESTION]: 'Switch to `for…of`.',
 };
 
-const forEachMethodCallSelector = methodCallSelector({
-	method: 'forEach',
-	includeOptionalCall: true,
-	includeOptionalMember: true,
-});
-
 const continueAbleNodeTypes = new Set([
 	'WhileStatement',
 	'DoWhileStatement',
@@ -407,8 +401,14 @@ const create = context => {
 			const {returnStatements} = functionInfo.get(currentFunction);
 			returnStatements.push(node);
 		},
-		[forEachMethodCallSelector](node) {
-			if (isNodeMatches(node.callee.object, ignoredObjects)) {
+		CallExpression(node) {
+			if (
+				!isMethodCall(node, {
+					method: 'forEach',
+					computed: false,
+				})
+				|| isNodeMatches(node.callee.object, ignoredObjects)
+			) {
 				return;
 			}
 

rules/no-array-push-push.js

@@ -1,9 +1,10 @@
 'use strict';
 const {hasSideEffect, isCommaToken, isSemicolonToken} = require('@eslint-community/eslint-utils');
-const {methodCallSelector} = require('./selectors/index.js');
 const getCallExpressionArgumentsText = require('./utils/get-call-expression-arguments-text.js');
 const isSameReference = require('./utils/is-same-reference.js');
 const {isNodeMatches} = require('./utils/is-node-matches.js');
+const getPreviousNode = require('./utils/get-previous-node.js');
+const {isMethodCall} = require('./ast/index.js');
 
 const ERROR = 'error';
 const SUGGESTION = 'suggestion';
@@ -12,30 +13,22 @@ const messages = {
 	[SUGGESTION]: 'Merge with previous one.',
 };
 
-const arrayPushExpressionStatement = [
-	'ExpressionStatement',
-	methodCallSelector({path: 'expression', method: 'push'}),
-].join('');
-
-const selector = `${arrayPushExpressionStatement} + ${arrayPushExpressionStatement}`;
-
-function getFirstExpression(node, sourceCode) {
-	const {parent} = node;
-	const visitorKeys = sourceCode.visitorKeys[parent.type] || Object.keys(parent);
-
-	for (const property of visitorKeys) {
-		const value = parent[property];
-		if (Array.isArray(value)) {
-			const index = value.indexOf(node);
-
-			if (index !== -1) {
-				return value[index - 1];
-			}
-		}
+const isArrayPushCall = node =>
+	node
+	&& node.parent.type === 'ExpressionStatement'
+	&& node.parent.expression === node
+	&& isMethodCall(node, {
+		method: 'push',
+		optionalCall: false,
+		optionalMember: false,
+		computed: false,
+	});
+
+function getFirstArrayPushCall(secondCall, sourceCode) {
+	const firstCall = getPreviousNode(secondCall.parent, sourceCode)?.expression;
+	if (isArrayPushCall(firstCall)) {
+		return firstCall;
 	}
-
-	/* c8 ignore next */
-	throw new Error('Cannot find the first `Array#push()` call.\nPlease open an issue at https://github.com/sindresorhus/eslint-plugin-unicorn/issues/new?title=%60no-array-push-push%60%3A%20Cannot%20find%20first%20%60push()%60');
 }
 
 function create(context) {
@@ -55,16 +48,22 @@ function create(context) {
 	const {sourceCode} = context;
 
 	return {
-		[selector](secondExpression) {
-			const secondCall = secondExpression.expression;
+		CallExpression(secondCall) {
+			if (!isArrayPushCall(secondCall)) {
+				return;
+			}
+
 			const secondCallArray = secondCall.callee.object;
 
 			if (isNodeMatches(secondCallArray, ignoredObjects)) {
 				return;
 			}
 
-			const firstExpression = getFirstExpression(secondExpression, sourceCode);
-			const firstCall = firstExpression.expression;
+			const firstCall = getFirstArrayPushCall(secondCall, sourceCode);
+			if (!firstCall) {
+				return;
+			}
+
 			const firstCallArray = firstCall.callee.object;
 
 			// Not same array
@@ -90,6 +89,8 @@ function create(context) {
 					);
 				}
 
+				const firstExpression = firstCall.parent;
+				const secondExpression = secondCall.parent;
 				const shouldKeepSemicolon = !isSemicolonToken(sourceCode.getLastToken(firstExpression))
 					&& isSemicolonToken(sourceCode.getLastToken(secondExpression));
 

rules/no-console-spaces.js

@@ -1,27 +1,12 @@
 'use strict';
-const {methodCallSelector} = require('./selectors/index.js');
 const toLocation = require('./utils/to-location.js');
-const {isStringLiteral} = require('./ast/index.js');
+const {isStringLiteral, isMethodCall} = require('./ast/index.js');
 
 const MESSAGE_ID = 'no-console-spaces';
 const messages = {
 	[MESSAGE_ID]: 'Do not use {{position}} space between `console.{{method}}` parameters.',
 };
 
-const methods = [
-	'log',
-	'debug',
-	'info',
-	'warn',
-	'error',
-];
-
-const selector = methodCallSelector({
-	methods,
-	minimumArguments: 1,
-	object: 'console',
-});
-
 // Find exactly one leading space, allow exactly one space
 const hasLeadingSpace = value => value.length > 1 && value.charAt(0) === ' ' && value.charAt(1) !== ' ';
 
@@ -46,7 +31,26 @@ const create = context => {
 	};
 
 	return {
-		* [selector](node) {
+		* CallExpression(node) {
+			if (
+				!isMethodCall(node, {
+					object: 'console',
+					methods: [
+						'log',
+						'debug',
+						'info',
+						'warn',
+						'error',
+					],
+					minimumArguments: 1,
+					optionalCall: false,
+					optionalMember: false,
+					computed: false,
+				})
+			) {
+				return;
+			}
+
 			const method = node.callee.property.name;
 			const {arguments: messages} = node;
 			const {length} = messages;

rules/utils/get-previous-node.js

@@ -0,0 +1,24 @@
+'use strict';
+
+function getPreviousNode(node, sourceCode) {
+	const {parent} = node;
+	const visitorKeys = sourceCode.visitorKeys[parent.type] || Object.keys(parent);
+
+	for (const property of visitorKeys) {
+		const value = parent[property];
+
+		if (value === node) {
+			return;
+		}
+
+		if (Array.isArray(value)) {
+			const index = value.indexOf(node);
+
+			if (index !== -1) {
+				return value[index - 1];
+			}
+		}
+	}
+}
+
+module.exports = getPreviousNode;

test/no-array-push-push.mjs

@@ -84,6 +84,7 @@ test.snapshot({
 				},
 			],
 		},
+		'for (const _ of []) foo.push(bar);',
 	],
 	invalid: [
 		outdent`