Thanks for the PR,@developer-bandi!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently onhttps://opencollective.com/typescript-eslint.

✅ Deploy Preview fortypescript-eslint ready!

Name	Link
🔨 Latest commit	702b828
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/662e656a32de340008a0189f
😎 Deploy Preview	https://deploy-preview-8640--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 99 (🟢 up 6 from production) Accessibility: 100 (no change from production) Best Practices: 92 (no change from production) SEO: 98 (no change from production) PWA: 80 (no change from production) View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to yourNetlify site configuration.

☁️ Nx Cloud Report

CI is running/has finished running commands for commit702b828. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

Sent with 💌 fromNxCloud.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.21%. Comparing base(216d1b0) to head(d95260e).

❗ Current headd95260e differs from pull request most recent head702b828. Consider uploading reports for the commit702b828 to get more accurate results

@@            Coverage Diff             @@##             main    #8640      +/-   ##==========================================- Coverage   87.40%   87.21%   -0.19%==========================================  Files         260      251       -9       Lines       12605    12308     -297       Branches     3937     3880      -57     ==========================================- Hits        11017    10735     -282+ Misses       1313     1303      -10+ Partials      275      270       -5

Hi@developer-bandi , I think it would be nice to explicitly specifyendColumn in some invalid test cases to guarantee the behavior and to prevent regressions! 👍

• no-unused-vars.test.ts

{messageId:'unusedVar',line:3,column:1,endColumn:3,<<<<<<<add `endColumn`onsometestcasesdata:{varName:'foo',action:'assigned a value',additional:'',},},

@yeonjuan thank you, i add test case withendline andendColumn that cover shorten error range

    {      code: `const foo: number = 1;      `,      errors: [        {          messageId: 'unusedVar',          data: {            varName: 'foo',            action: 'assigned a value',            additional: '',          },          line: 2,          column: 7,          endLine: 2,          endColumn:10,        },      ],    },

I believe this change brokeeslint-plugin-unused-imports, which relies onno-unused-vars from this plugin to pull the node and amend the error. Seehttps://github.com/sweepline/eslint-plugin-unused-imports/blob/master/lib/rules/load-rule.js for the code that loads this rule.

It now crashes here becausenode is undefined:https://github.com/sweepline/eslint-plugin-unused-imports/blob/master/lib/rules/predicates.js#L28

I don't see a good reason whynode has to be removed. Would you mind puttingnode back into thecontext.report call?

It does seem a very brittle that that plugin is depending on our private, internal implementation details like that.

For example if we were to change the rule to report on a different node then the rule would similarly break.

From the looks of it that's a horridly inefficient plugin too.
It utiliseseslint-rule-composer.
That util works by:

1. duplicating the rule by wrapping its create function in a new rule
2. monkey-patches thecontext object that ESLint passes so that it can intercept the reports generated by the wrapped rule.

(1) is especially bad because it means you have multiple copies of the rule. i.e. if you use bothunused-imports/no-unused-imports andunused-imports/no-unused-vars then you're running theno-unused-vars rule TWICE.

This is a really unstable setup and I'm really not sure if we should be supporting usage of our private internals like that.

I'm not sure I understand the point of the plugin - what is the usecase for separating out the imports from the regular variables? What's the benefit from working this way?

Yup, I get your argument. In this case you could argue it keeps API shape compatibility with the corresponding rule in eslint, if you need a reason.

The plugin comes with a fixer, which I run when hittingcmd+s in VS Code, and it removes all unused imports automatically. If you hoisted this feature into your version ofno-unused-vars, I can imagine droppingeslint-plugin-unused-imports.

Note that the ESLint plugin has almost 2m downloads per week, and I'm sure many are using it while also using typescript-eslint, so you'll likely hear from more people soon. I'm just the one who lives on the bleeding edge :D

cc@sweepline – would you be open to porting this rule intoeslint-plugin-unused-imports? If yes,@bradzacher would you be open to making a patch release that adds backnode until that happens?

@bradzacher we're using the plugin for auto-removing unused imports on save / build as well

I ran into the same issue, and now it's just messed up the whole plugin. I don't think we should be doing these breaking updates.

Let me be very clear:
We changed a private, explicitly not exposed to the public, internal implementation detail.

The other pluginmonkey patches eslint's API so it can hook into ourinternals.

There is no breaking change here.

If something builds on top of private implementation details and breaks when we change those details - that is not a breaking change.

That fact is not up for debate.

How to best remedy this is an open question that we shall discuss amongst the maintenance team and make a decision.

To be clear: the plugin in question is used by almost 10% oftypescript-eslint userbase. The plugin itself adds ~100 lines of code to automatically remove unused variables and imports, which is something those users need.

Is there any good reason such functionality shouldn't be a part oftypescript-eslint?

mabe typescript-eslint can set a new rule namedno-unused-imports and make it fixable. the fact is, I have three plugin have a rule named'no-unused-vars', and I also really need the auto-fixable ruleunused-imports
Besides, sometimesno-unused-imports is to noisy in editor, and that can refer to the solution of@antfu 's config, turn off the rule in editor and turn it on in the cli.

