Add no-param-reassign rule.

vrsource · Apr 30, 2016 · 90b2b23 · 90b2b23
1 parent 11250b2
commit 90b2b23
Show file tree

Hide file tree

Showing 7 changed files with 319 additions and 2 deletions.
diff --git a/.vscode/launch.json b/.vscode/launch.json
@@ -7,7 +7,7 @@
 			"request": "launch",
 			"program": "${workspaceRoot}/node_modules/.bin/tslint",
 			"stopOnEntry": false,
-			"args": ["--test", "test/rules/ext-variable-name/default"],
+			"args": ["--test", "test/rules/no-param-reassign/default"],
 			"cwd": "${workspaceRoot}",
 			"preLaunchTask": "build",
 			"runtimeExecutable": null,

diff --git a/README.md b/README.md
@@ -140,6 +140,20 @@ See: http://eslint.org/docs/rules/no-duplicate-imports
 "no-duplicate-imports": true
 ```
 
+## no-param-reassign
+
+Flag any place where a function parameter is assigned
+a value in the body of a function.
+
+See: eslint no-param-reassign
+
+```javascript
+"no-param-reassign": true
+```
+
+Note: for this rule to work correctly you also need to use `no-shadowed-variable`
+
+
 ## prefer-case-blocks
 
 This rule checks to make sure that all case clauses use a block
@@ -181,6 +195,10 @@ Flags locations where code calls "new Object()", "new Array()", "new Function()"
 
 # Changelog
 
+# 0.11.0
+
+  * Added no-param-reassign rule
+
 # 0.10.0
 
   * Added prefer-case-blocks rule

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "vrsource-tslint-rules",
-  "version": "0.10.0",
+  "version": "0.11.0",
   "description": "Extension rules for tslint",
   "repository": {
     "type": "git",

diff --git a/rules/noParamReassignRule.js b/rules/noParamReassignRule.js
diff --git a/rules/noParamReassignRule.ts b/rules/noParamReassignRule.ts
@@ -0,0 +1,124 @@
+/*
+## no-param-reassign
+
+Flag any place where a function parameter is assigned
+a value in the body of a function.
+
+See: eslint no-param-reassign
+
+```javascript
+"no-param-reassign": true
+```
+
+Note: for this rule to work correctly you also need to use `no-shadowed-variable`
+
+*/
+
+import * as Lint from "tslint/lib/lint";
+import * as ts from "typescript";
+
+const FAIL_STR = "Attempting to reassign to parameter";
+
+export class Rule extends Lint.Rules.AbstractRule {
+    public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
+        return this.applyWithWalker(new NoParamReassignWalker(sourceFile, this.getOptions()));
+    }
+}
+
+
+class ScopeInfo {
+    public varNames: string[] = [];
+}
+
+class NoParamReassignWalker extends Lint.ScopeAwareRuleWalker<ScopeInfo> {
+    public createScope() {
+        return new ScopeInfo();
+    }
+
+    protected checkAssignment(identifier: ts.Identifier) {
+        const name = identifier.text;
+        let allScopes = this.getAllScopes();
+
+        allScopes.forEach((scope) => {
+            if (scope.varNames.indexOf(name) !== -1) {
+                this.addFailure(this.createFailure(identifier.getStart(), identifier.getWidth(), FAIL_STR));
+            }
+        });
+    }
+
+    protected visitParameterDeclaration(node: ts.ParameterDeclaration) {
+        const currentScope = this.getCurrentScope();
+        const isSingleParameter = (node.name.kind === ts.SyntaxKind.Identifier);
+
+        if (isSingleParameter) {
+            currentScope.varNames.push((node.name as ts.Identifier).text);
+        }
+
+        super.visitParameterDeclaration(node);
+    }
+
+    protected visitBinaryExpression(node: ts.BinaryExpression) {
+        if (isAssignmentOperator(node.operatorToken.kind)) {
+            this.visitLeftHandSideExpression(node.left);
+        }
+        super.visitBinaryExpression(node);
+    }
+
+    protected visitPrefixUnaryExpression(node: ts.PrefixUnaryExpression) {
+        this.visitAnyUnaryExpression(node);
+        super.visitPrefixUnaryExpression(node);
+    }
+
+    protected visitPostfixUnaryExpression(node: ts.PostfixUnaryExpression) {
+        this.visitAnyUnaryExpression(node);
+        super.visitPostfixUnaryExpression(node);
+    }
+
+    // --- Extra Visitors --- //
+    // non-standard visitors called from code above
+    private visitLeftHandSideExpression(node: ts.Expression) {
+        while (node.kind === ts.SyntaxKind.ParenthesizedExpression) {
+            node = (node as ts.ParenthesizedExpression).expression;
+        }
+        if (node.kind === ts.SyntaxKind.Identifier) {
+            this.checkAssignment(node as ts.Identifier);
+        } else if (isBindingLiteralExpression(node)) {
+            this.visitBindingLiteralExpression(node as (ts.ArrayLiteralExpression | ts.ObjectLiteralExpression));
+        }
+    }
+
+    private visitBindingLiteralExpression(node: ts.ArrayLiteralExpression | ts.ObjectLiteralExpression) {
+        if (node.kind === ts.SyntaxKind.ObjectLiteralExpression) {
+            const pattern = node as ts.ObjectLiteralExpression;
+            for (const element of pattern.properties) {
+                const kind = element.kind;
+
+                if (kind === ts.SyntaxKind.ShorthandPropertyAssignment) {
+                    this.checkAssignment((element as ts.ShorthandPropertyAssignment).name);
+                } else if (kind === ts.SyntaxKind.PropertyAssignment) {
+                    this.visitLeftHandSideExpression((element as ts.PropertyAssignment).initializer);
+                }
+            }
+        } else if (node.kind === ts.SyntaxKind.ArrayLiteralExpression) {
+            const pattern = node as ts.ArrayLiteralExpression;
+            for (const element of pattern.elements) {
+                this.visitLeftHandSideExpression(element);
+            }
+        }
+    }
+
+    private visitAnyUnaryExpression(node: ts.PrefixUnaryExpression | ts.PostfixUnaryExpression) {
+        if (node.operator === ts.SyntaxKind.PlusPlusToken || node.operator === ts.SyntaxKind.MinusMinusToken) {
+            this.visitLeftHandSideExpression(node.operand);
+        }
+    }
+}
+
+function isAssignmentOperator(token: ts.SyntaxKind): boolean {
+    return token >= ts.SyntaxKind.FirstAssignment && token <= ts.SyntaxKind.LastAssignment;
+}
+
+function isBindingLiteralExpression(node: ts.Node): node is (ts.ArrayLiteralExpression | ts.ObjectLiteralExpression) {
+    return (!!node) && (node.kind === ts.SyntaxKind.ObjectLiteralExpression || node.kind === ts.SyntaxKind.ArrayLiteralExpression);
+}
+
diff --git a/test/rules/no-param-reassign/default/test.ts.lint b/test/rules/no-param-reassign/default/test.ts.lint
@@ -0,0 +1,56 @@
+/**
+Lint test where we should use const if possible
+*/
+function doSomething(param1: string, obj1: Object, obj2: any) {
+    let local1: string = param1;
+    let local2 = {};
+
+    local1 = "set";
+    local2 = 15;
+
+    param1 = "";
+    ~~~~~~  [assign]
+
+    obj1 = {};
+    ~~~~    [assign]
+
+    obj2.nested = 10;
+
+    // this should fail
+    [param1, obj1, local1] = ["other", {}, 10];
+     ~~~~~~           [assign]
+             ~~~~     [assign]
+
+    ({param1, local1} = {param1: 10, local1: 20});
+      ~~~~~~               [assign]
+}
+
+class TestClass {
+    testFunc(p1: string, p2: number, o1: any) {
+        let v1 = 10;
+
+        v1 = 20;
+
+        p1 = "this";
+        ~~    [assign]
+        p2++;
+        ~~    [assign]
+
+        o1.b = 13;
+        o1 = [];
+        ~~    [assign]
+
+        function other_func(nest: any) {
+           nest = 20;
+           ~~~~    [assign]
+        }
+
+        let x = (y) => {
+           y = 10;
+           ~         [assign]
+           return y;
+        };
+    }
+}
+
+[assign]: Attempting to reassign to parameter
diff --git a/test/rules/no-param-reassign/default/tslint.json b/test/rules/no-param-reassign/default/tslint.json
@@ -0,0 +1,6 @@
+{
+  "rulesDirectory": "./rules",
+  "rules": {
+    "no-param-reassign": true
+  }
+}