Repository: eslint/eslint
Pull request #12103 (2019-08-15 16:44:52)

Fix: class-methods-use-this reports 'undefined' names
<!--
    ESLint adheres to the [JS Foundation Code of Conduct](https://js.foundation/community/code-of-conduct).
-->

**What is the purpose of this pull request? (put an "X" next to item)**

[X] Bug fix

<!--
    If the item you've checked above has a template, please paste the template questions below and answer them. (If this pull request is addressing an issue, you can just paste a link to the issue here instead.)
-->
**Tell us about your environment**

* **ESLint Version:** 6.1.0
* **Node Version:** 10.16.0
* **npm Version:** 6.9.0

**What parser (default, Babel-ESLint, etc.) are you using?**

default

**Please show your full configuration:**

<details>
<summary>Configuration</summary>

<!-- Paste your configuration below: -->
```js
module.exports = {
  parserOptions: {
    ecmaVersion: 2015,
  },
};
```

</details>

**What did you do? Please include the actual source code causing the issue.**

```js
/*eslint class-methods-use-this: "error"*/

class A {
  "foo"() {}
  123() {}
  [f()]() {}
}
```

**What did you expect to happen?**

Error messages with method names, where it is possible.

**What actually happened? Please include the actual, raw output from ESLint.**

```
  5:8  error  Expected 'this' to be used by class method 'undefined'  class-methods-use-this
  6:6  error  Expected 'this' to be used by class method 'undefined'  class-methods-use-this
  7:8  error  Expected 'this' to be used by class method 'undefined'  class-methods-use-this
```
<!--
    Please ensure your pull request is ready:

    - Read the pull request guide (https://eslint.org/docs/developer-guide/contributing/pull-requests)
    - Include tests for this change
    - Update documentation for this change (if appropriate)
-->

<!--
    The following is required for all pull requests:
-->

**What changes did you make? (Give an overview)**

Fixed `class-methods-use-this` to use `astUtils.getFunctionNameWithKind()`, like other rules.

**Is there anything you'd like reviewers to focus on?**

In most cases the message before the fix should remain exactly the same after the fix.

The differences are:

* `"foo"(){}` and `123(){}` are fixed to show the name instead of `'undefined'`.
* Computed keys such as `[f()](){}` are fixed to just `... class method.` instead of `... class method 'undefined'`.
* The previous also applies to computed keys with only an identifier like `[a](){}`, but the old message was showing `'a'`. This is in line with how other rules work, it could be a future enhancement in `getFunctionNameWithKind()`.
* For accessors e.g. getters it's now `... class getter 'foo'` instead of `class method 'foo'`.
* The message can now have additional details, e.g. `class generator method 'foo'`

There is a new test case with full messages.


Patch:
diff --git a/lib/rules/class-methods-use-this.js b/lib/rules/class-methods-use-this.js
index c15f32e66..4bf17090a 100644
--- a/lib/rules/class-methods-use-this.js
+++ b/lib/rules/class-methods-use-this.js
@@ -5,6 +5,12 @@
 
 "use strict";
 
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const astUtils = require("./utils/ast-utils");
+
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -34,7 +40,7 @@ module.exports = {
         }],
 
         messages: {
-            missingThis: "Expected 'this' to be used by class method '{{name}}'."
+            missingThis: "Expected 'this' to be used by class {{name}}."
         }
     },
     create(context) {
@@ -90,7 +96,7 @@ module.exports = {
                     node,
                     messageId: "missingThis",
                     data: {
-                        name: node.parent.key.name
+                        name: astUtils.getFunctionNameWithKind(node)
                     }
                 });
             }
