Skip to content

perf(lsp): avoid some clones #16519

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
Merged

perf(lsp): avoid some clones #16519

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
50 changes: 25 additions & 25 deletions crates/oxc_language_server/src/backend.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,10 @@ impl LanguageServer for Backend {
let capabilities = Capabilities::from(params.capabilities);

// client sent workspace folders
let workers = if let Some(workspace_folders) = &params.workspace_folders {
let workers = if let Some(workspace_folders) = params.workspace_folders {
workspace_folders
.iter()
.map(|workspace_folder| WorkspaceWorker::new(workspace_folder.uri.clone()))
.into_iter()
.map(|workspace_folder| WorkspaceWorker::new(workspace_folder.uri))
.collect()
// client sent deprecated root uri
} else if let Some(root_uri) = params.root_uri {
Expand All @@ -130,8 +130,8 @@ impl LanguageServer for Backend {
// we will init the linter after requesting for the workspace configuration.
if !capabilities.workspace_configuration || options.is_some() {
for worker in &workers {
let option = &options
.clone()
let option = options
.as_deref()
.unwrap_or_default()
.iter()
.find(|workspace_option| {
Expand All @@ -140,7 +140,7 @@ impl LanguageServer for Backend {
.map(|workspace_options| workspace_options.options.clone())
.unwrap_or_default();

worker.start_worker(option.clone(), &self.tool_builders).await;
worker.start_worker(option, &self.tool_builders).await;
}
}

Expand Down Expand Up @@ -319,11 +319,11 @@ impl LanguageServer for Backend {

// Only create WorkspaceOption when the config is Some
configs
.iter()
.into_iter()
.enumerate()
.map(|(index, config)| WorkspaceOption {
.map(|(index, options)| WorkspaceOption {
workspace_uri: workers[index].get_root_uri().clone(),
options: config.clone(),
options,
})
.collect::<Vec<_>>()
} else {
Expand Down Expand Up @@ -456,8 +456,8 @@ impl LanguageServer for Backend {
)
.await;

for (index, folder) in params.event.added.iter().enumerate() {
let worker = WorkspaceWorker::new(folder.uri.clone());
for (index, folder) in params.event.added.into_iter().enumerate() {
let worker = WorkspaceWorker::new(folder.uri);
// get the configuration from the response and init the linter
let options = configurations.get(index).unwrap_or(&serde_json::Value::Null);
worker.start_worker(options.clone(), &self.tool_builders).await;
Expand Down Expand Up @@ -498,36 +498,36 @@ impl LanguageServer for Backend {
/// See: <https://microsoft.github.io/language-server-protocol/specifications/specification-current/#textDocument_didSave>
async fn did_save(&self, params: DidSaveTextDocumentParams) {
debug!("oxc server did save");
let uri = &params.text_document.uri;
let uri = params.text_document.uri;
let workers = self.workspace_workers.read().await;
let Some(worker) = workers.iter().find(|worker| worker.is_responsible_for_uri(uri)) else {
let Some(worker) = workers.iter().find(|worker| worker.is_responsible_for_uri(&uri)) else {
return;
};

if let Some(diagnostics) = worker.run_diagnostic_on_save(uri, params.text.as_deref()).await
if let Some(diagnostics) = worker.run_diagnostic_on_save(&uri, params.text.as_deref()).await
{
self.client.publish_diagnostics(uri.clone(), diagnostics, None).await;
self.client.publish_diagnostics(uri, diagnostics, None).await;
}
}
/// It will update the in-memory file content if the client supports dynamic formatting.
/// It will re-lint the file and send updated diagnostics, if necessary.
///
/// See: <https://microsoft.github.io/language-server-protocol/specifications/specification-current/#textDocument_didChange>
async fn did_change(&self, params: DidChangeTextDocumentParams) {
let uri = &params.text_document.uri;
let uri = params.text_document.uri;
let workers = self.workspace_workers.read().await;
let Some(worker) = workers.iter().find(|worker| worker.is_responsible_for_uri(uri)) else {
let Some(worker) = workers.iter().find(|worker| worker.is_responsible_for_uri(&uri)) else {
return;
};
let content = params.content_changes.first().map(|c| c.text.clone());

if let Some(content) = &content {
self.file_system.write().await.set(uri, content.clone());
self.file_system.write().await.set(&uri, content.clone());
}

if let Some(diagnostics) = worker.run_diagnostic_on_change(uri, content.as_deref()).await {
if let Some(diagnostics) = worker.run_diagnostic_on_change(&uri, content.as_deref()).await {
self.client
.publish_diagnostics(uri.clone(), diagnostics, Some(params.text_document.version))
.publish_diagnostics(uri, diagnostics, Some(params.text_document.version))
.await;
}
}
Expand All @@ -537,19 +537,19 @@ impl LanguageServer for Backend {
///
/// See: <https://microsoft.github.io/language-server-protocol/specifications/specification-current/#textDocument_didOpen>
async fn did_open(&self, params: DidOpenTextDocumentParams) {
let uri = &params.text_document.uri;
let uri = params.text_document.uri;
let workers = self.workspace_workers.read().await;
let Some(worker) = workers.iter().find(|worker| worker.is_responsible_for_uri(uri)) else {
let Some(worker) = workers.iter().find(|worker| worker.is_responsible_for_uri(&uri)) else {
return;
};

let content = params.text_document.text;

self.file_system.write().await.set(uri, content.clone());
self.file_system.write().await.set(&uri, content.clone());

if let Some(diagnostics) = worker.run_diagnostic(uri, Some(&content)).await {
if let Some(diagnostics) = worker.run_diagnostic(&uri, Some(&content)).await {
self.client
.publish_diagnostics(uri.clone(), diagnostics, Some(params.text_document.version))
.publish_diagnostics(uri, diagnostics, Some(params.text_document.version))
.await;
}
}
Expand Down
27 changes: 15 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 @@ -7,19 +7,19 @@ pub const CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC: CodeActionKind =
CodeActionKind::new("source.fixAll.oxc");

fn fix_content_to_code_action(
fixed_content: &FixedContent,
uri: &Uri,
fixed_content: FixedContent,
uri: Uri,
is_preferred: bool,
) -> CodeAction {
CodeAction {
title: fixed_content.message.clone(),
title: fixed_content.message,
kind: Some(CodeActionKind::QUICKFIX),
is_preferred: Some(is_preferred),
edit: Some(WorkspaceEdit {
#[expect(clippy::disallowed_types)]
changes: Some(std::collections::HashMap::from([(
uri.clone(),
vec![TextEdit { range: fixed_content.range, new_text: fixed_content.code.clone() }],
uri,
vec![TextEdit { range: fixed_content.range, new_text: fixed_content.code }],
)])),
..WorkspaceEdit::default()
}),
Expand All @@ -30,13 +30,13 @@ fn fix_content_to_code_action(
}
}

pub fn apply_fix_code_actions(action: &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 &action.fixed_content {
let action = fix_content_to_code_action(fixed, uri, preferred);
for fixed in action.fixed_content {
let action = fix_content_to_code_action(fixed, uri.clone(), preferred);
preferred = false;
code_actions.push(action);
}
Expand All @@ -46,7 +46,7 @@ pub fn apply_fix_code_actions(action: &LinterCodeAction, uri: &Uri) -> Vec<CodeA

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

Expand All @@ -60,7 +60,7 @@ pub fn apply_all_fix_code_action(
is_preferred: Some(true),
edit: Some(WorkspaceEdit {
#[expect(clippy::disallowed_types)]
changes: Some(std::collections::HashMap::from([(uri.clone(), quick_fixes)])),
changes: Some(std::collections::HashMap::from([(uri, quick_fixes)])),
..WorkspaceEdit::default()
}),
disabled: None,
Expand Down Expand Up @@ -98,8 +98,11 @@ pub fn fix_all_text_edit(actions: impl Iterator<Item = LinterCodeAction>) -> Vec
// and return them as one workspace edit.
// it is possible that one fix will change the range for the next fix
// see oxc-project/oxc#10422
text_edits
.push(TextEdit { range: fixed_content.range, new_text: fixed_content.code.clone() });
text_edits.push(TextEdit {
range: fixed_content.range,
// TODO: avoid cloning here
new_text: fixed_content.code.clone(),
});
}

text_edits
Expand Down
21 changes: 11 additions & 10 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 @@ -83,7 +83,7 @@ impl ServerLinterBuilder {

// TODO(refactor): pull this into a shared function, because in oxlint we have the same functionality.
let use_nested_config = options.use_nested_configs();
let fix_kind = FixKind::from(options.fix_kind.clone());
let fix_kind = FixKind::from(options.fix_kind);

let use_cross_module = config_builder.plugins().has_import()
|| (use_nested_config
Expand Down Expand Up @@ -124,7 +124,7 @@ impl ServerLinterBuilder {
&IsolatedLintHandlerOptions {
use_cross_module,
type_aware: options.type_aware,
fix_kind: FixKind::from(options.fix_kind.clone()),
fix_kind,
root_path: root_path.to_path_buf(),
tsconfig_path: options.ts_config_path.as_ref().map(|path| {
let path = Path::new(path).to_path_buf();
Expand Down Expand Up @@ -428,13 +428,13 @@ impl Tool for ServerLinter {
}

let args = FixAllCommandArgs::try_from(arguments).map_err(|_| ErrorCode::InvalidParams)?;
let uri = &Uri::from_str(&args.uri).map_err(|_| ErrorCode::InvalidParams)?;
let uri = Uri::from_str(&args.uri).map_err(|_| ErrorCode::InvalidParams)?;

if !self.is_responsible_for_uri(uri) {
if !self.is_responsible_for_uri(&uri) {
return Ok(None);
}

let actions = self.get_code_actions_for_uri(uri);
let actions = self.get_code_actions_for_uri(&uri);

let Some(actions) = actions else {
return Ok(None);
Expand All @@ -448,7 +448,7 @@ impl Tool for ServerLinter {

Ok(Some(WorkspaceEdit {
#[expect(clippy::disallowed_types)]
changes: Some(std::collections::HashMap::from([(uri.clone(), text_edits)])),
changes: Some(std::collections::HashMap::from([(uri, text_edits)])),
document_changes: None,
change_annotations: None,
}))
Expand Down Expand Up @@ -476,15 +476,16 @@ impl Tool for ServerLinter {
.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(actions, uri).map_or(vec![], |code_actions| {
vec![CodeActionOrCommand::CodeAction(code_actions)]
});
return apply_all_fix_code_action(actions, uri.clone())
.map_or(vec![], |code_actions| {
vec![CodeActionOrCommand::CodeAction(code_actions)]
});
}

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

for action in actions {
let fix_actions = apply_fix_code_actions(&action, uri);
let fix_actions = apply_fix_code_actions(action, uri);
code_actions_vec.extend(fix_actions.into_iter().map(CodeActionOrCommand::CodeAction));
}

Expand Down
21 changes: 12 additions & 9 deletions crates/oxc_language_server/src/worker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,19 +264,22 @@ impl WorkspaceWorker {
"
);

let result = self
.handle_tool_changes(file_system, |tool| {
tool.handle_configuration_change(
&self.root_uri,
&old_options,
changed_options_json.clone(),
)
})
.await;

{
let mut options_guard = self.options.lock().await;
*options_guard = Some(changed_options_json.clone());
*options_guard = Some(changed_options_json);
}

self.handle_tool_changes(file_system, |tool| {
tool.handle_configuration_change(
&self.root_uri,
&old_options,
changed_options_json.clone(),
)
})
.await
result
}

/// Common implementation for handling tool changes that may result in
Expand Down
Loading