From c5e66e108ffa1cb22e960bce6ff014a567dc63c2 Mon Sep 17 00:00:00 2001 From: DonIsaac <22823424+DonIsaac@users.noreply.github.com> Date: Tue, 15 Oct 2024 14:34:05 +0000 Subject: [PATCH] feat(linter/no-unused-vars): report own type references within class, 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 { #value: T; #next: LinkedList | null = null; constructor(value: T) { this.#value = value; } [Symbol.iterator]*() { let current: LinkedList | null = this; while (current !== null) { yield current.#value; current = current.#next; } } } ``` --- .../rules/eslint/no_unused_vars/tests/oxc.rs | 102 ++++++++++++++++++ .../src/rules/eslint/no_unused_vars/usage.rs | 41 +++++-- .../no_unused_vars@oxc-self-call.snap | 37 +++++++ .../no_unused_vars@oxc-type-aliases.snap | 8 ++ .../no_unused_vars@oxc-type-references.snap | 80 +++++++++++++- 5 files changed, 257 insertions(+), 11 deletions(-) create mode 100644 crates/oxc_linter/src/snapshots/no_unused_vars@oxc-self-call.snap diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs index 6933c9e7a6768..1c460c48fe9ad 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs +++ b/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![ @@ -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![ @@ -996,6 +1058,7 @@ fn test_type_aliases() { "type Foo = Foo", "type Foo = Array", "type Unbox = B extends Box ? Unbox : B", + "export type F = T extends infer R ? /* R not used */ string : never", ]; Tester::new(NoUnusedVars::NAME, pass, fail) @@ -1054,13 +1117,52 @@ fn test_type_references() { ]; let fail = vec![ + // Type aliases "type T = number; function foo(a: T): T { return a as T }; foo(1)", "type A = number; type B = A; console.log(3 as B<3>)", + "type T = { foo: T }", + "type T = { foo?: T | undefined }", + "type A = { foo: T extends Array ? A : 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 { + #next?: LinkedList; + public append(other: LinkedList) { + this.#next = other; + } + }", + "class LinkedList { + #next?: LinkedList; + public nextUnchecked(): LinkedList { + return >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 { + // next(): LinkedList | undefined; + // }" + + // Same is true for interfaces + "interface LinkedList { next: LinkedList | 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(); } diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/usage.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/usage.rs index a9fbda3671d8a..52aa4e656f7e9 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unused_vars/usage.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unused_vars/usage.rs @@ -60,8 +60,10 @@ 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. @@ -69,7 +71,7 @@ impl<'s, 'a> Symbol<'s, 'a> { // 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 { next?: LinkedList }` + 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, } } diff --git a/crates/oxc_linter/src/snapshots/no_unused_vars@oxc-self-call.snap b/crates/oxc_linter/src/snapshots/no_unused_vars@oxc-self-call.snap new file mode 100644 index 0000000000000..7dec1e1528445 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_unused_vars@oxc-self-call.snap @@ -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. diff --git a/crates/oxc_linter/src/snapshots/no_unused_vars@oxc-type-aliases.snap b/crates/oxc_linter/src/snapshots/no_unused_vars@oxc-type-aliases.snap index 02114b6cb8786..cdd711f8e76c9 100644 --- a/crates/oxc_linter/src/snapshots/no_unused_vars@oxc-type-aliases.snap +++ b/crates/oxc_linter/src/snapshots/no_unused_vars@oxc-type-aliases.snap @@ -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 extends infer R ? /* R not used */ string : never + · ┬ + · ╰── 'R' is declared here + ╰──── + help: Consider removing this declaration. diff --git a/crates/oxc_linter/src/snapshots/no_unused_vars@oxc-type-references.snap b/crates/oxc_linter/src/snapshots/no_unused_vars@oxc-type-references.snap index d1f6d85a110c8..bce0785b88204 100644 --- a/crates/oxc_linter/src/snapshots/no_unused_vars@oxc-type-references.snap +++ b/crates/oxc_linter/src/snapshots/no_unused_vars@oxc-type-references.snap @@ -2,7 +2,7 @@ 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(a: T): T { return a as T }; foo(1) · ┬ · ╰── 'T' is declared here @@ -10,9 +10,85 @@ source: crates/oxc_linter/src/tester.rs 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; 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 = { foo: T extends Array ? A : 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 { + · ─────┬──── + · ╰── 'LinkedList' is declared here + 2 │ #next?: LinkedList; + ╰──── + 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 { + · ─────┬──── + · ╰── 'LinkedList' is declared here + 2 │ #next?: LinkedList; + ╰──── + 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 { next: LinkedList | undefined } + · ─────┬──── + · ╰── 'LinkedList' is declared here + ╰──── + help: Consider removing this declaration.