Merge pull request #412 from adidahiya/destructuring-no-unused-parame…

…ters

Add destructuring syntax & support to no-unused-variable rule for parameter declarations

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 {

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);
+ });
+ }
 }

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,23 +139,35 @@ 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());
  }
 
  super.visitFunctionDeclaration(node);
  }
 
  public visitParameterDeclaration(node: ts.ParameterDeclaration): void {
- var nameNode = <ts.Identifier> 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 = <ts.Identifier> 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 = (<ts.Identifier> 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,28 +191,14 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker {
  var modifiers = node.modifiers;
  var variableName = (<ts.Identifier> node.name).text;
 
- if (this.hasModifier(modifiers, ts.SyntaxKind.PrivateKeyword)) {
+ if (Lint.hasModifier(modifiers, ts.SyntaxKind.PrivateKeyword)) {
  this.validateReferencesForVariable(variableName, node.name.getStart());
  }
  }
 
  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) {

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;
+ }
+}

test/rules/noUnusedVariableRuleTests.ts

@@ -88,14 +88,21 @@ describe("<no-unused-variable>", () => {
  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);
  });
 });