Skip to content

Commit 29a4e75

Browse files
fiskerfisker
authored
prefer-prototype-methods: Only check methods from [] and {} (#1347)
1 parent e2540cb commit 29a4e75

File tree

8 files changed

+90
-449
lines changed

8 files changed

+90
-449
lines changed

‎docs/rules/prefer-prototype-methods.md‎

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
# Prefer borrowing methods from the prototype instead of methods from an instance.
1+
# Prefer borrowing methods from the prototype instead of methods from an instance
22

3-
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.
3+
When “borrowing” a method from `Array` or `Object`, it‘s clearer to get it from the prototype than from an instance.
44

5-
This rule is fixable when using method from `[]` or `{}`.
5+
This rule is fixable.
66

77
## Fail
88

@@ -14,10 +14,6 @@ const array = [].slice.apply(bar);
1414
const hasProperty = {}.hasOwnProperty.call(foo, 'property');
1515
```
1616

17-
```js
18-
foo.bar.call(baz);
19-
```
20-
2117
```js
2218
Reflect.apply([].forEach, arrayLike, [callback]);
2319
```
@@ -32,14 +28,6 @@ const array = Array.prototype.slice.apply(bar);
3228
const hasProperty = Object.prototype.hasOwnProperty.call(foo, 'property');
3329
```
3430

35-
```js
36-
Foo.prototype.bar.call(baz);
37-
```
38-
39-
```js
40-
foo.constructor.prototype.bar.call(baz);
41-
```
42-
4331
```js
4432
Reflect.apply(Array.prototype.forEach, arrayLike, [callback]);
4533
```

‎rules/prefer-prototype-methods.js‎

Lines changed: 39 additions & 140 deletions
Original file line numberDiff line numberDiff line change
@@ -1,155 +1,54 @@
11
'use strict';
2-
const {findVariable} = require('eslint-utils');
32
const getDocumentationUrl = require('./utils/get-documentation-url.js');
4-
const {methodCallSelector} = require('./selectors/index.js');
3+
const {
4+
methodCallSelector,
5+
emptyObjectSelector,
6+
emptyArraySelector,
7+
matches
8+
} = require('./selectors/index.js');
59
const getPropertyName = require('./utils/get-property-name.js');
610

711
const messages = {
8-
'known-constructor-known-method': 'Prefer using `{{constructorName}}.prototype.{{methodName}}`.',
9-
'known-constructor-unknown-method': 'Prefer using method from `{{constructorName}}.prototype`.',
10-
'unknown-constructor-known-method': 'Prefer using `{{methodName}}` method from the constructor prototype.',
11-
'unknown-constructor-unknown-method': 'Prefer using method from the constructor prototype.'
12+
'known-method': 'Prefer using `{{constructorName}}.prototype.{{methodName}}`.',
13+
'unknown-method': 'Prefer using method from `{{constructorName}}.prototype`.'
1214
};
1315

14-
const functionMethodsSelector = [
15-
methodCallSelector(['apply', 'bind', 'call']),
16-
' > ',
17-
'.callee',
18-
' > ',
19-
'.object'
16+
const emptyObjectOrArrayMethodSelector = [
17+
'MemberExpression',
18+
matches([emptyObjectSelector('object'), emptyArraySelector('object')])
2019
].join('');
21-
22-
const reflectApplySelector = methodCallSelector({
23-
object: 'Reflect',
24-
name: 'apply',
25-
min: 1
26-
});
27-
28-
function getConstructorName(node) {
29-
switch (node.type) {
30-
case 'ArrayExpression':
31-
return 'Array';
32-
case 'ObjectExpression':
33-
return 'Object';
34-
// No default
35-
}
36-
}
37-
38-
function isSafeToFix(node) {
39-
switch (node.type) {
40-
case 'ArrayExpression':
41-
return node.elements.length === 0;
42-
case 'ObjectExpression':
43-
return node.properties.length === 0;
44-
// No default
45-
}
46-
}
47-
48-
function isObjectOwnProperty(property) {
49-
// eslint-disable-next-line no-useless-call
50-
return Object.prototype.hasOwnProperty.call(Object.prototype, property);
51-
}
20+
const selector = matches([
21+
// `[].foo.{apply,bind,call}(…)`
22+
// `({}).foo.{apply,bind,call}(…)`
23+
[
24+
methodCallSelector(['apply', 'bind', 'call']),
25+
' > ',
26+
'.callee',
27+
' > ',
28+
`${emptyObjectOrArrayMethodSelector}.object`
29+
].join(''),
30+
// `Reflect.apply([].foo, …)`
31+
// `Reflect.apply({}.foo, …)`
32+
[
33+
methodCallSelector({object: 'Reflect', name: 'apply', min: 1}),
34+
' > ',
35+
`${emptyObjectOrArrayMethodSelector}.arguments:first-child`
36+
].join('')
37+
]);
5238

5339
/** @param {import('eslint').Rule.RuleContext} context */
5440
function create(context) {
55-
const methods = new Set();
56-
const nonArrowFunctionStack = [];
57-
const thisExpressions = new Map();
58-
59-
function check(method, scope) {
60-
const {type, object} = method;
61-
if (
62-
type !== 'MemberExpression' ||
63-
// Most likely it's a static method of a class
64-
(object.type === 'Identifier' && /^[A-Z]/.test(object.name)) ||
65-
(
66-
object.type === 'MemberExpression' &&
67-
!object.computed &&
68-
!object.optional &&
69-
object.property.type === 'Identifier' &&
70-
object.property.name === 'prototype'
71-
)
72-
) {
73-
return;
74-
}
75-
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.value === functionNode &&
88-
functionNode.parent.parent.type === 'ObjectExpression'
89-
) {
90-
return;
91-
}
92-
}
93-
94-
if (object.type === 'Identifier') {
95-
const variable = findVariable(scope, object);
96-
if (variable && variable.defs.length === 1) {
97-
const [definition] = variable.defs;
98-
if (
99-
definition.type === 'Variable' &&
100-
definition.kind === 'const' &&
101-
definition.node.id === definition.name &&
102-
definition.node.type === 'VariableDeclarator' &&
103-
definition.node.init &&
104-
definition.node.init.type === 'ObjectExpression'
105-
) {
106-
return;
107-
}
108-
}
109-
}
110-
}
111-
112-
const constructorName = getConstructorName(object);
113-
const messageId = [
114-
constructorName ? 'known' : 'unknown',
115-
'constructor',
116-
methodName ? 'known' : 'unknown',
117-
'method'
118-
].join('-');
119-
120-
const problem = {
121-
node: method,
122-
messageId,
123-
data: {constructorName, methodName: String(methodName)}
124-
};
125-
126-
if (constructorName && isSafeToFix(object)) {
127-
problem.fix = fixer => fixer.replaceText(object, `${constructorName}.prototype`);
128-
}
129-
130-
context.report(problem);
131-
}
132-
13341
return {
134-
'FunctionExpression,FunctionDeclaration'(node) {
135-
nonArrowFunctionStack.push(node);
136-
},
137-
'FunctionExpression,FunctionDeclaration:exit'() {
138-
nonArrowFunctionStack.pop();
139-
},
140-
'ThisExpression'(node) {
141-
thisExpressions.set(node, nonArrowFunctionStack[nonArrowFunctionStack.length - 1]);
142-
},
143-
[reflectApplySelector](node) {
144-
methods.add({method: node.arguments[0], scope: context.getScope()});
145-
},
146-
[functionMethodsSelector](node) {
147-
methods.add({method: node, scope: context.getScope()});
148-
},
149-
'Program:exit'() {
150-
for (const {method, scope} of methods) {
151-
check(method, scope);
152-
}
42+
[selector](node) {
43+
const constructorName = node.object.type === 'ArrayExpression' ? 'Array' : 'Object';
44+
const methodName = getPropertyName(node, context.getScope());
45+
46+
context.report({
47+
node,
48+
messageId: methodName ? 'known-method' : 'unknown-method',
49+
data: {constructorName, methodName: String(methodName)},
50+
fix: fixer => fixer.replaceText(node.object, `${constructorName}.prototype`)
51+
});
15352
}
15453
};
15554
}

