From c7f04691d994c1d9d29b47fefc6d689d6f4fe18f Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 29 Mar 2024 11:18:38 +0100 Subject: [PATCH] Query code actions and hovers from all related local language servers (#9943) Screenshot 2024-03-28 at 21 51 18 Part of https://github.com/zed-industries/zed/issues/7947 and https://github.com/zed-industries/zed/issues/9912 that adds makes Zed query all related language servers instead of the primary one. Collab clients are still querying the primary one only, but this is quite hard to solve, https://github.com/zed-industries/zed/pull/8634 drafts a part of it. The local part is useful per se, as many people use Zed & Tailwind but do not use collab features. Unfortunately, eslint still returns empty actions list when queried, but querying actions for all related language servers looks reasonable and rare enough to be dangerous. Release Notes: - Added Tailwind CSS hover popovers for Zed in single player mode ([7947](https://github.com/zed-industries/zed/issues/7947)) --- crates/collab/src/tests/integration_tests.rs | 3 +- .../random_project_collaboration_tests.rs | 2 +- crates/copilot/src/copilot.rs | 1 + crates/editor/src/editor.rs | 16 +- crates/editor/src/hover_popover.rs | 2 +- crates/language/src/language_registry.rs | 13 +- crates/lsp/src/lsp.rs | 55 +-- crates/project/src/lsp_command.rs | 11 + crates/project/src/project.rs | 125 ++++++- crates/project/src/project_tests.rs | 321 +++++++++++++++++- 10 files changed, 493 insertions(+), 56 deletions(-) diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 20541c761eabd1af8688d9959b0709523eb51b92..fe130e68e443fbc36ec2c9ce8f4370a9d9d5a4e0 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -4980,8 +4980,7 @@ async fn test_lsp_hover( let hovers = project_b .update(cx_b, |p, cx| p.hover(&buffer_b, 22, cx)) - .await - .unwrap(); + .await; assert_eq!( hovers.len(), 1, diff --git a/crates/collab/src/tests/random_project_collaboration_tests.rs b/crates/collab/src/tests/random_project_collaboration_tests.rs index 008aed8880646037bfc2fe620db100873b666176..1874b9ea6da43ca22f59c9b32a0e648e7314480b 100644 --- a/crates/collab/src/tests/random_project_collaboration_tests.rs +++ b/crates/collab/src/tests/random_project_collaboration_tests.rs @@ -832,7 +832,7 @@ impl RandomizedTest for ProjectCollaborationTest { .boxed(), LspRequestKind::CodeAction => project .code_actions(&buffer, offset..offset, cx) - .map_ok(|_| ()) + .map(|_| Ok(())) .boxed(), LspRequestKind::Definition => project .definition(&buffer, offset, cx) diff --git a/crates/copilot/src/copilot.rs b/crates/copilot/src/copilot.rs index 48b2d4102a96dd0cf3ee632414f1705d45089adb..7049af0e434132f2e2b6ae53b7c53cf75ff03724 100644 --- a/crates/copilot/src/copilot.rs +++ b/crates/copilot/src/copilot.rs @@ -376,6 +376,7 @@ impl Copilot { use node_runtime::FakeNodeRuntime; let (server, fake_server) = FakeLanguageServer::new( + LanguageServerId(0), LanguageServerBinary { path: "path/to/copilot".into(), arguments: vec![], diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 9b6d5d6bf733428be08f468bd497e14fb6ca5c24..4802c4e91b3880d932473c761b4660cfff45dbb0 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -3758,19 +3758,17 @@ impl Editor { let actions = if let Ok(code_actions) = project.update(&mut cx, |project, cx| { project.code_actions(&start_buffer, start..end, cx) }) { - code_actions.await.log_err() + code_actions.await } else { - None + Vec::new() }; this.update(&mut cx, |this, cx| { - this.available_code_actions = actions.and_then(|actions| { - if actions.is_empty() { - None - } else { - Some((start_buffer, actions.into())) - } - }); + this.available_code_actions = if actions.is_empty() { + None + } else { + Some((start_buffer, actions.into())) + }; cx.notify(); }) .log_err(); diff --git a/crates/editor/src/hover_popover.rs b/crates/editor/src/hover_popover.rs index de27475fe48f8aa5f0c0ac3bf4573b6ff270e06e..fb9011e02b34d1f577f2cb252cbdc1f9bf581965 100644 --- a/crates/editor/src/hover_popover.rs +++ b/crates/editor/src/hover_popover.rs @@ -295,7 +295,7 @@ fn show_hover( }); })?; - let hovers_response = hover_request.await.ok().unwrap_or_default(); + let hovers_response = hover_request.await; let language_registry = project.update(&mut cx, |p, _| p.languages().clone())?; let snapshot = this.update(&mut cx, |this, cx| this.snapshot(cx))?; let mut hover_highlights = Vec::with_capacity(hovers_response.len()); diff --git a/crates/language/src/language_registry.rs b/crates/language/src/language_registry.rs index ae2913c2511e8e64dd54c901d157a1b6933fc0fe..468a4e93d3cd05ba8b5d4fab58a5fca48bbec512 100644 --- a/crates/language/src/language_registry.rs +++ b/crates/language/src/language_registry.rs @@ -234,13 +234,23 @@ impl LanguageRegistry { &self, language_name: &str, adapter: crate::FakeLspAdapter, + ) -> futures::channel::mpsc::UnboundedReceiver { + self.register_specific_fake_lsp_adapter(language_name, true, adapter) + } + + #[cfg(any(feature = "test-support", test))] + pub fn register_specific_fake_lsp_adapter( + &self, + language_name: &str, + primary: bool, + adapter: crate::FakeLspAdapter, ) -> futures::channel::mpsc::UnboundedReceiver { self.state .write() .lsp_adapters .entry(language_name.into()) .or_default() - .push(CachedLspAdapter::new(Arc::new(adapter), true)); + .push(CachedLspAdapter::new(Arc::new(adapter), primary)); self.fake_language_servers(language_name) } @@ -739,6 +749,7 @@ impl LanguageRegistry { .unwrap_or_default(); let (server, mut fake_server) = lsp::FakeLanguageServer::new( + server_id, binary, adapter.name.0.to_string(), capabilities, diff --git a/crates/lsp/src/lsp.rs b/crates/lsp/src/lsp.rs index 7422ff5d9c44b6f8f4acd95a2f08dd2818ba82fa..2c433bb195e192d481cb4a0f25e84367d948b30d 100644 --- a/crates/lsp/src/lsp.rs +++ b/crates/lsp/src/lsp.rs @@ -1108,6 +1108,7 @@ pub struct FakeLanguageServer { impl FakeLanguageServer { /// Construct a fake language server. pub fn new( + server_id: LanguageServerId, binary: LanguageServerBinary, name: String, capabilities: ServerCapabilities, @@ -1117,8 +1118,8 @@ impl FakeLanguageServer { let (stdout_writer, stdout_reader) = async_pipe::pipe(); let (notifications_tx, notifications_rx) = channel::unbounded(); - let server = LanguageServer::new_internal( - LanguageServerId(0), + let mut server = LanguageServer::new_internal( + server_id, stdin_writer, stdout_reader, None::, @@ -1129,30 +1130,35 @@ impl FakeLanguageServer { cx.clone(), |_| {}, ); + server.name = name.as_str().into(); let fake = FakeLanguageServer { binary, - server: Arc::new(LanguageServer::new_internal( - LanguageServerId(0), - stdout_writer, - stdin_reader, - None::, - Arc::new(Mutex::new(None)), - None, - Path::new("/"), - None, - cx, - move |msg| { - notifications_tx - .try_send(( - msg.method.to_string(), - msg.params - .map(|raw_value| raw_value.get()) - .unwrap_or("null") - .to_string(), - )) - .ok(); - }, - )), + server: Arc::new({ + let mut server = LanguageServer::new_internal( + server_id, + stdout_writer, + stdin_reader, + None::, + Arc::new(Mutex::new(None)), + None, + Path::new("/"), + None, + cx, + move |msg| { + notifications_tx + .try_send(( + msg.method.to_string(), + msg.params + .map(|raw_value| raw_value.get()) + .unwrap_or("null") + .to_string(), + )) + .ok(); + }, + ); + server.name = name.as_str().into(); + server + }), notifications_rx, }; fake.handle_request::({ @@ -1350,6 +1356,7 @@ mod tests { release_channel::init("0.0.0", cx); }); let (server, mut fake) = FakeLanguageServer::new( + LanguageServerId(0), LanguageServerBinary { path: "path/to/language-server".into(), arguments: vec![], diff --git a/crates/project/src/lsp_command.rs b/crates/project/src/lsp_command.rs index 2a7d52185b9057412b30a1fed4b7b29de9a8ddb6..ea6fdd0e65d163f488e55046322ffd290bbb1695 100644 --- a/crates/project/src/lsp_command.rs +++ b/crates/project/src/lsp_command.rs @@ -1855,6 +1855,17 @@ impl GetCodeActions { }) .unwrap_or(false) } + + pub fn supports_code_actions(capabilities: &ServerCapabilities) -> bool { + capabilities + .code_action_provider + .as_ref() + .map(|options| match options { + lsp::CodeActionProviderCapability::Simple(is_supported) => *is_supported, + lsp::CodeActionProviderCapability::Options(_) => true, + }) + .unwrap_or(false) + } } #[async_trait(?Send)] diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 8f1cba6f0858ad6decfca49130aac68d05c2f964..dcbb72883b3fbbaaa8c494936b975cc1e1392542 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -5192,14 +5192,64 @@ impl Project { buffer: &Model, position: PointUtf16, cx: &mut ModelContext, - ) -> Task>> { - let request_task = self.request_lsp( - buffer.clone(), - LanguageServerToQuery::Primary, - GetHover { position }, - cx, - ); - cx.spawn(|_, _| async move { request_task.await.map(|hover| hover.into_iter().collect()) }) + ) -> Task> { + if self.is_local() { + let snapshot = buffer.read(cx).snapshot(); + let offset = position.to_offset(&snapshot); + let scope = snapshot.language_scope_at(offset); + + let mut hover_responses = self + .language_servers_for_buffer(buffer.read(cx), cx) + .filter(|(_, server)| match server.capabilities().hover_provider { + Some(lsp::HoverProviderCapability::Simple(enabled)) => enabled, + Some(lsp::HoverProviderCapability::Options(_)) => true, + None => false, + }) + .filter(|(adapter, _)| { + scope + .as_ref() + .map(|scope| scope.language_allowed(&adapter.name)) + .unwrap_or(true) + }) + .map(|(_, server)| server.server_id()) + .map(|server_id| { + self.request_lsp( + buffer.clone(), + LanguageServerToQuery::Other(server_id), + GetHover { position }, + cx, + ) + }) + .collect::>(); + + cx.spawn(|_, _| async move { + let mut hovers = Vec::with_capacity(hover_responses.len()); + while let Some(hover_response) = hover_responses.next().await { + if let Some(hover) = hover_response.log_err().flatten() { + hovers.push(hover); + } + } + hovers + }) + } else if self.is_remote() { + let request_task = self.request_lsp( + buffer.clone(), + LanguageServerToQuery::Primary, + GetHover { position }, + cx, + ); + cx.spawn(|_, _| async move { + request_task + .await + .log_err() + .flatten() + .map(|hover| vec![hover]) + .unwrap_or_default() + }) + } else { + log::error!("cannot show hovers: project does not have a remote id"); + Task::ready(Vec::new()) + } } pub fn hover( @@ -5207,7 +5257,7 @@ impl Project { buffer: &Model, position: T, cx: &mut ModelContext, - ) -> Task>> { + ) -> Task> { let position = position.to_point_utf16(buffer.read(cx)); self.hover_impl(buffer, position, cx) } @@ -5561,13 +5611,54 @@ impl Project { buffer_handle: &Model, range: Range, cx: &mut ModelContext, - ) -> Task>> { - self.request_lsp( - buffer_handle.clone(), - LanguageServerToQuery::Primary, - GetCodeActions { range, kinds: None }, - cx, - ) + ) -> Task> { + if self.is_local() { + let snapshot = buffer_handle.read(cx).snapshot(); + let offset = range.start.to_offset(&snapshot); + let scope = snapshot.language_scope_at(offset); + + let mut hover_responses = self + .language_servers_for_buffer(buffer_handle.read(cx), cx) + .filter(|(_, server)| GetCodeActions::supports_code_actions(server.capabilities())) + .filter(|(adapter, _)| { + scope + .as_ref() + .map(|scope| scope.language_allowed(&adapter.name)) + .unwrap_or(true) + }) + .map(|(_, server)| server.server_id()) + .map(|server_id| { + self.request_lsp( + buffer_handle.clone(), + LanguageServerToQuery::Other(server_id), + GetCodeActions { + range: range.clone(), + kinds: None, + }, + cx, + ) + }) + .collect::>(); + + cx.spawn(|_, _| async move { + let mut hovers = Vec::with_capacity(hover_responses.len()); + while let Some(hover_response) = hover_responses.next().await { + hovers.extend(hover_response.log_err().unwrap_or_default()); + } + hovers + }) + } else if self.is_remote() { + let request_task = self.request_lsp( + buffer_handle.clone(), + LanguageServerToQuery::Primary, + GetCodeActions { range, kinds: None }, + cx, + ); + cx.spawn(|_, _| async move { request_task.await.log_err().unwrap_or_default() }) + } else { + log::error!("cannot fetch actions: project does not have a remote id"); + Task::ready(Vec::new()) + } } pub fn code_actions( @@ -5575,7 +5666,7 @@ impl Project { buffer_handle: &Model, range: Range, cx: &mut ModelContext, - ) -> Task>> { + ) -> Task> { let buffer = buffer_handle.read(cx); let range = buffer.anchor_before(range.start)..buffer.anchor_before(range.end); self.code_actions_impl(buffer_handle, range, cx) diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index 5726770c6c762ae4b8f21688ff4d0177f444f649..aa96e1f9741bfea5c40df5abeb2ea7b1d7a3a2eb 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -2522,7 +2522,7 @@ async fn test_apply_code_actions_with_commands(cx: &mut gpui::TestAppContext) { .next() .await; - let action = actions.await.unwrap()[0].clone(); + let action = actions.await[0].clone(); let apply = project.update(cx, |project, cx| { project.apply_code_action(buffer.clone(), action, true, cx) }); @@ -4404,6 +4404,311 @@ async fn test_create_entry(cx: &mut gpui::TestAppContext) { assert!(result.is_err()) } +#[gpui::test] +async fn test_multiple_language_server_hovers(cx: &mut gpui::TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/dir", + json!({ + "a.tsx": "a", + }), + ) + .await; + + let project = Project::test(fs, ["/dir".as_ref()], cx).await; + + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + language_registry.add(tsx_lang()); + let language_server_names = [ + "TypeScriptServer", + "TailwindServer", + "ESLintServer", + "NoHoverCapabilitiesServer", + ]; + let mut fake_tsx_language_servers = language_registry.register_specific_fake_lsp_adapter( + "tsx", + true, + FakeLspAdapter { + name: &language_server_names[0], + capabilities: lsp::ServerCapabilities { + hover_provider: Some(lsp::HoverProviderCapability::Simple(true)), + ..lsp::ServerCapabilities::default() + }, + ..FakeLspAdapter::default() + }, + ); + let _a = language_registry.register_specific_fake_lsp_adapter( + "tsx", + false, + FakeLspAdapter { + name: &language_server_names[1], + capabilities: lsp::ServerCapabilities { + hover_provider: Some(lsp::HoverProviderCapability::Simple(true)), + ..lsp::ServerCapabilities::default() + }, + ..FakeLspAdapter::default() + }, + ); + let _b = language_registry.register_specific_fake_lsp_adapter( + "tsx", + false, + FakeLspAdapter { + name: &language_server_names[2], + capabilities: lsp::ServerCapabilities { + hover_provider: Some(lsp::HoverProviderCapability::Simple(true)), + ..lsp::ServerCapabilities::default() + }, + ..FakeLspAdapter::default() + }, + ); + let _c = language_registry.register_specific_fake_lsp_adapter( + "tsx", + false, + FakeLspAdapter { + name: &language_server_names[3], + capabilities: lsp::ServerCapabilities { + hover_provider: None, + ..lsp::ServerCapabilities::default() + }, + ..FakeLspAdapter::default() + }, + ); + + let buffer = project + .update(cx, |p, cx| p.open_local_buffer("/dir/a.tsx", cx)) + .await + .unwrap(); + cx.executor().run_until_parked(); + + let mut servers_with_hover_requests = HashMap::default(); + for i in 0..language_server_names.len() { + let new_server = fake_tsx_language_servers + .next() + .await + .unwrap_or_else(|| panic!("Failed to get language server #{i}")); + let new_server_name = new_server.server.name(); + assert!( + !servers_with_hover_requests.contains_key(new_server_name), + "Unexpected: initialized server with the same name twice. Name: `{new_server_name}`" + ); + let new_server_name = new_server_name.to_string(); + match new_server_name.as_str() { + "TailwindServer" | "TypeScriptServer" => { + servers_with_hover_requests.insert( + new_server_name.clone(), + new_server.handle_request::(move |_, _| { + let name = new_server_name.clone(); + async move { + Ok(Some(lsp::Hover { + contents: lsp::HoverContents::Scalar(lsp::MarkedString::String( + format!("{name} hover"), + )), + range: None, + })) + } + }), + ); + } + "ESLintServer" => { + servers_with_hover_requests.insert( + new_server_name, + new_server.handle_request::( + |_, _| async move { Ok(None) }, + ), + ); + } + "NoHoverCapabilitiesServer" => { + let _never_handled = new_server.handle_request::( + |_, _| async move { + panic!( + "Should not call for hovers server with no corresponding capabilities" + ) + }, + ); + } + unexpected => panic!("Unexpected server name: {unexpected}"), + } + } + + let hover_task = project.update(cx, |project, cx| { + project.hover(&buffer, Point::new(0, 0), cx) + }); + let _: Vec<()> = futures::future::join_all(servers_with_hover_requests.into_values().map( + |mut hover_request| async move { + hover_request + .next() + .await + .expect("All hover requests should have been triggered") + }, + )) + .await; + assert_eq!( + vec!["TailwindServer hover", "TypeScriptServer hover"], + hover_task + .await + .into_iter() + .map(|hover| hover.contents.iter().map(|block| &block.text).join("|")) + .sorted() + .collect::>(), + "Should receive hover responses from all related servers with hover capabilities" + ); +} + +#[gpui::test] +async fn test_multiple_language_server_actions(cx: &mut gpui::TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/dir", + json!({ + "a.tsx": "a", + }), + ) + .await; + + let project = Project::test(fs, ["/dir".as_ref()], cx).await; + + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + language_registry.add(tsx_lang()); + let language_server_names = [ + "TypeScriptServer", + "TailwindServer", + "ESLintServer", + "NoActionsCapabilitiesServer", + ]; + let mut fake_tsx_language_servers = language_registry.register_specific_fake_lsp_adapter( + "tsx", + true, + FakeLspAdapter { + name: &language_server_names[0], + capabilities: lsp::ServerCapabilities { + code_action_provider: Some(lsp::CodeActionProviderCapability::Simple(true)), + ..lsp::ServerCapabilities::default() + }, + ..FakeLspAdapter::default() + }, + ); + let _a = language_registry.register_specific_fake_lsp_adapter( + "tsx", + false, + FakeLspAdapter { + name: &language_server_names[1], + capabilities: lsp::ServerCapabilities { + code_action_provider: Some(lsp::CodeActionProviderCapability::Simple(true)), + ..lsp::ServerCapabilities::default() + }, + ..FakeLspAdapter::default() + }, + ); + let _b = language_registry.register_specific_fake_lsp_adapter( + "tsx", + false, + FakeLspAdapter { + name: &language_server_names[2], + capabilities: lsp::ServerCapabilities { + code_action_provider: Some(lsp::CodeActionProviderCapability::Simple(true)), + ..lsp::ServerCapabilities::default() + }, + ..FakeLspAdapter::default() + }, + ); + let _c = language_registry.register_specific_fake_lsp_adapter( + "tsx", + false, + FakeLspAdapter { + name: &language_server_names[3], + capabilities: lsp::ServerCapabilities { + code_action_provider: None, + ..lsp::ServerCapabilities::default() + }, + ..FakeLspAdapter::default() + }, + ); + + let buffer = project + .update(cx, |p, cx| p.open_local_buffer("/dir/a.tsx", cx)) + .await + .unwrap(); + cx.executor().run_until_parked(); + + let mut servers_with_actions_requests = HashMap::default(); + for i in 0..language_server_names.len() { + let new_server = fake_tsx_language_servers + .next() + .await + .unwrap_or_else(|| panic!("Failed to get language server #{i}")); + let new_server_name = new_server.server.name(); + assert!( + !servers_with_actions_requests.contains_key(new_server_name), + "Unexpected: initialized server with the same name twice. Name: `{new_server_name}`" + ); + let new_server_name = new_server_name.to_string(); + match new_server_name.as_str() { + "TailwindServer" | "TypeScriptServer" => { + servers_with_actions_requests.insert( + new_server_name.clone(), + new_server.handle_request::( + move |_, _| { + let name = new_server_name.clone(); + async move { + Ok(Some(vec![lsp::CodeActionOrCommand::CodeAction( + lsp::CodeAction { + title: format!("{name} code action"), + ..lsp::CodeAction::default() + }, + )])) + } + }, + ), + ); + } + "ESLintServer" => { + servers_with_actions_requests.insert( + new_server_name, + new_server.handle_request::( + |_, _| async move { Ok(None) }, + ), + ); + } + "NoActionsCapabilitiesServer" => { + let _never_handled = new_server + .handle_request::(|_, _| async move { + panic!( + "Should not call for code actions server with no corresponding capabilities" + ) + }); + } + unexpected => panic!("Unexpected server name: {unexpected}"), + } + } + + let code_actions_task = project.update(cx, |project, cx| { + project.code_actions(&buffer, 0..buffer.read(cx).len(), cx) + }); + let _: Vec<()> = futures::future::join_all(servers_with_actions_requests.into_values().map( + |mut code_actions_request| async move { + code_actions_request + .next() + .await + .expect("All code actions requests should have been triggered") + }, + )) + .await; + assert_eq!( + vec!["TailwindServer code action", "TypeScriptServer code action"], + code_actions_task + .await + .into_iter() + .map(|code_action| code_action.lsp_action.title) + .sorted() + .collect::>(), + "Should receive code actions responses from all related servers with hover capabilities" + ); +} + async fn search( project: &Model, query: SearchQuery, @@ -4508,3 +4813,17 @@ fn typescript_lang() -> Arc { Some(tree_sitter_typescript::language_typescript()), )) } + +fn tsx_lang() -> Arc { + Arc::new(Language::new( + LanguageConfig { + name: "tsx".into(), + matcher: LanguageMatcher { + path_suffixes: vec!["tsx".to_string()], + ..Default::default() + }, + ..Default::default() + }, + Some(tree_sitter_typescript::language_tsx()), + )) +}