Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Commit

Permalink
no-duplicate-variable: handle parameters (#2597)
Browse files Browse the repository at this point in the history
[new-rule-option] `no-duplicate-variable` adds `check-parameters` to check if variable has the same name as a parameter
  • Loading branch information
ajafff authored and adidahiya committed May 31, 2017
1 parent d3177d3 commit 0295265
Show file tree
Hide file tree
Showing 5 changed files with 249 additions and 41 deletions.
85 changes: 52 additions & 33 deletions src/rules/noDuplicateVariableRule.ts
Expand Up @@ -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 = {
Expand All @@ -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,
};
Expand All @@ -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);
}
}
}
}
Expand Down
Expand Up @@ -2,6 +2,7 @@ var duplicated = 1;

function f(x) {
var x;
~ [err % ('x')]
}

class Test {
Expand All @@ -13,7 +14,7 @@ class Test {
};

var duplicated = null;
~~~~~~~~~~ [Duplicate variable: 'duplicated']
~~~~~~~~~~ [err % ('duplicated')]
}
}

Expand All @@ -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 {
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
Expand All @@ -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'
5 changes: 5 additions & 0 deletions test/rules/no-duplicate-variable/check-parameters/tslint.json
@@ -0,0 +1,5 @@
{
"rules": {
"no-duplicate-variable": [true, "check-parameters"]
}
}
177 changes: 177 additions & 0 deletions 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'

0 comments on commit 0295265

Please sign in to comment.