Skip to content

Commit 92b5671

Browse files
axetroysindresorhusfisker
authored
Add no-accessor-recursion rule (#2525)
Co-authored-by: Sindre Sorhus <[email protected]> Co-authored-by: fisker <[email protected]>
1 parent fed0ccf commit 92b5671

8 files changed

+1036
-0
lines changed

‎docs/rules/no-accessor-recursion.md

+74
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
# Disallow recursive access to `this` within getters and setters
2+
3+
💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#preset-configs-eslintconfigjs).
4+
5+
<!-- end auto-generated rule header -->
6+
<!-- Do not manually modify this header. Run: `npm run fix:eslint-docs` -->
7+
8+
This rule prevents recursive access to `this` within getter and setter methods in objects and classes, avoiding infinite recursion and stack overflow errors.
9+
10+
## Examples
11+
12+
```js
13+
// ❌
14+
const foo = {
15+
get bar() {
16+
return this.bar;
17+
}
18+
};
19+
20+
// ✅
21+
const foo = {
22+
get bar() {
23+
return this.baz;
24+
}
25+
};
26+
```
27+
28+
```js
29+
// ❌
30+
class Foo {
31+
get bar() {
32+
return this.bar;
33+
}
34+
}
35+
36+
// ✅
37+
class Foo {
38+
get bar() {
39+
return this.baz;
40+
}
41+
}
42+
```
43+
44+
```js
45+
// ❌
46+
const foo = {
47+
set bar(value) {
48+
this.bar = value;
49+
}
50+
};
51+
52+
// ✅
53+
const foo = {
54+
set bar(value) {
55+
this._bar = value;
56+
}
57+
};
58+
```
59+
60+
```js
61+
// ❌
62+
class Foo {
63+
set bar(value) {
64+
this.bar = value;
65+
}
66+
}
67+
68+
// ✅
69+
class Foo {
70+
set bar(value) {
71+
this._bar = value;
72+
}
73+
}
74+
```

‎readme.md

+1
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ export default [
7373
| [import-style](docs/rules/import-style.md) | Enforce specific import styles per module. | ✅ | | |
7474
| [new-for-builtins](docs/rules/new-for-builtins.md) | Enforce the use of `new` for all builtins, except `String`, `Number`, `Boolean`, `Symbol` and `BigInt`. | ✅ | 🔧 | |
7575
| [no-abusive-eslint-disable](docs/rules/no-abusive-eslint-disable.md) | Enforce specifying rules to disable in `eslint-disable` comments. | ✅ | | |
76+
| [no-accessor-recursion](docs/rules/no-accessor-recursion.md) | Disallow recursive access to `this` within getters and setters. | ✅ | | |
7677
| [no-anonymous-default-export](docs/rules/no-anonymous-default-export.md) | Disallow anonymous functions and classes as the default export. | ✅ | | 💡 |
7778
| [no-array-callback-reference](docs/rules/no-array-callback-reference.md) | Prevent passing a function reference directly to iterator methods. | ✅ | | 💡 |
7879
| [no-array-for-each](docs/rules/no-array-for-each.md) | Prefer `for…of` over the `forEach` method. | ✅ | 🔧 | 💡 |

‎rules/index.js

+2
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import filenameCase from './filename-case.js';
1818
import importStyle from './import-style.js';
1919
import newForBuiltins from './new-for-builtins.js';
2020
import noAbusiveEslintDisable from './no-abusive-eslint-disable.js';
21+
import noAccessorRecursion from './no-accessor-recursion.js';
2122
import noAnonymousDefaultExport from './no-anonymous-default-export.js';
2223
import noArrayCallbackReference from './no-array-callback-reference.js';
2324
import noArrayForEach from './no-array-for-each.js';
@@ -144,6 +145,7 @@ const rules = {
144145
'import-style': createRule(importStyle, 'import-style'),
145146
'new-for-builtins': createRule(newForBuiltins, 'new-for-builtins'),
146147
'no-abusive-eslint-disable': createRule(noAbusiveEslintDisable, 'no-abusive-eslint-disable'),
148+
'no-accessor-recursion': createRule(noAccessorRecursion, 'no-accessor-recursion'),
147149
'no-anonymous-default-export': createRule(noAnonymousDefaultExport, 'no-anonymous-default-export'),
148150
'no-array-callback-reference': createRule(noArrayCallbackReference, 'no-array-callback-reference'),
149151
'no-array-for-each': createRule(noArrayForEach, 'no-array-for-each'),

‎rules/no-accessor-recursion.js

+160
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
const MESSAGE_ID_ERROR = 'no-accessor-recursion/error';
2+
const messages = {
3+
[MESSAGE_ID_ERROR]: 'Disallow recursive access to `this` within {{kind}}ters.',
4+
};
5+
6+
/**
7+
Get the closest non-arrow function scope.
8+
9+
@param {import('eslint').SourceCode} sourceCode
10+
@param {import('estree').Node} node
11+
@return {import('eslint').Scope.Scope | undefined}
12+
*/
13+
const getClosestFunctionScope = (sourceCode, node) => {
14+
for (let scope = sourceCode.getScope(node); scope; scope = scope.upper) {
15+
if (scope.type === 'class') {
16+
return;
17+
}
18+
19+
if (scope.type === 'function' && scope.block.type !== 'ArrowFunctionExpression') {
20+
return scope;
21+
}
22+
}
23+
};
24+
25+
/** @param {import('estree').Identifier | import('estree').PrivateIdentifier} node */
26+
const isIdentifier = node => node.type === 'Identifier' || node.type === 'PrivateIdentifier';
27+
28+
/** @param {import('estree').ThisExpression} node */
29+
const isDotNotationAccess = node =>
30+
node.parent.type === 'MemberExpression'
31+
&& node.parent.object === node
32+
&& !node.parent.computed
33+
&& isIdentifier(node.parent.property);
34+
35+
/**
36+
Check if a property is a valid getter or setter.
37+
38+
@param {import('estree').Property | import('estree').MethodDefinition} property
39+
*/
40+
const isValidProperty = property =>
41+
['Property', 'MethodDefinition'].includes(property.type)
42+
&& !property.computed
43+
&& ['set', 'get'].includes(property.kind)
44+
&& isIdentifier(property.key);
45+
46+
/**
47+
Check if two property keys are the same.
48+
49+
@param {import('estree').Property['key']} keyLeft
50+
@param {import('estree').Property['key']} keyRight
51+
*/
52+
const isSameKey = (keyLeft, keyRight) => ['type', 'name'].every(key => keyLeft[key] === keyRight[key]);
53+
54+
/**
55+
Check if `this` is accessed recursively within a getter or setter.
56+
57+
@param {import('estree').ThisExpression} node
58+
@param {import('estree').Property | import('estree').MethodDefinition} property
59+
*/
60+
const isMemberAccess = (node, property) =>
61+
isDotNotationAccess(node)
62+
&& isSameKey(node.parent.property, property.key);
63+
64+
/**
65+
Check if `this` is accessed recursively within a destructuring assignment.
66+
67+
@param {import('estree').ThisExpression} node
68+
@param {import('estree').Property | import('estree').MethodDefinition} property
69+
*/
70+
const isRecursiveDestructuringAccess = (node, property) =>
71+
node.parent.type === 'VariableDeclarator'
72+
&& node.parent.init === node
73+
&& node.parent.id.type === 'ObjectPattern'
74+
&& node.parent.id.properties.some(declaratorProperty =>
75+
declaratorProperty.type === 'Property'
76+
&& !declaratorProperty.computed
77+
&& isSameKey(declaratorProperty.key, property.key),
78+
);
79+
80+
const isPropertyRead = (thisExpression, property) =>
81+
isMemberAccess(thisExpression, property)
82+
|| isRecursiveDestructuringAccess(thisExpression, property);
83+
84+
const isPropertyWrite = (thisExpression, property) => {
85+
if (!isMemberAccess(thisExpression, property)) {
86+
return false;
87+
}
88+
89+
const memberExpression = thisExpression.parent;
90+
const {parent} = memberExpression;
91+
92+
// This part is similar to `isLeftHandSide`, try to DRY in future
93+
return (
94+
// `this.foo = …`
95+
// `[this.foo = …] = …`
96+
// `({property: this.foo = …] = …)`
97+
(
98+
(parent.type === 'AssignmentExpression' || parent.type === 'AssignmentPattern')
99+
&& parent.left === memberExpression
100+
)
101+
// `++ this.foo`
102+
|| (parent.type === 'UpdateExpression' && parent.argument === memberExpression)
103+
// `[this.foo] = …`
104+
|| (parent.type === 'ArrayPattern' && parent.elements.includes(memberExpression))
105+
// `({property: this.foo} = …)`
106+
|| (
107+
parent.type === 'Property'
108+
&& parent.value === memberExpression
109+
&& parent.parent.type === 'ObjectPattern'
110+
&& parent.parent.properties.includes(memberExpression.parent)
111+
)
112+
);
113+
};
114+
115+
/** @param {import('eslint').Rule.RuleContext} context */
116+
const create = context => {
117+
const {sourceCode} = context;
118+
119+
return {
120+
/** @param {import('estree').ThisExpression} thisExpression */
121+
ThisExpression(thisExpression) {
122+
const scope = getClosestFunctionScope(sourceCode, thisExpression);
123+
124+
if (!scope) {
125+
return;
126+
}
127+
128+
/** @type {import('estree').Property | import('estree').MethodDefinition} */
129+
const property = scope.block.parent;
130+
131+
if (!isValidProperty(property)) {
132+
return;
133+
}
134+
135+
if (property.kind === 'get' && isPropertyRead(thisExpression, property)) {
136+
return {node: thisExpression.parent, messageId: MESSAGE_ID_ERROR, data: {kind: property.kind}};
137+
}
138+
139+
if (property.kind === 'set' && isPropertyWrite(thisExpression, property)) {
140+
return {node: thisExpression.parent, messageId: MESSAGE_ID_ERROR, data: {kind: property.kind}};
141+
}
142+
},
143+
};
144+
};
145+
146+
/** @type {import('eslint').Rule.RuleModule} */
147+
const config = {
148+
create,
149+
meta: {
150+
type: 'problem',
151+
docs: {
152+
description: 'Disallow recursive access to `this` within getters and setters.',
153+
recommended: true,
154+
},
155+
defaultOptions: [],
156+
messages,
157+
},
158+
};
159+
160+
export default config;

0 commit comments

Comments
 (0)