Skip to content

[no-unused-vars] More missing detections #45

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.

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

Closed
milesj opened this issue Jan 18, 2019 · 14 comments · Fixed by #102
Closed

[no-unused-vars] More missing detections #45

milesj opened this issue Jan 18, 2019 · 14 comments · Fixed by #102
Assignees
Labels
bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@milesj
Copy link
Contributor

milesj commented Jan 18, 2019

Repro

Run the linter on https://github.com/milesj/beemo

Some examples of it not working:

11:32  warning  'DriverContext' is defined but never used    no-unused-vars
6:10  warning  'Tool' is defined but never used          no-unused-vars
7:10  warning  'ExecaReturns' is defined but never used  no-unused-vars
  8:18  warning  'DriverArgs' is defined but never used  no-unused-vars
  9:10  warning  'BabelArgs' is defined but never used   no-unused-vars
  6:8  warning  'webpack' is defined but never used  no-unused-vars

Expected Result

They don't fail.

Actual Result

They do fail.

Additional Info

Versions

package version
eslint-plugin-typescript 0.14.0
typescript-eslint-parser 21.0.2
typescript 3.2.2
@bradzacher
Copy link
Member

Hi @milesj, could you please try upgrading to 1.0.0-rc.3 and see if that works for these cases?

Also just a tip, when linking to files in your repo, instead of linking to master, link to a commit. The commit link's content won't change, but the link to master will change when you push.

@j-f1
Copy link
Contributor

j-f1 commented Jan 18, 2019

(you can hit y to switch to a permalink for the current file)

@SevInf
Copy link

SevInf commented Jan 18, 2019

For me, this snippet still reports errors on 1.0.0-rc.3:

import { ExecutionResult } from 'graphql/execution/execute';
import { UnknownObject } from '../ts-utils';

export type RigelDispatch = <DataType extends UnknownObject>(
    action: Action<DataType>
) => Promise<ExecutionResult<DataType>>;
error  'UnknownObject' is defined but never used  typescript/no-unused-vars

@JamesHenry JamesHenry transferred this issue from bradzacher/eslint-plugin-typescript Jan 18, 2019
@bradzacher bradzacher added bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin awaiting response Issues waiting for a reply from the OP or another party labels Jan 18, 2019
@milesj
Copy link
Contributor Author

milesj commented Jan 18, 2019

I'll try the 1.0.0 version. typescript-eslint-parser has also been updated/deprecate, so I'll try that also.

@milesj
Copy link
Contributor Author

milesj commented Jan 19, 2019

Trying 1.0.0-rc3 and parser 22.0.0. It crashes on 2 repos with the following error:

TypeError: Cannot read property 'length' of undefined
    at TSInterfaceDeclaration (/Users/Miles/Sites/boost/node_modules/eslint-plugin-typescript/lib/rules/no-empty-interface.js:33:48)
    at listeners.(anonymous function).forEach.listener (/Users/Miles/Sites/boost/node_modules/eslint/lib/util/safe-emitter.js:45:58)
    at Array.forEach (<anonymous>)
    at Object.emit (/Users/Miles/Sites/boost/node_modules/eslint/lib/util/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/Users/Miles/Sites/boost/node_modules/eslint/lib/util/node-event-generator.js:251:26)
    at NodeEventGenerator.applySelectors (/Users/Miles/Sites/boost/node_modules/eslint/lib/util/node-event-generator.js:280:22)
    at NodeEventGenerator.enterNode (/Users/Miles/Sites/boost/node_modules/eslint/lib/util/node-event-generator.js:294:14)
    at CodePathAnalyzer.enterNode (/Users/Miles/Sites/boost/node_modules/eslint/lib/code-path-analysis/code-path-analyzer.js:632:23)
    at nodeQueue.forEach.traversalInfo (/Users/Miles/Sites/boost/node_modules/eslint/lib/linter.js:750:28)
    at Array.forEach (<anonymous>)

