importUnused1from'foo';importUnused2,{Used}from'bar';export{Used};import{Used}from'bar';export{Used};// -------------------import{Unused1}from'foo';importUsed1,{Unused2}from'bar';import{Used2,Unused3}from'baz';importUsed3,{Unused4,Used4}from'foobar';export{Used1,Used2,Used3,Used4};importUsed1from'bar';import{Used2}from'baz';importUsed3,{Used4}from'foobar';export{Used1,Used2,Used3,Used4};// -----------------import{/* cmt */Unused1,Used1}from'foo';export{Used1};import{/* cmt */Used1}from'foo';export{Used1};// ------------------importtype{UnusedType}from'foo';import{Used1,Unused1}from'foo';export{Used1};import{Used1}from'foo';export{Used1};// -----------------import{Unused1asu1,Used1asu2}from'foo';export{u2};import{Used1asu2}from'foo';export{u2};

nayounsang added2 commits

May 24, 2025 00:49

test: add testcase

e5394b2

feat: test successful logic

e339973

Copy link

Contributor

typescript-eslintbot commentedMay 23, 2025

Thanks for the PR,@nayounsang!

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 on https://opencollective.com/typescript-eslint.

Copy link

netlifybot commentedMay 23, 2025•
edited
Loading

✅ Deploy Preview fortypescript-eslint ready!

Name	Link
🔨 Latest commit	`eb3fd2b`
🔍 Latest deploy log	https://app.netlify.com/projects/typescript-eslint/deploys/6874a6c99b97730008ee5601
😎 Deploy Preview	https://deploy-preview-11243--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 98 (🟢 up 9 from production) Accessibility: 97 (no change from production) Best Practices: 100 (no change from production) SEO: 92 (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 project configuration.

nayounsang marked this pull request as draft

May 23, 2025 15:53

Copy link

nx-cloudbot commentedMay 23, 2025•
edited
Loading

View yourCI Pipeline Execution ↗ for commiteb3fd2b

Command	Status	Duration	Result
`nx test eslint-plugin --coverage=false`	✅ Succeeded	5m 6s	View ↗
`nx run-many -t lint`	✅ Succeeded	3m 11s	View ↗
`nx run-many -t typecheck`	✅ Succeeded	2m 5s	View ↗
`nx test eslint-plugin-internal --coverage=false`	✅ Succeeded	5s	View ↗
`nx run integration-tests:test`	✅ Succeeded	4s	View ↗
`nx test typescript-estree --coverage=false`	✅ Succeeded	2s	View ↗
`nx run types:build`	✅ Succeeded	5s	View ↗
`nx run generate-configs`	✅ Succeeded	6s	View ↗
`Additional runs (27)`	✅ Succeeded	...	View ↗

☁️Nx Cloud last updated this comment at2025-07-14 06:53:42 UTC

Copy link

ContributorAuthor

nayounsang commentedMay 23, 2025

Wow, when adding a new feature, a follow-up development is needed. I'll work on it tomorrow.

chore: generate schema

7fc5d69

Copy link

codecovbot commentedMay 24, 2025•
edited
Loading

Codecov Report

Attention: Patch coverage is94.52055% with4 lines in your changes missing coverage. Please review.

Project coverage is 90.85%. Comparing base(16c344e) to head(eb3fd2b).
Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
packages/eslint-plugin/src/rules/no-unused-vars.ts	94.52%	4 Missing⚠️

Additional details and impacted files

@@           Coverage Diff           @@##             main   #11243   +/-   ##=======================================  Coverage   90.85%   90.85%           =======================================  Files         501      501             Lines       50987    51060   +73       Branches     8410     8430   +20     =======================================+ Hits        46324    46393   +69- Misses       4648     4652    +4  Partials       15       15

