Added a no-this-reassignment rule

src/configs/all.ts

@@ -51,6 +51,7 @@ export const rules = {
     "no-namespace": true,
     "no-non-null-assertion": true,
     "no-reference": true,
+    "no-this-reassignment": true,
     "no-var-requires": true,
     "only-arrow-functions": true,
     "prefer-for-of": true,

src/rules/noThisReassignmentRule.ts

@@ -0,0 +1,145 @@
+/**
+ * @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 utils from "tsutils";
+import * as ts from "typescript";
+
+import * as Lint from "../index";
+
+const ALLOW_THIS_DESTRUCTURING = "allow-this-destructuring";
+const ALLOWED_THIS_NAMES = "allowed-this-names";
+
+interface Options {
+    allowedThisNames: string[];
+    allowThisDestructuring: boolean;
+}
+
+type RuleArgument = boolean | {
+    "allow-this-destructuring"?: boolean;
+    "allowed-this-names"?: string[];
+};
+
+export class Rule extends Lint.Rules.AbstractRule {
+    public static metadata: Lint.IRuleMetadata = {
+        description: "Disallows unnecessary references to `this`.",
+        optionExamples: [
+            true,
+            [
+                true,
+                {
+                    [ALLOWED_THIS_NAMES]: ["self"],
+                    [ALLOW_THIS_DESTRUCTURING]: true,
+                },
+            ],
+        ],
+        options: {
+            additionalProperties: false,
+            properties: {
+                [ALLOW_THIS_DESTRUCTURING]: {
+                    type: "boolean",
+                },
+                [ALLOWED_THIS_NAMES]: {
+                    listType: "string",
+                    type: "list",
+                },
+            },
+            type: "object",
+        },
+        optionsDescription: Lint.Utils.dedent`
+            Two options may be provided on an object:
+
+            * \`${ALLOW_THIS_DESTRUCTURING}\` allows using destructuring to access members of \`this\` (e.g. \`{ foo, bar } = this;\`).
+            * \`${ALLOWED_THIS_NAMES}\` may be specified as a list of regular expressions to match allowed variable names.`,
+        rationale: "Assigning a variable to `this` instead of properly using arrow lambdas"
+            + "may be a symptom of pre-ES6 practices or not manging scope well.",
+        ruleName: "no-this-reassignment",
+        type: "functionality",
+        typescriptOnly: false,
+    };
+
+    public static readonly FAILURE_STRING_BINDINGS = "Don't reassign members of `this` to local variables.";
+
+    public static FAILURE_STRING_FACTORY_IDENTIFIERS(name: string) {
+        return `Assigning \`this\` reference to local variable not allowed: ${name}.`;
+    }
+
+    public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
+        const allowedThisNames: string[] = [];
+        let allowThisDestructuring = false;
+
+        for (const ruleArgument of this.ruleArguments as RuleArgument[]) {
+            if (typeof ruleArgument === "object") {
+                allowThisDestructuring = !!ruleArgument[ALLOW_THIS_DESTRUCTURING];
+
+                if (ruleArgument[ALLOWED_THIS_NAMES] !== undefined) {
+                    allowedThisNames.push(...ruleArgument[ALLOWED_THIS_NAMES]!);
+                }
+            }
+        }
+
+        const options = { allowedThisNames, allowThisDestructuring };
+        const noThisReassignmentWalker = new NoThisReassignmentWalker(sourceFile, this.ruleName, options);
+
+        return this.applyWithWalker(noThisReassignmentWalker);
+    }
+}
+
+class NoThisReassignmentWalker extends Lint.AbstractWalker<Options> {
+    private readonly allowedThisNameTesters = this.options.allowedThisNames.map(
+        (allowedThisName) => new RegExp(allowedThisName));
+
+    public walk(sourceFile: ts.SourceFile): void {
+        ts.forEachChild(sourceFile, this.visitNode);
+    }
+
+    private visitNode = (node: ts.Node): void => {
+        if (utils.isVariableDeclaration(node)) {
+            this.visitVariableDeclaration(node);
+        }
+
+        ts.forEachChild(node, this.visitNode);
+    }
+
+    private visitVariableDeclaration(node: ts.VariableDeclaration): void {
+        if (node.initializer === undefined || node.initializer.kind !== ts.SyntaxKind.ThisKeyword) {
+            return;
+        }
+
+        switch (node.name.kind) {
+            case ts.SyntaxKind.Identifier:
+                if (this.variableNameIsBanned(node.name.text)) {
+                    this.addFailureAtNode(node, Rule.FAILURE_STRING_FACTORY_IDENTIFIERS(node.name.text));
+                }
+                break;
+
+            case ts.SyntaxKind.ObjectBindingPattern:
+                if (!this.options.allowThisDestructuring) {
+                    this.addFailureAtNode(node, Rule.FAILURE_STRING_BINDINGS);
+                }
+        }
+    }
+
+    private variableNameIsBanned(name: string): boolean {
+        for (const tester of this.allowedThisNameTesters) {
+            if (tester.test(name)) {
+                return false;
+            }
+        }
+
+        return true;
+    }
+}

test/rules/no-this-reassignment/allow-this-destructuring/test.ts.lint

@@ -0,0 +1,9 @@
+const { length } = this;
+
+const { length, toString } = this;
+
+const self = this;
+      ~~~~~~~~~~~       [name % ('self')]
+
+[name]: Assigning `this` reference to local variable not allowed: %s.
+

test/rules/no-this-reassignment/allow-this-destructuring/tslint.json

@@ -0,0 +1,7 @@
+{
+  "rules": {
+    "no-this-reassignment": [true, {
+      "allow-this-destructuring": true
+    }]
+  }
+}

test/rules/no-this-reassignment/allowed-this-names/test.ts.lint

@@ -0,0 +1,8 @@
+const start = this;
+
+const startEnd = this;
+
+const endStart = this;
+      ~~~~~~~~~~~~~~~       [name % ('endStart')]
+
+[name]: Assigning `this` reference to local variable not allowed: %s.

test/rules/no-this-reassignment/allowed-this-names/tslint.json

@@ -0,0 +1,7 @@
+{
+  "rules": {
+    "no-this-reassignment": [true, {
+      "allowed-this-names": ["^start"]
+    }]
+  }
+}

test/rules/no-this-reassignment/default/test.ts.lint

@@ -0,0 +1,40 @@
+var unscoped = this;
+    ~~~~~~~~~~~~~~~       [identifier % ('unscoped')]
+
+function testFunction() {
+    let inFunction = this;
+        ~~~~~~~~~~~~~~~~~       [identifier % ('inFunction')]
+}
+
+const testLambda = () => {
+    const inLambda = this;
+          ~~~~~~~~~~~~~~~       [identifier % ('inLambda')]
+};
+
+class TestClass {
+    constructor() {
+        const inConstructor = this;
+              ~~~~~~~~~~~~~~~~~~~~       [identifier % ('inConstructor')]
+
+        const asThis: this = this;
+              ~~~~~~~~~~~~~~~~~~~       [identifier % ('asThis')]
+
+        const asString = "this";
+        const asArray = [this];
+        const asArrayString = ["this"];
+    }
+
+    public act(scope: this = this) {
+        const inMemberFunction = this;
+              ~~~~~~~~~~~~~~~~~~~~~~~       [identifier % ('inMemberFunction')]
+
+        const { act } = this;
+              ~~~~~~~~~~~~~~       [binding]
+
+        const { act, constructor } = this;
+              ~~~~~~~~~~~~~~~~~~~~~~~~~~~       [binding]
+    }
+}
+
+[binding]: Don't reassign members of `this` to local variables.
+[identifier]: Assigning `this` reference to local variable not allowed: %s.

test/rules/no-this-reassignment/default/tslint.json

@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "no-this-reassignment": true
+  }
+}