‎rules/selectors/empty-array-selector.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
22

33
function emptyArraySelector(path) {
4-
const prefix = `${path}.`;
4+
const prefix = path ? `${path}.` : '';
55
return [
66
`[${prefix}type="ArrayExpression"]`,
77
`[${prefix}elements.length=0]`

‎rules/selectors/empty-object-selector.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
22

33
function emptyObjectSelector(path) {
4-
const prefix = `${path}.`;
4+
const prefix = path ? `${path}.` : '';
55
return [
66
`[${prefix}type="ObjectExpression"]`,
77
`[${prefix}properties.length=0]`

‎rules/selectors/index.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ module.exports = {
88
arrayPrototypeMethodSelector: require('./prototype-method-selector.js').arrayPrototypeMethodSelector,
99
objectPrototypeMethodSelector: require('./prototype-method-selector.js').objectPrototypeMethodSelector,
1010
emptyArraySelector: require('./empty-array-selector.js'),
11+
emptyObjectSelector: require('./empty-object-selector.js'),
1112
memberExpressionSelector: require('./member-expression-selector.js'),
1213
methodCallSelector: require('./method-call-selector.js'),
1314
notDomNodeSelector: require('./not-dom-node.js').notDomNodeSelector,

0 commit comments

Comments
 (0)