fix(eslint-plugin): [no-unused-vars] no `is assigned a value but only used as a type` error when it has a same name #11322

nayounsang · 2025-06-20T07:37:44Z

PR Checklist

Addresses an existing open issue: fixes Bug: [no-unused-vars] no is assigned a value but only used as a type error when it has a same name type alias declaration exported #10658
That issue was marked as accepting prs
Steps in Contributing were taken

Overview

Strict checks on exports with same type and variable name

If the name is the same, the variable is isTypeVariable && isValueVariable.
ClassName, TSEnumName, TSModuleName, ImportBinding and arguments also have same characteristics
Therefore, only the cases that correspond to the issue are properly verified.

Perform additional validating for abnormal cases

def.node.type !== AST_NODE_TYPES.TSTypeAliasDeclaration

Add test cases

tc mentioned in issue and similar but valid

typescript-eslint · 2025-06-20T07:37:48Z

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.

netlify · 2025-06-20T07:37:49Z

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	`85d49d7`
🔍 Latest deploy log	https://app.netlify.com/projects/typescript-eslint/deploys/6864f64ff5c51e0008930638
😎 Deploy Preview	https://deploy-preview-11322--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 (no change from production) Accessibility: 100 (no change from production) Best Practices: 100 (no change from production) SEO: 100 (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 your Netlify project configuration.

nx-cloud · 2025-06-20T07:39:13Z

View your CI Pipeline Execution ↗ for commit 85d49d7.

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

☁️ Nx Cloud last updated this comment at 2025-07-02 09:16:05 UTC

codecov · 2025-06-20T07:48:57Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.85%. Comparing base (5b378e2) to head (85d49d7).

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #11322   +/-   ##
=======================================
  Coverage   90.84%   90.85%           
=======================================
  Files         501      501           
  Lines       50919    50929   +10     
  Branches     8387     8390    +3     
=======================================
+ Hits        46256    46269   +13     
+ Misses       4648     4645    -3     
  Partials       15       15

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

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

Files with missing lines	Coverage Δ
...s/eslint-plugin/src/util/collectUnusedVariables.ts	`96.85% <100.00%> (+0.66%)`	⬆️

🚀 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 · 2025-06-20T07:53:10Z

packages/eslint-plugin/src/util/collectUnusedVariables.ts

