Skip to content

Commit

Permalink
feat: Add suggestion for no-prototype-builtins (#17677)
Browse files Browse the repository at this point in the history 
Suggest a fix e.g. a.hasOwnProperty(b) -> Object.prototype.hasOwnProperty.call(a, b). However, if the method call follows an optional chain, then make no suggestions.
  • Loading branch information
@yonran
yonran committed Oct 26, 2023
1 parent 3aec1c5 commit 3486e20
Show file tree
Hide file tree
Showing 2 changed files with 192 additions and 6 deletions.
69 changes: 67 additions & 2 deletions lib/rules/no-prototype-builtins.js
View file
Expand Up @@ -10,6 +10,37 @@

const astUtils = require("./utils/ast-utils");

//------------------------------------------------------------------------------
// Helpers
//------------------------------------------------------------------------------

/**
* Returns true if the node or any of the objects
* to the left of it in the member/call chain is optional.
*
* e.g. `a?.b`, `a?.b.c`, `a?.()`, `a()?.()`
* @param {ASTNode} node The expression to check
* @returns {boolean} `true` if there is a short-circuiting optional `?.`
* in the same option chain to the left of this call or member expression,
* or the node itself is an optional call or member `?.`.
*/
function isAfterOptional(node) {
let leftNode;

if (node.type === "MemberExpression") {
leftNode = node.object;
} else if (node.type === "CallExpression") {
leftNode = node.callee;
} else {
return false;
}
if (node.optional) {
return true;
}
return isAfterOptional(leftNode);
}


//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand All @@ -25,10 +56,13 @@ module.exports = {
url: "https://eslint.org/docs/latest/rules/no-prototype-builtins"
},

hasSuggestions: true,

schema: [],

messages: {
prototypeBuildIn: "Do not access Object.prototype method '{{prop}}' from target object."
prototypeBuildIn: "Do not access Object.prototype method '{{prop}}' from target object.",
callObjectPrototype: "Call Object.prototype.{{prop}} explicitly."
}
},

