feat(linter/no-unused-vars): delete non-root arrows, skip await (#5083

)

This PR updates no-unused-var's fixer for `VariableDeclarator`s in two
ways:
1. Unused function expressions and arrow functions not declared in the
top scope
   will now be removed.
   ```ts
   // not deleted, no change
   const x = function() {}
   const y = (a) => a
   function z() {}

   // new behavior
   function foo() {            // <- not deleted
       const x = () => {}      // <- deleted
       const y = function() {} // <- this too
   }
   ```
2. Variables initialized to an `await` expression will not be deleted.
   ```ts
// unused await-initialized variables are often API calls with side
effects
   // in the real world; we don't want to delete these.
   const res = await createUser(data)
   ```

crates/oxc_linter/src/rules/eslint/no_unused_vars/fixers/fix_vars.rs

@@ -12,9 +12,16 @@ use crate::fixer::{RuleFix, RuleFixer};
 impl NoUnusedVars {
     /// Delete a variable declaration or rename it to match `varsIgnorePattern`.
     ///
-    /// Variable declarations will only be deleted if they have 0 references of any kind. Renaming
-    /// is only attempted if this is not the case. Only a small set of `varsIgnorePattern` values
-    /// are supported for renaming. Feel free to add support for more as needed.
+    /// - Variable declarations will only be deleted if they have 0 references of any kind.
+    /// - Renaming is only attempted if this is not the case.
+    /// - Fixing is skipped for the following cases:
+    ///   * Function expressions and arrow functions declared in the root scope
+    ///     (`const x = function () {}`)
+    ///   * Variables initialized with an `await` expression, since these often
+    ///     have side effects (`const unusedRes = await api.createUser(data)`)
+    ///
+    /// Only a small set of `varsIgnorePattern` values are supported for
+    /// renaming. Feel free to add support for more as needed.
     #[allow(clippy::cast_possible_truncation)]
     pub(in super::super) fn rename_or_remove_var_declaration<'a>(
         &self,
@@ -23,7 +30,7 @@ impl NoUnusedVars {
         decl: &VariableDeclarator<'a>,
         decl_id: AstNodeId,
     ) -> RuleFix<'a> {
-        if decl.init.as_ref().is_some_and(Expression::is_function) {
+        if decl.init.as_ref().is_some_and(|init| is_skipped_init(symbol, init)) {
             return fixer.noop();
         }
 
@@ -119,3 +126,17 @@ impl NoUnusedVars {
         Some(new_name.into())
     }
 }
+
+fn is_skipped_init<'a>(symbol: &Symbol<'_, 'a>, init: &Expression<'a>) -> bool {
+    match init.get_inner_expression() {
+        // Do not delete function expressions or arrow functions declared in the
+        // root scope
+        Expression::FunctionExpression(_) | Expression::ArrowFunctionExpression(_) => {
+            symbol.is_root()
+        }
+        // Skip await expressions, since these are often effectful (e.g.
+        // sending a POST request to an API and then not using the response)
+        Expression::AwaitExpression(_) => true,
+        _ => false,
+    }
+}

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

@@ -67,17 +67,11 @@ fn test_vars_simple() {
             None,
             FixKind::DangerousSuggestion,
         ),
-        // function expressions do not get changed
-        (r"const foo = () => {}", r"const foo = () => {}", None, FixKind::DangerousSuggestion),
-        (
-            r"const foo = function() {}",
-            r"const foo = function() {}",
-            None,
-            FixKind::DangerousSuggestion,
-        ),
+        // vars initialized to `await` are not removed
+        ("const x = await foo();", "const x = await foo();", None, FixKind::DangerousSuggestion),
         (
-            r"const foo = function foo() {}",
-            r"const foo = function foo() {}",
+            "const x = (await foo()) as unknown as MyType",
+            "const x = (await foo()) as unknown as MyType",
             None,
             FixKind::DangerousSuggestion,
         ),
@@ -469,8 +463,61 @@ fn test_functions() {
 
     let fail = vec!["function foo() {}", "function foo() { foo() }"];
 
+    let fix = vec![
+        // function declarations are never removed
+        ("function foo() {}", "function foo() {}", None, FixKind::DangerousSuggestion),
+        (
+            "function foo() { function bar() {} }",
+            "function foo() { function bar() {} }",
+            None,
+            FixKind::DangerousSuggestion,
+        ),
+        (
+            "function foo() { function bar() {} }\nfoo()",
+            "function foo() { function bar() {} }\nfoo()",
+            None,
+            FixKind::DangerousSuggestion,
+        ),
+        // function expressions + arrow functions are not removed if declared in
+        // the root scope
+        (
+            "const foo = function foo() {}",
+            "const foo = function foo() {}",
+            None,
+            FixKind::DangerousSuggestion,
+        ),
+        (r"const foo = () => {}", r"const foo = () => {}", None, FixKind::DangerousSuggestion),
+        // function expressions + arrow functions are removed if not declared in
+        // root scope
+        (
+            "
+                function foo() { const bar = function bar() {} }
+                foo();
+            ",
+            "
+                function foo() {  }
+                foo();
+            ",
+            None,
+            FixKind::DangerousSuggestion,
+        ),
+        (
+            "
+                function foo() { const bar = x => x }
+                foo();
+            ",
+            "
+                function foo() {  }
+                foo();
+            ",
+            None,
+            FixKind::DangerousSuggestion,
+        ),
+    ];
+
     Tester::new(NoUnusedVars::NAME, pass, fail)
         .with_snapshot_suffix("oxc-functions")
+        .expect_fix(fix)
         .test_and_snapshot();
 }