feat(linter): add eslint/no-unused-vars (⭐ attempt 3.2) (#4445)

> Re-creation of #4427 due to rebasing issues. Original attempt: #642
-----

Third time's the charm?

Each time I attempt this rule, I find a bunch of bugs in `Semantic`, and I expect this attempt to be no different. Expect sidecar issues+PRs stemming from this PR here.

## Not Supported
These are cases supported in the original eslint rule, but that I'm intentionally deciding not to support
- export comments in scripts
  ```js
  /* exported a */ var a;
  ```
- global comments
  ```js
  /* global a */ var a;
   ```

## Behavior Changes
These are intentional deviations from the original rule's behavior:
- logical re-assignments are not considered usages
  ```js
  // passes in eslint/no-unused-vars, fails in this implementation
  let a = 0; a ||= 1;
  let b = 0; b &&= 2;
  let c = undefined; c ??= []
  ```

## Known Limitations
- Lint rules do not have babel or tsconfig information, meaning we can't determine if `React` imports are being used or not. The relevant tsconfig settings here are `jsx`, `jsxPragma`, and `jsxFragmentName`. To accommodate this, all imports to symbols named `React` or `h` are ignored in JSX files.
- References to symbols used in JSDoc `{@link}` tags are not created, so symbols that are only used in doc comments will be reported as unused. See: #4443
- `.vue` files are skipped completely, since variables can be used in templates in ways we cannot detect
  > note: `.d.ts` files are skipped as well.

## Todo
- [x] Skip unused TS enum members on used enums
- [x] Skip unused parameters followed by used variables in object/array spreads
- [x] Re-assignments to array/object spreads do not respect `destructuredArrayIgnorePattern` (related to: #4435)
- [x] #4493
- [x] References inside a nested scope are not considered usages (#4447)
- [x] Port over typescript-eslint test cases _(wip, they've been copied and I'm slowly enabling them)_
- [x] Handle constructor properties
  ```ts
  class Foo {
    constructor(public a) {} // `a` should be allowed
  }
  ```
- [x] Read references in sequence expressions (that are not in the last position) should not count as a usage
  ```js
  let a = 0; let b = (a++, 0); console.log(b)
  ```
  > Honestly, is anyone even writing code like this?
- [x] function overload signatures should not be reported
- [x] Named functions returned from other functions get incorrectly reported as unused (found by @camc314)
  ```js
  function foo() {
    return function bar() { }
  }
  Foo()()
  ```
- [x] false positive for TS modules within ambient modules
  ```ts
  declare global {
    // incorrectly marked as unused
    namespace jest { }
  }
  ```

## Blockers
- #4436
- #4437
- #4446
- #4447
- #4494
- #4495

## Non-Blocking Issues
- #4443
- #4475 (prevents checks on exported symbols from namespaces)

.typos.toml

@@ -7,6 +7,9 @@ extend-exclude = [
   "**/*.snap",
   "**/*/CHANGELOG.md",
   "crates/oxc_linter/fixtures",
+  "crates/oxc_linter/src/rules/eslint/no_unused_vars/ignored.rs",
+  "crates/oxc_linter/src/rules/eslint/no_unused_vars/options.rs",
+  "crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/eslint.rs",
   "crates/oxc_linter/src/rules/jsx_a11y/aria_props.rs",
   "crates/oxc_linter/src/rules/jsx_a11y/img_redundant_alt.rs",
   "crates/oxc_linter/src/rules/react/no_unknown_property.rs",

apps/oxlint/src/lint/mod.rs

@@ -427,7 +427,7 @@ mod test {
         ];
         let result = test(args);
         assert_eq!(result.number_of_files, 1);
-        assert_eq!(result.number_of_warnings, 2);
+        assert_eq!(result.number_of_warnings, 3);
         assert_eq!(result.number_of_errors, 0);
     }
 
@@ -441,7 +441,7 @@ mod test {
         ];
         let result = test(args);
         assert_eq!(result.number_of_files, 1);
-        assert_eq!(result.number_of_warnings, 1);
+        assert_eq!(result.number_of_warnings, 2);
         assert_eq!(result.number_of_errors, 0);
     }
 
@@ -477,7 +477,7 @@ mod test {
         let args = &["fixtures/svelte/debugger.svelte"];
         let result = test(args);
         assert_eq!(result.number_of_files, 1);
-        assert_eq!(result.number_of_warnings, 1);
+        assert_eq!(result.number_of_warnings, 2);
         assert_eq!(result.number_of_errors, 0);
     }
 

crates/oxc_ast/src/ast_impl/js.rs

@@ -1203,6 +1203,11 @@ impl<'a> Hash for Class<'a> {
 }
 
 impl<'a> ClassElement<'a> {
+    /// Returns `true` if this is a [`ClassElement::StaticBlock`].
+    pub fn is_static_block(&self) -> bool {
+        matches!(self, Self::StaticBlock(_))
+    }
+
     /// Returns `true` if this [`ClassElement`] is a static block or has a
     /// static modifier.
     pub fn r#static(&self) -> bool {

crates/oxc_linter/src/rules.rs

@@ -106,6 +106,7 @@ mod eslint {
     pub mod no_unsafe_optional_chaining;
     pub mod no_unused_labels;
     pub mod no_unused_private_class_members;
+    pub mod no_unused_vars;
     pub mod no_useless_catch;
     pub mod no_useless_concat;
     pub mod no_useless_constructor;
@@ -526,6 +527,7 @@ oxc_macros::declare_all_lint_rules! {
     eslint::no_unsafe_negation,
     eslint::no_unsafe_optional_chaining,
     eslint::no_unused_labels,
+    eslint::no_unused_vars,
     eslint::no_unused_private_class_members,
     eslint::no_useless_catch,
     eslint::no_useless_escape,

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

@@ -0,0 +1,290 @@
+//! This module checks if an unused variable is allowed. Note that this does not
+//! consider variables ignored by name pattern, but by where they are declared.
+#[allow(clippy::wildcard_imports)]
+use oxc_ast::{ast::*, AstKind};
+use oxc_semantic::{AstNode, AstNodeId, Semantic};
+use oxc_span::GetSpan;
+
+use crate::rules::eslint::no_unused_vars::binding_pattern::{BindingContext, HasAnyUsedBinding};
+
+use super::{options::ArgsOption, NoUnusedVars, Symbol};
+
+impl<'s, 'a> Symbol<'s, 'a> {
+    /// Returns `true` if this function is use.
+    ///
+    /// Checks for these cases
+    /// 1. passed as a callback to another [`CallExpression`] or [`NewExpression`]
+    /// 2. invoked as an IIFE
+    /// 3. Returned from another function
+    /// 4. Used as an attribute in a JSX element
+    #[inline]
+    pub fn is_function_or_class_declaration_used(&self) -> bool {
+        #[cfg(debug_assertions)]
+        {
+            let kind = self.declaration().kind();
+            assert!(kind.is_function_like() || matches!(kind, AstKind::Class(_)));
+        }
+
+        for parent in self.iter_parents() {
+            match parent.kind() {
+                AstKind::MemberExpression(_) | AstKind::ParenthesizedExpression(_) => {
+                    continue;
+                }
+                // Returned from another function. Definitely won't be the same
+                // function because we're walking up from its declaration
+                AstKind::ReturnStatement(_)
+                // <Component onClick={function onClick(e) { }} />
+                | AstKind::JSXExpressionContainer(_)
+                // Function declaration is passed as an argument to another function.
+                | AstKind::CallExpression(_) | AstKind::Argument(_)
+                // e.g. `const x = { foo: function foo() {} }`
+                | AstKind::ObjectProperty(_)
+                // e.g. var foo = function bar() { }
+                // we don't want to check for violations on `bar`, just `foo`
+                | AstKind::VariableDeclarator(_)
+                => {
+                    return true;
+                }
+                // !function() {}; is an IIFE
+                AstKind::UnaryExpression(expr) => return expr.operator.is_not(),
+                // function is used as a value for an assignment
+                // e.g. Array.prototype.sort ||= function sort(a, b) { }
+                AstKind::AssignmentExpression(assignment) if assignment.right.span().contains_inclusive(self.span()) => {
+                    return self != &assignment.left;
+                }
+                _ => {
+                    return false;
+                }
+            }
+        }
+
+        false
+    }
+
+    fn is_declared_in_for_of_loop(&self) -> bool {
+        for parent in self.iter_parents() {
+            match parent.kind() {
+                AstKind::ParenthesizedExpression(_)
+                | AstKind::VariableDeclaration(_)
+                | AstKind::BindingIdentifier(_)
+                | AstKind::SimpleAssignmentTarget(_)
+                | AstKind::AssignmentTarget(_) => continue,
+                AstKind::ForInStatement(ForInStatement { body, .. })
+                | AstKind::ForOfStatement(ForOfStatement { body, .. }) => match body {
+                    Statement::ReturnStatement(_) => return true,
+                    Statement::BlockStatement(b) => {
+                        return b
+                            .body
+                            .first()
+                            .is_some_and(|s| matches!(s, Statement::ReturnStatement(_)))
+                    }
+                    _ => return false,
+                },
+                _ => return false,
+            }
+        }
+
+        false
+    }
+
+    pub fn is_in_declared_module(&self) -> bool {
+        let scopes = self.scopes();
+        let nodes = self.nodes();
+        scopes.ancestors(self.scope_id())
+            .map(|scope_id| scopes.get_node_id(scope_id))
+            .map(|node_id| nodes.get_node(node_id))
+            .any(|node| matches!(node.kind(), AstKind::TSModuleDeclaration(namespace) if is_ambient_namespace(namespace)))
+    }
+}
+
+#[inline]
+fn is_ambient_namespace(namespace: &TSModuleDeclaration) -> bool {
+    namespace.declare || namespace.kind.is_global()
+}
+
+impl NoUnusedVars {
+    #[allow(clippy::unused_self)]
+    pub(super) fn is_allowed_class_or_function(&self, symbol: &Symbol<'_, '_>) -> bool {
+        symbol.is_function_or_class_declaration_used()
+        // || symbol.is_function_or_class_assigned_to_same_name_variable()
+    }
+
+    #[allow(clippy::unused_self)]
+    pub(super) fn is_allowed_ts_namespace<'a>(
+        &self,
+        symbol: &Symbol<'_, 'a>,
+        namespace: &TSModuleDeclaration<'a>,
+    ) -> bool {
+        if is_ambient_namespace(namespace) {
+            return true;
+        }
+        symbol.is_in_declared_module()
+    }
+
+    /// Returns `true` if this unused variable declaration should be allowed
+    /// (i.e. not reported)
+    pub(super) fn is_allowed_variable_declaration<'a>(
+        &self,
+        symbol: &Symbol<'_, 'a>,
+        decl: &VariableDeclarator<'a>,
+    ) -> bool {
+        if decl.kind.is_var() && self.vars.is_local() && symbol.is_root() {
+            return true;
+        }
+
+        // allow unused iterators, since they're required for valid syntax
+        if symbol.is_declared_in_for_of_loop() {
+            return true;
+        }
+
+        false
+    }
+
+    #[allow(clippy::unused_self)]
+    pub(super) fn is_allowed_type_parameter(
+        &self,
+        symbol: &Symbol<'_, '_>,
+        declaration_id: AstNodeId,
+    ) -> bool {
+        matches!(symbol.nodes().parent_kind(declaration_id), Some(AstKind::TSMappedType(_)))
+    }
+
+    /// Returns `true` if this unused parameter should be allowed (i.e. not
+    /// reported)
+    pub(super) fn is_allowed_argument<'a>(
+        &self,
+        semantic: &Semantic<'a>,
+        symbol: &Symbol<'_, 'a>,
+        param: &FormalParameter<'a>,
+    ) -> bool {
+        // early short-circuit when no argument checking should be performed
+        if self.args.is_none() {
+            return true;
+        }
+
+        // find FormalParameters. Should be the next parent of param, but this
+        // is safer.
+        let Some((params, params_id)) = symbol.iter_parents().find_map(|p| {
+            if let AstKind::FormalParameters(params) = p.kind() {
+                Some((params, p.id()))
+            } else {
+                None
+            }
+        }) else {
+            debug_assert!(false, "FormalParameter should always have a parent FormalParameters");
+            return false;
+        };
+
+        if Self::is_allowed_param_because_of_method(semantic, param, params_id) {
+            return true;
+        }
+
+        // Parameters are always checked. Must be done after above checks,
+        // because in those cases a parameter is required. However, even if
+        // `args` is `all`, it may be ignored using `ignoreRestSiblings` or `destructuredArrayIgnorePattern`.
+        if self.args.is_all() {
+            return false;
+        }
+
+        debug_assert_eq!(self.args, ArgsOption::AfterUsed);
+
+        // from eslint rule documentation:
+        // after-used - unused positional arguments that occur before the last
+        // used argument will not be checked, but all named arguments and all
+        // positional arguments after the last used argument will be checked.
+
+        // unused non-positional arguments are never allowed
+        if param.pattern.kind.is_destructuring_pattern() {
+            return false;
+        }
+
+        // find the index of the parameter in the parameters list. We want to
+        // check all parameters after this one for usages.
+        let position =
+            params.items.iter().enumerate().find(|(_, p)| p.span == param.span).map(|(i, _)| i);
+        debug_assert!(
+            position.is_some(),
+            "could not find FormalParameter in a FormalParameters node that is its parent."
+        );
+        let Some(position) = position else {
+            return false;
+        };
+
+        // This is the last parameter, so need to check for usages on following parameters
+        if position == params.items.len() - 1 {
+            return false;
+        }
+
+        let ctx = BindingContext { options: self, semantic };
+        params
+            .items
+            .iter()
+            .skip(position + 1)
+            // has_modifier() to handle:
+            // constructor(unused: number, public property: string) {}
+            // no need to check if param is in a constructor, because if it's
+            // not that's a parse error.
+            .any(|p| p.has_modifier() || p.pattern.has_any_used_binding(ctx))
+    }
+
+    /// `params_id` is the [`AstNodeId`] to a [`AstKind::FormalParameters`] node.
+    ///
+    /// The following allowed conditions are handled:
+    /// 1. setter parameters - removing them causes a syntax error.
+    /// 2. TS constructor property definitions - they declare class members.
+    fn is_allowed_param_because_of_method<'a>(
+        semantic: &Semantic<'a>,
+        param: &FormalParameter<'a>,
+        params_id: AstNodeId,
+    ) -> bool {
+        let mut parents_iter = semantic.nodes().iter_parents(params_id).skip(1).map(AstNode::kind);
+
+        // in function declarations, the parent immediately before the
+        // FormalParameters is a TSDeclareBlock
+        let Some(parent) = parents_iter.next() else {
+            return false;
+        };
+        if matches!(parent, AstKind::Function(f) if f.r#type == FunctionType::TSDeclareFunction) {
+            return true;
+        }
+
+        // for non-overloads, the next parent will be the function
+        let Some(maybe_method_or_fn) = parents_iter.next() else {
+            return false;
+        };
+
+        match maybe_method_or_fn {
+            // arguments inside setters are allowed. Without them, the program
+            // has invalid syntax
+            AstKind::MethodDefinition(MethodDefinition {
+                kind: MethodDefinitionKind::Set, ..
+            })
+            | AstKind::ObjectProperty(ObjectProperty { kind: PropertyKind::Set, .. }) => true,
+
+            // Allow unused parameters in function overloads
+            AstKind::Function(f)
+                if f.body.is_none() || f.r#type == FunctionType::TSDeclareFunction =>
+            {
+                true
+            }
+            // Allow unused parameters in method overloads and overrides
+            AstKind::MethodDefinition(method)
+                if method.value.r#type == FunctionType::TSEmptyBodyFunctionExpression
+                    || method.r#override =>
+            {
+                true
+            }
+            // constructor property definitions are allowed because they declare
+            // class members
+            // e.g. `class Foo { constructor(public a) {} }`
+            AstKind::MethodDefinition(method) if method.kind.is_constructor() => {
+                param.has_modifier()
+            }
+            // parameters in abstract methods will never be directly used b/c
+            // abstract methods have no bodies. However, since this establishes
+            // an API contract and gets used by subclasses, it is allowed.
+            AstKind::MethodDefinition(method) if method.r#type.is_abstract() => true,
+            _ => false,
+        }
+    }
+}