Skip to content

Commit

Permalink
fix(linter): Don't mark binding rest elements as unused in TS functio…
Browse files Browse the repository at this point in the history
…n overloads (#5470)

- Fixes #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.
  • Loading branch information
camchenry authored Sep 6, 2024
1 parent fce549e commit ff88c1f
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 0 deletions.
15 changes: 15 additions & 0 deletions crates/oxc_linter/src/rules/eslint/no_unused_vars/allowed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
6 changes: 6 additions & 0 deletions crates/oxc_linter/src/rules/eslint/no_unused_vars/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
19 changes: 19 additions & 0 deletions crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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![
Expand Down
20 changes: 20 additions & 0 deletions crates/oxc_linter/src/snapshots/no_unused_vars@oxc-functions.snap
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,23 @@ source: crates/oxc_linter/src/tester.rs
2foo()
╰────
help: Consider removing this declaration.

eslint(no-unused-vars): Variable 'interpolations' is declared but never used.
╭─[no_unused_vars.tsx:3:46]
2export function log(message: string, ...interpolations: unknown[]): void;
3export function log(message: string, ...interpolations: unknown[]): void {
· ──────────────┬─────────────
· ╰── 'interpolations' is declared here
4console.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
2export function log(...messages: unknown[]): void {
· ───────────┬──────────
· ╰── 'messages' is declared here
3return;
╰────
help: Consider removing this declaration.

0 comments on commit ff88c1f

Please sign in to comment.