Skip to content

perf(oxlint/lsp): store only LinterCodeAction instead of the complete DiagnosticReport #16516

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
graphite-app merged 1 commit into from
Dec 5, 2025
+63 −70
Merged

perf(oxlint/lsp): store only LinterCodeAction instead of the complete DiagnosticReport #16516

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 12 additions & 12 deletions crates/oxc_language_server/src/linter/code_actions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ fn fix_content_to_code_action(
}
}

pub fn apply_fix_code_actions(report: &LinterCodeAction, uri: &Uri) -> Vec<CodeAction> {
pub fn apply_fix_code_actions(action: &LinterCodeAction, uri: &Uri) -> Vec<CodeAction> {
let mut code_actions = vec![];

// only the first code action is preferred
let mut preferred = true;
for fixed in &report.fixed_content {
for fixed in &action.fixed_content {
let action = fix_content_to_code_action(fixed, uri, preferred);
preferred = false;
code_actions.push(action);
Expand All @@ -44,11 +44,11 @@ pub fn apply_fix_code_actions(report: &LinterCodeAction, uri: &Uri) -> Vec<CodeA
code_actions
}

pub fn apply_all_fix_code_action<'a>(
reports: impl Iterator<Item = &'a LinterCodeAction>,
pub fn apply_all_fix_code_action(
actions: impl Iterator<Item = LinterCodeAction>,
uri: &Uri,
) -> Option<CodeAction> {
let quick_fixes: Vec<TextEdit> = fix_all_text_edit(reports);
let quick_fixes: Vec<TextEdit> = fix_all_text_edit(actions);

if quick_fixes.is_empty() {
return None;
Expand All @@ -72,28 +72,28 @@ pub fn apply_all_fix_code_action<'a>(

/// 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 fix_all_text_edit<'a>(reports: impl Iterator<Item = &'a LinterCodeAction>) -> Vec<TextEdit> {
pub fn fix_all_text_edit(actions: impl Iterator<Item = LinterCodeAction>) -> Vec<TextEdit> {
let mut text_edits: Vec<TextEdit> = vec![];

for report in reports {
if report.fixed_content.is_empty() {
for action in actions {
if action.fixed_content.is_empty() {
continue;
}

// for a real linter fix, we expect at least 3 fixes
if report.fixed_content.len() == 2 {
if action.fixed_content.len() == 2 {
debug!("Multiple fixes found, but only ignore fixes available");
#[cfg(debug_assertions)]
{
debug_assert!(report.fixed_content[0].message.starts_with("Disable"));
debug_assert!(report.fixed_content[0].message.ends_with("for this line"));
debug_assert!(action.fixed_content[0].message.starts_with("Disable"));
debug_assert!(action.fixed_content[0].message.ends_with("for this line"));
}
continue;
}

// For multiple fixes, we take the first one as a representative fix.
// Applying all possible fixes at once is not possible in this context.
let fixed_content = report.fixed_content.first().unwrap();
let fixed_content = action.fixed_content.first().unwrap();
// 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
12 changes: 3 additions & 9 deletions crates/oxc_language_server/src/linter/error_with_position.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub struct DiagnosticReport {

#[derive(Debug, Clone, Default)]
pub struct LinterCodeAction {
// pub range: Range,
pub range: Range,
pub fixed_content: Vec<FixedContent>,
}

Expand Down Expand Up @@ -139,10 +139,7 @@ pub fn message_to_lsp_diagnostic(
if error_offset == section_offset && message.span.end == section_offset {
return DiagnosticReport {
diagnostic,
code_action: Some(LinterCodeAction {
// range,
fixed_content,
}),
code_action: Some(LinterCodeAction { range, fixed_content }),
};
}

Expand All @@ -158,10 +155,7 @@ pub fn message_to_lsp_diagnostic(
let code_action = if fixed_content.is_empty() {
None
} else {
Some(LinterCodeAction {
// range,
fixed_content,
})
Some(LinterCodeAction { range, fixed_content })
};

DiagnosticReport { diagnostic, code_action }
Expand Down
97 changes: 48 additions & 49 deletions crates/oxc_language_server/src/linter/server_linter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use oxc_linter::{
LintIgnoreMatcher, LintOptions, Oxlintrc,
};

use crate::linter::error_with_position::LinterCodeAction;
use crate::{
ConcurrentHashMap,
linter::{
Expand All @@ -30,7 +31,6 @@ use crate::{
},
commands::{FIX_ALL_COMMAND_ID, FixAllCommandArgs},
config_walker::ConfigWalker,
error_with_position::DiagnosticReport,
isolated_lint_handler::{IsolatedLintHandler, IsolatedLintHandlerOptions},
options::{LintOptions as LSPLintOptions, Run, UnusedDisableDirectives},
},
Expand Down Expand Up @@ -302,7 +302,7 @@ pub struct ServerLinter {
ignore_matcher: LintIgnoreMatcher,
gitignore_glob: Vec<Gitignore>,
extended_paths: FxHashSet<PathBuf>,
diagnostics: Arc<ConcurrentHashMap<String, Option<Vec<DiagnosticReport>>>>,
code_actions: Arc<ConcurrentHashMap<Uri, Option<Vec<LinterCodeAction>>>>,
}

impl Tool for ServerLinter {
Expand All @@ -311,9 +311,7 @@ impl Tool for ServerLinter {
}

fn shutdown(&self) -> ToolShutdownChanges {
ToolShutdownChanges {
uris_to_clear_diagnostics: Some(self.get_cached_files_of_diagnostics()),
}
ToolShutdownChanges { uris_to_clear_diagnostics: Some(self.get_cached_uris()) }
}

/// # Panics
Expand Down Expand Up @@ -436,19 +434,17 @@ impl Tool for ServerLinter {
return Ok(None);
}

let value = if let Some(cached_diagnostics) = self.get_cached_diagnostics(uri) {
cached_diagnostics
} else {
let diagnostics = self.run_file(uri, None);
diagnostics.unwrap_or_default()
let actions = self.get_code_actions_for_uri(uri);

let Some(actions) = actions else {
return Ok(None);
};

if value.is_empty() {
if actions.is_empty() {
return Ok(None);
}

let text_edits =
fix_all_text_edit(value.iter().filter_map(|report| report.code_action.as_ref()));
let text_edits = fix_all_text_edit(actions.into_iter());

Ok(Some(WorkspaceEdit {
#[expect(clippy::disallowed_types)]
Expand All @@ -464,39 +460,31 @@ impl Tool for ServerLinter {
range: &Range,
only_code_action_kinds: Option<Vec<CodeActionKind>>,
) -> Vec<CodeActionOrCommand> {
let value = if let Some(cached_diagnostics) = self.get_cached_diagnostics(uri) {
cached_diagnostics
} else {
let diagnostics = self.run_file(uri, None);
diagnostics.unwrap_or_default()
let actions = self.get_code_actions_for_uri(uri);

let Some(actions) = actions else {
return vec![];
};

if value.is_empty() {
if actions.is_empty() {
return vec![];
}

let reports = value
.iter()
.filter(|r| r.diagnostic.range == *range || range_overlaps(*range, r.diagnostic.range));

let actions =
actions.into_iter().filter(|r| r.range == *range || range_overlaps(*range, r.range));
let is_source_fix_all_oxc = only_code_action_kinds
.is_some_and(|only| only.contains(&CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC));

if is_source_fix_all_oxc {
return apply_all_fix_code_action(
reports.filter_map(|report| report.code_action.as_ref()),
uri,
)
.map_or(vec![], |code_actions| vec![CodeActionOrCommand::CodeAction(code_actions)]);
return apply_all_fix_code_action(actions, uri).map_or(vec![], |code_actions| {
vec![CodeActionOrCommand::CodeAction(code_actions)]
});
}

let mut code_actions_vec: Vec<CodeActionOrCommand> = vec![];

for report in reports {
let Some(ref code_action) = report.code_action else {
continue;
};
let fix_actions = apply_fix_code_actions(code_action, uri);
for action in actions {
let fix_actions = apply_fix_code_actions(&action, uri);
code_actions_vec.extend(fix_actions.into_iter().map(CodeActionOrCommand::CodeAction));
}

Expand All @@ -508,7 +496,6 @@ impl Tool for ServerLinter {
/// - If the file is lintable, but no diagnostics are found, an empty vector is returned
fn run_diagnostic(&self, uri: &Uri, content: Option<&str>) -> Option<Vec<Diagnostic>> {
self.run_file(uri, content)
.map(|reports| reports.into_iter().map(|report| report.diagnostic).collect())
}

/// Lint a file with the current linter
Expand Down Expand Up @@ -538,7 +525,7 @@ impl Tool for ServerLinter {
}

fn remove_diagnostics(&self, uri: &Uri) {
self.diagnostics.pin().remove(&uri.to_string());
self.code_actions.pin().remove(uri);
}
}

Expand All @@ -560,21 +547,21 @@ impl ServerLinter {
ignore_matcher,
gitignore_glob,
extended_paths,
diagnostics: Arc::new(ConcurrentHashMap::default()),
code_actions: Arc::new(ConcurrentHashMap::default()),
}
}

fn get_cached_diagnostics(&self, uri: &Uri) -> Option<Vec<DiagnosticReport>> {
if let Some(diagnostics) = self.diagnostics.pin().get(&uri.to_string()) {
// when the uri is ignored, diagnostics is None.
// We want to return Some(vec![]), so the Worker knows there are no diagnostics for this file.
return Some(diagnostics.clone().unwrap_or_default());
}
None
fn get_cached_uris(&self) -> Vec<Uri> {
self.code_actions.pin().keys().cloned().collect()
}

fn get_cached_files_of_diagnostics(&self) -> Vec<Uri> {
self.diagnostics.pin().keys().filter_map(|s| Uri::from_str(s).ok()).collect()
fn get_code_actions_for_uri(&self, uri: &Uri) -> Option<Vec<LinterCodeAction>> {
if let Some(cached_code_actions) = self.code_actions.pin().get(uri) {
cached_code_actions.clone()
} else {
self.run_file(uri, None);
self.code_actions.pin().get(uri).and_then(std::clone::Clone::clone)
}
}

fn is_ignored(&self, uri: &Uri) -> bool {
Expand All @@ -600,16 +587,28 @@ impl ServerLinter {
}

/// Lint a single file, return `None` if the file is ignored.
fn run_file(&self, uri: &Uri, content: Option<&str>) -> Option<Vec<DiagnosticReport>> {
fn run_file(&self, uri: &Uri, content: Option<&str>) -> Option<Vec<Diagnostic>> {
if self.is_ignored(uri) {
return None;
}

let diagnostics = self.isolated_linter.run_single(uri, content);
let mut diagnostics = vec![];
let mut code_actions = vec![];

let reports = self.isolated_linter.run_single(uri, content);
if let Some(reports) = reports {
for report in reports {
diagnostics.push(report.diagnostic);

if let Some(code_action) = report.code_action {
code_actions.push(code_action);
}
}
}

self.diagnostics.pin().insert(uri.to_string(), diagnostics.clone());
self.code_actions.pin().insert(uri.clone(), Some(code_actions));

diagnostics
Some(diagnostics)
}

fn needs_restart(old_options: &LSPLintOptions, new_options: &LSPLintOptions) -> bool {
Expand Down
Loading