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