Skip to content

Commit e887d24

Browse files
committed
refactor(oxlint/lsp): prefill FixedContent.message and remove Option wrapper (#16515)
> This PR refactors the linter's fix message handling by removing the Option wrapper from FixedContent.message and prefilling messages earlier in the processing pipeline. The change simplifies code action generation by moving the fallback message logic from fix_content_to_code_action into message_to_lsp_diagnostic, where fix messages are now populated before being converted to FixedContent. The goal in the next PR is that the alternative message (from diagnostic) is no longer be stored in memory. So the title should be updated beforehand.
1 parent 6bac4ec commit e887d24

File tree

4 files changed

+40
-47
lines changed

4 files changed

+40
-47
lines changed

crates/oxc_language_server/src/linter/code_actions.rs

Lines changed: 5 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -9,25 +9,10 @@ pub const CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC: CodeActionKind =
99
fn fix_content_to_code_action(
1010
fixed_content: &FixedContent,
1111
uri: &Uri,
12-
alternative_message: &str,
1312
is_preferred: bool,
1413
) -> CodeAction {
15-
// 1) Use `fixed_content.message` if it exists
16-
// 2) Try to parse the report diagnostic message
17-
// 3) Fallback to "Fix this problem"
18-
let title = match fixed_content.message.clone() {
19-
Some(msg) => msg,
20-
None => {
21-
if let Some(code) = alternative_message.split(':').next() {
22-
format!("Fix this {code} problem")
23-
} else {
24-
"Fix this problem".to_string()
25-
}
26-
}
27-
};
28-
2914
CodeAction {
30-
title,
15+
title: fixed_content.message.clone(),
3116
kind: Some(CodeActionKind::QUICKFIX),
3217
is_preferred: Some(is_preferred),
3318
edit: Some(WorkspaceEdit {
@@ -45,17 +30,13 @@ fn fix_content_to_code_action(
4530
}
4631
}
4732

48-
pub fn apply_fix_code_actions(
49-
report: &LinterCodeAction,
50-
alternative_message: &str,
51-
uri: &Uri,
52-
) -> Vec<CodeAction> {
33+
pub fn apply_fix_code_actions(report: &LinterCodeAction, uri: &Uri) -> Vec<CodeAction> {
5334
let mut code_actions = vec![];
5435

5536
// only the first code action is preferred
5637
let mut preferred = true;
5738
for fixed in &report.fixed_content {
58-
let action = fix_content_to_code_action(fixed, uri, alternative_message, preferred);
39+
let action = fix_content_to_code_action(fixed, uri, preferred);
5940
preferred = false;
6041
code_actions.push(action);
6142
}
@@ -104,13 +85,8 @@ pub fn fix_all_text_edit<'a>(reports: impl Iterator<Item = &'a LinterCodeAction>
10485
debug!("Multiple fixes found, but only ignore fixes available");
10586
#[cfg(debug_assertions)]
10687
{
107-
debug_assert!(report.fixed_content[0].message.as_ref().is_some());
108-
debug_assert!(
109-
report.fixed_content[0].message.as_ref().unwrap().starts_with("Disable")
110-
);
111-
debug_assert!(
112-
report.fixed_content[0].message.as_ref().unwrap().ends_with("for this line")
113-
);
88+
debug_assert!(report.fixed_content[0].message.starts_with("Disable"));
89+
debug_assert!(report.fixed_content[0].message.ends_with("for this line"));
11490
}
11591
continue;
11692
}

crates/oxc_language_server/src/linter/error_with_position.rs

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::str::FromStr;
1+
use std::{borrow::Cow, str::FromStr};
22

33
use tower_lsp_server::lsp_types::{
44
self, CodeDescription, Diagnostic, DiagnosticRelatedInformation, DiagnosticSeverity,
@@ -23,7 +23,7 @@ pub struct LinterCodeAction {
2323

2424
#[derive(Debug, Clone, Default)]
2525
pub struct FixedContent {
26-
pub message: Option<String>,
26+
pub message: String,
2727
pub code: String,
2828
pub range: Range,
2929
}
@@ -32,7 +32,7 @@ pub struct FixedContent {
3232
// we assume that the fix offset will not exceed 2GB in either direction
3333
#[expect(clippy::cast_possible_truncation)]
3434
pub fn message_to_lsp_diagnostic(
35-
message: &Message,
35+
mut message: Message,
3636
uri: &Uri,
3737
source_text: &str,
3838
rope: &Rope,
@@ -87,6 +87,16 @@ pub fn message_to_lsp_diagnostic(
8787
None => message.error.message.to_string(),
8888
};
8989

90+
// 1) Use `fixed_content.message` if it exists
91+
// 2) Try to parse the report diagnostic message
92+
// 3) Fallback to "Fix this problem"
93+
let alternative_fix_title: Cow<'static, str> =
94+
if let Some(code) = diagnostic_message.split(':').next() {
95+
format!("Fix this {code} problem").into()
96+
} else {
97+
std::borrow::Cow::Borrowed("Fix this problem")
98+
};
99+
90100
let diagnostic = Diagnostic {
91101
range,
92102
severity,
@@ -101,14 +111,21 @@ pub fn message_to_lsp_diagnostic(
101111

102112
let mut fixed_content = vec![];
103113
// Convert PossibleFixes directly to PossibleFixContent
104-
match &message.fixes {
114+
match &mut message.fixes {
105115
PossibleFixes::None => {}
106116
PossibleFixes::Single(fix) => {
117+
if fix.message.is_none() {
118+
fix.message = Some(alternative_fix_title);
119+
}
107120
fixed_content.push(fix_to_fixed_content(fix, rope, source_text));
108121
}
109122
PossibleFixes::Multiple(fixes) => {
110-
fixed_content
111-
.extend(fixes.iter().map(|fix| fix_to_fixed_content(fix, rope, source_text)));
123+
fixed_content.extend(fixes.iter_mut().map(|fix| {
124+
if fix.message.is_none() {
125+
fix.message = Some(alternative_fix_title.clone());
126+
}
127+
fix_to_fixed_content(fix, rope, source_text)
128+
}));
112129
}
113130
}
114131

@@ -154,8 +171,13 @@ fn fix_to_fixed_content(fix: &Fix, rope: &Rope, source_text: &str) -> FixedConte
154171
let start_position = offset_to_position(rope, fix.span.start, source_text);
155172
let end_position = offset_to_position(rope, fix.span.end, source_text);
156173

174+
debug_assert!(
175+
fix.message.is_some(),
176+
"Fix message should be present. `message_to_lsp_diagnostic` should modify fixes to include messages."
177+
);
178+
157179
FixedContent {
158-
message: fix.message.as_ref().map(std::string::ToString::to_string),
180+
message: fix.message.as_ref().map(std::string::ToString::to_string).unwrap_or_default(),
159181
code: fix.content.to_string(),
160182
range: Range::new(start_position, end_position),
161183
}
@@ -220,12 +242,7 @@ fn add_ignore_fixes(
220242
source_text: &str,
221243
) {
222244
// do not append ignore code actions when the error is the ignore action
223-
if fixes.len() == 1
224-
&& fixes[0]
225-
.message
226-
.as_ref()
227-
.is_some_and(|message| message.starts_with("remove unused disable directive"))
228-
{
245+
if fixes.len() == 1 && fixes[0].message.starts_with("remove unused disable directive") {
229246
return;
230247
}
231248

@@ -281,7 +298,7 @@ fn disable_for_this_line(
281298

282299
let position = offset_to_position(rope, insert_offset, source_text);
283300
FixedContent {
284-
message: Some(format!("Disable {rule_name} for this line")),
301+
message: format!("Disable {rule_name} for this line"),
285302
code: format!(
286303
"{content_prefix}{whitespace_string}// oxlint-disable-next-line {rule_name}\n"
287304
),
@@ -304,7 +321,7 @@ fn disable_for_this_section(
304321
let position = offset_to_position(rope, insert_offset, source_text);
305322

306323
FixedContent {
307-
message: Some(format!("Disable {rule_name} for this whole file")),
324+
message: format!("Disable {rule_name} for this whole file"),
308325
code: content,
309326
range: Range::new(position, position),
310327
}

crates/oxc_language_server/src/linter/isolated_lint_handler.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ impl IsolatedLintHandler {
126126
let mut messages: Vec<DiagnosticReport> = self
127127
.runner
128128
.run_source(&Arc::from(path.as_os_str()), source_text.to_string(), &fs)
129-
.iter()
129+
.into_iter()
130130
.map(|message| message_to_lsp_diagnostic(message, uri, source_text, rope))
131131
.collect();
132132

@@ -136,7 +136,7 @@ impl IsolatedLintHandler {
136136
{
137137
messages.extend(
138138
create_unused_directives_messages(&directives, severity, source_text)
139-
.iter()
139+
.into_iter()
140140
.map(|message| message_to_lsp_diagnostic(message, uri, source_text, rope)),
141141
);
142142
}

crates/oxc_language_server/src/linter/server_linter.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -496,7 +496,7 @@ impl Tool for ServerLinter {
496496
let Some(ref code_action) = report.code_action else {
497497
continue;
498498
};
499-
let fix_actions = apply_fix_code_actions(code_action, &report.diagnostic.message, uri);
499+
let fix_actions = apply_fix_code_actions(code_action, uri);
500500
code_actions_vec.extend(fix_actions.into_iter().map(CodeActionOrCommand::CodeAction));
501501
}
502502

0 commit comments

Comments
 (0)