prefer-prototype-methods: Only check methods from [] and {}

docs/rules/prefer-prototype-methods.md

@@ -1,8 +1,8 @@
-# Prefer borrowing methods from the prototype instead of methods from an instance.
+# Prefer borrowing methods from the prototype instead of methods from an instance
 
-When “borrowing” a method from a different object (especially generic methods from `Array`), it‘s clearer to get it from the prototype than from an instance.
+When “borrowing” a method from `Array` or `Object`, it‘s clearer to get it from the prototype than from an instance.
 
-This rule is fixable when using method from `[]` or `{}`.
+This rule is fixable.
 
 ## Fail
 
@@ -14,10 +14,6 @@ const array = [].slice.apply(bar);
 const hasProperty = {}.hasOwnProperty.call(foo, 'property');
 ```
 
-```js
-foo.bar.call(baz);
-```
-
 ```js
 Reflect.apply([].forEach, arrayLike, [callback]);
 ```
@@ -32,14 +28,6 @@ const array = Array.prototype.slice.apply(bar);
 const hasProperty = Object.prototype.hasOwnProperty.call(foo, 'property');
 ```
 
-```js
-Foo.prototype.bar.call(baz);
-```
-
-```js
-foo.constructor.prototype.bar.call(baz);
-```
-
 ```js
 Reflect.apply(Array.prototype.forEach, arrayLike, [callback]);
 ```

rules/prefer-prototype-methods.js

@@ -1,155 +1,54 @@
 'use strict';
-const {findVariable} = require('eslint-utils');
 const getDocumentationUrl = require('./utils/get-documentation-url.js');
-const {methodCallSelector} = require('./selectors/index.js');
+const {
+	methodCallSelector,
+	emptyObjectSelector,
+	emptyArraySelector,
+	matches
+} = require('./selectors/index.js');
 const getPropertyName = require('./utils/get-property-name.js');
 
 const messages = {
-	'known-constructor-known-method': 'Prefer using `{{constructorName}}.prototype.{{methodName}}`.',
-	'known-constructor-unknown-method': 'Prefer using method from `{{constructorName}}.prototype`.',
-	'unknown-constructor-known-method': 'Prefer using `{{methodName}}` method from the constructor prototype.',
-	'unknown-constructor-unknown-method': 'Prefer using method from the constructor prototype.'
+	'known-method': 'Prefer using `{{constructorName}}.prototype.{{methodName}}`.',
+	'unknown-method': 'Prefer using method from `{{constructorName}}.prototype`.'
 };
 
