Add prefer-function-over-method rule (#2037)

docs/rules/no-unused-this/index.html

@@ -0,0 +1,28 @@
+---
+ruleName: no-unused-this
+description: Warns for methods that do not use 'this'.
+optionsDescription: |-
+
+  "allow-public" excludes public methods.
+  "allow-protected" excludes protected methods.
+options:
+  type: string
+  enum:
+    - allow-public
+    - allow-protected
+optionExamples:
+  - 'true'
+  - '[true, "allow-public", "allow-protected"]'
+type: style
+typescriptOnly: false
+layout: rule
+title: 'Rule: no-unused-this'
+optionsJSON: |-
+  {
+    "type": "string",
+    "enum": [
+      "allow-public",
+      "allow-protected"
+    ]
+  }
+---

src/rules/preferFunctionOverMethodRule.ts

@@ -0,0 +1,132 @@
+/**
+ * @license
+ * Copyright 2017 Palantir Technologies, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import * as ts from "typescript";
+
+import * as Lint from "../index";
+
+const OPTION_ALLOW_PUBLIC = "allow-public";
+const OPTION_ALLOW_PROTECTED = "allow-protected";
+
+export class Rule extends Lint.Rules.AbstractRule {
+    /* tslint:disable:object-literal-sort-keys */
+    public static metadata: Lint.IRuleMetadata = {
+        ruleName: "prefer-function-over-method",
+        description: "Warns for methods that do not use 'this'.",
+        optionsDescription: Lint.Utils.dedent`
+            "${OPTION_ALLOW_PUBLIC}" excludes public methods.
+            "${OPTION_ALLOW_PROTECTED}" excludes protected methods.`,
+        options: {
+            type: "string",
+            enum: [OPTION_ALLOW_PUBLIC, OPTION_ALLOW_PROTECTED],
+        },
+        optionExamples: [
+            "true",
+            `[true, "${OPTION_ALLOW_PUBLIC}", "${OPTION_ALLOW_PROTECTED}"]`,
+        ],
+        type: "style",
+        typescriptOnly: false,
+    };
+    /* tslint:enable:object-literal-sort-keys */
+
+    public static FAILURE_STRING = "Method does not use 'this'. Use a function instead.";
+
+    public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
+        return this.applyWithWalker(new Walker(sourceFile, this.getOptions()));
+    }
+}
+
+class Walker extends Lint.RuleWalker {
+    private allowPublic = this.hasOption(OPTION_ALLOW_PUBLIC);
+    private allowProtected = this.hasOption(OPTION_ALLOW_PROTECTED);
+    private stack: ThisUsed[] = [];
+
+    public visitNode(node: ts.Node) {
+        switch (node.kind) {
+            case ts.SyntaxKind.ThisKeyword:
+            case ts.SyntaxKind.SuperKeyword:
+                this.setThisUsed(node);
+                break;
+
+            case ts.SyntaxKind.MethodDeclaration:
+                const { name } = node as ts.MethodDeclaration;
+                const usesThis = this.withThisScope(
+                    name.kind === ts.SyntaxKind.Identifier ? name.text : undefined,
+                    () => super.visitNode(node));
+                if (!usesThis &&
+                    node.parent!.kind !== ts.SyntaxKind.ObjectLiteralExpression &&
+                    this.shouldWarnForModifiers(node as ts.MethodDeclaration)) {
+                    this.addFailureAtNode((node as ts.MethodDeclaration).name, Rule.FAILURE_STRING);
+                }
+                break;
+
+            case ts.SyntaxKind.FunctionDeclaration:
+            case ts.SyntaxKind.FunctionExpression:
+                this.withThisScope(undefined, () => super.visitNode(node));
+                break;
+
+            default:
+                super.visitNode(node);
+        }
+    }
+
+    private setThisUsed(node: ts.Node) {
+        const cur = this.stack[this.stack.length - 1];
+        if (cur && !isRecursiveCall(node, cur)) {
+            cur.isThisUsed = true;
+        }
+    }
+
+    private withThisScope(name: string | undefined, recur: () => void): boolean {
+        this.stack.push({ name, isThisUsed: false });
+        recur();
+        return this.stack.pop()!.isThisUsed;
+    }
+
+    private shouldWarnForModifiers(node: ts.MethodDeclaration): boolean {
+        if (Lint.hasModifier(node.modifiers, ts.SyntaxKind.StaticKeyword)) {
+            return false;
+        }
+        // TODO: Also return false if it's marked "override" (https://github.com/palantir/tslint/pull/2037)
+
+        switch (methodVisibility(node)) {
+            case Visibility.Public:
+                return !this.allowPublic;
+            case Visibility.Protected:
+                return !this.allowProtected;
+            default:
+                return true;
+        }
+    }
+}
+
+interface ThisUsed { readonly name: string | undefined; isThisUsed: boolean; }
+
+function isRecursiveCall(thisOrSuper: ts.Node, cur: ThisUsed) {
+    const parent = thisOrSuper.parent!;
+    return thisOrSuper.kind === ts.SyntaxKind.ThisKeyword &&
+        parent.kind === ts.SyntaxKind.PropertyAccessExpression &&
+        (parent as ts.PropertyAccessExpression).name.text === cur.name;
+}
+
+const enum Visibility { Public, Protected, Private }
+
+function methodVisibility(node: ts.MethodDeclaration): Visibility {
+    return Lint.hasModifier(node.modifiers, ts.SyntaxKind.PrivateKeyword) ? Visibility.Private :
+        Lint.hasModifier(node.modifiers, ts.SyntaxKind.ProtectedKeyword) ? Visibility.Protected :
+        Visibility.Public;
+}

