Skip to content

refactor(oxlint/lsp): store Option<Vec<FixedContent>> for DiagnosticReport code action part #16514

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.

Sign up for GitHub

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

Merged
graphite-app merged 1 commit into from
Dec 5, 2025
+100 −123
Merged

refactor(oxlint/lsp): store Option<Vec<FixedContent>> for DiagnosticReport code action part #16514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
110 changes: 41 additions & 69 deletions crates/oxc_language_server/src/linter/code_actions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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");
Expand Down Expand Up @@ -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<Vec<CodeAction>> {
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<CodeAction> {
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<Item = &'a PossibleFixContent>,
reports: impl Iterator<Item = &'a LinterCodeAction>,
uri: &Uri,
) -> Option<CodeAction> {
let quick_fixes: Vec<TextEdit> = fix_all_text_edit(reports);
Expand All @@ -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<Item = &'a PossibleFixContent>,
) -> Vec<TextEdit> {
pub fn fix_all_text_edit<'a>(reports: impl Iterator<Item = &'a LinterCodeAction>) -> Vec<TextEdit> {
let mut text_edits: Vec<TextEdit> = 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
Expand Down
90 changes: 47 additions & 43 deletions crates/oxc_language_server/src/linter/error_with_position.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<LinterCodeAction>,
}

#[derive(Debug, Clone)]
#[derive(Debug, Clone, Default)]
pub struct LinterCodeAction {
// pub range: Range,
pub fixed_content: Vec<FixedContent>,
}

#[derive(Debug, Clone, Default)]
pub struct FixedContent {
pub message: Option<String>,
pub code: String,
pub range: Range,
}

#[derive(Debug, Clone, Default)]
pub enum PossibleFixContent {
#[default]
None,
Single(FixedContent),
Multiple(Vec<FixedContent>),
}

// 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)]
Expand Down Expand Up @@ -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;
Expand All @@ -120,19 +120,34 @@ 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,
rope,
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 {
Expand Down Expand Up @@ -180,7 +195,7 @@ pub fn generate_inverted_diagnostics(
tags: None,
data: None,
},
fixed_content: PossibleFixContent::None,
code_action: None,
});
}
}
Expand All @@ -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<FixedContent>,
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<FixedContent> = 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));
}
}

Expand Down
23 changes: 12 additions & 11 deletions crates/oxc_language_server/src/linter/server_linter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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<CodeActionOrCommand> = 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
Expand Down
Loading