Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
45 changes: 35 additions & 10 deletions crates/oxc_language_server/src/code_actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use tower_lsp_server::lsp_types::{
WorkspaceEdit,
};

use crate::linter::error_with_position::DiagnosticReport;
use crate::linter::error_with_position::{DiagnosticReport, FixedContent, PossibleFixContent};

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

pub fn apply_fix_code_action(report: &DiagnosticReport, uri: &Uri) -> Option<CodeAction> {
let Some(fixed_content) = &report.fixed_content else {
return None;
};

fn fix_content_to_code_action(
fixed_content: &FixedContent,
uri: &Uri,
alternative_message: &str,
) -> CodeAction {
// 1) Use `fixed_content.message` if it exists
// 2) Try to parse the report diagnostic message
// 3) Fallback to "Fix this problem"
let title = match fixed_content.message.clone() {
Some(msg) => msg,
None => {
if let Some(code) = report.diagnostic.message.split(':').next() {
if let Some(code) = alternative_message.split(':').next() {
format!("Fix this {code} problem")
} else {
"Fix this problem".to_string()
}
}
};

Some(CodeAction {
CodeAction {
title,
kind: Some(CodeActionKind::QUICKFIX),
is_preferred: Some(true),
Expand All @@ -55,7 +55,24 @@ pub fn apply_fix_code_action(report: &DiagnosticReport, uri: &Uri) -> Option<Cod
data: None,
diagnostics: None,
command: None,
})
}
}

pub fn apply_fix_code_actions(report: &DiagnosticReport, uri: &Uri) -> Option<Vec<CodeAction>> {
match &report.fixed_content {
PossibleFixContent::None => None,
PossibleFixContent::Single(fixed_content) => {
Some(vec![fix_content_to_code_action(fixed_content, uri, &report.diagnostic.message)])
}
PossibleFixContent::Multiple(fixed_contents) => Some(
fixed_contents
.iter()
.map(|fixed_content| {
fix_content_to_code_action(fixed_content, uri, &report.diagnostic.message)
})
.collect(),
),
}
}

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

for report in reports {
if let Some(fixed_content) = &report.fixed_content {
let fix = match &report.fixed_content {
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) => multi.first(),
};

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
Expand Down
45 changes: 29 additions & 16 deletions crates/oxc_language_server/src/linter/error_with_position.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::{borrow::Cow, str::FromStr};

use oxc_linter::MessageWithPosition;
use oxc_linter::{FixWithPosition, MessageWithPosition, PossibleFixesWithPosition};
use tower_lsp_server::lsp_types::{
self, CodeDescription, DiagnosticRelatedInformation, NumberOrString, Position, Range, Uri,
};
Expand All @@ -14,7 +14,7 @@ const LSP_MAX_INT: u32 = 2u32.pow(31) - 1;
#[derive(Debug, Clone)]
pub struct DiagnosticReport {
pub diagnostic: lsp_types::Diagnostic,
pub fixed_content: Option<FixedContent>,
pub fixed_content: PossibleFixContent,
}

#[derive(Debug, Clone)]
Expand All @@ -24,6 +24,13 @@ pub struct FixedContent {
pub range: Range,
}

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

fn cmp_range(first: &Range, other: &Range) -> std::cmp::Ordering {
match first.start.cmp(&other.start) {
std::cmp::Ordering::Equal => first.end.cmp(&other.end),
Expand Down Expand Up @@ -101,25 +108,31 @@ fn message_with_position_to_lsp_diagnostic(
}
}

fn fix_with_position_to_fix_content(fix: &FixWithPosition<'_>) -> FixedContent {
FixedContent {
message: fix.span.message().map(std::string::ToString::to_string),
code: fix.content.to_string(),
range: Range {
start: Position { line: fix.span.start().line, character: fix.span.start().character },
end: Position { line: fix.span.end().line, character: fix.span.end().character },
},
}
}

pub fn message_with_position_to_lsp_diagnostic_report(
message: &MessageWithPosition<'_>,
uri: &Uri,
) -> DiagnosticReport {
DiagnosticReport {
diagnostic: message_with_position_to_lsp_diagnostic(message, uri),
fixed_content: message.fix.as_ref().map(|infos| FixedContent {
message: infos.span.message().map(std::string::ToString::to_string),
code: infos.content.to_string(),
range: Range {
start: Position {
line: infos.span.start().line,
character: infos.span.start().character,
},
end: Position {
line: infos.span.end().line,
character: infos.span.end().character,
},
},
}),
fixed_content: match &message.fixes {
PossibleFixesWithPosition::None => PossibleFixContent::None,
PossibleFixesWithPosition::Single(fix) => {
PossibleFixContent::Single(fix_with_position_to_fix_content(fix))
}
PossibleFixesWithPosition::Multiple(fixes) => PossibleFixContent::Multiple(
fixes.iter().map(fix_with_position_to_fix_content).collect(),
),
},
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use oxc_linter::{
};

use super::error_with_position::{
DiagnosticReport, message_with_position_to_lsp_diagnostic_report,
DiagnosticReport, PossibleFixContent, message_with_position_to_lsp_diagnostic_report,
};

/// smaller subset of LintServiceOptions, which is used by IsolatedLintHandler
Expand Down Expand Up @@ -109,7 +109,7 @@ impl IsolatedLintHandler {
tags: None,
data: None,
},
fixed_content: None,
fixed_content: PossibleFixContent::None,
});
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ related_information[0].location.range: Range { start: Position { line: 1, charac
severity: Some(Warning)
source: Some("oxc")
tags: None
fixed: Some(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 1, character: 0 }, end: Position { line: 1, character: 8 } } })
fixed: Single(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 1, character: 0 }, end: Position { line: 1, character: 8 } } })


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


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


