no-duplicate-variable: handle parameters

src/rules/noDuplicateVariableRule.ts

@@ -20,6 +20,12 @@ import * as ts from "typescript";
 
 import * as Lint from "../index";
 
+const OPTION_CHECK_PARAMETERS = "check-parameters";
+
+interface Options {
+    parameters: boolean;
+}
+
 export class Rule extends Lint.Rules.AbstractRule {
     /* tslint:disable:object-literal-sort-keys */
     public static metadata: Lint.IRuleMetadata = {
@@ -31,9 +37,15 @@ export class Rule extends Lint.Rules.AbstractRule {
         rationale: Lint.Utils.dedent`
             A variable can be reassigned if necessary -
             there's no good reason to have a duplicate variable declaration.`,
-        optionsDescription: "Not configurable.",
-        options: null,
-        optionExamples: [true],
+        optionsDescription: `You can specify \`"${OPTION_CHECK_PARAMETERS}"\` to check for variables with the same name as a paramter.`,
+        options: {
+            type: "string",
+            enum: [OPTION_CHECK_PARAMETERS],
+        },
+        optionExamples: [
+            true,
+            [true, OPTION_CHECK_PARAMETERS],
+        ],
         type: "functionality",
         typescriptOnly: false,
     };
@@ -44,41 +56,48 @@ export class Rule extends Lint.Rules.AbstractRule {
     }
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        return this.applyWithFunction(sourceFile, walk);
+        return this.applyWithWalker(new NoDuplicateVariableWalker(sourceFile, this.ruleName, {
+            parameters: this.ruleArguments.indexOf(OPTION_CHECK_PARAMETERS) !== - 1,
+        }));
     }
 }
 
