Skip to content

Commit d8815f9

Browse files
committed
refactor(linter): use PossibleFixes instead of Option<Fix>
1 parent d6fc750 commit d8815f9

File tree

14 files changed

+206
-77
lines changed

14 files changed

+206
-77
lines changed

crates/oxc_language_server/src/code_actions.rs

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use tower_lsp_server::lsp_types::{
33
WorkspaceEdit,
44
};
55

6-
use crate::linter::error_with_position::DiagnosticReport;
6+
use crate::linter::error_with_position::{DiagnosticReport, FixedContent, PossibleFixContent};
77

88
pub const CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC: CodeActionKind =
99
CodeActionKind::new("source.fixAll.oxc");
@@ -20,26 +20,26 @@ fn get_rule_name(diagnostic: &Diagnostic) -> Option<String> {
2020
None
2121
}
2222

23-
pub fn apply_fix_code_action(report: &DiagnosticReport, uri: &Uri) -> Option<CodeAction> {
24-
let Some(fixed_content) = &report.fixed_content else {
25-
return None;
26-
};
27-
23+
fn fix_content_to_code_action(
24+
fixed_content: &FixedContent,
25+
uri: &Uri,
26+
alternative_message: &str,
27+
) -> CodeAction {
2828
// 1) Use `fixed_content.message` if it exists
2929
// 2) Try to parse the report diagnostic message
3030
// 3) Fallback to "Fix this problem"
3131
let title = match fixed_content.message.clone() {
3232
Some(msg) => msg,
3333
None => {
34-
if let Some(code) = report.diagnostic.message.split(':').next() {
34+
if let Some(code) = alternative_message.split(':').next() {
3535
format!("Fix this {code} problem")
3636
} else {
3737
"Fix this problem".to_string()
3838
}
3939
}
4040
};
4141

42-
Some(CodeAction {
42+
CodeAction {
4343
title,
4444
kind: Some(CodeActionKind::QUICKFIX),
4545
is_preferred: Some(true),
@@ -55,7 +55,24 @@ pub fn apply_fix_code_action(report: &DiagnosticReport, uri: &Uri) -> Option<Cod
5555
data: None,
5656
diagnostics: None,
5757
command: None,
58-
})
58+
}
59+
}
60+
61+
pub fn apply_fix_code_actions(report: &DiagnosticReport, uri: &Uri) -> Option<Vec<CodeAction>> {
62+
match &report.fixed_content {
63+
PossibleFixContent::None => None,
64+
PossibleFixContent::Single(fixed_content) => {
65+
Some(vec![fix_content_to_code_action(fixed_content, uri, &report.diagnostic.message)])
66+
}
67+
PossibleFixContent::Multiple(fixed_contents) => Some(
68+
fixed_contents
69+
.iter()
70+
.map(|fixed_content| {
71+
fix_content_to_code_action(fixed_content, uri, &report.diagnostic.message)
72+
})
73+
.collect(),
74+
),
75+
}
5976
}
6077

6178
pub fn apply_all_fix_code_action<'a>(
@@ -65,7 +82,15 @@ pub fn apply_all_fix_code_action<'a>(
6582
let mut quick_fixes: Vec<TextEdit> = vec![];
6683

6784
for report in reports {
68-
if let Some(fixed_content) = &report.fixed_content {
85+
let fix = match &report.fixed_content {
86+
PossibleFixContent::None => None,
87+
PossibleFixContent::Single(fixed_content) => Some(fixed_content),
88+
// For multiple fixes, we take the first one as a representative fix.
89+
// Applying all possible fixes at once is not possible in this context.
90+
PossibleFixContent::Multiple(multi) => multi.first(),
91+
};
92+
93+
if let Some(fixed_content) = &fix {
6994
// when source.fixAll.oxc we collect all changes at ones
7095
// and return them as one workspace edit.
7196
// it is possible that one fix will change the range for the next fix

crates/oxc_language_server/src/linter/error_with_position.rs

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

3-
use oxc_linter::MessageWithPosition;
3+
use oxc_linter::{MessageWithPosition, PossibleFixesWithPosition};
44
use tower_lsp_server::lsp_types::{
55
self, CodeDescription, DiagnosticRelatedInformation, NumberOrString, Position, Range, Uri,
66
};
@@ -14,7 +14,7 @@ const LSP_MAX_INT: u32 = 2u32.pow(31) - 1;
1414
#[derive(Debug, Clone)]
1515
pub struct DiagnosticReport {
1616
pub diagnostic: lsp_types::Diagnostic,
17-
pub fixed_content: Option<FixedContent>,
17+
pub fixed_content: PossibleFixContent,
1818
}
1919

2020
#[derive(Debug, Clone)]
@@ -24,6 +24,14 @@ pub struct FixedContent {
2424
pub range: Range,
2525
}
2626

27+
#[derive(Debug, Clone)]
28+
pub enum PossibleFixContent {
29+
None,
30+
Single(FixedContent),
31+
#[expect(dead_code)]
32+
Multiple(Vec<FixedContent>),
33+
}
34+
2735
fn cmp_range(first: &Range, other: &Range) -> std::cmp::Ordering {
2836
match first.start.cmp(&other.start) {
2937
std::cmp::Ordering::Equal => first.end.cmp(&other.end),
@@ -107,19 +115,24 @@ pub fn message_with_position_to_lsp_diagnostic_report(
107115
) -> DiagnosticReport {
108116
DiagnosticReport {
109117
diagnostic: message_with_position_to_lsp_diagnostic(message, uri),
110-
fixed_content: message.fix.as_ref().map(|infos| FixedContent {
111-
message: infos.span.message().map(std::string::ToString::to_string),
112-
code: infos.content.to_string(),
113-
range: Range {
114-
start: Position {
115-
line: infos.span.start().line,
116-
character: infos.span.start().character,
117-
},
118-
end: Position {
119-
line: infos.span.end().line,
120-
character: infos.span.end().character,
118+
fixed_content: match &message.fixes {
119+
PossibleFixesWithPosition::None | PossibleFixesWithPosition::Multiple(_) => {
120+
PossibleFixContent::None
121+
}
122+
PossibleFixesWithPosition::Single(fix) => PossibleFixContent::Single(FixedContent {
123+
message: fix.span.message().map(std::string::ToString::to_string),
124+
code: fix.content.to_string(),
125+
range: Range {
126+
start: Position {
127+
line: fix.span.start().line,
128+
character: fix.span.start().character,
129+
},
130+
end: Position {
131+
line: fix.span.end().line,
132+
character: fix.span.end().character,
133+
},
121134
},
122-
},
123-
}),
135+
}),
136+
},
124137
}
125138
}

crates/oxc_language_server/src/linter/isolated_lint_handler.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use oxc_linter::{
1818
};
1919

2020
use super::error_with_position::{
21-
DiagnosticReport, message_with_position_to_lsp_diagnostic_report,
21+
DiagnosticReport, PossibleFixContent, message_with_position_to_lsp_diagnostic_report,
2222
};
2323

2424
/// smaller subset of LintServiceOptions, which is used by IsolatedLintHandler
@@ -109,7 +109,7 @@ impl IsolatedLintHandler {
109109
tags: None,
110110
data: None,
111111
},
112-
fixed_content: None,
112+
fixed_content: PossibleFixContent::None,
113113
});
114114
}
115115
}

crates/oxc_language_server/src/snapshots/[email protected]

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ related_information[0].location.range: Range { start: Position { line: 1, charac
1212
severity: Some(Warning)
1313
source: Some("oxc")
1414
tags: None
15-
fixed: Some(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 1, character: 0 }, end: Position { line: 1, character: 8 } } })
15+
fixed: Single(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 1, character: 0 }, end: Position { line: 1, character: 8 } } })
1616

