perf(lsp): avoid some clones (#16519)

Sysix · Sysix · commit 67cfa58363d7 · 2025-12-05T14:05:00.000Z
&gt; This PR improves performance in the language server by systematically reducing unnecessary clone operations across multiple components. The changes primarily involve refactoring function signatures to take ownership where beneficial and restructuring code to avoid cloning data that can be moved instead.

These clones were easy to fix, looking for more bad tech deps
diff --git a/crates/oxc_language_server/src/backend.rs b/crates/oxc_language_server/src/backend.rs
@@ -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 {
@@ -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| {
@@ -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;
             }
         }
 
@@ -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 {
@@ -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;
@@ -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;
         }
     }
@@ -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;
         }
     }
diff --git a/crates/oxc_language_server/src/linter/code_actions.rs b/crates/oxc_language_server/src/linter/code_actions.rs
@@ -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()
         }),
@@ -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);
     }
@@ -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);
 
@@ -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,
@@ -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
diff --git a/crates/oxc_language_server/src/linter/server_linter.rs b/crates/oxc_language_server/src/linter/server_linter.rs
@@ -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
@@ -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();
@@ -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);
@@ -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,
         }))
@@ -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));
         }
 
diff --git a/crates/oxc_language_server/src/worker.rs b/crates/oxc_language_server/src/worker.rs
@@ -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
