Skip to content
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

[@typescript-eslint/no-unused-vars] does not work? #4617

Closed
3 tasks done
AnderssonPeter opened this issue Mar 1, 2022 · 4 comments
Closed
3 tasks done

[@typescript-eslint/no-unused-vars] does not work? #4617

AnderssonPeter opened this issue Mar 1, 2022 · 4 comments
Labels
package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin working as intended Issues that are closed as they are working as intended

Comments

@AnderssonPeter
Copy link

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have read the FAQ and my problem is not listed.

Repro

{
  "rules": {
     "no-unused-vars": "off",
     "@typescript-eslint/no-unused-vars": ["warn", { "vars": "all", "args": "all" }],
  }
}
@Injectable()
export class BackendCommunicationService {
    private keepRotating = false; <-- unused

    constructor(
        private logger: Logger, <-- unused
        private database: Database, <-- unused
        private http: HttpClient
    ) {}

tsconfig

{
  "compileOnSave": false,
  "compilerOptions": {
    "baseUrl": "./",
    "outDir": "./dist/out-tsc",
    "forceConsistentCasingInFileNames": true,
    "strict": true,
    "noImplicitOverride": true,
    "noPropertyAccessFromIndexSignature": true,
    "noImplicitReturns": true,
    "noFallthroughCasesInSwitch": true,
    "sourceMap": true,
    "declaration": false,
    "downlevelIteration": true,
    "experimentalDecorators": true,
    "moduleResolution": "node",
    "importHelpers": true,
    "target": "es2021",
    "module": "es2020",
    "lib": [
      "es2021",
      "dom"
    ]
  },
  "angularCompilerOptions": {
    "enableI18nLegacyMessageIdFormat": false,
    "strictInjectionParameters": true,
    "strictInputAccessModifiers": true,
    "strictTemplates": true
  }
}

Expected Result

It should warn me about the unused fields/properties.

Actual Result

No warning is given

Additional Info

I'm using Angular 13, How do i generate a debug lint? (Normally I just run ng lint but it doesn't seem to have a debug flag

Versions

package version
@typescript-eslint/eslint-plugin 5.13.0
@typescript-eslint/parser 5.13.0
TypeScript 4.5.5
ESLint 8.10.0
node 14.17.6
@AnderssonPeter AnderssonPeter added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels Mar 1, 2022
@JoshuaKGoldberg
Copy link
Member

no-unused-vars is specifically for variables, not class properties. https://typescript-eslint.io/rules/no-unused-vars -> https://eslint.org/docs/rules/no-unused-vars

I think it'd be reasonable to add in a rule like no-unused-class-members that suggests removing private ones. Note that it'd have to be a suggestion rather than a fix (https://eslint.org/docs/developer-guide/working-with-rules): #4571 (comment).

#private members are completely safe to mark as unused because they are 100%, runtime enforced to be only accessible within the class they are defined.

However private members are not. They are accessible outside the class via instance['member'] (yes, that's valid and safe TS). And are ultimately just type-only sugar for a normal class member.

Which means we cannot statically detect the valid usages across module boundaries.

This is generally I would recommend using #private instead of private in all modern code.

In the meantime TypeScript's built-in noUnusedLocals setting can catch unused class members.

cc @bradzacher since I know you've worked around this area a bunch in the past.

@bradzacher
Copy link
Member

Yeah we definitely could implement a private class member rule as suggested in #4571. It's probably a good idea so that we can further help people migrate off of the TS flag.

Though this definitely shouldn't be implemented within no-unused-vars - as you mention it's for variables, and properties aren't variables.

So I'll close this and tag #4571 as accepting PRs.

@bradzacher bradzacher added working as intended Issues that are closed as they are working as intended and removed triage Waiting for maintainers to take a look labels Mar 1, 2022
@ChristianHardy
Copy link

Greetings, I have an error using no-unused-vars @bradzacher can you give me a hand with this?

image

@JoshuaKGoldberg
Copy link
Member

@ChristianHardy please don't post on closed, unrelated issues. That spams the thread and is not very discoverable for other folks who have your issue.

For info on the @typescript-eslint/no-unused-vars rule (hint: and how to configure it), please see https://typescript-eslint.io/rules/no-unused-vars.

@typescript-eslint typescript-eslint locked as off-topic and limited conversation to collaborators Mar 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin working as intended Issues that are closed as they are working as intended
Projects
None yet
Development

No branches or pull requests

4 participants