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

Start to deprecate no-unused-variable rule #1481

Closed
adidahiya opened this issue Aug 13, 2016 · 30 comments
Closed

Start to deprecate no-unused-variable rule #1481

adidahiya opened this issue Aug 13, 2016 · 30 comments

Comments

@adidahiya
Copy link
Contributor

adidahiya commented Aug 13, 2016

I just noticed these new compiler checks in v2.0: --noUnusedParameters and --noUnusedLocals. These essentially make the no-unused-variable rule obsolete. Since it's a very core rule, I don't think we should remove it right away.

Proposal:

  • remove no-unused-variable from tslint:latest & tslint:recommended in v4.0
  • mark the rule as deprecated in v4.0
    • maybe in the failure message?
  • remove the rule implementation completely in TSLint v5.0
@jkillian
Copy link
Contributor

jkillian commented Aug 19, 2016

A good feature here is that the compiler provides an opt-out: give a variable name a leading underscore and the compiler won't flag it. This should make it easier for people who want to enable the complier checks but may have an edge-case or two which they don't want to change.

@glen-84
Copy link
Contributor

glen-84 commented Aug 19, 2016

@jkillian That's only applicable to parameters, right?

@jkillian
Copy link
Contributor

@glen-84 - good catch, I think you're correct.

@charsleysa
Copy link

@adidahiya just to note that --noUnusedLocals and no-unused-variable operate quite differently.

The --noUnusedLocals checks class properties as well it seems (at TS version 2.0.2).

@adidahiya
Copy link
Contributor Author

@charsleysa yeah, the implementation is slightly different. I think the compiler options are better than the TSLint rule, which is why I want to favor it. As noted in the original issue post, we'll remove it slowly from TSLint core.

adidahiya added a commit that referenced this issue Oct 9, 2016
Resolves #1481. Adds a `deprecationMessage` field to rule metadata.
adidahiya added a commit that referenced this issue Oct 10, 2016
Resolves #1481. Adds a `deprecationMessage` field to rule metadata.
adidahiya added a commit that referenced this issue Oct 10, 2016
Resolves #1481. Adds a `deprecationMessage` field to rule metadata.
adidahiya added a commit that referenced this issue Oct 10, 2016
Resolves #1481. Adds a `deprecationMessage` field to rule metadata.
@adidahiya adidahiya added this to the TSLint v4.0 milestone Oct 10, 2016
Zalastax added a commit to Zalastax/openschedule that referenced this issue Oct 26, 2016
@michaeljota
Copy link

michaeljota commented Nov 6, 2016

I think you should remove it in version 4. There should be many preview releases where all the developers should see the deprecation warning. And you have removed other rules in favor of compiler options in anyway.

PD: In dev-1 release, I think this is as a recommended settings.

@ryanguill
Copy link

Just as a bit of feedback, I hit this issue tonight and even after finding this issue it took me a bit to understand that the --noUnusedLocals and --noUnusedParameters were tsc configs and not tslint. It might be useful to clarify that in the error message.

@nchen63
Copy link
Contributor

nchen63 commented Nov 19, 2016

@ryanguill thanks for the feedback. I committed a change that clarifies the message

mprobst added a commit to mprobst/tslint that referenced this issue Feb 27, 2017
As discussed in feedback on  palantir#1481 and palantir#1617, for many users `--noUnusedParameters` and `--noUnusedLocals` do not work as a replacement for `noUnusedVariable`. This change de-deprecates the rule and documents the alternative compiler flags (and why they might not work for users) in the description.

Fixes palantir#1617.
@adidahiya
Copy link
Contributor Author

Looks like there's a lot of interest in keeping this rule around. I went ahead and merged #2256, which un-deprecates the rule. It still won't be encouraged in the built-in configs, but it'll be available on an opt-in basis.

@ccorcos
Copy link

ccorcos commented Apr 19, 2017

Seems like if you your tsconfig has "jsx": "react" then tslint should throw an error for missing the react import.

@adidahiya
Copy link
Contributor Author

@ccorcos "missing import" is an entirely different error from "no unused variable". tsc ought to help you out there.

@ccorcos
Copy link

ccorcos commented Apr 19, 2017

sounds good. im just not seeing this error in VSCode... :/

@javier-tarazaga
Copy link

Hi @adidahiya ,

Thanks for taking this rule back. Question is, when will be released? As I can see is not yet included in 5.1.0.

@adidahiya
Copy link
Contributor Author

@javier-tarazaga it is there in v5, just as it's always been. it's not enabled in the tslint:recommended configuration though.

@javier-tarazaga
Copy link

Hi @adidahiya,

Uhm weird. When I include the rule in version of tslint 5.1.0 I get the following error,

Could not find implementations for the following rules specified in the configuration:
    no-unused-vars
Try upgrading TSLint and/or ensuring that you have all necessary custom rules installed.
If TSLint was recently upgraded, you may have old rules configured which need to be cleaned up.

@ajafff
Copy link
Contributor

ajafff commented May 2, 2017 via email

@javier-tarazaga
Copy link

Thanks for the heads up! Now I am facing #2650 so I cannot really verify it.

@quantuminformation
Copy link

quantuminformation commented Feb 21, 2018

I installed https://github.com/blakeembrey/tslint-config-standard
Is there anyway to get rid of these warnings, I'm not sure if is related to standard or just tslint

> [email protected] lint /Users/nikos/WebstormProjects/common
> tslint 'src/**/*.ts' 'test/**/*.ts'

Warning: The 'await-promise' rule requires type information.
Warning: The 'no-unused-variable' rule requires type information.
Warning: The 'no-use-before-declare' rule requires type information.
Warning: The 'return-undefined' rule requires type information.
Warning: The 'no-floating-promises' rule requires type information.
Warning: The 'no-unnecessary-qualifier' rule requires type information.
Warning: The 'no-unnecessary-type-assertion' rule requires type information.
Warning: The 'strict-type-predicates' rule requires type information.

@glen-84
Copy link
Contributor

glen-84 commented Mar 10, 2018

@quantuminformation Have you tried using --project?

@quantuminformation
Copy link

Yeah that works thx

@JoshuaKGoldberg
Copy link
Contributor

🤖 Beep boop! 👉 TSLint is deprecated 👈 and you should switch to typescript-eslint! 🤖

🔒 This issue is being locked to prevent further unnecessary discussions. Thank you! 👋

@palantir palantir locked and limited conversation to collaborators Sep 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests