Skip to content

Commit

Permalink
feat(linter/no-unused-vars): report own type references within class,…
Browse files Browse the repository at this point in the history
… interface, and type alias declarations (#6557)

## What This PR Does

**Tl;Dr**: Using a class/interface/type alias as a type within its own declaration is no longer considered a usage.

### Example
```ts
// LinkedList is only ever referenced within its own definition.
// It could be safely deleted and not affect other code.
class LinkedList<T> {
    #value: T;
    #next: LinkedList<T> | null = null;

    constructor(value: T) {
        this.#value = value;
    }

    [Symbol.iterator]*() {
        let current: LinkedList<T> | null = this;
        while (current !== null) {
            yield current.#value;
            current = current.#next;
        }
    }
}
```
  • Loading branch information
DonIsaac committed Oct 15, 2024
1 parent aa6ba24 commit c5e66e1
Show file tree
Hide file tree
Showing 5 changed files with 257 additions and 11 deletions.
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]
1function 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]
1class Foo {
· ─┬─
· ╰── 'Foo' is declared here
2static createFoo() {
╰────
help: Consider removing this declaration.

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

eslint(no-unused-vars): Class 'Point' is declared but never used.
╭─[no_unused_vars.tsx:1:7]
1class Point {
· ──┬──
· ╰── 'Point' is declared here
2public 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]
1export 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]
1type 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]
1type 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]
1type 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]
1type 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]
1type 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]
1type 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]
1class Foo {
· ─┬─
· ╰── 'Foo' is declared here
2private _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]
1class 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.

0 comments on commit c5e66e1

Please sign in to comment.