plugin in question is used by almost 10% oftypescript-eslint userbase

No, it's very unlikely that 1 in 10 of our users are usingeslint-plugin-unused-imports. npm download numbers are an inaccurate metric (reference: 2.3m for the plugin, 27m for us today). Another inaccurate measurement would be GitHub stars, 420 vs 14.6k today.

A more reliable metric would be something like Sourcegraph searches of open source repositories'package.json files today:

• 3.3k results foreslint-plugin-unused-imports
• 111.3k results for@typescript-eslint/eslint-plugin
  • 841 results for the newtypescript-eslint

At least in open source land, it seems more likely that theno-unused-imports plugin is floating around 3% of our users.

Is there any good reason such functionality shouldn't be a part oftypescript-eslint?

Two reasons. The first is that@typescript-eslint/no-unused-vars acts as an "extension rule" on top of the baseeslint/no-unused-vars rule. We don't add any major new functionality on top of base rules here. If you want that added, it'd be an issue for ESLint core.

The reason why ESLint core isn't enthusiastically adding in the automatic removal of values is that it's actually a tricky thing to get right. Seemingly unused imports and variables can actually have side effects that change your code's behavior - which is very dangerous to have in an automatic rule fixer!

Example of an import with a side effect:

// a.jsletcount=0;console.log("Gotcha!");exportconstgetCount=()=>count;

// b.js// Unused. Should we auto fix to...//  import {} from "./a.js" ?//  import "./a.js" ?//  nothing at all?import{getCount}from"./a.js";console.log("Hi!");

Example of a variable with a side effect:

letcount=0;exportfunctiongetNextCount(){return(count+=1);}// Unused. Should we auto-fix to...//  getNextCount()//  nothing at all?letunusedIsh=getNextCount();

Example of a variable without:

functionhasNoSideEffect(){for(leti=0;i<9001**7;i+=1){}}// Unused. Should we auto-fix to...//  hasNoSideEffect()//  nothing at all?letalsoUnusedIsh=hasNoSideEffect()

By using a plugin likeeslint-plugin-unused-imports, you explicitly are taking on the risk of changing code behavior due to side effects like that. Which, hey, might be a great sign about how you've structured your code 😄 nice job avoiding side effects! But lint rules can't make that assumption for the general case.

One thing lint rulescan do is addsuggestions: which are likefixes but won't be applied automatically.Rule Change: Add support for suggestions for unused var tracks adding that on the ESLint side. That issue was accepted, assigned, and has an open pull request now! 🙌

For more info on the difference between afix and asuggestion, see:

• https://eslint.org/docs/latest/use/core-concepts/#rule-fixes
• https://eslint.org/docs/latest/use/core-concepts/#rule-suggestions

Given that:

• Reports from our rules are an implementation detail not covered by our public API contract (seetypescript-eslint.io > Versioning >eslint-plugin and(I'll file an issue in a moment)Docs: Consider explicitly mentioning rule report properties as not covered by semver #9075 to make this more clear)
• eslint-plugin-unused-imports is stuck having to rely on the specifics of what the rule reports (there's nothing better itcould use)

...the "right" solution would be foreslint-plugin-unused-imports to work with what it's given: either theloc ornode depending on the version of@typescript-eslint/eslint-plugin. That's being tracked bysweepline/eslint-plugin-unused-imports#77, and has a PR open already insweepline/eslint-plugin-unused-imports#78.

Edit: that PR was merged the same minute as I posted this comment. Nice!
Edit 2:eslint-plugin-unused-imports@3.2.0 includes the fix. 👏

Anyway, perour PR contributing guide, a previously-merged PR isn't the right place to discuss changes. We'd generally ask that bug reports and feature requests be reported as a standalone new issue so they're more discoverable. We get that this was a blocker for everyone usingeslint-plugin-unused-imports so it's reasonable that the discussion ended up happening here. Thanks for the reports, all! ❤️

Locking this issue as this specific typescript-eslint PR doesn't need any more discussion. If more issues on the topic get filed I'll make sure they show up here. Cheers!

The issue with an autofixer is that a lot of people run them on save.

So imagine this:

1. you addimport foo from "foo" to your file
2. you save the file
3. you attempt to use the variablefoo
4. you get an error because the import was deleted in (2)

There's many variations of this situation - the development loop is an inherently messy thing and there's no way for a static analysis tool to delineate between "unused and it should be deleted" and "unused but it should not be deleted".

There's also the issue of side-effects that Josh mentioned. If you autofix delete an import or a variable - you can change runtime behaviour because side-effects are lost.

There actually used to be a fixer forno-unused-vars a long, long time ago but it was removed because people found it both annoying and dangerous.

There's always scope to introduce an optional fixer for people to opt-in to the danger / annoyance. But as Josh mentioned the rule is intended to be a TS-compatible replacement for the base rule. Whilst itis a complete rewrite - the options, features and reports of the rule are the same.
We don't like to break too far from the base rules for our extension rules because it places a heavy maintenance burden on us. The more forking we do; the harder it is to merge upstream changes.