Improve documentation for `typescript/no-unused-vars` about typescript-eslint HOT 28 CLOSED

Alright @bradzacher, I understand.

The issue is that typescript-eslint/no-unused-vars simply tag types and interfaces (that are imported or defined in the file) as unused, even if they are.

Well for my specific case I ended up with this workaround

overrides: [
    {
      files: ['*.ts', '*.tsx'],
      rules: {
        '@typescript-eslint/no-unused-vars': [2, { args: 'none' }]
      }
    }
  ]

which seems to fit my use case, but I think it should work without having to do this workaround.

If I find some time, I'll open an issue with minimal reproduction through a repository.

from typescript-eslint.

This is a very good point. I will continue working on this rule for as long as it's needed.

from typescript-eslint.

@bradzacher Does your last comment mean that the issue regarding no-unused-vars will be fixed in next release(s) ?

Cause it causes frustation to have to disable the no-unused-vars rule for the whole project (even JS files) to be able to lint without errors typescript files that are simply using type or interface definitions.

My, minimal reproduction is

import { IOptions } from './types'
const options: IOptions = {}
... // Use `options` later

resulting in

'IOptions' is defined but never used

Where should we follow the status on this ? We should have a unclosed issue pending until it's not fixed. After more than 1 hour figuring what's going on with the severals issues refering to no-unused-vars, this issue seemed the one where people exposed the more use cases which cause the lint issue.

from typescript-eslint.

Here's a scenario where it fails:

import { FieldRenderProps } from 'react-final-form'

const Error = ({ meta }: { meta: FieldRenderProps['meta'] }) => null

from typescript-eslint.

Think I found a few more.

import { Foo, Bar } from './types';

// Doesnt catch either mapped types 
const NAMES: { [name in Foo]: Bar } = {};

from typescript-eslint.

Another false positive:

declare type Checker = (v) => v is number;
const checker: Checker = (w): w is number => true;

Here w is used in the type guard, but according to the lint it's unused.

from typescript-eslint.

I think that TypeScript did not include noUnusedParameters and noUnusedLocals in strict because they see them more as style things. I think they might even regret having these options at all, as they are supposed to be linting issues and not type issues.

from typescript-eslint.

@ThomasdenH - please raise that error as a separate issue.
This issue is about documentation.

There is discussion about no-unused-vars in the recommended set in #122.

Considering we're working on improving this constantly, I'm going to close this issue.
The team is fixing edge cases and working for a better solution to this.

from typescript-eslint.

Thank you @kevinmarrec! It's working fine, now.

from typescript-eslint.

@kevinmarrec Thank you!

from typescript-eslint.

@JamesHenry We can close this issue now 😃

from typescript-eslint.

Actually I don't think we can? My PR added this rule to the list, but it doesn't add more detail as per this issue.

from typescript-eslint.

@JamesHenry can we consider removing this rule in favor of TypeScript own flags --noUnusedParameters and --noUnusedLocals?.

I think this rule is causing a lot of confusion and giving a lot of false/positives.
And more important, if we are already getting this out of box, then I don't see the point in keeping it... thoughts?

from typescript-eslint.

