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
5 changes: 5 additions & 0 deletions apps/oxlint/fixtures/lsp/dangerous_fix/.oxlintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"rules": {
"no-unused-vars": ["error", { "fix": { "variables": "fix" } }]
}
}
113 changes: 104 additions & 9 deletions apps/oxlint/src/lsp/code_actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ use crate::lsp::{
pub const CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC: CodeActionKind =
CodeActionKind::new("source.fixAll.oxc");

/// A custom code action kind for dangerous fix-all operations.
/// This is intentionally outside the `source.fixAll.*` hierarchy because the LSP spec
/// restricts `source.fixAll` to safe fixes only.
pub const CODE_ACTION_KIND_SOURCE_FIX_ALL_DANGEROUS_OXC: CodeActionKind =
CodeActionKind::new("source.fixAllDangerous.oxc");

fn fix_content_to_code_action(
fixed_content: FixedContent,
uri: Uri,
Expand Down Expand Up @@ -82,8 +88,33 @@ pub fn apply_all_fix_code_action(
})
}

/// 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 apply_dangerous_fix_code_action(
actions: impl Iterator<Item = LinterCodeAction>,
uri: Uri,
) -> Option<CodeAction> {
let quick_fixes: Vec<TextEdit> = dangerous_fix_all_text_edit(actions);

if quick_fixes.is_empty() {
return None;
}

Some(CodeAction {
title: "fix all fixable oxlint issues".to_string(),
kind: Some(CODE_ACTION_KIND_SOURCE_FIX_ALL_DANGEROUS_OXC),
is_preferred: Some(true),
edit: Some(WorkspaceEdit {
#[expect(clippy::disallowed_types)]
changes: Some(std::collections::HashMap::from([(uri, quick_fixes)])),
..WorkspaceEdit::default()
}),
disabled: None,
data: None,
diagnostics: None,
command: None,
})
}

/// Collect safe text edits from the provided diagnostic reports, which can be applied at once.
pub fn fix_all_text_edit(actions: impl Iterator<Item = LinterCodeAction>) -> Vec<TextEdit> {
let mut text_edits: Vec<TextEdit> = vec![];

Expand Down Expand Up @@ -129,16 +160,34 @@ pub fn fix_all_text_edit(actions: impl Iterator<Item = LinterCodeAction>) -> Vec
result
}

fn dangerous_fix_all_text_edit(actions: impl Iterator<Item = LinterCodeAction>) -> Vec<TextEdit> {
let mut text_edits: Vec<TextEdit> = vec![];

for action in actions {
let Some(fixed_content) = action.fixed_content.into_iter().find(|fixed| {
matches!(fixed.lsp_kind, FixedContentKind::LintRule | FixedContentKind::UnusedDirective)
}) else {
continue;
};

text_edits.push(TextEdit { range: fixed_content.range, new_text: fixed_content.code });
}

text_edits
}

#[cfg(test)]
mod tests {
use oxc_linter::FixKind;
use std::str::FromStr;

use tower_lsp_server::ls_types::{Position, Range};

use crate::lsp::error_with_position::{FixedContent, FixedContentKind, LinterCodeAction};
use oxc_linter::FixKind;

use super::fix_all_text_edit;
use super::*;
use crate::lsp::error_with_position::{FixedContent, FixedContentKind, LinterCodeAction};

fn make_action(
fn make_ranged_action(
start_line: u32,
start_char: u32,
end_line: u32,
Expand All @@ -161,7 +210,7 @@ mod tests {
#[test]
fn test_fix_all_text_edit_sorts_by_position() {
// Edits provided in reverse order should be sorted by start position.
let actions = vec![make_action(2, 0, 2, 5), make_action(0, 0, 0, 5)];
let actions = vec![make_ranged_action(2, 0, 2, 5), make_ranged_action(0, 0, 0, 5)];
let result = fix_all_text_edit(actions.into_iter());
assert_eq!(result.len(), 2);
assert_eq!(result[0].range.start.line, 0);
Expand All @@ -171,7 +220,7 @@ mod tests {
#[test]
fn test_fix_all_text_edit_drops_overlapping() {
// The second edit overlaps the first; only the first should be kept.
let actions = vec![make_action(0, 0, 0, 10), make_action(0, 5, 0, 15)];
let actions = vec![make_ranged_action(0, 0, 0, 10), make_ranged_action(0, 5, 0, 15)];
let result = fix_all_text_edit(actions.into_iter());
assert_eq!(result.len(), 1);
assert_eq!(result[0].range.end.character, 10);
Expand All @@ -183,8 +232,54 @@ mod tests {
// equals the start of the next) are conservatively treated as overlapping and the second
// is dropped. This is a safe trade-off: the fix can be applied on the next invocation,
// and it avoids any risk of producing unexpected output for boundary-sharing edits.
let actions = vec![make_action(0, 0, 0, 5), make_action(0, 5, 0, 10)];
let actions = vec![make_ranged_action(0, 0, 0, 5), make_ranged_action(0, 5, 0, 10)];
let result = fix_all_text_edit(actions.into_iter());
assert_eq!(result.len(), 1);
}

fn make_fix_kind_action(kind: FixKind) -> LinterCodeAction {
LinterCodeAction {
range: Range::default(),
fixed_content: vec![FixedContent {
message: "remove unused import".to_string(),
code: String::new(),
range: Range::new(Position::new(0, 0), Position::new(0, 10)),
kind,
lsp_kind: FixedContentKind::LintRule,
}],
}
}

#[test]
fn test_fix_all_text_edit_skips_dangerous_fix() {
let text_edits =
fix_all_text_edit(std::iter::once(make_fix_kind_action(FixKind::DangerousFix)));
assert!(text_edits.is_empty(), "dangerous fix should always be skipped in safe fix-all");
}

#[test]
fn test_fix_all_text_edit_includes_safe_fix() {
let text_edits = fix_all_text_edit(std::iter::once(make_fix_kind_action(FixKind::SafeFix)));
assert!(!text_edits.is_empty(), "safe fix should be included");
}

#[test]
fn test_apply_all_fix_code_action_uses_safe_kind() {
let action = apply_all_fix_code_action(
std::iter::once(make_fix_kind_action(FixKind::SafeFix)),
Uri::from_str("file:///test.js").unwrap(),
)
.unwrap();
assert_eq!(action.kind, Some(CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC));
}

#[test]
fn test_apply_dangerous_fix_code_action_uses_dangerous_kind() {
let action = apply_dangerous_fix_code_action(
std::iter::once(make_fix_kind_action(FixKind::DangerousFix)),
Uri::from_str("file:///test.js").unwrap(),
)
.unwrap();
assert_eq!(action.kind, Some(CODE_ACTION_KIND_SOURCE_FIX_ALL_DANGEROUS_OXC));
}
}
67 changes: 63 additions & 4 deletions apps/oxlint/src/lsp/server_linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ use crate::{
config_loader::{ConfigLoader, build_nested_configs, discover_configs_in_tree},
lsp::{
code_actions::{
CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC, apply_all_fix_code_action, apply_fix_code_actions,
CODE_ACTION_KIND_SOURCE_FIX_ALL_DANGEROUS_OXC, CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC,
apply_all_fix_code_action, apply_dangerous_fix_code_action, apply_fix_code_actions,
fix_all_text_edit,
},
commands::{FIX_ALL_COMMAND_ID, FixAllCommandArgs},
Expand Down Expand Up @@ -227,6 +228,7 @@ impl ServerLinterBuilder {
Self::create_ignore_glob(&root_path),
extended_paths,
runner,
fix_kind,
lint_options.report_unused_directive,
)
}
Expand All @@ -243,6 +245,7 @@ impl ToolBuilder for ServerLinterBuilder {
code_action_kinds: Some(vec![
CodeActionKind::QUICKFIX,
CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC,
CODE_ACTION_KIND_SOURCE_FIX_ALL_DANGEROUS_OXC,
CodeActionKind::SOURCE_FIX_ALL,
]),
work_done_progress_options: WorkDoneProgressOptions::default(),
Expand Down Expand Up @@ -376,6 +379,7 @@ pub struct ServerLinter {
extended_paths: FxHashSet<PathBuf>,
code_actions: Arc<ConcurrentHashMap<Uri, Option<Vec<LinterCodeAction>>>>,
runner: LintRunner,
fix_kind: FixKind,
unused_directives_severity: Option<AllowWarnDeny>,
}

Expand Down Expand Up @@ -603,6 +607,18 @@ impl Tool for ServerLinter {
continue;
};
code_actions_vec.push(CodeActionOrCommand::CodeAction(fix_all));
} else if kind == CODE_ACTION_KIND_SOURCE_FIX_ALL_DANGEROUS_OXC {
if !self.fix_kind.is_dangerous() {
warn!(
"Linter is not configured to provide dangerous fixes. Please set `fixKind` to `dangerous_fix` or `dangerous_fix_or_suggestion` in the server configuration to enable it."
);
continue;
}
let Some(fix_all) = apply_dangerous_fix_code_action(actions.clone(), uri.clone())
else {
continue;
};
code_actions_vec.push(CodeActionOrCommand::CodeAction(fix_all));
} else if kind == CodeActionKind::QUICKFIX {
for action in actions.clone() {
let fix_actions = apply_fix_code_actions(action, uri);
Expand Down Expand Up @@ -656,6 +672,7 @@ impl ServerLinter {
gitignore_glob: Vec<Gitignore>,
extended_paths: FxHashSet<PathBuf>,
runner: LintRunner,
fix_kind: FixKind,
unused_directives_severity: Option<AllowWarnDeny>,
) -> Self {
Self {
Expand All @@ -666,6 +683,7 @@ impl ServerLinter {
extended_paths,
code_actions: Arc::new(ConcurrentHashMap::default()),
runner,
fix_kind,
unused_directives_severity,
}
}
Expand Down Expand Up @@ -837,7 +855,10 @@ mod tests_builder {
use oxc_language_server::{Capabilities, DiagnosticMode, ToolBuilder};

use crate::lsp::{
code_actions::CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC, commands::FIX_ALL_COMMAND_ID,
code_actions::{
CODE_ACTION_KIND_SOURCE_FIX_ALL_DANGEROUS_OXC, CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC,
},
commands::FIX_ALL_COMMAND_ID,
server_linter::ServerLinterBuilder,
};

Expand All @@ -854,8 +875,9 @@ mod tests_builder {
let code_action_kinds = options.code_action_kinds.as_ref().unwrap();
assert!(code_action_kinds.contains(&CodeActionKind::QUICKFIX));
assert!(code_action_kinds.contains(&CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC));
assert!(code_action_kinds.contains(&CODE_ACTION_KIND_SOURCE_FIX_ALL_DANGEROUS_OXC));
assert!(code_action_kinds.contains(&CodeActionKind::SOURCE_FIX_ALL));
assert_eq!(code_action_kinds.len(), 3);
assert_eq!(code_action_kinds.len(), 4);
}
_ => panic!("Expected code action provider options"),
}
Expand Down Expand Up @@ -1074,7 +1096,9 @@ mod test {
};

use crate::lsp::{
code_actions::CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC,
code_actions::{
CODE_ACTION_KIND_SOURCE_FIX_ALL_DANGEROUS_OXC, CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC,
},
server_linter::ServerLinterBuilder,
tester::{Tester, get_file_path},
};
Expand Down Expand Up @@ -1103,6 +1127,41 @@ mod test {
assert!(configs_dirs[0].ends_with("deep1"));
}

#[test]
fn test_fix_all_dangerous_returns_dangerous_fix_action() {
let tester =
Tester::new("fixtures/lsp/dangerous_fix", json!({ "fixKind": "dangerous_fix" }));
let linter = tester.create_linter();
let range = Range::new(Position::new(0, 0), Position::new(u32::MAX, u32::MAX));
let uri = tester.get_file_uri("unused_var.js");
let _ = linter.run_file(&uri, Some("let a = 1;")).unwrap();

// source.fixAll should only return safe fixes, not dangerous ones
let safe_actions = linter.get_code_actions_or_commands(
&uri,
&range,
&CodeActionContext {
only: Some(vec![CodeActionKind::SOURCE_FIX_ALL]),
..Default::default()
},
);
assert!(safe_actions.is_empty(), "source.fixAll should not apply dangerous fixes");

// source.fixAllDangerous.oxc should return dangerous fix actions when fix_kind is dangerous
let dangerous_actions = linter.get_code_actions_or_commands(
&uri,
&range,
&CodeActionContext {
only: Some(vec![CODE_ACTION_KIND_SOURCE_FIX_ALL_DANGEROUS_OXC]),
..Default::default()
},
);
assert!(
!dangerous_actions.is_empty(),
"source.fixAllDangerous.oxc should return dangerous fix action when fix_kind is dangerous"
);
}

#[test]
fn test_code_action_context_only() {
// this directory doesn't matter because we are directly calling the linter methods, and not relying on the file system for this test.
Expand Down
Loading