From 09881dc8c3cce33f6b8501e6882601caa01f7f84 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Fri, 11 Jun 2021 12:52:10 +0800 Subject: [PATCH 1/5] `prefer-prototype-methods`: Only check methods from `[]` and `{}` --- docs/rules/prefer-prototype-methods.md | 18 +- rules/prefer-prototype-methods.js | 169 +++---------- rules/selectors/empty-array-selector.js | 2 +- rules/selectors/empty-object-selector.js | 2 +- rules/selectors/index.js | 1 + test/prefer-prototype-methods.mjs | 110 ++------- .../snapshots/prefer-prototype-methods.mjs.md | 222 ++---------------- .../prefer-prototype-methods.mjs.snap | Bin 1809 -> 1004 bytes 8 files changed, 82 insertions(+), 442 deletions(-) diff --git a/docs/rules/prefer-prototype-methods.md b/docs/rules/prefer-prototype-methods.md index 097647f04d..26e91ae492 100644 --- a/docs/rules/prefer-prototype-methods.md +++ b/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 a `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]); ``` diff --git a/rules/prefer-prototype-methods.js b/rules/prefer-prototype-methods.js index 1e98493c25..a2e9d96e59 100644 --- a/rules/prefer-prototype-methods.js +++ b/rules/prefer-prototype-methods.js @@ -1,155 +1,58 @@ '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 emptyObjectOrArraySelector = matches([emptyObjectSelector(), emptyArraySelector()]); const functionMethodsSelector = [ methodCallSelector(['apply', 'bind', 'call']), ' > ', '.callee', ' > ', - '.object' + 'MemberExpression.object', + ' > ', + `${emptyObjectOrArraySelector}.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 - } -} +const reflectApplySelector = [ + methodCallSelector({object: 'Reflect', name: 'apply', min: 1}), + ' > ', + 'MemberExpression.arguments:first-child', + ' > ', + `${emptyObjectOrArraySelector}.object` +].join(''); -function isObjectOwnProperty(property) { - // eslint-disable-next-line no-useless-call - return Object.prototype.hasOwnProperty.call(Object.prototype, property); -} +const selector = matches([ + functionMethodsSelector, + reflectApplySelector +]); /** @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.type === 'ArrayExpression' ? 'Array' : 'Object'; + const methodName = getPropertyName(node.parent, context.getScope()); + const problem = { + node: node.parent, + messageId: methodName ? 'known-method' : 'unknown-method', + data: { + constructorName, methodName: String(methodName) + }, + fix: fixer => fixer.replaceText(node, `${constructorName}.prototype`) + }; + + context.report(problem); } }; } diff --git a/rules/selectors/empty-array-selector.js b/rules/selectors/empty-array-selector.js index a0ca810155..8f48fc376c 100644 --- a/rules/selectors/empty-array-selector.js +++ b/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]` diff --git a/rules/selectors/empty-object-selector.js b/rules/selectors/empty-object-selector.js index f4cd94e046..d231abdc1b 100644 --- a/rules/selectors/empty-object-selector.js +++ b/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]` diff --git a/rules/selectors/index.js b/rules/selectors/index.js index 1dbde3289d..294f39bac7 100644 --- a/rules/selectors/index.js +++ b/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, diff --git a/test/prefer-prototype-methods.mjs b/test/prefer-prototype-methods.mjs index 582e1c4acb..50494a3653 100644 --- a/test/prefer-prototype-methods.mjs +++ b/test/prefer-prototype-methods.mjs @@ -1,4 +1,3 @@ -import outdent from 'outdent'; import {getTester} from './utils/test.mjs'; const {test} = getTester(import.meta); @@ -10,6 +9,9 @@ test.snapshot({ 'const foo = Object.prototype.toString.call(bar);', 'const foo = Object.prototype.hasOwnProperty.call(bar, "property");', 'const foo = Object.prototype.propertyIsEnumerable.call(bar, "property");', + 'const foo = "".charCodeAt.call(bar, 0);', + 'const foo = [notEmpty].push.apply(bar, elements);', + 'const foo = {notEmpty}.toString.call(bar);', 'Array.prototype.forEach.call(foo, () => {})', 'const push = Array.prototype.push.bind(foo)', 'const push = [].push', @@ -24,8 +26,7 @@ test.snapshot({ 'NotReflect.apply([].slice, foo, [])', 'Reflect.notApply([].slice, foo, [])', 'Reflect.apply([]?.slice, foo, [])', - // This better use `Foo.prototype.bar.call(baz)`, not handled - 'foo.constructor.prototype.bar.call(baz)' + 'Reflect.apply(foo, [].bar, [].bar)' ], invalid: [ 'const foo = [].push.apply(bar, elements);', @@ -35,98 +36,13 @@ test.snapshot({ 'const foo = {}.propertyIsEnumerable.call(bar, "property");', '[].forEach.call(foo, () => {})', 'const push = [].push.bind(foo)', - 'const foo = bar.method.call(foo)', - 'const foo = bar[method].call(foo)', - 'const method = "realMethodName";const foo = bar[method].call(foo)', 'const foo = [][method].call(foo)', 'const method = "realMethodName";const foo = [][method].call(foo)', - 'const foo = [1].push.apply(bar, elements);', 'const array = Reflect.apply([].slice, foo, [])', - 'Reflect.apply(foo.bar, baz, [])', - // False positive - 'Array["prototype"].slice.call();', - 'Array?.prototype.slice.call();', - 'window.Math.max.apply(null, numbers)' - ] -}); - -// Object method -test.snapshot({ - valid: [ - outdent` - const foo = { - a() { - this.method = this.method.bind(this); - }, - b: function() { - this.method = this.method.bind(this); - }, - c() { - const foo = () => this.method.bind(this); - }, - d: { - d1() { - this.method.call(this); - } - } - } - `, - outdent` - const foo = {}; - const bar = foo.method.call(foo, 'property'); - `, - '({method() {}}).method.call(foo)' - ], - invalid: [ - outdent` - const foo = { - a: () => { - this.method = this.method.bind(this); - } - } - `, - outdent` - const foo = { - [function() {this.method.bind(this);}]: 1 - } - `, - outdent` - const {foo} = {foo: {}}; - const bar = foo.method.call(foo, 'property'); - `, - outdent` - const [foo] = [{}]; - const bar = foo.method.call(foo, 'property'); - `, - outdent` - const foo = { - a() { - this.propertyIsEnumerable.apply(this, []); - } - } - `, - outdent` - const foo = { - a() { - function fn() { - // this is not the object foo - this.method.call(this); - } - return fn; - } - } - `, - 'this.method = this.method.bind(this)', - outdent` - const foo = {}; - const bar = foo.hasOwnProperty.call(foo, 'property'); - `, - 'for (const foo of []) foo.bar.call(foo)', - outdent` - let foo = {}; - const bar = foo.method.call(foo, 'property'); - `, - '({method() {}}).propertyIsEnumerable.call(foo)' + 'Reflect.apply([].bar, baz, [])', + 'const foo = ({}).toString.call(bar);', + 'const foo = ({}.toString).call(bar);', + 'const foo = ({}.toString.call)(bar);' ] }); @@ -137,12 +53,14 @@ test.babel({ valid: [], invalid: [ { - code: 'Reflect.apply(foo[Symbol()], baz, [])', - errors: [{message: 'Prefer using method from the constructor prototype.'}] + code: 'Reflect.apply({}[Symbol()], baz, [])', + output: 'Reflect.apply(Object.prototype[Symbol()], baz, [])', + errors: [{message: 'Prefer using method from `Object.prototype`.'}] }, { - code: 'Reflect.apply(foo[Symbol("symbol description")], baz, [])', - errors: [{message: 'Prefer using method from the constructor prototype.'}] + code: 'Reflect.apply({}[Symbol("symbol description")], baz, [])', + output: 'Reflect.apply(Object.prototype[Symbol("symbol description")], baz, [])', + errors: [{message: 'Prefer using method from `Object.prototype`.'}] }, { code: 'Reflect.apply([][Symbol()], baz, [])', diff --git a/test/snapshots/prefer-prototype-methods.mjs.md b/test/snapshots/prefer-prototype-methods.mjs.md index ce3c046f11..37be2cbe6f 100644 --- a/test/snapshots/prefer-prototype-methods.mjs.md +++ b/test/snapshots/prefer-prototype-methods.mjs.md @@ -117,36 +117,6 @@ Generated by [AVA](https://avajs.dev). ` ## Invalid #8 - 1 | const foo = bar.method.call(foo) - -> Error 1/1 - - `␊ - > 1 | const foo = bar.method.call(foo)␊ - | ^^^^^^^^^^ Prefer using \`method\` method from the constructor prototype.␊ - ` - -## Invalid #9 - 1 | const foo = bar[method].call(foo) - -> Error 1/1 - - `␊ - > 1 | const foo = bar[method].call(foo)␊ - | ^^^^^^^^^^^ Prefer using method from the constructor prototype.␊ - ` - -## Invalid #10 - 1 | const method = "realMethodName";const foo = bar[method].call(foo) - -> Error 1/1 - - `␊ - > 1 | const method = "realMethodName";const foo = bar[method].call(foo)␊ - | ^^^^^^^^^^^ Prefer using \`realMethodName\` method from the constructor prototype.␊ - ` - -## Invalid #11 1 | const foo = [][method].call(foo) > Output @@ -162,7 +132,7 @@ Generated by [AVA](https://avajs.dev). | ^^^^^^^^^^ Prefer using method from \`Array.prototype\`.␊ ` -## Invalid #12 +## Invalid #9 1 | const method = "realMethodName";const foo = [][method].call(foo) > Output @@ -178,17 +148,7 @@ Generated by [AVA](https://avajs.dev). | ^^^^^^^^^^ Prefer using \`Array.prototype.realMethodName\`.␊ ` -## Invalid #13 - 1 | const foo = [1].push.apply(bar, elements); - -> Error 1/1 - - `␊ - > 1 | const foo = [1].push.apply(bar, elements);␊ - | ^^^^^^^^ Prefer using \`Array.prototype.push\`.␊ - ` - -## Invalid #14 +## Invalid #10 1 | const array = Reflect.apply([].slice, foo, []) > Output @@ -204,196 +164,66 @@ Generated by [AVA](https://avajs.dev). | ^^^^^^^^ Prefer using \`Array.prototype.slice\`.␊ ` -## Invalid #15 - 1 | Reflect.apply(foo.bar, baz, []) - -> Error 1/1 - - `␊ - > 1 | Reflect.apply(foo.bar, baz, [])␊ - | ^^^^^^^ Prefer using \`bar\` method from the constructor prototype.␊ - ` - -## Invalid #16 - 1 | Array["prototype"].slice.call(); - -> Error 1/1 - - `␊ - > 1 | Array["prototype"].slice.call();␊ - | ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using \`slice\` method from the constructor prototype.␊ - ` - -## Invalid #17 - 1 | Array?.prototype.slice.call(); - -> Error 1/1 - - `␊ - > 1 | Array?.prototype.slice.call();␊ - | ^^^^^^^^^^^^^^^^^^^^^^ Prefer using \`slice\` method from the constructor prototype.␊ - ` - -## Invalid #18 - 1 | window.Math.max.apply(null, numbers) - -> Error 1/1 - - `␊ - > 1 | window.Math.max.apply(null, numbers)␊ - | ^^^^^^^^^^^^^^^ Prefer using \`max\` method from the constructor prototype.␊ - ` - -## Invalid #1 - 1 | const foo = { - 2 | a: () => { - 3 | this.method = this.method.bind(this); - 4 | } - 5 | } - -> Error 1/1 - - `␊ - 1 | const foo = {␊ - 2 | a: () => {␊ - > 3 | this.method = this.method.bind(this);␊ - | ^^^^^^^^^^^ Prefer using \`method\` method from the constructor prototype.␊ - 4 | }␊ - 5 | }␊ - ` - -## Invalid #2 - 1 | const foo = { - 2 | [function() {this.method.bind(this);}]: 1 - 3 | } - -> Error 1/1 - - `␊ - 1 | const foo = {␊ - > 2 | [function() {this.method.bind(this);}]: 1␊ - | ^^^^^^^^^^^ Prefer using \`method\` method from the constructor prototype.␊ - 3 | }␊ - ` - -## Invalid #3 - 1 | const {foo} = {foo: {}}; - 2 | const bar = foo.method.call(foo, 'property'); - -> Error 1/1 - - `␊ - 1 | const {foo} = {foo: {}};␊ - > 2 | const bar = foo.method.call(foo, 'property');␊ - | ^^^^^^^^^^ Prefer using \`method\` method from the constructor prototype.␊ - ` - -## Invalid #4 - 1 | const [foo] = [{}]; - 2 | const bar = foo.method.call(foo, 'property'); +## Invalid #11 + 1 | Reflect.apply([].bar, baz, []) -> Error 1/1 +> Output `␊ - 1 | const [foo] = [{}];␊ - > 2 | const bar = foo.method.call(foo, 'property');␊ - | ^^^^^^^^^^ Prefer using \`method\` method from the constructor prototype.␊ + 1 | Reflect.apply(Array.prototype.bar, baz, [])␊ ` -## Invalid #5 - 1 | const foo = { - 2 | a() { - 3 | this.propertyIsEnumerable.apply(this, []); - 4 | } - 5 | } - > Error 1/1 `␊ - 1 | const foo = {␊ - 2 | a() {␊ - > 3 | this.propertyIsEnumerable.apply(this, []);␊ - | ^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using \`propertyIsEnumerable\` method from the constructor prototype.␊ - 4 | }␊ - 5 | }␊ + > 1 | Reflect.apply([].bar, baz, [])␊ + | ^^^^^^ Prefer using \`Array.prototype.bar\`.␊ ` -## Invalid #6 - 1 | const foo = { - 2 | a() { - 3 | function fn() { - 4 | // this is not the object foo - 5 | this.method.call(this); - 6 | } - 7 | return fn; - 8 | } - 9 | } +## Invalid #12 + 1 | const foo = ({}).toString.call(bar); -> Error 1/1 +> Output `␊ - 1 | const foo = {␊ - 2 | a() {␊ - 3 | function fn() {␊ - 4 | // this is not the object foo␊ - > 5 | this.method.call(this);␊ - | ^^^^^^^^^^^ Prefer using \`method\` method from the constructor prototype.␊ - 6 | }␊ - 7 | return fn;␊ - 8 | }␊ - 9 | }␊ + 1 | const foo = (Object.prototype).toString.call(bar);␊ ` -## Invalid #7 - 1 | this.method = this.method.bind(this) - > Error 1/1 `␊ - > 1 | this.method = this.method.bind(this)␊ - | ^^^^^^^^^^^ Prefer using \`method\` method from the constructor prototype.␊ + > 1 | const foo = ({}).toString.call(bar);␊ + | ^^^^^^^^^^^^^ Prefer using \`Object.prototype.toString\`.␊ ` -## Invalid #8 - 1 | const foo = {}; - 2 | const bar = foo.hasOwnProperty.call(foo, 'property'); +## Invalid #13 + 1 | const foo = ({}.toString).call(bar); -> Error 1/1 +> Output `␊ - 1 | const foo = {};␊ - > 2 | const bar = foo.hasOwnProperty.call(foo, 'property');␊ - | ^^^^^^^^^^^^^^^^^^ Prefer using \`hasOwnProperty\` method from the constructor prototype.␊ + 1 | const foo = (Object.prototype.toString).call(bar);␊ ` -## Invalid #9 - 1 | for (const foo of []) foo.bar.call(foo) - > Error 1/1 `␊ - > 1 | for (const foo of []) foo.bar.call(foo)␊ - | ^^^^^^^ Prefer using \`bar\` method from the constructor prototype.␊ + > 1 | const foo = ({}.toString).call(bar);␊ + | ^^^^^^^^^^^ Prefer using \`Object.prototype.toString\`.␊ ` -## Invalid #10 - 1 | let foo = {}; - 2 | const bar = foo.method.call(foo, 'property'); +## Invalid #14 + 1 | const foo = ({}.toString.call)(bar); -> Error 1/1 +> Output `␊ - 1 | let foo = {};␊ - > 2 | const bar = foo.method.call(foo, 'property');␊ - | ^^^^^^^^^^ Prefer using \`method\` method from the constructor prototype.␊ + 1 | const foo = (Object.prototype.toString.call)(bar);␊ ` -## Invalid #11 - 1 | ({method() {}}).propertyIsEnumerable.call(foo) - > Error 1/1 `␊ - > 1 | ({method() {}}).propertyIsEnumerable.call(foo)␊ - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using \`Object.prototype.propertyIsEnumerable\`.␊ + > 1 | const foo = ({}.toString.call)(bar);␊ + | ^^^^^^^^^^^ Prefer using \`Object.prototype.toString\`.␊ ` diff --git a/test/snapshots/prefer-prototype-methods.mjs.snap b/test/snapshots/prefer-prototype-methods.mjs.snap index 5615c4ef70f1a7d548cf19a93a67180e97e6f47b..de1f4afd7f3cbf74530afc52d0be743a228b18ca 100644 GIT binary patch literal 1004 zcmV`#(<}v$ITKQ9mfY1jGT#Q6aB2wYrvMg&XZYcx49@Sk#mm zf`0&UUQc7z-cvybicO{KM9ez5n82czED*c~i1YI};(WhP;+w%eJN@;O6TM7e(Z5hU znU#T|^yr$|rEhv;{7&!Qb>~m|6(+FgeJGA)V_;aSzc5|Y=rDsC-|zN|iGR;AfkhWU zu{=8i!>%j5sX>iPr#@aNJNf#Q3SB0!XaW?U1>*hbwqa~bUmG&sD|9oIUTMYz78T}z z;K@LIKXt>2!o1^KP1rA(Sv;*-zyub(4aISs3=D7QIc~PUcf`Npy6bb+^)D2dz@qb^ zSeT1};hlx_%~dR!EfJ3Ig!);-n3=$$p-_Aph<|2&J0KCAKfSBZKK6yY>O&^5s4zDK zPXl6R26k|4GO{uVG6rxdC>Sc#C?x0S6_+Ta<>xEdD)=X5r6!l?6%^%{LqGPcb zix~!l0L}nH^Mf9$ThK$v7#d1JscAW&a7`>I$f?voHv^m$lM<_RfUZXk1IAQn5TF_m z9gC(1n+J(?Q)egl-7oSR*oy zzy_mx(+*2!MAukQTATrOA2_^Ib5e6t^Gb@*Gh_jl3<>lQ9_=`DB>}I1j70Z|DPf-w z$ZCv*SUo^gPQsN_pxHI8C_fj&8@RFwx^L6+i(C_vGvM~?C}?OZ*xCV$bTs$JL){My zbbNXst|vNgLybartpzc)7r{cj2zymUauH5$0Zvq9gz6kkZ2l<4UKwH6i6u)@!7Iin zUIAqhTzNSuGcN^8@=rtY1JnRm21e+??RqkDtqBq354hmMnSwH~mpT}la3>#f%1zSJ ajV4al6G}Il_`N`8x&Z(b{=gu34FCWLL%O{H literal 1809 zcmV+s2k!VmRzV~Bp6-C%0vkO!&G~kH|S6PP&MG!obgw#H4zy78Hs|vez$Dl zt*ZLQnAX~TD>q3n8WG@-6G##U|JWz&NM+PR3nx9bfA zMq80&_ZjqL>#7sd*2Tjv!o;f$1V%Ro17NX*zEioq@s-u%*6m1mcy%p-(exJpuCps6$*usbKTz`O9`m_tm#Zp@ji-_) zNibT^N^v&;D#{|mCKJbczV=YnOMCj!1qnt^vXasrfUV`*BOfg3eKL56q3$W{Y9cV& z=S9Sj}v3T>^Yn%G7IbSw^xCEoWvobvtfJ52w z>0LJep(L8;B`f<@M@ukT&PwkdD3%Xul5Peaey{dG{J^hL@}5aBs$=CTlBTVn-kZ5) zr2R-L*j1pclMom!4g=s~$mRug*Iyj6cc-jk+*@-e5*Y0l4nXtNU1c}PXUm)9MVp2_ z(IYQ1=n*7W2Nx9u>^M67=!fRG;p+R32#lt@1i)`=svAZ<9`Gobu)iTIJWxYmG^8g0 zw?9kVJ-+!^>Z0>WH-dJwKnX^tvT_K?otVBCwgu{!%}l%#UK*4hAi*fn%OQD40zfdX z86pUT5mZP5RKfz7Winc5s5O~jJX9&w3bWOsQ)tX)Lw-c2hLS_lK<1K0+7cNT0?}0f z-LLoA3#L(|mZYH7qBmy4j6{mk! zMD2Cg6T|}H=&>S~q;;k_iY$%65P|OHa46csOj30IP_OLT@pCB3fR0b7rA)by){%}4 zDQgyOqF@_)4sRljL)+oE)XW?*i*`4d43%V@b_3rm=R3ipVD( zA|g(eh*R8Jz(FcH0of6p&OC5eb12+Jy&Uh%JNR1%{-%ZoxlCn5|Qqb4s*<$YQe)ST` zaqq#4g(!US3H%7UJm7Yx$kZF>-~hS>Ppl~&Y=_>%wbU6dd8v2Dvm zQ)GhSrT%%j(&=Rb*;c>L^BE(l5zY$-W2s7OHD=LzlaVQI4>@30$3mqu5&o||IXu#T{L|SO^?r9Miw{=Xg}9JNb0< zkx#tClINw-zH7vX7l?U+rP0AD)bkL^H;7*w-w8HskB5xV?AWox#)IE^v!Y))Tfn+9 z4;73lPm!Xbb&6cge73_Ftp$qKZ_^ag>Uv+`_jsY#a`d(#w9adO_)6mgVY+B^^P3iI|R>Wa)`Z(jsk9Je8lZf!WvcTc1nDABaz{9OpZJ=3*Cp* z@%&$l8c{oMKwTpAipiX%ykWE>-RnQipXE+M5jMwiW0PZvVF-5_YC=(Pu21i zmXp4$0_R}bwniWh;wo5R8YM=Fn5*4tS)kMEgs16ti#$g`WaIf4iIhrrzZd`jf!u$N From e7262220aecb0784c1c4a0bdba3b76d03e2d8582 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Fri, 11 Jun 2021 12:55:07 +0800 Subject: [PATCH 2/5] More test --- test/prefer-prototype-methods.mjs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/prefer-prototype-methods.mjs b/test/prefer-prototype-methods.mjs index 50494a3653..e03925f999 100644 --- a/test/prefer-prototype-methods.mjs +++ b/test/prefer-prototype-methods.mjs @@ -26,7 +26,11 @@ test.snapshot({ 'NotReflect.apply([].slice, foo, [])', 'Reflect.notApply([].slice, foo, [])', 'Reflect.apply([]?.slice, foo, [])', - 'Reflect.apply(foo, [].bar, [].bar)' + 'Reflect.apply(foo, [].bar, [].bar)', + 'class Foo {bar() { this.baz.bind(this) }}', + 'const a = {bar() { this.baz.bind(this) }}', + 'foo.bar.bind(foo)', + 'foo.bar.bind(bar)' ], invalid: [ 'const foo = [].push.apply(bar, elements);', From c1f0d8343cad2144560c0dc0f5019085a8a7dec0 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Fri, 11 Jun 2021 13:01:28 +0800 Subject: [PATCH 3/5] Simplify --- rules/prefer-prototype-methods.js | 37 +++++++++++++------------------ test/prefer-prototype-methods.mjs | 3 ++- 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/rules/prefer-prototype-methods.js b/rules/prefer-prototype-methods.js index a2e9d96e59..73d00265f0 100644 --- a/rules/prefer-prototype-methods.js +++ b/rules/prefer-prototype-methods.js @@ -13,46 +13,39 @@ const messages = { 'unknown-method': 'Prefer using method from `{{constructorName}}.prototype`.' }; -const emptyObjectOrArraySelector = matches([emptyObjectSelector(), emptyArraySelector()]); +const emptyObjectOrArrayMethodSelector = [ + 'MemberExpression', + matches([emptyObjectSelector('object'), emptyArraySelector('object')]) +].join(''); const functionMethodsSelector = [ methodCallSelector(['apply', 'bind', 'call']), ' > ', '.callee', ' > ', - 'MemberExpression.object', - ' > ', - `${emptyObjectOrArraySelector}.object` + `${emptyObjectOrArrayMethodSelector}.object` ].join(''); const reflectApplySelector = [ methodCallSelector({object: 'Reflect', name: 'apply', min: 1}), ' > ', - 'MemberExpression.arguments:first-child', - ' > ', - `${emptyObjectOrArraySelector}.object` + `${emptyObjectOrArrayMethodSelector}.arguments:first-child` ].join(''); -const selector = matches([ - functionMethodsSelector, - reflectApplySelector -]); +const selector = matches([functionMethodsSelector, reflectApplySelector]); /** @param {import('eslint').Rule.RuleContext} context */ function create(context) { return { [selector](node) { - const constructorName = node.type === 'ArrayExpression' ? 'Array' : 'Object'; - const methodName = getPropertyName(node.parent, context.getScope()); - const problem = { - node: node.parent, - messageId: methodName ? 'known-method' : 'unknown-method', - data: { - constructorName, methodName: String(methodName) - }, - fix: fixer => fixer.replaceText(node, `${constructorName}.prototype`) - }; + const constructorName = node.object.type === 'ArrayExpression' ? 'Array' : 'Object'; + const methodName = getPropertyName(node, context.getScope()); - context.report(problem); + context.report({ + node, + messageId: methodName ? 'known-method' : 'unknown-method', + data: {constructorName, methodName: String(methodName)}, + fix: fixer => fixer.replaceText(node.object, `${constructorName}.prototype`) + }); } }; } diff --git a/test/prefer-prototype-methods.mjs b/test/prefer-prototype-methods.mjs index e03925f999..33b82dfd10 100644 --- a/test/prefer-prototype-methods.mjs +++ b/test/prefer-prototype-methods.mjs @@ -30,7 +30,8 @@ test.snapshot({ 'class Foo {bar() { this.baz.bind(this) }}', 'const a = {bar() { this.baz.bind(this) }}', 'foo.bar.bind(foo)', - 'foo.bar.bind(bar)' + 'foo.bar.bind(bar)', + 'foo[{}].call(bar)' ], invalid: [ 'const foo = [].push.apply(bar, elements);', From 1ea90bd6cce13b4ca1fba7467510d7c08d0de491 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Fri, 11 Jun 2021 13:08:29 +0800 Subject: [PATCH 4/5] Add example --- rules/prefer-prototype-methods.js | 33 +++++++++++++++++-------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/rules/prefer-prototype-methods.js b/rules/prefer-prototype-methods.js index 73d00265f0..7d36ac3f4c 100644 --- a/rules/prefer-prototype-methods.js +++ b/rules/prefer-prototype-methods.js @@ -17,21 +17,24 @@ const emptyObjectOrArrayMethodSelector = [ 'MemberExpression', matches([emptyObjectSelector('object'), emptyArraySelector('object')]) ].join(''); -const functionMethodsSelector = [ - methodCallSelector(['apply', 'bind', 'call']), - ' > ', - '.callee', - ' > ', - `${emptyObjectOrArrayMethodSelector}.object` -].join(''); - -const reflectApplySelector = [ - methodCallSelector({object: 'Reflect', name: 'apply', min: 1}), - ' > ', - `${emptyObjectOrArrayMethodSelector}.arguments:first-child` -].join(''); - -const selector = matches([functionMethodsSelector, reflectApplySelector]); +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) { From 261422ea040127b766bd17320e7372cd3c66f56f Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Fri, 11 Jun 2021 16:09:30 +0800 Subject: [PATCH 5/5] Update prefer-prototype-methods.md --- docs/rules/prefer-prototype-methods.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/prefer-prototype-methods.md b/docs/rules/prefer-prototype-methods.md index 26e91ae492..0b618f5d5c 100644 --- a/docs/rules/prefer-prototype-methods.md +++ b/docs/rules/prefer-prototype-methods.md @@ -1,6 +1,6 @@ # Prefer borrowing methods from the prototype instead of methods from an instance -When “borrowing” a method from a `Array` or `Object`, 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.