Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(linter/no-unused-vars): report own type references within class, interface, and type alias declarations #6557

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 102 additions & 0 deletions crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@ use serde_json::json;
use super::NoUnusedVars;
use crate::{tester::Tester, FixKind, RuleMeta as _};

// uncomment to only run a single test. useful for step-through debugging.
// #[test]
// fn test_debug() {
// let pass: Vec<&str> = vec![];
// let fail = vec![];
// Tester::new(NoUnusedVars::NAME, pass, fail).intentionally_allow_no_fix_tests().test();
// }

#[test]
fn test_vars_simple() {
let pass = vec![
Expand Down Expand Up @@ -621,6 +629,60 @@ fn test_functions() {
.test_and_snapshot();
}

#[test]
fn test_self_call() {
let pass = vec![
"const _thunk = (function createThunk(count) {
if (count === 0) return () => count
return () => createThunk(count - 1)()
})()",
];

let fail = vec![
// Functions that call themselves are considered unused, even if that
// call happens within an inner function.
"function foo() { return function bar() { return foo() } }",
// Classes that construct themselves are considered unused
"class Foo {
static createFoo() {
return new Foo();
}
}",
"class Foo {
static createFoo(): Foo {
return new Foo();
}
}",
"class Point {
public x: number;
public y: number;
public add(other): Point {
const p = new Point();
p.x = this.x + (other as Point).x;
p.y = this.y + (other as Point).y;
return p;
}
}
",
// FIXME
// "class Foo {
// inner: any
// public foo(): Foo {
// if(this.inner?.constructor.name === Foo.name) {
// return this.inner;
// } else {
// return new Foo();
// }
// }
// }",
];

Tester::new(NoUnusedVars::NAME, pass, fail)
.intentionally_allow_no_fix_tests()
.with_snapshot_suffix("oxc-self-call")
.test_and_snapshot();
}

#[test]
fn test_imports() {
let pass = vec![
Expand Down Expand Up @@ -996,6 +1058,7 @@ fn test_type_aliases() {
"type Foo = Foo",
"type Foo = Array<Foo>",
"type Unbox<B> = B extends Box<infer R> ? Unbox<R> : B",
"export type F<T> = T extends infer R ? /* R not used */ string : never",
];

Tester::new(NoUnusedVars::NAME, pass, fail)
Expand Down Expand Up @@ -1054,13 +1117,52 @@ fn test_type_references() {
];

let fail = vec![
// Type aliases
"type T = number; function foo<T>(a: T): T { return a as T }; foo(1)",
"type A = number; type B<A> = A; console.log(3 as B<3>)",
"type T = { foo: T }",
"type T = { foo?: T | undefined }",
"type A<T> = { foo: T extends Array<infer R> ? A<R> : T }",
"type T = { foo(): T }",
// Type references on class symbols within that classes' definition is
// not considered used
"class Foo {
private _inner: Foo | undefined;
}",
"class Foo {
_inner: any;
constructor(other: Foo);
constructor(somethingElse: any) {
this._inner = somethingElse;
}
}",
"class LinkedList<T> {
#next?: LinkedList<T>;
public append(other: LinkedList<T>) {
this.#next = other;
}
}",
"class LinkedList<T> {
#next?: LinkedList<T>;
public nextUnchecked(): LinkedList<T> {
return <LinkedList<T>>this.#next!;
}
}",
// FIXME: ambient classes declared using `declare` are not bound by
// semantic's binder.
// https://github.com/oxc-project/oxc/blob/a9260cf6d1b83917c7a61b25cabd2d40858b0fff/crates/oxc_semantic/src/binder.rs#L105
// "declare class LinkedList<T> {
// next(): LinkedList<T> | undefined;
// }"

// Same is true for interfaces
"interface LinkedList<T> { next: LinkedList<T> | undefined }",
];

Tester::new(NoUnusedVars::NAME, pass, fail)
.intentionally_allow_no_fix_tests()
.with_snapshot_suffix("oxc-type-references")
.change_rule_path_extension("ts")
.test_and_snapshot();
}

