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

Fix no-unused-variable "react" option to support self-closing JSX elements #776

Merged
merged 1 commit into from
Nov 13, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 80 additions & 75 deletions src/rules/noUnusedVariableRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,6 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker {
}
}

public visitJsxElement(node: ts.JsxElement) {
this.hasSeenJsxElement = true;
super.visitJsxElement(node);
}

public visitBindingElement(node: ts.BindingElement) {
const isSingleVariable = node.name.kind === ts.SyntaxKind.Identifier;

Expand All @@ -98,6 +93,28 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker {
super.visitBindingElement(node);
}

public visitCatchClause(node: ts.CatchClause) {
// don't visit the catch clause variable declaration, just visit the block
// the catch clause variable declaration needs to be there but doesn't need to be used
this.visitBlock(node.block);
}

// skip exported and declared functions
public visitFunctionDeclaration(node: ts.FunctionDeclaration) {
if (!Lint.hasModifier(node.modifiers, ts.SyntaxKind.ExportKeyword, ts.SyntaxKind.DeclareKeyword)) {
const variableName = node.name.text;
this.validateReferencesForVariable(variableName, node.name.getStart());
}

super.visitFunctionDeclaration(node);
}

public visitFunctionType(node: ts.FunctionOrConstructorTypeNode) {
this.skipParameterDeclaration = true;
super.visitFunctionType(node);
this.skipParameterDeclaration = false;
}

public visitImportDeclaration(node: ts.ImportDeclaration) {
if (!Lint.hasModifier(node.modifiers, ts.SyntaxKind.ExportKeyword)) {
const importClause = node.importClause;
Expand All @@ -121,10 +138,47 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker {
super.visitImportEqualsDeclaration(node);
}

public visitCatchClause(node: ts.CatchClause) {
// don't visit the catch clause variable declaration, just visit the block
// the catch clause variable declaration needs to be there but doesn't need to be used
this.visitBlock(node.block);
// skip parameters in index signatures (stuff like [key: string]: string)
public visitIndexSignatureDeclaration(node: ts.IndexSignatureDeclaration) {
this.skipParameterDeclaration = true;
super.visitIndexSignatureDeclaration(node);
this.skipParameterDeclaration = false;
}

// skip parameters in interfaces
public visitInterfaceDeclaration(node: ts.InterfaceDeclaration) {
this.skipParameterDeclaration = true;
super.visitInterfaceDeclaration(node);
this.skipParameterDeclaration = false;
}

public visitJsxElement(node: ts.JsxElement) {
this.hasSeenJsxElement = true;
super.visitJsxElement(node);
}

public visitJsxSelfClosingElement(node: ts.JsxSelfClosingElement) {
this.hasSeenJsxElement = true;
super.visitJsxSelfClosingElement(node);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this method was added. the others were reordered to be alphabetized.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - this was indeed the bug. You beat me to a PR 👍


// check private member functions
public visitMethodDeclaration(node: ts.MethodDeclaration) {
if (node.name != null && node.name.kind === ts.SyntaxKind.Identifier) {
const modifiers = node.modifiers;
const variableName = (<ts.Identifier> node.name).text;

if (Lint.hasModifier(modifiers, ts.SyntaxKind.PrivateKeyword)) {
this.validateReferencesForVariable(variableName, node.name.getStart());
}
}

// abstract methods can't have a body so their parameters are always unused
if (Lint.hasModifier(node.modifiers, ts.SyntaxKind.AbstractKeyword)) {
this.skipParameterDeclaration = true;
}
super.visitMethodDeclaration(node);
this.skipParameterDeclaration = false;
}

public visitNamedImports(node: ts.NamedImports) {
Expand Down Expand Up @@ -156,59 +210,6 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker {
super.visitNamespaceImport(node);
}

public visitVariableDeclaration(node: ts.VariableDeclaration) {
const isSingleVariable = node.name.kind === ts.SyntaxKind.Identifier;

if (isSingleVariable && !this.skipVariableDeclaration) {
const variableIdentifier = <ts.Identifier> node.name;
this.validateReferencesForVariable(variableIdentifier.text, variableIdentifier.getStart());
}

super.visitVariableDeclaration(node);
}

// skip parameters in interfaces
public visitInterfaceDeclaration(node: ts.InterfaceDeclaration) {
this.skipParameterDeclaration = true;
super.visitInterfaceDeclaration(node);
this.skipParameterDeclaration = false;
}

// skip parameters in index signatures (stuff like [key: string]: string)
public visitIndexSignatureDeclaration(node: ts.IndexSignatureDeclaration) {
this.skipParameterDeclaration = true;
super.visitIndexSignatureDeclaration(node);
this.skipParameterDeclaration = false;
}

// skip exported and declared variables
public visitVariableStatement(node: ts.VariableStatement) {
if (Lint.hasModifier(node.modifiers, ts.SyntaxKind.ExportKeyword, ts.SyntaxKind.DeclareKeyword)) {
this.skipBindingElement = true;
this.skipVariableDeclaration = true;
}

super.visitVariableStatement(node);
this.skipBindingElement = false;
this.skipVariableDeclaration = false;
}

public visitFunctionType(node: ts.FunctionOrConstructorTypeNode) {
this.skipParameterDeclaration = true;
super.visitFunctionType(node);
this.skipParameterDeclaration = false;
}

// skip exported and declared functions
public visitFunctionDeclaration(node: ts.FunctionDeclaration) {
if (!Lint.hasModifier(node.modifiers, ts.SyntaxKind.ExportKeyword, ts.SyntaxKind.DeclareKeyword)) {
const variableName = node.name.text;
this.validateReferencesForVariable(variableName, node.name.getStart());
}

super.visitFunctionDeclaration(node);
}

public visitParameterDeclaration(node: ts.ParameterDeclaration) {
const isSingleVariable = node.name.kind === ts.SyntaxKind.Identifier;
const isPropertyParameter = Lint.hasModifier(
Expand Down Expand Up @@ -250,23 +251,27 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker {
super.visitPropertyDeclaration(node);
}

// check private member functions
public visitMethodDeclaration(node: ts.MethodDeclaration) {
if (node.name != null && node.name.kind === ts.SyntaxKind.Identifier) {
const modifiers = node.modifiers;
const variableName = (<ts.Identifier> node.name).text;
public visitVariableDeclaration(node: ts.VariableDeclaration) {
const isSingleVariable = node.name.kind === ts.SyntaxKind.Identifier;

if (Lint.hasModifier(modifiers, ts.SyntaxKind.PrivateKeyword)) {
this.validateReferencesForVariable(variableName, node.name.getStart());
}
if (isSingleVariable && !this.skipVariableDeclaration) {
const variableIdentifier = <ts.Identifier> node.name;
this.validateReferencesForVariable(variableIdentifier.text, variableIdentifier.getStart());
}

// abstract methods can't have a body so their parameters are always unused
if (Lint.hasModifier(node.modifiers, ts.SyntaxKind.AbstractKeyword)) {
this.skipParameterDeclaration = true;
super.visitVariableDeclaration(node);
}

// skip exported and declared variables
public visitVariableStatement(node: ts.VariableStatement) {
if (Lint.hasModifier(node.modifiers, ts.SyntaxKind.ExportKeyword, ts.SyntaxKind.DeclareKeyword)) {
this.skipBindingElement = true;
this.skipVariableDeclaration = true;
}
super.visitMethodDeclaration(node);
this.skipParameterDeclaration = false;

super.visitVariableStatement(node);
this.skipBindingElement = false;
this.skipVariableDeclaration = false;
}

private validateReferencesForVariable(name: string, position: number) {
Expand Down
3 changes: 3 additions & 0 deletions test/files/rules/nounusedvariable-react4-react.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import * as React from "react";

console.log(<input type="text" />);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no newline at end of file

4 changes: 4 additions & 0 deletions test/rules/noUnusedVariableRuleTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,10 @@ describe("<no-unused-variable>", () => {

describeSuite("react");
describeSuite("react/addons");

it("should not fail if JSX self-closing element is used", () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do this like the other tests for consistency?

assertNoFailures("rules/nounusedvariable-react4-react.test.tsx", true);
});
});

describe("not set", () => {
Expand Down