diff --git a/docs/rules/prefer-prototype-methods.md b/docs/rules/prefer-prototype-methods.md index 75e41e44bb..89f8f1af62 100644 --- 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 index e3bdb718a7..363622ee93 100644 --- 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 index 16b702d444..26eaf676ee 100644 --- 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 index 6dadca6953..078d4adb8d 100644 --- 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, [])', ], }); diff --git a/test/snapshots/prefer-prototype-methods.mjs.md b/test/snapshots/prefer-prototype-methods.mjs.md index 44b86b577e..625eb1aba6 100644 --- a/test/snapshots/prefer-prototype-methods.mjs.md +++ b/test/snapshots/prefer-prototype-methods.mjs.md @@ -486,3 +486,255 @@ Generated by [AVA](https://avajs.dev). > 1 | const foo = [].findLast.call(bar)␊ | ^^^^^^^^^^^ Prefer using \`Array.prototype.findLast\`.␊ ` + +## invalid(24): /* globals hasOwnProperty: readonly */ hasOwnProperty.call(bar) + +> Input + + `␊ + 1 | /* globals hasOwnProperty: readonly */ hasOwnProperty.call(bar)␊ + ` + +> Output + + `␊ + 1 | /* globals hasOwnProperty: readonly */ Object.prototype.hasOwnProperty.call(bar)␊ + ` + +> Error 1/1 + + `␊ + > 1 | /* globals hasOwnProperty: readonly */ hasOwnProperty.call(bar)␊ + | ^^^^^^^^^^^^^^ Prefer using \`Object.prototype.hasOwnProperty\`.␊ + ` + +## invalid(25): /* globals toString: readonly */ toString.apply(bar, []) + +> Input + + `␊ + 1 | /* globals toString: readonly */ toString.apply(bar, [])␊ + ` + +> Output + + `␊ + 1 | /* globals toString: readonly */ Object.prototype.toString.apply(bar, [])␊ + ` + +> Error 1/1 + + `␊ + > 1 | /* globals toString: readonly */ toString.apply(bar, [])␊ + | ^^^^^^^^ Prefer using \`Object.prototype.toString\`.␊ + ` + +## invalid(26): /* globals toString: readonly */ Reflect.apply(toString, baz, []) + +> Input + + `␊ + 1 | /* globals toString: readonly */ Reflect.apply(toString, baz, [])␊ + ` + +> Output + + `␊ + 1 | /* globals toString: readonly */ Reflect.apply(Object.prototype.toString, baz, [])␊ + ` + +> Error 1/1 + + `␊ + > 1 | /* globals toString: readonly */ Reflect.apply(toString, baz, [])␊ + | ^^^^^^^^ Prefer using \`Object.prototype.toString\`.␊ + ` + +## invalid(27): globalThis.toString.call(bar) + +> Input + + `␊ + 1 | globalThis.toString.call(bar)␊ + ` + +> Output + + `␊ + 1 | Object.prototype.toString.call(bar)␊ + ` + +> Error 1/1 + + `␊ + > 1 | globalThis.toString.call(bar)␊ + | ^^^^^^^^^^^^^^^^^^^ Prefer using \`Object.prototype.toString\`.␊ + ` + +## invalid(28): const _ = globalThis; _.hasOwnProperty.call(bar) + +> Input + + `␊ + 1 | const _ = globalThis; _.hasOwnProperty.call(bar)␊ + ` + +> Output + + `␊ + 1 | const _ = globalThis; Object.prototype.hasOwnProperty.call(bar)␊ + ` + +> Error 1/1 + + `␊ + > 1 | const _ = globalThis; _.hasOwnProperty.call(bar)␊ + | ^^^^^^^^^^^^^^^^ Prefer using \`Object.prototype.hasOwnProperty\`.␊ + ` + +## invalid(29): const _ = globalThis; _["hasOwnProperty"].call(bar) + +> Input + + `␊ + 1 | const _ = globalThis; _["hasOwnProperty"].call(bar)␊ + ` + +> Output + + `␊ + 1 | const _ = globalThis; Object.prototype.hasOwnProperty.call(bar)␊ + ` + +> Error 1/1 + + `␊ + > 1 | const _ = globalThis; _["hasOwnProperty"].call(bar)␊ + | ^^^^^^^^^^^^^^^^^^^ Prefer using \`Object.prototype.hasOwnProperty\`.␊ + ` + +## invalid(30): const _ = globalThis; _["hasOwn" + "Property"].call(bar) + +> Input + + `␊ + 1 | const _ = globalThis; _["hasOwn" + "Property"].call(bar)␊ + ` + +> Output + + `␊ + 1 | const _ = globalThis; Object.prototype.hasOwnProperty.call(bar)␊ + ` + +> Error 1/1 + + `␊ + > 1 | const _ = globalThis; _["hasOwn" + "Property"].call(bar)␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using \`Object.prototype.hasOwnProperty\`.␊ + ` + +## invalid(31): Reflect.apply(globalThis.toString, baz, []) + +> Input + + `␊ + 1 | Reflect.apply(globalThis.toString, baz, [])␊ + ` + +> Output + + `␊ + 1 | Reflect.apply(Object.prototype.toString, baz, [])␊ + ` + +> Error 1/1 + + `␊ + > 1 | Reflect.apply(globalThis.toString, baz, [])␊ + | ^^^^^^^^^^^^^^^^^^^ Prefer using \`Object.prototype.toString\`.␊ + ` + +## invalid(32): Reflect.apply(window.toString, baz, []) + +> Input + + `␊ + 1 | Reflect.apply(window.toString, baz, [])␊ + ` + +> Output + + `␊ + 1 | Reflect.apply(Object.prototype.toString, baz, [])␊ + ` + +> Error 1/1 + + `␊ + > 1 | Reflect.apply(window.toString, baz, [])␊ + | ^^^^^^^^^^^^^^^ Prefer using \`Object.prototype.toString\`.␊ + ` + +## invalid(33): Reflect.apply(global.toString, baz, []) + +> Input + + `␊ + 1 | Reflect.apply(global.toString, baz, [])␊ + ` + +> Output + + `␊ + 1 | Reflect.apply(Object.prototype.toString, baz, [])␊ + ` + +> Error 1/1 + + `␊ + > 1 | Reflect.apply(global.toString, baz, [])␊ + | ^^^^^^^^^^^^^^^ Prefer using \`Object.prototype.toString\`.␊ + ` + +## invalid(34): /* globals toString: readonly */ Reflect.apply(toString, baz, []) + +> Input + + `␊ + 1 | /* globals toString: readonly */ Reflect.apply(toString, baz, [])␊ + ` + +> Output + + `␊ + 1 | /* globals toString: readonly */ Reflect.apply(Object.prototype.toString, baz, [])␊ + ` + +> Error 1/1 + + `␊ + > 1 | /* globals toString: readonly */ Reflect.apply(toString, baz, [])␊ + | ^^^^^^^^ Prefer using \`Object.prototype.toString\`.␊ + ` + +## invalid(35): Reflect.apply(globalThis["toString"], baz, []) + +> Input + + `␊ + 1 | Reflect.apply(globalThis["toString"], baz, [])␊ + ` + +> Output + + `␊ + 1 | Reflect.apply(Object.prototype.toString, baz, [])␊ + ` + +> Error 1/1 + + `␊ + > 1 | Reflect.apply(globalThis["toString"], baz, [])␊ + | ^^^^^^^^^^^^^^^^^^^^^^ Prefer using \`Object.prototype.toString\`.␊ + ` diff --git a/test/snapshots/prefer-prototype-methods.mjs.snap b/test/snapshots/prefer-prototype-methods.mjs.snap index e78b97dc7f..3f1ec46922 100644 Binary files a/test/snapshots/prefer-prototype-methods.mjs.snap and b/test/snapshots/prefer-prototype-methods.mjs.snap differ