Expand Down
41 changes: 32 additions & 9 deletions crates/oxc_linter/src/rules/eslint/no_unused_vars/usage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,16 +60,18 @@ impl<'s, 'a> Symbol<'s, 'a> {
}

#[inline]
const fn is_type_alias(&self) -> bool {
self.flags().contains(SymbolFlags::TypeAlias)
const fn could_have_type_reference_within_own_decl(&self) -> bool {
const TYPE_DECLS: SymbolFlags =
SymbolFlags::TypeAlias.union(SymbolFlags::Interface).union(SymbolFlags::Class);
self.flags().intersects(TYPE_DECLS)
}

/// Check if this [`Symbol`] has an [`Reference`]s that are considered a usage.
pub fn has_usages(&self, options: &NoUnusedVars) -> bool {
// Use symbol flags to skip the usage checks we are certain don't need
// to be run.
let do_reassignment_checks = self.is_possibly_reassignable();
let do_type_self_usage_checks = self.is_type_alias();
let do_type_self_usage_checks = self.could_have_type_reference_within_own_decl();
let do_self_call_check = self.is_maybe_callable();
let do_discarded_read_checks = self.is_definitely_reassignable_variable();

Expand Down Expand Up @@ -251,12 +253,12 @@ impl<'s, 'a> Symbol<'s, 'a> {
}
// definitely not within a type alias, we can be sure this isn't
// a self-usage. Safe CPU cycles by breaking early.
AstKind::CallExpression(_)
| AstKind::BinaryExpression(_)
| AstKind::Function(_)
| AstKind::Class(_)
| AstKind::TSInterfaceDeclaration(_)
| AstKind::TSModuleDeclaration(_)
// NOTE: we cannot short-circuit on functions since they could
// be methods with annotations referencing the type they're in.
// e.g.:
// - `type Foo = { bar(): Foo }`
// - `class Foo { static factory(): Foo { return new Foo() } }`
AstKind::TSModuleDeclaration(_)
| AstKind::VariableDeclaration(_)
| AstKind::VariableDeclarator(_)
| AstKind::ExportNamedDeclaration(_)
Expand All @@ -266,6 +268,27 @@ impl<'s, 'a> Symbol<'s, 'a> {
return false;
}

AstKind::CallExpression(_) | AstKind::BinaryExpression(_) => {
// interfaces/type aliases cannot have value expressions
// within their declarations, so we know we're not in one.
// However, classes can.
if self.flags().is_class() {
continue;
}
return false;
}

// `interface LinkedList<T> { next?: LinkedList<T> }`
AstKind::TSInterfaceDeclaration(iface) => {
return self.flags().is_interface() && self == &iface.id;
}

// `class Foo { bar(): Foo }`
AstKind::Class(class) => {
return self.flags().is_class()
&& class.id.as_ref().is_some_and(|id| self == id);
}

_ => continue,
}
}
Expand Down
37 changes: 37 additions & 0 deletions crates/oxc_linter/src/snapshots/no_unused_vars@oxc-self-call.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
---
source: crates/oxc_linter/src/tester.rs
---
⚠ eslint(no-unused-vars): Function 'foo' is declared but never used.
╭─[no_unused_vars.tsx:1:10]
1 │ function foo() { return function bar() { return foo() } }
· ─┬─
· ╰── 'foo' is declared here
╰────
help: Consider removing this declaration.

