Skip to content

Commit 387af3a

Browse files
camc314camc314
committed
feat(linter): report vars only used as types (#10664)
closes #8488 I ported accross the additional tests added in https://github.com/typescript-eslint/typescript-eslint/pull/9330/files, then fixed the resulting test cases
1 parent 374e19e commit 387af3a

File tree

7 files changed

+230
-17
lines changed

7 files changed

+230
-17
lines changed

‎crates/oxc_linter/src/rules/eslint/no_unused_vars/diagnostic.rs‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,13 @@ where
5757
pronoun_singular.cow_to_ascii_lowercase()
5858
))
5959
}
60+
6061
/// Variable 'x' is declared but never used.
61-
pub fn declared<R>(symbol: &Symbol<'_, '_>, pat: &IgnorePattern<R>) -> OxcDiagnostic
62+
pub fn declared<R>(
63+
symbol: &Symbol<'_, '_>,
64+
pat: &IgnorePattern<R>,
65+
only_used_as_type: bool,
66+
) -> OxcDiagnostic
6267
where
6368
R: fmt::Display,
6469
{
@@ -71,9 +76,13 @@ where
7176
let (pronoun, pronoun_plural) = pronoun_for_symbol(symbol.flags());
7277
let suffix = pat.diagnostic_help(pronoun_plural);
7378

74-
OxcDiagnostic::warn(format!("{pronoun} '{name}' is {verb} but never used.{suffix}"))
75-
.with_label(symbol.span().label(format!("'{name}' is declared here")))
76-
.with_help(help)
79+
OxcDiagnostic::warn(if only_used_as_type {
80+
format!("{pronoun} is {verb} but only used as a type{suffix}.",)
81+
} else {
82+
format!("{pronoun} '{name}' is {verb} but never used.{suffix}")
83+
})
84+
.with_label(symbol.span().label(format!("'{name}' is declared here")))
85+
.with_help(help)
7786
}
7887

7988
/// Variable 'x' is assigned a value but never used.

‎crates/oxc_linter/src/rules/eslint/no_unused_vars/mod.rs‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,11 @@ impl NoUnusedVars {
285285
let span = ctx.nodes().get_node(last_write.node_id()).kind().span();
286286
diagnostic::assign(symbol, span, &self.vars_ignore_pattern)
287287
}
288-
_ => diagnostic::declared(symbol, &self.vars_ignore_pattern),
288+
_ => diagnostic::declared(
289+
symbol,
290+
&self.vars_ignore_pattern,
291+
symbol.has_reference_used_as_type_query(),
292+
),
289293
};
290294

291295
ctx.diagnostic_with_suggestion(report, |fixer| {
@@ -309,30 +313,34 @@ impl NoUnusedVars {
309313
if NoUnusedVars::is_allowed_binding_rest_element(symbol) {
310314
return;
311315
}
312-
ctx.diagnostic(diagnostic::declared(symbol, &self.vars_ignore_pattern));
316+
ctx.diagnostic(diagnostic::declared(symbol, &self.vars_ignore_pattern, false));
313317
}
314318
AstKind::TSModuleDeclaration(namespace) => {
315319
if self.is_allowed_ts_namespace(symbol, namespace) {
316320
return;
317321
}
318-
ctx.diagnostic(diagnostic::declared(symbol, &IgnorePattern::<&str>::None));
322+
ctx.diagnostic(diagnostic::declared(symbol, &IgnorePattern::<&str>::None, false));
319323
}
320324
AstKind::TSInterfaceDeclaration(_) => {
321325
if symbol.is_in_declared_module() {
322326
return;
323327
}
324-
ctx.diagnostic(diagnostic::declared(symbol, &IgnorePattern::<&str>::None));
328+
ctx.diagnostic(diagnostic::declared(symbol, &IgnorePattern::<&str>::None, false));
325329
}
326330
AstKind::TSTypeParameter(_) => {
327331
if self.is_allowed_type_parameter(symbol, declaration.id()) {
328332
return;
329333
}
330-
ctx.diagnostic(diagnostic::declared(symbol, &self.vars_ignore_pattern));
334+
ctx.diagnostic(diagnostic::declared(symbol, &self.vars_ignore_pattern, false));
331335
}
332336
AstKind::CatchParameter(_) => {
333-
ctx.diagnostic(diagnostic::declared(symbol, &self.caught_errors_ignore_pattern));
337+
ctx.diagnostic(diagnostic::declared(
338+
symbol,
339+
&self.caught_errors_ignore_pattern,
340+
false,
341+
));
334342
}
335-
_ => ctx.diagnostic(diagnostic::declared(symbol, &IgnorePattern::<&str>::None)),
343+
_ => ctx.diagnostic(diagnostic::declared(symbol, &IgnorePattern::<&str>::None, false)),
336344
}
337345
}
338346

