Refactor simpleArraySearchRule (#2111)

sindresorhus · May 17, 2023 · 2bfc7c2 · 2bfc7c2
1 parent 64342cb
commit 2bfc7c2
Show file tree

Hide file tree

Showing 5 changed files with 128 additions and 105 deletions.
diff --git a/rules/prefer-array-index-of.js b/rules/prefer-array-index-of.js
@@ -13,10 +13,10 @@ const lastIndexOfOverFindLastIndexRule = simpleArraySearchRule({
 
 /** @type {import('eslint').Rule.RuleModule} */
 module.exports = {
-	create: context => ({
-		...indexOfOverFindIndexRule.createListeners(context),
-		...lastIndexOfOverFindLastIndexRule.createListeners(context),
-	}),
+	create(context) {
+		indexOfOverFindIndexRule.listen(context);
+		lastIndexOfOverFindLastIndexRule.listen(context);
+	},
 	meta: {
 		type: 'suggestion',
 		docs: {

diff --git a/rules/prefer-includes.js b/rules/prefer-includes.js
@@ -43,8 +43,10 @@ const includesOverSomeRule = simpleArraySearchRule({
 });
 
 /** @param {import('eslint').Rule.RuleContext} context */
-const create = context => ({
-	BinaryExpression(node) {
+const create = context => {
+	includesOverSomeRule.listen(context);
+
+	context.on('BinaryExpression', node => {
 		const {left, right, operator} = node;
 
 		if (!isMethodNamed(left, 'indexOf')) {
@@ -75,9 +77,8 @@ const create = context => ({
 				argumentsNodes,
 			);
 		}
-	},
-	...includesOverSomeRule.createListeners(context),
-});
+	});
+};
 
 /** @type {import('eslint').Rule.RuleModule} */
 module.exports = {

diff --git a/rules/shared/simple-array-search-rule.js b/rules/shared/simple-array-search-rule.js
@@ -1,38 +1,36 @@
 'use strict';
 
 const {hasSideEffect, isParenthesized, findVariable} = require('@eslint-community/eslint-utils');
-const {matches, methodCallSelector} = require('../selectors/index.js');
-const isFunctionSelfUsedInside = require('../utils/is-function-self-used-inside.js');
-
-const getBinaryExpressionSelector = path => [
-	`[${path}.type="BinaryExpression"]`,
-	`[${path}.operator="==="]`,
-	`:matches([${path}.left.type="Identifier"], [${path}.right.type="Identifier"])`,
-].join('');
-const getFunctionSelector = path => [
-	`[${path}.generator!=true]`,
-	`[${path}.async!=true]`,
-	`[${path}.params.length=1]`,
-	`[${path}.params.0.type="Identifier"]`,
-].join('');
-const callbackFunctionSelector = path => matches([
+const {isMethodCall} = require('../ast/index.js');
+const {isSameIdentifier, isFunctionSelfUsedInside} = require('../utils/index.js');
+
+const isSimpleCompare = (node, compareNode) =>
+	node.type === 'BinaryExpression'
+	&& node.operator === '==='
+	&& (
+		isSameIdentifier(node.left, compareNode)
+		|| isSameIdentifier(node.right, compareNode)
+	);
+const isSimpleCompareCallbackFunction = node =>
 	// Matches `foo.findIndex(bar => bar === baz)`
-	[
-		`[${path}.type="ArrowFunctionExpression"]`,
-		getFunctionSelector(path),
-		getBinaryExpressionSelector(`${path}.body`),
-	].join(''),
+	(
+		node.type === 'ArrowFunctionExpression'
+		&& !node.async
+		&& node.params.length === 1
+		&& isSimpleCompare(node.body, node.params[0])
+	)
 	// Matches `foo.findIndex(bar => {return bar === baz})`
 	// Matches `foo.findIndex(function (bar) {return bar === baz})`
-	[
-		`:matches([${path}.type="ArrowFunctionExpression"], [${path}.type="FunctionExpression"])`,
-		getFunctionSelector(path),
-		`[${path}.body.type="BlockStatement"]`,
-		`[${path}.body.body.length=1]`,
-		`[${path}.body.body.0.type="ReturnStatement"]`,
-		getBinaryExpressionSelector(`${path}.body.body.0.argument`),
-	].join(''),
-]);
+	|| (
+		(node.type === 'ArrowFunctionExpression' || node.type === 'FunctionExpression')
+		&& !node.async
+		&& !node.generator
+		&& node.params.length === 1
+		&& node.body.type === 'BlockStatement'
+		&& node.body.body.length === 1
+		&& node.body.body[0].type === 'ReturnStatement'
+		&& isSimpleCompare(node.body.body[0].argument, node.params[0])
+	);
 const isIdentifierNamed = ({type, name}, expectName) => type === 'Identifier' && name === expectName;
 
 function simpleArraySearchRule({method, replacement}) {
@@ -51,78 +49,80 @@ function simpleArraySearchRule({method, replacement}) {
 		[SUGGESTION]: `Replace \`.${method}()\` with \`.${replacement}()\`.`,
 	};
 
-	const selector = [
-		methodCallSelector({
-			method,
-			argumentsLength: 1,
-		}),
-		callbackFunctionSelector('arguments.0'),
-	].join('');
-
-	function createListeners(context) {
+	function listen(context) {
 		const {sourceCode} = context;
 		const {scopeManager} = sourceCode;
 
-		return {
-			[selector](node) {
-				const [callback] = node.arguments;
-				const binaryExpression = callback.body.type === 'BinaryExpression'
-					? callback.body
-					: callback.body.body[0].argument;
-				const [parameter] = callback.params;
-				const {left, right} = binaryExpression;
-				const {name} = parameter;
-
-				let searchValueNode;
-				let parameterInBinaryExpression;
-				if (isIdentifierNamed(left, name)) {
-					searchValueNode = right;
-					parameterInBinaryExpression = left;
-				} else if (isIdentifierNamed(right, name)) {
-					searchValueNode = left;
-					parameterInBinaryExpression = right;
-				} else {
-					return;
+		context.on('CallExpression', callExpression => {
+			if (
+				!isMethodCall(callExpression, {
+					method,
+					argumentsLength: 1,
+					optionalCall: false,
+					optionalMember: false,
+				})
+				|| !isSimpleCompareCallbackFunction(callExpression.arguments[0])
+			) {
+				return;
+			}
+
+			const [callback] = callExpression.arguments;
+			const binaryExpression = callback.body.type === 'BinaryExpression'
+				? callback.body
+				: callback.body.body[0].argument;
+			const [parameter] = callback.params;
+			const {left, right} = binaryExpression;
+			const {name} = parameter;
+
+			let searchValueNode;
+			let parameterInBinaryExpression;
+			if (isIdentifierNamed(left, name)) {
+				searchValueNode = right;
+				parameterInBinaryExpression = left;
+			} else if (isIdentifierNamed(right, name)) {
+				searchValueNode = left;
+				parameterInBinaryExpression = right;
+			} else {
+				return;
+			}
+
+			const callbackScope = scopeManager.acquire(callback);
+			if (
+				// `parameter` is used somewhere else
+				findVariable(callbackScope, parameter).references.some(({identifier}) => identifier !== parameterInBinaryExpression)
+				|| isFunctionSelfUsedInside(callback, callbackScope)
+			) {
+				return;
+			}
+
+			const methodNode = callExpression.callee.property;
+			const problem = {
+				node: methodNode,
+				messageId: ERROR,
+				suggest: [],
+			};
+
+			const fix = function * (fixer) {
+				let text = sourceCode.getText(searchValueNode);
+				if (isParenthesized(searchValueNode, sourceCode) && !isParenthesized(callback, sourceCode)) {
+					text = `(${text})`;
 				}
 
-				const callbackScope = scopeManager.acquire(callback);
-				if (
-					// `parameter` is used somewhere else
-					findVariable(callbackScope, parameter).references.some(({identifier}) => identifier !== parameterInBinaryExpression)
-					|| isFunctionSelfUsedInside(callback, callbackScope)
-				) {
-					return;
-				}
+				yield fixer.replaceText(methodNode, replacement);
+				yield fixer.replaceText(callback, text);
+			};
 
-				const method = node.callee.property;
-				const problem = {
-					node: method,
-					messageId: ERROR,
-					suggest: [],
-				};
-
-				const fix = function * (fixer) {
-					let text = sourceCode.getText(searchValueNode);
-					if (isParenthesized(searchValueNode, sourceCode) && !isParenthesized(callback, sourceCode)) {
-						text = `(${text})`;
-					}
-
-					yield fixer.replaceText(method, replacement);
-					yield fixer.replaceText(callback, text);
-				};
-
-				if (hasSideEffect(searchValueNode, sourceCode)) {
-					problem.suggest.push({messageId: SUGGESTION, fix});
-				} else {
-					problem.fix = fix;
-				}
+			if (hasSideEffect(searchValueNode, sourceCode)) {
+				problem.suggest.push({messageId: SUGGESTION, fix});
+			} else {
+				problem.fix = fix;
+			}
 
-				return problem;
-			},
-		};
+			return problem;
+		});
 	}
 
-	return {messages, createListeners};
+	return {messages, listen};
 }
 
 module.exports = simpleArraySearchRule;
diff --git a/rules/utils/index.js b/rules/utils/index.js
@@ -24,6 +24,7 @@ module.exports = {
 	getVariableIdentifiers: require('./get-variable-identifiers.js'),
 	isArrayPrototypeProperty,
 	isBooleanNode,
+	isFunctionSelfUsedInside: require('./is-function-self-used-inside.js'),
 	isLogicalExpression: require('./is-logical-expression.js'),
 	isNodeMatches,
 	isNodeMatchesNameOrPath,

diff --git a/rules/utils/rule.js b/rules/utils/rule.js
@@ -77,15 +77,36 @@ function reportProblems(create) {
 	}
 
 	const wrapped = context => {
-		const listeners = create(context);
+		const listeners = {};
+		const addListener = (selector, listener) => {
+			listeners[selector] ??= [];
+			listeners[selector].push(listener);
+		};
+
+		const contextProxy = new Proxy(context, {
+			get(target, property, receiver) {
+				if (property === 'on') {
+					return addListener;
+				}
+
+				return Reflect.get(target, property, receiver);
+			},
+		});
 
-		if (!listeners) {
-			return {};
+		for (const [selector, listener] of Object.entries(create(contextProxy) ?? {})) {
+			addListener(selector, listener);
 		}
 
 		return Object.fromEntries(
 			Object.entries(listeners)
-				.map(([selector, listener]) => [selector, reportListenerProblems(listener, context)]),
+				.map(([selector, listeners]) => [
+					selector,
+					(...listenerArguments) => {
+						for (const listener of listeners) {
+							reportListenerProblems(listener, context)(...listenerArguments);
+						}
+					},
+				]),
 		);
 	};