Skip to content

Commit d5ca528

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

File tree

14 files changed

+159
-67
lines changed

14 files changed

+159
-67
lines changed

‎crates/oxc_language_server/src/code_actions.rs

Lines changed: 5 additions & 3 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, PossibleFixContent};
77

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

2323
pub fn apply_fix_code_action(report: &DiagnosticReport, uri: &Uri) -> Option<CodeAction> {
24-
let Some(fixed_content) = &report.fixed_content else {
24+
// ToDo: Handle multiple fixes in a single diagnostic report
25+
let PossibleFixContent::Single(fixed_content) = &report.fixed_content else {
2526
return None;
2627
};
2728

@@ -65,7 +66,8 @@ pub fn apply_all_fix_code_action<'a>(
6566
let mut quick_fixes: Vec<TextEdit> = vec![];
6667

6768
for report in reports {
68-
if let Some(fixed_content) = &report.fixed_content {
69+
// ToDo: Handle multiple fixes in a single diagnostic report
70+
if let PossibleFixContent::Single(fixed_content) = &report.fixed_content {
6971
// when source.fixAll.oxc we collect all changes at ones
7072
// and return them as one workspace edit.
7173
// 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: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use crate::{
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
};
@@ -258,7 +258,8 @@ impl WorkspaceWorker {
258258
let mut text_edits = vec![];
259259

260260
for report in value {
261-
if let Some(fixed_content) = &report.fixed_content {
261+
// ToDo: Handle multiple fixes in a single diagnostic report
262+
if let PossibleFixContent::Single(fixed_content) = &report.fixed_content {
262263
text_edits.push(TextEdit {
263264
range: fixed_content.range,
264265
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

‎crates/oxc_linter/src/fixer/mod.rs

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@ use crate::LintContext;
99
#[cfg(feature = "language_server")]
1010
use crate::service::offset_to_position::SpanPositionMessage;
1111
#[cfg(feature = "language_server")]
12-
pub use fix::FixWithPosition;
12+
pub use fix::{FixWithPosition, PossibleFixesWithPosition};
1313
#[cfg(feature = "language_server")]
1414
use oxc_diagnostics::{OxcCode, Severity};
1515

1616
mod fix;
17-
pub use fix::{CompositeFix, Fix, FixKind, RuleFix};
17+
pub use fix::{CompositeFix, Fix, FixKind, PossibleFixes, RuleFix};
1818
use oxc_allocator::{Allocator, CloneIn};
1919

2020
/// Produces [`RuleFix`] instances. Inspired by ESLint's [`RuleFixer`].
@@ -227,7 +227,7 @@ pub struct FixResult<'a> {
227227
#[derive(Clone)]
228228
pub struct Message<'a> {
229229
pub error: OxcDiagnostic,
230-
pub fix: Option<Fix<'a>>,
230+
pub fixes: PossibleFixes<'a>,
231231
span: Span,
232232
fixed: bool,
233233
}
@@ -241,7 +241,7 @@ pub struct MessageWithPosition<'a> {
241241
pub severity: Severity,
242242
pub code: OxcCode,
243243
pub url: Option<Cow<'a, str>>,
244-
pub fix: Option<FixWithPosition<'a>>,
244+
pub fixes: PossibleFixesWithPosition<'a>,
245245
}
246246

247247
#[cfg(feature = "language_server")]
@@ -254,7 +254,7 @@ impl From<OxcDiagnostic> for MessageWithPosition<'_> {
254254
severity: from.severity,
255255
code: from.code.clone(),
256256
url: from.url.clone(),
257-
fix: None,
257+
fixes: PossibleFixesWithPosition::None,
258258
}
259259
}
260260
}
@@ -265,7 +265,7 @@ impl<'new> CloneIn<'new> for Message<'_> {
265265
fn clone_in(&self, allocator: &'new Allocator) -> Self::Cloned {
266266
Message {
267267
error: self.error.clone(),
268-
fix: self.fix.clone_in(allocator),
268+
fixes: self.fixes.clone_in(allocator),
269269
span: self.span,
270270
fixed: self.fixed,
271271
}
@@ -288,7 +288,9 @@ impl<'a> Message<'a> {
288288
} else {
289289
(0, 0)
290290
};
291-
Self { error, span: Span::new(start, end), fix, fixed: false }
291+
// ToDo support multiple fixes
292+
let fixes = fix.map_or(PossibleFixes::None, PossibleFixes::Single);
293+
Self { error, span: Span::new(start, end), fixes, fixed: false }
292294
}
293295
}
294296

@@ -321,15 +323,15 @@ impl<'a> Fixer<'a> {
321323
/// # Panics
322324
pub fn fix(mut self) -> FixResult<'a> {
323325
let source_text = self.source_text;
324-
if self.messages.iter().all(|m| m.fix.is_none()) {
326+
if self.messages.iter().all(|m| m.fixes.is_empty()) {
325327
return FixResult {
326328
fixed: false,
327329
fixed_code: Cow::Borrowed(source_text),
328330
messages: self.messages,
329331
};
330332
}
331333

332-
self.messages.sort_unstable_by_key(|m| m.fix.as_ref().unwrap_or(&Fix::default()).span);
334+
self.messages.sort_unstable_by_key(|m| m.fixes.span());
333335
let mut fixed = false;
334336
let mut output = String::with_capacity(source_text.len());
335337
let mut last_pos: i64 = -1;
@@ -338,7 +340,9 @@ impl<'a> Fixer<'a> {
338340
let mut filtered_messages = Vec::with_capacity(self.messages.len());
339341

340342
for mut m in self.messages {
341-
let Some(Fix { content, span, .. }) = m.fix.as_ref() else {
343+
// ToDo: handle `PossibleFixes::Multiple` case
344+
// check how ESLint will detect the preferred fix, important for autofix
345+
let PossibleFixes::Single(Fix { content, span, .. }) = &m.fixes else {
342346
filtered_messages.push(m);
343347
continue;
344348
};

‎crates/oxc_linter/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ use crate::{
5151
};
5252

5353
#[cfg(feature = "language_server")]
54-
pub use crate::fixer::{FixWithPosition, MessageWithPosition};
54+
pub use crate::fixer::{FixWithPosition, MessageWithPosition, PossibleFixesWithPosition};
5555

5656
#[cfg(target_pointer_width = "64")]
5757
#[test]

0 commit comments

Comments
 (0)