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

no-unused-variable rule should not be deprecated #4100

Closed
UselessPickles opened this issue Aug 6, 2018 · 28 comments
Closed

no-unused-variable rule should not be deprecated #4100

UselessPickles opened this issue Aug 6, 2018 · 28 comments

Comments

@UselessPickles
Copy link
Contributor

Bug Report

  • TSLint version: 5.11.0
  • TypeScript version: 2.9.2
  • Running TSLint via: CLI

Actual behavior

"no-unused-variable is deprecated. Since TypeScript 2.9. Please use the built-in compiler checks instead."

Expected behavior

This rule should not be deprecated.

The TypeScript compiler check is not a replacement for a linter. The lint rule has benefits that cannot be provided by the compiler option:

  • Linter supports "fix" behavior.
  • Linter supports enabling/disabling the rule with file, block, or line granularity, while the compiler option applies to the entire project.

Here's a comment from a TypeScript developer that is applicable: microsoft/TypeScript#11051 (comment)

@donaldpipowitch

This comment has been minimized.

@DannyDelott

This comment has been minimized.

@UselessPickles

This comment has been minimized.

@aindeev

This comment has been minimized.

@karfau

This comment has been minimized.

@chris-allnutt

This comment has been minimized.

@iensu

This comment has been minimized.

@ajafff

This comment has been minimized.

@iensu

This comment has been minimized.

@cyberhck

This comment has been minimized.

karfau added a commit to bettermarks/bm-tslint-rules that referenced this issue Sep 19, 2018
in agreement with palantir/tslint#4100
this ruleset doesn't force people to switch to compile error for getting rid of unused things,
this currently means paying the price of the deprecation warning
karfau added a commit to bettermarks/bm-tslint-rules that referenced this issue Sep 20, 2018
in agreement with palantir/tslint#4100
this ruleset doesn't force people to switch to compile error for getting rid of unused things,
this currently means paying the price of the deprecation warning
@JoshuaKGoldberg

This comment has been minimized.

@donaldpipowitch
Copy link
Contributor

If we would have access to the language service we could just use TypeScripts diagnostics instead of mirroring them, if I'm not mistaken? I don't know why, but it seems like access to the language service was removed in v5 (I once created an issue for this: #4232).

@glen-84

This comment has been minimized.

@Serginho

This comment has been minimized.

@dmccown1500

This comment has been minimized.

@lukewlms

This comment has been minimized.

@fibo

This comment has been minimized.

@JoshuaKGoldberg
Copy link
Contributor

Note: per #4534, this issue will be closed in less than a month if no PR is sent to add the rule. If you really need the rule, custom rules are always an option and can be maintained outside this repo!

@fibo

This comment has been minimized.

@cyberhck

This comment has been minimized.

@JoshuaKGoldberg
Copy link
Contributor

JoshuaKGoldberg commented Jul 4, 2019

To be clear, what we're accepting are PRs to resolve the technical limitations that originally caused the rule to be deprecated in the first place. Just removing the rule's deprecation is not enough.

tl;dr:

  • Everybody wants the rule to not be deprecated, maintainers included
  • There are technical blockers that need to be overcome to un-deprecate the rule
  • You can enable noUnusedLocals and noUnusedParameters in your tsconfig.json
  • Alternately, you can always fork TSLint and remove the deprecation warning in your fork

It's a little heartbreaking seeing everyone post how nice it would be to be able to use the rule. There's very little that can be done until the above blockers are either fixed or found to no longer be relevant. Per #4534 you should divert your energy towards the typescript-eslint project.

I'm going to mark supporting comments as off topic so folks are more likely to read this one.

Deprecation Reasoning

#3919: The latest versions of TS 2.9-nightly change how they report some diagnostics, this PR makes sure the rule doesn't throw given those changes... However, the changes in 2.9 make it much more complex for this rule to work as intended, especially with respect to the ignorePattern option.

#3918: The rule's original implementation didn't play well with other TS tooling and had to be disabled. #4232 tracks finding a way to enable it again.

So, we'd need to either:

  • Demonstrate a lack of issues with the rule as is (I'm having a hard time finding details...)
  • Completely re-implement the TypeScript compiler logic to detect unused variables
  • Find a way to expose the TypeScript language service to TSLint rules: Enable access to the LanguageService for rules #4232

(thanks @dakotahawkins for the obvious-in-hindsight suggestion to summarize the blockers here...)

/cc @adidahiya / @jkillian in case either of you have references for what the issues were?

@dakotahawkins

This comment has been minimized.

@JoshuaKGoldberg
Copy link
Contributor

💀 It's time! 💀

TSLint is being deprecated and no longer accepting pull requests for new rules. See #4534. 😱

If you'd like to see this rule implemented, you have two choices:

👋 It was a pleasure open sourcing with you!

If you believe this message was posted here in error, please comment so we can re-open the issue!

@JoshuaKGoldberg
Copy link
Contributor

Linking here: #3671 looks like good supporting docs for why it was deprecated.

@libinvarghese
Copy link

You could use tslint-consistent-codestyle no-unused rule instead

@beenotung
Copy link

@libinvarghese extends tslint-consistent-codestyle with rule no-unused seems cannot be auto fixed.
I'm extending tslint-etc with rule no-unused-declaration instead.

@JoshuaKGoldberg
Copy link
Contributor

🤖 Beep boop! 👉 TSLint is deprecated 👈 (#4534) 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 Mar 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.