From 4ee80bcbbb38cbaed13457af007aa955b6f7ac08 Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki <40514306+aviatesk@users.noreply.github.com> Date: Wed, 9 Sep 2026 14:55:56 +0000 Subject: [PATCH] lsp: Handle dynamic registration of `textDocument/documentHighlight` (#63029) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zed did not advertise document highlight dynamic registration support or handle `textDocument/documentHighlight` registrations. Dynamically registered providers therefore fell through to the unhandled-capability path, while servers with a fallback had to advertise a static provider for every attached document. Advertise dynamic registration support and track document highlight registrations and unregistrations alongside the other selector-aware dynamic text document capabilities. Refresh editors whenever the effective document highlight registration state changes, including selector-only changes that leave the merged provider unchanged, so runtime registrations take effect immediately and unregistrations clear stale or in-flight highlights. Add coverage for the client capability, registration lifecycle with multiple selectors sharing identical provider options, and editor refresh behavior when an applicable selector is added or removed. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed’s UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - N/A --------- Co-authored-by: GPT-5.6 Sol Co-authored-by: Kirill Bulatov --- crates/collab/src/rpc.rs | 3 + .../collab/tests/integration/editor_tests.rs | 205 +++++++++++++++++- crates/editor/src/editor.rs | 32 +++ crates/editor/src/editor_tests.rs | 177 +++++++++++++++ crates/lsp/src/lsp.rs | 13 ++ crates/project/src/lsp_store.rs | 97 ++++++--- .../src/lsp_store/dynamic_registration.rs | 70 ++++-- crates/project/src/project.rs | 29 ++- .../tests/integration/dynamic_registration.rs | 190 ++++++++++++++++ crates/proto/proto/lsp.proto | 5 + crates/proto/proto/zed.proto | 3 +- crates/proto/src/proto.rs | 3 + crates/remote_server/src/headless_project.rs | 4 + 13 files changed, 787 insertions(+), 44 deletions(-) diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index b99cf949c1..6c41ceb061 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -437,6 +437,9 @@ impl Server { broadcast_project_message_from_host::, ) .add_message_handler(broadcast_project_message_from_host::) + .add_message_handler( + broadcast_project_message_from_host::, + ) .add_message_handler(broadcast_project_message_from_host::) .add_message_handler( broadcast_project_message_from_host::, diff --git a/crates/collab/tests/integration/editor_tests.rs b/crates/collab/tests/integration/editor_tests.rs index 60f5ae6e19..737800833a 100644 --- a/crates/collab/tests/integration/editor_tests.rs +++ b/crates/collab/tests/integration/editor_tests.rs @@ -45,7 +45,7 @@ use rpc::RECEIVE_TIMEOUT; use serde_json::json; use settings::{ DocumentFoldingRanges, DocumentSymbols, InlayHintSettingsContent, InlineBlameSettings, - SemanticTokens, SettingsStore, + SemanticTokens, Settings as _, SettingsStore, }; use std::{ collections::BTreeSet, @@ -1606,6 +1606,209 @@ async fn test_remote_dynamic_call_hierarchy_followups_use_prepared_server( assert_eq!(outgoing_request_count.load(atomic::Ordering::SeqCst), 1); } +#[gpui::test] +async fn test_remote_dynamic_document_highlight_registration_refreshes_guest_editor( + cx_a: &mut TestAppContext, + cx_b: &mut TestAppContext, +) { + let mut server = TestServer::start(cx_a.executor()).await; + let client_a = server.create_client(cx_a, "user_a").await; + let client_b = server.create_client(cx_b, "user_b").await; + server + .create_room(&mut [(&client_a, cx_a), (&client_b, cx_b)]) + .await; + let active_call_a = cx_a.read(ActiveCall::global); + cx_b.update(editor::init); + + let request_count = Arc::new(AtomicUsize::new(0)); + client_a.language_registry().add(rust_lang()); + let mut fake_servers = client_a.language_registry().register_fake_lsp( + "Rust", + FakeLspAdapter { + capabilities: lsp::ServerCapabilities::default(), + initializer: Some(Box::new({ + let request_count = request_count.clone(); + move |fake_server| { + let request_count = request_count.clone(); + fake_server + .set_request_handler::( + move |_, _| { + request_count.fetch_add(1, atomic::Ordering::SeqCst); + async move { + Ok(Some(vec![lsp::DocumentHighlight { + range: lsp::Range::new( + lsp::Position::new(0, 0), + lsp::Position::new(0, 2), + ), + kind: Some(lsp::DocumentHighlightKind::READ), + }])) + } + }, + ); + } + })), + ..FakeLspAdapter::default() + }, + ); + client_b.language_registry().add(rust_lang()); + client_b.language_registry().register_fake_lsp_adapter( + "Rust", + FakeLspAdapter { + capabilities: lsp::ServerCapabilities::default(), + ..FakeLspAdapter::default() + }, + ); + + client_a + .fs() + .insert_tree(path!("/dir"), json!({ "one.rs": "fn one() {}" })) + .await; + let (project_a, worktree_id) = client_a.build_local_project(path!("/dir"), cx_a).await; + let project_id = active_call_a + .update(cx_a, |call, cx| call.share_project(project_a.clone(), cx)) + .await + .unwrap(); + let project_b = client_b.join_remote_project(project_id, cx_b).await; + let (workspace_b, cx_b) = client_b.build_workspace(&project_b, cx_b); + let editor_b = workspace_b + .update_in(cx_b, |workspace, window, cx| { + workspace.open_path((worktree_id, rel_path("one.rs")), None, true, window, cx) + }) + .await + .unwrap() + .downcast::() + .unwrap(); + let fake_server = fake_servers.next().await.unwrap(); + let debounce = Duration::from_millis(cx_b.update(|_, cx| { + editor::EditorSettings::get_global(cx) + .lsp_highlight_debounce + .0 + })); + cx_a.executor().advance_clock(debounce); + cx_a.run_until_parked(); + cx_b.run_until_parked(); + assert_eq!( + request_count.load(atomic::Ordering::SeqCst), + 0, + "expected no document highlight request before the capability is registered", + ); + + fake_server + .request::( + lsp::RegistrationParams { + registrations: vec![lsp::Registration { + id: "rust-document-highlight".to_string(), + method: "textDocument/documentHighlight".to_string(), + register_options: Some(json!({ + "documentSelector": [{ "language": "rust", "scheme": "file" }], + })), + }], + }, + DEFAULT_LSP_REQUEST_TIMEOUT, + ) + .await + .into_response() + .unwrap(); + cx_a.executor().advance_clock(debounce); + cx_a.run_until_parked(); + cx_b.run_until_parked(); + assert_eq!( + request_count.load(atomic::Ordering::SeqCst), + 1, + "expected the guest editor to request document highlights after the host registration", + ); + assert!(editor_b.read_with(cx_b, |editor, _| { + editor.has_background_highlights(editor::HighlightKey::DocumentHighlightRead) + })); + + fake_server + .request::( + lsp::UnregistrationParams { + unregisterations: vec![lsp::Unregistration { + id: "rust-document-highlight".to_string(), + method: "textDocument/documentHighlight".to_string(), + }], + }, + DEFAULT_LSP_REQUEST_TIMEOUT, + ) + .await + .into_response() + .unwrap(); + cx_a.executor().advance_clock(debounce); + cx_a.run_until_parked(); + cx_b.run_until_parked(); + assert_eq!( + request_count.load(atomic::Ordering::SeqCst), + 1, + "expected no document highlight request after the host unregistration", + ); + assert!( + !editor_b.read_with(cx_b, |editor, _| { + editor.has_background_highlights(editor::HighlightKey::DocumentHighlightRead) + }), + "expected the guest editor to clear stale document highlights after the host unregistration", + ); + + fake_server + .request::( + lsp::RegistrationParams { + registrations: vec![lsp::Registration { + id: "rust-document-highlight".to_string(), + method: "textDocument/documentHighlight".to_string(), + register_options: Some(json!({ + "documentSelector": [{ "language": "rust", "scheme": "file" }], + })), + }], + }, + DEFAULT_LSP_REQUEST_TIMEOUT, + ) + .await + .into_response() + .unwrap(); + cx_a.executor().advance_clock(debounce); + cx_a.run_until_parked(); + cx_b.run_until_parked(); + assert_eq!(request_count.load(atomic::Ordering::SeqCst), 2); + assert!(editor_b.read_with(cx_b, |editor, _| { + editor.has_background_highlights(editor::HighlightKey::DocumentHighlightRead) + })); + + let server_id = fake_server.server.server_id(); + let buffer_a = project_a + .update(cx_a, |project, cx| { + project.open_local_buffer(path!("/dir/one.rs"), cx) + }) + .await + .unwrap(); + project_a.update(cx_a, |project, cx| { + project.stop_language_servers_for_buffers(vec![buffer_a], HashSet::default(), cx); + }); + cx_a.executor().advance_clock(debounce); + cx_a.run_until_parked(); + cx_b.run_until_parked(); + assert_eq!( + project_b.read_with(cx_b, |project, cx| { + project + .language_server_statuses(cx) + .map(|(server_id, _)| server_id) + .collect::>() + }), + Vec::::new(), + "expected the guest to drop the stopped server {server_id}", + ); + assert_eq!( + request_count.load(atomic::Ordering::SeqCst), + 2, + "expected no document highlight request after the host stopped the server", + ); + assert!( + !editor_b.read_with(cx_b, |editor, _| { + editor.has_background_highlights(editor::HighlightKey::DocumentHighlightRead) + }), + "expected the guest editor to clear stale document highlights after the host stopped the server", + ); +} + #[gpui::test] async fn test_slow_lsp_server(cx_a: &mut TestAppContext, cx_b: &mut TestAppContext) { let mut server = TestServer::start(cx_a.executor()).await; diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 855827d57a..f001d24bfa 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -2080,6 +2080,9 @@ impl Editor { project::Event::RefreshDocumentLinks { .. } => { editor.refresh_document_links(None, cx); } + project::Event::RefreshDocumentHighlights { server_id } => { + editor.refresh_document_highlights_for_server(*server_id, cx); + } project::Event::RefreshFoldingRanges { .. } => { editor.refresh_folding_ranges(None, window, cx); } @@ -2104,6 +2107,7 @@ impl Editor { editor.refresh_runnables(None, window, cx); editor.update_lsp_data(None, window, cx); editor.refresh_inlay_hints(InlayHintRefreshReason::ServerRemoved, cx); + editor.refresh_document_highlights(cx); } project::Event::SnippetEdit(id, snippet_edits) => { // todo(lw): Non singletons @@ -3805,6 +3809,33 @@ impl Editor { None } + fn refresh_document_highlights_for_server( + &mut self, + server_id: Option, + cx: &mut Context, + ) { + let server_relevant = server_id.is_none_or(|server_id| { + let Some(project) = self.project.as_ref() else { + return false; + }; + let cursor_position = self.selections.newest_anchor().head(); + self.buffer + .read(cx) + .text_anchor_for_position(cursor_position, cx) + .is_some_and(|(cursor_buffer, _)| { + project + .read(cx) + .lsp_store() + .read(cx) + .relevant_server_ids_for_capability_check(&cursor_buffer, cx) + .contains(&server_id) + }) + }); + if server_relevant { + self.refresh_document_highlights(cx); + } + } + fn prepare_highlight_query_from_selection( &mut self, snapshot: &DisplaySnapshot, @@ -10086,6 +10117,7 @@ impl Editor { self.invalidate_semantic_tokens(Some(*buffer_id)); self.update_lsp_data(Some(*buffer_id), window, cx); self.refresh_inlay_hints(InlayHintRefreshReason::ServerRemoved, cx); + self.refresh_document_highlights(cx); } jsx_tag_auto_close::refresh_enabled_in_any_buffer(self, multibuffer, cx); cx.emit(EditorEvent::Reparsed(*buffer_id)); diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index ba56029d12..5afae02e9d 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -11360,6 +11360,7 @@ async fn test_paste_shifts_block_by_first_line_delta(cx: &mut TestAppContext) { )); cx.set_state("package test\n\nˇ"); cx.update_editor(|editor, window, cx| editor.paste(&Paste, window, cx)); + cx.run_until_parked(); cx.assert_editor_state("package test\n\nfunc find() {\n\treturn 1\n}ˇ"); // The block moves by however far its first line moved, so a first line that @@ -11367,6 +11368,7 @@ async fn test_paste_shifts_block_by_first_line_delta(cx: &mut TestAppContext) { cx.write_to_clipboard(ClipboardItem::new_string(" foo()\n bar()".into())); cx.set_state("func test() {\nˇ\n}"); cx.update_editor(|editor, window, cx| editor.paste(&Paste, window, cx)); + cx.run_until_parked(); cx.assert_editor_state("func test() {\n foo()\nbar()ˇ\n}"); } @@ -35502,6 +35504,150 @@ async fn test_bookmarks_tab_retries_failed_path_when_file_appears(cx: &mut TestA ); } +#[gpui::test] +async fn test_dynamic_document_highlight_registration_refreshes_editor(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + let mut cx = EditorLspTestContext::new_rust(lsp::ServerCapabilities::default(), cx).await; + let debounce = Duration::from_millis( + cx.update(|_, cx| EditorSettings::get_global(cx).lsp_highlight_debounce.0), + ); + let project = cx.update_workspace(|workspace, _, _| workspace.project().clone()); + let server_id = cx.lsp.server.server_id(); + cx.set_state(indoc! {" + fn main() { + let foo = 1; + fˇoo; + } + "}); + cx.executor().advance_clock(debounce); + cx.run_until_parked(); + + let request_count = Arc::new(AtomicUsize::new(0)); + let _request_handler = + cx.set_request_handler::({ + let request_count = request_count.clone(); + move |_, _, _| { + request_count.fetch_add(1, atomic::Ordering::SeqCst); + async move { + Ok(Some(vec![lsp::DocumentHighlight { + range: lsp::Range::new(lsp::Position::new(2, 4), lsp::Position::new(2, 7)), + kind: Some(lsp::DocumentHighlightKind::READ), + }])) + } + } + }); + + register_document_highlight_capability(&cx.lsp, "python-document-highlight", "python").await; + cx.executor().advance_clock(debounce); + cx.run_until_parked(); + assert_eq!( + request_count.load(atomic::Ordering::SeqCst), + 0, + "expected a registration for another language to not query the server", + ); + assert_eq!(document_highlight_count(&mut cx), 0); + + register_document_highlight_capability(&cx.lsp, "rust-document-highlight", "rust").await; + cx.executor().advance_clock(debounce); + cx.run_until_parked(); + assert_eq!( + request_count.load(atomic::Ordering::SeqCst), + 1, + "expected an applicable registration to refresh document highlights", + ); + assert_eq!(document_highlight_count(&mut cx), 1); + + cx.update_editor(|editor, _, cx| { + editor.refresh_document_highlights(cx); + }); + for _ in 0..3 { + cx.executor().advance_clock(debounce / 4); + cx.run_until_parked(); + cx.update(|_, cx| { + project.update(cx, |_, cx| { + cx.emit(project::Event::RefreshDocumentHighlights { + server_id: Some(LanguageServerId(server_id.0 + 1)), + }); + }); + }); + cx.run_until_parked(); + } + cx.executor().advance_clock(debounce / 4); + cx.run_until_parked(); + assert_eq!( + request_count.load(atomic::Ordering::SeqCst), + 2, + "expected refreshes of servers unrelated to the buffer to not restart the debounce", + ); + + cx.update(|_, cx| { + project.update(cx, |_, cx| { + cx.emit(project::Event::RefreshDocumentHighlights { + server_id: Some(server_id), + }); + }); + }); + cx.executor().advance_clock(debounce); + cx.run_until_parked(); + assert_eq!( + request_count.load(atomic::Ordering::SeqCst), + 3, + "expected a refresh of the buffer's server to query document highlights", + ); + + cx.lsp + .request::( + lsp::UnregistrationParams { + unregisterations: vec![lsp::Unregistration { + id: "rust-document-highlight".to_string(), + method: "textDocument/documentHighlight".to_string(), + }], + }, + DEFAULT_LSP_REQUEST_TIMEOUT, + ) + .await + .into_response() + .unwrap(); + cx.executor().advance_clock(debounce); + cx.run_until_parked(); + assert_eq!( + request_count.load(atomic::Ordering::SeqCst), + 3, + "expected no server query after the applicable registration is removed", + ); + assert_eq!( + document_highlight_count(&mut cx), + 0, + "expected stale document highlights to be cleared after unregistration", + ); + + register_document_highlight_capability(&cx.lsp, "rust-document-highlight", "rust").await; + cx.executor().advance_clock(debounce); + cx.run_until_parked(); + assert_eq!(request_count.load(atomic::Ordering::SeqCst), 4); + assert_eq!(document_highlight_count(&mut cx), 1); + + let buffer = cx.update_editor(|editor, _, cx| editor.buffer().read(cx).as_singleton().unwrap()); + cx.update(|_, cx| { + project.update(cx, |project, cx| { + project.stop_language_servers_for_buffers(vec![buffer], HashSet::default(), cx); + }); + }); + cx.executor().advance_clock(debounce); + cx.run_until_parked(); + assert_eq!( + request_count.load(atomic::Ordering::SeqCst), + 4, + "expected no document highlight request after the server is stopped", + ); + assert_eq!( + document_highlight_count(&mut cx), + 0, + "expected stale document highlights to be cleared after the server is stopped", + ); +} + #[gpui::test] async fn test_rename_with_duplicate_edits(cx: &mut TestAppContext) { init_test(cx, |_| {}); @@ -46212,3 +46358,34 @@ async fn test_lsp_show_document_unsupported_uri(cx: &mut TestAppContext) { .expect("show document request should not error"); assert_eq!(response, lsp::ShowDocumentResult { success: false }); } + +async fn register_document_highlight_capability( + lsp: &lsp::FakeLanguageServer, + registration_id: &str, + language: &str, +) { + lsp.request::( + lsp::RegistrationParams { + registrations: vec![lsp::Registration { + id: registration_id.to_string(), + method: "textDocument/documentHighlight".to_string(), + register_options: Some(json!({ + "documentSelector": [{ "language": language, "scheme": "file" }], + })), + }], + }, + DEFAULT_LSP_REQUEST_TIMEOUT, + ) + .await + .into_response() + .unwrap(); +} + +fn document_highlight_count(cx: &mut EditorLspTestContext) -> usize { + cx.editor(|editor, _, _| { + editor + .background_highlights + .get(&HighlightKey::DocumentHighlightRead) + .map_or(0, |(_, ranges)| ranges.len()) + }) +} diff --git a/crates/lsp/src/lsp.rs b/crates/lsp/src/lsp.rs index ece0e2c3d6..6567486487 100644 --- a/crates/lsp/src/lsp.rs +++ b/crates/lsp/src/lsp.rs @@ -946,6 +946,9 @@ impl LanguageServer { link_support: Some(true), dynamic_registration: Some(true), }), + document_highlight: Some(DocumentHighlightClientCapabilities { + dynamic_registration: Some(true), + }), code_action: Some(CodeActionClientCapabilities { code_action_literal_support: Some(CodeActionLiteralSupport { code_action_kind: CodeActionKindLiteralSupport { @@ -2494,6 +2497,16 @@ mod tests { let params = cx.update(|cx| server.default_initialize_params(false, false, cx)); + assert_eq!( + params + .capabilities + .text_document + .as_ref() + .and_then(|capabilities| capabilities.document_highlight.as_ref()) + .and_then(|capabilities| capabilities.dynamic_registration), + Some(true) + ); + #[allow(deprecated)] let root_uri = params.root_uri.expect("root_uri should be set"); #[allow(deprecated)] diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 8b980757d5..726252f022 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -4642,6 +4642,9 @@ pub enum LspStoreEvent { RefreshDocumentLinks { server_id: Option, }, + RefreshDocumentHighlights { + server_id: Option, + }, RefreshFoldingRanges { server_id: Option, }, @@ -4757,6 +4760,7 @@ impl LspStore { client.add_entity_request_handler(Self::handle_refresh_code_lens); client.add_entity_request_handler(Self::handle_refresh_document_colors); client.add_entity_request_handler(Self::handle_refresh_document_links); + client.add_entity_request_handler(Self::handle_refresh_document_highlights); client.add_entity_request_handler(Self::handle_refresh_folding_ranges); client.add_entity_request_handler(Self::handle_refresh_document_symbols); client.add_entity_request_handler(Self::handle_on_type_formatting); @@ -5737,7 +5741,7 @@ impl LspStore { ) } - fn relevant_server_ids_for_capability_check( + pub fn relevant_server_ids_for_capability_check( &self, buffer: &Entity, cx: &App, @@ -5810,32 +5814,44 @@ impl LspStore { } fn notify_server_capabilities_updated(&self, server: &LanguageServer, cx: &mut Context) { - if let Some(capabilities) = self.serialize_synced_server_capabilities(server).log_err() { - cx.emit(LspStoreEvent::LanguageServerUpdate { - language_server_id: server.server_id(), - name: Some(server.name()), - message: proto::update_language_server::Variant::MetadataUpdated( - proto::ServerMetadataUpdated { - capabilities: Some(capabilities), - binary: Some(proto::LanguageServerBinaryInfo { - path: server.binary().path.to_string_lossy().into_owned(), - arguments: server - .binary() - .arguments - .iter() - .map(|arg| arg.to_string_lossy().into_owned()) - .collect(), - }), - configuration: serde_json::to_string(server.configuration()).ok(), - workspace_folders: server - .workspace_folders() - .iter() - .map(|uri| uri.to_string()) - .collect(), - }, - ), + let Some(capabilities) = self.serialize_synced_server_capabilities(server).log_err() else { + return; + }; + let message = + proto::update_language_server::Variant::MetadataUpdated(proto::ServerMetadataUpdated { + capabilities: Some(capabilities), + binary: Some(proto::LanguageServerBinaryInfo { + path: server.binary().path.to_string_lossy().into_owned(), + arguments: server + .binary() + .arguments + .iter() + .map(|arg| arg.to_string_lossy().into_owned()) + .collect(), + }), + configuration: serde_json::to_string(server.configuration()).ok(), + workspace_folders: server + .workspace_folders() + .iter() + .map(|uri| uri.to_string()) + .collect(), }); + if let Some((downstream_client, project_id)) = self.downstream_client.as_ref() { + downstream_client + .send(proto::UpdateLanguageServer { + project_id: *project_id, + server_name: Some(server.name().to_string()), + language_server_id: server.server_id().to_proto(), + variant: Some(message.clone()), + }) + .context("sending server metadata downstream") + .log_err(); } + cx.emit(LspStoreEvent::LanguageServerUpdate { + language_server_id: server.server_id(), + name: Some(server.name()), + message, + }); } pub(crate) fn insert_synced_server_capabilities( @@ -5870,6 +5886,37 @@ impl LspStore { } } + fn refresh_document_highlights( + &mut self, + for_server: Option, + cx: &mut Context, + ) { + cx.emit(LspStoreEvent::RefreshDocumentHighlights { + server_id: for_server, + }); + if let Some((downstream_client, project_id)) = self.downstream_client.as_ref() { + downstream_client + .send(proto::RefreshDocumentHighlights { + project_id: *project_id, + server_id: for_server.map(|server_id| server_id.to_proto()), + }) + .context("sending refresh document highlights downstream") + .log_err(); + } + } + + async fn handle_refresh_document_highlights( + lsp_store: Entity, + envelope: TypedEnvelope, + mut cx: AsyncApp, + ) -> Result { + lsp_store.update(&mut cx, |lsp_store, cx| { + let server_id = envelope.payload.server_id.map(LanguageServerId::from_proto); + lsp_store.refresh_document_highlights(server_id, cx); + }); + Ok(proto::Ack {}) + } + fn remote_document_selector_context( &self, server_id: LanguageServerId, diff --git a/crates/project/src/lsp_store/dynamic_registration.rs b/crates/project/src/lsp_store/dynamic_registration.rs index b251a35cbf..73a5a1cdf6 100644 --- a/crates/project/src/lsp_store/dynamic_registration.rs +++ b/crates/project/src/lsp_store/dynamic_registration.rs @@ -102,26 +102,32 @@ impl LspStore { .or_default(); // Guests track registrations by id, so notify on any id-visible change; caches only // consume the (selector, options) value set, so refresh flags compare sets. - let registration_changed = registrations.get(®istration_id) != Some(&new_registration); - let applicable_set_changed; - if registrations.contains_key(®istration_id) { + let previous_registration = registrations.get(®istration_id); + let registration_changed = previous_registration != Some(&new_registration); + let applicable_set_changed = match previous_registration { + Some(previous_registration) if previous_registration == &new_registration => false, + Some(previous_registration) => { + let mut other_registrations = registrations + .iter() + .filter(|(id, _)| **id != registration_id) + .map(|(_, registration)| registration); + let previous_value_remains = other_registrations + .clone() + .any(|registration| registration == previous_registration); + let new_value_exists = + other_registrations.any(|registration| registration == &new_registration); + !previous_value_remains || !new_value_exists + } + None => !registrations + .values() + .any(|registration| registration == &new_registration), + }; + if previous_registration.is_some() { log::warn!( "Received a duplicate {method} registration with ID {registration_id}, replacing the previous one" ); - let previous_values = registrations.values().cloned().collect::>(); - registrations.insert(registration_id, new_registration); - applicable_set_changed = !previous_values - .iter() - .all(|previous| registrations.values().any(|current| current == previous)) - || !registrations - .values() - .all(|current| previous_values.contains(current)); - } else { - applicable_set_changed = !registrations - .values() - .any(|existing| existing == &new_registration); - registrations.insert(registration_id, new_registration); } + registrations.insert(registration_id, new_registration); let new_active_envelope = registrations .last() .map(|(_, registration)| registration.server_capabilities.clone()); @@ -617,6 +623,25 @@ impl LspStore { |capabilities| &mut capabilities.definition_provider, )?; } + "textDocument/documentHighlight" => { + let document_selector = + parse_text_document_registration(reg.register_options.as_ref())?; + let options = parse_register_capabilities(reg.register_options)?; + if self + .register_dynamic_text_document_capability( + &server, + ®.method, + reg.id, + options, + document_selector, + cx, + |capabilities| &mut capabilities.document_highlight_provider, + )? + .applicability_may_have_changed() + { + self.refresh_document_highlights(Some(server_id), cx); + } + } "textDocument/completion" => { if let Some(registration_options) = reg .register_options @@ -1038,6 +1063,19 @@ impl LspStore { |capabilities| &mut capabilities.definition_provider, )?; } + "textDocument/documentHighlight" => { + if self + .unregister_dynamic_text_document_capability( + &server, + unreg, + cx, + |capabilities| &mut capabilities.document_highlight_provider, + )? + .is_some_and(CapabilityRegistrationChange::applicability_may_have_changed) + { + self.refresh_document_highlights(Some(server_id), cx); + } + } "textDocument/completion" => { if self .unregister_dynamic_text_document_capability( diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index a25101949a..2bdff41b16 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -417,6 +417,9 @@ pub enum Event { RefreshDocumentLinks { server_id: Option, }, + RefreshDocumentHighlights { + server_id: Option, + }, RefreshFoldingRanges { server_id: Option, }, @@ -3710,6 +3713,20 @@ impl Project { Event::SupplementaryLanguageServerAdded(*server_id, name.clone()), ), LspStoreEvent::LanguageServerRemoved(server_id) => { + if self.is_local() + && let Some(project_id) = self.remote_id() + { + self.collab_client + .send(proto::UpdateLanguageServer { + project_id, + server_name: None, + language_server_id: server_id.to_proto(), + variant: Some(proto::update_language_server::Variant::Removed( + proto::ServerRemoved {}, + )), + }) + .log_err(); + } cx.emit(Event::LanguageServerRemoved(*server_id)) } LspStoreEvent::SupplementaryLanguageServerRemoved(server_id) => { @@ -3748,6 +3765,11 @@ impl Project { server_id: *server_id, }) } + LspStoreEvent::RefreshDocumentHighlights { server_id } => { + cx.emit(Event::RefreshDocumentHighlights { + server_id: *server_id, + }) + } LspStoreEvent::RefreshFoldingRanges { server_id } => { cx.emit(Event::RefreshFoldingRanges { server_id: *server_id, @@ -3779,7 +3801,12 @@ impl Project { name, message, } => { - if self.is_local() { + if self.is_local() + && !matches!( + message, + proto::update_language_server::Variant::MetadataUpdated(_) + ) + { self.enqueue_buffer_ordered_message( BufferOrderedMessage::LanguageServerUpdate { language_server_id: *language_server_id, diff --git a/crates/project/tests/integration/dynamic_registration.rs b/crates/project/tests/integration/dynamic_registration.rs index 1c434780b8..ae5da9cb3a 100644 --- a/crates/project/tests/integration/dynamic_registration.rs +++ b/crates/project/tests/integration/dynamic_registration.rs @@ -652,6 +652,156 @@ async fn test_inlay_hint_resolve_state_uses_matching_dynamic_registration( ); } +#[gpui::test] +async fn test_dynamic_document_highlight_registration(cx: &mut gpui::TestAppContext) { + init_test(cx); + let (project, fake_server) = + setup_dynamic_registration_test(cx, lsp::ServerCapabilities::default()).await; + let server_id = fake_server.server.server_id(); + let method = "textDocument/documentHighlight"; + let (refresh_events, _refresh_events_subscription) = observe_refresh_events(&project, cx); + + assert_eq!( + server_capabilities(&project, server_id, cx).document_highlight_provider, + None, + ); + + let registration_options = |language: &str| { + json!({ + "documentSelector": [{ "language": language, "scheme": "file" }], + }) + }; + let registered_provider = Some(lsp::OneOf::Right(lsp::DocumentHighlightOptions { + work_done_progress_options: lsp::WorkDoneProgressOptions::default(), + })); + register_capability( + &fake_server, + method, + "python-document-highlight", + Some(registration_options("python")), + ) + .await; + cx.executor().run_until_parked(); + assert_eq!( + server_capabilities(&project, server_id, cx).document_highlight_provider, + registered_provider, + ); + assert_eq!( + refresh_events.lock().drain(..).collect::>(), + vec![format!("document_highlights({server_id})")], + "expected the first registration to refresh", + ); + + register_capability( + &fake_server, + method, + "rust-document-highlight", + Some(registration_options("rust")), + ) + .await; + cx.executor().run_until_parked(); + assert_eq!( + server_capabilities(&project, server_id, cx).document_highlight_provider, + registered_provider, + ); + assert_eq!( + refresh_events.lock().drain(..).collect::>(), + vec![format!("document_highlights({server_id})")], + "expected adding a distinct selector with identical provider options to refresh", + ); + + register_capability( + &fake_server, + method, + "rust-document-highlight-duplicate", + Some(registration_options("rust")), + ) + .await; + cx.executor().run_until_parked(); + assert_eq!( + refresh_events.lock().as_slice(), + &[] as &[String], + "expected a registration identical to an existing one to not refresh", + ); + + unregister_capabilities(&fake_server, method, &["rust-document-highlight-duplicate"]).await; + cx.executor().run_until_parked(); + assert_eq!( + refresh_events.lock().as_slice(), + &[] as &[String], + "expected removing a duplicate registration to not refresh", + ); + + unregister_capabilities(&fake_server, method, &["rust-document-highlight"]).await; + cx.executor().run_until_parked(); + assert_eq!( + server_capabilities(&project, server_id, cx).document_highlight_provider, + registered_provider, + ); + assert_eq!( + refresh_events.lock().drain(..).collect::>(), + vec![format!("document_highlights({server_id})")], + "expected removing a selector while another provider remains to refresh", + ); + + unregister_capabilities(&fake_server, method, &["python-document-highlight"]).await; + cx.executor().run_until_parked(); + assert_eq!( + server_capabilities(&project, server_id, cx).document_highlight_provider, + None, + ); + assert_eq!( + refresh_events.lock().drain(..).collect::>(), + vec![format!("document_highlights({server_id})")], + "expected the last unregistration to clear the capability and refresh", + ); +} + +#[gpui::test] +async fn test_dynamic_registration_sends_metadata_downstream_before_refresh( + cx: &mut gpui::TestAppContext, +) { + init_test(cx); + let (project, fake_server) = + setup_dynamic_registration_test(cx, lsp::ServerCapabilities::default()).await; + let downstream = Arc::new(RecordingProtoClient::default()); + project.update(cx, |project, cx| { + project.lsp_store().update(cx, |lsp_store, cx| { + lsp_store.shared(1, rpc::AnyProtoClient::new(downstream.clone()), cx); + }); + }); + cx.executor().run_until_parked(); + downstream.sent.lock().clear(); + + register_capability( + &fake_server, + "textDocument/documentHighlight", + "rust-document-highlight", + Some(json!({ + "documentSelector": [{ "language": "rust", "scheme": "file" }], + })), + ) + .await; + cx.executor().run_until_parked(); + assert_eq!( + downstream.sent.lock().drain(..).collect::>(), + vec!["UpdateLanguageServer", "RefreshDocumentHighlights"], + "expected the capability update to reach guests before the refresh that depends on it", + ); + + unregister_capabilities( + &fake_server, + "textDocument/documentHighlight", + &["rust-document-highlight"], + ) + .await; + cx.executor().run_until_parked(); + assert_eq!( + downstream.sent.lock().drain(..).collect::>(), + vec!["UpdateLanguageServer", "RefreshDocumentHighlights"], + ); +} + #[gpui::test] async fn test_multi_registration_inlay_hint(cx: &mut gpui::TestAppContext) { init_test(cx); @@ -2715,6 +2865,9 @@ fn observe_refresh_events( label("document_colors", server_id) } Event::RefreshDocumentLinks { server_id } => label("document_links", server_id), + Event::RefreshDocumentHighlights { server_id } => { + label("document_highlights", server_id) + } Event::RefreshFoldingRanges { server_id } => label("folding_ranges", server_id), Event::RefreshDocumentSymbols { server_id } => { label("document_symbols", server_id) @@ -3092,3 +3245,40 @@ async fn fetch_all_lsp_data( labels.sort(); labels } + +#[derive(Default)] +struct RecordingProtoClient { + sent: Mutex>, + handler_set: Mutex, +} + +impl rpc::ProtoClient for RecordingProtoClient { + fn request( + &self, + _: rpc::proto::Envelope, + _: &'static str, + ) -> futures::future::BoxFuture<'static, Result> { + unimplemented!() + } + + fn send(&self, _: rpc::proto::Envelope, message_type: &'static str) -> Result<()> { + self.sent.lock().push(message_type); + Ok(()) + } + + fn send_response(&self, _: rpc::proto::Envelope, _: &'static str) -> Result<()> { + Ok(()) + } + + fn message_handler_set(&self) -> &Mutex { + &self.handler_set + } + + fn is_via_collab(&self) -> bool { + false + } + + fn has_wsl_interop(&self) -> bool { + false + } +} diff --git a/crates/proto/proto/lsp.proto b/crates/proto/proto/lsp.proto index 3ebe3c65d0..12e2b9a003 100644 --- a/crates/proto/proto/lsp.proto +++ b/crates/proto/proto/lsp.proto @@ -673,6 +673,11 @@ message RefreshDocumentLinks { optional uint64 server_id = 2; } +message RefreshDocumentHighlights { + uint64 project_id = 1; + optional uint64 server_id = 2; +} + message RefreshFoldingRanges { uint64 project_id = 1; optional uint64 server_id = 2; diff --git a/crates/proto/proto/zed.proto b/crates/proto/proto/zed.proto index 9b91481458..d376daf9aa 100644 --- a/crates/proto/proto/zed.proto +++ b/crates/proto/proto/zed.proto @@ -521,7 +521,8 @@ message Envelope { LspExtExpandAbbreviationResponse lsp_ext_expand_abbreviation_response = 489; LanguageServerShowDocumentRequest language_server_show_document_request = 490; ExecuteLspCommand execute_lsp_command = 491; - ExecuteLspCommandResponse execute_lsp_command_response = 492; // current max + ExecuteLspCommandResponse execute_lsp_command_response = 492; + RefreshDocumentHighlights refresh_document_highlights = 493; // current max } reserved 87 to 88; diff --git a/crates/proto/src/proto.rs b/crates/proto/src/proto.rs index 30e9502fe0..b4d2a48a6f 100644 --- a/crates/proto/src/proto.rs +++ b/crates/proto/src/proto.rs @@ -238,6 +238,7 @@ messages!( (RefreshSemanticTokens, Background), (RefreshDocumentColors, Background), (RefreshDocumentLinks, Background), + (RefreshDocumentHighlights, Background), (RefreshFoldingRanges, Background), (RefreshDocumentSymbols, Background), (RegisterBufferWithLanguageServers, Background), @@ -521,6 +522,7 @@ request_messages!( (RefreshCodeLens, Ack), (RefreshDocumentColors, Ack), (RefreshDocumentLinks, Ack), + (RefreshDocumentHighlights, Ack), (RefreshFoldingRanges, Ack), (RefreshDocumentSymbols, Ack), (RejoinChannelBuffers, RejoinChannelBuffersResponse), @@ -777,6 +779,7 @@ entity_messages!( RefreshCodeLens, RefreshDocumentColors, RefreshDocumentLinks, + RefreshDocumentHighlights, RefreshFoldingRanges, RefreshDocumentSymbols, ReloadBuffers, diff --git a/crates/remote_server/src/headless_project.rs b/crates/remote_server/src/headless_project.rs index 6a172ca154..0a6905362e 100644 --- a/crates/remote_server/src/headless_project.rs +++ b/crates/remote_server/src/headless_project.rs @@ -456,6 +456,10 @@ impl HeadlessProject { }) .log_err(); } + LspStoreEvent::LanguageServerUpdate { + message: proto::update_language_server::Variant::MetadataUpdated(_), + .. + } => {} LspStoreEvent::LanguageServerUpdate { language_server_id, name,