I appreciate having a no-unused-vars linter rule because I may not necessarily want tsc to return an error (one that might, say, fail a CI build) on something that I might consider more of a warning. (And there's unfortunately no way to configure the severity of the TypeScript flags.)

from typescript-eslint.

What is the correct setting for this rule? If I'm understanding correctly, it should prevent ESLint from erroneously throwing these errors, but I can't for the life of me to get it to work and since it's not operating as a rule so much as a workaround it's not clear to me what value I should set it to.

from typescript-eslint.

Hi @tenshiemi, this rule extends eslint/no-unused-vars to include TypeScript syntax.

You will need both defined in your eslint config to make it work. What I usually do is extend the eslint:recommended settings, then manually enable typescript/no-unused-vars

Hope this helps.

from typescript-eslint.

@weirdpattern Thanks, this is what I have both it throws a bunch of no-unused-vars and no-undef warnings:

{
    "parser": "typescript-eslint-parser",
    "plugins": [
        "react",
        "standard",
        "typescript",
        "prettier"
    ],
    "extends": [
        "eslint:recommended",
        "plugin:react/recommended",
        "prettier",
        "prettier/react",
        "prettier/standard"
    ],
    "parserOptions": {
        "sourceType": "module",
        "ecmaFeatures": {
            "jsx": true
        }
    },
    "rules": {
        "no-undef": "warn",
        "no-unused-vars": "warn",
        "typescript/no-use-before-define": "warn",
        "typescript/no-unused-vars": "warn",
        "prettier/prettier": ["error", {
            "parser": "typescript",
            "semi": true,
            "singleQuote": true,
            "tabWidth": 4,
            "trailingComma": "all"
        }]
    }

from typescript-eslint.

@tenshiemi, we are still working on this rule, we keep finding scenarios that we missed and have to be included.
Could you please provide a snippet of the code that is failing?

from typescript-eslint.

@weirdpattern I just realized it works when I run arc lint, just not when I run eslint directly on a file, so that's a relief!

an example is
10:18 warning 'SearchTableState' is not defined no-undef
when there is export interface SearchTableState { data: object[]; searchText: string; } in the code

from typescript-eslint.

Instead of creating a new issue, I'll just report some more findings.

1. "'Foo' is defined but never used". Doesn't catch extends in generics.

import { Foo } from './types';

class Bar<T extends Foo> {}

2. "'this' is defined but never used". This error comes from the base no-unused-vars rule. This disappears if I set the rule option args to "none".

import webpack from 'webpack';

export default function webpackLoader(this: webpack.loader.LoaderContext) {}

3. "'ExecaOptions' is defined but never used". Doesn't catch aliased imports.

import execa, { Options as ExecaOptions } from 'execa';

function foo(options: ExecaOptions) {}

4. "'Bar' is defined but never used". Doesn't catch intersection generics. (Happens a lot with React).

import { Foo, Bar } from './types';

class Baz<Foo & Bar> {}

from typescript-eslint.

As of the latest commit (5e410a36c43d82ab679bed8a8ef68a655008fde9), I'm seeing this snippet pass as a local no-unused-vars test case. Can add it to the codebase in a PR if that would be helpful.

import { FieldRenderProps } from 'react-final-form'

const Error = ({ meta }: { meta: FieldRenderProps['meta'] }) => null

I've confirmed the following as a failing test and will open a PR to address what I believe is the issue:

import { Foo } from './types';

class Bar<T extends Foo> {}

I'm not able to reproduce "'ExecaOptions' is defined by never used" in the following snippet. I see the following errors locally. @milesj do you have a reproducible test case of this issue, perhaps by adding a failing test in a PR? That would be awesome 😄

import execa, { Options as ExecaOptions } from 'execa';

function foo(options: ExecaOptions) {}

AssertionError [ERR_ASSERTION]: Should have no errors but had 3: 
[ { ruleId: 'no-unused-vars',
    severity: 1,
    message: '\'execa\' is defined but never used.',
    line: 2,
    column: 8,
    nodeType: 'Identifier',
    source: 'import execa, { Options as ExecaOptions } from \'execa\';',
    endLine: 2,
    endColumn: 13 },
  { ruleId: 'no-unused-vars',
    severity: 1,
    message: '\'foo\' is defined but never used.',
    line: 4,
    column: 10,
    nodeType: 'Identifier',
    source: 'function foo(options: ExecaOptions) {}',
    endLine: 4,
    endColumn: 13 },
  { ruleId: 'no-unused-vars',
    severity: 1,
    message: '\'options\' is defined but never used.',
    line: 4,
    column: 14,
    nodeType: 'Identifier',
    source: 'function foo(options: ExecaOptions) {}',
    endLine: 4,
    endColumn: 35 } ]

from typescript-eslint.

@macklinu Here's the execa full code: https://github.com/milesj/boost/blob/master/src/Routine.ts#L111 It's probably the wrapping parens + union causing it.

from typescript-eslint.

@milesj pushed up 9f0caf3 in #135 to address https://github.com/nzakas/eslint-plugin-typescript/issues/33#issuecomment-400063994.

from typescript-eslint.

Any update on this? If you're open to assigning collaborators, I could help go through some of these issues/PRs.

from typescript-eslint.

// warning 'B' is defined but never used
const a: Array<{b: B}> = [] 

from typescript-eslint.

Also, since this lint seems to be mostly covered by the Typescript option, maybe it's a good idea to remove it from /recommended?

from typescript-eslint.

@kevinmarrec - which issue? There are a number of edge cases that aren't properly covered by the rule. Some we know about, many we don't.

This issue is specifically about attempting to document the cases that are currently unsupported, but it has accidentally gone off the rails and become a place people are putting bug reports.

A growing thread which has a mixture between discussions, bug reports and examples is not a good way to document issues with the rule. One issue opened per bug, on the other hand, clearly documents what problems are known and open with the rule.

It's very hard to track which bugs are fixed if they are all logged as comments in a single thread: a thread isn't easily searchable, we can't close comments once their issue is fixed, and we can't clearly link to comments in PRs.

This the primary reason I've closed this issue.

Please open a new issue if you find a case that's not properly covered by the rule so that we can properly track the bug.
Please use the search first to double check that your bug isn't already logged.

from typescript-eslint.

@bradzacher : I had the same problem. The solution which @kevinmarrec provided worked for me as well. using 2.6.1 version of the plugin.

from typescript-eslint.