prefer-prototype-methods: Check Object.prototype methods from `gl…

…obalThis` (#2286)
sindresorhus · Feb 23, 2024 · 1792d33 · 1792d33
1 parent a3be554
commit 1792d33
Show file tree

Hide file tree

Showing 6 changed files with 408 additions and 56 deletions.
diff --git a/docs/rules/prefer-prototype-methods.md b/docs/rules/prefer-prototype-methods.md
@@ -23,6 +23,10 @@ const type = {}.toString.call(foo);
 Reflect.apply([].forEach, arrayLike, [callback]);
 ```
 
+```js
+const type = globalThis.toString.call(foo);
+```
+
 ## Pass
 
 ```js

diff --git a/rules/prefer-prototype-methods.js b/rules/prefer-prototype-methods.js
@@ -1,5 +1,5 @@
 'use strict';
-const {getPropertyName} = require('@eslint-community/eslint-utils');
+const {getPropertyName, ReferenceTracker} = require('@eslint-community/eslint-utils');
 const {fixSpaceAroundKeyword} = require('./fix/index.js');
 const {isMemberExpression, isMethodCall} = require('./ast/index.js');
 
@@ -8,72 +8,144 @@ const messages = {
 	'unknown-method': 'Prefer using method from `{{constructorName}}.prototype`.',
 };
 
-/** @param {import('eslint').Rule.RuleContext} context */
-function create(context) {
+const OBJECT_PROTOTYPE_METHODS = [
+	'hasOwnProperty',
+	'isPrototypeOf',
+	'propertyIsEnumerable',
+	'toLocaleString',
+	'toString',
+	'valueOf',
+];
+
+function getConstructorAndMethodName(methodNode, {sourceCode, globalReferences}) {
+	if (!methodNode) {
+		return;
+	}
+
+	const isGlobalReference = globalReferences.has(methodNode);
+	if (isGlobalReference) {
+		const path = globalReferences.get(methodNode);
+		return {
+			isGlobalReference: true,
+			constructorName: 'Object',
+			methodName: path.at(-1),
+		};
+	}
+
+	if (!isMemberExpression(methodNode, {optional: false})) {
+		return;
+	}
+
+	const objectNode = methodNode.object;
+
+	if (!(
+		(objectNode.type === 'ArrayExpression' && objectNode.elements.length === 0)
+		|| (objectNode.type === 'ObjectExpression' && objectNode.properties.length === 0)
+	)) {
+		return;
+	}
+
+	const constructorName = objectNode.type === 'ArrayExpression' ? 'Array' : 'Object';
+	const methodName = getPropertyName(methodNode, sourceCode.getScope(methodNode));
+
 	return {
-		CallExpression(callExpression) {
-			let methodNode;
-
-			if (
-				// `Reflect.apply([].foo, …)`
-				// `Reflect.apply({}.foo, …)`
-				isMethodCall(callExpression, {
-					object: 'Reflect',
-					method: 'apply',
-					minimumArguments: 1,
-					optionalCall: false,
-					optionalMember: false,
-				})
-			) {
-				methodNode = callExpression.arguments[0];
-			} else if (
-				// `[].foo.{apply,bind,call}(…)`
-				// `({}).foo.{apply,bind,call}(…)`
-				isMethodCall(callExpression, {
-					methods: ['apply', 'bind', 'call'],
-					optionalCall: false,
-					optionalMember: false,
-				})
-			) {
-				methodNode = callExpression.callee.object;
-			}
+		constructorName,
+		methodName,
+	};
+}
 
-			if (!methodNode || !isMemberExpression(methodNode, {optional: false})) {
-				return;
-			}
+function getProblem(callExpression, {sourceCode, globalReferences}) {
+	let methodNode;
+
+	if (
+		// `Reflect.apply([].foo, …)`
+		// `Reflect.apply({}.foo, …)`
+		isMethodCall(callExpression, {
+			object: 'Reflect',
+			method: 'apply',
+			minimumArguments: 1,
+			optionalCall: false,
+			optionalMember: false,
+		})
+	) {
+		methodNode = callExpression.arguments[0];
+	} else if (
+		// `[].foo.{apply,bind,call}(…)`
+		// `({}).foo.{apply,bind,call}(…)`
+		isMethodCall(callExpression, {
+			methods: ['apply', 'bind', 'call'],
+			optionalCall: false,
+			optionalMember: false,
+		})
+	) {
+		methodNode = callExpression.callee.object;
+	}
 
-			const objectNode = methodNode.object;
+	const {
+		isGlobalReference,
+		constructorName,
+		methodName,
+	} = getConstructorAndMethodName(methodNode, {sourceCode, globalReferences}) ?? {};
 
-			if (!(
-				(objectNode.type === 'ArrayExpression' && objectNode.elements.length === 0)
-				|| (objectNode.type === 'ObjectExpression' && objectNode.properties.length === 0)
-			)) {
+	if (!constructorName) {
+		return;
+	}
+
+	return {
+		node: methodNode,
+		messageId: methodName ? 'known-method' : 'unknown-method',
+		data: {constructorName, methodName},
+		* fix(fixer) {
+			if (isGlobalReference) {
+				yield fixer.replaceText(methodNode, `${constructorName}.prototype.${methodName}`);
 				return;
 			}
 
-			const constructorName = objectNode.type === 'ArrayExpression' ? 'Array' : 'Object';
-			const {sourceCode} = context;
-			const methodName = getPropertyName(methodNode, sourceCode.getScope(methodNode));
-
-			return {
-				node: methodNode,
-				messageId: methodName ? 'known-method' : 'unknown-method',
-				data: {constructorName, methodName},
-				* fix(fixer) {
-					yield fixer.replaceText(objectNode, `${constructorName}.prototype`);
-
-					if (
-						objectNode.type === 'ArrayExpression'
-						|| objectNode.type === 'ObjectExpression'
-					) {
-						yield * fixSpaceAroundKeyword(fixer, callExpression, sourceCode);
-					}
-				},
-			};
+			if (isMemberExpression(methodNode)) {
+				const objectNode = methodNode.object;
+
+				yield fixer.replaceText(objectNode, `${constructorName}.prototype`);
+
+				if (
+					objectNode.type === 'ArrayExpression'
+					|| objectNode.type === 'ObjectExpression'
+				) {
+					yield * fixSpaceAroundKeyword(fixer, callExpression, sourceCode);
+				}
+			}
 		},
 	};
 }
 
+/** @param {import('eslint').Rule.RuleContext} context */
+function create(context) {
+	const {sourceCode} = context;
+	const callExpressions = [];
+
+	context.on('CallExpression', callExpression => {
+		callExpressions.push(callExpression);
+	});
+
+	context.on('Program:exit', function * (program) {
+		const globalReferences = new WeakMap();
+
+		const tracker = new ReferenceTracker(sourceCode.getScope(program));
+
+		for (const {node, path} of tracker.iterateGlobalReferences(
+			Object.fromEntries(OBJECT_PROTOTYPE_METHODS.map(method => [method, {[ReferenceTracker.READ]: true}])),
+		)) {
+			globalReferences.set(node, path);
+		}
+
+		for (const callExpression of callExpressions) {
+			yield getProblem(callExpression, {
+				sourceCode,
+				globalReferences,
+			});
+		}
+	});
+}
+
 /** @type {import('eslint').Rule.RuleModule} */
 module.exports = {
 	create,

diff --git a/rules/utils/rule.js b/rules/utils/rule.js
@@ -43,6 +43,10 @@ function reportListenerProblems(problems, context) {
 	}
 
 	for (const problem of problems) {
+		if (!problem) {
+			continue;
+		}
+
 		if (problem.fix) {
 			problem.fix = wrapFixFunction(problem.fix);
 		}

diff --git a/test/prefer-prototype-methods.mjs b/test/prefer-prototype-methods.mjs
@@ -36,6 +36,14 @@ test.snapshot({
 		'const foo = [].push.notApply(bar, elements);',
 		'const push = [].push.notBind(foo)',
 		'[].forEach.notCall(foo, () => {})',
+		'/* globals foo: readonly */ foo.call(bar)',
+		'const toString = () => {}; toString.call(bar)',
+		'/* globals toString: off */ toString.call(bar)',
+		// Make sure the fix won't break code
+		'const _hasOwnProperty = globalThis.hasOwnProperty; _hasOwnProperty.call(bar)',
+		'const _globalThis = globalThis; globalThis[hasOwnProperty].call(bar)',
+		'const _ = globalThis, TO_STRING = "toString"; _[TO_STRING].call(bar)',
+		'const _ = [globalThis.toString]; _[0].call(bar)',
 	],
 	invalid: [
 		'const foo = [].push.apply(bar, elements);',
@@ -61,5 +69,17 @@ test.snapshot({
 		'[][Symbol.iterator].call(foo)',
 		'const foo = [].at.call(bar)',
 		'const foo = [].findLast.call(bar)',
+		'/* globals hasOwnProperty: readonly */ hasOwnProperty.call(bar)',
+		'/* globals toString: readonly */ toString.apply(bar, [])',
+		'/* globals toString: readonly */ Reflect.apply(toString, baz, [])',
+		'globalThis.toString.call(bar)',
+		'const _ = globalThis; _.hasOwnProperty.call(bar)',
+		'const _ = globalThis; _["hasOwnProperty"].call(bar)',
+		'const _ = globalThis; _["hasOwn" + "Property"].call(bar)',
+		'Reflect.apply(globalThis.toString, baz, [])',
+		'Reflect.apply(window.toString, baz, [])',
+		'Reflect.apply(global.toString, baz, [])',
+		'/* globals toString: readonly */ Reflect.apply(toString, baz, [])',
+		'Reflect.apply(globalThis["toString"], baz, [])',
 	],
 });