diff --git a/apps/oxlint/fixtures/lsp/dangerous_fix/.oxlintrc.json b/apps/oxlint/fixtures/lsp/dangerous_fix/.oxlintrc.json new file mode 100644 index 0000000000000..a19e7cae4cc1c --- /dev/null +++ b/apps/oxlint/fixtures/lsp/dangerous_fix/.oxlintrc.json @@ -0,0 +1,5 @@ +{ + "rules": { + "no-unused-vars": ["error", { "fix": { "variables": "fix" } }] + } +} diff --git a/apps/oxlint/src/lsp/code_actions.rs b/apps/oxlint/src/lsp/code_actions.rs index 29b9fe66e4669..3401680948ba4 100644 --- a/apps/oxlint/src/lsp/code_actions.rs +++ b/apps/oxlint/src/lsp/code_actions.rs @@ -10,6 +10,12 @@ use crate::lsp::{ pub const CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC: CodeActionKind = CodeActionKind::new("source.fixAll.oxc"); +/// A custom code action kind for dangerous fix-all operations. +/// This is intentionally outside the `source.fixAll.*` hierarchy because the LSP spec +/// restricts `source.fixAll` to safe fixes only. +pub const CODE_ACTION_KIND_SOURCE_FIX_ALL_DANGEROUS_OXC: CodeActionKind = + CodeActionKind::new("source.fixAllDangerous.oxc"); + fn fix_content_to_code_action( fixed_content: FixedContent, uri: Uri, @@ -82,8 +88,33 @@ pub fn apply_all_fix_code_action( }) } -/// Collect all text edits from the provided diagnostic reports, which can be applied at once. -/// This is useful for implementing a "fix all" code action / command that applies multiple fixes in one go. +pub fn apply_dangerous_fix_code_action( + actions: impl Iterator, + uri: Uri, +) -> Option { + let quick_fixes: Vec = dangerous_fix_all_text_edit(actions); + + if quick_fixes.is_empty() { + return None; + } + + Some(CodeAction { + title: "fix all fixable oxlint issues".to_string(), + kind: Some(CODE_ACTION_KIND_SOURCE_FIX_ALL_DANGEROUS_OXC), + is_preferred: Some(true), + edit: Some(WorkspaceEdit { + #[expect(clippy::disallowed_types)] + changes: Some(std::collections::HashMap::from([(uri, quick_fixes)])), + ..WorkspaceEdit::default() + }), + disabled: None, + data: None, + diagnostics: None, + command: None, + }) +} + +/// Collect safe text edits from the provided diagnostic reports, which can be applied at once. pub fn fix_all_text_edit(actions: impl Iterator) -> Vec { let mut text_edits: Vec = vec![]; @@ -129,16 +160,34 @@ pub fn fix_all_text_edit(actions: impl Iterator) -> Vec result } +fn dangerous_fix_all_text_edit(actions: impl Iterator) -> Vec { + let mut text_edits: Vec = vec![]; + + for action in actions { + let Some(fixed_content) = action.fixed_content.into_iter().find(|fixed| { + matches!(fixed.lsp_kind, FixedContentKind::LintRule | FixedContentKind::UnusedDirective) + }) else { + continue; + }; + + text_edits.push(TextEdit { range: fixed_content.range, new_text: fixed_content.code }); + } + + text_edits +} + #[cfg(test)] mod tests { - use oxc_linter::FixKind; + use std::str::FromStr; + use tower_lsp_server::ls_types::{Position, Range}; - use crate::lsp::error_with_position::{FixedContent, FixedContentKind, LinterCodeAction}; + use oxc_linter::FixKind; - use super::fix_all_text_edit; + use super::*; + use crate::lsp::error_with_position::{FixedContent, FixedContentKind, LinterCodeAction}; - fn make_action( + fn make_ranged_action( start_line: u32, start_char: u32, end_line: u32, @@ -161,7 +210,7 @@ mod tests { #[test] fn test_fix_all_text_edit_sorts_by_position() { // Edits provided in reverse order should be sorted by start position. - let actions = vec![make_action(2, 0, 2, 5), make_action(0, 0, 0, 5)]; + let actions = vec![make_ranged_action(2, 0, 2, 5), make_ranged_action(0, 0, 0, 5)]; let result = fix_all_text_edit(actions.into_iter()); assert_eq!(result.len(), 2); assert_eq!(result[0].range.start.line, 0); @@ -171,7 +220,7 @@ mod tests { #[test] fn test_fix_all_text_edit_drops_overlapping() { // The second edit overlaps the first; only the first should be kept. - let actions = vec![make_action(0, 0, 0, 10), make_action(0, 5, 0, 15)]; + let actions = vec![make_ranged_action(0, 0, 0, 10), make_ranged_action(0, 5, 0, 15)]; let result = fix_all_text_edit(actions.into_iter()); assert_eq!(result.len(), 1); assert_eq!(result[0].range.end.character, 10); @@ -183,8 +232,54 @@ mod tests { // equals the start of the next) are conservatively treated as overlapping and the second // is dropped. This is a safe trade-off: the fix can be applied on the next invocation, // and it avoids any risk of producing unexpected output for boundary-sharing edits. - let actions = vec![make_action(0, 0, 0, 5), make_action(0, 5, 0, 10)]; + let actions = vec![make_ranged_action(0, 0, 0, 5), make_ranged_action(0, 5, 0, 10)]; let result = fix_all_text_edit(actions.into_iter()); assert_eq!(result.len(), 1); } + + fn make_fix_kind_action(kind: FixKind) -> LinterCodeAction { + LinterCodeAction { + range: Range::default(), + fixed_content: vec![FixedContent { + message: "remove unused import".to_string(), + code: String::new(), + range: Range::new(Position::new(0, 0), Position::new(0, 10)), + kind, + lsp_kind: FixedContentKind::LintRule, + }], + } + } + + #[test] + fn test_fix_all_text_edit_skips_dangerous_fix() { + let text_edits = + fix_all_text_edit(std::iter::once(make_fix_kind_action(FixKind::DangerousFix))); + assert!(text_edits.is_empty(), "dangerous fix should always be skipped in safe fix-all"); + } + + #[test] + fn test_fix_all_text_edit_includes_safe_fix() { + let text_edits = fix_all_text_edit(std::iter::once(make_fix_kind_action(FixKind::SafeFix))); + assert!(!text_edits.is_empty(), "safe fix should be included"); + } + + #[test] + fn test_apply_all_fix_code_action_uses_safe_kind() { + let action = apply_all_fix_code_action( + std::iter::once(make_fix_kind_action(FixKind::SafeFix)), + Uri::from_str("file:///test.js").unwrap(), + ) + .unwrap(); + assert_eq!(action.kind, Some(CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC)); + } + + #[test] + fn test_apply_dangerous_fix_code_action_uses_dangerous_kind() { + let action = apply_dangerous_fix_code_action( + std::iter::once(make_fix_kind_action(FixKind::DangerousFix)), + Uri::from_str("file:///test.js").unwrap(), + ) + .unwrap(); + assert_eq!(action.kind, Some(CODE_ACTION_KIND_SOURCE_FIX_ALL_DANGEROUS_OXC)); + } } diff --git a/apps/oxlint/src/lsp/server_linter.rs b/apps/oxlint/src/lsp/server_linter.rs index 0ae76db718e8f..95fc299318db4 100644 --- a/apps/oxlint/src/lsp/server_linter.rs +++ b/apps/oxlint/src/lsp/server_linter.rs @@ -32,7 +32,8 @@ use crate::{ config_loader::{ConfigLoader, build_nested_configs, discover_configs_in_tree}, lsp::{ code_actions::{ - CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC, apply_all_fix_code_action, apply_fix_code_actions, + CODE_ACTION_KIND_SOURCE_FIX_ALL_DANGEROUS_OXC, CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC, + apply_all_fix_code_action, apply_dangerous_fix_code_action, apply_fix_code_actions, fix_all_text_edit, }, commands::{FIX_ALL_COMMAND_ID, FixAllCommandArgs}, @@ -227,6 +228,7 @@ impl ServerLinterBuilder { Self::create_ignore_glob(&root_path), extended_paths, runner, + fix_kind, lint_options.report_unused_directive, ) } @@ -243,6 +245,7 @@ impl ToolBuilder for ServerLinterBuilder { code_action_kinds: Some(vec![ CodeActionKind::QUICKFIX, CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC, + CODE_ACTION_KIND_SOURCE_FIX_ALL_DANGEROUS_OXC, CodeActionKind::SOURCE_FIX_ALL, ]), work_done_progress_options: WorkDoneProgressOptions::default(), @@ -376,6 +379,7 @@ pub struct ServerLinter { extended_paths: FxHashSet, code_actions: Arc>>>, runner: LintRunner, + fix_kind: FixKind, unused_directives_severity: Option, } @@ -603,6 +607,18 @@ impl Tool for ServerLinter { continue; }; code_actions_vec.push(CodeActionOrCommand::CodeAction(fix_all)); + } else if kind == CODE_ACTION_KIND_SOURCE_FIX_ALL_DANGEROUS_OXC { + if !self.fix_kind.is_dangerous() { + warn!( + "Linter is not configured to provide dangerous fixes. Please set `fixKind` to `dangerous_fix` or `dangerous_fix_or_suggestion` in the server configuration to enable it." + ); + continue; + } + let Some(fix_all) = apply_dangerous_fix_code_action(actions.clone(), uri.clone()) + else { + continue; + }; + code_actions_vec.push(CodeActionOrCommand::CodeAction(fix_all)); } else if kind == CodeActionKind::QUICKFIX { for action in actions.clone() { let fix_actions = apply_fix_code_actions(action, uri); @@ -656,6 +672,7 @@ impl ServerLinter { gitignore_glob: Vec, extended_paths: FxHashSet, runner: LintRunner, + fix_kind: FixKind, unused_directives_severity: Option, ) -> Self { Self { @@ -666,6 +683,7 @@ impl ServerLinter { extended_paths, code_actions: Arc::new(ConcurrentHashMap::default()), runner, + fix_kind, unused_directives_severity, } } @@ -837,7 +855,10 @@ mod tests_builder { use oxc_language_server::{Capabilities, DiagnosticMode, ToolBuilder}; use crate::lsp::{ - code_actions::CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC, commands::FIX_ALL_COMMAND_ID, + code_actions::{ + CODE_ACTION_KIND_SOURCE_FIX_ALL_DANGEROUS_OXC, CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC, + }, + commands::FIX_ALL_COMMAND_ID, server_linter::ServerLinterBuilder, }; @@ -854,8 +875,9 @@ mod tests_builder { let code_action_kinds = options.code_action_kinds.as_ref().unwrap(); assert!(code_action_kinds.contains(&CodeActionKind::QUICKFIX)); assert!(code_action_kinds.contains(&CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC)); + assert!(code_action_kinds.contains(&CODE_ACTION_KIND_SOURCE_FIX_ALL_DANGEROUS_OXC)); assert!(code_action_kinds.contains(&CodeActionKind::SOURCE_FIX_ALL)); - assert_eq!(code_action_kinds.len(), 3); + assert_eq!(code_action_kinds.len(), 4); } _ => panic!("Expected code action provider options"), } @@ -1074,7 +1096,9 @@ mod test { }; use crate::lsp::{ - code_actions::CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC, + code_actions::{ + CODE_ACTION_KIND_SOURCE_FIX_ALL_DANGEROUS_OXC, CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC, + }, server_linter::ServerLinterBuilder, tester::{Tester, get_file_path}, }; @@ -1103,6 +1127,41 @@ mod test { assert!(configs_dirs[0].ends_with("deep1")); } + #[test] + fn test_fix_all_dangerous_returns_dangerous_fix_action() { + let tester = + Tester::new("fixtures/lsp/dangerous_fix", json!({ "fixKind": "dangerous_fix" })); + let linter = tester.create_linter(); + let range = Range::new(Position::new(0, 0), Position::new(u32::MAX, u32::MAX)); + let uri = tester.get_file_uri("unused_var.js"); + let _ = linter.run_file(&uri, Some("let a = 1;")).unwrap(); + + // source.fixAll should only return safe fixes, not dangerous ones + let safe_actions = linter.get_code_actions_or_commands( + &uri, + &range, + &CodeActionContext { + only: Some(vec![CodeActionKind::SOURCE_FIX_ALL]), + ..Default::default() + }, + ); + assert!(safe_actions.is_empty(), "source.fixAll should not apply dangerous fixes"); + + // source.fixAllDangerous.oxc should return dangerous fix actions when fix_kind is dangerous + let dangerous_actions = linter.get_code_actions_or_commands( + &uri, + &range, + &CodeActionContext { + only: Some(vec![CODE_ACTION_KIND_SOURCE_FIX_ALL_DANGEROUS_OXC]), + ..Default::default() + }, + ); + assert!( + !dangerous_actions.is_empty(), + "source.fixAllDangerous.oxc should return dangerous fix action when fix_kind is dangerous" + ); + } + #[test] fn test_code_action_context_only() { // this directory doesn't matter because we are directly calling the linter methods, and not relying on the file system for this test.