Skip to content

Commit 45aa6a3

Browse files
mdjermanovicilyavolodin
mdjermanovic
authored and
ilyavolodin
committed
New: Add no-setter-return rule (fixes #12285) (#12346)
1 parent 0afb518 commit 45aa6a3

File tree

5 files changed

+840
-0
lines changed

5 files changed

+840
-0
lines changed

‎docs/rules/no-setter-return.md‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
# Disallow returning values from setters (no-setter-return)
2+
3+
Setters cannot return values.
4+
5+
While returning a value from a setter does not produce an error, the returned value is being ignored. Therefore, returning a value from a setter is either unnecessary or a possible error, since the returned value cannot be used.
6+
7+
## Rule Details
8+
9+
This rule disallows returning values from setters and reports `return` statements in setter functions.
10+
11+
Only `return` without a value is allowed, as it's a control flow statement.
12+
13+
This rule checks setters in:
14+
15+
* Object literals.
16+
* Class declarations and class expressions.
17+
* Property descriptors in `Object.create`, `Object.defineProperty`, `Object.defineProperties`, and `Reflect.defineProperty` methods of the global objects.
18+
19+
Examples of **incorrect** code for this rule:
20+
21+
```js
22+
/*eslint no-setter-return: "error"*/
23+
24+
var foo = {
25+
set a(value) {
26+
this.val = value;
27+
return value;
28+
}
29+
};
30+
31+
class Foo {
32+
set a(value) {
33+
this.val = value * 2;
34+
return this.val;
35+
}
36+
}
37+
38+
const Bar = class {
39+
static set a(value) {
40+
if (value < 0) {
41+
this.val = 0;
42+
return 0;
43+
}
44+
this.val = value;
45+
}
46+
};
47+
48+
Object.defineProperty(foo, "bar", {
49+
set(value) {
50+
if (value < 0) {
51+
return false;
52+
}
53+
this.val = value;
54+
}
55+
});
56+
```
57+
58+
Examples of **correct** code for this rule:
59+
60+
```js
61+
/*eslint no-setter-return: "error"*/
62+
63+
var foo = {
64+
set a(value) {
65+
this.val = value;
66+
}
67+
};
68+
69+
class Foo {
70+
set a(value) {
71+
this.val = value * 2;
72+
}
73+
}
74+
75+
const Bar = class {
76+
static set a(value) {
77+
if (value < 0) {
78+
this.val = 0;
79+
return;
80+
}
81+
this.val = value;
82+
}
83+
};
84+
85+
Object.defineProperty(foo, "bar", {
86+
set(value) {
87+
if (value < 0) {
88+
throw new Error("Negative value.");
89+
}
90+
this.val = value;
91+
}
92+
});
93+
```
94+
95+
## Further Reading
96+
97+
* [MDN setter](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/set)
98+
99+
## Related Rules
100+
101+
* [getter-return](getter-return.md)

‎lib/rules/index.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ module.exports = new LazyLoadingRuleMap(Object.entries({
186186
"no-self-assign": () => require("./no-self-assign"),
187187
"no-self-compare": () => require("./no-self-compare"),
188188
"no-sequences": () => require("./no-sequences"),
189+
"no-setter-return": () => require("./no-setter-return"),
189190
"no-shadow": () => require("./no-shadow"),
190191
"no-shadow-restricted-names": () => require("./no-shadow-restricted-names"),
191192
"no-spaced-func": () => require("./no-spaced-func"),

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

Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
1+
/**
2+
* @fileoverview Rule to disallow returning values from setters
3+
* @author Milos Djermanovic
4+
*/
5+
6+
"use strict";
7+
8+
//------------------------------------------------------------------------------
9+
// Requirements
10+
//------------------------------------------------------------------------------
11+
12+
const astUtils = require("./utils/ast-utils");
13+
const { findVariable } = require("eslint-utils");
14+
15+
//------------------------------------------------------------------------------
16+
// Helpers
17+
//------------------------------------------------------------------------------
18+
19+
/**
20+
* Determines whether the given identifier node is a reference to a global variable.
21+
* @param {ASTNode} node `Identifier` node to check.
22+
* @param {Scope} scope Scope to which the node belongs.
23+
* @returns {boolean} True if the identifier is a reference to a global variable.
24+
*/
25+
function isGlobalReference(node, scope) {
26+
const variable = findVariable(scope, node);
27+
28+
return variable !== null && variable.scope.type === "global" && variable.defs.length === 0;
29+
}
30+
31+
/**
32+
* Determines whether the given node is an argument of the specified global method call, at the given `index` position.
33+
* E.g., for given `index === 1`, this function checks for `objectName.methodName(foo, node)`, where objectName is a global variable.
34+
* @param {ASTNode} node The node to check.
35+
* @param {Scope} scope Scope to which the node belongs.
36+
* @param {string} objectName Name of the global object.
37+
* @param {string} methodName Name of the method.
38+
* @param {number} index The given position.
39+
* @returns {boolean} `true` if the node is argument at the given position.
40+
*/
41+
function isArgumentOfGlobalMethodCall(node, scope, objectName, methodName, index) {
42+
const parent = node.parent;
43+
44+
return parent.type === "CallExpression" &&
45+
parent.arguments[index] === node &&
46+
parent.callee.type === "MemberExpression" &&
47+
astUtils.getStaticPropertyName(parent.callee) === methodName &&
48+
parent.callee.object.type === "Identifier" &&
49+
parent.callee.object.name === objectName &&
50+
isGlobalReference(parent.callee.object, scope);
51+
}
52+
53+
/**
54+
* Determines whether the given node is used as a property descriptor.
55+
* @param {ASTNode} node The node to check.
56+
* @param {Scope} scope Scope to which the node belongs.
57+
* @returns {boolean} `true` if the node is a property descriptor.
58+
*/
59+
function isPropertyDescriptor(node, scope) {
60+
if (
61+
isArgumentOfGlobalMethodCall(node, scope, "Object", "defineProperty", 2) ||
62+
isArgumentOfGlobalMethodCall(node, scope, "Reflect", "defineProperty", 2)
63+
) {
64+
return true;
65+
}
66+
67+
const parent = node.parent;
68+
69+
if (
70+
parent.type === "Property" &&
71+
parent.value === node
72+
) {
73+
const grandparent = parent.parent;
74+
75+
if (
76+
grandparent.type === "ObjectExpression" &&
77+
(
78+
isArgumentOfGlobalMethodCall(grandparent, scope, "Object", "create", 1) ||
79+
isArgumentOfGlobalMethodCall(grandparent, scope, "Object", "defineProperties", 1)
80+
)
81+
) {
82+
return true;
83+
}
84+
}
85+
86+
return false;
87+
}
88+
89+
/**
90+
* Determines whether the given function node is used as a setter function.
91+
* @param {ASTNode} node The node to check.
92+
* @param {Scope} scope Scope to which the node belongs.
93+
* @returns {boolean} `true` if the node is a setter.
94+
*/
95+
function isSetter(node, scope) {
96+
const parent = node.parent;
97+
98+
if (
99+
parent.kind === "set" &&
100+
parent.value === node
101+
) {
102+
103+
// Setter in an object literal or in a class
104+
return true;
105+
}
106+
107+
if (
108+
parent.type === "Property" &&
109+
parent.value === node &&
110+
astUtils.getStaticPropertyName(parent) === "set" &&
111+
parent.parent.type === "ObjectExpression" &&
112+
isPropertyDescriptor(parent.parent, scope)
113+
) {
114+
115+
// Setter in a property descriptor
116+
return true;
117+
}
118+
119+
return false;
120+
}
121+
122+
/**
123+
* Finds function's outer scope.
124+
* @param {Scope} scope Function's own scope.
125+
* @returns {Scope} Function's outer scope.
126+
*/
127+
function getOuterScope(scope) {
128+
const upper = scope.upper;
129+
130+
if (upper.type === "function-expression-name") {
131+
return upper.upper;
132+
}
133+
134+
return upper;
135+
}
136+
137+
//------------------------------------------------------------------------------
138+
// Rule Definition
139+
//------------------------------------------------------------------------------
140+
141+
module.exports = {
142+
meta: {
143+
type: "problem",
144+
145+
docs: {
146+
description: "disallow returning values from setters",
147+
category: "Possible Errors",
148+
recommended: false,
149+
url: "https://eslint.org/docs/rules/no-setter-return"
150+
},
151+
152+
schema: [],
153+
154+
messages: {
155+
returnsValue: "Setter cannot return a value."
156+
}
157+
},
158+
159+
create(context) {
160+
let funcInfo = null;
161+
162+
/**
163+
* Creates and pushes to the stack a function info object for the given function node.
164+
* @param {ASTNode} node The function node.
165+
* @returns {void}
166+
*/
167+
function enterFunction(node) {
168+
const outerScope = getOuterScope(context.getScope());
169+
170+
funcInfo = {
171+
upper: funcInfo,
172+
isSetter: isSetter(node, outerScope)
173+
};
174+
}
175+
176+
/**
177+
* Pops the current function info object from the stack.
178+
* @returns {void}
179+
*/
180+
function exitFunction() {
181+
funcInfo = funcInfo.upper;
182+
}
183+
184+
/**
185+
* Reports the given node.
186+
* @param {ASTNode} node Node to report.
187+
* @returns {void}
188+
*/
189+
function report(node) {
190+
context.report({ node, messageId: "returnsValue" });
191+
}
192+
193+
return {
194+
195+
/*
196+
* Function declarations cannot be setters, but we still have to track them in the `funcInfo` stack to avoid
197+
* false positives, because a ReturnStatement node can belong to a function declaration inside a setter.
198+
*
199+
* Note: A previously declared function can be referenced and actually used as a setter in a property descriptor,
200+
* but that's out of scope for this rule.
201+
*/
202+
FunctionDeclaration: enterFunction,
203+
FunctionExpression: enterFunction,
204+
ArrowFunctionExpression(node) {
205+
enterFunction(node);
206+
207+
if (funcInfo.isSetter && node.expression) {
208+
209+
// { set: foo => bar } property descriptor. Report implicit return 'bar' as the equivalent for a return statement.
210+
report(node.body);
211+
}
212+
},
213+
214+
"FunctionDeclaration:exit": exitFunction,
215+
"FunctionExpression:exit": exitFunction,
216+
"ArrowFunctionExpression:exit": exitFunction,
217+
218+
ReturnStatement(node) {
219+
220+
// Global returns (e.g., at the top level of a Node module) don't have `funcInfo`.
221+
if (funcInfo && funcInfo.isSetter && node.argument) {
222+
report(node);
223+
}
224+
}
225+
};
226+
}
227+
};

0 commit comments

Comments
 (0)