diff --git a/lib/rules/no-extra-bind.js b/lib/rules/no-extra-bind.js
index 5380cf217..cc0b1f843 100644
--- a/lib/rules/no-extra-bind.js
+++ b/lib/rules/no-extra-bind.js
@@ -98,6 +98,7 @@ module.exports = {
                 grandparent.type === "CallExpression" &&
                 grandparent.callee === parent &&
                 grandparent.arguments.length === 1 &&
+                grandparent.arguments[0].type !== "SpreadElement" &&
                 parent.type === "MemberExpression" &&
                 parent.object === node &&
                 astUtils.getStaticPropertyName(parent) === "bind"


Test patch:
diff --git a/tests/lib/rules/class-methods-use-this.js b/tests/lib/rules/class-methods-use-this.js
index 89af434fd..5238b9d68 100644
--- a/tests/lib/rules/class-methods-use-this.js
+++ b/tests/lib/rules/class-methods-use-this.js
@@ -37,49 +37,49 @@ ruleTester.run("class-methods-use-this", rule, {
             code: "class A { foo() {} }",
             parserOptions: { ecmaVersion: 6 },
             errors: [
-                { type: "FunctionExpression", line: 1, column: 14, messageId: "missingThis", data: { name: "foo" } }
+                { type: "FunctionExpression", line: 1, column: 14, messageId: "missingThis", data: { name: "method 'foo'" } }
             ]
         },
         {
             code: "class A { foo() {/**this**/} }",
             parserOptions: { ecmaVersion: 6 },
             errors: [
-                { type: "FunctionExpression", line: 1, column: 14, messageId: "missingThis", data: { name: "foo" } }
+                { type: "FunctionExpression", line: 1, column: 14, messageId: "missingThis", data: { name: "method 'foo'" } }
             ]
         },
         {
             code: "class A { foo() {var a = function () {this};} }",
             parserOptions: { ecmaVersion: 6 },
             errors: [
-                { type: "FunctionExpression", line: 1, column: 14, messageId: "missingThis", data: { name: "foo" } }
+                { type: "FunctionExpression", line: 1, column: 14, messageId: "missingThis", data: { name: "method 'foo'" } }
             ]
         },
         {
             code: "class A { foo() {var a = function () {var b = function(){this}};} }",
             parserOptions: { ecmaVersion: 6 },
             errors: [
-                { type: "FunctionExpression", line: 1, column: 14, messageId: "missingThis", data: { name: "foo" } }
+                { type: "FunctionExpression", line: 1, column: 14, messageId: "missingThis", data: { name: "method 'foo'" } }
             ]
         },
         {
             code: "class A { foo() {window.this} }",
             parserOptions: { ecmaVersion: 6 },
             errors: [
-                { type: "FunctionExpression", line: 1, column: 14, messageId: "missingThis", data: { name: "foo" } }
+                { type: "FunctionExpression", line: 1, column: 14, messageId: "missingThis", data: { name: "method 'foo'" } }
             ]
         },
         {
             code: "class A { foo() {that.this = 'this';} }",
             parserOptions: { ecmaVersion: 6 },
             errors: [
-                { type: "FunctionExpression", line: 1, column: 14, messageId: "missingThis", data: { name: "foo" } }
+                { type: "FunctionExpression", line: 1, column: 14, messageId: "missingThis", data: { name: "method 'foo'" } }
             ]
         },
         {
             code: "class A { foo() { () => undefined; } }",
             parserOptions: { ecmaVersion: 6 },
             errors: [
-                { type: "FunctionExpression", line: 1, column: 14, messageId: "missingThis", data: { name: "foo" } }
+                { type: "FunctionExpression", line: 1, column: 14, messageId: "missingThis", data: { name: "method 'foo'" } }
             ]
         },
         {
@@ -87,7 +87,7 @@ ruleTester.run("class-methods-use-this", rule, {
             options: [{ exceptMethods: ["bar"] }],
             parserOptions: { ecmaVersion: 6 },
             errors: [
-                { type: "FunctionExpression", line: 1, column: 14, messageId: "missingThis", data: { name: "foo" } }
+                { type: "FunctionExpression", line: 1, column: 14, messageId: "missingThis", data: { name: "method 'foo'" } }
             ]
         },
         {
@@ -95,7 +95,7 @@ ruleTester.run("class-methods-use-this", rule, {
             options: [{ exceptMethods: ["foo"] }],
             parserOptions: { ecmaVersion: 6 },
             errors: [
-                { type: "FunctionExpression", line: 1, column: 34, messageId: "missingThis", data: { name: "hasOwnProperty" } }
+                { type: "FunctionExpression", line: 1, column: 34, messageId: "missingThis", data: { name: "method 'hasOwnProperty'" } }
             ]
         },
         {
@@ -103,7 +103,22 @@ ruleTester.run("class-methods-use-this", rule, {
             options: [{ exceptMethods: ["foo"] }],
             parserOptions: { ecmaVersion: 6 },
             errors: [
-                { type: "FunctionExpression", line: 1, column: 16, messageId: "missingThis", data: { name: "foo" } }
+                { type: "FunctionExpression", line: 1, column: 16, messageId: "missingThis", data: { name: "method" } }
+            ]
+        },
+        {
+            code: "class A { foo(){} 'bar'(){} 123(){} [`baz`](){} [a](){} [f(a)](){} get quux(){} set[a](b){} *quuux(){} }",
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                { message: "Expected 'this' to be used by class method 'foo'.", type: "FunctionExpression", column: 14 },
+                { message: "Expected 'this' to be used by class method 'bar'.", type: "FunctionExpression", column: 24 },
+                { message: "Expected 'this' to be used by class method '123'.", type: "FunctionExpression", column: 32 },
+                { message: "Expected 'this' to be used by class method 'baz'.", type: "FunctionExpression", column: 44 },
+                { message: "Expected 'this' to be used by class method.", type: "FunctionExpression", column: 52 },
+                { message: "Expected 'this' to be used by class method.", type: "FunctionExpression", column: 63 },
+                { message: "Expected 'this' to be used by class getter 'quux'.", type: "FunctionExpression", column: 76 },
+                { message: "Expected 'this' to be used by class setter.", type: "FunctionExpression", column: 87 },
+                { message: "Expected 'this' to be used by class generator method 'quuux'.", type: "FunctionExpression", column: 99 }
             ]
         }
     ]
diff --git a/tests/lib/rules/no-extra-bind.js b/tests/lib/rules/no-extra-bind.js
index e9c72bca6..cc2b164e8 100644
--- a/tests/lib/rules/no-extra-bind.js
+++ b/tests/lib/rules/no-extra-bind.js
@@ -21,6 +21,7 @@ const errors = [{ messageId: "unexpected", type: "CallExpression" }];
 ruleTester.run("no-extra-bind", rule, {
     valid: [
         "var a = function(b) { return b }.bind(c, d)",
+        { code: "var a = function(b) { return b }.bind(...c)", parserOptions: { ecmaVersion: 6 } },
         "var a = function() { this.b }()",
         "var a = function() { this.b }.foo()",
         "var a = f.bind(a)",