Skip to content

Add no-accessor-recursion rule #2525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 59 commits into from
Jan 24, 2025
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
59 commits
Select commit Hold shift + click to select a range
33155ab
Add `no-accessor-recursion` rule
axetroy Jan 15, 2025
fc62f17
update docs
axetroy Jan 16, 2025
e67c4d0
ignore no-accessor-recursion
axetroy Jan 16, 2025
664998d
update docs
axetroy Jan 16, 2025
eb7cfa8
update test case
axetroy Jan 16, 2025
3335bbe
add more test
axetroy Jan 16, 2025
2bc1ffb
add test
axetroy Jan 16, 2025
a8e0195
support nest scope
axetroy Jan 16, 2025
c3adada
add test
axetroy Jan 16, 2025
2ce59e2
support detect nest scope in array function
axetroy Jan 16, 2025
a519d44
add more test case
axetroy Jan 18, 2025
08a09e8
Update no-accessor-recursion.md
sindresorhus Jan 18, 2025
35bc0ce
Update no-accessor-recursion.js
sindresorhus Jan 18, 2025
6d3a3a1
update description
axetroy Jan 18, 2025
1e45d9b
update test
axetroy Jan 18, 2025
54d8ab7
add more test
axetroy Jan 18, 2025
cd67a7c
rename mjs to js
axetroy Jan 20, 2025
cccc72c
Merge branch 'main' into no_accessor_recursion
sindresorhus Jan 20, 2025
c936b88
Update no-accessor-recursion.js
sindresorhus Jan 20, 2025
91eace3
update
axetroy Jan 20, 2025
354c0c6
fix lint
axetroy Jan 20, 2025
b8b7390
remove snapshot
axetroy Jan 20, 2025
610f2b0
update jsdoc
axetroy Jan 20, 2025
28ab834
fix computed accessor
axetroy Jan 21, 2025
97d2285
update problem report
axetroy Jan 21, 2025
1e12f24
simplify the selector
axetroy Jan 21, 2025
b619216
simplify the condition branch with early return
axetroy Jan 21, 2025
823cf60
add more test
axetroy Jan 21, 2025
72e47c8
move some logic to the top for performance
axetroy Jan 21, 2025
baa6a7d
fix setter checker when node in the right of AssignmentExpression
axetroy Jan 21, 2025
d79aa76
simplify the node selector
axetroy Jan 21, 2025
d96eead
simplify the code to reduce complexity
axetroy Jan 21, 2025
eff4ae3
fix some edge case
axetroy Jan 21, 2025
675722f
simplify the code
axetroy Jan 21, 2025
d0e7629
simplify the code
axetroy Jan 21, 2025
684baf2
simplify the code, remove the selector
axetroy Jan 21, 2025
090a434
Improve `getClosestFunctionScope`
fisker Jan 21, 2025
818e194
Add `isAccessor` and check missing `computed`
fisker Jan 21, 2025
f699a4c
feat: support private field
axetroy Jan 21, 2025
4dc5489
add static method test case
axetroy Jan 21, 2025
7b516a2
update jsdoc
axetroy Jan 21, 2025
9351864
fix false positive for private field
axetroy Jan 21, 2025
2b66ed8
support detect static method
axetroy Jan 21, 2025
be11037
Merge branch 'main' into no_accessor_recursion
axetroy Jan 21, 2025
8a73ee2
support destructuring assignment within getter
axetroy Jan 21, 2025
58b2058
add more test case
axetroy Jan 21, 2025
9b355f4
simplify the code
axetroy Jan 21, 2025
8ff5bbd
add more test case
axetroy Jan 21, 2025
ef9b2cb
add test for destructuring assignment with computed property
axetroy Jan 21, 2025
689c49a
early return for performance
axetroy Jan 22, 2025
3e3f740
The params of isSameKey should be camelCase
axetroy Jan 22, 2025
a17cef9
Handle more cases
fisker Jan 22, 2025
c6e2dfd
More specific kind
fisker Jan 22, 2025
75542e0
Tweak report location
fisker Jan 22, 2025
abd9d62
Add test for private destructuring
fisker Jan 22, 2025
aeb880e
Update test
fisker Jan 22, 2025
810c31f
Add test for class static block
axetroy Jan 22, 2025
bf1ac9d
Another case
fisker Jan 22, 2025
f7b7013
Merge branch 'main' into no_accessor_recursion
axetroy Jan 24, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions docs/rules/no-accessor-recursion.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# Disallow recursive access to `this` within getters and setters

💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#preset-configs-eslintconfigjs).

<!-- end auto-generated rule header -->
<!-- Do not manually modify this header. Run: `npm run fix:eslint-docs` -->

This rule prevents recursive access to `this` within getter and setter methods in objects and classes, avoiding infinite recursion and stack overflow errors.

## Examples

```js
// ❌
const foo = {
get bar() {
return this.bar;
}
};

// ✅
const foo = {
get bar() {
return this.baz;
}
};
```

```js
// ❌
class Foo {
get bar() {
return this.bar;
}
}

// ✅
class Foo {
get bar() {
return this.baz;
}
}
```

```js
// ❌
const foo = {
set bar(value) {
this.bar = value;
}
};

// ✅
const foo = {
set bar(value) {
this._bar = value;
}
};
```

```js
// ❌
class Foo {
set bar(value) {
this.bar = value;
}
}

// ✅
class Foo {
set bar(value) {
this._bar = value;
}
}
```
1 change: 1 addition & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export default [
| [import-style](docs/rules/import-style.md) | Enforce specific import styles per module. | ✅ | | |
| [new-for-builtins](docs/rules/new-for-builtins.md) | Enforce the use of `new` for all builtins, except `String`, `Number`, `Boolean`, `Symbol` and `BigInt`. | ✅ | 🔧 | |
| [no-abusive-eslint-disable](docs/rules/no-abusive-eslint-disable.md) | Enforce specifying rules to disable in `eslint-disable` comments. | ✅ | | |
| [no-accessor-recursion](docs/rules/no-accessor-recursion.md) | Disallow recursive access to `this` within getters and setters. | ✅ | | |
| [no-anonymous-default-export](docs/rules/no-anonymous-default-export.md) | Disallow anonymous functions and classes as the default export. | ✅ | | 💡 |
| [no-array-callback-reference](docs/rules/no-array-callback-reference.md) | Prevent passing a function reference directly to iterator methods. | ✅ | | 💡 |
| [no-array-for-each](docs/rules/no-array-for-each.md) | Prefer `for…of` over the `forEach` method. | ✅ | 🔧 | 💡 |
Expand Down
2 changes: 2 additions & 0 deletions rules/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import filenameCase from './filename-case.js';
import importStyle from './import-style.js';
import newForBuiltins from './new-for-builtins.js';
import noAbusiveEslintDisable from './no-abusive-eslint-disable.js';
import noAccessorRecursion from './no-accessor-recursion.js';
import noAnonymousDefaultExport from './no-anonymous-default-export.js';
import noArrayCallbackReference from './no-array-callback-reference.js';
import noArrayForEach from './no-array-for-each.js';
Expand Down Expand Up @@ -142,6 +143,7 @@ const rules = {
'import-style': createRule(importStyle, 'import-style'),
'new-for-builtins': createRule(newForBuiltins, 'new-for-builtins'),
'no-abusive-eslint-disable': createRule(noAbusiveEslintDisable, 'no-abusive-eslint-disable'),
'no-accessor-recursion': createRule(noAccessorRecursion, 'no-accessor-recursion'),
'no-anonymous-default-export': createRule(noAnonymousDefaultExport, 'no-anonymous-default-export'),
'no-array-callback-reference': createRule(noArrayCallbackReference, 'no-array-callback-reference'),
'no-array-for-each': createRule(noArrayForEach, 'no-array-for-each'),
Expand Down
99 changes: 99 additions & 0 deletions rules/no-accessor-recursion.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
const MESSAGE_ID_ERROR = 'no-accessor-recursion/error';
const messages = {
[MESSAGE_ID_ERROR]: 'Disallow recursive access to `this` within getters and setters.',
};

/** @param {import('eslint').Scope.Scope} scope */
const isArrowFunctionScope = scope => scope.type === 'function' && scope.block.type === 'ArrowFunctionExpression';

/** @param {import('eslint').Rule.RuleContext} context */
const create = context => {
const functionExpressionStack = [];

return {
// Getter for object literal
'Property[value.type="FunctionExpression"], [kind="get"]'(node) {
functionExpressionStack.push(node);
},
'Property[value.type="FunctionExpression"], [kind="get"]:exit'() {
functionExpressionStack.pop();
},
// Setter for object literal
'Property[value.type="FunctionExpression"], [kind="set"]'(node) {
functionExpressionStack.push(node);
},
'Property[value.type="FunctionExpression"], [kind="set"]:exit'() {
functionExpressionStack.pop();
},
// Getter for class
'MethodDefinition[kind="get"]'(node) {
functionExpressionStack.push(node);
},
'MethodDefinition[kind="get"]:exit'() {
functionExpressionStack.pop();
},
// Setter for class
'MethodDefinition[kind="set"]'(node) {
functionExpressionStack.push(node);
},
'MethodDefinition[kind="set"]:exit'() {
functionExpressionStack.pop();
},
ThisExpression(node) {
if (functionExpressionStack.length > 0) {
/** @type {import('estree').Property | import('estree').MethodDefinition} */
const property = functionExpressionStack.at(-1);

let scope = context.sourceCode.getScope(node);

while (scope.type !== 'function' || isArrowFunctionScope(scope)) {
scope = scope.upper;
}

// Check if `this` is in the current function expression scope
if (scope.block === property.value) {
/** @type {import('estree').MemberExpression} */
const {parent} = node;

const isThisAccessed = () => parent.type === 'MemberExpression' && parent.property.type === 'Identifier' && parent.property.name === property.key.name;

switch (property.kind) {
case 'get': {
if (isThisAccessed()) {
context.report({node: parent, messageId: MESSAGE_ID_ERROR});
}

break;
}

case 'set': {
if (isThisAccessed() && parent.parent.type === 'AssignmentExpression') {
Copy link
Collaborator

@fisker fisker Jan 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need make sure the memberExpression is on left

a = this.foo

context.report({node: parent.parent, messageId: MESSAGE_ID_ERROR});
}

break;
}

default:
}
}
}
},
};
};

/** @type {import('eslint').Rule.RuleModule} */
const config = {
create,
meta: {
type: 'problem',
docs: {
description: 'Disallow recursive access to `this` within getters and setters.',
recommended: true,
},
defaultOptions: [],
messages,
},
};

export default config;
161 changes: 161 additions & 0 deletions test/no-accessor-recursion.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
import outdent from 'outdent';
import {getTester} from './utils/test.js';

const {test} = getTester(import.meta);

test.snapshot({
valid: [
'function foo () { this.bar }',
'function foo () { this.foo }',
'function foo (value) { this.bar = value }',
'this.foo = foo',
outdent`
{
this.foo = foo;
}
`,
'this.foo = function () { this.foo }',
'const foo = () => this.foo',
outdent`
const foo = {
bar() {
this.bar = void 0;
return this.bar;
}
};
`,
outdent`
class Foo {
foo() {
this.foo = void 0;
return this.foo;
}
}
`,
// Deep property setter
outdent`
class Foo {
set bar(value) {
this.bar.baz = value;
}
}
`,
// Define this to alias
outdent`
class Foo {
get bar() {
const self = this;
return self.bar;
}
}
`,
outdent`
class Foo {
set bar(value) {
const self = this;
return self.bar = value;
}
}
`,
outdent`
const foo = {
get bar() {
return this._bar;
}
};
`,
// Access this in function scope
outdent`
const foo = {
get bar() {
function baz() {
return this.bar;
}
}
};
`,
// Nest getter
outdent`
const foo = {
get bar() {
const qux = {
get quux () {
return this.bar;
}
}
}
};
`,
],
invalid: [
// Getter
outdent`
const foo = {
get bar() {
return this.bar;
}
};
`,
outdent`
class Foo {
get bar() {
return this.bar;
}
}
`,
// Setter
outdent`
const foo = {
set bar(value) {
this.bar = value;
}
};
`,
outdent`
class Foo {
set bar(value) {
this.bar = value;
}
}
`,
// Deep property getter
outdent`
const foo = {
get bar() {
return this.bar.baz;
}
};
`,
// Access this in nest block
outdent`
const foo = {
get bar() {
if (true) {
return this.bar;
}
}
};
`,
// Access this in arrow function scope
outdent`
const foo = {
get bar() {
const baz = () => {
return this.bar;
}
}
};
`,
outdent`
const foo = {
get bar() {
const baz = () => {
return () => {
return this.bar;
}
}
}
};
`,
],
});
1 change: 1 addition & 0 deletions test/package.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const RULES_WITHOUT_PASS_FAIL_SECTIONS = new Set([
'prefer-math-min-max',
'consistent-existence-index-check',
'prefer-global-this',
'no-accessor-recursion',
]);

test('Every rule is defined in index file in alphabetical order', t => {
Expand Down
Loading
Loading