test/rules/prefer-function-over-method/allow-protected/test.ts.lint

@@ -0,0 +1,9 @@
+class C {
+    a() {}
+    ~ [0]
+    protected b() {}
+    private c() {}
+            ~ [0]
+}
+
+[0]: Method does not use 'this'. Use a function instead.

test/rules/prefer-function-over-method/allow-protected/tslint.json

@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "prefer-function-over-method": [true, "allow-protected"]
+  }
+}

test/rules/prefer-function-over-method/allow-public-and-protected/test.ts.lint

@@ -0,0 +1,8 @@
+class C {
+    a() {}
+    protected b() {}
+    private c() {}
+            ~ [0]
+}
+
+[0]: Method does not use 'this'. Use a function instead.

test/rules/prefer-function-over-method/allow-public-and-protected/tslint.json

@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "prefer-function-over-method": [true, "allow-public", "allow-protected"]
+  }
+}

test/rules/prefer-function-over-method/allow-public/test.ts.lint

@@ -0,0 +1,9 @@
+class C {
+    a() {}
+    protected b() {}
+              ~ [0]
+    private c() {}
+            ~ [0]
+}
+
+[0]: Method does not use 'this'. Use a function instead.

test/rules/prefer-function-over-method/allow-public/tslint.json

@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "prefer-function-over-method": [true, "allow-public"]
+  }
+}

test/rules/prefer-function-over-method/default/test.js.lint

@@ -0,0 +1,4 @@
+class C {
+    a() {}
+    ~ [Method does not use 'this'. Use a function instead.]
+}

test/rules/prefer-function-over-method/default/test.ts.lint

@@ -0,0 +1,51 @@
+class C {
+    static s() {}
+
+    plain() {}
+    ~~~~~ [0]
+
+    thisInFunction() {
+    ~~~~~~~~~~~~~~ [0]
+        function() {
+            this;
+        }
+    }
+
+    thisInObjectMethod() {
+    ~~~~~~~~~~~~~~~~~~ [0]
+        return { x() { this; } }
+    }
+
+    recur() {
+    ~~~~~ [0]
+        this.recur();
+    }
+
+    thisInParameter(this: string) {}
+    ~~~~~~~~~~~~~~~ [0]
+
+    thisInParameterDefault(x = this) {}
+
+    thisUsed() {
+        this;
+    }
+
+    super() {
+        super;
+    }
+
+    protected protected() {}
+              ~~~~~~~~~ [0]
+
+    private private() {}
+            ~~~~~~~ [0]
+
+    [Symbol.iterator]() {}
+    ~~~~~~~~~~~~~~~~~ [0]
+}
+
+const o = {
+    x() {}
+}
+
+[0]: Method does not use 'this'. Use a function instead.

test/rules/prefer-function-over-method/default/tslint.json

@@ -0,0 +1,8 @@
+{
+  "rules": {
+    "prefer-function-over-method": true
+  },
+  "jsRules": {
+    "prefer-function-over-method": true
+  }
+}