Skip to content

Commit 54a867f

Browse files
ota-meshiota-meshi
authored
Improve rules for checking prototype methods to reduce false positives (#62)
* Improve rules for checking prototype methods to reduce false positives * use cache
1 parent 92ca2b5 commit 54a867f

File tree

5 files changed

+1104
-133
lines changed

5 files changed

+1104
-133
lines changed

‎lib/util/define-prototype-method-handler/index.js‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
"use strict"
2+
3+
const { getPropertyName } = require("@eslint-community/eslint-utils")
4+
const { buildObjectTypeChecker } = require("./object-type-checker")
5+
const { buildObjectTypeCheckerForTS } = require("./object-type-checker-for-ts")
6+
7+
/**
8+
* @typedef {object} CreateReportArgument
9+
* @property {true | 'aggressive'} objectTypeResult
10+
* @property {string} propertyName
11+
* @property {MemberExpression} node
12+
*/
13+
/**
14+
* @typedef {object} Options
15+
* @property { (arg: CreateReportArgument) => ReportDescriptor } [Options.createReport]
16+
*/
17+
18+
/**
19+
* Define handlers to disallow prototype methods.
20+
* @param {RuleContext} context The rule context.
21+
* @param {Record<string, readonly string[]>} nameMap The method names to disallow. The key is class names and that value is method names.
22+
* @param {Options} [options] The options.
23+
* @returns {Record<string, (node: ASTNode) => void>} The defined handlers.
24+
*/
25+
function definePrototypeMethodHandler(context, nameMap, options) {
26+
const aggressiveOption = getAggressiveOption(context)
27+
const aggressiveResult = aggressiveOption ? "aggressive" : false
28+
29+
const objectTypeChecker =
30+
buildObjectTypeCheckerForTS(context, aggressiveResult) ||
31+
buildObjectTypeChecker(context, aggressiveResult)
32+
33+
/**
34+
* Check if the type of the given node is one of given class or not.
35+
* @param {MemberExpression} memberAccessNode The MemberExpression node.
36+
* @param {string} className The class name to disallow.
37+
* @returns {boolean | "aggressive"} `true` if should disallow it.
38+
*/
39+
function checkObjectType(memberAccessNode, className) {
40+
// If it's obvious, shortcut.
41+
if (memberAccessNode.object.type === "ArrayExpression") {
42+
return className === "Array"
43+
}
44+
if (
45+
memberAccessNode.object.type === "Literal" &&
46+
memberAccessNode.object.regex
47+
) {
48+
return className === "RegExp"
49+
}
50+
if (
51+
(memberAccessNode.object.type === "Literal" &&
52+
typeof memberAccessNode.object.value === "string") ||
53+
memberAccessNode.object.type === "TemplateLiteral"
54+
) {
55+
return className === "String"
56+
}
57+
if (
58+
memberAccessNode.object.type === "FunctionExpression" ||
59+
memberAccessNode.object.type === "ArrowFunctionExpression"
60+
) {
61+
return className === "Function"
62+
}
63+
64+
// Test object type.
65+
return objectTypeChecker(memberAccessNode, className)
66+
}
67+
68+
// For performance
69+
const nameMapEntries = Object.entries(nameMap)
70+
if (nameMapEntries.length === 1) {
71+
const [[className, methodNames]] = nameMapEntries
72+
return {
73+
MemberExpression(node) {
74+
const propertyName = getPropertyName(node, context.getScope())
75+
let objectTypeResult = undefined
76+
if (
77+
methodNames.includes(propertyName) &&
78+
(objectTypeResult = checkObjectType(node, className))
79+
) {
80+
context.report({
81+
node,
82+
messageId: "forbidden",
83+
data: {
84+
name: `${className}.prototype.${propertyName}`,
85+
},
86+
...((options &&
87+
options.createReport &&
88+
options.createReport({
89+
objectTypeResult,
90+
propertyName,
91+
node,
92+
})) ||
93+
{}),
94+
})
95+
}
96+
},
97+
}
98+
}
99+
100+
return {
101+
MemberExpression(node) {
102+
const propertyName = getPropertyName(node, context.getScope())
103+
for (const [className, methodNames] of nameMapEntries) {
104+
if (
105+
methodNames.includes(propertyName) &&
106+
checkObjectType(node, className)
107+
) {
108+
context.report({
109+
node,
110+
messageId: "forbidden",
111+
data: {
112+
name: `${className}.prototype.${propertyName}`,
113+
},
114+
})
115+
return
116+
}
117+
}
118+
},
119+
}
120+
}
121+
122+
/**
123+
* Get `aggressive` option value.
124+
* @param {RuleContext} context The rule context.
125+
* @returns {boolean} The gotten `aggressive` option value.
126+
*/
127+
function getAggressiveOption(context) {
128+
const options = context.options[0]
129+
const globalOptions = context.settings["es-x"]
130+
131+
if (options && typeof options.aggressive === "boolean") {
132+
return options.aggressive
133+
}
134+
if (globalOptions && typeof globalOptions.aggressive === "boolean") {
135+
return globalOptions.aggressive
136+
}
137+
138+
return false
139+
}
140+
141+
module.exports = { definePrototypeMethodHandler }

‎lib/util/define-prototype-method-handler.js‎ renamed to ‎lib/util/define-prototype-method-handler/object-type-checker-for-ts.js‎

Lines changed: 18 additions & 132 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,22 @@
11
"use strict"
22

3-
const { getPropertyName } = require("@eslint-community/eslint-utils")
4-
const { optionalRequire } = require("./optional-require")
5-
3+
const { optionalRequire } = require("../optional-require")
64
/** @type {import("typescript")} */
75
const ts = optionalRequire(require, "typescript")
86

7+
module.exports = { buildObjectTypeCheckerForTS }
8+
99
/**
10-
* @typedef {object} CreateReportArgument
11-
* @property {true | 'aggressive'} objectTypeResult
12-
* @property {string} propertyName
13-
* @property {MemberExpression} node
14-
*/
15-
/**
16-
* @typedef {object} Options
17-
* @property { (arg: CreateReportArgument) => ReportDescriptor } [Options.createReport]
10+
* @typedef {import("eslint").Rule.RuleContext} RuleContext
11+
* @typedef {import("estree").MemberExpression} MemberExpression
1812
*/
19-
2013
/**
21-
* Define handlers to disallow prototype methods.
14+
* Build object type checker for TypeScript.
2215
* @param {RuleContext} context The rule context.
23-
* @param {Record<string, readonly string[]>} nameMap The method names to disallow. The key is class names and that value is method names.
24-
* @param {Options} [options] The options.
25-
* @returns {Record<string, (node: ASTNode) => void>} The defined handlers.
16+
* @param {boolean} aggressiveResult The value to return if the type cannot be determined.
17+
* @returns {((memberAccessNode: MemberExpression, className: string) => boolean | "aggressive") | null} Returns an object type checker. Returns null if TypeScript is not available.
2618
*/
27-
function definePrototypeMethodHandler(context, nameMap, options) {
28-
const aggressiveOption = getAggressiveOption(context)
29-
const aggressiveResult = aggressiveOption ? "aggressive" : false
30-
19+
function buildObjectTypeCheckerForTS(context, aggressiveResult) {
3120
/** @type {ReadonlyMap<any, import("typescript").Node>} */
3221
const tsNodeMap = context.parserServices.esTreeNodeToTSNodeMap
3322
/** @type {import("typescript").TypeChecker} */
@@ -36,45 +25,16 @@ function definePrototypeMethodHandler(context, nameMap, options) {
3625
context.parserServices.program.getTypeChecker()
3726

3827
const isTS = Boolean(ts && tsNodeMap && checker)
39-
const hasFullType =
40-
isTS && context.parserServices.hasFullTypeInformation !== false
41-
42-
/**
43-
* Check if the type of the given node is one of given class or not.
44-
* @param {MemberExpression} memberAccessNode The MemberExpression node.
45-
* @param {string} className The class name to disallow.
46-
* @returns {boolean | "aggressive"} `true` if should disallow it.
47-
*/
48-
function checkObjectType(memberAccessNode, className) {
49-
// If it's obvious, shortcut.
50-
if (memberAccessNode.object.type === "ArrayExpression") {
51-
return className === "Array"
52-
}
53-
if (
54-
memberAccessNode.object.type === "Literal" &&
55-
memberAccessNode.object.regex
56-
) {
57-
return className === "RegExp"
58-
}
59-
if (
60-
(memberAccessNode.object.type === "Literal" &&
61-
typeof memberAccessNode.object.value === "string") ||
62-
memberAccessNode.object.type === "TemplateLiteral"
63-
) {
64-
return className === "String"
65-
}
66-
if (
67-
memberAccessNode.object.type === "FunctionExpression" ||
68-
memberAccessNode.object.type === "ArrowFunctionExpression"
69-
) {
70-
return className === "Function"
71-
}
28+
if (!isTS) {
29+
return null
30+
}
31+
const hasFullType = context.parserServices.hasFullTypeInformation !== false
7232

73-
// Test object type.
74-
return isTS
75-
? checkByPropertyDeclaration(memberAccessNode, className) ||
76-
checkByObjectExpressionType(memberAccessNode, className)
77-
: aggressiveResult
33+
return function (memberAccessNode, className) {
34+
return (
35+
checkByPropertyDeclaration(memberAccessNode, className) ||
36+
checkByObjectExpressionType(memberAccessNode, className)
37+
)
7838
}
7939

8040
/**
@@ -237,78 +197,6 @@ function definePrototypeMethodHandler(context, nameMap, options) {
237197
}
238198
return undefined
239199
}
240-
241-
// For performance
242-
const nameMapEntries = Object.entries(nameMap)
243-
if (nameMapEntries.length === 1) {
244-
const [[className, methodNames]] = nameMapEntries
245-
return {
246-
MemberExpression(node) {
247-
const propertyName = getPropertyName(node, context.getScope())
248-
let objectTypeResult = undefined
249-
if (
250-
methodNames.includes(propertyName) &&
251-
(objectTypeResult = checkObjectType(node, className))
252-
) {
253-
context.report({
254-
node,
255-
messageId: "forbidden",
256-
data: {
257-
name: `${className}.prototype.${propertyName}`,
258-
},
259-
...((options &&
260-
options.createReport &&
261-
options.createReport({
262-
objectTypeResult,
263-
propertyName,
264-
node,
265-
})) ||
266-
{}),
267-
})
268-
}
269-
},
270-
}
271-
}
272-
273-
return {
274-
MemberExpression(node) {
275-
const propertyName = getPropertyName(node, context.getScope())
276-
for (const [className, methodNames] of nameMapEntries) {
277-
if (
278-
methodNames.includes(propertyName) &&
279-
checkObjectType(node, className)
280-
) {
281-
context.report({
282-
node,
283-
messageId: "forbidden",
284-
data: {
285-
name: `${className}.prototype.${propertyName}`,
286-
},
287-
})
288-
return
289-
}
290-
}
291-
},
292-
}
293-
}
294-
295-
/**
296-
* Get `aggressive` option value.
297-
* @param {RuleContext} context The rule context.
298-
* @returns {boolean} The gotten `aggressive` option value.
299-
*/
300-
function getAggressiveOption(context) {
301-
const options = context.options[0]
302-
const globalOptions = context.settings["es-x"]
303-
304-
if (options && typeof options.aggressive === "boolean") {
305-
return options.aggressive
306-
}
307-
if (globalOptions && typeof globalOptions.aggressive === "boolean") {
308-
return globalOptions.aggressive
309-
}
310-
311-
return false
312200
}
313201

314202
/**
@@ -429,5 +317,3 @@ function isFunction(type) {
429317
const signatures = type.getCallSignatures()
430318
return signatures.length > 0
431319
}
432-
433-
module.exports = { definePrototypeMethodHandler }

0 commit comments

Comments
 (0)