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
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 }
]
}
]
});