From 2cc33b5c4760f77a1f3bd5386f4d3ca2f9a7b0ea Mon Sep 17 00:00:00 2001
From: Michael Davis <mcarsondavis@gmail.com>
Date: Fri, 21 Mar 2025 10:05:50 -0400
Subject: [PATCH] Add pull diagnostics `identifier` to LSP diagnostic provider

This includes a change to lsp-types to store the identifier as an Arc
since it will be cloned for each diagnostic.
---
 helix-core/src/diagnostic.rs               | 19 +++++++++++--
 helix-lsp-types/src/document_diagnostic.rs | 32 +++++++++++++++++++---
 helix-term/src/application.rs              |  6 ++--
 3 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/helix-core/src/diagnostic.rs b/helix-core/src/diagnostic.rs
index ba7df1b0a..b9360b525 100644
--- a/helix-core/src/diagnostic.rs
+++ b/helix-core/src/diagnostic.rs
@@ -1,5 +1,5 @@
 //! LSP diagnostic utility types.
-use std::fmt;
+use std::{fmt, sync::Arc};
 
 pub use helix_stdx::range::Range;
 use serde::{Deserialize, Serialize};
@@ -50,9 +50,24 @@ pub struct Diagnostic {
     pub data: Option<serde_json::Value>,
 }
 
+/// The source of a diagnostic.
+///
+/// This type is cheap to clone: all data is either `Copy` or wrapped in an `Arc`.
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
 pub enum DiagnosticProvider {
-    Lsp { server_id: LanguageServerId },
+    Lsp {
+        /// The ID of the language server which sent the diagnostic.
+        server_id: LanguageServerId,
+        /// An optional identifier under which diagnostics are managed by the client.
+        ///
+        /// `identifier` is a field from the LSP "Pull Diagnostics" feature meant to provide an
+        /// optional "namespace" for diagnostics: a language server can respond to a diagnostics
+        /// pull request with an identifier and these diagnostics should be treated as separate
+        /// from push diagnostics. Rust-analyzer uses this feature for example to provide Cargo
+        /// diagnostics with push and internal diagnostics with pull. The push diagnostics should
+        /// not clear the pull diagnostics and vice-versa.
+        identifier: Option<Arc<str>>,
+    },
     // Future internal features can go here...
 }
 
diff --git a/helix-lsp-types/src/document_diagnostic.rs b/helix-lsp-types/src/document_diagnostic.rs
index 5a1c6b35a..d2dcc5449 100644
--- a/helix-lsp-types/src/document_diagnostic.rs
+++ b/helix-lsp-types/src/document_diagnostic.rs
@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use std::{collections::HashMap, sync::Arc};
 
 use serde::{Deserialize, Serialize};
 
@@ -33,8 +33,13 @@ pub struct DiagnosticClientCapabilities {
 pub struct DiagnosticOptions {
     /// An optional identifier under which the diagnostics are
     /// managed by the client.
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub identifier: Option<String>,
+    #[serde(
+        default,
+        skip_serializing_if = "Option::is_none",
+        serialize_with = "serialize_option_arc_str",
+        deserialize_with = "deserialize_option_arc_str"
+    )]
+    pub identifier: Option<Arc<str>>,
 
     /// Whether the language has inter file dependencies, meaning that editing code in one file can
     /// result in a different diagnostic set in another file. Inter file dependencies are common
@@ -48,6 +53,19 @@ pub struct DiagnosticOptions {
     pub work_done_progress_options: WorkDoneProgressOptions,
 }
 
+fn serialize_option_arc_str<S: serde::Serializer>(
+    val: &Option<Arc<str>>,
+    serializer: S,
+) -> Result<S::Ok, S::Error> {
+    serializer.serialize_str(val.as_ref().unwrap())
+}
+
+fn deserialize_option_arc_str<'de, D: serde::Deserializer<'de>>(
+    deserializer: D,
+) -> Result<Option<Arc<str>>, D::Error> {
+    Option::<String>::deserialize(deserializer).map(|opt| opt.map(|s| s.into()))
+}
+
 /// Diagnostic registration options.
 ///
 /// @since 3.17.0
@@ -81,7 +99,13 @@ pub struct DocumentDiagnosticParams {
     pub text_document: TextDocumentIdentifier,
 
     /// The additional identifier provided during registration.
-    pub identifier: Option<String>,
+    #[serde(
+        default,
+        skip_serializing_if = "Option::is_none",
+        serialize_with = "serialize_option_arc_str",
+        deserialize_with = "deserialize_option_arc_str"
+    )]
+    pub identifier: Option<Arc<str>>,
 
     /// The result ID of a previous response if provided.
     pub previous_result_id: Option<String>,
diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs
index afcb1a10d..bd6ac7588 100644
--- a/helix-term/src/application.rs
+++ b/helix-term/src/application.rs
@@ -740,8 +740,10 @@ impl Application {
                             log::error!("Discarding publishDiagnostic notification sent by an uninitialized server: {}", language_server.name());
                             return;
                         }
-                        let provider =
-                            helix_core::diagnostic::DiagnosticProvider::Lsp { server_id };
+                        let provider = helix_core::diagnostic::DiagnosticProvider::Lsp {
+                            server_id,
+                            identifier: None,
+                        };
                         self.editor.handle_lsp_diagnostics(
                             &provider,
                             uri,