1717

1818
code: "eslint(no-debugger)"
@@ -25,7 +25,7 @@ related_information[0].location.range: Range { start: Position { line: 10, chara
2525
severity: Some(Warning)
2626
source: Some("oxc")
2727
tags: None
28-
fixed: Some(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 10, character: 2 }, end: Position { line: 10, character: 10 } } })
28+
fixed: Single(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 10, character: 2 }, end: Position { line: 10, character: 10 } } })
2929

3030

3131
code: "eslint(no-debugger)"
@@ -38,7 +38,7 @@ related_information[0].location.range: Range { start: Position { line: 14, chara
3838
severity: Some(Warning)
3939
source: Some("oxc")
4040
tags: None
41-
fixed: Some(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 14, character: 2 }, end: Position { line: 14, character: 10 } } })
41+
fixed: Single(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 14, character: 2 }, end: Position { line: 14, character: 10 } } })
4242

4343

4444
code: "eslint(no-debugger)"
@@ -51,4 +51,4 @@ related_information[0].location.range: Range { start: Position { line: 18, chara
5151
severity: Some(Warning)
5252
source: Some("oxc")
5353
tags: None
54-
fixed: Some(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 18, character: 2 }, end: Position { line: 18, character: 10 } } })
54+
fixed: Single(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 18, character: 2 }, end: Position { line: 18, character: 10 } } })

