no-duplicate-variable: handle parameters

src/rules/noDuplicateVariableRule.ts

@@ -44,41 +44,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, undefined));
     }
 }
 
-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<void> {
+    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 (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);
+        };
 
-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);
+        return ts.forEachChild(sourceFile, cb);
+    }
+
+    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

@@ -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'