Refactor DiagnosticProvider as an enum

This resolves a TODO in the core diagnostic module to refactor this
type. It was originally an alias of `LanguageServerId` for simplicity.
Refactoring as an enum is a necessary step towards introducing
"internal" diagnostics - diagnostics emitted by core features such as
a spell checker. Fully supporting this use-case will require further
larger changes to the diagnostic type, but the change to the provider
can be made first.

Note that `Copy` is not derived for `DiagnosticProvider` (as it was
previously because `LanguageServerId` is `Copy`). In the child commits
we will add the `identifier` used in LSP pull diagnostics which is a
string - not `Copy`.

helix-core/src/diagnostic.rs

@@ -50,8 +50,20 @@ pub struct Diagnostic {
     pub data: Option<serde_json::Value>,
 }
 
-// TODO turn this into an enum + feature flag when lsp becomes optional
+#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
-pub type DiagnosticProvider = LanguageServerId;
+pub enum DiagnosticProvider {
+    Lsp { server_id: LanguageServerId },
+    // Future internal features can go here...
+}
+
+impl DiagnosticProvider {
+    pub fn language_server_id(&self) -> Option<LanguageServerId> {
+        match self {
+            Self::Lsp { server_id, .. } => Some(*server_id),
+            // _ => None,
+        }
+    }
+}
 
 // while I would prefer having this in helix-lsp that necessitates a bunch of
 // conversions I would rather not add. I think its fine since this just a very

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 };
                         self.editor.handle_lsp_diagnostics(
-                            language_server.id(),
+                            &provider,
                             uri,
                             params.version,
                             params.diagnostics,
@@ -854,14 +856,16 @@ impl Application {
                         // we need to clear those and remove the entries from the list if this leads to
                         // an empty diagnostic list for said files
                         for diags in self.editor.diagnostics.values_mut() {
-                            diags.retain(|(_, lsp_id)| *lsp_id != server_id);
+                            diags.retain(|(_, provider)| {
+                                provider.language_server_id() != Some(server_id)
+                            });
                         }
 
                         self.editor.diagnostics.retain(|_, diags| !diags.is_empty());
 
                         // Clear any diagnostics for documents with this server open.
                         for doc in self.editor.documents_mut() {
-                            doc.clear_diagnostics(Some(server_id));
+                            doc.clear_diagnostics_for_language_server(server_id);
                         }
 
                         // Remove the language server from the registry.

helix-term/src/commands/lsp.rs

@@ -14,7 +14,8 @@ use tui::{text::Span, widgets::Row};
 use super::{align_view, push_jump, Align, Context, Editor};
 
 use helix_core::{
-    syntax::LanguageServerFeature, text_annotations::InlineAnnotation, Selection, Uri,
+    diagnostic::DiagnosticProvider, syntax::LanguageServerFeature,
+    text_annotations::InlineAnnotation, Selection, Uri,
 };
 use helix_stdx::path;
 use helix_view::{
@@ -31,13 +32,7 @@ use crate::{
     ui::{self, overlay::overlaid, FileLocation, Picker, Popup, PromptEvent},
 };
 
-use std::{
+use std::{cmp::Ordering, collections::HashSet, fmt::Display, future::Future, path::Path};
-    cmp::Ordering,
-    collections::{BTreeMap, HashSet},
-    fmt::Display,
-    future::Future,
-    path::Path,
-};
 
 /// Gets the first language server that is attached to a document which supports a specific feature.
 /// If there is no configured language server that supports the feature, this displays a status message.
@@ -209,7 +204,7 @@ type DiagnosticsPicker = Picker<PickerDiagnostic, DiagnosticStyles>;
 
 fn diag_picker(
     cx: &Context,
-    diagnostics: BTreeMap<Uri, Vec<(lsp::Diagnostic, LanguageServerId)>>,
+    diagnostics: impl IntoIterator<Item = (Uri, Vec<(lsp::Diagnostic, DiagnosticProvider)>)>,
     format: DiagnosticsFormat,
 ) -> DiagnosticsPicker {
     // TODO: drop current_path comparison and instead use workspace: bool flag?
@@ -219,8 +214,11 @@ fn diag_picker(
     for (uri, diags) in diagnostics {
         flat_diag.reserve(diags.len());
 
-        for (diag, ls) in diags {
+        for (diag, provider) in diags {
-            if let Some(ls) = cx.editor.language_server_by_id(ls) {
+            if let Some(ls) = provider
+                .language_server_id()
+                .and_then(|id| cx.editor.language_server_by_id(id))
+            {
                 flat_diag.push(PickerDiagnostic {
                     location: Location {
                         uri: uri.clone(),
@@ -560,11 +558,7 @@ pub fn diagnostics_picker(cx: &mut Context) {
     let doc = doc!(cx.editor);
     if let Some(uri) = doc.uri() {
         let diagnostics = cx.editor.diagnostics.get(&uri).cloned().unwrap_or_default();
-        let picker = diag_picker(
+        let picker = diag_picker(cx, [(uri, diagnostics)], DiagnosticsFormat::HideSourcePath);
-            cx,
-            [(uri, diagnostics)].into(),
-            DiagnosticsFormat::HideSourcePath,
-        );
         cx.push_layer(Box::new(overlaid(picker)));
     }
 }

helix-term/src/commands/typed.rs

@@ -1622,7 +1622,7 @@ fn lsp_stop(cx: &mut compositor::Context, args: Args, event: PromptEvent) -> any
 
         for doc in cx.editor.documents_mut() {
             if let Some(client) = doc.remove_language_server_by_name(ls_name) {
-                doc.clear_diagnostics(Some(client.id()));
+                doc.clear_diagnostics_for_language_server(client.id());
                 doc.reset_all_inlay_hints();
                 doc.inlay_hints_oudated = true;
             }

helix-view/src/document.rs

@@ -5,6 +5,7 @@ use futures_util::future::BoxFuture;
 use futures_util::FutureExt;
 use helix_core::auto_pairs::AutoPairs;
 use helix_core::chars::char_is_word;
+use helix_core::diagnostic::DiagnosticProvider;
 use helix_core::doc_formatter::TextFormat;
 use helix_core::encoding::Encoding;
 use helix_core::snippets::{ActiveSnippet, SnippetRenderCtx};
@@ -1433,8 +1434,13 @@ impl Document {
             true
         });
 
-        self.diagnostics
+        self.diagnostics.sort_by_key(|diagnostic| {
-            .sort_by_key(|diagnostic| (diagnostic.range, diagnostic.severity, diagnostic.provider));
+            (
+                diagnostic.range,
+                diagnostic.severity,
+                diagnostic.provider.clone(),
+            )
+        });
 
         // Update the inlay hint annotations' positions, helping ensure they are displayed in the proper place
         let apply_inlay_hint_changes = |annotations: &mut Vec<InlineAnnotation>| {
@@ -1980,7 +1986,7 @@ impl Document {
         text: &Rope,
         language_config: Option<&LanguageConfiguration>,
         diagnostic: &helix_lsp::lsp::Diagnostic,
-        language_server_id: LanguageServerId,
+        provider: DiagnosticProvider,
         offset_encoding: helix_lsp::OffsetEncoding,
     ) -> Option<Diagnostic> {
         use helix_core::diagnostic::{Range, Severity::*};
@@ -2060,7 +2066,7 @@ impl Document {
             tags,
             source: diagnostic.source.clone(),
             data: diagnostic.data.clone(),
-            provider: language_server_id,
+            provider,
         })
     }
 
@@ -2073,13 +2079,18 @@ impl Document {
         &mut self,
         diagnostics: impl IntoIterator<Item = Diagnostic>,
         unchanged_sources: &[String],
-        language_server_id: Option<LanguageServerId>,
+        provider: Option<&DiagnosticProvider>,
     ) {
         if unchanged_sources.is_empty() {
-            self.clear_diagnostics(language_server_id);
+            if let Some(provider) = provider {
+                self.diagnostics
+                    .retain(|diagnostic| &diagnostic.provider != provider);
+            } else {
+                self.diagnostics.clear();
+            }
         } else {
             self.diagnostics.retain(|d| {
-                if language_server_id.is_some_and(|id| id != d.provider) {
+                if provider.is_some_and(|provider| provider != &d.provider) {
                     return true;
                 }
 
@@ -2091,17 +2102,19 @@ impl Document {
             });
         }
         self.diagnostics.extend(diagnostics);
-        self.diagnostics
+        self.diagnostics.sort_by_key(|diagnostic| {
-            .sort_by_key(|diagnostic| (diagnostic.range, diagnostic.severity, diagnostic.provider));
+            (
+                diagnostic.range,
+                diagnostic.severity,
+                diagnostic.provider.clone(),
+            )
+        });
     }
 
     /// clears diagnostics for a given language server id if set, otherwise all diagnostics are cleared
-    pub fn clear_diagnostics(&mut self, language_server_id: Option<LanguageServerId>) {
+    pub fn clear_diagnostics_for_language_server(&mut self, id: LanguageServerId) {
-        if let Some(id) = language_server_id {
+        self.diagnostics
-            self.diagnostics.retain(|d| d.provider != id);
+            .retain(|d| d.provider.language_server_id() != Some(id));
-        } else {
-            self.diagnostics.clear();
-        }
     }
 
     /// Get the document's auto pairs. If the document has a recognized

helix-view/src/editor.rs

@@ -45,6 +45,7 @@ use anyhow::{anyhow, bail, Error};
 pub use helix_core::diagnostic::Severity;
 use helix_core::{
     auto_pairs::AutoPairs,
+    diagnostic::DiagnosticProvider,
     syntax::{self, AutoPairConfig, IndentationHeuristic, LanguageServerFeature, SoftWrap},
     Change, LineEnding, Position, Range, Selection, Uri, NATIVE_LINE_ENDING,
 };
@@ -1041,6 +1042,8 @@ pub struct Breakpoint {
 
 use futures_util::stream::{Flatten, Once};
 
+type Diagnostics = BTreeMap<Uri, Vec<(lsp::Diagnostic, DiagnosticProvider)>>;
+
 pub struct Editor {
     /// Current editing mode.
     pub mode: Mode,
@@ -1060,7 +1063,7 @@ pub struct Editor {
     pub macro_recording: Option<(char, Vec<KeyEvent>)>,
     pub macro_replaying: Vec<char>,
     pub language_servers: helix_lsp::Registry,
-    pub diagnostics: BTreeMap<Uri, Vec<(lsp::Diagnostic, LanguageServerId)>>,
+    pub diagnostics: Diagnostics,
     pub diff_providers: DiffProviderRegistry,
 
     pub debugger: Option<dap::Client>,
@@ -1207,7 +1210,7 @@ impl Editor {
             macro_replaying: Vec::new(),
             theme: theme_loader.default(),
             language_servers,
-            diagnostics: BTreeMap::new(),
+            diagnostics: Diagnostics::new(),
             diff_providers: DiffProviderRegistry::default(),
             debugger: None,
             debugger_events: SelectAll::new(),
@@ -2007,7 +2010,7 @@ impl Editor {
     /// Returns all supported diagnostics for the document
     pub fn doc_diagnostics<'a>(
         language_servers: &'a helix_lsp::Registry,
-        diagnostics: &'a BTreeMap<Uri, Vec<(lsp::Diagnostic, LanguageServerId)>>,
+        diagnostics: &'a Diagnostics,
         document: &Document,
     ) -> impl Iterator<Item = helix_core::Diagnostic> + 'a {
         Editor::doc_diagnostics_with_filter(language_servers, diagnostics, document, |_, _| true)
@@ -2017,9 +2020,9 @@ impl Editor {
     /// filtered by `filter` which is invocated with the raw `lsp::Diagnostic` and the language server id it came from
     pub fn doc_diagnostics_with_filter<'a>(
         language_servers: &'a helix_lsp::Registry,
-        diagnostics: &'a BTreeMap<Uri, Vec<(lsp::Diagnostic, LanguageServerId)>>,
+        diagnostics: &'a Diagnostics,
         document: &Document,
-        filter: impl Fn(&lsp::Diagnostic, LanguageServerId) -> bool + 'a,
+        filter: impl Fn(&lsp::Diagnostic, &DiagnosticProvider) -> bool + 'a,
     ) -> impl Iterator<Item = helix_core::Diagnostic> + 'a {
         let text = document.text().clone();
         let language_config = document.language.clone();
@@ -2027,8 +2030,9 @@ impl Editor {
             .uri()
             .and_then(|uri| diagnostics.get(&uri))
             .map(|diags| {
-                diags.iter().filter_map(move |(diagnostic, lsp_id)| {
+                diags.iter().filter_map(move |(diagnostic, provider)| {
-                    let ls = language_servers.get_by_id(*lsp_id)?;
+                    let server_id = provider.language_server_id()?;
+                    let ls = language_servers.get_by_id(server_id)?;
                     language_config
                         .as_ref()
                         .and_then(|c| {
@@ -2038,12 +2042,12 @@ impl Editor {
                             })
                         })
                         .and_then(|_| {
-                            if filter(diagnostic, *lsp_id) {
+                            if filter(diagnostic, provider) {
                                 Document::lsp_diagnostic_to_diagnostic(
                                     &text,
                                     language_config.as_deref(),
                                     diagnostic,
-                                    *lsp_id,
+                                    provider.clone(),
                                     ls.offset_encoding(),
                                 )
                             } else {

helix-view/src/gutter.rs

@@ -69,9 +69,10 @@ pub fn diagnostic<'doc>(
                 .iter()
                 .take_while(|d| {
                     d.line == line
-                        && doc
+                        && d.provider.language_server_id().map_or(true, |id| {
-                            .language_servers_with_feature(LanguageServerFeature::Diagnostics)
+                            doc.language_servers_with_feature(LanguageServerFeature::Diagnostics)
-                            .any(|ls| ls.id() == d.provider)
+                                .any(|ls| ls.id() == id)
+                        })
                 });
             diagnostics_on_line.max_by_key(|d| d.severity).map(|d| {
                 write!(out, "●").ok();

helix-view/src/handlers/lsp.rs

@@ -4,6 +4,7 @@ use std::fmt::Display;
 use crate::editor::Action;
 use crate::events::DiagnosticsDidChange;
 use crate::Editor;
+use helix_core::diagnostic::DiagnosticProvider;
 use helix_core::Uri;
 use helix_lsp::util::generate_transaction_from_edits;
 use helix_lsp::{lsp, LanguageServerId, OffsetEncoding};
@@ -276,7 +277,7 @@ impl Editor {
 
     pub fn handle_lsp_diagnostics(
         &mut self,
-        server_id: LanguageServerId,
+        provider: &DiagnosticProvider,
         uri: Uri,
         version: Option<i32>,
         mut diagnostics: Vec<lsp::Diagnostic>,
@@ -309,8 +310,8 @@ impl Editor {
                     .filter(|d| d.source.as_ref() == Some(source));
                 let old_diagnostics = old_diagnostics
                     .iter()
-                    .filter(|(d, d_server)| {
+                    .filter(|(d, d_provider)| {
-                        *d_server == server_id && d.source.as_ref() == Some(source)
+                        d_provider == provider && d.source.as_ref() == Some(source)
                     })
                     .map(|(d, _)| d);
                 if new_diagnostics.eq(old_diagnostics) {
@@ -319,7 +320,7 @@ impl Editor {
             }
         }
 
-        let diagnostics = diagnostics.into_iter().map(|d| (d, server_id));
+        let diagnostics = diagnostics.into_iter().map(|d| (d, provider.clone()));
 
         // Insert the original lsp::Diagnostics here because we may have no open document
         // for diagnostic message and so we can't calculate the exact position.
@@ -328,7 +329,7 @@ impl Editor {
             Entry::Occupied(o) => {
                 let current_diagnostics = o.into_mut();
                 // there may entries of other language servers, which is why we can't overwrite the whole entry
-                current_diagnostics.retain(|(_, lsp_id)| *lsp_id != server_id);
+                current_diagnostics.retain(|(_, d_provider)| d_provider != provider);
                 current_diagnostics.extend(diagnostics);
                 current_diagnostics
                 // Sort diagnostics first by severity and then by line numbers.
@@ -338,12 +339,12 @@ impl Editor {
 
         // Sort diagnostics first by severity and then by line numbers.
         // Note: The `lsp::DiagnosticSeverity` enum is already defined in decreasing order
-        diagnostics.sort_by_key(|(d, server_id)| (d.severity, d.range.start, *server_id));
+        diagnostics.sort_by_key(|(d, provider)| (d.severity, d.range.start, provider.clone()));
 
         if let Some(doc) = doc {
             let diagnostic_of_language_server_and_not_in_unchanged_sources =
-                |diagnostic: &lsp::Diagnostic, ls_id| {
+                |diagnostic: &lsp::Diagnostic, d_provider: &DiagnosticProvider| {
-                    ls_id == server_id
+                    d_provider == provider
                         && diagnostic
                             .source
                             .as_ref()
@@ -355,7 +356,7 @@ impl Editor {
                 doc,
                 diagnostic_of_language_server_and_not_in_unchanged_sources,
             );
-            doc.replace_diagnostics(diagnostics, &unchanged_diag_sources, Some(server_id));
+            doc.replace_diagnostics(diagnostics, &unchanged_diag_sources, Some(provider));
 
             let doc = doc.id();
             helix_event::dispatch(DiagnosticsDidChange { editor: self, doc });