diff --git a/crates/collab/src/tests/editor_tests.rs b/crates/collab/src/tests/editor_tests.rs index 621e7e100ad9c6da8352d9ecad4061a1ebf7ae51..be168c5f31f555257770090cf092267c9b367813 100644 --- a/crates/collab/src/tests/editor_tests.rs +++ b/crates/collab/src/tests/editor_tests.rs @@ -2703,7 +2703,7 @@ async fn test_lsp_pull_diagnostics( let (closure_workspace_diagnostic_received_tx, workspace_diagnostic_received_rx) = smol::channel::bounded::<()>(1); let expected_workspace_diagnostic_token = lsp::ProgressToken::String(format!( - "workspace/diagnostic-{}-1", + "workspace/diagnostic/{}/1", fake_language_server.server.server_id() )); let closure_expected_workspace_diagnostic_token = expected_workspace_diagnostic_token.clone(); diff --git a/crates/project/src/lsp_command.rs b/crates/project/src/lsp_command.rs index 5ec6e502bd85a25b6755c6994feff7a3062c919c..43ac965af0c467a74f3cb1d4f959e4b6e30acd58 100644 --- a/crates/project/src/lsp_command.rs +++ b/crates/project/src/lsp_command.rs @@ -26,8 +26,8 @@ use language::{ use lsp::{ AdapterServerCapabilities, CodeActionKind, CodeActionOptions, CodeDescription, CompletionContext, CompletionListItemDefaultsEditRange, CompletionTriggerKind, - DocumentHighlightKind, LanguageServer, LanguageServerId, LinkedEditingRangeServerCapabilities, - OneOf, RenameOptions, ServerCapabilities, + DiagnosticServerCapabilities, DocumentHighlightKind, LanguageServer, LanguageServerId, + LinkedEditingRangeServerCapabilities, OneOf, RenameOptions, ServerCapabilities, }; use serde_json::Value; use signature_help::{lsp_to_proto_signature, proto_to_lsp_signature}; @@ -262,6 +262,9 @@ pub(crate) struct LinkedEditingRange { #[derive(Clone, Debug)] pub(crate) struct GetDocumentDiagnostics { + /// We cannot blindly rely on server's capabilities.diagnostic_provider, as they're a singular field, whereas + /// a server can register multiple diagnostic providers post-mortem. + pub dynamic_caps: DiagnosticServerCapabilities, pub previous_result_id: Option, } @@ -4019,26 +4022,22 @@ impl LspCommand for GetDocumentDiagnostics { "Get diagnostics" } - fn check_capabilities(&self, server_capabilities: AdapterServerCapabilities) -> bool { - server_capabilities - .server_capabilities - .diagnostic_provider - .is_some() + fn check_capabilities(&self, _: AdapterServerCapabilities) -> bool { + true } fn to_lsp( &self, path: &Path, _: &Buffer, - language_server: &Arc, + _: &Arc, _: &App, ) -> Result { - let identifier = match language_server.capabilities().diagnostic_provider { - Some(lsp::DiagnosticServerCapabilities::Options(options)) => options.identifier, - Some(lsp::DiagnosticServerCapabilities::RegistrationOptions(options)) => { - options.diagnostic_options.identifier + let identifier = match &self.dynamic_caps { + lsp::DiagnosticServerCapabilities::Options(options) => options.identifier.clone(), + lsp::DiagnosticServerCapabilities::RegistrationOptions(options) => { + options.diagnostic_options.identifier.clone() } - None => None, }; Ok(lsp::DocumentDiagnosticParams { diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 1cc2ffb00b377e85e5f685bed67cac32a1fa810e..5f9449b08a2e4a0cc63e89a0b1ed41ad7209fcef 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -71,12 +71,12 @@ use language::{ range_from_lsp, range_to_lsp, }; use lsp::{ - AdapterServerCapabilities, CodeActionKind, CompletionContext, DiagnosticSeverity, - DiagnosticTag, DidChangeWatchedFilesRegistrationOptions, Edit, FileOperationFilter, - FileOperationPatternKind, FileOperationRegistrationOptions, FileRename, FileSystemWatcher, - LSP_REQUEST_TIMEOUT, LanguageServer, LanguageServerBinary, LanguageServerBinaryOptions, - LanguageServerId, LanguageServerName, LanguageServerSelector, LspRequestFuture, - MessageActionItem, MessageType, OneOf, RenameFilesParams, SymbolKind, + AdapterServerCapabilities, CodeActionKind, CompletionContext, DiagnosticServerCapabilities, + DiagnosticSeverity, DiagnosticTag, DidChangeWatchedFilesRegistrationOptions, Edit, + FileOperationFilter, FileOperationPatternKind, FileOperationRegistrationOptions, FileRename, + FileSystemWatcher, LSP_REQUEST_TIMEOUT, LanguageServer, LanguageServerBinary, + LanguageServerBinaryOptions, LanguageServerId, LanguageServerName, LanguageServerSelector, + LspRequestFuture, MessageActionItem, MessageType, OneOf, RenameFilesParams, SymbolKind, TextDocumentSyncSaveOptions, TextEdit, Uri, WillRenameFiles, WorkDoneProgressCancelParams, WorkspaceFolder, notification::DidRenameFiles, }; @@ -182,6 +182,12 @@ pub struct DocumentDiagnostics { version: Option, } +#[derive(Default)] +struct DynamicRegistrations { + did_change_watched_files: HashMap>, + diagnostics: HashMap, DiagnosticServerCapabilities>, +} + pub struct LocalLspStore { weak: WeakEntity, worktree_store: Entity, @@ -199,8 +205,7 @@ pub struct LocalLspStore { watched_manifest_filenames: HashSet, language_server_paths_watched_for_rename: HashMap, - language_server_watcher_registrations: - HashMap>>, + language_server_dynamic_registrations: HashMap, supplementary_language_servers: HashMap)>, prettier_store: Entity, @@ -3175,7 +3180,7 @@ impl LocalLspStore { for watcher in watchers { if let Some((worktree, literal_prefix, pattern)) = - self.worktree_and_path_for_file_watcher(&worktrees, watcher, cx) + Self::worktree_and_path_for_file_watcher(&worktrees, watcher, cx) { worktree.update(cx, |worktree, _| { if let Some((tree, glob)) = @@ -3273,7 +3278,6 @@ impl LocalLspStore { } fn worktree_and_path_for_file_watcher( - &self, worktrees: &[Entity], watcher: &FileSystemWatcher, cx: &App, @@ -3321,15 +3325,18 @@ impl LocalLspStore { language_server_id: LanguageServerId, cx: &mut Context, ) { - let Some(watchers) = self - .language_server_watcher_registrations + let Some(registrations) = self + .language_server_dynamic_registrations .get(&language_server_id) else { return; }; - let watch_builder = - self.rebuild_watched_paths_inner(language_server_id, watchers.values().flatten(), cx); + let watch_builder = self.rebuild_watched_paths_inner( + language_server_id, + registrations.did_change_watched_files.values().flatten(), + cx, + ); let watcher = watch_builder.build(self.fs.clone(), language_server_id, cx); self.language_server_watched_paths .insert(language_server_id, watcher); @@ -3345,11 +3352,13 @@ impl LocalLspStore { cx: &mut Context, ) { let registrations = self - .language_server_watcher_registrations + .language_server_dynamic_registrations .entry(language_server_id) .or_default(); - registrations.insert(registration_id.to_string(), params.watchers); + registrations + .did_change_watched_files + .insert(registration_id.to_string(), params.watchers); self.rebuild_watched_paths(language_server_id, cx); } @@ -3361,11 +3370,15 @@ impl LocalLspStore { cx: &mut Context, ) { let registrations = self - .language_server_watcher_registrations + .language_server_dynamic_registrations .entry(language_server_id) .or_default(); - if registrations.remove(registration_id).is_some() { + if registrations + .did_change_watched_files + .remove(registration_id) + .is_some() + { log::info!( "language server {}: unregistered workspace/DidChangeWatchedFiles capability with id {}", language_server_id, @@ -3736,7 +3749,7 @@ impl LspStore { last_workspace_edits_by_language_server: Default::default(), language_server_watched_paths: Default::default(), language_server_paths_watched_for_rename: Default::default(), - language_server_watcher_registrations: Default::default(), + language_server_dynamic_registrations: Default::default(), buffers_being_formatted: Default::default(), buffer_snapshots: Default::default(), prettier_store, @@ -4324,7 +4337,7 @@ impl LspStore { cx: &Context, ) -> bool where - F: Fn(&lsp::ServerCapabilities) -> bool, + F: FnMut(&lsp::ServerCapabilities) -> bool, { let Some(language) = buffer.read(cx).language().cloned() else { return false; @@ -6326,12 +6339,30 @@ impl LspStore { let buffer_id = buffer.read(cx).remote_id(); if let Some((client, upstream_project_id)) = self.upstream_client() { + let mut suitable_capabilities = None; + // Are we capable for proto request? + let any_server_has_diagnostics_provider = self.check_if_capable_for_proto_request( + &buffer, + |capabilities| { + if let Some(caps) = &capabilities.diagnostic_provider { + suitable_capabilities = Some(caps.clone()); + true + } else { + false + } + }, + cx, + ); + // We don't really care which caps are passed into the request, as they're ignored by RPC anyways. + let Some(dynamic_caps) = suitable_capabilities else { + return Task::ready(Ok(None)); + }; + assert!(any_server_has_diagnostics_provider); + let request = GetDocumentDiagnostics { previous_result_id: None, + dynamic_caps, }; - if !self.is_capable_for_proto_request(&buffer, &request, cx) { - return Task::ready(Ok(None)); - } let request_task = client.request_lsp( upstream_project_id, LSP_REQUEST_TIMEOUT, @@ -6346,23 +6377,44 @@ impl LspStore { Ok(None) }) } else { - let server_ids = buffer.update(cx, |buffer, cx| { + let servers = buffer.update(cx, |buffer, cx| { self.language_servers_for_local_buffer(buffer, cx) - .map(|(_, server)| server.server_id()) + .map(|(_, server)| server.clone()) .collect::>() }); - let pull_diagnostics = server_ids + + let pull_diagnostics = servers .into_iter() - .map(|server_id| { - let result_id = self.result_id(server_id, buffer_id, cx); - self.request_lsp( - buffer.clone(), - LanguageServerToQuery::Other(server_id), - GetDocumentDiagnostics { - previous_result_id: result_id, - }, - cx, - ) + .flat_map(|server| { + let result = maybe!({ + let local = self.as_local()?; + let server_id = server.server_id(); + let providers_with_identifiers = local + .language_server_dynamic_registrations + .get(&server_id) + .into_iter() + .flat_map(|registrations| registrations.diagnostics.values().cloned()) + .collect::>(); + Some( + providers_with_identifiers + .into_iter() + .map(|dynamic_caps| { + let result_id = self.result_id(server_id, buffer_id, cx); + self.request_lsp( + buffer.clone(), + LanguageServerToQuery::Other(server_id), + GetDocumentDiagnostics { + previous_result_id: result_id, + dynamic_caps, + }, + cx, + ) + }) + .collect::>(), + ) + }); + + result.unwrap_or_default() }) .collect::>(); @@ -8895,14 +8947,17 @@ impl LspStore { ); } lsp::ProgressParamsValue::WorkspaceDiagnostic(report) => { + let identifier = token.split_once("id:").map(|(_, id)| id.to_owned()); if let Some(LanguageServerState::Running { - workspace_refresh_task: Some(workspace_refresh_task), + workspace_diagnostics_refresh_tasks, .. }) = self .as_local_mut() .and_then(|local| local.language_servers.get_mut(&language_server_id)) + && let Some(workspace_diagnostics) = + workspace_diagnostics_refresh_tasks.get_mut(&identifier) { - workspace_refresh_task.progress_tx.try_send(()).ok(); + workspace_diagnostics.progress_tx.try_send(()).ok(); self.apply_workspace_diagnostic_report(language_server_id, report, cx) } } @@ -10408,13 +10463,31 @@ impl LspStore { let workspace_folders = workspace_folders.lock().clone(); language_server.set_workspace_folders(workspace_folders); + let workspace_diagnostics_refresh_tasks = language_server + .capabilities() + .diagnostic_provider + .and_then(|provider| { + let workspace_refresher = lsp_workspace_diagnostics_refresh( + None, + provider.clone(), + language_server.clone(), + cx, + )?; + local + .language_server_dynamic_registrations + .entry(server_id) + .or_default() + .diagnostics + .entry(None) + .or_insert(provider); + Some((None, workspace_refresher)) + }) + .into_iter() + .collect(); local.language_servers.insert( server_id, LanguageServerState::Running { - workspace_refresh_task: lsp_workspace_diagnostics_refresh( - language_server.clone(), - cx, - ), + workspace_diagnostics_refresh_tasks, adapter: adapter.clone(), server: language_server.clone(), simulate_disk_based_diagnostics_completion: None, @@ -11124,13 +11197,15 @@ impl LspStore { pub fn pull_workspace_diagnostics(&mut self, server_id: LanguageServerId) { if let Some(LanguageServerState::Running { - workspace_refresh_task: Some(workspace_refresh_task), + workspace_diagnostics_refresh_tasks, .. }) = self .as_local_mut() .and_then(|local| local.language_servers.get_mut(&server_id)) { - workspace_refresh_task.refresh_tx.try_send(()).ok(); + for diagnostics in workspace_diagnostics_refresh_tasks.values_mut() { + diagnostics.refresh_tx.try_send(()).ok(); + } } } @@ -11146,11 +11221,13 @@ impl LspStore { local.language_server_ids_for_buffer(buffer, cx) }) { if let Some(LanguageServerState::Running { - workspace_refresh_task: Some(workspace_refresh_task), + workspace_diagnostics_refresh_tasks, .. }) = local.language_servers.get_mut(&server_id) { - workspace_refresh_task.refresh_tx.try_send(()).ok(); + for diagnostics in workspace_diagnostics_refresh_tasks.values_mut() { + diagnostics.refresh_tx.try_send(()).ok(); + } } } } @@ -11476,26 +11553,49 @@ impl LspStore { "textDocument/diagnostic" => { if let Some(caps) = reg .register_options - .map(serde_json::from_value) + .map(serde_json::from_value::) .transpose()? { - let state = self + let local = self .as_local_mut() - .context("Expected LSP Store to be local")? + .context("Expected LSP Store to be local")?; + let state = local .language_servers .get_mut(&server_id) .context("Could not obtain Language Servers state")?; - server.update_capabilities(|capabilities| { - capabilities.diagnostic_provider = Some(caps); - }); + local + .language_server_dynamic_registrations + .get_mut(&server_id) + .and_then(|registrations| { + registrations + .diagnostics + .insert(Some(reg.id.clone()), caps.clone()) + }); + + let mut can_now_provide_diagnostics = false; if let LanguageServerState::Running { - workspace_refresh_task, + workspace_diagnostics_refresh_tasks, .. } = state - && workspace_refresh_task.is_none() + && let Some(task) = lsp_workspace_diagnostics_refresh( + Some(reg.id.clone()), + caps.clone(), + server.clone(), + cx, + ) { - *workspace_refresh_task = - lsp_workspace_diagnostics_refresh(server.clone(), cx) + workspace_diagnostics_refresh_tasks.insert(Some(reg.id), task); + can_now_provide_diagnostics = true; + } + + // We don't actually care about capabilities.diagnostic_provider, but it IS relevant for the remote peer + // to know that there's at least one provider. Otherwise, it will never ask us to issue documentdiagnostic calls on their behalf, + // as it'll think that they're not supported. + if can_now_provide_diagnostics { + server.update_capabilities(|capabilities| { + debug_assert!(capabilities.diagnostic_provider.is_none()); + capabilities.diagnostic_provider = Some(caps); + }); } notify_server_capabilities_updated(&server, cx); @@ -11658,22 +11758,45 @@ impl LspStore { notify_server_capabilities_updated(&server, cx); } "textDocument/diagnostic" => { - server.update_capabilities(|capabilities| { - capabilities.diagnostic_provider = None; - }); - let state = self + let local = self .as_local_mut() - .context("Expected LSP Store to be local")? + .context("Expected LSP Store to be local")?; + + let state = local .language_servers .get_mut(&server_id) .context("Could not obtain Language Servers state")?; - if let LanguageServerState::Running { - workspace_refresh_task, - .. - } = state + let options = local + .language_server_dynamic_registrations + .get_mut(&server_id) + .with_context(|| { + format!("Expected dynamic registration to exist for server {server_id}") + })?.diagnostics + .remove(&Some(unreg.id.clone())) + .with_context(|| format!( + "Attempted to unregister non-existent diagnostic registration with ID {}", + unreg.id) + )?; + + let mut has_any_diagnostic_providers_still = true; + if let Some(identifier) = diagnostic_identifier(&options) + && let LanguageServerState::Running { + workspace_diagnostics_refresh_tasks, + .. + } = state { - _ = workspace_refresh_task.take(); + workspace_diagnostics_refresh_tasks.remove(&identifier); + has_any_diagnostic_providers_still = + !workspace_diagnostics_refresh_tasks.is_empty(); } + + if !has_any_diagnostic_providers_still { + server.update_capabilities(|capabilities| { + debug_assert!(capabilities.diagnostic_provider.is_some()); + capabilities.diagnostic_provider = None; + }); + } + notify_server_capabilities_updated(&server, cx); } "textDocument/documentColor" => { @@ -11859,24 +11982,12 @@ fn subscribe_to_binary_statuses( } fn lsp_workspace_diagnostics_refresh( + registration_id: Option, + options: DiagnosticServerCapabilities, server: Arc, cx: &mut Context<'_, LspStore>, ) -> Option { - let identifier = match server.capabilities().diagnostic_provider? { - lsp::DiagnosticServerCapabilities::Options(diagnostic_options) => { - if !diagnostic_options.workspace_diagnostics { - return None; - } - diagnostic_options.identifier - } - lsp::DiagnosticServerCapabilities::RegistrationOptions(registration_options) => { - let diagnostic_options = registration_options.diagnostic_options; - if !diagnostic_options.workspace_diagnostics { - return None; - } - diagnostic_options.identifier - } - }; + let identifier = diagnostic_identifier(&options)?; let (progress_tx, mut progress_rx) = mpsc::channel(1); let (mut refresh_tx, mut refresh_rx) = mpsc::channel(1); @@ -11922,7 +12033,14 @@ fn lsp_workspace_diagnostics_refresh( return; }; - let token = format!("workspace/diagnostic-{}-{}", server.server_id(), requests); + let token = if let Some(identifier) = ®istration_id { + format!( + "workspace/diagnostic/{}/{requests}/id:{identifier}", + server.server_id(), + ) + } else { + format!("workspace/diagnostic/{}/{requests}", server.server_id()) + }; progress_rx.try_recv().ok(); let timer = @@ -11988,6 +12106,24 @@ fn lsp_workspace_diagnostics_refresh( }) } +fn diagnostic_identifier(options: &DiagnosticServerCapabilities) -> Option> { + match &options { + lsp::DiagnosticServerCapabilities::Options(diagnostic_options) => { + if !diagnostic_options.workspace_diagnostics { + return None; + } + Some(diagnostic_options.identifier.clone()) + } + lsp::DiagnosticServerCapabilities::RegistrationOptions(registration_options) => { + let diagnostic_options = ®istration_options.diagnostic_options; + if !diagnostic_options.workspace_diagnostics { + return None; + } + Some(diagnostic_options.identifier.clone()) + } + } +} + fn resolve_word_completion(snapshot: &BufferSnapshot, completion: &mut Completion) { let CompletionSource::BufferWord { word_range, @@ -12392,7 +12528,7 @@ pub enum LanguageServerState { adapter: Arc, server: Arc, simulate_disk_based_diagnostics_completion: Option>, - workspace_refresh_task: Option, + workspace_diagnostics_refresh_tasks: HashMap, WorkspaceRefreshTask>, }, }