-const functionMethodsSelector = [
-	methodCallSelector(['apply', 'bind', 'call']),
-	' > ',
-	'.callee',
-	' > ',
-	'.object'
+const emptyObjectOrArrayMethodSelector = [
+	'MemberExpression',
+	matches([emptyObjectSelector('object'), emptyArraySelector('object')])
 ].join('');
-
-const reflectApplySelector = methodCallSelector({
-	object: 'Reflect',
-	name: 'apply',
-	min: 1
-});
-
-function getConstructorName(node) {
-	switch (node.type) {
-		case 'ArrayExpression':
-			return 'Array';
-		case 'ObjectExpression':
-			return 'Object';
-		// No default
-	}
-}
-
-function isSafeToFix(node) {
-	switch (node.type) {
-		case 'ArrayExpression':
-			return node.elements.length === 0;
-		case 'ObjectExpression':
-			return node.properties.length === 0;
-		// No default
-	}
-}
-
-function isObjectOwnProperty(property) {
-	// eslint-disable-next-line no-useless-call
-	return Object.prototype.hasOwnProperty.call(Object.prototype, property);
-}
+const selector = matches([
+	// `[].foo.{apply,bind,call}(…)`
+	// `({}).foo.{apply,bind,call}(…)`
+	[
+		methodCallSelector(['apply', 'bind', 'call']),
+		' > ',
+		'.callee',
+		' > ',
+		`${emptyObjectOrArrayMethodSelector}.object`
+	].join(''),
+	// `Reflect.apply([].foo, …)`
+	// `Reflect.apply({}.foo, …)`
+	[
+		methodCallSelector({object: 'Reflect', name: 'apply', min: 1}),
+		' > ',
+		`${emptyObjectOrArrayMethodSelector}.arguments:first-child`
+	].join('')
+]);
 
 /** @param {import('eslint').Rule.RuleContext} context */
 function create(context) {
-	const methods = new Set();
-	const nonArrowFunctionStack = [];
-	const thisExpressions = new Map();
-
-	function check(method, scope) {
-		const {type, object} = method;
-		if (
-			type !== 'MemberExpression' ||
-			// Most likely it's a static method of a class
-			(object.type === 'Identifier' && /^[A-Z]/.test(object.name)) ||
-			(
-				object.type === 'MemberExpression' &&
-				!object.computed &&
-				!object.optional &&
-				object.property.type === 'Identifier' &&
-				object.property.name === 'prototype'
-			)
-		) {
-			return;
-		}
-
-		const methodName = getPropertyName(method, scope);
-		if (!isObjectOwnProperty(methodName)) {
-			if (object.type === 'ObjectExpression' && object.properties.length > 0) {
-				return;
-			}
-
-			if (object.type === 'ThisExpression') {
-				const functionNode = thisExpressions.get(object);
-				if (
-					functionNode &&
-					functionNode.parent.type === 'Property' &&
-					functionNode.parent.value === functionNode &&
-					functionNode.parent.parent.type === 'ObjectExpression'
-				) {
-					return;
-				}
-			}
-
-			if (object.type === 'Identifier') {
-				const variable = findVariable(scope, object);
-				if (variable && variable.defs.length === 1) {
-					const [definition] = variable.defs;
-					if (
-						definition.type === 'Variable' &&
-						definition.kind === 'const' &&
-						definition.node.id === definition.name &&
-						definition.node.type === 'VariableDeclarator' &&
-						definition.node.init &&
-						definition.node.init.type === 'ObjectExpression'
-					) {
-						return;
-					}
-				}
-			}
-		}
-
-		const constructorName = getConstructorName(object);
-		const messageId = [
-			constructorName ? 'known' : 'unknown',
-			'constructor',
-			methodName ? 'known' : 'unknown',
-			'method'
-		].join('-');
-
-		const problem = {
-			node: method,
-			messageId,
-			data: {constructorName, methodName: String(methodName)}
-		};
-
-		if (constructorName && isSafeToFix(object)) {
-			problem.fix = fixer => fixer.replaceText(object, `${constructorName}.prototype`);
-		}
-
-		context.report(problem);
-	}
-
 	return {
-		'FunctionExpression,FunctionDeclaration'(node) {
-			nonArrowFunctionStack.push(node);
-		},
-		'FunctionExpression,FunctionDeclaration:exit'() {
-			nonArrowFunctionStack.pop();
-		},
-		'ThisExpression'(node) {
-			thisExpressions.set(node, nonArrowFunctionStack[nonArrowFunctionStack.length - 1]);
-		},
-		[reflectApplySelector](node) {
-			methods.add({method: node.arguments[0], scope: context.getScope()});
-		},
-		[functionMethodsSelector](node) {
-			methods.add({method: node, scope: context.getScope()});
-		},
-		'Program:exit'() {
-			for (const {method, scope} of methods) {
-				check(method, scope);
-			}
+		[selector](node) {
+			const constructorName = node.object.type === 'ArrayExpression' ? 'Array' : 'Object';
+			const methodName = getPropertyName(node, context.getScope());
+
+			context.report({
+				node,
+				messageId: methodName ? 'known-method' : 'unknown-method',
+				data: {constructorName, methodName: String(methodName)},
+				fix: fixer => fixer.replaceText(node.object, `${constructorName}.prototype`)
+			});
 		}
 	};
 }

rules/selectors/empty-array-selector.js

@@ -1,7 +1,7 @@
 'use strict';
 
 function emptyArraySelector(path) {
-	const prefix = `${path}.`;
+	const prefix = path ? `${path}.` : '';
 	return [
 		`[${prefix}type="ArrayExpression"]`,
 		`[${prefix}elements.length=0]`

rules/selectors/empty-object-selector.js

@@ -1,7 +1,7 @@
 'use strict';
 
 function emptyObjectSelector(path) {
-	const prefix = `${path}.`;
+	const prefix = path ? `${path}.` : '';
 	return [
 		`[${prefix}type="ObjectExpression"]`,
 		`[${prefix}properties.length=0]`

rules/selectors/index.js

@@ -8,6 +8,7 @@ module.exports = {
 	arrayPrototypeMethodSelector: require('./prototype-method-selector.js').arrayPrototypeMethodSelector,
 	objectPrototypeMethodSelector: require('./prototype-method-selector.js').objectPrototypeMethodSelector,
 	emptyArraySelector: require('./empty-array-selector.js'),
+	emptyObjectSelector: require('./empty-object-selector.js'),
 	memberExpressionSelector: require('./member-expression-selector.js'),
 	methodCallSelector: require('./method-call-selector.js'),
 	notDomNodeSelector: require('./not-dom-node.js').notDomNodeSelector,