-function walk(ctx: Lint.WalkContext<void>): void {
-    let scope = new Set<string>();
-    return ts.forEachChild(ctx.sourceFile, function cb(node: ts.Node): void {
-        if (utils.isFunctionScopeBoundary(node)) {
-            const oldScope = scope;
-            scope = new Set();
-            ts.forEachChild(node, cb);
-            scope = oldScope;
-            return;
-        } else if (utils.isVariableDeclaration(node) && !utils.isBlockScopedVariableDeclaration(node)) {
-            forEachBoundIdentifier(node.name, (id) => {
-                const { text } = id;
-                if (scope.has(text)) {
-                    ctx.addFailureAtNode(id, Rule.FAILURE_STRING(text));
-                } else {
-                    scope.add(text);
+class NoDuplicateVariableWalker extends Lint.AbstractWalker<Options> {
+    private scope: Set<string>;
+    public walk(sourceFile: ts.SourceFile) {
+        this.scope = new Set();
+        const cb = (node: ts.Node): void => {
+            if (utils.isFunctionScopeBoundary(node)) {
+                const oldScope = this.scope;
+                this.scope = new Set();
+                ts.forEachChild(node, cb);
+                this.scope = oldScope;
+                return;
+            }
+            if (this.options.parameters && utils.isParameterDeclaration(node)) {
+                this.handleBindingName(node.name, false);
+            } else if (utils.isVariableDeclarationList(node) && !utils.isBlockScopedVariableDeclarationList(node)) {
+                for (const variable of node.declarations) {
+                    this.handleBindingName(variable.name, true);
                 }
-            });
-        }
-
-        return ts.forEachChild(node, cb);
-    });
-}
+            }
+            return ts.forEachChild(node, cb);
+        };
+        return ts.forEachChild(sourceFile, cb);
+    }
 
-function forEachBoundIdentifier(name: ts.BindingName, action: (id: ts.Identifier) => void): void {
-    if (name.kind === ts.SyntaxKind.Identifier) {
-        action(name);
-    } else {
-        for (const e of name.elements) {
-            if (e.kind !== ts.SyntaxKind.OmittedExpression) {
-                forEachBoundIdentifier(e.name, action);
+    private handleBindingName(name: ts.BindingName, check: boolean) {
+        if (name.kind === ts.SyntaxKind.Identifier) {
+            if (check && this.scope.has(name.text)) {
+                this.addFailureAtNode(name, Rule.FAILURE_STRING(name.text));
+            } else {
+                this.scope.add(name.text);
+            }
+        } else {
+            for (const e of name.elements) {
+                if (e.kind !== ts.SyntaxKind.OmittedExpression) {
+                    this.handleBindingName(e.name, check);
+                }
             }
         }
     }

test/rules/no-duplicate-variable/test.ts.lint → test/rules/no-duplicate-variable/check-parameters/test.ts.lint

@@ -2,6 +2,7 @@ var duplicated = 1;
 
 function f(x) {
     var x;
+        ~ [err % ('x')]
 }
 
 class Test {
@@ -13,7 +14,7 @@ class Test {
             };
 
         var duplicated = null;
-            ~~~~~~~~~~         [Duplicate variable: 'duplicated']
+            ~~~~~~~~~~         [err % ('duplicated')]
     }
 }
 
@@ -25,12 +26,12 @@ function test() {
         };
 
     var duplicated = null;
-        ~~~~~~~~~~         [Duplicate variable: 'duplicated']
+        ~~~~~~~~~~         [err % ('duplicated')]
 }
 
 duplicated = 2;
 var duplicated = 3;
-    ~~~~~~~~~~      [Duplicate variable: 'duplicated']
+    ~~~~~~~~~~      [err % ('duplicated')]
 
 // valid code
 module tmp {
@@ -108,6 +109,7 @@ function testArguments1(arg: number, arg: number): void {
 // failure: local var/let declarations shadow arguments.
 function testArguments2(x: number, y: number): void {
     var x = 1;
+        ~ [err % ('x')]
     let y = 2;
 }
 
@@ -146,7 +148,7 @@ function testDestructuring() {
 
     var [x, y] = myFunc();
     var [z, z] = myFunc(); // failure
-            ~                         [Duplicate variable: 'z']
+            ~                         [err % ('z')]
 
     let [x1, y1] = myFunc();
     let [z1, z1] = myFunc(); // tsc error
@@ -159,15 +161,20 @@ function testDestructuring() {
 
     var [a2, [b2, c2]] = [1, [2, 3]];
     var [{a2, d2}] = [{a2: 1, d2: 4}]; // failure
-          ~~                                      [Duplicate variable: 'a2']
+          ~~                                      [err % ('a2')]
 
     function myFunc2([a, b]) {
-        var a; // not a failure; caught by no-shadowed-variable
+        var a;
+            ~ [err % ('a')]
         return b;
     }
 
     var [x, y3] = myFunc(); // failure
-         ~                             [Duplicate variable: 'x']
+         ~                             [err % ('x')]
     var [x3, ...y] = [1, 2, 3, 4]; // failure
-                ~                             [Duplicate variable: 'y']
+                ~                             [err % ('y')]
 }
+
+function compileErrors(foo, {bar}, bar, foo, {baz}, [baz]) {}
+
+[err]: Duplicate variable: '%s'

test/rules/no-duplicate-variable/check-parameters/tslint.json

@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "no-duplicate-variable": [true, "check-parameters"]
+  }
+}

test/rules/no-duplicate-variable/default/test.ts.lint

@@ -0,0 +1,177 @@
+var duplicated = 1;
+
+function f(x) {
+    var x;
+}
+
+class Test {
+    private myFunc() {
+        var notDuplicated = 123,
+            duplicated = 234,
+            someFunc = () => {
+                var notDuplicated = 345;
+            };
+
+        var duplicated = null;
+            ~~~~~~~~~~         [err % ('duplicated')]
+    }
+}
+
+function test() {
+    var notDuplicated = 123,
+        duplicated = 234,
+        someFunc = () => {
+            var notDuplicated = 345;
+        };
+
+    var duplicated = null;
+        ~~~~~~~~~~         [err % ('duplicated')]
+}
+
+duplicated = 2;
+var duplicated = 3;
+    ~~~~~~~~~~      [err % ('duplicated')]
+
+// valid code
+module tmp {
+    var path = require("path");
+    export class MyType {
+        path: string;
+    }
+}
+
+module MyModule {
+    export class ClassA {
+        id: string;
+    }
+
+    export class ClassB {
+        id: string;
+    }
+}
+
+var a = {
+    foo(): void {
+        var bar = 1;
+    },
+    baz(): void {
+        var bar = 1;
+    }
+};
+
+class AccessorTest {
+    get accesor1(): number {
+        var x = 0;
+        return x;
+    }
+
+    get accesor2(): number {
+        var x = 0;
+        return x;
+    }
+
+}
+
+class NoDupConstructor {
+    private test: string;
+    constructor() {
+        var test = "test";
+        this.test = test;
+    }
+}
+
+// valid/invalid code
+function letTesting() {
+    var a = 1;
+    let b = 1;
+    let d = 1;
+    if (true) {
+        let a = 2;
+        let b = 2;
+        let c = 2;
+        var d = 2;
+        var e = 2;
+    }
+    else {
+        let b = 3;
+        let c = 3;
+        let e = 3;
+        let f = 3;
+    }
+    var f = 4;
+}
+
+// failure: two arguments have the same name.
+function testArguments1(arg: number, arg: number): void {
+}
+
+// local var/let declarations shadow arguments. Use options "check-parameters" to catch this
+function testArguments2(x: number, y: number): void {
+    var x = 1;
+    let y = 2;
+}
+
+var references: {[vertex: string]: any};
+var dependents: {[vertex: string]: any};
+
+function blah(arg1: {[key: string]: any}, arg2: {[key:string]: any}) {
+}
+
+interface IClipboard {
+    copy(key: string, state: any): void;
+    paste(key: string): any;
+    findMaxOrMin(values: any[], defaultValue: number, operation: (...values: any[]) => number);
+    functionA: (value: string) => void;
+    functionB: (value: string) => void;
+}
+
+try {
+    //
+} catch (e) {
+    e.blah();
+    //
+}
+
+try {
+    //
+} catch (e) {
+    e.blah();
+    //
+}
+
+function testDestructuring() {
+    function myFunc() {
+        return [1, 2];
+    }
+
+    var [x, y] = myFunc();
+    var [z, z] = myFunc(); // failure
+            ~                         [err % ('z')]
+
+    let [x1, y1] = myFunc();
+    let [z1, z1] = myFunc(); // tsc error
+
+    const [x2, y2] = myFunc();
+    const [z2, z2] = myFunc(); // tsc error
+
+    let [a1, [b1, c1]] = [1, [2, 3]];
+    let [{a1, d1}] = [{a1: 1, d1: 4}]; // tsc error
+
+    var [a2, [b2, c2]] = [1, [2, 3]];
+    var [{a2, d2}] = [{a2: 1, d2: 4}]; // failure
+          ~~                                      [err % ('a2')]
+
+    function myFunc2([a, b]) {
+        var a;
+        return b;
+    }
+
+    var [x, y3] = myFunc(); // failure
+         ~                             [err % ('x')]
+    var [x3, ...y] = [1, 2, 3, 4]; // failure
+                ~                             [err % ('y')]
+}
+
+function compileErrors(foo, {bar}, bar, foo, {baz}, [baz]) {}
+
+[err]: Duplicate variable: '%s'