From ff88c1fc0723a3b103315e8023ef772b4cfa38ff Mon Sep 17 00:00:00 2001
From: Cam McHenry <camchenry@users.noreply.github.com>
Date: Fri, 6 Sep 2024 03:30:28 -0400
Subject: [PATCH] fix(linter): Don't mark binding rest elements as unused in TS
 function overloads (#5470)

- Fixes https://github.com/oxc-project/oxc/issues/5406

This implements a fix for the `BindingRestElement` symbol, which is
currently unhandled and gets automatically marked as unused. If we
happen to find that it is a child of declaration, then we will
automatically allow the binding rest element.

The code for this was based on what we currently do in
`is_allowed_param_because_of_method`:
https://github.com/oxc-project/oxc/blob/5187f384cb38b1ba69acc2cb9b677d72ef175e3e/crates/oxc_linter/src/rules/eslint/no_unused_vars/allowed.rs#L258

I opted not to refactor this to re-use the same code though, as I think
the duplication is still incidental and the implementations could
diverge in the future.
---
 .../rules/eslint/no_unused_vars/allowed.rs    | 15 ++++++++++++++
 .../src/rules/eslint/no_unused_vars/mod.rs    |  6 ++++++
 .../rules/eslint/no_unused_vars/tests/oxc.rs  | 19 ++++++++++++++++++
 .../no_unused_vars@oxc-functions.snap         | 20 +++++++++++++++++++
 4 files changed, 60 insertions(+)

diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/allowed.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/allowed.rs
index 66a405bca59c8..3a9cc1cb34f1b 100644
--- a/crates/oxc_linter/src/rules/eslint/no_unused_vars/allowed.rs
+++ b/crates/oxc_linter/src/rules/eslint/no_unused_vars/allowed.rs
@@ -294,4 +294,19 @@ impl NoUnusedVars {
             _ => false,
         }
     }
+
+    /// Returns `true` if this binding rest element should be allowed (i.e. not
+    /// reported). Currently, this handles the case where a rest element is part
+    /// of a TS function declaration.
+    pub(super) fn is_allowed_binding_rest_element(symbol: &Symbol) -> bool {
+        for parent in symbol.iter_parents() {
+            // If this is a binding rest element that is part of a TS function parameter,
+            // for example: `function foo(...messages: string[]) {}`, then we will allow it.
+            if let AstKind::Function(f) = parent.kind() {
+                return f.is_typescript_syntax();
+            }
+        }
+
+        false
+    }
 }
diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/mod.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/mod.rs
index e372623322665..426a50bf05f77 100644
--- a/crates/oxc_linter/src/rules/eslint/no_unused_vars/mod.rs
+++ b/crates/oxc_linter/src/rules/eslint/no_unused_vars/mod.rs
@@ -297,6 +297,12 @@ impl NoUnusedVars {
                 }
                 ctx.diagnostic(diagnostic::param(symbol));
             }
+            AstKind::BindingRestElement(_) => {
+                if NoUnusedVars::is_allowed_binding_rest_element(symbol) {
+                    return;
+                }
+                ctx.diagnostic(diagnostic::declared(symbol));
+            }
             AstKind::Class(_) | AstKind::Function(_) => {
                 if self.is_allowed_class_or_function(symbol) {
                     return;
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 b3bf68f00e264..6b79ab96c2601 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
@@ -495,12 +495,31 @@ fn test_functions() {
         ",
         "const foo = () => function bar() { }\nfoo()",
         "module.exports.foo = () => function bar() { }",
+        // https://github.com/oxc-project/oxc/issues/5406
+        "
+        export function log(message: string, ...interpolations: unknown[]): void;
+        export function log(message: string, ...interpolations: unknown[]): void {
+            console.log(message, interpolations);
+        }
+        ",
+        "declare function func(strings: any, ...values: any[]): object"
     ];
 
     let fail = vec![
         "function foo() {}",
         "function foo() { foo() }",
         "const foo = () => { function bar() { } }\nfoo()",
+        "
+        export function log(message: string, ...interpolations: unknown[]): void;
+        export function log(message: string, ...interpolations: unknown[]): void {
+            console.log(message);
+        }
+        ",
+        "
+        export function log(...messages: unknown[]): void {
+            return;
+        }
+        ",
     ];
 
     let fix = vec![
diff --git a/crates/oxc_linter/src/snapshots/no_unused_vars@oxc-functions.snap b/crates/oxc_linter/src/snapshots/no_unused_vars@oxc-functions.snap
index 6903fab79de39..31c927622e235 100644
--- a/crates/oxc_linter/src/snapshots/no_unused_vars@oxc-functions.snap
+++ b/crates/oxc_linter/src/snapshots/no_unused_vars@oxc-functions.snap
@@ -25,3 +25,23 @@ source: crates/oxc_linter/src/tester.rs
  2 │ foo()
    ╰────
   help: Consider removing this declaration.
+
+  ⚠ eslint(no-unused-vars): Variable 'interpolations' is declared but never used.
+   ╭─[no_unused_vars.tsx:3:46]
+ 2 │         export function log(message: string, ...interpolations: unknown[]): void;
+ 3 │         export function log(message: string, ...interpolations: unknown[]): void {
+   ·                                              ──────────────┬─────────────
+   ·                                                            ╰── 'interpolations' is declared here
+ 4 │             console.log(message);
+   ╰────
+  help: Consider removing this declaration.
+
+  ⚠ eslint(no-unused-vars): Variable 'messages' is declared but never used.
+   ╭─[no_unused_vars.tsx:2:29]
+ 1 │ 
+ 2 │         export function log(...messages: unknown[]): void {
+   ·                             ───────────┬──────────
+   ·                                        ╰── 'messages' is declared here
+ 3 │             return;
+   ╰────
+  help: Consider removing this declaration.