From aed044462c47a611de7db639d0bb74f117a593cf Mon Sep 17 00:00:00 2001 From: Don Isaac Date: Thu, 1 Jan 2026 15:40:02 -0500 Subject: [PATCH 1/3] feat(linter/no-unused-vars): add fixer to remove unused catch bindings --- .../eslint/no_unused_vars/fixers/fix_vars.rs | 34 ++++++++++++++++++- .../src/rules/eslint/no_unused_vars/mod.rs | 28 +++++++++++---- .../rules/eslint/no_unused_vars/tests/oxc.rs | 16 ++++++++- 3 files changed, 69 insertions(+), 9 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/fixers/fix_vars.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/fixers/fix_vars.rs index 78dbb2fe9e469..880b21dc79ba2 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unused_vars/fixers/fix_vars.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unused_vars/fixers/fix_vars.rs @@ -3,7 +3,7 @@ use oxc_ast::{ ast::{Expression, ForInStatement, ForOfStatement, VariableDeclarator}, }; use oxc_semantic::NodeId; -use oxc_span::{CompactStr, GetSpan}; +use oxc_span::{CompactStr, GetSpan, Span}; use super::{BindingInfo, NoUnusedVars, Symbol, count_whitespace_or_commas}; use crate::{ @@ -145,6 +145,38 @@ impl NoUnusedVars { Some(new_name.into()) } + pub(in super::super) fn find_prev_token_pos<'a>( + &self, + source_text: &'a str, + span: Span, + expected: char, + ) -> Option { + let source = source_text[0..span.start as usize].chars().rev(); + let mut pos: u32 = 0; + for c in source { + pos += c.len_utf8() as u32; + if c.is_whitespace() { + continue; + } + return (c == expected).then_some(span.start - pos); + } + None + } + + pub(in super::super) fn find_next_token_pos<'a>( + &self, + source_text: &'a str, + span: Span, + expected: u8, + ) -> Option { + if span.end >= source_text.len() as u32 { + return None; + } + memchr::memchr(expected, source_text[span.end as usize..].as_bytes()).and_then(|idx| { + let source_idx = span.end + idx as u32; + (source_idx < source_text.len() as u32).then_some(source_idx) + }) + } } fn is_skipped_init<'a>(symbol: &Symbol<'_, 'a>, init: &Expression<'a>) -> bool { 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 aec8d09f79663..61bd9c01697e9 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 @@ -15,7 +15,7 @@ use options::{IgnorePattern, NoUnusedVarsOptions}; use oxc_ast::AstKind; use oxc_macros::declare_oxc_lint; use oxc_semantic::{AstNode, ScopeFlags, SymbolFlags}; -use oxc_span::GetSpan; +use oxc_span::{GetSpan, Span}; use symbol::Symbol; use crate::{ @@ -338,12 +338,26 @@ impl NoUnusedVars { } ctx.diagnostic(diagnostic::declared(symbol, &self.vars_ignore_pattern, false)); } - AstKind::CatchParameter(_) => { - ctx.diagnostic(diagnostic::declared( - symbol, - &self.caught_errors_ignore_pattern, - false, - )); + AstKind::CatchParameter(catch) => { + // NOTE: these are safe suggestions as deleting unused catch + // bindings wont have any side effects. + ctx.diagnostic_with_suggestion( + diagnostic::declared(symbol, &self.caught_errors_ignore_pattern, false), + |fixer| { + let source = fixer.source_text(); + let span = catch.span(); + // note: adding 1 will never cause the span to exceed the source text + // because there must always be at least a `{}` after the catch binding + // for source text to parse correctly + let (Some(start), Some(end)) = ( + self.find_prev_token_pos(source, span, '('), + self.find_next_token_pos(source, span, b')').map(|end| end + 1), + ) else { + return fixer.noop(); + }; + fixer.delete_range(Span::new(start, end)) + }, + ); } _ => ctx.diagnostic(diagnostic::declared(symbol, &IgnorePattern::<&str>::None, false)), } 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 d23409cc2634c..dacaa75895046 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 @@ -518,8 +518,22 @@ fn test_vars_catch() { ), ]; + // these suggestion fixes are safe + let fix = vec![ + ("try {} catch (error) { }", "try {} catch { }", None, FixKind::Suggestion), + ("try {} catch ({ msg }) { }", "try {} catch { }", None, FixKind::Suggestion), + // spacing + ("try {} catch (e) { }", "try {} catch { }", None, FixKind::Suggestion), + ("try {} catch(e){ }", "try {} catch{ }", None, FixKind::Suggestion), + ("try {} catch ( e) { }", "try {} catch { }", None, FixKind::Suggestion), + ("try {} catch ( e \t\n ) { }", "try {} catch { }", None, FixKind::Suggestion), + // typescript + ("try {} catch (error: Error) { }", "try {} catch { }", None, FixKind::Suggestion), + ("try {} catch (error: (typeof thing)[number]) { }", "try {} catch { }", None, FixKind::Suggestion), + ]; + Tester::new(NoUnusedVars::NAME, NoUnusedVars::PLUGIN, pass, fail) - .intentionally_allow_no_fix_tests() + .expect_fix(fix) .with_snapshot_suffix("oxc-vars-catch") .test_and_snapshot(); } From 99f11cf3a37309afa75a15d7df0ce331564cd13a Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Thu, 1 Jan 2026 20:42:56 +0000 Subject: [PATCH 2/3] [autofix.ci] apply automated fixes --- .../src/rules/eslint/no_unused_vars/tests/oxc.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 dacaa75895046..915f31bfaba48 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 @@ -529,7 +529,12 @@ fn test_vars_catch() { ("try {} catch ( e \t\n ) { }", "try {} catch { }", None, FixKind::Suggestion), // typescript ("try {} catch (error: Error) { }", "try {} catch { }", None, FixKind::Suggestion), - ("try {} catch (error: (typeof thing)[number]) { }", "try {} catch { }", None, FixKind::Suggestion), + ( + "try {} catch (error: (typeof thing)[number]) { }", + "try {} catch { }", + None, + FixKind::Suggestion, + ), ]; Tester::new(NoUnusedVars::NAME, NoUnusedVars::PLUGIN, pass, fail) From 8dc89b62611f61b2cbdc97775a50b5feba02ebe3 Mon Sep 17 00:00:00 2001 From: Don Isaac Date: Thu, 1 Jan 2026 16:33:04 -0500 Subject: [PATCH 3/3] fix: handle comments --- crates/oxc_linter/src/context/mod.rs | 12 +++++++ .../eslint/no_unused_vars/fixers/fix_vars.rs | 34 +------------------ .../src/rules/eslint/no_unused_vars/mod.rs | 18 +++++----- .../rules/eslint/no_unused_vars/tests/oxc.rs | 24 +++++++++++++ 4 files changed, 46 insertions(+), 42 deletions(-) diff --git a/crates/oxc_linter/src/context/mod.rs b/crates/oxc_linter/src/context/mod.rs index c982cb9238ad0..59c927cde2520 100644 --- a/crates/oxc_linter/src/context/mod.rs +++ b/crates/oxc_linter/src/context/mod.rs @@ -148,6 +148,18 @@ impl<'a> LintContext<'a> { .map(|(a, _)| a as u32) } + /// Finds the previous occurrence of the given token in the source code, + /// starting from the specified position, skipping over comments. + #[expect(clippy::cast_possible_truncation)] + pub fn find_prev_token_from(&self, start: u32, token: &str) -> Option { + let source = self.source_range(Span::from(0..start)); + + source + .rmatch_indices(token) + .find(|(a, _)| !self.is_inside_comment(*a as u32)) + .map(|(a, _)| a as u32) + } + /// Finds the next occurrence of the given token within a bounded span, /// starting from the specified position, skipping over comments. /// diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/fixers/fix_vars.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/fixers/fix_vars.rs index 880b21dc79ba2..78dbb2fe9e469 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unused_vars/fixers/fix_vars.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unused_vars/fixers/fix_vars.rs @@ -3,7 +3,7 @@ use oxc_ast::{ ast::{Expression, ForInStatement, ForOfStatement, VariableDeclarator}, }; use oxc_semantic::NodeId; -use oxc_span::{CompactStr, GetSpan, Span}; +use oxc_span::{CompactStr, GetSpan}; use super::{BindingInfo, NoUnusedVars, Symbol, count_whitespace_or_commas}; use crate::{ @@ -145,38 +145,6 @@ impl NoUnusedVars { Some(new_name.into()) } - pub(in super::super) fn find_prev_token_pos<'a>( - &self, - source_text: &'a str, - span: Span, - expected: char, - ) -> Option { - let source = source_text[0..span.start as usize].chars().rev(); - let mut pos: u32 = 0; - for c in source { - pos += c.len_utf8() as u32; - if c.is_whitespace() { - continue; - } - return (c == expected).then_some(span.start - pos); - } - None - } - - pub(in super::super) fn find_next_token_pos<'a>( - &self, - source_text: &'a str, - span: Span, - expected: u8, - ) -> Option { - if span.end >= source_text.len() as u32 { - return None; - } - memchr::memchr(expected, source_text[span.end as usize..].as_bytes()).and_then(|idx| { - let source_idx = span.end + idx as u32; - (source_idx < source_text.len() as u32).then_some(source_idx) - }) - } } fn is_skipped_init<'a>(symbol: &Symbol<'_, 'a>, init: &Expression<'a>) -> bool { 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 61bd9c01697e9..7191f31ef958a 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 @@ -344,18 +344,18 @@ impl NoUnusedVars { ctx.diagnostic_with_suggestion( diagnostic::declared(symbol, &self.caught_errors_ignore_pattern, false), |fixer| { - let source = fixer.source_text(); - let span = catch.span(); - // note: adding 1 will never cause the span to exceed the source text - // because there must always be at least a `{}` after the catch binding - // for source text to parse correctly - let (Some(start), Some(end)) = ( - self.find_prev_token_pos(source, span, '('), - self.find_next_token_pos(source, span, b')').map(|end| end + 1), + let Span { start, end, .. } = catch.span(); + + let (Some(paren_start), Some(paren_end_offset)) = ( + ctx.find_prev_token_from(start, "("), + ctx.find_next_token_from(end, ")"), ) else { return fixer.noop(); }; - fixer.delete_range(Span::new(start, end)) + + let paren_end = end + paren_end_offset; + let delete_span = Span::new(paren_start, paren_end + 1); + fixer.delete_range(delete_span) }, ); } 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 915f31bfaba48..16bda7e5ab993 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 @@ -521,12 +521,36 @@ fn test_vars_catch() { // these suggestion fixes are safe let fix = vec![ ("try {} catch (error) { }", "try {} catch { }", None, FixKind::Suggestion), + ( + "try { const x = (1 + 1); } catch (error) { }", + "try { const x = (1 + 1); } catch { }", + None, + FixKind::Suggestion, + ), ("try {} catch ({ msg }) { }", "try {} catch { }", None, FixKind::Suggestion), // spacing ("try {} catch (e) { }", "try {} catch { }", None, FixKind::Suggestion), ("try {} catch(e){ }", "try {} catch{ }", None, FixKind::Suggestion), ("try {} catch ( e) { }", "try {} catch { }", None, FixKind::Suggestion), ("try {} catch ( e \t\n ) { }", "try {} catch { }", None, FixKind::Suggestion), + // comments + ("try {} catch (/* comment() */ e) { }", "try {} catch { }", None, FixKind::Suggestion), + ("try {} catch (e /* comment() */) { }", "try {} catch { }", None, FixKind::Suggestion), + ( + "try {} catch /* comment */ (e) { }", + "try {} catch /* comment */ { }", + None, + FixKind::Suggestion, + ), + ( + r"try {} catch ( + // comment + // () + e) { }", + "try {} catch { }", + None, + FixKind::Suggestion, + ), // typescript ("try {} catch (error: Error) { }", "try {} catch { }", None, FixKind::Suggestion), (