crates/oxc_language_server/src/snapshots/[email protected]

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,4 @@ related_information[0].location.range: Range { start: Position { line: 1, charac
1212
severity: Some(Warning)
1313
source: Some("oxc")
1414
tags: None
15-
fixed: Some(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 1, character: 0 }, end: Position { line: 1, character: 9 } } })
15+
fixed: Single(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 1, character: 0 }, end: Position { line: 1, character: 9 } } })

crates/oxc_language_server/src/snapshots/[email protected]

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ related_information[1].location.range: Range { start: Position { line: 11, chara
2828
severity: Some(Error)
2929
source: Some("oxc")
3030
tags: None
31-
fixed: Some(FixedContent { message: Some("Delete this code."), code: "", range: Range { start: Position { line: 11, character: 21 }, end: Position { line: 11, character: 22 } } })
31+
fixed: Single(FixedContent { message: Some("Delete this code."), code: "", range: Range { start: Position { line: 11, character: 21 }, end: Position { line: 11, character: 22 } } })
3232

3333

3434
code: "None"

crates/oxc_language_server/src/snapshots/[email protected]

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,4 @@ related_information[0].location.range: Range { start: Position { line: 0, charac
2525
severity: Some(Error)
2626
source: Some("oxc")
2727
tags: None
28-
fixed: Some(FixedContent { message: Some("Replace `\\/` with `/`."), code: "/", range: Range { start: Position { line: 0, character: 16 }, end: Position { line: 0, character: 18 } } })
28+
fixed: Single(FixedContent { message: Some("Replace `\\/` with `/`."), code: "/", range: Range { start: Position { line: 0, character: 16 }, end: Position { line: 0, character: 18 } } })

crates/oxc_language_server/src/snapshots/[email protected]

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,4 @@ related_information[0].location.range: Range { start: Position { line: 1, charac
1212
severity: Some(Warning)
1313
source: Some("oxc")
1414
tags: None
15-
fixed: Some(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 1, character: 1 }, end: Position { line: 1, character: 10 } } })
15+
fixed: Single(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 1, character: 1 }, end: Position { line: 1, character: 10 } } })

crates/oxc_language_server/src/snapshots/[email protected]

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ related_information[0].location.range: Range { start: Position { line: 5, charac
1212
severity: Some(Warning)
1313
source: Some("oxc")
1414
tags: None
15-
fixed: Some(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 5, character: 4 }, end: Position { line: 5, character: 12 } } })
15+
fixed: Single(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 5, character: 4 }, end: Position { line: 5, character: 12 } } })
1616

1717

1818
code: "eslint(no-debugger)"
@@ -25,4 +25,4 @@ related_information[0].location.range: Range { start: Position { line: 10, chara
2525
severity: Some(Warning)
2626
source: Some("oxc")
2727
tags: None
28-
fixed: Some(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 10, character: 4 }, end: Position { line: 10, character: 13 } } })
28+
fixed: Single(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 10, character: 4 }, end: Position { line: 10, character: 13 } } })

