From cdbfcfb39a537420d0d1b329dd4ba8d7fd18139c Mon Sep 17 00:00:00 2001 From: DonIsaac <22823424+DonIsaac@users.noreply.github.com> Date: Sun, 18 Aug 2024 23:10:00 +0000 Subject: [PATCH] feat(linter): start import fixer for eslint/no-unused-vars (#4849) --- .../no_unused_vars/fixers/fix_imports.rs | 34 ++++++++++++++++ .../rules/eslint/no_unused_vars/fixers/mod.rs | 1 + .../src/rules/eslint/no_unused_vars/mod.rs | 17 +++++++- .../rules/eslint/no_unused_vars/tests/oxc.rs | 40 +++++++++++++++++++ 4 files changed, 90 insertions(+), 2 deletions(-) create mode 100644 crates/oxc_linter/src/rules/eslint/no_unused_vars/fixers/fix_imports.rs diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/fixers/fix_imports.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/fixers/fix_imports.rs new file mode 100644 index 0000000000000..974ea600baf63 --- /dev/null +++ b/crates/oxc_linter/src/rules/eslint/no_unused_vars/fixers/fix_imports.rs @@ -0,0 +1,34 @@ +use oxc_ast::ast::ImportDeclaration; +use oxc_span::GetSpan; + +use super::{count_whitespace_or_commas, NoUnusedVars, Symbol}; +use crate::fixer::{RuleFix, RuleFixer}; + +impl NoUnusedVars { + #[allow(clippy::unused_self)] + pub(in super::super) fn remove_unused_import_declaration<'a>( + &self, + fixer: RuleFixer<'_, 'a>, + symbol: &Symbol<'_, 'a>, + import: &ImportDeclaration<'a>, + ) -> RuleFix<'a> { + let specifiers = import + .specifiers + .as_ref() + .expect("Found an unused variable in an ImportDeclaration with no specifiers. This should be impossible."); + + debug_assert!( + !specifiers.is_empty(), + "Found an unused variable in an ImportDeclaration with no specifiers. This should be impossible." + ); + + if specifiers.len() == 1 { + return fixer.delete(import).dangerously(); + } + let span = symbol.span(); + let text_after = fixer.source_text()[(span.end as usize)..].chars(); + let span = span.expand_right(count_whitespace_or_commas(text_after)); + + fixer.delete_range(span).dangerously() + } +} diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/fixers/mod.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/fixers/mod.rs index 3ae64d42c0ec9..01c3a985e1d66 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unused_vars/fixers/mod.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unused_vars/fixers/mod.rs @@ -1,3 +1,4 @@ +mod fix_imports; mod fix_symbol; mod fix_vars; 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 206fd8872c1c0..7c25868447d1f 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 @@ -13,7 +13,7 @@ use std::ops::Deref; use oxc_ast::AstKind; use oxc_macros::declare_oxc_lint; -use oxc_semantic::{ScopeFlags, SymbolFlags, SymbolId}; +use oxc_semantic::{AstNode, ScopeFlags, SymbolFlags, SymbolId}; use oxc_span::GetSpan; use crate::{context::LintContext, rule::Rule}; @@ -209,7 +209,20 @@ impl NoUnusedVars { | AstKind::ImportExpression(_) | AstKind::ImportDefaultSpecifier(_) | AstKind::ImportNamespaceSpecifier(_) => { - ctx.diagnostic(diagnostic::imported(symbol)); + let diagnostic = diagnostic::imported(symbol); + let declaration = + symbol.iter_self_and_parents().map(AstNode::kind).find_map(|kind| match kind { + AstKind::ImportDeclaration(import) => Some(import), + _ => None, + }); + + if let Some(declaration) = declaration { + ctx.diagnostic_with_suggestion(diagnostic, |fixer| { + self.remove_unused_import_declaration(fixer, symbol, declaration) + }); + } else { + ctx.diagnostic(diagnostic); + } } AstKind::VariableDeclarator(decl) => { if self.is_allowed_variable_declaration(symbol, decl) { 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 47f8ef1a6522f..f36a6fff0cd89 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 @@ -496,7 +496,47 @@ fn test_imports() { ("import { a as b } from 'a'; console.log(a)", None), ]; + let fix = vec![ + // None used + ("import foo from './foo';", "", None, FixKind::DangerousSuggestion), + ("import * as foo from './foo';", "", None, FixKind::DangerousSuggestion), + ("import { Foo } from './foo';", "", None, FixKind::DangerousSuggestion), + ("import { Foo as Bar } from './foo';", "", None, FixKind::DangerousSuggestion), + // Some used + ( + "import foo, { bar } from './foo'; bar();", + "import { bar } from './foo'; bar();", + None, + FixKind::DangerousSuggestion, + ), + ( + "import foo, { bar } from './foo'; foo();", + "import foo, { } from './foo'; foo();", + None, + FixKind::DangerousSuggestion, + ), + ( + "import { foo, bar, baz } from './foo'; foo(bar);", + "import { foo, bar, } from './foo'; foo(bar);", + None, + FixKind::DangerousSuggestion, + ), + ( + "import { foo, bar, baz } from './foo'; foo(baz);", + "import { foo, baz } from './foo'; foo(baz);", + None, + FixKind::DangerousSuggestion, + ), + ( + "import { foo, bar, baz } from './foo'; bar(baz);", + "import { bar, baz } from './foo'; bar(baz);", + None, + FixKind::DangerousSuggestion, + ), + ]; + Tester::new(NoUnusedVars::NAME, pass, fail) + .expect_fix(fix) .with_snapshot_suffix("oxc-imports") .test_and_snapshot(); }