+function isSafeUnusedExportCondition(variable: ScopeVariable): boolean {
+  if (variable instanceof ESLintScopeVariable) {
+    return true;
+  }


need advice

I don't know why this condition is necessary. Can't the arg type of isExported, isMergableExported be used as Variable?
Actually, it is used as Variable only.

it is used as Variable only

👍 any reason not to change its type from ScopeVariable to Variable? I tried the three : ScopeVariable parameters to : Variable locally and everything looked happy.

I committed.

packages/eslint-plugin/src/util/collectUnusedVariables.ts

nayounsang · 2025-06-27T10:43:45Z

Its a different issue:#8315, but we need to figure out if this is a workable solution.

nayounsang · 2025-06-27T15:03:03Z

Its a different issue:#8315, but we need to figure out if this is a workable solution.

Ah, it is different. should change validate logic.
I think I need to work on another PR. If this PR is headed in the right direction.

JoshuaKGoldberg · 2025-06-30T11:55:21Z

I think I need to work on another PR. If this PR is headed in the right direction.

@nayounsang apologies, I'm not following - are you suggesting we should or shouldn't review this PR?

nayounsang · 2025-06-30T12:09:18Z

@JoshuaKGoldberg
Oh, please review this PR. I'm ready.

The comments are my own monologue. I tend to take notes of everything and I just saw another issue that seemed related to this PR. After looking into it, it seems that even if this PR is resolved, there will be additional work needed to resolve the other issues.

JoshuaKGoldberg

🙌 Looks like a good start! I didn't review deeply because the .every line looks like it's either unnecessary or not fully tested. Which makes me suspect things might change up a bit. Could you please take a look?

JoshuaKGoldberg · 2025-06-23T16:31:11Z

packages/eslint-plugin/src/util/collectUnusedVariables.ts

@@ -441,7 +467,9 @@ function isMergableExported(variable: ScopeVariable): boolean {
        def.node.parent?.type === AST_NODE_TYPES.ExportNamedDeclaration) ||
      def.node.parent?.type === AST_NODE_TYPES.ExportDefaultDeclaration
    ) {
-      return true;
+      return (
+        safeFlag || def.node.type !== AST_NODE_TYPES.TSTypeAliasDeclaration


[Performance] There's no need to run through isSafeUnusedExportCondition(...) until it's used:

Suggested change

safeFlag || def.node.type !== AST_NODE_TYPES.TSTypeAliasDeclaration

def.node.type !== AST_NODE_TYPES.TSTypeAliasDeclaration || isSafeUnusedExportCondition(variable)

Yes, I learned more and your opinion is reasonable.
The number of defs is small, so the overhead of calling them unconditionally is greater than the complexity due to nested loops under certain conditions.
I committed.

packages/eslint-plugin/src/util/collectUnusedVariables.ts

JoshuaKGoldberg · 2025-06-23T16:49:39Z

packages/eslint-plugin/src/util/collectUnusedVariables.ts

+function isSafeUnusedExportCondition(variable: ScopeVariable): boolean {
+  if (variable instanceof ESLintScopeVariable) {
+    return true;
+  }


it is used as Variable only

👍 any reason not to change its type from ScopeVariable to Variable? I tried the three : ScopeVariable parameters to : Variable locally and everything looked happy.

JoshuaKGoldberg · 2025-06-30T14:47:13Z

packages/eslint-plugin/src/util/collectUnusedVariables.ts

+  if (variable.isTypeVariable && variable.isValueVariable) {
+    return !variable.defs
+      .map(d => d.type)
+      .every(t => exportExceptDefTypes.includes(t));


[Testing] If I comment out this .every, then all tests still pass. In fact, if the inside of the if just does return false; then tests still pass. So either this is unnecessary code or there's a gap in test coverage.

The reason for this is because of #11322 (comment).
If type of node.parent start with Export and node is a TSTypeAliasDeclaration, then these syntaxes exist:

export type Foo = /* What */ declare namespace(module) Foo { export type A = / * What */ }

Now, let's see what's possible with /* What */.
This issue is about variable that isTypeVariable && isValueVariable
So, tests are passed even if commented code.
But it's inconvenient. I think there's something I haven't anticipated yet. I need to dig deeper & I can refactor the code to make it clean.
I will test these cases in play ground

More detail memo...

TSTypeAliasDeclaration & export

declare has been verified before, so we only test one (which also succeeds locally).

export type Foo = /* What */

isTypeVariable && isValueVariable

Merged declaration, ClassName, TSEnumName,TSEnumMemberDefinition, TSModuleName, ImportBinding and arguments satisfy this condition.
Most of these are impossible syntax with export type A = /* ... */ .

nayounsang added 2 commits June 20, 2025 16:28

test: add test case

f7d55fd

fix: strict checks on exports with same type and variable name

bc379fc

nayounsang commented Jun 20, 2025

View reviewed changes

Merge branch 'main' into type-unused

bfffde3

nayounsang commented Jun 20, 2025

View reviewed changes

packages/eslint-plugin/src/util/collectUnusedVariables.ts Outdated Show resolved Hide resolved

refactor: resolve self-code-review

2f474b3

nayounsang mentioned this pull request Jun 27, 2025

Bug: [consistent-type-imports] False negatives when type name also refers to a local value #8315

Open

4 tasks

JoshuaKGoldberg requested changes Jun 30, 2025

View reviewed changes

JoshuaKGoldberg added the awaiting response Issues waiting for a reply from the OP or another party label Jun 30, 2025

nayounsang and others added 7 commits July 1, 2025 16:57

Merge branch 'main' into type-unused

2bfdd0b

refactor: change ScopeVariable to Variable

52eede8

refactor: improve perf when call isSafeUnusedExportCondition

df466a4

test: add test case

0d5801d

refactor: validate logic

ed639ea

Merge branch 'main' into type-unused

1d36292

fix: lint test file

85d49d7

	safeFlag \|\| def.node.type !== AST_NODE_TYPES.TSTypeAliasDeclaration
	def.node.type !== AST_NODE_TYPES.TSTypeAliasDeclaration \|\| isSafeUnusedExportCondition(variable)

Uh oh!

fix(eslint-plugin): [no-unused-vars] no is assigned a value but only used as a type error when it has a same name #11322

Are you sure you want to change the base?

fix(eslint-plugin): [no-unused-vars] no is assigned a value but only used as a type error when it has a same name #11322

Conversation

nayounsang commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

Overview

Strict checks on exports with same type and variable name

Perform additional validating for abnormal cases

Add test cases

Uh oh!

typescript-eslint bot commented Jun 20, 2025

Uh oh!

netlify bot commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for typescript-eslint ready!

Uh oh!

nx-cloud bot commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nayounsang Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

need advice

Uh oh!

JoshuaKGoldberg Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

nayounsang Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nayounsang commented Jun 27, 2025

Uh oh!

nayounsang commented Jun 27, 2025

Uh oh!

JoshuaKGoldberg commented Jun 30, 2025

Uh oh!

nayounsang commented Jun 30, 2025

Uh oh!

JoshuaKGoldberg left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JoshuaKGoldberg Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

nayounsang Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JoshuaKGoldberg Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

JoshuaKGoldberg Jun 30, 2025

Choose a reason for hiding this comment

Uh oh!

nayounsang Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nayounsang Jul 2, 2025 • edited Loading 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!

fix(eslint-plugin): [no-unused-vars] no `is assigned a value but only used as a type` error when it has a same name #11322

fix(eslint-plugin): [no-unused-vars] no `is assigned a value but only used as a type` error when it has a same name #11322

nayounsang commented Jun 20, 2025 •

edited

Loading

netlify bot commented Jun 20, 2025 •

edited

Loading

nx-cloud bot commented Jun 20, 2025 •

edited

Loading

codecov bot commented Jun 20, 2025 •

edited

Loading

nayounsang Jun 20, 2025 •

edited

Loading

JoshuaKGoldberg left a comment •

edited

Loading

nayounsang Jul 2, 2025 •

edited

Loading

nayounsang Jul 2, 2025 •

edited

Loading