crates/oxc_language_server/src/worker.rs

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@ use tower_lsp_server::{
1414
use crate::{
1515
ConcurrentHashMap, Options, Run,
1616
code_actions::{
17-
apply_all_fix_code_action, apply_fix_code_action, ignore_this_line_code_action,
17+
apply_all_fix_code_action, apply_fix_code_actions, ignore_this_line_code_action,
1818
ignore_this_rule_code_action,
1919
},
2020
linter::{
21-
error_with_position::DiagnosticReport,
21+
error_with_position::{DiagnosticReport, PossibleFixContent},
2222
server_linter::{ServerLinter, normalize_path},
2323
},
2424
};
@@ -228,8 +228,9 @@ impl WorkspaceWorker {
228228
let mut code_actions_vec: Vec<CodeActionOrCommand> = vec![];
229229

230230
for report in reports {
231-
if let Some(fix_action) = apply_fix_code_action(report, uri) {
232-
code_actions_vec.push(CodeActionOrCommand::CodeAction(fix_action));
231+
if let Some(fix_actions) = apply_fix_code_actions(report, uri) {
232+
code_actions_vec
233+
.extend(fix_actions.into_iter().map(CodeActionOrCommand::CodeAction));
233234
}
234235

235236
code_actions_vec
@@ -242,6 +243,7 @@ impl WorkspaceWorker {
242243
code_actions_vec
243244
}
244245

246+
/// This function is used for executing the `oxc.fixAll` command
245247
pub async fn get_diagnostic_text_edits(&self, uri: &Uri) -> Vec<TextEdit> {
246248
let report_map_ref = self.diagnostics_report_map.pin_owned();
247249
let value = match report_map_ref.get(&uri.to_string()) {
@@ -258,7 +260,15 @@ impl WorkspaceWorker {
258260
let mut text_edits = vec![];
259261

260262
for report in value {
261-
if let Some(fixed_content) = &report.fixed_content {
263+
let fix = match &report.fixed_content {
264+
PossibleFixContent::None => None,
265+
PossibleFixContent::Single(fixed_content) => Some(fixed_content),
266+
// For multiple fixes, we take the first one as a representative fix.
267+
// Applying all possible fixes at once is not possible in this context.
268+
PossibleFixContent::Multiple(multi) => multi.first(),
269+
};
270+
271+
if let Some(fixed_content) = &fix {
262272
text_edits.push(TextEdit {
263273
range: fixed_content.range,
264274
new_text: fixed_content.code.clone(),

crates/oxc_linter/src/fixer/fix.rs

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,62 @@ impl<'a> Fix<'a> {
343343
}
344344
}
345345

346+
#[derive(Clone)]
347+
pub enum PossibleFixes<'a> {
348+
None,
349+
Single(Fix<'a>),
350+
Multiple(Vec<Fix<'a>>),
351+
}
352+
353+
impl<'new> CloneIn<'new> for PossibleFixes<'_> {
354+
type Cloned = PossibleFixes<'new>;
355+
356+
fn clone_in(&self, allocator: &'new Allocator) -> Self::Cloned {
357+
match self {
358+
Self::None => PossibleFixes::None,
359+
Self::Single(fix) => PossibleFixes::Single(fix.clone_in(allocator)),
360+
Self::Multiple(fixes) => {
361+
//ToDo: what about the vec?
362+
PossibleFixes::Multiple(fixes.iter().map(|fix| fix.clone_in(allocator)).collect())
363+
}
364+
}
365+
}
366+
}
367+
368+
impl PossibleFixes<'_> {
369+
/// Gets the number of [`Fix`]es contained in this [`PossibleFixes`].
370+
pub fn len(&self) -> usize {
371+
match self {
372+
PossibleFixes::None => 0,
373+
PossibleFixes::Single(_) => 1,
374+
PossibleFixes::Multiple(fixes) => fixes.len(),
375+
}
376+
}
377+
378+
/// Returns `true` if this [`PossibleFixes`] contains no [`Fix`]es
379+
pub fn is_empty(&self) -> bool {
380+
self.len() == 0
381+
}
382+
383+
pub fn span(&self) -> Span {
384+
match self {
385+
PossibleFixes::None => SPAN,
386+
PossibleFixes::Single(fix) => fix.span,
387+
PossibleFixes::Multiple(fixes) => {
388+
fixes.iter().map(|fix| fix.span).reduce(Span::merge).unwrap_or(SPAN)
389+
}
390+
}
391+
}
392+
}
393+
394+
#[cfg(feature = "language_server")]
395+
#[derive(Debug)]
396+
pub enum PossibleFixesWithPosition<'a> {
397+
None,
398+
Single(FixWithPosition<'a>),
399+
Multiple(Vec<FixWithPosition<'a>>),
400+
}
401+
346402
// NOTE (@DonIsaac): having these variants is effectively the same as interning
347403
// single or 0-element Vecs. I experimented with using smallvec here, but the
348404
// resulting struct size was larger (40 bytes vs 32). So, we're sticking with

0 commit comments

Comments
 (0)