Skip to content
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

Update: enforceForClassFields in class-methods-use-this (refs #14857) #15018

Merged
merged 13 commits into from Sep 10, 2021
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"` enforce utilize `this` in class fields. (default: true)
yeonjuan marked this conversation as resolved.
Show resolved Hide resolved

### 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 enforce that the class fields utilize `this`. (default: true)
yeonjuan marked this conversation as resolved.
Show resolved Hide resolved

Examples of **correct** code for this rule with the `{ "enforceForClassFields": true }` option:
yeonjuan marked this conversation as resolved.
Show resolved Hide resolved

```js
/*eslint class-methods-use-this: ["error", {"enforceForClassFields": true }] */
yeonjuan marked this conversation as resolved.
Show resolved Hide resolved

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

Examples of **incorrect** code for this rule with the `{ "enforceForClassFields": true }` option:
yeonjuan marked this conversation as resolved.
Show resolved Hide resolved

```js
/*eslint class-methods-use-this: ["error", {"enforceForClassFields": true }] */
yeonjuan marked this conversation as resolved.
Show resolved Hide resolved

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

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

```js
/*eslint class-methods-use-this: ["error", {"enforceForClassFields": false }] */
yeonjuan marked this conversation as resolved.
Show resolved Hide resolved

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

Expand Down
33 changes: 27 additions & 6 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,6 +48,7 @@ module.exports = {
},
create(context) {
const config = Object.assign({}, context.options[0]);
const enforceForClassFields = config.enforceForClassFields !== false;
const exceptMethods = new Set(config.exceptMethods || []);

const stack = [];
Expand All @@ -69,7 +74,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 +87,12 @@ module.exports = {
* @private
*/
function isIncludedInstanceMethod(node) {
return isInstanceMethod(node) &&
(node.computed || !exceptMethods.has(node.key.name));
if (isInstanceMethod(node)) {
const hashIfNeeded = node.key.type === "PrivateIdentifier" ? "#" : "";

return (node.computed || !exceptMethods.has(hashIfNeeded + node.key.name));
}
return false;
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down Expand Up @@ -125,10 +134,22 @@ module.exports = {
"FunctionDeclaration:exit": exitFunction,
FunctionExpression: enterFunction,
"FunctionExpression:exit": exitFunction,
"PropertyDefinition > ArrowFunctionExpression.value": enterFunction,
"PropertyDefinition > ArrowFunctionExpression.value:exit": exitFunction,

/*
* Consuming marked 'this' in PropertyDefinition.
* ex: class foo { a = this; }
*/
PropertyDefinition: () => stack.push(false),
"PropertyDefinition:exit": () => stack.pop(),
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved

ThisExpression: markThisUsed,
Super: markThisUsed
Super: markThisUsed,
...(
enforceForClassFields && {
"PropertyDefinition > ArrowFunctionExpression.value": enterFunction,
"PropertyDefinition > ArrowFunctionExpression.value:exit": exitFunction
}
)
};
}
};
28 changes: 27 additions & 1 deletion tests/lib/rules/class-methods-use-this.js
Expand Up @@ -35,7 +35,10 @@ ruleTester.run("class-methods-use-this", rule, {
{ 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 } }
],
invalid: [
{
Expand Down Expand Up @@ -111,6 +114,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 +186,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 }
]
}
]
});