Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(linter): Don't mark binding rest elements as unused in TS function overloads #5470

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 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 @@ -298,4 +298,40 @@ 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<'a>(
&self,
semantic: &Semantic<'a>,
symbol: &Symbol<'_, 'a>,
) -> bool {
// Handle binding rest element as part of a function parameter, for example:
// `function foo(...messages: string[]) {}`
let parent = symbol.iter_parents().find_map(|p| match p.kind() {
AstKind::FormalParameters(_) => Some(p.id()),
_ => None,
});
match parent {
Some(params_id) => {
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;
};
// allow unused rest parameters in function overloads
if matches!(parent, AstKind::Function(f) if f.r#type == FunctionType::TSDeclareFunction)
{
return true;
}

false
}
None => 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 @@ -298,6 +298,12 @@ impl NoUnusedVars {
}
ctx.diagnostic(diagnostic::param(symbol));
}
AstKind::BindingRestElement(_) => {
if self.is_allowed_binding_rest_element(ctx.semantic().as_ref(), symbol) {
return;
}
ctx.diagnostic(diagnostic::declared(symbol));
}
AstKind::Class(_) | AstKind::Function(_) => {
if self.is_allowed_class_or_function(symbol) {
return;
Expand Down
21 changes: 20 additions & 1 deletion crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,13 +476,32 @@ fn test_functions() {
});
",
"const foo = () => function bar() { }\nfoo()",
"module.exports.foo = () => function bar() { }"
"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
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.