The line in no-empty-interfaces is the following, which is different than this repo. Seems like an AST/tree mismatch.

                const heritage = node.heritage.length;

                if (node.body.body.length === 0 && heritage < 2) {

Also noticed that the eslint-plugin-typescript has typescript-eslint-parser as a dependency, which it probably shouldn't? Or at minimum should be a peer. It also has a peer dependency on ~3.1.1, which will be annoying to maintain/consume, ^3.1.0 is much easier.

@armano2
Copy link
Collaborator

armano2 commented Jan 19, 2019

This is expected, there was a lot of changes in parser, and its not compatible with plugin, you have to wait for new release

@armano2
Copy link
Collaborator

armano2 commented Jan 19, 2019

Please read #73

@milesj
Copy link
Contributor Author

milesj commented Jan 19, 2019

Yup, assumed that was the problem. Will report back in a week.

@aserra54
Copy link

Not sure if I should piggy-back off of this issue, but another false negative I'm seeing is in interface declarations, e.g.

import { Foo, Bar } from 'qux';

export class Baz implements Foo {
  public bar: Bar;
}

Both Foo and Bar get tagged as being unused. Sample here:

https://github.com/aserra54/typescript-eslint-parser-no-unused-vars

@armano2
Copy link
Collaborator

armano2 commented Jan 21, 2019

@aserra54 you are missing plugin https://github.com/typescript-eslint/typescript-eslint/tree/master/packages/eslint-plugin

"extends": ["eslint:recommended", "plugin:@typescript-eslint/recommended"],

or atleast

{
  "parser": "@typescript-eslint/parser",
  "plugins": ["@typescript-eslint"],
  "rules": {
    "no-unused-vars": "off",
    "@typescript-eslint/no-unused-vars": "error"
  }
}

and you should move jsx to ecmaFeatures see https://github.com/typescript-eslint/typescript-eslint/tree/master/packages/parser#eslintrcjson

@milesj
Copy link
Contributor Author

milesj commented Jan 21, 2019

Tested with the new @typescript-eslint packages. All of them are fixed except one -- when referencing an imported namespace. For example, i18next here.

import i18next from 'i18next';

export interface Translator extends i18next.i18n {}

Seems to trigger on both rules.

/Users/Miles/Sites/boost/packages/core/src/types.ts
  8:8  warning  'debug' is defined but never used    no-unused-vars
  8:8  error    'debug' is defined but never used    @typescript-eslint/no-unused-vars
  9:8  warning  'i18next' is defined but never used  no-unused-vars
  9:8  error    'i18next' is defined but never used  @typescript-eslint/no-unused-vars

@armano2 armano2 removed the awaiting response Issues waiting for a reply from the OP or another party label Jan 21, 2019
@armano2 armano2 self-assigned this Jan 21, 2019
@SimenB
Copy link
Contributor

SimenB commented Jan 21, 2019

I'm getting false positives on React components.

// react.tsx
import * as React from 'react';
import { Heading2 } from 'some-place';

export const Comp: React.FunctionComponent = () => (
  <Heading2>Some content</Heading2>
);
/Users/simen/repos/monorepo/react.tsx
  2:10  warning  'Heading2' is defined but never used  @typescript-eslint/no-unused-vars

✖ 1 problem (0 errors, 1 warning)

config:

{
  "extends": ["plugin:@typescript-eslint/recommended"],
  "plugins": ["@typescript-eslint"]
}

(this creates a couple of more errors for rules I've turned off)

@armano2
Copy link
Collaborator

armano2 commented Jan 21, 2019

You have to install and use eslint plugin React

@SimenB
Copy link
Contributor

SimenB commented Jan 21, 2019

Ah! Fair enough 🙂 I've forgotten all about that plugin, gonna be awesome to be able to use it again 😅

@typescript-eslint typescript-eslint locked as resolved and limited conversation to collaborators Feb 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants