fix(linter/no-unused-vars): report more type references within own declarations

Author: DonIsaac

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

@@ -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![
@@ -618,6 +626,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![
@@ -993,6 +1055,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)
@@ -1051,13 +1114,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();
 }
 

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

@@ -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();
 
@@ -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(_)
@@ -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,
             }
         }

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

@@ -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.

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

@@ -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.

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

@@ -16,3 +16,61 @@ source: crates/oxc_linter/src/tester.rs
    ·      ╰── 'A' is declared here
    ╰────
   help: Consider removing this declaration.
+
+  ⚠ eslint(no-unused-vars): Type alias 'T' is declared but never used.
+   ╭─[no_unused_vars.tsx: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.tsx: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.tsx: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.tsx: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.tsx: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.tsx:1:7]
+ 1 │ class Foo {
+   ·       ─┬─
+   ·        ╰── 'Foo' is declared here
+ 2 │             _inner: any;
+   ╰────
+  help: Consider removing this declaration.
+
+  ⚠ eslint(no-unused-vars): Interface 'LinkedList' is declared but never used.
+   ╭─[no_unused_vars.tsx:1:11]
+ 1 │ interface LinkedList<T> { next: LinkedList<T> | undefined }
+   ·           ─────┬────
+   ·                ╰── 'LinkedList' is declared here
+   ╰────
+  help: Consider removing this declaration.