diff --git a/src/rules/noDuplicateVariableRule.ts b/src/rules/noDuplicateVariableRule.ts index c34687e6b87..a5c68acb346 100644 --- a/src/rules/noDuplicateVariableRule.ts +++ b/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 { - let scope = new Set(); - 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 { + private scope: Set; + 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); + } } } } diff --git a/test/rules/no-duplicate-variable/test.ts.lint b/test/rules/no-duplicate-variable/check-parameters/test.ts.lint similarity index 83% rename from test/rules/no-duplicate-variable/test.ts.lint rename to test/rules/no-duplicate-variable/check-parameters/test.ts.lint index f4e919eda8c..76fe1d878a0 100644 --- a/test/rules/no-duplicate-variable/test.ts.lint +++ b/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' \ No newline at end of file diff --git a/test/rules/no-duplicate-variable/check-parameters/tslint.json b/test/rules/no-duplicate-variable/check-parameters/tslint.json new file mode 100644 index 00000000000..3c4e30e3d0b --- /dev/null +++ b/test/rules/no-duplicate-variable/check-parameters/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "no-duplicate-variable": [true, "check-parameters"] + } +} diff --git a/test/rules/no-duplicate-variable/default/test.ts.lint b/test/rules/no-duplicate-variable/default/test.ts.lint new file mode 100644 index 00000000000..9676f576c3d --- /dev/null +++ b/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' \ No newline at end of file diff --git a/test/rules/no-duplicate-variable/tslint.json b/test/rules/no-duplicate-variable/default/tslint.json similarity index 100% rename from test/rules/no-duplicate-variable/tslint.json rename to test/rules/no-duplicate-variable/default/tslint.json