Expand Down Expand Up @@ -59,7 +93,38 @@ module.exports = {
messageId: "prototypeBuildIn",
loc: callee.property.loc,
data: { prop: propName },
node
node,
suggest: [
{
messageId: "callObjectPrototype",
data: { prop: propName },
fix(fixer) {

/*
* a call after an optional chain (e.g. a?.b.hasOwnProperty(c))
* must be fixed manually because the call can be short-circuited
*/
if (isAfterOptional(node)) {
return null;
}

const sourceCode = context.sourceCode;
const openParenToken = sourceCode.getTokenAfter(
node.callee,
astUtils.isOpeningParenToken
);
const objectText = sourceCode.getText(callee.object);
const isEmptyParameters = node.arguments.length === 0;
const delim = isEmptyParameters ? "" : ", ";
const fixes = [
fixer.replaceText(callee, `Object.prototype.${propName}.call`),
fixer.insertTextAfter(openParenToken, objectText + delim)
];

return fixes;
}
}
]
});
}
}
Expand Down
129 changes: 125 additions & 4 deletions tests/lib/rules/no-prototype-builtins.js
View file
Expand Up @@ -61,6 +61,12 @@ ruleTester.run("no-prototype-builtins", rule, {
endColumn: 19,
messageId: "prototypeBuildIn",
data: { prop: "hasOwnProperty" },
suggestions: [
{
messageId: "callObjectPrototype",
output: "Object.prototype.hasOwnProperty.call(foo, 'bar')"
}
],
type: "CallExpression"
}]
},
Expand All @@ -73,6 +79,12 @@ ruleTester.run("no-prototype-builtins", rule, {
endColumn: 18,
messageId: "prototypeBuildIn",
data: { prop: "isPrototypeOf" },
suggestions: [
{
messageId: "callObjectPrototype",
output: "Object.prototype.isPrototypeOf.call(foo, 'bar')"
}
],
type: "CallExpression"
}]
},
Expand All @@ -84,6 +96,12 @@ ruleTester.run("no-prototype-builtins", rule, {
endLine: 1,
endColumn: 25,
messageId: "prototypeBuildIn",
suggestions: [
{
messageId: "callObjectPrototype",
output: "Object.prototype.propertyIsEnumerable.call(foo, 'bar')"
}
],
data: { prop: "propertyIsEnumerable" }
}]
},
Expand All @@ -96,6 +114,12 @@ ruleTester.run("no-prototype-builtins", rule, {
endColumn: 23,
messageId: "prototypeBuildIn",
data: { prop: "hasOwnProperty" },
suggestions: [
{
messageId: "callObjectPrototype",
output: "Object.prototype.hasOwnProperty.call(foo.bar, 'bar')"
}
],
type: "CallExpression"
}]
},
Expand All @@ -108,6 +132,12 @@ ruleTester.run("no-prototype-builtins", rule, {
endColumn: 26,
messageId: "prototypeBuildIn",
data: { prop: "isPrototypeOf" },
suggestions: [
{
messageId: "callObjectPrototype",
output: "Object.prototype.isPrototypeOf.call(foo.bar.baz, 'bar')"
}
],
type: "CallExpression"
}]
},
Expand All @@ -120,6 +150,12 @@ ruleTester.run("no-prototype-builtins", rule, {
endColumn: 21,
messageId: "prototypeBuildIn",
data: { prop: "hasOwnProperty" },
suggestions: [
{
messageId: "callObjectPrototype",
output: "Object.prototype.hasOwnProperty.call(foo, 'bar')"
}
],
type: "CallExpression"
}]
},
Expand All @@ -133,6 +169,12 @@ ruleTester.run("no-prototype-builtins", rule, {
endColumn: 20,
messageId: "prototypeBuildIn",
data: { prop: "isPrototypeOf" },
suggestions: [
{
messageId: "callObjectPrototype",
output: "Object.prototype.isPrototypeOf.call(foo, 'bar').baz"
}
],
type: "CallExpression"
}]
},
Expand All @@ -145,6 +187,12 @@ ruleTester.run("no-prototype-builtins", rule, {
endColumn: 31,
messageId: "prototypeBuildIn",
data: { prop: "propertyIsEnumerable" },
suggestions: [
{
messageId: "callObjectPrototype",
output: String.raw`Object.prototype.propertyIsEnumerable.call(foo.bar, 'baz')`
}
],
type: "CallExpression"
}]
},
Expand All @@ -153,22 +201,95 @@ ruleTester.run("no-prototype-builtins", rule, {
{
code: "foo?.hasOwnProperty('bar')",
parserOptions: { ecmaVersion: 2020 },
errors: [{ messageId: "prototypeBuildIn", data: { prop: "hasOwnProperty" } }]
errors: [{ messageId: "prototypeBuildIn", data: { prop: "hasOwnProperty" }, suggestions: [] }]
},
{
code: "foo?.bar.hasOwnProperty('baz')",
parserOptions: { ecmaVersion: 2020 },
errors: [{ messageId: "prototypeBuildIn", data: { prop: "hasOwnProperty" }, suggestions: [] }]
},
{
code: "foo.hasOwnProperty?.('bar')",
parserOptions: { ecmaVersion: 2020 },
errors: [{ messageId: "prototypeBuildIn", data: { prop: "hasOwnProperty" }, suggestions: [] }]
},
{

/*
* if hasOwnProperty is part of a ChainExpresion
* and the optional part is before it, then don't suggest the fix
*/
code: "foo?.hasOwnProperty('bar').baz",
parserOptions: { ecmaVersion: 2020 },
errors: [{ messageId: "prototypeBuildIn", data: { prop: "hasOwnProperty" }, suggestions: [] }]
},
{

/*
* if hasOwnProperty is part of a ChainExpresion
* but the optional part is after it, then the fix is safe
*/
code: "foo.hasOwnProperty('bar')?.baz",
parserOptions: { ecmaVersion: 2020 },
errors: [{
messageId: "prototypeBuildIn",
data: { prop: "hasOwnProperty" },
suggestions: [
{
messageId: "callObjectPrototype",
output: "Object.prototype.hasOwnProperty.call(foo, 'bar')?.baz"
}
]
}]
},
{
code: "(foo?.hasOwnProperty)('bar')",
parserOptions: { ecmaVersion: 2020 },
errors: [{ messageId: "prototypeBuildIn", data: { prop: "hasOwnProperty" } }]
errors: [{
messageId: "prototypeBuildIn",
data: { prop: "hasOwnProperty" },
suggestions: [
{

/*
* Note that the original may throw TypeError: (intermediate value) is not a function
* whereas the replacement may throw TypeError: Cannot convert undefined or null to object
*/
messageId: "callObjectPrototype",
output: "(Object.prototype.hasOwnProperty.call)(foo, 'bar')"
}
]
}]
},
{
code: "(foo?.hasOwnProperty)?.('bar')",
parserOptions: { ecmaVersion: 2020 },
errors: [{ messageId: "prototypeBuildIn", data: { prop: "hasOwnProperty" }, suggestions: [] }]
},
{
code: "foo?.['hasOwnProperty']('bar')",
parserOptions: { ecmaVersion: 2020 },
errors: [{ messageId: "prototypeBuildIn", data: { prop: "hasOwnProperty" } }]
errors: [{ messageId: "prototypeBuildIn", data: { prop: "hasOwnProperty" }, suggestions: [] }]
},
{
code: "(foo?.[`hasOwnProperty`])('bar')",
parserOptions: { ecmaVersion: 2020 },
errors: [{ messageId: "prototypeBuildIn", data: { prop: "hasOwnProperty" } }]
errors: [{
messageId: "prototypeBuildIn",
data: { prop: "hasOwnProperty" },
suggestions: [
{

/*
* Note that the original may throw TypeError: (intermediate value) is not a function
* whereas the replacement may throw TypeError: Cannot convert undefined or null to object
*/
messageId: "callObjectPrototype",
output: "(Object.prototype.hasOwnProperty.call)(foo, 'bar')"
}
]

}]
}
]
});

0 comments on commit 3486e20

Please sign in to comment.