@@ -342,8 +350,7 @@ impl NoUnusedVars {
342350
let flags = symbol.flags();
343351

344352
// 1. ignore enum members. Only enums get checked
345-
// 2. ignore all ambient TS declarations, e.g. `declare class Foo {}`
346-
if flags.intersects(SymbolFlags::EnumMember.union(SymbolFlags::Ambient))
353+
if flags.intersects(SymbolFlags::EnumMember)
347354
// ambient namespaces
348355
|| flags == AMBIENT_NAMESPACE_FLAGS
349356
|| (symbol.is_in_ts() && symbol.is_in_declare_global())

‎crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1119,9 +1119,6 @@ fn test_type_references() {
11191119
export type C = B<A<number>>;
11201120
",
11211121
"const x: number = 1; function foo(): typeof x { return x }; foo()",
1122-
// not handled by typescript-eslint. Maybe we'll add this one day
1123-
"function foo(): typeof foo { }",
1124-
"function foo(): typeof foo { return foo }",
11251122
// ---
11261123
"type T = number; console.log(3 as T);",
11271124
"type T = number; console.log(((3) as T));",
@@ -1194,6 +1191,8 @@ fn test_type_references() {
11941191

11951192
// Same is true for interfaces
11961193
"interface LinkedList<T> { next: LinkedList<T> | undefined }",
1194+
"function foo(): typeof foo { }",
1195+
"function foo(): typeof foo { return foo }",
11971196
];
11981197

11991198
Tester::new(NoUnusedVars::NAME, NoUnusedVars::PLUGIN, pass, fail)

‎crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/typescript_eslint.rs‎

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1013,7 +1013,7 @@ fn test() {
10131013
// https://github.com/typescript-eslint/typescript-eslint/issues/5152
10141014
(
10151015
"
1016-
function foo<T>(value: T): T {
1016+
export function foo<T>(value: T): T {
10171017
return { value };
10181018
}
10191019
export type Foo<T> = typeof foo<T>;
@@ -1380,6 +1380,28 @@ fn test() {
13801380
",
13811381
None,
13821382
),
1383+
(
1384+
"
1385+
type _Foo = 1;
1386+
export const x: _Foo = 1;
1387+
",
1388+
Some(json!([{ "varsIgnorePattern": "^_", "reportUnusedIgnorePattern": false }])),
1389+
),
1390+
(
1391+
"
1392+
export const foo: number = 1;
1393+
export type Foo = typeof foo;
1394+
",
1395+
None,
1396+
),
1397+
(
1398+
"
1399+
import { foo } from 'foo';
1400+
export type Foo = typeof foo;
1401+
export const bar = (): Foo => foo;
1402+
",
1403+
None,
1404+
),
13831405
];
13841406

13851407
let fail = vec![
@@ -1697,6 +1719,63 @@ fn test() {
16971719
None,
16981720
),
16991721
("const foo: number = 1;", None),
1722+
(
1723+
"
1724+
const foo: number = 1;
1725+
export type Foo = typeof foo;
1726+
",
1727+
None,
1728+
),
1729+
(
1730+
"
1731+
declare const foo: number;
1732+
export type Foo = typeof foo;
1733+
",
1734+
None,
1735+
),
1736+
(
1737+
"
1738+
const foo: number = 1;
1739+
export type Foo = typeof foo | string;
1740+
",
1741+
None,
1742+
),
1743+
(
1744+
"
1745+
const foo: number = 1;
1746+
export type Foo = (typeof foo | string) & { __brand: 'foo' };
1747+
",
1748+
None,
1749+
),
1750+
(
1751+
"
1752+
const foo = {
1753+
bar: {
1754+
baz: 123,
1755+
},
1756+
};
1757+
export type Bar = typeof foo.bar;
1758+
",
1759+
None,
1760+
),
1761+
(
1762+
"
1763+
const foo = {
1764+
bar: {
1765+
baz: 123,
1766+
},
1767+
};
1768+
export type Bar = (typeof foo)['bar'];
1769+
",
1770+
None,
1771+
),
1772+
(
1773+
"
1774+
import { foo } from 'foo';
1775+
export type Bar = typeof foo;
1776+
",
1777+
None,
1778+
),
17001779
];
17011780

17021781
Tester::new(NoUnusedVars::NAME, NoUnusedVars::PLUGIN, pass, fail)

‎crates/oxc_linter/src/rules/eslint/no_unused_vars/usage.rs‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,13 @@ impl<'a> Symbol<'_, 'a> {
136136
if do_type_self_usage_checks && self.is_type_self_usage(reference) {
137137
continue;
138138
}
139+
140+
if !self.flags().intersects(SymbolFlags::TypeImport)
141+
&& self.reference_contains_type_query(reference)
142+
{
143+
continue;
144+
}
145+
139146
return true;
140147
}
141148

@@ -779,4 +786,30 @@ impl<'a> Symbol<'_, 'a> {
779786

780787
None
781788
}
789+
790+
pub fn has_reference_used_as_type_query(&self) -> bool {
791+
self.references().any(|reference| self.reference_contains_type_query(reference))
792+
}
793+
794+
fn reference_contains_type_query(&self, reference: &Reference) -> bool {
795+
let Some(mut node) = self.get_ref_relevant_node(reference) else {
796+
debug_assert!(false);
797+
return false;
798+
};
799+
800+
loop {
801+
node = match node.kind() {
802+
AstKind::TSTypeQuery(_) => return true,
803+
AstKind::TSQualifiedName(_) | AstKind::TSTypeName(_) => {
804+
if let Some(parent) = self.nodes().parent_node(node.id()) {
805+
parent
806+
} else {
807+
debug_assert!(false);
808+
return false;
809+
}
810+
}
811+
_ => return false,
812+
};
813+
}
814+
}
782815
}

‎crates/oxc_linter/src/snapshots/[email protected]‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,3 +92,19 @@ source: crates/oxc_linter/src/tester.rs
9292
· ╰── 'LinkedList' is declared here
9393
╰────
9494
help: Consider removing this declaration.
95+
96+
⚠ eslint(no-unused-vars): Function 'foo' is declared but never used.
97+
╭─[no_unused_vars.ts:1:10]
98+
1 │ function foo(): typeof foo { }
99+
· ─┬─
100+
· ╰── 'foo' is declared here
101+
╰────
102+
help: Consider removing this declaration.
103+
104+
⚠ eslint(no-unused-vars): Function 'foo' is declared but never used.
105+
╭─[no_unused_vars.ts:1:10]
106+
1 │ function foo(): typeof foo { return foo }
107+
· ─┬─
108+
· ╰── 'foo' is declared here
109+
╰────
110+
help: Consider removing this declaration.

‎crates/oxc_linter/src/snapshots/[email protected]‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,3 +340,73 @@ source: crates/oxc_linter/src/tester.rs
340340
· ╰── 'foo' is declared here
341341
╰────
342342
help: Consider removing this declaration.
343+
344+
⚠ eslint(no-unused-vars): Variable is declared but only used as a type Unused variables should start with a '_'..
345+
╭─[no_unused_vars.ts:2:16]
346+
1 │
347+
2 │ const foo: number = 1;
348+
· ─┬─
349+
· ╰── 'foo' is declared here
350+
3 │ export type Foo = typeof foo;
351+
╰────
352+
help: Consider removing this declaration.
353+
354+
⚠ eslint(no-unused-vars): Variable is declared but only used as a type Unused variables should start with a '_'..
355+
╭─[no_unused_vars.ts:2:23]
356+
1 │
357+
2 │ declare const foo: number;
358+
· ─┬─
359+
· ╰── 'foo' is declared here
360+
3 │ export type Foo = typeof foo;
361+
╰────
362+
help: Consider removing this declaration.
363+
364+
⚠ eslint(no-unused-vars): Variable is declared but only used as a type Unused variables should start with a '_'..
365+
╭─[no_unused_vars.ts:2:15]
366+
1 │
367+
2 │ const foo: number = 1;
368+
· ─┬─
369+
· ╰── 'foo' is declared here
370+
3 │ export type Foo = typeof foo | string;
371+
╰────
372+
help: Consider removing this declaration.
373+
374+
⚠ eslint(no-unused-vars): Variable is declared but only used as a type Unused variables should start with a '_'..
375+
╭─[no_unused_vars.ts:2:15]
376+
1 │
377+
2 │ const foo: number = 1;
378+
· ─┬─
379+
· ╰── 'foo' is declared here
380+
3 │ export type Foo = (typeof foo | string) & { __brand: 'foo' };
381+
╰────
382+
help: Consider removing this declaration.
383+
384+
⚠ eslint(no-unused-vars): Variable is declared but only used as a type Unused variables should start with a '_'..
385+
╭─[no_unused_vars.ts:2:15]
386+
1 │
387+
2 │ const foo = {
388+
· ─┬─
389+
· ╰── 'foo' is declared here
390+
3 │ bar: {
391+
╰────
392+
help: Consider removing this declaration.
393+
394+
⚠ eslint(no-unused-vars): Variable is declared but only used as a type Unused variables should start with a '_'..
395+
╭─[no_unused_vars.ts:2:15]
396+
1 │
397+
2 │ const foo = {
398+
· ─┬─
399+
· ╰── 'foo' is declared here
400+
3 │ bar: {
401+
╰────
402+
help: Consider removing this declaration.
403+
404+
⚠ eslint(no-unused-vars): Identifier 'foo' is imported but never used.
405+
╭─[no_unused_vars.ts:2:18]
406+
1 │
407+
2 │ import { foo } from 'foo';
408+
· ─┬─
409+
· ╰── 'foo' is imported here
410+
3 │ export type Bar = typeof foo;
411+
╰────
412+
help: Consider removing this import.

0 commit comments

Comments
 (0)