Skip to content

Commit 639e7af

Browse files
fiskerfisker
authored
prefer-prototype-methods: Ignore known object literal methods (#1330)
1 parent 7d4165c commit 639e7af

File tree

4 files changed

+280
-4
lines changed

4 files changed

+280
-4
lines changed

‎rules/prefer-prototype-methods.js‎

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
'use strict';
2+
const {findVariable} = require('eslint-utils');
23
const getDocumentationUrl = require('./utils/get-documentation-url');
34
const {methodCallSelector} = require('./selectors');
45
const getPropertyName = require('./utils/get-property-name');
@@ -44,9 +45,18 @@ function isSafeToFix(node) {
4445
}
4546
}
4647

48+
function isObjectOwnProperty(property) {
49+
// eslint-disable-next-line no-useless-call
50+
return Object.prototype.hasOwnProperty.call(Object.prototype, property);
51+
}
52+
4753
/** @param {import('eslint').Rule.RuleContext} context */
4854
function create(context) {
49-
function check(method) {
55+
const methods = new Set();
56+
const nonArrowFunctionStack = [];
57+
const thisExpressions = new Map();
58+
59+
function check(method, scope) {
5060
const {type, object} = method;
5161
if (
5262
type !== 'MemberExpression' ||
@@ -63,8 +73,42 @@ function create(context) {
6373
return;
6474
}
6575

76+
const methodName = getPropertyName(method, scope);
77+
if (!isObjectOwnProperty(methodName)) {
78+
if (object.type === 'ObjectExpression' && object.properties.length > 0) {
79+
return;
80+
}
81+
82+
if (object.type === 'ThisExpression') {
83+
const functionNode = thisExpressions.get(object);
84+
if (
85+
functionNode &&
86+
functionNode.parent.type === 'Property' &&
87+
functionNode.parent.parent.type === 'ObjectExpression'
88+
) {
89+
return;
90+
}
91+
}
92+
93+
if (object.type === 'Identifier') {
94+
const variable = findVariable(scope, object);
95+
if (variable && variable.defs.length === 1) {
96+
const [definition] = variable.defs;
97+
if (
98+
definition.type === 'Variable' &&
99+
definition.kind === 'const' &&
100+
definition.node.id === definition.name &&
101+
definition.node.type === 'VariableDeclarator' &&
102+
definition.node.init &&
103+
definition.node.init.type === 'ObjectExpression'
104+
) {
105+
return;
106+
}
107+
}
108+
}
109+
}
110+
66111
const constructorName = getConstructorName(object);
67-
const methodName = getPropertyName(method, context.getScope());
68112
const messageId = [
69113
constructorName ? 'known' : 'unknown',
70114
'constructor',
@@ -86,10 +130,26 @@ function create(context) {
86130
}
87131

88132
return {
133+
'FunctionExpression,FunctionDeclaration'(node) {
134+
nonArrowFunctionStack.push(node);
135+
},
136+
'FunctionExpression,FunctionDeclaration:exit'() {
137+
nonArrowFunctionStack.pop();
138+
},
139+
'ThisExpression'(node) {
140+
thisExpressions.set(node, nonArrowFunctionStack[nonArrowFunctionStack.length - 1]);
141+
},
89142
[reflectApplySelector](node) {
90-
check(node.arguments[0]);
143+
methods.add({method: node.arguments[0], scope: context.getScope()});
144+
},
145+
[functionMethodsSelector](node) {
146+
methods.add({method: node, scope: context.getScope()});
91147
},
92-
[functionMethodsSelector]: check
148+
'Program:exit'() {
149+
for (const {method, scope} of methods) {
150+
check(method, scope);
151+
}
152+
}
93153
};
94154
}
95155

‎test/prefer-prototype-methods.mjs‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import outdent from 'outdent';
12
import {getTester} from './utils/test.mjs';
23

34
const {test} = getTester(import.meta);
@@ -49,6 +50,81 @@ test.snapshot({
4950
]
5051
});
5152

53+
// Object method
54+
test.snapshot({
55+
valid: [
56+
outdent`
57+
const foo = {
58+
a() {
59+
this.method = this.method.bind(this);
60+
},
61+
b: function() {
62+
this.method = this.method.bind(this);
63+
},
64+
c() {
65+
const foo = () => this.method.bind(this);
66+
},
67+
d: {
68+
d1() {
69+
this.method.call(this);
70+
}
71+
}
72+
}
73+
`,
74+
outdent`
75+
const foo = {};
76+
const bar = foo.method.call(foo, 'property');
77+
`,
78+
'({method() {}}).method.call(foo)'
79+
],
80+
invalid: [
81+
outdent`
82+
const foo = {
83+
a: () => {
84+
this.method = this.method.bind(this);
85+
}
86+
}
87+
`,
88+
outdent`
89+
const {foo} = {foo: {}};
90+
const bar = foo.method.call(foo, 'property');
91+
`,
92+
outdent`
93+
const [foo] = [{}];
94+
const bar = foo.method.call(foo, 'property');
95+
`,
96+
outdent`
97+
const foo = {
98+
a() {
99+
this.propertyIsEnumerable.apply(this, []);
100+
}
101+
}
102+
`,
103+
outdent`
104+
const foo = {
105+
a() {
106+
function fn() {
107+
// this is not the object foo
108+
this.method.call(this);
109+
}
110+
return fn;
111+
}
112+
}
113+
`,
114+
'this.method = this.method.bind(this)',
115+
outdent`
116+
const foo = {};
117+
const bar = foo.hasOwnProperty.call(foo, 'property');
118+
`,
119+
'for (const foo of []) foo.bar.call(foo)',
120+
outdent`
121+
let foo = {};
122+
const bar = foo.method.call(foo, 'property');
123+
`,
124+
'({method() {}}).propertyIsEnumerable.call(foo)'
125+
]
126+
});
127+
52128
test.babel({
53129
testerOptions: {
54130
env: {es2021: true}

‎test/snapshots/prefer-prototype-methods.mjs.md‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,3 +243,143 @@ Generated by [AVA](https://avajs.dev).
243243
> 1 | window.Math.max.apply(null, numbers)␊
244244
| ^^^^^^^^^^^^^^^ Prefer using \`max\` method from the constructor prototype.␊
245245
`
246+
247+
## Invalid #1
248+
1 | const foo = {
249+
2 | a: () => {
250+
3 | this.method = this.method.bind(this);
251+
4 | }
252+
5 | }
253+
254+
> Error 1/1
255+
256+
`␊
257+
1 | const foo = {␊
258+
2 | a: () => {␊
259+
> 3 | this.method = this.method.bind(this);␊
260+
| ^^^^^^^^^^^ Prefer using \`method\` method from the constructor prototype.␊
261+
4 | }␊
262+
5 | }␊
263+
`
264+
265+
## Invalid #2
266+
1 | const {foo} = {foo: {}};
267+
2 | const bar = foo.method.call(foo, 'property');
268+
269+
> Error 1/1
270+
271+
`␊
272+
1 | const {foo} = {foo: {}};␊
273+
> 2 | const bar = foo.method.call(foo, 'property');␊
274+
| ^^^^^^^^^^ Prefer using \`method\` method from the constructor prototype.␊
275+
`
276+
277+
## Invalid #3
278+
1 | const [foo] = [{}];
279+
2 | const bar = foo.method.call(foo, 'property');
280+
281+
> Error 1/1
282+
283+
`␊
284+
1 | const [foo] = [{}];␊
285+
> 2 | const bar = foo.method.call(foo, 'property');␊
286+
| ^^^^^^^^^^ Prefer using \`method\` method from the constructor prototype.␊
287+
`
288+
289+
## Invalid #4
290+
1 | const foo = {
291+
2 | a() {
292+
3 | this.propertyIsEnumerable.apply(this, []);
293+
4 | }
294+
5 | }
295+
296+
> Error 1/1
297+
298+
`␊
299+
1 | const foo = {␊
300+
2 | a() {␊
301+
> 3 | this.propertyIsEnumerable.apply(this, []);␊
302+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using \`propertyIsEnumerable\` method from the constructor prototype.␊
303+
4 | }␊
304+
5 | }␊
305+
`
306+
307+
## Invalid #5
308+
1 | const foo = {
309+
2 | a() {
310+
3 | function fn() {
311+
4 | // this is not the object foo
312+
5 | this.method.call(this);
313+
6 | }
314+
7 | return fn;
315+
8 | }
316+
9 | }
317+
318+
> Error 1/1
319+
320+
`␊
321+
1 | const foo = {␊
322+
2 | a() {␊
323+
3 | function fn() {␊
324+
4 | // this is not the object foo␊
325+
> 5 | this.method.call(this);␊
326+
| ^^^^^^^^^^^ Prefer using \`method\` method from the constructor prototype.␊
327+
6 | }␊
328+
7 | return fn;␊
329+
8 | }␊
330+
9 | }␊
331+
`
332+
333+
## Invalid #6
334+
1 | this.method = this.method.bind(this)
335+
336+
> Error 1/1
337+
338+
`␊
339+
> 1 | this.method = this.method.bind(this)␊
340+
| ^^^^^^^^^^^ Prefer using \`method\` method from the constructor prototype.␊
341+
`
342+
343+
## Invalid #7
344+
1 | const foo = {};
345+
2 | const bar = foo.hasOwnProperty.call(foo, 'property');
346+
347+
> Error 1/1
348+
349+
`␊
350+
1 | const foo = {};␊
351+
> 2 | const bar = foo.hasOwnProperty.call(foo, 'property');␊
352+
| ^^^^^^^^^^^^^^^^^^ Prefer using \`hasOwnProperty\` method from the constructor prototype.␊
353+
`
354+
355+
## Invalid #8
356+
1 | for (const foo of []) foo.bar.call(foo)
357+
358+
> Error 1/1
359+
360+
`␊
361+
> 1 | for (const foo of []) foo.bar.call(foo)␊
362+
| ^^^^^^^ Prefer using \`bar\` method from the constructor prototype.␊
363+
`
364+
365+
## Invalid #9
366+
1 | let foo = {};
367+
2 | const bar = foo.method.call(foo, 'property');
368+
369+
> Error 1/1
370+
371+
`␊
372+
1 | let foo = {};␊
373+
> 2 | const bar = foo.method.call(foo, 'property');␊
374+
| ^^^^^^^^^^ Prefer using \`method\` method from the constructor prototype.␊
375+
`
376+
377+
## Invalid #10
378+
1 | ({method() {}}).propertyIsEnumerable.call(foo)
379+
380+
> Error 1/1
381+
382+
`␊
383+
> 1 | ({method() {}}).propertyIsEnumerable.call(foo)␊
384+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using \`Object.prototype.propertyIsEnumerable\`.␊
385+
`

‎test/snapshots/prefer-prototype-methods.mjs.snap‎

572 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)