New: class-methods-use-this rule (fixes #5139) (#6881)

conf/eslint.json

@@ -140,6 +140,7 @@
         "brace-style": "off",
         "callback-return": "off",
         "camelcase": "off",
+        "class-methods-use-this": "off",
         "comma-dangle": "off",
         "comma-spacing": "off",
         "comma-style": "off",

docs/rules/class-methods-use-this.md

@@ -0,0 +1,85 @@
+# Enforce that class methods utilize `this` (class-methods-use-this)
+
+If a class method does not use `this`, it can safely be made a static function.
+
+It's possible to have a class method which doesn't use `this`, such as:
+
+```js
+class A {
+    constructor() {
+        this.a = "hi";
+    }
+
+    print() {
+        console.log(this.a);
+    }
+
+    sayHi() {
+        console.log("hi");
+    }
+}
+
+let aObj = new A();
+a.sayHi(); // => "hi"
+```
+
+In the example above, the `sayHi` method doesn't use `this`, so we can make it a static method:
+
+```js
+class A {
+    constructor() {
+        this.a = "hi";
+    }
+
+    print() {
+        console.log(this.a);
+    }
+
+    static sayHi() {
+        console.log("hi");
+    }
+}
+
+A.sayHi(); // => "hi"
+```
+
+## Rule Details
+
+This rule is aimed to flag class methods that do not use `this`.
+
+Examples of **incorrect** code for this rule:
+
+```js
+/*eslint class-methods-use-this: "error"*/
+/*eslint-env es6*/
+
+class A {
+    foo() {
+        console.log("Hello World");     /*error Expected 'this' to be used by class method 'foo'.*/
+    }
+}
+```
+
+Examples of **correct** code for this rule:
+
+```js
+/*eslint class-methods-use-this: "error"*/
+/*eslint-env es6*/
+class A {
+    foo() {
+        this.bar = "Hello World"; // OK, this is used
+    }
+}
+
+class A {
+    constructor() {
+        // OK. constructor is exempt
+    }
+}
+
+class A {
+    static foo() {
+        // OK. static methods aren't expected to use this.
+    }
+}
+```

lib/rules/class-methods-use-this.js

@@ -0,0 +1,80 @@
+/**
+ * @fileoverview Rule to enforce that all class methods use 'this'.
+ * @author Patrick Williams
+ */
+
+"use strict";
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+module.exports = {
+    meta: {
+        docs: {
+            description: "enforce that class methods utilize `this`",
+            category: "Best Practices",
+            recommended: false
+        },
+        schema: []
+    },
+    create(context) {
+        const stack = [];
+
+        /**
+         * 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);
+        }
+
+        /**
+         * Check if the node is an instance method
+         * @param {ASTNode} node - node to check
+         * @returns {boolean} True if its an instance method
+         * @private
+         */
+        function isInstanceMethod(node) {
+            return !node.static && node.kind !== "constructor" && node.type === "MethodDefinition";
+        }
+
+        /**
+         * Checks if we are leaving a function that is a method, and reports if 'this' has not been used.
+         * Static methods and the constructor are exempt.
+         * Then pops the context off the stack.
+         * @param {ASTNode} node - A function node that was entered.
+         * @returns {void}
+         * @private
+         */
+        function exitFunction(node) {
+            const methodUsesThis = stack.pop();
+
+            if (isInstanceMethod(node.parent) && !methodUsesThis) {
+                context.report(node, "Expected 'this' to be used by class method '" + node.parent.key.name + "'.");
+            }
+        }
+
+        /**
+         * Mark the current context as having used 'this'.
+         * @returns {void}
+         * @private
+         */
+        function markThisUsed() {
+            if (stack.length) {
+                stack[stack.length - 1] = true;
+            }
+        }
+
+        return {
+            FunctionDeclaration: enterFunction,
+            "FunctionDeclaration:exit": exitFunction,
+            FunctionExpression: enterFunction,
+            "FunctionExpression:exit": exitFunction,
+            ThisExpression: markThisUsed,
+            Super: markThisUsed
+        };
+    }
+};

packages/eslint-config-eslint/default.yml

@@ -11,6 +11,7 @@ rules:
     brace-style: ["error", "1tbs"]
     camelcase: ["error", { properties: "never" }]
     callback-return: ["error", ["cb", "callback", "next"]]
+    class-methods-use-this: "error"
     comma-spacing: "error"
     comma-style: ["error", "last"]
     consistent-return: "error"

tests/lib/rules/class-methods-use-this.js

@@ -0,0 +1,85 @@
+/**
+ * @fileoverview Tests for class-methods-use-this rule.
+ * @author Patrick Williams
+ */
+
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const rule = require("../../../lib/rules/class-methods-use-this");
+const RuleTester = require("../../../lib/testers/rule-tester");
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+const ruleTester = new RuleTester();
+
+ruleTester.run("class-methods-use-this", rule, {
+    valid: [
+        {code: "class A { constructor() {} }", parserOptions: { ecmaVersion: 6 }},
+        {code: "class A { foo() {this} }", parserOptions: { ecmaVersion: 6 }},
+        {code: "class A { foo() {this.bar = 'bar';} }", parserOptions: { ecmaVersion: 6 }},
+        {code: "class A { foo() {bar(this);} }", parserOptions: { ecmaVersion: 6 }},
+        {code: "class A extends B { foo() {super.foo();} }", parserOptions: { ecmaVersion: 6 }},
+        {code: "class A { foo() { if(true) { return this; } } }", parserOptions: { ecmaVersion: 6 }},
+        {code: "class A { static foo() {} }", parserOptions: { ecmaVersion: 6 }},
+        {code: "({ a(){} });", parserOptions: { ecmaVersion: 6 }},
+        {code: "class A { foo() { () => this; } }", parserOptions: { ecmaVersion: 6 }},
+        {code: "({ a: function () {} });", parserOptions: { ecmaVersion: 6 }}
+    ],
+    invalid: [
+        {
+            code: "class A { foo() {} }",
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                {type: "FunctionExpression", line: 1, column: 14, message: "Expected 'this' to be used by class method 'foo'."}
+            ]
+        },
+        {
+            code: "class A { foo() {/**this**/} }",
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                {type: "FunctionExpression", line: 1, column: 14, message: "Expected 'this' to be used by class method 'foo'."}
+            ]
+        },
+        {
+            code: "class A { foo() {var a = function () {this};} }",
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                {type: "FunctionExpression", line: 1, column: 14, message: "Expected 'this' to be used by class method 'foo'."}
+            ]
+        },
+        {
+            code: "class A { foo() {var a = function () {var b = function(){this}};} }",
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                {type: "FunctionExpression", line: 1, column: 14, message: "Expected 'this' to be used by class method 'foo'."}
+            ]
+        },
+        {
+            code: "class A { foo() {window.this} }",
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                {type: "FunctionExpression", line: 1, column: 14, message: "Expected 'this' to be used by class method 'foo'."}
+            ]
+        },
+        {
+            code: "class A { foo() {that.this = 'this';} }",
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                {type: "FunctionExpression", line: 1, column: 14, message: "Expected 'this' to be used by class method 'foo'."}
+            ]
+        },
+        {
+            code: "class A { foo() { () => undefined; } }",
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                {type: "FunctionExpression", line: 1, column: 14, message: "Expected 'this' to be used by class method 'foo'."}
+            ]
+        }
+    ]
+});