⚠ eslint(no-unused-vars): Class 'Foo' is declared but never used.
╭─[no_unused_vars.tsx:1:7]
1 │ class Foo {
· ─┬─
· ╰── 'Foo' is declared here
2 │ static createFoo() {
╰────
help: Consider removing this declaration.

⚠ eslint(no-unused-vars): Class 'Foo' is declared but never used.
╭─[no_unused_vars.tsx:1:7]
1 │ class Foo {
· ─┬─
· ╰── 'Foo' is declared here
2 │ static createFoo(): Foo {
╰────
help: Consider removing this declaration.

⚠ eslint(no-unused-vars): Class 'Point' is declared but never used.
╭─[no_unused_vars.tsx:1:7]
1 │ class Point {
· ──┬──
· ╰── 'Point' is declared here
2 │ public x: number;
╰────
help: Consider removing this declaration.
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,11 @@ source: crates/oxc_linter/src/tester.rs
· ╰── 'Unbox' is declared here
╰────
help: Consider removing this declaration.

⚠ eslint(no-unused-vars): Variable 'R' is declared but never used.
╭─[no_unused_vars.tsx:1:36]
1 │ export type F<T> = T extends infer R ? /* R not used */ string : never
· ┬
· ╰── 'R' is declared here
╰────
help: Consider removing this declaration.
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,93 @@
source: crates/oxc_linter/src/tester.rs
---
⚠ eslint(no-unused-vars): Type alias 'T' is declared but never used.
╭─[no_unused_vars.tsx:1:6]
╭─[no_unused_vars.ts:1:6]
1 │ type T = number; function foo<T>(a: T): T { return a as T }; foo(1)
· ┬
· ╰── 'T' is declared here
╰────
help: Consider removing this declaration.

⚠ eslint(no-unused-vars): Type alias 'A' is declared but never used.
╭─[no_unused_vars.tsx:1:6]
╭─[no_unused_vars.ts:1:6]
1 │ type A = number; type B<A> = A; console.log(3 as B<3>)
· ┬
· ╰── 'A' is declared here
╰────
help: Consider removing this declaration.

⚠ eslint(no-unused-vars): Type alias 'T' is declared but never used.
╭─[no_unused_vars.ts:1:6]
1 │ type T = { foo: T }
· ┬
· ╰── 'T' is declared here
╰────
help: Consider removing this declaration.

⚠ eslint(no-unused-vars): Type alias 'T' is declared but never used.
╭─[no_unused_vars.ts:1:6]
1 │ type T = { foo?: T | undefined }
· ┬
· ╰── 'T' is declared here
╰────
help: Consider removing this declaration.

⚠ eslint(no-unused-vars): Type alias 'A' is declared but never used.
╭─[no_unused_vars.ts:1:6]
1 │ type A<T> = { foo: T extends Array<infer R> ? A<R> : T }
· ┬
· ╰── 'A' is declared here
╰────
help: Consider removing this declaration.

⚠ eslint(no-unused-vars): Type alias 'T' is declared but never used.
╭─[no_unused_vars.ts:1:6]
1 │ type T = { foo(): T }
· ┬
· ╰── 'T' is declared here
╰────
help: Consider removing this declaration.

⚠ eslint(no-unused-vars): Class 'Foo' is declared but never used.
╭─[no_unused_vars.ts:1:7]
1 │ class Foo {
· ─┬─
· ╰── 'Foo' is declared here
2 │ private _inner: Foo | undefined;
╰────
help: Consider removing this declaration.

⚠ eslint(no-unused-vars): Class 'Foo' is declared but never used.
╭─[no_unused_vars.ts:1:7]
1 │ class Foo {
· ─┬─
· ╰── 'Foo' is declared here
2 │ _inner: any;
╰────
help: Consider removing this declaration.

⚠ eslint(no-unused-vars): Class 'LinkedList' is declared but never used.
╭─[no_unused_vars.ts:1:7]
1 │ class LinkedList<T> {
· ─────┬────
· ╰── 'LinkedList' is declared here
2 │ #next?: LinkedList<T>;
╰────
help: Consider removing this declaration.

⚠ eslint(no-unused-vars): Class 'LinkedList' is declared but never used.
╭─[no_unused_vars.ts:1:7]
1 │ class LinkedList<T> {
· ─────┬────
· ╰── 'LinkedList' is declared here
2 │ #next?: LinkedList<T>;
╰────
help: Consider removing this declaration.

⚠ eslint(no-unused-vars): Interface 'LinkedList' is declared but never used.
╭─[no_unused_vars.ts:1:11]
1 │ interface LinkedList<T> { next: LinkedList<T> | undefined }
· ─────┬────
· ╰── 'LinkedList' is declared here
╰────
help: Consider removing this declaration.
Loading