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

yeonjuan · mdjermanovic · web-flow · commit 6d1ccb676fed · 2021-09-10T12:14:56.000+02:00
* 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>
diff --git 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"` enforces that functions used as instance field initializers utilize `this`. (default: `true`)
+
+### exceptMethods
 
 ```
 "class-methods-use-this": [<enabled>, { "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": [<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 = () => {}
 }
 ```
 
diff --git 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,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();
         }
 
         /**
@@ -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;
             }
@@ -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;
         }
 
         /**
@@ -95,7 +127,7 @@ module.exports = {
          * @private
          */
         function exitFunction(node) {
-            const methodUsesThis = stack.pop();
+            const methodUsesThis = popContext();
 
             if (isIncludedInstanceMethod(node.parent) && !methodUsesThis) {
                 context.report({
@@ -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
+                }
+            )
         };
     }
 };
diff --git a/tests/lib/rules/class-methods-use-this.js b/tests/lib/rules/class-methods-use-this.js
@@ -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: [
         {
@@ -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 },
@@ -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 }
+            ]
         }
     ]
 });
