Skip to content

[no-unused-vars] False positive because React JSXIdentifier nodes are ignored #868

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
octogonz opened this issue Aug 15, 2019 · 2 comments
Closed

[no-unused-vars] False positive because React JSXIdentifier nodes are ignored #868

octogonz opened this issue Aug 15, 2019 · 2 comments
Labels
package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look

Comments

@octogonz
Copy link
Contributor

Repro

{
  "rules": {
    "@typescript-eslint/no-unused-vars": [ "error" ]
  }
}
import * as React from 'react';

// ERROR: 'Foo' is defined but never used. [Error/@typescript-eslint/no-unused-vars]
class Foo extends React.Component { }

export class Bar extends React.Component {
  public render(): React.ReactNode {
    return <Foo />
  }
}

Expected Result

There should be no error for Foo, because it is used in the expression <Foo />.

Actual Result

The error is reported.

Additional Info

In the AST, the reference appears inside a JSXIdentifier node like this:

{
  "type": "ReturnStatement",
  "argument": {
	"type": "JSXElement",
	"openingElement": {
	  "type": "JSXOpeningElement",
	  "typeParameters": null,
	  "selfClosing": true,
	  "name": {
		"type": "JSXIdentifier",  // <====
		"name": "Foo"
	  },
	  "attributes": []
	},
	"closingElement": null,
	"children": []
  }
}

Whereas the rule only considers regular Identifier nodes:

typescript-eslint/packages/eslint-plugin/src/rules/no-unused-vars.ts

Lines 42 to 72 in 5eb40dc

return Object.assign({}, rules, {
'TSTypeReference Identifier'(node: TSESTree.Identifier) {
context.markVariableAsUsed(node.name);
},
TSInterfaceHeritage(node: TSESTree.TSInterfaceHeritage) {
if (node.expression) {
markHeritageAsUsed(node.expression);
}
},
TSClassImplements(node: TSESTree.TSClassImplements) {
if (node.expression) {
markHeritageAsUsed(node.expression);
}
},
'TSParameterProperty Identifier'(node: TSESTree.Identifier) {
// just assume parameter properties are used
context.markVariableAsUsed(node.name);
},
'TSEnumMember Identifier'(node: TSESTree.Identifier) {
context.markVariableAsUsed(node.name);
},
'*[declare=true] Identifier'(node: TSESTree.Identifier) {
context.markVariableAsUsed(node.name);
const scope = context.getScope();
const { variableScope } = scope;
if (variableScope !== scope) {
const superVar = variableScope.set.get(node.name);
if (superVar) {
superVar.eslintUsed = true;
}
}

Versions

package version
@typescript-eslint/eslint-plugin 2.0.0
@typescript-eslint/parser 2.0.0
TypeScript 3.5.2
ESLint 6.1.0
node 8.15.0
npm 6.4.1
@octogonz octogonz added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Aug 15, 2019
@bradzacher
Copy link
Member

This is a "known problem" of sorts.

The solution to this is to use the no-unused-vars rule from eslint-plugin-react.

We don't really want to encode JSX-specific information into the rule when there's another plugin which does it already.

This will be "fixed" by #688 though by virtue of the fact that the TS diagnostics has knowledge of JSX.

Prior art: #111, bradzacher/eslint-plugin-typescript#283

octogonz added a commit to octogonz/typescript-eslint that referenced this issue Aug 15, 2019
@octogonz
fix(eslint-plugin): [no-unused-vars] support React JSX (typescript-es…
01164e3 
…lint#868)
@octogonz
Copy link
Contributor Author

Cool, that works in my case because I am using React. (It may not be a good option for people using JSX with other libraries unrelated to React. #688 seems like a great long-term solution.)

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@octogonz @bradzacher