From d37fa5a355572f7809c652ef0232fa94ee0c7aba Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Wed, 1 Sep 2021 22:56:20 +0900 Subject: [PATCH 01/13] Update: update options in class-methods-use (refs #14857) --- docs/rules/class-methods-use-this.md | 49 ++++++++++++++++++++++- lib/rules/class-methods-use-this.js | 33 ++++++++++++--- tests/lib/rules/class-methods-use-this.js | 28 ++++++++++++- 3 files changed, 101 insertions(+), 9 deletions(-) diff --git a/docs/rules/class-methods-use-this.md b/docs/rules/class-methods-use-this.md index 89f6762b222..98d3e8a019e 100644 --- a/docs/rules/class-methods-use-this.md +++ b/docs/rules/class-methods-use-this.md @@ -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) + +### exceptMethods ``` "class-methods-use-this": [, { "exceptMethods": [<...exceptions>] }] @@ -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": [, { "enforceForClassFields": true | false }] +``` + +The `enforceForClassFields` option enforce that the class fields utilize `this`. (default: true) + +Examples of **correct** code for this rule with the `{ "enforceForClassFields": true }` option: + +```js +/*eslint class-methods-use-this: ["error", {"enforceForClassFields": true }] */ + +class A { + foo = () => {this;} +} +``` + +Examples of **incorrect** code for this rule with the `{ "enforceForClassFields": true }` option: + +```js +/*eslint class-methods-use-this: ["error", {"enforceForClassFields": true }] */ + +class A { + foo = () => {} +} +``` + +Examples of **correct** code for this rule with the `{ "enforceForClassFields": false }` option: + +```js +/*eslint class-methods-use-this: ["error", {"enforceForClassFields": false }] */ + +class A { + foo = () => {} } ``` diff --git a/lib/rules/class-methods-use-this.js b/lib/rules/class-methods-use-this.js index 7e98f4bb739..0321bc55107 100644 --- a/lib/rules/class-methods-use-this.js +++ b/lib/rules/class-methods-use-this.js @@ -33,6 +33,10 @@ module.exports = { items: { type: "string" } + }, + enforceForClassFields: { + type: "boolean", + default: true } }, additionalProperties: false @@ -44,6 +48,7 @@ module.exports = { }, create(context) { const config = Object.assign({}, context.options[0]); + const enforceForClassFields = config.enforceForClassFields ?? true; const exceptMethods = new Set(config.exceptMethods || []); const stack = []; @@ -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; } @@ -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; } /** @@ -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(), + ThisExpression: markThisUsed, - Super: markThisUsed + Super: markThisUsed, + ...( + enforceForClassFields && { + "PropertyDefinition > ArrowFunctionExpression.value": enterFunction, + "PropertyDefinition > ArrowFunctionExpression.value:exit": exitFunction + } + ) }; } }; diff --git a/tests/lib/rules/class-methods-use-this.js b/tests/lib/rules/class-methods-use-this.js index 5cc04351c99..e74da0ef0e1 100644 --- a/tests/lib/rules/class-methods-use-this.js +++ b/tests/lib/rules/class-methods-use-this.js @@ -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: [ { @@ -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 }, @@ -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 } + ] } ] }); From 8542428666a90eb6a5be62b490d31d61a3d5d02e Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Wed, 1 Sep 2021 23:13:00 +0900 Subject: [PATCH 02/13] fix default option --- lib/rules/class-methods-use-this.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/class-methods-use-this.js b/lib/rules/class-methods-use-this.js index 0321bc55107..9edaed385a6 100644 --- a/lib/rules/class-methods-use-this.js +++ b/lib/rules/class-methods-use-this.js @@ -48,7 +48,7 @@ module.exports = { }, create(context) { const config = Object.assign({}, context.options[0]); - const enforceForClassFields = config.enforceForClassFields ?? true; + const enforceForClassFields = config.enforceForClassFields !== false; const exceptMethods = new Set(config.exceptMethods || []); const stack = []; From 0d6a8e30d73738a4d1b0ea9aa011bead50013e10 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Sun, 5 Sep 2021 00:42:04 +0900 Subject: [PATCH 03/13] fix review --- lib/rules/class-methods-use-this.js | 38 ++++++++++++++++++----- tests/lib/rules/class-methods-use-this.js | 5 ++- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/lib/rules/class-methods-use-this.js b/lib/rules/class-methods-use-this.js index 9edaed385a6..0ac960d142e 100644 --- a/lib/rules/class-methods-use-this.js +++ b/lib/rules/class-methods-use-this.js @@ -53,6 +53,22 @@ module.exports = { const stack = []; + /** + * Push `this` used flags initialized with `false` onto the stack. + * @returns {void} + */ + function pushContext() { + stack.push(false); + } + + /** + * Pop `this` used flags from the stack. + * @returns {boolean | undefined} + */ + 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. @@ -60,7 +76,7 @@ module.exports = { * @private */ function enterFunction() { - stack.push(false); + pushContext(); } /** @@ -88,9 +104,16 @@ module.exports = { */ function isIncludedInstanceMethod(node) { 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 (node.computed || !exceptMethods.has(hashIfNeeded + node.key.name)); + return !exceptMethods.has(hashIfNeeded + name); } return false; } @@ -104,7 +127,7 @@ module.exports = { * @private */ function exitFunction(node) { - const methodUsesThis = stack.pop(); + const methodUsesThis = popContext(); if (isIncludedInstanceMethod(node.parent) && !methodUsesThis) { context.report({ @@ -136,11 +159,12 @@ module.exports = { "FunctionExpression:exit": exitFunction, /* - * Consuming marked 'this' in PropertyDefinition. - * ex: class foo { a = this; } + * Class field value are implicit functions. */ - PropertyDefinition: () => stack.push(false), - "PropertyDefinition:exit": () => stack.pop(), + PropertyDefinition: pushContext, + "PropertyDefinition:exit": popContext, + "PropertyDefinition > *.key": popContext, + "PropertyDefinition > *.key:exit": pushContext, ThisExpression: markThisUsed, Super: markThisUsed, diff --git a/tests/lib/rules/class-methods-use-this.js b/tests/lib/rules/class-methods-use-this.js index e74da0ef0e1..95c1a6f64e6 100644 --- a/tests/lib/rules/class-methods-use-this.js +++ b/tests/lib/rules/class-methods-use-this.js @@ -31,6 +31,8 @@ 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 } }, @@ -38,7 +40,8 @@ ruleTester.run("class-methods-use-this", rule, { { 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 = () => {} }", options: [{ enforceForClassFields: false }], parserOptions: { ecmaVersion: 2022 } }, + { code: "class A { foo() { return class { [this.foo] = 1 }; } }", parserOptions: { ecmaVersion: 2022 } } ], invalid: [ { From cf023b488f63f7aee8c35b3278b0e3bb49c6b654 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Sun, 5 Sep 2021 00:50:47 +0900 Subject: [PATCH 04/13] fix --- lib/rules/class-methods-use-this.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/rules/class-methods-use-this.js b/lib/rules/class-methods-use-this.js index 0ac960d142e..291cdc9ad36 100644 --- a/lib/rules/class-methods-use-this.js +++ b/lib/rules/class-methods-use-this.js @@ -54,7 +54,7 @@ module.exports = { const stack = []; /** - * Push `this` used flags initialized with `false` onto the stack. + * Push `this` used flag initialized with `false` onto the stack. * @returns {void} */ function pushContext() { @@ -62,8 +62,8 @@ module.exports = { } /** - * Pop `this` used flags from the stack. - * @returns {boolean | undefined} + * Pop `this` used flag from the stack. + * @returns {boolean | undefined} `this` used flag */ function popContext() { return stack.pop(); From 07135f8aeef1d5787470e18e12e90bea5511adc0 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Sun, 5 Sep 2021 01:53:33 +0900 Subject: [PATCH 05/13] remove unnecessary listener --- lib/rules/class-methods-use-this.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/rules/class-methods-use-this.js b/lib/rules/class-methods-use-this.js index 291cdc9ad36..034ba3abcaf 100644 --- a/lib/rules/class-methods-use-this.js +++ b/lib/rules/class-methods-use-this.js @@ -161,9 +161,7 @@ module.exports = { /* * Class field value are implicit functions. */ - PropertyDefinition: pushContext, "PropertyDefinition:exit": popContext, - "PropertyDefinition > *.key": popContext, "PropertyDefinition > *.key:exit": pushContext, ThisExpression: markThisUsed, From f16a42fd29853aa6d0fae59a905c8f525abe6c5e Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Mon, 6 Sep 2021 01:44:01 +0900 Subject: [PATCH 06/13] Update docs/rules/class-methods-use-this.md Co-authored-by: Milos Djermanovic --- docs/rules/class-methods-use-this.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/class-methods-use-this.md b/docs/rules/class-methods-use-this.md index 98d3e8a019e..cf854f6a8f2 100644 --- a/docs/rules/class-methods-use-this.md +++ b/docs/rules/class-methods-use-this.md @@ -91,7 +91,7 @@ class A { 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) +* `"enforceForClassFields"` enforces that functions used as instance field initializers utilize `this`. (default: `true`) ### exceptMethods From 97d82d187659602e2b341ced02f39db94499fd35 Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Mon, 6 Sep 2021 01:52:37 +0900 Subject: [PATCH 07/13] Update docs/rules/class-methods-use-this.md Co-authored-by: Milos Djermanovic --- docs/rules/class-methods-use-this.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/class-methods-use-this.md b/docs/rules/class-methods-use-this.md index cf854f6a8f2..43affbb9ea1 100644 --- a/docs/rules/class-methods-use-this.md +++ b/docs/rules/class-methods-use-this.md @@ -136,7 +136,7 @@ The `enforceForClassFields` option enforce that the class fields utilize `this`. Examples of **correct** code for this rule with the `{ "enforceForClassFields": true }` option: ```js -/*eslint class-methods-use-this: ["error", {"enforceForClassFields": true }] */ +/*eslint class-methods-use-this: ["error", { "enforceForClassFields": true }] */ class A { foo = () => {this;} From 656500e5940aa1ef32d9399878029256819ae58f Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Mon, 6 Sep 2021 01:52:43 +0900 Subject: [PATCH 08/13] Update docs/rules/class-methods-use-this.md Co-authored-by: Milos Djermanovic --- docs/rules/class-methods-use-this.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/class-methods-use-this.md b/docs/rules/class-methods-use-this.md index 43affbb9ea1..a36d854cb64 100644 --- a/docs/rules/class-methods-use-this.md +++ b/docs/rules/class-methods-use-this.md @@ -131,7 +131,7 @@ class A { "class-methods-use-this": [, { "enforceForClassFields": true | false }] ``` -The `enforceForClassFields` option enforce that the class fields utilize `this`. (default: true) +The `enforceForClassFields` option enforces that arrow functions and function expressions used as instance field initializers utilize `this`. (default: `true`) Examples of **correct** code for this rule with the `{ "enforceForClassFields": true }` option: From ac8b309ac79599de612269df8a320a7e6ce2eb91 Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Mon, 6 Sep 2021 01:52:54 +0900 Subject: [PATCH 09/13] Update docs/rules/class-methods-use-this.md Co-authored-by: Milos Djermanovic --- docs/rules/class-methods-use-this.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/class-methods-use-this.md b/docs/rules/class-methods-use-this.md index a36d854cb64..28ca316e540 100644 --- a/docs/rules/class-methods-use-this.md +++ b/docs/rules/class-methods-use-this.md @@ -133,7 +133,7 @@ class A { The `enforceForClassFields` option enforces that arrow functions and function expressions used as instance field initializers utilize `this`. (default: `true`) -Examples of **correct** code for this rule with the `{ "enforceForClassFields": true }` option: +Examples of **correct** code for this rule with the `{ "enforceForClassFields": true }` option (default): ```js /*eslint class-methods-use-this: ["error", { "enforceForClassFields": true }] */ From 3303d70947dbdf6e503578a01aa4817b0b5ece9b Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Mon, 6 Sep 2021 01:52:58 +0900 Subject: [PATCH 10/13] Update docs/rules/class-methods-use-this.md Co-authored-by: Milos Djermanovic --- docs/rules/class-methods-use-this.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/class-methods-use-this.md b/docs/rules/class-methods-use-this.md index 28ca316e540..12122bc33d3 100644 --- a/docs/rules/class-methods-use-this.md +++ b/docs/rules/class-methods-use-this.md @@ -143,7 +143,7 @@ class A { } ``` -Examples of **incorrect** code for this rule with the `{ "enforceForClassFields": true }` option: +Examples of **incorrect** code for this rule with the `{ "enforceForClassFields": true }` option (default): ```js /*eslint class-methods-use-this: ["error", {"enforceForClassFields": true }] */ From 61e354c586160dab5ea412f2bdd36daa71bc095d Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Mon, 6 Sep 2021 01:53:10 +0900 Subject: [PATCH 11/13] Update docs/rules/class-methods-use-this.md Co-authored-by: Milos Djermanovic --- docs/rules/class-methods-use-this.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/class-methods-use-this.md b/docs/rules/class-methods-use-this.md index 12122bc33d3..4ac09bde656 100644 --- a/docs/rules/class-methods-use-this.md +++ b/docs/rules/class-methods-use-this.md @@ -146,7 +146,7 @@ class A { Examples of **incorrect** code for this rule with the `{ "enforceForClassFields": true }` option (default): ```js -/*eslint class-methods-use-this: ["error", {"enforceForClassFields": true }] */ +/*eslint class-methods-use-this: ["error", { "enforceForClassFields": true }] */ class A { foo = () => {} From d563bfc6b9ead2dc97be37f0fb54b02564d3829c Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Mon, 6 Sep 2021 01:53:16 +0900 Subject: [PATCH 12/13] Update docs/rules/class-methods-use-this.md Co-authored-by: Milos Djermanovic --- docs/rules/class-methods-use-this.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/class-methods-use-this.md b/docs/rules/class-methods-use-this.md index 4ac09bde656..6de0916f397 100644 --- a/docs/rules/class-methods-use-this.md +++ b/docs/rules/class-methods-use-this.md @@ -156,7 +156,7 @@ class A { Examples of **correct** code for this rule with the `{ "enforceForClassFields": false }` option: ```js -/*eslint class-methods-use-this: ["error", {"enforceForClassFields": false }] */ +/*eslint class-methods-use-this: ["error", { "enforceForClassFields": false }] */ class A { foo = () => {} From 31eb1d101ff7586b289ba6ca3aed3514441ecb7d Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Mon, 6 Sep 2021 01:54:06 +0900 Subject: [PATCH 13/13] change example order --- docs/rules/class-methods-use-this.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/rules/class-methods-use-this.md b/docs/rules/class-methods-use-this.md index 6de0916f397..9d3ca7f1940 100644 --- a/docs/rules/class-methods-use-this.md +++ b/docs/rules/class-methods-use-this.md @@ -133,23 +133,23 @@ class A { The `enforceForClassFields` option enforces that arrow functions and function expressions used as instance field initializers utilize `this`. (default: `true`) -Examples of **correct** code for this rule with the `{ "enforceForClassFields": true }` option (default): +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 = () => {this;} + foo = () => {} } ``` -Examples of **incorrect** code for this rule with the `{ "enforceForClassFields": true }` option (default): +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 = () => {} + foo = () => {this;} } ```