code: "eslint(no-debugger)"
Expand All @@ -51,4 +51,4 @@ related_information[0].location.range: Range { start: Position { line: 18, chara
severity: Some(Warning)
source: Some("oxc")
tags: None
fixed: Some(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 18, character: 2 }, end: Position { line: 18, character: 10 } } })
fixed: Single(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 18, character: 2 }, end: Position { line: 18, character: 10 } } })
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ related_information[0].location.range: Range { start: Position { line: 1, charac
severity: Some(Warning)
source: Some("oxc")
tags: None
fixed: Some(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 1, character: 0 }, end: Position { line: 1, character: 9 } } })
fixed: Single(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 1, character: 0 }, end: Position { line: 1, character: 9 } } })
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ related_information[1].location.range: Range { start: Position { line: 11, chara
severity: Some(Error)
source: Some("oxc")
tags: None
fixed: Some(FixedContent { message: Some("Delete this code."), code: "", range: Range { start: Position { line: 11, character: 21 }, end: Position { line: 11, character: 22 } } })
fixed: Single(FixedContent { message: Some("Delete this code."), code: "", range: Range { start: Position { line: 11, character: 21 }, end: Position { line: 11, character: 22 } } })


code: "None"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,4 @@ related_information[0].location.range: Range { start: Position { line: 0, charac
severity: Some(Error)
source: Some("oxc")
tags: None
fixed: Some(FixedContent { message: Some("Replace `\\/` with `/`."), code: "/", range: Range { start: Position { line: 0, character: 16 }, end: Position { line: 0, character: 18 } } })
fixed: Single(FixedContent { message: Some("Replace `\\/` with `/`."), code: "/", range: Range { start: Position { line: 0, character: 16 }, end: Position { line: 0, character: 18 } } })
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ related_information[0].location.range: Range { start: Position { line: 1, charac
severity: Some(Warning)
source: Some("oxc")
tags: None
fixed: Some(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 1, character: 1 }, end: Position { line: 1, character: 10 } } })
fixed: Single(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 1, character: 1 }, end: Position { line: 1, character: 10 } } })
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ related_information[0].location.range: Range { start: Position { line: 5, charac
severity: Some(Warning)
source: Some("oxc")
tags: None
fixed: Some(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 5, character: 4 }, end: Position { line: 5, character: 12 } } })
fixed: Single(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 5, character: 4 }, end: Position { line: 5, character: 12 } } })


code: "eslint(no-debugger)"
Expand All @@ -25,4 +25,4 @@ related_information[0].location.range: Range { start: Position { line: 10, chara
severity: Some(Warning)
source: Some("oxc")
tags: None
fixed: Some(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 10, character: 4 }, end: Position { line: 10, character: 13 } } })
fixed: Single(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 10, character: 4 }, end: Position { line: 10, character: 13 } } })
20 changes: 15 additions & 5 deletions crates/oxc_language_server/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ use tower_lsp_server::{
use crate::{
ConcurrentHashMap, Options, Run,
code_actions::{
apply_all_fix_code_action, apply_fix_code_action, ignore_this_line_code_action,
apply_all_fix_code_action, apply_fix_code_actions, ignore_this_line_code_action,
ignore_this_rule_code_action,
},
linter::{
error_with_position::DiagnosticReport,
error_with_position::{DiagnosticReport, PossibleFixContent},
server_linter::{ServerLinter, normalize_path},
},
};
Expand Down Expand Up @@ -228,8 +228,9 @@ impl WorkspaceWorker {
let mut code_actions_vec: Vec<CodeActionOrCommand> = vec![];

for report in reports {
if let Some(fix_action) = apply_fix_code_action(report, uri) {
code_actions_vec.push(CodeActionOrCommand::CodeAction(fix_action));
if let Some(fix_actions) = apply_fix_code_actions(report, uri) {
code_actions_vec
.extend(fix_actions.into_iter().map(CodeActionOrCommand::CodeAction));
}

code_actions_vec
Expand All @@ -242,6 +243,7 @@ impl WorkspaceWorker {
code_actions_vec
}

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

for report in value {
if let Some(fixed_content) = &report.fixed_content {
let fix = match &report.fixed_content {
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) => multi.first(),
};

if let Some(fixed_content) = &fix {
text_edits.push(TextEdit {
range: fixed_content.range,
new_text: fixed_content.code.clone(),
Expand Down
56 changes: 56 additions & 0 deletions crates/oxc_linter/src/fixer/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,62 @@ impl<'a> Fix<'a> {
}
}

#[derive(Clone)]
pub enum PossibleFixes<'a> {
None,
Single(Fix<'a>),
Multiple(Vec<Fix<'a>>),
}

impl<'new> CloneIn<'new> for PossibleFixes<'_> {
type Cloned = PossibleFixes<'new>;

fn clone_in(&self, allocator: &'new Allocator) -> Self::Cloned {
match self {
Self::None => PossibleFixes::None,
Self::Single(fix) => PossibleFixes::Single(fix.clone_in(allocator)),
Self::Multiple(fixes) => {
//ToDo: what about the vec?
PossibleFixes::Multiple(fixes.iter().map(|fix| fix.clone_in(allocator)).collect())
}
}
}
}

impl PossibleFixes<'_> {
/// Gets the number of [`Fix`]es contained in this [`PossibleFixes`].
pub fn len(&self) -> usize {
match self {
PossibleFixes::None => 0,
PossibleFixes::Single(_) => 1,
PossibleFixes::Multiple(fixes) => fixes.len(),
}
}

/// Returns `true` if this [`PossibleFixes`] contains no [`Fix`]es
pub fn is_empty(&self) -> bool {
self.len() == 0
}

pub fn span(&self) -> Span {
match self {
PossibleFixes::None => SPAN,
PossibleFixes::Single(fix) => fix.span,
PossibleFixes::Multiple(fixes) => {
fixes.iter().map(|fix| fix.span).reduce(Span::merge).unwrap_or(SPAN)
}
}
}
}

#[cfg(feature = "language_server")]
#[derive(Debug)]
pub enum PossibleFixesWithPosition<'a> {
None,
Single(FixWithPosition<'a>),
Multiple(Vec<FixWithPosition<'a>>),
}

// NOTE (@DonIsaac): having these variants is effectively the same as interning
// single or 0-element Vecs. I experimented with using smallvec here, but the
// resulting struct size was larger (40 bytes vs 32). So, we're sticking with
Expand Down
Loading
Loading