Flag	Coverage Δ
unittest	`90.85% <94.52%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown.Click here to find out more.

Files with missing lines	Coverage Δ
packages/eslint-plugin/src/rules/no-unused-vars.ts	`99.41% <94.52%> (-0.59%)`	⬇️

🚀 New features to boost your workflow:

❄️Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nayounsang commented

May 24, 2025

View reviewed changes

packages/eslint-plugin/tests/rules/no-unused-vars/no-unused-vars.test.ts OutdatedShow resolvedHide resolved

nayounsangand others added9 commits

June 3, 2025 17:42

chore: Revert "test: add testcase"

7b06e7c

This reverts commite5394b2.

chore: remove uninteded code changes

a2dce5f

refactor: simplify conditional statements to increase converage

855acdf

test: add testacase autofixer should do nothing in import-autofix fea…

233fb1e

…ture

test: add more testcases

622aaf0

refactor: remove duplicate condition validate

122509f

test: add more test case

556ae92

chore: logic that is currently difficult to implement is left as TODO

6d35a71

Merge branch 'main' into auto-unused

440d112

nayounsang marked this pull request as ready for review

June 5, 2025 06:13

chore: add line for test category

d9f1609

kirkwaiblinger requested changes

Jun 6, 2025

View reviewed changes

Copy link

Member

kirkwaiblinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

A great start!

I'm leaving a bit of a drive-by review since I see that there are several major TODOs in the code that need to get resolved. Feel free to reach out if you're looking for help with those! In the meantime, I'm going to leave a Changes Requested review so that this isn't in our ready-to-review queue.

Thanks!

packages/eslint-plugin/src/rules/no-unused-vars.ts Outdated


		const source = context.sourceCode;
		const node = def.node;
		const decl = node.parent as TSESTree.ImportDeclaration;

Copy link

Member

kirkwaiblingerJun 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I think that this will be unsound forimport = require-style imports, for example

importx= require('foo')

Let's be sure to include some test coverage with that style ofimport.

Copy link

ContributorAuthor

nayounsangJun 6, 2025•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Oh, nice catch. This isTSImportEqualsDeclaration type.
I test with this code:

{code:`import x = require('foo');import y = require('bar');export { y };      `,errors:[{messageId:'unusedVar'}],options:[{enableAutofixRemoval:{imports:true}}],output:`import y = require('bar');export { y };      `,}

In the existing logic, an error occurs and a more accurate logic is used rather than type casting.
related commit:11ac4fa

packages/eslint-plugin/src/rules/no-unused-vars.ts Outdated

		@@ -687,6 +708,80 @@ export default createRule<Options, MessageIds>({
		data: unusedVar.references.some(ref => ref.isWrite())
		? getAssignedMessageData(unusedVar)
		: getDefinedMessageData(unusedVar),
		fix: options.enableAutofixRemoval?.imports

Copy link

Member

kirkwaiblingerJun 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This seems useful as a suggestion, even if the autofix option is disabled. Consider usinggetFixOrSuggest to conditionally provide this fix as a suggestion?

typescript-eslint/packages/eslint-plugin/src/util/getFixOrSuggest.ts

Lines 3 to 21 in5b2ebcd

	exportfunctiongetFixOrSuggest<MessageIdextendsstring>({
	fixOrSuggest,
	suggestion,
	}:{
	fixOrSuggest:'fix'\|'none'\|'suggest';
	suggestion:TSESLint.SuggestionReportDescriptor<MessageId>;
	}):
	\|{fix:TSESLint.ReportFixFunction}
	\|{suggest:TSESLint.SuggestionReportDescriptor<MessageId>[]}
	\|undefined{
	switch(fixOrSuggest){
	case'fix':
	return{fix:suggestion.fix};
	case'none':
	returnundefined;
	case'suggest':
	return{suggest:[suggestion]};
	}
	}

Copy link

ContributorAuthor

nayounsangJun 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Good idea. However, it would be better to provide suggestions to existing test cases and then resolvethis comment before processing.

packages/eslint-plugin/src/rules/no-unused-vars.ts Outdated

		if (decl.specifiers.length === 1) {
		return fixer.removeRange([
		decl.range[0],
		decl.range[1] + 1, // +1 to include "\n"

Copy link

Member

kirkwaiblingerJun 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

to include "\n"

Why do we want to do this exactly? This causes a bug with this technically valid code (please include as a test case - hint, you'll need to usenoFormat):

importxfrom'foo';importyfrom'bar'

My instinct is to say we're better off just leaving a blank line and letting the user decide how to format it afterwards (whether by hand or with prettier or similar).

Copy link

ContributorAuthor

nayounsangJun 6, 2025•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

we're better off just leaving a blank line and letting the user decide how to format it afterwards (whether by hand or with prettier or similar).

Wow, that's right. That should have been it. Plus, it lowers the difficulty of solving the problem and the code becomes simpler.
related commit:9f76441 &633d2c8

Copy link

ContributorAuthor

nayounsangJun 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Add new message:unusedVarSuggestion. However, I'm concerned about the proposed message:Remove unused variable.. Any thoughts?

related commit:2c8b740 &9a15d49

kirkwaiblinger added the awaiting responseIssues waiting for a reply from the OP or another party label

Jun 6, 2025

nayounsang added3 commits

June 6, 2025 18:20

fix: remove type casting and support TSImportEqulasDeclaration node

11ac4fa

test: simplify test case with noFormat

633d2c8

fix: remove format-related logic

9f76441

nayounsang commented

Jun 6, 2025

View reviewed changes

packages/eslint-plugin/tests/rules/no-unused-vars/no-unused-vars.test.ts OutdatedShow resolvedHide resolved

nayounsangand others added3 commits

June 8, 2025 21:53

Merge branch 'main' into auto-unused

99699fc

feat: add suggestion for enableAUtofixRemoval

2c8b740

test: add suggestion effected by import statement

9a15d49

nayounsangand others added5 commits

June 8, 2025 23:43

test: add suggestion for no-unused-vars-eslint test

6de9050

feat: remove all unused specifers at once if no used specifers in decl

a177e5c

test: add test

1a05465

Merge branch 'main' into auto-unused

5662583

chore: resolve real conflict

90c2d9a

Copy link

ContributorAuthor

nayounsang commentedJun 11, 2025•
edited
Loading

@kirkwaiblinger Hi. I committed my code about your review and TODOs! Please make sure I'm on the right track.
The problem to be solved now is: Problem with unformatted code. (Based on 0-index) Odd specifiers are not deleted.
Um... I run console.log in the local and see results of the ranges need to be deleted, and in this case, the range to delete did not conflict! I don't know why.

// test caseimport{A,B,C,D,E,F,G,Used1,H,I,J,K}from'foo';export{Used1};// expectimport{Used1}from'foo';export{Used1};// receivedimport{B,D,F,Used1,I,K}from'foo'export{Used1};

How can I solve this problem? My idea is to merge the contiguous segments and remove multiple specifiers at once.
For ex

import{A,B,C,D,E,F,G,Used1,H,I,J,K}from'foo';// ->import{Used1,H,I,J,K}from'foo';// ->import{Used1}from'foo';

nayounsang commented

Jul 5, 2025

View reviewed changes

Copy link

ContributorAuthor

nayounsang left a comment•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I'm going to try something different than my previous commit.This is reason & plan:1. Fixing multiple specifers at once is confusing and incompatible with the existing way of reporting.2. Test cases that test for multiple types of imports are more suitable for Unit testing.

Done, I apologize to the reviewer for the change, but I think this is the right direction.

Merge branch 'main' into auto-unused

80ec92b

bradzacher changed the title~~feat(eslint-plugin): add a default-off option to autofix remove unused imports~~feat(eslint-plugin): [no-unused-vars] add a default-off option to autofix remove unused imports

Jul 9, 2025

bradzacher removed the awaiting responseIssues waiting for a reply from the OP or another party label

Jul 13, 2025

nayounsangand others added2 commits

July 14, 2025 14:35

Merge branch 'main' into auto-unused

8759180

fix: simplyfy logic and modify tc to fit unit tests

eb3fd2b

nayounsang requested a review fromkirkwaiblinger

July 14, 2025 06:53

Labels

None yet

3 participants

Movatterモバイル変換

Uh oh!

feat(eslint-plugin): [no-unused-vars] add a default-off option to autofix remove unused imports#11243

Are you sure you want to change the base?

feat(eslint-plugin): [no-unused-vars] add a default-off option to autofix remove unused imports#11243

Conversation

nayounsang commentedMay 23, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

PR Checklist

Overview

Auto fix & suggest unused import statements

Uh oh!

typescript-eslintbot commentedMay 23, 2025

Uh oh!

netlifybot commentedMay 23, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

✅ Deploy Preview fortypescript-eslint ready!

Uh oh!

nx-cloudbot commentedMay 23, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

nayounsang commentedMay 23, 2025

Uh oh!

codecovbot commentedMay 24, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

kirkwaiblinger left a comment

Choose a reason for hiding this comment

Uh oh!

kirkwaiblingerJun 6, 2025

Choose a reason for hiding this comment

Uh oh!

nayounsangJun 6, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kirkwaiblingerJun 6, 2025

Choose a reason for hiding this comment

Uh oh!

nayounsangJun 6, 2025

Choose a reason for hiding this comment

Uh oh!

kirkwaiblingerJun 6, 2025

Choose a reason for hiding this comment

Uh oh!

nayounsangJun 6, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nayounsangJun 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nayounsang commentedJun 11, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

nayounsang left a comment• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nayounsang commentedMay 23, 2025•
edited
Loading

netlifybot commentedMay 23, 2025•
edited
Loading

nx-cloudbot commentedMay 23, 2025•
edited
Loading

codecovbot commentedMay 24, 2025•
edited
Loading

nayounsangJun 6, 2025•
edited
Loading

nayounsangJun 6, 2025•
edited
Loading

nayounsang commentedJun 11, 2025•
edited
Loading

nayounsang left a comment•
edited
Loading