From 1792d3307f59d03b1fba2a935855ecede60e5866 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Fri, 23 Feb 2024 11:08:42 +0800 Subject: [PATCH] `prefer-prototype-methods`: Check `Object.prototype` methods from `globalThis` (#2286) --- docs/rules/prefer-prototype-methods.md | 4 + rules/prefer-prototype-methods.js | 184 +++++++++---- rules/utils/rule.js | 4 + test/prefer-prototype-methods.mjs | 20 ++ .../snapshots/prefer-prototype-methods.mjs.md | 252 ++++++++++++++++++ .../prefer-prototype-methods.mjs.snap | Bin 1406 -> 1893 bytes 6 files changed, 408 insertions(+), 56 deletions(-) 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 e78b97dc7f17d79f3d90033806ad0ac3c3ff24c9..3f1ec469224249b8160a63a19a08dae232b70ad5 100644 GIT binary patch literal 1893 zcmV-r2b%anRzVF00000000B+nq6-jMHGfp5JJiw|6t?_u#?D%?Kqz}OygMc;Gg zeP(vfoS9$i70sxAYW;Nnh}){I@z0B@zOQI%EuG6Ou&SY3Hme&3E3xu+w$-*8S*6v| zI_ZjHPB5m$zZ+-oCmoXd?emSgwmf}>nh zDqQ2MdaG@_l50S636Z>Lno1|zG7a0XJ1riqU*Blky#QZ;0Ikubm^m}X*ge+&+d28M zO_SHT$=a5x@3A{rns>5pO6rPf8Yat4<~$_h9+G?*iKVGko~}b)-$^hF#E8ZwT)Z0sp@-DzvsYs#t@ru zCCu_GKx+=s+NgZOt9IY`#LCe2uYk0P*e*y@?0ndKVRjM#%N~H~Fo1?)Z5-&ErqSZ2 z-SM-UV5ye-cPR|XbuiXDM*_}qq7Z1)m}5aD zm5`!6Fc5 z7oOw!>Dchz!9fsz*y2~9TwKXyuJ*SiqVCbP!_uVCD8p0t;7&1 z*@p!>e)4Zw-87mZ_a(-!dB)HC#=A--mNL1b-8lZ!y3*vS#rR-80K>ip>>NA}2CfPs z-5wy)U|^qv&Ey%}OokXDveF`c%p}dYu zn2K^&fC}Ha*_r`iQDo(W{n z5ouwbUju3jm|C#xlxaij9{}-vWE&Hv8c`!BaJucW2=*7!edq1W@GMZRgjmS5A&dJ! zyp6NK<-}!N;wXZhhIFv?kx2$czgE&H5ba@50J&2~G2^3f+5`2rUbR(2chX8{4oz;i zO&w_hcBx%wvLFWK29UfUlZa7KiVDURtoP+dJ3k#Q2FP9T-^w{135S{-)Fw>r&=p`|n1vH}bLwo$a`-!`Zg z9H~SbqWKz#-$kn}L8{`KEAj*zOjp8V#pf|6;gPbA|I2FJs+wxcnN=w{qjfOwC6a_fC1v)77-Xfew&Ns;|LR<@jc@?P5Q_ec%HTVcI3yR${!9U=2$~5IW-B)3aY8=WJ#a3aRK2#QSWBV_B;d z_gJVqz$+X?OW-$YihFXDTZp{VE|?r+dzw*EG>gGI@C9aarDo__hmB1#!9_em<~A7h zRoBiT!(iO%AkbF4K-8BEIN5)|zVes1uM9g#$*D?u&PwuqL$A6h!r9>+*5FwD!(rb; zvKZCHp!(Q+*aZ=8QX`0u(P&6G5&i%Z;R!Mk;th*90}?!7ulu}bD7<0mGZ%XsoZoDa?>w>FKm!P6qRSL$#2`5ViD_ zieO~tfy}roL+F5^s1AUd>XMQt#fUZMAP$#&hkGwFc3G(}qeZqW+@Zpz%U)a3bukY3 z=8h{5x5fSjmeUXL3OL3fWUD7GO7bS^KmE+li-DF?P%pJDW4a2QUz9VINxxXEe*&yO zM_`qXBM#Y9sgMd&(Qs32oTX@l@d{~B{TGY*8NmEI5wmzOF#kRX+SpOdJcLbR|25JKv^eS=dFV2>y?X`7}k?YfPQZR*;>1e#RZ z?VOw!563p=OGlOkkTx`UhlB(#5&Z^8d;`81a$>uA?B~vx)VzYcnKo&^d+z_Ueb1f! zhi$oj=d19yb0!?&vTxQL?~J-m*JzlVq~m*`Ahz$5ZE}244+decPJ_T58W!a>!d%v8 zo(Rn?C-i6#_Iwe(Jnwd?pbxLVS$Vq>{jR)Kxq79sL`Z|Y&<8Ftms-qacfDXBV&pzR z?jXn;oYP@F;J)z1Fkr=e`*0vG2KW{X5G?J?Su-Joydbl`)9H^Kab`142BG6UA&;># zAJ^lOoITEcP8uIJ5+xrbN}72kq3d*5y+d6$sssUo+$HfgZW%TV!}ySrAp z<|ik^*uMv8j>Il0Q=WW2d}(zG0lSHS)x3Zn4G*7rN8Aq>7sIrw8mR{Hzg00M_rP2~ z!j)*TF-Z>r{hp4bdB%d;exDuejh`#HH$A@6q@GVWkizQi#Ohk!YEskPaL*g`8K;)Z zwA~(o^l##B=NJJqd$h|u$&!1KAp^E{(y*w%B74XQAIEivCRH={?n?ejg_ z=}pVDeZNKwlWgyh(b&w}T?NU%7bmZ#UUB3P+M|fw>rHAC~7}S>FPrwcAk)Bj6s?)P))D}vuo zg5OAkk4f94${BUeV(znCw;wC{j87as=`*8wAazQ9O}iTxWaCV#=j;Zx?8k>5%+D$N@8nWidSXkzF0BsYaW!p{_8%F;M zz|W9nOj>F|k6gm;_QFN>e8HI6Z=08QfwD?+p<+W9F9CdjyTEbsHm-CP*^>q%d-P$J zqhgj-whDxO85IEcud8UKDx%{7d*F41<9ktAhB@L)47i6J0*-Z0owb3Ol=}dAU4<+> z03D-degY$mVl(=|9^bEjv>M8>;ycz@MYtmZVg9&sDgC z&6cb1v6uR2D|}SL$)BXl!VY(W=&Gu!9UXv)?-n>J;$z;v1S1^G-3J9ie7hszByN6;XRP+OYK1*Lcs5m6^=0VnQ zWV^#ybB1gxVmH#*kxv$=xM+#;p*O(9-oZatm_=Zb?*PcM#-|Dh5eDu8