Skip to content

Commit 151ba3f

Browse files
Grit03Grit03
authored
fix: false positives in getter-return and accessor-pairs (#21163)
1 parent 65a6519 commit 151ba3f

7 files changed

Lines changed: 284 additions & 185 deletions

File tree

‎lib/rules/accessor-pairs.js‎

Lines changed: 1 addition & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -87,56 +87,6 @@ function isAccessorKind(node) {
8787
return node.kind === "get" || node.kind === "set";
8888
}
8989

90-
/**
91-
* Checks whether or not a given node is an argument of a specified method call.
92-
* @param {ASTNode} node A node to check.
93-
* @param {number} index An expected index of the node in arguments.
94-
* @param {string} object An expected name of the object of the method.
95-
* @param {string} property An expected name of the method.
96-
* @returns {boolean} `true` if the node is an argument of the specified method call.
97-
*/
98-
function isArgumentOfMethodCall(node, index, object, property) {
99-
const parent = node.parent;
100-
101-
return (
102-
parent.type === "CallExpression" &&
103-
astUtils.isSpecificMemberAccess(parent.callee, object, property) &&
104-
parent.arguments[index] === node
105-
);
106-
}
107-
108-
/**
109-
* Checks whether or not a given node is a property descriptor.
110-
* @param {ASTNode} node A node to check.
111-
* @returns {boolean} `true` if the node is a property descriptor.
112-
*/
113-
function isPropertyDescriptor(node) {
114-
// Object.defineProperty(obj, "foo", {set: ...})
115-
if (
116-
isArgumentOfMethodCall(node, 2, "Object", "defineProperty") ||
117-
isArgumentOfMethodCall(node, 2, "Reflect", "defineProperty")
118-
) {
119-
return true;
120-
}
121-
122-
/*
123-
* Object.defineProperties(obj, {foo: {set: ...}})
124-
* Object.create(proto, {foo: {set: ...}})
125-
*/
126-
const grandparent = node.parent.parent;
127-
128-
return (
129-
grandparent.type === "ObjectExpression" &&
130-
(isArgumentOfMethodCall(grandparent, 1, "Object", "create") ||
131-
isArgumentOfMethodCall(
132-
grandparent,
133-
1,
134-
"Object",
135-
"defineProperties",
136-
))
137-
);
138-
}
139-
14090
//------------------------------------------------------------------------------
14191
// Rule Definition
14292
//------------------------------------------------------------------------------
@@ -367,7 +317,7 @@ module.exports = {
367317
*/
368318
function checkObjectExpression(node) {
369319
checkObjectLiteral(node);
370-
if (isPropertyDescriptor(node)) {
320+
if (astUtils.isPropertyDescriptor(node, sourceCode)) {
371321
checkPropertyDescriptor(node);
372322
}
373323
}

‎lib/rules/getter-return.js‎

Lines changed: 4 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -117,52 +117,11 @@ module.exports = {
117117
if (
118118
parent.type === "Property" &&
119119
astUtils.getStaticPropertyName(parent) === "get" &&
120-
parent.parent.type === "ObjectExpression"
120+
parent.parent.type === "ObjectExpression" &&
121+
astUtils.isPropertyDescriptor(parent.parent, sourceCode)
121122
) {
122-
// Object.defineProperty() or Reflect.defineProperty()
123-
if (parent.parent.parent.type === "CallExpression") {
124-
const callNode = parent.parent.parent.callee;
125-
126-
if (
127-
astUtils.isSpecificMemberAccess(
128-
callNode,
129-
"Object",
130-
"defineProperty",
131-
) ||
132-
astUtils.isSpecificMemberAccess(
133-
callNode,
134-
"Reflect",
135-
"defineProperty",
136-
)
137-
) {
138-
return true;
139-
}
140-
}
141-
142-
// Object.defineProperties() or Object.create()
143-
if (
144-
parent.parent.parent.type === "Property" &&
145-
parent.parent.parent.parent.type ===
146-
"ObjectExpression" &&
147-
parent.parent.parent.parent.parent.type ===
148-
"CallExpression"
149-
) {
150-
const callNode =
151-
parent.parent.parent.parent.parent.callee;
152-
153-
return (
154-
astUtils.isSpecificMemberAccess(
155-
callNode,
156-
"Object",
157-
"defineProperties",
158-
) ||
159-
astUtils.isSpecificMemberAccess(
160-
callNode,
161-
"Object",
162-
"create",
163-
)
164-
);
165-
}
123+
// Getter in a property descriptor
124+
return true;
166125
}
167126
}
168127
return false;

‎lib/rules/no-setter-return.js‎

Lines changed: 1 addition & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -15,94 +15,6 @@ const astUtils = require("./utils/ast-utils");
1515
// Helpers
1616
//------------------------------------------------------------------------------
1717

18-
/**
19-
* Determines whether the given node is an argument of the specified global method call, at the given `index` position.
20-
* E.g., for given `index === 1`, this function checks for `objectName.methodName(foo, node)`, where objectName is a global variable.
21-
* @param {ASTNode} node The node to check.
22-
* @param {SourceCode} sourceCode Source code to which the node belongs.
23-
* @param {string} objectName Name of the global object.
24-
* @param {string} methodName Name of the method.
25-
* @param {number} index The given position.
26-
* @returns {boolean} `true` if the node is argument at the given position.
27-
*/
28-
function isArgumentOfGlobalMethodCall(
29-
node,
30-
sourceCode,
31-
objectName,
32-
methodName,
33-
index,
34-
) {
35-
const callNode = node.parent;
36-
37-
return (
38-
callNode.type === "CallExpression" &&
39-
callNode.arguments[index] === node &&
40-
astUtils.isSpecificMemberAccess(
41-
callNode.callee,
42-
objectName,
43-
methodName,
44-
) &&
45-
sourceCode.isGlobalReference(
46-
astUtils.skipChainExpression(callNode.callee).object,
47-
)
48-
);
49-
}
50-
51-
/**
52-
* Determines whether the given node is used as a property descriptor.
53-
* @param {ASTNode} node The node to check.
54-
* @param {SourceCode} sourceCode Source code to which the node belongs.
55-
* @returns {boolean} `true` if the node is a property descriptor.
56-
*/
57-
function isPropertyDescriptor(node, sourceCode) {
58-
if (
59-
isArgumentOfGlobalMethodCall(
60-
node,
61-
sourceCode,
62-
"Object",
63-
"defineProperty",
64-
2,
65-
) ||
66-
isArgumentOfGlobalMethodCall(
67-
node,
68-
sourceCode,
69-
"Reflect",
70-
"defineProperty",
71-
2,
72-
)
73-
) {
74-
return true;
75-
}
76-
77-
const parent = node.parent;
78-
79-
if (parent.type === "Property" && parent.value === node) {
80-
const grandparent = parent.parent;
81-
82-
if (
83-
grandparent.type === "ObjectExpression" &&
84-
(isArgumentOfGlobalMethodCall(
85-
grandparent,
86-
sourceCode,
87-
"Object",
88-
"create",
89-
1,
90-
) ||
91-
isArgumentOfGlobalMethodCall(
92-
grandparent,
93-
sourceCode,
94-
"Object",
95-
"defineProperties",
96-
1,
97-
))
98-
) {
99-
return true;
100-
}
101-
}
102-
103-
return false;
104-
}
105-
10618
/**
10719
* Determines whether the given function node is used as a setter function.
10820
* @param {ASTNode} node The node to check.
@@ -126,7 +38,7 @@ function isSetter(node, sourceCode) {
12638
parent.value === node &&
12739
astUtils.getStaticPropertyName(parent) === "set" &&
12840
parent.parent.type === "ObjectExpression" &&
129-
isPropertyDescriptor(parent.parent, sourceCode)
41+
astUtils.isPropertyDescriptor(parent.parent, sourceCode)
13042
) {
13143
// Setter in a property descriptor
13244
return true;

‎lib/rules/utils/ast-utils.js‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,91 @@ function isMethodWhichHasThisArg(node) {
551551
return isSpecificMemberAccess(node, null, arrayMethodWithThisArgPattern);
552552
}
553553

554+
/**
555+
* Determines whether the given node is an argument of the specified global method call, at the given `index` position.
556+
* E.g., for given `index === 1`, this function checks for `objectName.methodName(foo, node)`, where objectName is a global variable.
557+
* @param {ASTNode} node The node to check.
558+
* @param {SourceCode} sourceCode Source code to which the node belongs.
559+
* @param {string} objectName Name of the global object.
560+
* @param {string} methodName Name of the method.
561+
* @param {number} index The given position.
562+
* @returns {boolean} `true` if the node is argument at the given position.
563+
* @private
564+
*/
565+
function isArgumentOfGlobalMethodCall(
566+
node,
567+
sourceCode,
568+
objectName,
569+
methodName,
570+
index,
571+
) {
572+
const callNode = node.parent;
573+
574+
return (
575+
callNode.type === "CallExpression" &&
576+
callNode.arguments[index] === node &&
577+
isSpecificMemberAccess(callNode.callee, objectName, methodName) &&
578+
sourceCode.isGlobalReference(
579+
skipChainExpression(callNode.callee).object,
580+
)
581+
);
582+
}
583+
584+
/**
585+
* Determines whether the given node is used as a property descriptor.
586+
* @param {ASTNode} node The node to check.
587+
* @param {SourceCode} sourceCode Source code to which the node belongs.
588+
* @returns {boolean} `true` if the node is a property descriptor.
589+
*/
590+
function isPropertyDescriptor(node, sourceCode) {
591+
if (
592+
isArgumentOfGlobalMethodCall(
593+
node,
594+
sourceCode,
595+
"Object",
596+
"defineProperty",
597+
2,
598+
) ||
599+
isArgumentOfGlobalMethodCall(
600+
node,
601+
sourceCode,
602+
"Reflect",
603+
"defineProperty",
604+
2,
605+
)
606+
) {
607+
return true;
608+
}
609+
610+
const parent = node.parent;
611+
612+
if (parent.type === "Property" && parent.value === node) {
613+
const grandparent = parent.parent;
614+
615+
if (
616+
grandparent.type === "ObjectExpression" &&
617+
(isArgumentOfGlobalMethodCall(
618+
grandparent,
619+
sourceCode,
620+
"Object",
621+
"create",
622+
1,
623+
) ||
624+
isArgumentOfGlobalMethodCall(
625+
grandparent,
626+
sourceCode,
627+
"Object",
628+
"defineProperties",
629+
1,
630+
))
631+
) {
632+
return true;
633+
}
634+
}
635+
636+
return false;
637+
}
638+
554639
/**
555640
* Creates the negate function of the given function.
556641
* @param {Function} f The function to negate.
@@ -1510,6 +1595,7 @@ module.exports = {
15101595
isInLoop,
15111596
isArrayFromMethod,
15121597
isArrayFromAsyncMethod,
1598+
isPropertyDescriptor,
15131599
isParenthesised,
15141600
createGlobalLinebreakMatcher,
15151601
equalTokens,

‎tests/lib/rules/accessor-pairs.js‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,46 @@ ruleTester.run("accessor-pairs", rule, {
333333
languageOptions: { ecmaVersion: 6 },
334334
},
335335

336+
// wrong argument index (not in the property descriptor position)
337+
"Object.defineProperty({ set: function(value) {} }, 'foo', { value: 1 });",
338+
{
339+
code: "Reflect.defineProperty({ get() {} }, 'foo', { value: 1 });",
340+
options: [{ getWithoutSet: true }],
341+
languageOptions: { ecmaVersion: 6 },
342+
},
343+
{
344+
code: "Object.defineProperties({ foo: { get() {} } }, { bar: { value: 1 } });",
345+
options: [{ getWithoutSet: true }],
346+
},
347+
"Object.create({ foo: { set(value) {} } }, { bar: { value: 1 } });",
348+
349+
// global object is shadowed
350+
{
351+
code: "let Object; Object.defineProperty(foo, 'bar', { get() {} })",
352+
options: [{ getWithoutSet: true }],
353+
},
354+
{
355+
code: "function f() { Reflect.defineProperty(foo, 'bar', { set(value) {} }); var Reflect;}",
356+
languageOptions: { ecmaVersion: 6 },
357+
},
358+
"function f(Object) { Object.defineProperties(foo, { bar: { set(value) {} } }) }",
359+
{
360+
code: "if (x) { const Object = getObject(); Object.create(foo, { bar: { get() {} } }) }",
361+
options: [{ getWithoutSet: true }],
362+
},
363+
364+
// global object doesn't exist
365+
{
366+
code: "Reflect.defineProperty(foo, 'bar', { get() {} })",
367+
options: [{ getWithoutSet: true }],
368+
languageOptions: { globals: { Reflect: "off" } },
369+
},
370+
"/* globals Object:off */ Object.defineProperty(foo, 'bar', { set(value) {} })",
371+
{
372+
code: "Object.defineProperties(foo, { bar: { set(value) {} } })",
373+
languageOptions: { globals: { Object: "off" } },
374+
},
375+
336376
//------------------------------------------------------------------------------
337377
// Classes
338378
//------------------------------------------------------------------------------

0 commit comments

Comments
 (0)