diff --git a/lib/tslint.d.ts b/lib/tslint.d.ts index 55bfdb6b594..82623c2e5f1 100644 --- a/lib/tslint.d.ts +++ b/lib/tslint.d.ts @@ -143,6 +143,7 @@ declare module Lint { function doesIntersect(failure: RuleFailure, disabledIntervals: Lint.IDisabledInterval[]): boolean; function abstract(): string; function scanAllTokens(scanner: ts.Scanner, callback: (scanner: ts.Scanner) => void): void; + function hasModifier(modifiers: ts.ModifiersArray, ...modifierKinds: ts.SyntaxKind[]): boolean; } declare module Lint { class RuleWalker extends Lint.SyntaxWalker { diff --git a/src/language/utils.ts b/src/language/utils.ts index ff0cc4d78b6..db4f10f65fc 100644 --- a/src/language/utils.ts +++ b/src/language/utils.ts @@ -76,4 +76,17 @@ module Lint { callback(scanner); } } + + /** + * @returns true if any modifier kinds passed along exist in the given modifiers array + */ + export function hasModifier(modifiers: ts.ModifiersArray, ...modifierKinds: ts.SyntaxKind[]) { + if (modifiers == null || modifierKinds == null) { + return false; + } + + return modifiers.some((m) => { + return modifierKinds.some((k) => m.kind === k); + }); + } } diff --git a/src/rules/noUnusedVariableRule.ts b/src/rules/noUnusedVariableRule.ts index 92cca411d49..012d7c1c326 100644 --- a/src/rules/noUnusedVariableRule.ts +++ b/src/rules/noUnusedVariableRule.ts @@ -53,7 +53,7 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker { } public visitImportDeclaration(node: ts.ImportDeclaration): void { - if (!this.hasModifier(node.modifiers, ts.SyntaxKind.ExportKeyword)) { + if (!Lint.hasModifier(node.modifiers, ts.SyntaxKind.ExportKeyword)) { var importClause = node.importClause; // named imports & namespace imports handled by other walker methods @@ -67,7 +67,7 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker { } public visitImportEqualsDeclaration(node: ts.ImportEqualsDeclaration): void { - if (!this.hasModifier(node.modifiers, ts.SyntaxKind.ExportKeyword)) { + if (!Lint.hasModifier(node.modifiers, ts.SyntaxKind.ExportKeyword)) { var name = node.name; this.validateReferencesForVariable(name.text, name.getStart()); } @@ -119,8 +119,7 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker { // skip exported and declared variables public visitVariableStatement(node: ts.VariableStatement): void { - if (this.hasModifier(node.modifiers, ts.SyntaxKind.ExportKeyword) - || this.hasModifier(node.modifiers, ts.SyntaxKind.DeclareKeyword)) { + if (Lint.hasModifier(node.modifiers, ts.SyntaxKind.ExportKeyword, ts.SyntaxKind.DeclareKeyword)) { this.skipBindingElement = true; this.skipVariableDeclaration = true; } @@ -140,7 +139,7 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker { public visitFunctionDeclaration(node: ts.FunctionDeclaration): void { var variableName = node.name.text; - if (!this.hasModifier(node.modifiers, ts.SyntaxKind.ExportKeyword)) { + if (!Lint.hasModifier(node.modifiers, ts.SyntaxKind.ExportKeyword)) { this.validateReferencesForVariable(variableName, node.name.getStart()); } @@ -148,15 +147,27 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker { } public visitParameterDeclaration(node: ts.ParameterDeclaration): void { - var nameNode = node.name; - var variableName = nameNode.text; + var isSingleVariable = node.name.kind === ts.SyntaxKind.Identifier; + var isPropertyParameter = Lint.hasModifier(node.modifiers, + ts.SyntaxKind.PublicKeyword, + ts.SyntaxKind.PrivateKeyword, + ts.SyntaxKind.ProtectedKeyword); - if (!this.hasModifier(node.modifiers, ts.SyntaxKind.PublicKeyword) - && !this.skipParameterDeclaration && this.hasOption(OPTION_CHECK_PARAMETERS)) { - this.validateReferencesForVariable(variableName, node.name.getStart()); + if (!isSingleVariable && isPropertyParameter) { + // tsc error: a parameter property may not be a binding pattern + this.skipBindingElement = true; + } + + if (this.hasOption(OPTION_CHECK_PARAMETERS) + && isSingleVariable + && !this.skipParameterDeclaration + && !Lint.hasModifier(node.modifiers, ts.SyntaxKind.PublicKeyword)) { + var nameNode = node.name; + this.validateReferencesForVariable(nameNode.text, node.name.getStart()); } super.visitParameterDeclaration(node); + this.skipBindingElement = false; } // check private member variables @@ -166,7 +177,7 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker { var variableName = ( node.name).text; // check only if an explicit 'private' modifier is specified - if (this.hasModifier(modifiers, ts.SyntaxKind.PrivateKeyword)) { + if (Lint.hasModifier(modifiers, ts.SyntaxKind.PrivateKeyword)) { this.validateReferencesForVariable(variableName, node.name.getStart()); } } @@ -180,7 +191,7 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker { var modifiers = node.modifiers; var variableName = ( node.name).text; - if (this.hasModifier(modifiers, ts.SyntaxKind.PrivateKeyword)) { + if (Lint.hasModifier(modifiers, ts.SyntaxKind.PrivateKeyword)) { this.validateReferencesForVariable(variableName, node.name.getStart()); } } @@ -188,20 +199,6 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker { super.visitMethodDeclaration(node); } - private hasModifier(modifiers: ts.ModifiersArray, modifierKind: ts.SyntaxKind) { - if (modifiers == null) { - return false; - } - for (var i = 0, n = modifiers.length; i < n; i++) { - var modifier = modifiers[i]; - if (modifier.kind === modifierKind) { - return true; - } - } - - return false; - } - private validateReferencesForVariable(name: string, position: number) { var highlights = this.languageService.getDocumentHighlights("file.ts", position, ["file.ts"]); if (highlights[0].highlightSpans.length <= 1) { diff --git a/test/files/rules/nounusedvariable-parameter.test.ts b/test/files/rules/nounusedvariable-parameter.test.ts index 6d521444590..f9e85f5aea0 100644 --- a/test/files/rules/nounusedvariable-parameter.test.ts +++ b/test/files/rules/nounusedvariable-parameter.test.ts @@ -1,12 +1,12 @@ -export function func1(x: number, y: number, ...args: number[]) { +export function func1(x: number, y: number, ...args: number[]) { // 'args' failure return x + y; } -export function func2(x: number, y: number, ...args: number[]) { +export function func2(x: number, y: number, ...args: number[]) { // 'y' failure return x + args[0]; } -export function func3(x?: number, y?: number) { +export function func3(x?: number, y?: number) { // 'y' failure return x; } @@ -15,7 +15,7 @@ export interface ITestInterface { } export class ABCD { - constructor(private x: number, public y: number, private z: number) { + constructor(private x: number, public y: number, private z: number) { // 'x' failure } func5() { @@ -34,3 +34,25 @@ export function func6(...args: number[]) { export function func7(f: (x: number) => number) { return f; } + +export function func8([x, y]: [number, number]) { // 'y' failure + return x; +} + +export class DestructuringTests { + constructor(public x: number, public [y, z]) { // tsc error on binding pattern + } + + public func9({a, b}) { // 'b' failure + return a; + } + + public func10([a, b]) { // 'b' failure + return [a]; + } + + // destructuring with default value + public func11([x = 0]) { // 'x' failure + return; + } +} diff --git a/test/rules/noUnusedVariableRuleTests.ts b/test/rules/noUnusedVariableRuleTests.ts index 9cab1c8d10d..c24ab11a319 100644 --- a/test/rules/noUnusedVariableRuleTests.ts +++ b/test/rules/noUnusedVariableRuleTests.ts @@ -88,14 +88,21 @@ describe("", () => { var failure2 = Lint.Test.createFailuresOnFile(fileName, Rule.FAILURE_STRING + "'y'")([5, 34], [5, 35]); var failure3 = Lint.Test.createFailuresOnFile(fileName, Rule.FAILURE_STRING + "'y'")([9, 35], [9, 36]); var failure4 = Lint.Test.createFailuresOnFile(fileName, Rule.FAILURE_STRING + "'x'")([18, 25], [18, 26]); + var failure5 = Lint.Test.createFailuresOnFile(fileName, Rule.FAILURE_STRING + "'y'")([38, 27], [38, 28]); + var failure6 = Lint.Test.createFailuresOnFile(fileName, Rule.FAILURE_STRING + "'b'")([46, 22], [46, 23]); + var failure7 = Lint.Test.createFailuresOnFile(fileName, Rule.FAILURE_STRING + "'b'")([50, 23], [50, 24]); + var failure8 = Lint.Test.createFailuresOnFile(fileName, Rule.FAILURE_STRING + "'x'")([55, 20], [55, 21]); var actualFailures = Lint.Test.applyRuleOnFile(fileName, Rule, [true, "check-parameters"]); - assert.lengthOf(actualFailures, 4); - + assert.lengthOf(actualFailures, 8); Lint.Test.assertContainsFailure(actualFailures, failure1); Lint.Test.assertContainsFailure(actualFailures, failure2); Lint.Test.assertContainsFailure(actualFailures, failure3); Lint.Test.assertContainsFailure(actualFailures, failure4); + Lint.Test.assertContainsFailure(actualFailures, failure5); + Lint.Test.assertContainsFailure(actualFailures, failure6); + Lint.Test.assertContainsFailure(actualFailures, failure7); + Lint.Test.assertContainsFailure(actualFailures, failure8); }); });