Skip to content

Commit

Permalink
Update: enforceForClassFields in class-methods-use-this (refs #14857) (
Browse files Browse the repository at this point in the history
…#15018)

* Update: update options in class-methods-use (refs #14857)

* fix default option

* fix review

* fix

* remove unnecessary listener

* Update docs/rules/class-methods-use-this.md

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Update docs/rules/class-methods-use-this.md

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Update docs/rules/class-methods-use-this.md

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Update docs/rules/class-methods-use-this.md

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Update docs/rules/class-methods-use-this.md

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Update docs/rules/class-methods-use-this.md

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Update docs/rules/class-methods-use-this.md

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* change example order

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
  • Loading branch information
yeonjuan and mdjermanovic committed Sep 10, 2021
1 parent 91e82f5 commit 6d1ccb6
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 11 deletions.
49 changes: 47 additions & 2 deletions docs/rules/class-methods-use-this.md
Expand Up @@ -88,7 +88,12 @@ class A {

## Options

### Exceptions
This rule has two options:

* `"exceptMethods"` allows specified method names to be ignored with this rule.
* `"enforceForClassFields"` enforces that functions used as instance field initializers utilize `this`. (default: `true`)

### exceptMethods

```
"class-methods-use-this": [<enabled>, { "exceptMethods": [<...exceptions>] }]
Expand All @@ -110,11 +115,51 @@ class A {
Examples of **correct** code for this rule when used with exceptMethods:

```js
/*eslint class-methods-use-this: ["error", { "exceptMethods": ["foo"] }] */
/*eslint class-methods-use-this: ["error", { "exceptMethods": ["foo", "#bar"] }] */

class A {
foo() {
}
#bar() {
}
}
```

## enforceForClassFields

```
"class-methods-use-this": [<enabled>, { "enforceForClassFields": true | false }]
```

The `enforceForClassFields` option enforces that arrow functions and function expressions used as instance field initializers utilize `this`. (default: `true`)

Examples of **incorrect** code for this rule with the `{ "enforceForClassFields": true }` option (default):

```js
/*eslint class-methods-use-this: ["error", { "enforceForClassFields": true }] */

class A {
foo = () => {}
}
```

Examples of **correct** code for this rule with the `{ "enforceForClassFields": true }` option (default):

```js
/*eslint class-methods-use-this: ["error", { "enforceForClassFields": true }] */

class A {
foo = () => {this;}
}
```

Examples of **correct** code for this rule with the `{ "enforceForClassFields": false }` option:

```js
/*eslint class-methods-use-this: ["error", { "enforceForClassFields": false }] */

class A {
foo = () => {}
}
```

Expand Down
59 changes: 51 additions & 8 deletions lib/rules/class-methods-use-this.js
Expand Up @@ -33,6 +33,10 @@ module.exports = {
items: {
type: "string"
}
},
enforceForClassFields: {
type: "boolean",
default: true
}
},
additionalProperties: false
Expand All @@ -44,18 +48,35 @@ module.exports = {
},
create(context) {
const config = Object.assign({}, context.options[0]);
const enforceForClassFields = config.enforceForClassFields !== false;
const exceptMethods = new Set(config.exceptMethods || []);

const stack = [];

/**
* Push `this` used flag initialized with `false` onto the stack.
* @returns {void}
*/
function pushContext() {
stack.push(false);
}

/**
* Pop `this` used flag from the stack.
* @returns {boolean | undefined} `this` used flag
*/
function popContext() {
return stack.pop();
}

/**
* Initializes the current context to false and pushes it onto the stack.
* These booleans represent whether 'this' has been used in the context.
* @returns {void}
* @private
*/
function enterFunction() {
stack.push(false);
pushContext();
}

/**
Expand All @@ -69,7 +90,7 @@ module.exports = {
case "MethodDefinition":
return !node.static && node.kind !== "constructor";
case "PropertyDefinition":
return !node.static;
return !node.static && enforceForClassFields;
default:
return false;
}
Expand All @@ -82,8 +103,19 @@ module.exports = {
* @private
*/
function isIncludedInstanceMethod(node) {
return isInstanceMethod(node) &&
(node.computed || !exceptMethods.has(node.key.name));
if (isInstanceMethod(node)) {
if (node.computed) {
return true;
}

const hashIfNeeded = node.key.type === "PrivateIdentifier" ? "#" : "";
const name = node.key.type === "Literal"
? astUtils.getStaticStringValue(node.key)
: (node.key.name || "");

return !exceptMethods.has(hashIfNeeded + name);
}
return false;
}

/**
Expand All @@ -95,7 +127,7 @@ module.exports = {
* @private
*/
function exitFunction(node) {
const methodUsesThis = stack.pop();
const methodUsesThis = popContext();

if (isIncludedInstanceMethod(node.parent) && !methodUsesThis) {
context.report({
Expand Down Expand Up @@ -125,10 +157,21 @@ module.exports = {
"FunctionDeclaration:exit": exitFunction,
FunctionExpression: enterFunction,
"FunctionExpression:exit": exitFunction,
"PropertyDefinition > ArrowFunctionExpression.value": enterFunction,
"PropertyDefinition > ArrowFunctionExpression.value:exit": exitFunction,

/*
* Class field value are implicit functions.
*/
"PropertyDefinition:exit": popContext,
"PropertyDefinition > *.key:exit": pushContext,

ThisExpression: markThisUsed,
Super: markThisUsed
Super: markThisUsed,
...(
enforceForClassFields && {
"PropertyDefinition > ArrowFunctionExpression.value": enterFunction,
"PropertyDefinition > ArrowFunctionExpression.value:exit": exitFunction
}
)
};
}
};
31 changes: 30 additions & 1 deletion tests/lib/rules/class-methods-use-this.js
Expand Up @@ -31,11 +31,17 @@ ruleTester.run("class-methods-use-this", rule, {
{ code: "class A { foo() { () => this; } }", parserOptions: { ecmaVersion: 6 } },
{ code: "({ a: function () {} });", parserOptions: { ecmaVersion: 6 } },
{ code: "class A { foo() {this} bar() {} }", options: [{ exceptMethods: ["bar"] }], parserOptions: { ecmaVersion: 6 } },
{ code: "class A { \"foo\"() { } }", options: [{ exceptMethods: ["foo"] }], parserOptions: { ecmaVersion: 6 } },
{ code: "class A { 42() { } }", options: [{ exceptMethods: ["42"] }], parserOptions: { ecmaVersion: 6 } },
{ code: "class A { foo = function() {this} }", parserOptions: { ecmaVersion: 2022 } },
{ code: "class A { foo = () => {this} }", parserOptions: { ecmaVersion: 2022 } },
{ code: "class A { foo = () => {super.toString} }", parserOptions: { ecmaVersion: 2022 } },
{ code: "class A { static foo = function() {} }", parserOptions: { ecmaVersion: 2022 } },
{ code: "class A { static foo = () => {} }", parserOptions: { ecmaVersion: 2022 } }
{ code: "class A { static foo = () => {} }", parserOptions: { ecmaVersion: 2022 } },
{ code: "class A { #bar() {} }", options: [{ exceptMethods: ["#bar"] }], parserOptions: { ecmaVersion: 2022 } },
{ code: "class A { foo = function () {} }", options: [{ enforceForClassFields: false }], parserOptions: { ecmaVersion: 2022 } },
{ code: "class A { foo = () => {} }", options: [{ enforceForClassFields: false }], parserOptions: { ecmaVersion: 2022 } },
{ code: "class A { foo() { return class { [this.foo] = 1 }; } }", parserOptions: { ecmaVersion: 2022 } }
],
invalid: [
{
Expand Down Expand Up @@ -111,6 +117,15 @@ ruleTester.run("class-methods-use-this", rule, {
{ type: "FunctionExpression", line: 1, column: 11, messageId: "missingThis", data: { name: "method" } }
]
},
{
code: "class A { #foo() { } foo() {} #bar() {} }",
options: [{ exceptMethods: ["#foo"] }],
parserOptions: { ecmaVersion: 2022 },
errors: [
{ type: "FunctionExpression", line: 1, column: 22, messageId: "missingThis", data: { name: "method 'foo'" } },
{ type: "FunctionExpression", line: 1, column: 31, messageId: "missingThis", data: { name: "private method #bar" } }
]
},
{
code: "class A { foo(){} 'bar'(){} 123(){} [`baz`](){} [a](){} [f(a)](){} get quux(){} set[a](b){} *quuux(){} }",
parserOptions: { ecmaVersion: 6 },
Expand Down Expand Up @@ -174,6 +189,20 @@ ruleTester.run("class-methods-use-this", rule, {
errors: [
{ messageId: "missingThis", data: { name: "private setter #foo" }, column: 11, endColumn: 19 }
]
},
{
code: "class A { foo () { return class { foo = this }; } }",
parserOptions: { ecmaVersion: 2022 },
errors: [
{ messageId: "missingThis", data: { name: "method 'foo'" }, column: 11, endColumn: 15 }
]
},
{
code: "class A { foo () { return function () { foo = this }; } }",
parserOptions: { ecmaVersion: 2022 },
errors: [
{ messageId: "missingThis", data: { name: "method 'foo'" }, column: 11, endColumn: 15 }
]
}
]
});

0 comments on commit 6d1ccb6

Please sign in to comment.