Skip to content

Commit 1792d33

Browse files
fiskerfisker
authored
prefer-prototype-methods: Check Object.prototype methods from globalThis (#2286)
1 parent a3be554 commit 1792d33

6 files changed

+408
-56
lines changed

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

+4
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ const type = {}.toString.call(foo);
2323
Reflect.apply([].forEach, arrayLike, [callback]);
2424
```
2525

26+
```js
27+
const type = globalThis.toString.call(foo);
28+
```
29+
2630
## Pass
2731

2832
```js

‎rules/prefer-prototype-methods.js

+128-56
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
'use strict';
2-
const {getPropertyName} = require('@eslint-community/eslint-utils');
2+
const {getPropertyName, ReferenceTracker} = require('@eslint-community/eslint-utils');
33
const {fixSpaceAroundKeyword} = require('./fix/index.js');
44
const {isMemberExpression, isMethodCall} = require('./ast/index.js');
55

@@ -8,72 +8,144 @@ const messages = {
88
'unknown-method': 'Prefer using method from `{{constructorName}}.prototype`.',
99
};
1010

11-
/** @param {import('eslint').Rule.RuleContext} context */
12-
function create(context) {
11+
const OBJECT_PROTOTYPE_METHODS = [
12+
'hasOwnProperty',
13+
'isPrototypeOf',
14+
'propertyIsEnumerable',
15+
'toLocaleString',
16+
'toString',
17+
'valueOf',
18+
];
19+
20+
function getConstructorAndMethodName(methodNode, {sourceCode, globalReferences}) {
21+
if (!methodNode) {
22+
return;
23+
}
24+
25+
const isGlobalReference = globalReferences.has(methodNode);
26+
if (isGlobalReference) {
27+
const path = globalReferences.get(methodNode);
28+
return {
29+
isGlobalReference: true,
30+
constructorName: 'Object',
31+
methodName: path.at(-1),
32+
};
33+
}
34+
35+
if (!isMemberExpression(methodNode, {optional: false})) {
36+
return;
37+
}
38+
39+
const objectNode = methodNode.object;
40+
41+
if (!(
42+
(objectNode.type === 'ArrayExpression' && objectNode.elements.length === 0)
43+
|| (objectNode.type === 'ObjectExpression' && objectNode.properties.length === 0)
44+
)) {
45+
return;
46+
}
47+
48+
const constructorName = objectNode.type === 'ArrayExpression' ? 'Array' : 'Object';
49+
const methodName = getPropertyName(methodNode, sourceCode.getScope(methodNode));
50+
1351
return {
14-
CallExpression(callExpression) {
15-
let methodNode;
16-
17-
if (
18-
// `Reflect.apply([].foo, …)`
19-
// `Reflect.apply({}.foo, …)`
20-
isMethodCall(callExpression, {
21-
object: 'Reflect',
22-
method: 'apply',
23-
minimumArguments: 1,
24-
optionalCall: false,
25-
optionalMember: false,
26-
})
27-
) {
28-
methodNode = callExpression.arguments[0];
29-
} else if (
30-
// `[].foo.{apply,bind,call}(…)`
31-
// `({}).foo.{apply,bind,call}(…)`
32-
isMethodCall(callExpression, {
33-
methods: ['apply', 'bind', 'call'],
34-
optionalCall: false,
35-
optionalMember: false,
36-
})
37-
) {
38-
methodNode = callExpression.callee.object;
39-
}
52+
constructorName,
53+
methodName,
54+
};
55+
}
4056

41-
if (!methodNode || !isMemberExpression(methodNode, {optional: false})) {
42-
return;
43-
}
57+
function getProblem(callExpression, {sourceCode, globalReferences}) {
58+
let methodNode;
59+
60+
if (
61+
// `Reflect.apply([].foo, …)`
62+
// `Reflect.apply({}.foo, …)`
63+
isMethodCall(callExpression, {
64+
object: 'Reflect',
65+
method: 'apply',
66+
minimumArguments: 1,
67+
optionalCall: false,
68+
optionalMember: false,
69+
})
70+
) {
71+
methodNode = callExpression.arguments[0];
72+
} else if (
73+
// `[].foo.{apply,bind,call}(…)`
74+
// `({}).foo.{apply,bind,call}(…)`
75+
isMethodCall(callExpression, {
76+
methods: ['apply', 'bind', 'call'],
77+
optionalCall: false,
78+
optionalMember: false,
79+
})
80+
) {
81+
methodNode = callExpression.callee.object;
82+
}
4483

45-
const objectNode = methodNode.object;
84+
const {
85+
isGlobalReference,
86+
constructorName,
87+
methodName,
88+
} = getConstructorAndMethodName(methodNode, {sourceCode, globalReferences}) ?? {};
4689

47-
if (!(
48-
(objectNode.type === 'ArrayExpression' && objectNode.elements.length === 0)
49-
|| (objectNode.type === 'ObjectExpression' && objectNode.properties.length === 0)
50-
)) {
90+
if (!constructorName) {
91+
return;
92+
}
93+
94+
return {
95+
node: methodNode,
96+
messageId: methodName ? 'known-method' : 'unknown-method',
97+
data: {constructorName, methodName},
98+
* fix(fixer) {
99+
if (isGlobalReference) {
100+
yield fixer.replaceText(methodNode, `${constructorName}.prototype.${methodName}`);
51101
return;
52102
}
53103

54-
const constructorName = objectNode.type === 'ArrayExpression' ? 'Array' : 'Object';
55-
const {sourceCode} = context;
56-
const methodName = getPropertyName(methodNode, sourceCode.getScope(methodNode));
57-
58-
return {
59-
node: methodNode,
60-
messageId: methodName ? 'known-method' : 'unknown-method',
61-
data: {constructorName, methodName},
62-
* fix(fixer) {
63-
yield fixer.replaceText(objectNode, `${constructorName}.prototype`);
64-
65-
if (
66-
objectNode.type === 'ArrayExpression'
67-
|| objectNode.type === 'ObjectExpression'
68-
) {
69-
yield * fixSpaceAroundKeyword(fixer, callExpression, sourceCode);
70-
}
71-
},
72-
};
104+
if (isMemberExpression(methodNode)) {
105+
const objectNode = methodNode.object;
106+
107+
yield fixer.replaceText(objectNode, `${constructorName}.prototype`);
108+
109+
if (
110+
objectNode.type === 'ArrayExpression'
111+
|| objectNode.type === 'ObjectExpression'
112+
) {
113+
yield * fixSpaceAroundKeyword(fixer, callExpression, sourceCode);
114+
}
115+
}
73116
},
74117
};
75118
}
76119

120+
/** @param {import('eslint').Rule.RuleContext} context */
121+
function create(context) {
122+
const {sourceCode} = context;
123+
const callExpressions = [];
124+
125+
context.on('CallExpression', callExpression => {
126+
callExpressions.push(callExpression);
127+
});
128+
129+
context.on('Program:exit', function * (program) {
130+
const globalReferences = new WeakMap();
131+
132+
const tracker = new ReferenceTracker(sourceCode.getScope(program));
133+
134+
for (const {node, path} of tracker.iterateGlobalReferences(
135+
Object.fromEntries(OBJECT_PROTOTYPE_METHODS.map(method => [method, {[ReferenceTracker.READ]: true}])),
136+
)) {
137+
globalReferences.set(node, path);
138+
}
139+
140+
for (const callExpression of callExpressions) {
141+
yield getProblem(callExpression, {
142+
sourceCode,
143+
globalReferences,
144+
});
145+
}
146+
});
147+
}
148+
77149
/** @type {import('eslint').Rule.RuleModule} */
78150
module.exports = {
79151
create,

‎rules/utils/rule.js

+4
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ function reportListenerProblems(problems, context) {
4343
}
4444

4545
for (const problem of problems) {
46+
if (!problem) {
47+
continue;
48+
}
49+
4650
if (problem.fix) {
4751
problem.fix = wrapFixFunction(problem.fix);
4852
}

‎test/prefer-prototype-methods.mjs

+20
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,14 @@ test.snapshot({
3636
'const foo = [].push.notApply(bar, elements);',
3737
'const push = [].push.notBind(foo)',
3838
'[].forEach.notCall(foo, () => {})',
39+
'/* globals foo: readonly */ foo.call(bar)',
40+
'const toString = () => {}; toString.call(bar)',
41+
'/* globals toString: off */ toString.call(bar)',
42+
// Make sure the fix won't break code
43+
'const _hasOwnProperty = globalThis.hasOwnProperty; _hasOwnProperty.call(bar)',
44+
'const _globalThis = globalThis; globalThis[hasOwnProperty].call(bar)',
45+
'const _ = globalThis, TO_STRING = "toString"; _[TO_STRING].call(bar)',
46+
'const _ = [globalThis.toString]; _[0].call(bar)',
3947
],
4048
invalid: [
4149
'const foo = [].push.apply(bar, elements);',
@@ -61,5 +69,17 @@ test.snapshot({
6169
'[][Symbol.iterator].call(foo)',
6270
'const foo = [].at.call(bar)',
6371
'const foo = [].findLast.call(bar)',
72+
'/* globals hasOwnProperty: readonly */ hasOwnProperty.call(bar)',
73+
'/* globals toString: readonly */ toString.apply(bar, [])',
74+
'/* globals toString: readonly */ Reflect.apply(toString, baz, [])',
75+
'globalThis.toString.call(bar)',
76+
'const _ = globalThis; _.hasOwnProperty.call(bar)',
77+
'const _ = globalThis; _["hasOwnProperty"].call(bar)',
78+
'const _ = globalThis; _["hasOwn" + "Property"].call(bar)',
79+
'Reflect.apply(globalThis.toString, baz, [])',
80+
'Reflect.apply(window.toString, baz, [])',
81+
'Reflect.apply(global.toString, baz, [])',
82+
'/* globals toString: readonly */ Reflect.apply(toString, baz, [])',
83+
'Reflect.apply(globalThis["toString"], baz, [])',
6484
],
6585
});

0 commit comments

Comments
 (0)