diff --git a/crates/oxc_language_server/src/linter/code_actions.rs b/crates/oxc_language_server/src/linter/code_actions.rs index 7359c8d76b7a2..6c1904dd9002a 100644 --- a/crates/oxc_language_server/src/linter/code_actions.rs +++ b/crates/oxc_language_server/src/linter/code_actions.rs @@ -1,7 +1,7 @@ use log::debug; use tower_lsp_server::lsp_types::{CodeAction, CodeActionKind, TextEdit, Uri, WorkspaceEdit}; -use crate::linter::error_with_position::{FixedContent, PossibleFixContent}; +use crate::linter::error_with_position::{FixedContent, LinterCodeAction}; pub const CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC: CodeActionKind = CodeActionKind::new("source.fixAll.oxc"); @@ -46,39 +46,25 @@ fn fix_content_to_code_action( } pub fn apply_fix_code_actions( - report: &PossibleFixContent, + report: &LinterCodeAction, alternative_message: &str, uri: &Uri, -) -> Option> { - match &report { - PossibleFixContent::None => None, - PossibleFixContent::Single(fixed_content) => { - Some(vec![fix_content_to_code_action(fixed_content, uri, alternative_message, true)]) - } - PossibleFixContent::Multiple(fixed_contents) => { - // only the first code action is preferred - let mut preferred = true; - Some( - fixed_contents - .iter() - .map(|fixed_content| { - let action = fix_content_to_code_action( - fixed_content, - uri, - alternative_message, - preferred, - ); - preferred = false; - action - }) - .collect(), - ) - } +) -> Vec { + let mut code_actions = vec![]; + + // only the first code action is preferred + let mut preferred = true; + for fixed in &report.fixed_content { + let action = fix_content_to_code_action(fixed, uri, alternative_message, preferred); + preferred = false; + code_actions.push(action); } + + code_actions } pub fn apply_all_fix_code_action<'a>( - reports: impl Iterator, + reports: impl Iterator, uri: &Uri, ) -> Option { let quick_fixes: Vec = fix_all_text_edit(reports); @@ -105,53 +91,39 @@ pub fn apply_all_fix_code_action<'a>( /// 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 fix_all_text_edit<'a>( - reports: impl Iterator, -) -> Vec { +pub fn fix_all_text_edit<'a>(reports: impl Iterator) -> Vec { let mut text_edits: Vec = vec![]; for report in reports { - let fix = match &report { - PossibleFixContent::None => None, - PossibleFixContent::Single(fixed_content) => Some(fixed_content), - // For multiple fixes, we take the first one as a representative fix. - // Applying all possible fixes at once is not possible in this context. - PossibleFixContent::Multiple(multi) => { - // for a real linter fix, we expect at least 3 fixes - if multi.len() > 2 { - multi.first() - } else { - debug!("Multiple fixes found, but only ignore fixes available"); - #[cfg(debug_assertions)] - { - if !multi.is_empty() { - debug_assert!(multi[0].message.as_ref().is_some()); - debug_assert!( - multi[0].message.as_ref().unwrap().starts_with("Disable") - ); - debug_assert!( - multi[0].message.as_ref().unwrap().ends_with("for this line") - ); - } - } + if report.fixed_content.is_empty() { + continue; + } - // this fix is only for "ignore this line/file" fixes - // do not apply them for "fix all" code action - None - } + // for a real linter fix, we expect at least 3 fixes + if report.fixed_content.len() == 2 { + debug!("Multiple fixes found, but only ignore fixes available"); + #[cfg(debug_assertions)] + { + debug_assert!(report.fixed_content[0].message.as_ref().is_some()); + debug_assert!( + report.fixed_content[0].message.as_ref().unwrap().starts_with("Disable") + ); + debug_assert!( + report.fixed_content[0].message.as_ref().unwrap().ends_with("for this line") + ); } - }; - - if let Some(fixed_content) = &fix { - // when source.fixAll.oxc we collect all changes at ones - // and return them as one workspace edit. - // it is possible that one fix will change the range for the next fix - // see oxc-project/oxc#10422 - text_edits.push(TextEdit { - range: fixed_content.range, - new_text: fixed_content.code.clone(), - }); + continue; } + + // For multiple fixes, we take the first one as a representative fix. + // Applying all possible fixes at once is not possible in this context. + let fixed_content = report.fixed_content.first().unwrap(); + // when source.fixAll.oxc we collect all changes at ones + // and return them as one workspace edit. + // it is possible that one fix will change the range for the next fix + // see oxc-project/oxc#10422 + text_edits + .push(TextEdit { range: fixed_content.range, new_text: fixed_content.code.clone() }); } text_edits diff --git a/crates/oxc_language_server/src/linter/error_with_position.rs b/crates/oxc_language_server/src/linter/error_with_position.rs index 152eb1a41b17e..3bc9576640623 100644 --- a/crates/oxc_language_server/src/linter/error_with_position.rs +++ b/crates/oxc_language_server/src/linter/error_with_position.rs @@ -12,24 +12,22 @@ use oxc_linter::{Fix, Message, PossibleFixes}; #[derive(Debug, Clone, Default)] pub struct DiagnosticReport { pub diagnostic: Diagnostic, - pub fixed_content: PossibleFixContent, + pub code_action: Option, } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Default)] +pub struct LinterCodeAction { + // pub range: Range, + pub fixed_content: Vec, +} + +#[derive(Debug, Clone, Default)] pub struct FixedContent { pub message: Option, pub code: String, pub range: Range, } -#[derive(Debug, Clone, Default)] -pub enum PossibleFixContent { - #[default] - None, - Single(FixedContent), - Multiple(Vec), -} - // clippy: the source field is checked and assumed to be less than 4GB, and // we assume that the fix offset will not exceed 2GB in either direction #[expect(clippy::cast_possible_truncation)] @@ -101,16 +99,18 @@ pub fn message_to_lsp_diagnostic( data: None, }; + let mut fixed_content = vec![]; // Convert PossibleFixes directly to PossibleFixContent - let fixed_content = match &message.fixes { - PossibleFixes::None => PossibleFixContent::None, + match &message.fixes { + PossibleFixes::None => {} PossibleFixes::Single(fix) => { - PossibleFixContent::Single(fix_to_fixed_content(fix, rope, source_text)) + fixed_content.push(fix_to_fixed_content(fix, rope, source_text)); } - PossibleFixes::Multiple(fixes) => PossibleFixContent::Multiple( - fixes.iter().map(|fix| fix_to_fixed_content(fix, rope, source_text)).collect(), - ), - }; + PossibleFixes::Multiple(fixes) => { + fixed_content + .extend(fixes.iter().map(|fix| fix_to_fixed_content(fix, rope, source_text))); + } + } // Add ignore fixes let error_offset = message.span.start; @@ -120,11 +120,17 @@ pub fn message_to_lsp_diagnostic( // and attaching a ignore comment would not ignore the error. // This is because the ignore comment would need to be placed before the error offset, which is not possible. if error_offset == section_offset && message.span.end == section_offset { - return DiagnosticReport { diagnostic, fixed_content }; + return DiagnosticReport { + diagnostic, + code_action: Some(LinterCodeAction { + // range, + fixed_content, + }), + }; } - let fixed_content = add_ignore_fixes( - fixed_content, + add_ignore_fixes( + &mut fixed_content, &message.error.code, error_offset, section_offset, @@ -132,7 +138,16 @@ pub fn message_to_lsp_diagnostic( source_text, ); - DiagnosticReport { diagnostic, fixed_content } + let code_action = if fixed_content.is_empty() { + None + } else { + Some(LinterCodeAction { + // range, + fixed_content, + }) + }; + + DiagnosticReport { diagnostic, code_action } } fn fix_to_fixed_content(fix: &Fix, rope: &Rope, source_text: &str) -> FixedContent { @@ -180,7 +195,7 @@ pub fn generate_inverted_diagnostics( tags: None, data: None, }, - fixed_content: PossibleFixContent::None, + code_action: None, }); } } @@ -197,44 +212,33 @@ pub fn offset_to_position(rope: &Rope, offset: u32, source_text: &str) -> Positi /// If the existing fixes already contain an "remove unused disable directive" fix, /// then no ignore fixes will be added. fn add_ignore_fixes( - fixes: PossibleFixContent, + fixes: &mut Vec, code: &OxcCode, error_offset: u32, section_offset: u32, rope: &Rope, source_text: &str, -) -> PossibleFixContent { +) { // do not append ignore code actions when the error is the ignore action - if matches!(fixes, PossibleFixContent::Single(ref fix) if fix.message.as_ref().is_some_and(|message| message.starts_with("remove unused disable directive"))) + if fixes.len() == 1 + && fixes[0] + .message + .as_ref() + .is_some_and(|message| message.starts_with("remove unused disable directive")) { - return fixes; - } - - let mut new_fixes: Vec = vec![]; - if let PossibleFixContent::Single(fix) = fixes { - new_fixes.push(fix); - } else if let PossibleFixContent::Multiple(existing_fixes) = fixes { - new_fixes.extend(existing_fixes); + return; } if let Some(rule_name) = code.number.as_ref() { // TODO: doesn't support disabling multiple rules by name for a given line. - new_fixes.push(disable_for_this_line( + fixes.push(disable_for_this_line( rule_name, error_offset, section_offset, rope, source_text, )); - new_fixes.push(disable_for_this_section(rule_name, section_offset, rope, source_text)); - } - - if new_fixes.is_empty() { - PossibleFixContent::None - } else if new_fixes.len() == 1 { - PossibleFixContent::Single(new_fixes.remove(0)) - } else { - PossibleFixContent::Multiple(new_fixes) + fixes.push(disable_for_this_section(rule_name, section_offset, rope, source_text)); } } diff --git a/crates/oxc_language_server/src/linter/server_linter.rs b/crates/oxc_language_server/src/linter/server_linter.rs index f8fb4b5ce4030..71f571320ea6d 100644 --- a/crates/oxc_language_server/src/linter/server_linter.rs +++ b/crates/oxc_language_server/src/linter/server_linter.rs @@ -447,7 +447,8 @@ impl Tool for ServerLinter { return Ok(None); } - let text_edits = fix_all_text_edit(value.iter().map(|report| &report.fixed_content)); + let text_edits = + fix_all_text_edit(value.iter().filter_map(|report| report.code_action.as_ref())); Ok(Some(WorkspaceEdit { #[expect(clippy::disallowed_types)] @@ -482,21 +483,21 @@ impl Tool for ServerLinter { .is_some_and(|only| only.contains(&CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC)); if is_source_fix_all_oxc { - return apply_all_fix_code_action(reports.map(|report| &report.fixed_content), uri) - .map_or(vec![], |code_actions| { - vec![CodeActionOrCommand::CodeAction(code_actions)] - }); + return apply_all_fix_code_action( + reports.filter_map(|report| report.code_action.as_ref()), + uri, + ) + .map_or(vec![], |code_actions| vec![CodeActionOrCommand::CodeAction(code_actions)]); } let mut code_actions_vec: Vec = vec![]; for report in reports { - if let Some(fix_actions) = - apply_fix_code_actions(&report.fixed_content, &report.diagnostic.message, uri) - { - code_actions_vec - .extend(fix_actions.into_iter().map(CodeActionOrCommand::CodeAction)); - } + let Some(ref code_action) = report.code_action else { + continue; + }; + let fix_actions = apply_fix_code_actions(code_action, &report.diagnostic.message, uri); + code_actions_vec.extend(fix_actions.into_iter().map(CodeActionOrCommand::CodeAction)); } code_actions_vec