From 2f762955cd0abce4b4ff894caea9c4f066e3b74c Mon Sep 17 00:00:00 2001 From: Michael Sloan Date: Mon, 13 Jan 2025 12:26:28 -0700 Subject: [PATCH] Take a reference in LSP notify (#23077) In current code this doesn't have benefit. In preparation for avoiding a clone of workspace configuration. Having the interface this way may make opportunities for efficiency clearer in the future Release Notes: - N/A --- crates/collab/src/tests/editor_tests.rs | 4 +-- crates/collab/src/tests/integration_tests.rs | 14 +++++------ crates/copilot/src/copilot.rs | 12 ++++----- crates/editor/src/inlay_hint_cache.rs | 4 +-- .../src/test/editor_lsp_test_context.rs | 2 +- crates/language_tools/src/lsp_log.rs | 2 +- crates/language_tools/src/lsp_log_tests.rs | 2 +- crates/lsp/src/lsp.rs | 22 ++++++++-------- crates/project/src/lsp_store.rs | 25 ++++++++++--------- crates/project/src/project_tests.rs | 16 ++++++------ 10 files changed, 52 insertions(+), 51 deletions(-) diff --git a/crates/collab/src/tests/editor_tests.rs b/crates/collab/src/tests/editor_tests.rs index 773ee97a9d..350b1d41be 100644 --- a/crates/collab/src/tests/editor_tests.rs +++ b/crates/collab/src/tests/editor_tests.rs @@ -1007,7 +1007,7 @@ async fn test_language_server_statuses(cx_a: &mut TestAppContext, cx_b: &mut Tes fake_language_server.start_progress("the-token").await; executor.advance_clock(SERVER_PROGRESS_THROTTLE_TIMEOUT); - fake_language_server.notify::(lsp::ProgressParams { + fake_language_server.notify::(&lsp::ProgressParams { token: lsp::NumberOrString::String("the-token".to_string()), value: lsp::ProgressParamsValue::WorkDone(lsp::WorkDoneProgress::Report( lsp::WorkDoneProgressReport { @@ -1041,7 +1041,7 @@ async fn test_language_server_statuses(cx_a: &mut TestAppContext, cx_b: &mut Tes }); executor.advance_clock(SERVER_PROGRESS_THROTTLE_TIMEOUT); - fake_language_server.notify::(lsp::ProgressParams { + fake_language_server.notify::(&lsp::ProgressParams { token: lsp::NumberOrString::String("the-token".to_string()), value: lsp::ProgressParamsValue::WorkDone(lsp::WorkDoneProgress::Report( lsp::WorkDoneProgressReport { diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index ce5b2d5ad6..30c4cedacb 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -3900,7 +3900,7 @@ async fn test_collaborating_with_diagnostics( .receive_notification::() .await; fake_language_server.notify::( - lsp::PublishDiagnosticsParams { + &lsp::PublishDiagnosticsParams { uri: lsp::Url::from_file_path("/a/a.rs").unwrap(), version: None, diagnostics: vec![lsp::Diagnostic { @@ -3920,7 +3920,7 @@ async fn test_collaborating_with_diagnostics( .await .unwrap(); fake_language_server.notify::( - lsp::PublishDiagnosticsParams { + &lsp::PublishDiagnosticsParams { uri: lsp::Url::from_file_path("/a/a.rs").unwrap(), version: None, diagnostics: vec![lsp::Diagnostic { @@ -3994,7 +3994,7 @@ async fn test_collaborating_with_diagnostics( // Simulate a language server reporting more errors for a file. fake_language_server.notify::( - lsp::PublishDiagnosticsParams { + &lsp::PublishDiagnosticsParams { uri: lsp::Url::from_file_path("/a/a.rs").unwrap(), version: None, diagnostics: vec![ @@ -4088,7 +4088,7 @@ async fn test_collaborating_with_diagnostics( // Simulate a language server reporting no errors for a file. fake_language_server.notify::( - lsp::PublishDiagnosticsParams { + &lsp::PublishDiagnosticsParams { uri: lsp::Url::from_file_path("/a/a.rs").unwrap(), version: None, diagnostics: vec![], @@ -4183,7 +4183,7 @@ async fn test_collaborating_with_lsp_progress_updates_and_diagnostics_ordering( }) .await .unwrap(); - fake_language_server.notify::(lsp::ProgressParams { + fake_language_server.notify::(&lsp::ProgressParams { token: lsp::NumberOrString::String("the-disk-based-token".to_string()), value: lsp::ProgressParamsValue::WorkDone(lsp::WorkDoneProgress::Begin( lsp::WorkDoneProgressBegin { @@ -4194,7 +4194,7 @@ async fn test_collaborating_with_lsp_progress_updates_and_diagnostics_ordering( }); for file_name in file_names { fake_language_server.notify::( - lsp::PublishDiagnosticsParams { + &lsp::PublishDiagnosticsParams { uri: lsp::Url::from_file_path(Path::new("/test").join(file_name)).unwrap(), version: None, diagnostics: vec![lsp::Diagnostic { @@ -4207,7 +4207,7 @@ async fn test_collaborating_with_lsp_progress_updates_and_diagnostics_ordering( }, ); } - fake_language_server.notify::(lsp::ProgressParams { + fake_language_server.notify::(&lsp::ProgressParams { token: lsp::NumberOrString::String("the-disk-based-token".to_string()), value: lsp::ProgressParamsValue::WorkDone(lsp::WorkDoneProgress::End( lsp::WorkDoneProgressEnd { message: None }, diff --git a/crates/copilot/src/copilot.rs b/crates/copilot/src/copilot.rs index bc424d2d5a..72e4251488 100644 --- a/crates/copilot/src/copilot.rs +++ b/crates/copilot/src/copilot.rs @@ -270,7 +270,7 @@ impl RegisteredBuffer { server .lsp .notify::( - lsp::DidChangeTextDocumentParams { + &lsp::DidChangeTextDocumentParams { text_document: lsp::VersionedTextDocumentIdentifier::new( buffer.uri.clone(), buffer.snapshot_version, @@ -659,7 +659,7 @@ impl Copilot { let snapshot = buffer.read(cx).snapshot(); server .notify::( - lsp::DidOpenTextDocumentParams { + &lsp::DidOpenTextDocumentParams { text_document: lsp::TextDocumentItem { uri: uri.clone(), language_id: language_id.clone(), @@ -707,7 +707,7 @@ impl Copilot { server .lsp .notify::( - lsp::DidSaveTextDocumentParams { + &lsp::DidSaveTextDocumentParams { text_document: lsp::TextDocumentIdentifier::new( registered_buffer.uri.clone(), ), @@ -727,14 +727,14 @@ impl Copilot { server .lsp .notify::( - lsp::DidCloseTextDocumentParams { + &lsp::DidCloseTextDocumentParams { text_document: lsp::TextDocumentIdentifier::new(old_uri), }, )?; server .lsp .notify::( - lsp::DidOpenTextDocumentParams { + &lsp::DidOpenTextDocumentParams { text_document: lsp::TextDocumentItem::new( registered_buffer.uri.clone(), registered_buffer.language_id.clone(), @@ -759,7 +759,7 @@ impl Copilot { server .lsp .notify::( - lsp::DidCloseTextDocumentParams { + &lsp::DidCloseTextDocumentParams { text_document: lsp::TextDocumentIdentifier::new(buffer.uri), }, ) diff --git a/crates/editor/src/inlay_hint_cache.rs b/crates/editor/src/inlay_hint_cache.rs index 739fb98226..327ebbb5e2 100644 --- a/crates/editor/src/inlay_hint_cache.rs +++ b/crates/editor/src/inlay_hint_cache.rs @@ -1479,7 +1479,7 @@ pub mod tests { .await .expect("work done progress create request failed"); cx.executor().run_until_parked(); - fake_server.notify::(lsp::ProgressParams { + fake_server.notify::(&lsp::ProgressParams { token: lsp::ProgressToken::String(progress_token.to_string()), value: lsp::ProgressParamsValue::WorkDone(lsp::WorkDoneProgress::Begin( lsp::WorkDoneProgressBegin::default(), @@ -1504,7 +1504,7 @@ pub mod tests { }) .unwrap(); - fake_server.notify::(lsp::ProgressParams { + fake_server.notify::(&lsp::ProgressParams { token: lsp::ProgressToken::String(progress_token.to_string()), value: lsp::ProgressParamsValue::WorkDone(lsp::WorkDoneProgress::End( lsp::WorkDoneProgressEnd::default(), diff --git a/crates/editor/src/test/editor_lsp_test_context.rs b/crates/editor/src/test/editor_lsp_test_context.rs index 3831ca963f..23e37a1267 100644 --- a/crates/editor/src/test/editor_lsp_test_context.rs +++ b/crates/editor/src/test/editor_lsp_test_context.rs @@ -331,7 +331,7 @@ impl EditorLspTestContext { } pub fn notify(&self, params: T::Params) { - self.lsp.notify::(params); + self.lsp.notify::(¶ms); } #[cfg(target_os = "windows")] diff --git a/crates/language_tools/src/lsp_log.rs b/crates/language_tools/src/lsp_log.rs index 01ef202227..e55a6e2b8e 100644 --- a/crates/language_tools/src/lsp_log.rs +++ b/crates/language_tools/src/lsp_log.rs @@ -963,7 +963,7 @@ impl LspLogView { }); server - .notify::(SetTraceParams { value: level }) + .notify::(&SetTraceParams { value: level }) .ok(); } } diff --git a/crates/language_tools/src/lsp_log_tests.rs b/crates/language_tools/src/lsp_log_tests.rs index ad3cc87f2d..cbcb900c3f 100644 --- a/crates/language_tools/src/lsp_log_tests.rs +++ b/crates/language_tools/src/lsp_log_tests.rs @@ -71,7 +71,7 @@ async fn test_lsp_logs(cx: &mut TestAppContext) { let log_view = window.root(cx).unwrap(); let mut cx = VisualTestContext::from_window(*window, cx); - language_server.notify::(lsp::LogMessageParams { + language_server.notify::(&lsp::LogMessageParams { message: "hello from the server".into(), typ: lsp::MessageType::INFO, }); diff --git a/crates/lsp/src/lsp.rs b/crates/lsp/src/lsp.rs index 3157948530..3999240d89 100644 --- a/crates/lsp/src/lsp.rs +++ b/crates/lsp/src/lsp.rs @@ -815,7 +815,7 @@ impl LanguageServer { } self.capabilities = RwLock::new(response.capabilities); - self.notify::(InitializedParams {})?; + self.notify::(&InitializedParams {})?; Ok(Arc::new(self)) }) } @@ -835,7 +835,7 @@ impl LanguageServer { &executor, (), ); - let exit = Self::notify_internal::(&outbound_tx, ()); + let exit = Self::notify_internal::(&outbound_tx, &()); outbound_tx.close(); let server = self.server.clone(); @@ -1146,7 +1146,7 @@ impl LanguageServer { if let Some(outbound_tx) = outbound_tx.upgrade() { Self::notify_internal::( &outbound_tx, - CancelParams { + &CancelParams { id: NumberOrString::Number(id), }, ) @@ -1174,13 +1174,13 @@ impl LanguageServer { /// Sends a RPC notification to the language server. /// /// [LSP Specification](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#notificationMessage) - pub fn notify(&self, params: T::Params) -> Result<()> { + pub fn notify(&self, params: &T::Params) -> Result<()> { Self::notify_internal::(&self.outbound_tx, params) } fn notify_internal( outbound_tx: &channel::Sender, - params: T::Params, + params: &T::Params, ) -> Result<()> { let message = serde_json::to_string(&Notification { jsonrpc: JSON_RPC_VERSION, @@ -1372,7 +1372,7 @@ impl LanguageServer { #[cfg(any(test, feature = "test-support"))] impl FakeLanguageServer { /// See [`LanguageServer::notify`]. - pub fn notify(&self, params: T::Params) { + pub fn notify(&self, params: &T::Params) { self.server.notify::(params).ok(); } @@ -1480,7 +1480,7 @@ impl FakeLanguageServer { }) .await .unwrap(); - self.notify::(ProgressParams { + self.notify::(&ProgressParams { token: NumberOrString::String(token), value: ProgressParamsValue::WorkDone(WorkDoneProgress::Begin(progress)), }); @@ -1488,7 +1488,7 @@ impl FakeLanguageServer { /// Simulate that the server has completed work and notifies about that with the specified token. pub fn end_progress(&self, token: impl Into) { - self.notify::(ProgressParams { + self.notify::(&ProgressParams { token: NumberOrString::String(token.into()), value: ProgressParamsValue::WorkDone(WorkDoneProgress::End(Default::default())), }); @@ -1540,7 +1540,7 @@ mod tests { let server = cx.update(|cx| server.initialize(None, cx)).await.unwrap(); server - .notify::(DidOpenTextDocumentParams { + .notify::(&DidOpenTextDocumentParams { text_document: TextDocumentItem::new( Url::from_str("file://a/b").unwrap(), "rust".to_string(), @@ -1558,11 +1558,11 @@ mod tests { "file://a/b" ); - fake.notify::(ShowMessageParams { + fake.notify::(&ShowMessageParams { typ: MessageType::ERROR, message: "ok".to_string(), }); - fake.notify::(PublishDiagnosticsParams { + fake.notify::(&PublishDiagnosticsParams { uri: Url::from_str("file://b/c").unwrap(), version: Some(5), diagnostics: vec![], diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index c4c01214a0..61294f0b0e 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -302,7 +302,7 @@ impl LocalLspStore { language_server .notify::( - lsp::DidChangeConfigurationParams { + &lsp::DidChangeConfigurationParams { settings: workspace_config, }, ) @@ -1922,7 +1922,7 @@ impl LocalLspStore { }; server - .notify::(lsp::DidOpenTextDocumentParams { + .notify::(&lsp::DidOpenTextDocumentParams { text_document: lsp::TextDocumentItem::new( uri.clone(), adapter.language_id(&language.name()), @@ -1968,7 +1968,7 @@ impl LocalLspStore { for (_, language_server) in self.language_servers_for_buffer(buffer, cx) { language_server .notify::( - lsp::DidCloseTextDocumentParams { + &lsp::DidCloseTextDocumentParams { text_document: lsp::TextDocumentIdentifier::new(file_url.clone()), }, ) @@ -5068,7 +5068,7 @@ impl LspStore { language_server .notify::( - lsp::DidChangeTextDocumentParams { + &lsp::DidChangeTextDocumentParams { text_document: lsp::VersionedTextDocumentIdentifier::new( uri.clone(), next_version, @@ -5104,7 +5104,7 @@ impl LspStore { }; server .notify::( - lsp::DidSaveTextDocumentParams { + &lsp::DidSaveTextDocumentParams { text_document: text_document.clone(), text, }, @@ -5174,7 +5174,7 @@ impl LspStore { server .notify::( - lsp::DidChangeConfigurationParams { settings }, + &lsp::DidChangeConfigurationParams { settings }, ) .ok(); } @@ -6215,7 +6215,7 @@ impl LspStore { if filter.should_send_did_rename(&old_uri, is_dir) { language_server - .notify::(RenameFilesParams { + .notify::(&RenameFilesParams { files: vec![FileRename { old_uri: old_uri.clone(), new_uri: new_uri.clone(), @@ -6322,7 +6322,7 @@ impl LspStore { if !changes.is_empty() { server .notify::( - lsp::DidChangeWatchedFilesParams { changes }, + &lsp::DidChangeWatchedFilesParams { changes }, ) .log_err(); } @@ -7534,7 +7534,7 @@ impl LspStore { let uri = lsp::Url::from_file_path(file.abs_path(cx)).unwrap(); language_server .notify::( - lsp::DidOpenTextDocumentParams { + &lsp::DidOpenTextDocumentParams { text_document: lsp::TextDocumentItem::new( uri, adapter.language_id(&language.name()), @@ -7636,10 +7636,11 @@ impl LspStore { continue; } } + if progress.is_cancellable { server .notify::( - WorkDoneProgressCancelParams { + &WorkDoneProgressCancelParams { token: lsp::NumberOrString::String(token.clone()), }, ) @@ -7649,7 +7650,7 @@ impl LspStore { if progress.is_cancellable { server .notify::( - WorkDoneProgressCancelParams { + &WorkDoneProgressCancelParams { token: lsp::NumberOrString::String(token.clone()), }, ) @@ -7784,7 +7785,7 @@ impl LspStore { }; if !params.changes.is_empty() { server - .notify::(params) + .notify::(¶ms) .log_err(); } } diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index 005cd34cba..b216d15b1e 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -1269,7 +1269,7 @@ async fn test_disk_based_diagnostics_progress(cx: &mut gpui::TestAppContext) { } ); - fake_server.notify::(lsp::PublishDiagnosticsParams { + fake_server.notify::(&lsp::PublishDiagnosticsParams { uri: Url::from_file_path("/dir/a.rs").unwrap(), version: None, diagnostics: vec![lsp::Diagnostic { @@ -1321,7 +1321,7 @@ async fn test_disk_based_diagnostics_progress(cx: &mut gpui::TestAppContext) { }); // Ensure publishing empty diagnostics twice only results in one update event. - fake_server.notify::(lsp::PublishDiagnosticsParams { + fake_server.notify::(&lsp::PublishDiagnosticsParams { uri: Url::from_file_path("/dir/a.rs").unwrap(), version: None, diagnostics: Default::default(), @@ -1334,7 +1334,7 @@ async fn test_disk_based_diagnostics_progress(cx: &mut gpui::TestAppContext) { } ); - fake_server.notify::(lsp::PublishDiagnosticsParams { + fake_server.notify::(&lsp::PublishDiagnosticsParams { uri: Url::from_file_path("/dir/a.rs").unwrap(), version: None, diagnostics: Default::default(), @@ -1453,7 +1453,7 @@ async fn test_restarting_server_with_diagnostics_published(cx: &mut gpui::TestAp // Publish diagnostics let fake_server = fake_servers.next().await.unwrap(); - fake_server.notify::(lsp::PublishDiagnosticsParams { + fake_server.notify::(&lsp::PublishDiagnosticsParams { uri: Url::from_file_path("/dir/a.rs").unwrap(), version: None, diagnostics: vec![lsp::Diagnostic { @@ -1534,7 +1534,7 @@ async fn test_restarted_server_reporting_invalid_buffer_version(cx: &mut gpui::T // Before restarting the server, report diagnostics with an unknown buffer version. let fake_server = fake_servers.next().await.unwrap(); - fake_server.notify::(lsp::PublishDiagnosticsParams { + fake_server.notify::(&lsp::PublishDiagnosticsParams { uri: lsp::Url::from_file_path("/dir/a.rs").unwrap(), version: Some(10000), diagnostics: Vec::new(), @@ -1784,7 +1784,7 @@ async fn test_transforming_diagnostics(cx: &mut gpui::TestAppContext) { assert!(change_notification_1.text_document.version > open_notification.text_document.version); // Report some diagnostics for the initial version of the buffer - fake_server.notify::(lsp::PublishDiagnosticsParams { + fake_server.notify::(&lsp::PublishDiagnosticsParams { uri: lsp::Url::from_file_path("/dir/a.rs").unwrap(), version: Some(open_notification.text_document.version), diagnostics: vec![ @@ -1870,7 +1870,7 @@ async fn test_transforming_diagnostics(cx: &mut gpui::TestAppContext) { }); // Ensure overlapping diagnostics are highlighted correctly. - fake_server.notify::(lsp::PublishDiagnosticsParams { + fake_server.notify::(&lsp::PublishDiagnosticsParams { uri: lsp::Url::from_file_path("/dir/a.rs").unwrap(), version: Some(open_notification.text_document.version), diagnostics: vec![ @@ -1962,7 +1962,7 @@ async fn test_transforming_diagnostics(cx: &mut gpui::TestAppContext) { ); // Handle out-of-order diagnostics - fake_server.notify::(lsp::PublishDiagnosticsParams { + fake_server.notify::(&lsp::PublishDiagnosticsParams { uri: lsp::Url::from_file_path("/dir/a.rs").unwrap(), version: Some(change_notification_2.text_document.version), diagnostics: vec![