support unchagedReport and disable pullD if server supports pushD

Add a flag to `Client' that becomes true on the first PushDiagnostic from the server.

Author: woojiq

helix-lsp/src/client.rs

@@ -15,13 +15,13 @@ use lsp_types as lsp;
 use parking_lot::Mutex;
 use serde::Deserialize;
 use serde_json::Value;
-use std::future::Future;
 use std::process::Stdio;
 use std::sync::{
     atomic::{AtomicU64, Ordering},
     Arc,
 };
 use std::{collections::HashMap, path::PathBuf};
+use std::{future::Future, sync::atomic::AtomicBool};
 use tokio::{
     io::{BufReader, BufWriter},
     process::{Child, Command},
@@ -57,6 +57,9 @@ pub struct Client {
     initialize_notify: Arc<Notify>,
     /// workspace folders added while the server is still initializing
     req_timeout: u64,
+    // there is no server capability to know if server supports it
+    // set to true on first PublishDiagnostic
+    pub supports_publish_diagnostic: AtomicBool,
 }
 
 impl Client {
@@ -238,6 +241,7 @@ impl Client {
             root_uri,
             workspace_folders: Mutex::new(workspace_folders),
             initialize_notify: initialize_notify.clone(),
+            supports_publish_diagnostic: AtomicBool::new(false),
         };
 
         Ok((client, server_rx, initialize_notify))

helix-term/src/application.rs

@@ -31,7 +31,12 @@ use crate::{
 use log::{debug, error, warn};
 #[cfg(not(feature = "integration"))]
 use std::io::stdout;
-use std::{collections::btree_map::Entry, io::stdin, path::Path, sync::Arc};
+use std::{
+    collections::btree_map::Entry,
+    io::stdin,
+    path::Path,
+    sync::{atomic, Arc},
+};
 
 use anyhow::{Context, Error};
 
@@ -732,6 +737,9 @@ impl Application {
                             log::error!("Discarding publishDiagnostic notification sent by an uninitialized server: {}", language_server.name());
                             return;
                         }
+                        language_server
+                            .supports_publish_diagnostic
+                            .fetch_or(true, atomic::Ordering::Relaxed);
                         let offset_encoding = language_server.offset_encoding();
                         let doc = self.editor.document_by_path_mut(&path).filter(|doc| {
                             if let Some(version) = params.version {

helix-term/src/commands/lsp.rs

@@ -41,11 +41,11 @@ use crate::{
 
 use std::{
     cmp::Ordering,
-    collections::{BTreeMap, HashSet},
+    collections::{BTreeMap, HashMap, HashSet},
     fmt::Write,
     future::Future,
     path::PathBuf,
-    sync::Arc,
+    sync::{atomic, Arc},
 };
 
 /// Gets the first language server that is attached to a document which supports a specific feature.
@@ -1498,17 +1498,13 @@ pub fn select_references_to_symbol_under_cursor(cx: &mut Context) {
 }
 
 pub fn pull_diagnostic_for_current_doc(editor: &mut Editor, jobs: &mut crate::job::Jobs) {
-    fn parse_diagnostic<P: AsRef<std::path::Path>>(
-        editor: &mut Editor,
-        path: P,
+    fn parse_diagnostic(
+        doc: &mut Document,
         offset_encoding: OffsetEncoding,
         server_id: usize,
         report: Vec<lsp::Diagnostic>,
         result_id: Option<String>,
     ) {
-        let Some(doc) = editor.document_by_path_mut(path) else {
-            return;
-        };
         let diagnostics = lsp_diagnostic_to_diagnostic(
             &report,
             doc.text(),
@@ -1520,14 +1516,49 @@ pub fn pull_diagnostic_for_current_doc(editor: &mut Editor, jobs: &mut crate::jo
         doc.replace_diagnostics(diagnostics, server_id);
     }
 
-    let doc = doc!(editor);
+    fn handle_document_diagnostic_report_kind(
+        editor: &mut Editor,
+        offset_encoding: OffsetEncoding,
+        server_id: usize,
+        report: Option<HashMap<lsp::Url, lsp::DocumentDiagnosticReportKind>>,
+    ) {
+        for (url, report) in report.into_iter().flatten() {
+            let Some(doc) = editor.document_by_path_mut(url.path())
+            else {
+                continue;
+            };
+            match report {
+                lsp::DocumentDiagnosticReportKind::Full(report) => {
+                    parse_diagnostic(
+                        doc,
+                        offset_encoding,
+                        server_id,
+                        report.items,
+                        report.result_id,
+                    );
+                }
+                lsp::DocumentDiagnosticReportKind::Unchanged(report) => {
+                    doc.previous_diagnostic_id = Some(report.result_id);
+                }
+            }
+        }
+    }
 
+    let doc = doc!(editor);
     let Some(language_server) = doc
         .language_servers_with_feature(LanguageServerFeature::PullDiagnostics)
         .next()
     else {
         return;
     };
+    // Specialization does not say whether it is possible to have both types of diagnostics.
+    // Assume we should prefer PublishDiagnostic if possible
+    if language_server
+        .supports_publish_diagnostic
+        .load(atomic::Ordering::Relaxed)
+    {
+        return;
+    }
 
     let future = language_server
         .text_document_diagnostic(doc.identifier(), doc.previous_diagnostic_id.clone());
@@ -1558,32 +1589,31 @@ pub fn pull_diagnostic_for_current_doc(editor: &mut Editor, jobs: &mut crate::jo
                     lsp::DocumentDiagnosticReport::Full(report) => {
                         // Original file diagnostic
                         parse_diagnostic(
-                            editor,
-                            &original_path,
+                            doc,
                             offset_encoding,
                             server_id,
                             report.full_document_diagnostic_report.items,
                             report.full_document_diagnostic_report.result_id,
                         );
 
                         // Related file diagnostic
-                        for (url, report) in report.related_documents.into_iter().flatten() {
-                            match report {
-                                lsp::DocumentDiagnosticReportKind::Full(report) => {
-                                    parse_diagnostic(
-                                        editor,
-                                        url.path(),
-                                        offset_encoding,
-                                        server_id,
-                                        report.items,
-                                        report.result_id,
-                                    );
-                                }
-                                lsp::DocumentDiagnosticReportKind::Unchanged(_) => (),
-                            }
-                        }
+                        handle_document_diagnostic_report_kind(
+                            editor,
+                            offset_encoding,
+                            server_id,
+                            report.related_documents,
+                        );
+                    }
+                    lsp::DocumentDiagnosticReport::Unchanged(report) => {
+                        doc.previous_diagnostic_id =
+                            Some(report.unchanged_document_diagnostic_report.result_id);
+                        handle_document_diagnostic_report_kind(
+                            editor,
+                            offset_encoding,
+                            server_id,
+                            report.related_documents,
+                        );
                     }
-                    lsp::DocumentDiagnosticReport::Unchanged(_) => (),
                 }
             }
         },