Initialize diagnostics when opening a document (#8873)

helix-core/src/syntax.rs

@@ -1795,7 +1795,6 @@ impl HighlightConfiguration {
                 let mut best_index = None;
                 let mut best_match_len = 0;
                 for (i, recognized_name) in recognized_names.iter().enumerate() {
-                    let recognized_name = recognized_name;
                     let mut len = 0;
                     let mut matches = true;
                     for (i, part) in recognized_name.split('.').enumerate() {

helix-term/src/application.rs

@@ -1,14 +1,8 @@
 use arc_swap::{access::Map, ArcSwap};
 use futures_util::Stream;
-use helix_core::{
+use helix_core::{path::get_relative_path, pos_at_coords, syntax, Selection};
-    chars::char_is_word,
-    diagnostic::{DiagnosticTag, NumberOrString},
-    path::get_relative_path,
-    pos_at_coords, syntax, Selection,
-};
 use helix_lsp::{
     lsp::{self, notification::Notification},
-    util::lsp_pos_to_pos,
     LspProgressMap,
 };
 use helix_view::{
@@ -392,6 +386,12 @@ impl Application {
         self.editor.syn_loader = self.syn_loader.clone();
         for document in self.editor.documents.values_mut() {
             document.detect_language(self.syn_loader.clone());
+            let diagnostics = Editor::doc_diagnostics(
+                &self.editor.language_servers,
+                &self.editor.diagnostics,
+                document,
+            );
+            document.replace_diagnostics(diagnostics, &[], None);
         }
 
         Ok(())
@@ -567,6 +567,14 @@ impl Application {
             let id = doc.id();
             doc.detect_language(loader);
             self.editor.refresh_language_servers(id);
+            // and again a borrow checker workaround...
+            let doc = doc_mut!(self.editor, &doc_save_event.doc_id);
+            let diagnostics = Editor::doc_diagnostics(
+                &self.editor.language_servers,
+                &self.editor.diagnostics,
+                doc,
+            );
+            doc.replace_diagnostics(diagnostics, &[], None);
         }
 
         // TODO: fix being overwritten by lsp
@@ -731,7 +739,6 @@ impl Application {
                             log::error!("Discarding publishDiagnostic notification sent by an uninitialized server: {}", language_server.name());
                             return;
                         }
-                        let offset_encoding = language_server.offset_encoding();
                         // have to inline the function because of borrow checking...
                         let doc = self.editor.documents.values_mut()
                             .find(|doc| doc.path().map(|p| p == &path).unwrap_or(false))
@@ -745,11 +752,10 @@ impl Application {
                                 true
                             });
 
-                        if let Some(doc) = doc {
+                        let mut unchanged_diag_sources = Vec::new();
+                        if let Some(doc) = &doc {
                             let lang_conf = doc.language.clone();
-                            let text = doc.text().clone();
 
-                            let mut unchaged_diag_sources_ = Vec::new();
                             if let Some(lang_conf) = &lang_conf {
                                 if let Some(old_diagnostics) =
                                     self.editor.diagnostics.get(&params.uri)
@@ -774,118 +780,11 @@ impl Application {
                                             })
                                             .map(|(d, _)| d);
                                         if new_diagnostics.eq(old_diagnostics) {
-                                            unchaged_diag_sources_.push(source.clone())
+                                            unchanged_diag_sources.push(source.clone())
                                         }
                                     }
                                 }
                             }
-
-                            let unchaged_diag_sources = &unchaged_diag_sources_;
-                            let diagnostics =
-                                params.diagnostics.iter().filter_map(move |diagnostic| {
-                                    use helix_core::diagnostic::{Diagnostic, Range, Severity::*};
-                                    use lsp::DiagnosticSeverity;
-
-                                    if diagnostic.source.as_ref().map_or(false, |source| {
-                                        unchaged_diag_sources.contains(source)
-                                    }) {
-                                        return None;
-                                    }
-
-                                    // TODO: convert inside server
-                                    let start = if let Some(start) = lsp_pos_to_pos(
-                                        &text,
-                                        diagnostic.range.start,
-                                        offset_encoding,
-                                    ) {
-                                        start
-                                    } else {
-                                        log::warn!("lsp position out of bounds - {:?}", diagnostic);
-                                        return None;
-                                    };
-
-                                    let end = if let Some(end) =
-                                        lsp_pos_to_pos(&text, diagnostic.range.end, offset_encoding)
-                                    {
-                                        end
-                                    } else {
-                                        log::warn!("lsp position out of bounds - {:?}", diagnostic);
-                                        return None;
-                                    };
-                                    let severity =
-                                        diagnostic.severity.map(|severity| match severity {
-                                            DiagnosticSeverity::ERROR => Error,
-                                            DiagnosticSeverity::WARNING => Warning,
-                                            DiagnosticSeverity::INFORMATION => Info,
-                                            DiagnosticSeverity::HINT => Hint,
-                                            severity => unreachable!(
-                                                "unrecognized diagnostic severity: {:?}",
-                                                severity
-                                            ),
-                                        });
-
-                                    if let Some(lang_conf) = &lang_conf {
-                                        if let Some(severity) = severity {
-                                            if severity < lang_conf.diagnostic_severity {
-                                                return None;
-                                            }
-                                        }
-                                    };
-
-                                    let code = match diagnostic.code.clone() {
-                                        Some(x) => match x {
-                                            lsp::NumberOrString::Number(x) => {
-                                                Some(NumberOrString::Number(x))
-                                            }
-                                            lsp::NumberOrString::String(x) => {
-                                                Some(NumberOrString::String(x))
-                                            }
-                                        },
-                                        None => None,
-                                    };
-
-                                    let tags = if let Some(tags) = &diagnostic.tags {
-                                        let new_tags = tags
-                                            .iter()
-                                            .filter_map(|tag| match *tag {
-                                                lsp::DiagnosticTag::DEPRECATED => {
-                                                    Some(DiagnosticTag::Deprecated)
-                                                }
-                                                lsp::DiagnosticTag::UNNECESSARY => {
-                                                    Some(DiagnosticTag::Unnecessary)
-                                                }
-                                                _ => None,
-                                            })
-                                            .collect();
-
-                                        new_tags
-                                    } else {
-                                        Vec::new()
-                                    };
-
-                                    let ends_at_word = start != end
-                                        && end != 0
-                                        && text.get_char(end - 1).map_or(false, char_is_word);
-                                    let starts_at_word = start != end
-                                        && text.get_char(start).map_or(false, char_is_word);
-
-                                    Some(Diagnostic {
-                                        range: Range { start, end },
-                                        ends_at_word,
-                                        starts_at_word,
-                                        zero_width: start == end,
-                                        line: diagnostic.range.start.line as usize,
-                                        message: diagnostic.message.clone(),
-                                        severity,
-                                        code,
-                                        tags,
-                                        source: diagnostic.source.clone(),
-                                        data: diagnostic.data.clone(),
-                                        language_server_id: server_id,
-                                    })
-                                });
-
-                            doc.replace_diagnostics(diagnostics, unchaged_diag_sources, server_id);
                         }
 
                         let diagnostics = params.diagnostics.into_iter().map(|d| (d, server_id));
@@ -910,6 +809,27 @@ impl Application {
                         diagnostics.sort_unstable_by_key(|(d, server_id)| {
                             (d.severity, d.range.start, *server_id)
                         });
+
+                        if let Some(doc) = doc {
+                            let diagnostic_of_language_server_and_not_in_unchanged_sources =
+                                |diagnostic: &lsp::Diagnostic, ls_id| {
+                                    ls_id == server_id
+                                        && diagnostic.source.as_ref().map_or(true, |source| {
+                                            !unchanged_diag_sources.contains(source)
+                                        })
+                                };
+                            let diagnostics = Editor::doc_diagnostics_with_filter(
+                                &self.editor.language_servers,
+                                &self.editor.diagnostics,
+                                doc,
+                                diagnostic_of_language_server_and_not_in_unchanged_sources,
+                            );
+                            doc.replace_diagnostics(
+                                diagnostics,
+                                &unchanged_diag_sources,
+                                Some(server_id),
+                            );
+                        }
                     }
                     Notification::ShowMessage(params) => {
                         log::warn!("unhandled window/showMessage: {:?}", params);
@@ -1017,7 +937,7 @@ impl Application {
 
                         // Clear any diagnostics for documents with this server open.
                         for doc in self.editor.documents_mut() {
-                            doc.clear_diagnostics(server_id);
+                            doc.clear_diagnostics(Some(server_id));
                         }
 
                         // Remove the language server from the registry.

helix-term/src/commands.rs

@@ -3350,7 +3350,7 @@ fn exit_select_mode(cx: &mut Context) {
 
 fn goto_first_diag(cx: &mut Context) {
     let (view, doc) = current!(cx.editor);
-    let selection = match doc.shown_diagnostics().next() {
+    let selection = match doc.diagnostics().first() {
         Some(diag) => Selection::single(diag.range.start, diag.range.end),
         None => return,
     };
@@ -3359,7 +3359,7 @@ fn goto_first_diag(cx: &mut Context) {
 
 fn goto_last_diag(cx: &mut Context) {
     let (view, doc) = current!(cx.editor);
-    let selection = match doc.shown_diagnostics().last() {
+    let selection = match doc.diagnostics().last() {
         Some(diag) => Selection::single(diag.range.start, diag.range.end),
         None => return,
     };
@@ -3375,9 +3375,10 @@ fn goto_next_diag(cx: &mut Context) {
         .cursor(doc.text().slice(..));
 
     let diag = doc
-        .shown_diagnostics()
+        .diagnostics()
+        .iter()
         .find(|diag| diag.range.start > cursor_pos)
-        .or_else(|| doc.shown_diagnostics().next());
+        .or_else(|| doc.diagnostics().first());
 
     let selection = match diag {
         Some(diag) => Selection::single(diag.range.start, diag.range.end),
@@ -3395,10 +3396,11 @@ fn goto_prev_diag(cx: &mut Context) {
         .cursor(doc.text().slice(..));
 
     let diag = doc
-        .shown_diagnostics()
+        .diagnostics()
+        .iter()
         .rev()
         .find(|diag| diag.range.start < cursor_pos)
-        .or_else(|| doc.shown_diagnostics().last());
+        .or_else(|| doc.diagnostics().last());
 
     let selection = match diag {
         // NOTE: the selection is reversed because we're jumping to the
@@ -4185,9 +4187,13 @@ fn replace_with_yanked(cx: &mut Context) {
 }
 
 fn replace_with_yanked_impl(editor: &mut Editor, register: char, count: usize) {
-    let Some(values) = editor.registers
+    let Some(values) = editor
+        .registers
         .read(register, editor)
-        .filter(|values| values.len() > 0) else { return };
+        .filter(|values| values.len() > 0)
+    else {
+        return;
+    };
     let values: Vec<_> = values.map(|value| value.to_string()).collect();
 
     let (view, doc) = current!(editor);
@@ -4224,7 +4230,9 @@ fn replace_selections_with_primary_clipboard(cx: &mut Context) {
 }
 
 fn paste(editor: &mut Editor, register: char, pos: Paste, count: usize) {
-    let Some(values) = editor.registers.read(register, editor) else { return };
+    let Some(values) = editor.registers.read(register, editor) else {
+        return;
+    };
     let values: Vec<_> = values.map(|value| value.to_string()).collect();
 
     let (view, doc) = current!(editor);

helix-term/src/commands/typed.rs

@@ -1502,7 +1502,7 @@ fn lsp_stop(
 
         for doc in cx.editor.documents_mut() {
             if let Some(client) = doc.remove_language_server_by_name(ls_name) {
-                doc.clear_diagnostics(client.id());
+                doc.clear_diagnostics(Some(client.id()));
             }
         }
     }
@@ -2008,6 +2008,10 @@ fn language(
 
     let id = doc.id();
     cx.editor.refresh_language_servers(id);
+    let doc = doc_mut!(cx.editor);
+    let diagnostics =
+        Editor::doc_diagnostics(&cx.editor.language_servers, &cx.editor.diagnostics, doc);
+    doc.replace_diagnostics(diagnostics, &[], None);
     Ok(())
 }
 

helix-term/src/ui/editor.rs

@@ -386,7 +386,7 @@ impl EditorView {
         let mut warning_vec = Vec::new();
         let mut error_vec = Vec::new();
 
-        for diagnostic in doc.shown_diagnostics() {
+        for diagnostic in doc.diagnostics() {
             // Separate diagnostics into different Vecs by severity.
             let (vec, scope) = match diagnostic.severity {
                 Some(Severity::Info) => (&mut info_vec, info),
@@ -684,7 +684,7 @@ impl EditorView {
             .primary()
             .cursor(doc.text().slice(..));
 
-        let diagnostics = doc.shown_diagnostics().filter(|diagnostic| {
+        let diagnostics = doc.diagnostics().iter().filter(|diagnostic| {
             diagnostic.range.start <= cursor && diagnostic.range.end >= cursor
         });
 

helix-term/src/ui/picker.rs

@@ -480,8 +480,7 @@ impl<T: Item + 'static> Picker<T> {
                                 .find::<Overlay<DynamicPicker<T>>>()
                                 .map(|overlay| &mut overlay.content.file_picker),
                         };
-                        let Some(picker) = picker
+                        let Some(picker) = picker else {
-                        else {
                             log::info!("picker closed before syntax highlighting finished");
                             return;
                         };
@@ -489,7 +488,15 @@ impl<T: Item + 'static> Picker<T> {
                         let doc = match current_file {
                             PathOrId::Id(doc_id) => doc_mut!(editor, &doc_id),
                             PathOrId::Path(path) => match picker.preview_cache.get_mut(&path) {
-                                Some(CachedPreview::Document(ref mut doc)) => doc,
+                                Some(CachedPreview::Document(ref mut doc)) => {
+                                    let diagnostics = Editor::doc_diagnostics(
+                                        &editor.language_servers,
+                                        &editor.diagnostics,
+                                        doc,
+                                    );
+                                    doc.replace_diagnostics(diagnostics, &[], None);
+                                    doc
+                                }
                                 _ => return,
                             },
                         };

helix-term/src/ui/statusline.rs

@@ -227,7 +227,8 @@ where
 {
     let (warnings, errors) = context
         .doc
-        .shown_diagnostics()
+        .diagnostics()
+        .iter()
         .fold((0, 0), |mut counts, diag| {
             use helix_core::diagnostic::Severity;
             match diag.severity {

helix-view/src/document.rs

@@ -4,10 +4,12 @@ use arc_swap::ArcSwap;
 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::doc_formatter::TextFormat;
 use helix_core::encoding::Encoding;
 use helix_core::syntax::{Highlight, LanguageServerFeature};
 use helix_core::text_annotations::{InlineAnnotation, TextAnnotations};
+use helix_lsp::util::lsp_pos_to_pos;
 use helix_vcs::{DiffHandle, DiffProviderRegistry};
 
 use ::parking_lot::Mutex;
@@ -1075,14 +1077,6 @@ impl Document {
         };
     }
 
-    /// Set the programming language for the file if you know the name (scope) but don't have the
-    /// [`syntax::LanguageConfiguration`] for it.
-    pub fn set_language2(&mut self, scope: &str, config_loader: Arc<syntax::Loader>) {
-        let language_config = config_loader.language_config_for_scope(scope);
-
-        self.set_language(language_config, Some(config_loader));
-    }
-
     /// Set the programming language for the file if you know the language but don't have the
     /// [`syntax::LanguageConfiguration`] for it.
     pub fn set_language_by_language_id(
@@ -1714,29 +1708,107 @@ impl Document {
         )
     }
 
+    pub fn lsp_diagnostic_to_diagnostic(
+        text: &Rope,
+        language_config: Option<&LanguageConfiguration>,
+        diagnostic: &helix_lsp::lsp::Diagnostic,
+        language_server_id: usize,
+        offset_encoding: helix_lsp::OffsetEncoding,
+    ) -> Option<Diagnostic> {
+        use helix_core::diagnostic::{Range, Severity::*};
+
+        // TODO: convert inside server
+        let start =
+            if let Some(start) = lsp_pos_to_pos(text, diagnostic.range.start, offset_encoding) {
+                start
+            } else {
+                log::warn!("lsp position out of bounds - {:?}", diagnostic);
+                return None;
+            };
+
+        let end = if let Some(end) = lsp_pos_to_pos(text, diagnostic.range.end, offset_encoding) {
+            end
+        } else {
+            log::warn!("lsp position out of bounds - {:?}", diagnostic);
+            return None;
+        };
+
+        let severity = diagnostic.severity.map(|severity| match severity {
+            lsp::DiagnosticSeverity::ERROR => Error,
+            lsp::DiagnosticSeverity::WARNING => Warning,
+            lsp::DiagnosticSeverity::INFORMATION => Info,
+            lsp::DiagnosticSeverity::HINT => Hint,
+            severity => unreachable!("unrecognized diagnostic severity: {:?}", severity),
+        });
+
+        if let Some(lang_conf) = language_config {
+            if let Some(severity) = severity {
+                if severity < lang_conf.diagnostic_severity {
+                    return None;
+                }
+            }
+        };
+        use helix_core::diagnostic::{DiagnosticTag, NumberOrString};
+
+        let code = match diagnostic.code.clone() {
+            Some(x) => match x {
+                lsp::NumberOrString::Number(x) => Some(NumberOrString::Number(x)),
+                lsp::NumberOrString::String(x) => Some(NumberOrString::String(x)),
+            },
+            None => None,
+        };
+
+        let tags = if let Some(tags) = &diagnostic.tags {
+            let new_tags = tags
+                .iter()
+                .filter_map(|tag| match *tag {
+                    lsp::DiagnosticTag::DEPRECATED => Some(DiagnosticTag::Deprecated),
+                    lsp::DiagnosticTag::UNNECESSARY => Some(DiagnosticTag::Unnecessary),
+                    _ => None,
+                })
+                .collect();
+
+            new_tags
+        } else {
+            Vec::new()
+        };
+
+        let ends_at_word =
+            start != end && end != 0 && text.get_char(end - 1).map_or(false, char_is_word);
+        let starts_at_word = start != end && text.get_char(start).map_or(false, char_is_word);
+
+        Some(Diagnostic {
+            range: Range { start, end },
+            ends_at_word,
+            starts_at_word,
+            zero_width: start == end,
+            line: diagnostic.range.start.line as usize,
+            message: diagnostic.message.clone(),
+            severity,
+            code,
+            tags,
+            source: diagnostic.source.clone(),
+            data: diagnostic.data.clone(),
+            language_server_id,
+        })
+    }
+
     #[inline]
     pub fn diagnostics(&self) -> &[Diagnostic] {
         &self.diagnostics
     }
 
-    pub fn shown_diagnostics(&self) -> impl Iterator<Item = &Diagnostic> + DoubleEndedIterator {
-        self.diagnostics.iter().filter(|d| {
-            self.language_servers_with_feature(LanguageServerFeature::Diagnostics)
-                .any(|ls| ls.id() == d.language_server_id)
-        })
-    }
-
     pub fn replace_diagnostics(
         &mut self,
         diagnostics: impl IntoIterator<Item = Diagnostic>,
         unchanged_sources: &[String],
-        language_server_id: usize,
+        language_server_id: Option<usize>,
     ) {
         if unchanged_sources.is_empty() {
             self.clear_diagnostics(language_server_id);
         } else {
             self.diagnostics.retain(|d| {
-                if d.language_server_id != language_server_id {
+                if language_server_id.map_or(false, |id| id != d.language_server_id) {
                     return true;
                 }
 
@@ -1757,9 +1829,13 @@ impl Document {
         });
     }
 
-    pub fn clear_diagnostics(&mut self, language_server_id: usize) {
+    /// clears diagnostics for a given language server id if set, otherwise all diagnostics are cleared
-        self.diagnostics
+    pub fn clear_diagnostics(&mut self, language_server_id: Option<usize>) {
-            .retain(|d| d.language_server_id != language_server_id);
+        if let Some(id) = language_server_id {
+            self.diagnostics.retain(|d| d.language_server_id != id);
+        } else {
+            self.diagnostics.clear();
+        }
     }
 
     /// Get the document's auto pairs. If the document has a recognized

helix-view/src/editor.rs

@@ -42,7 +42,7 @@ use anyhow::{anyhow, bail, Error};
 pub use helix_core::diagnostic::Severity;
 use helix_core::{
     auto_pairs::AutoPairs,
-    syntax::{self, AutoPairConfig, IndentationHeuristic, SoftWrap},
+    syntax::{self, AutoPairConfig, IndentationHeuristic, LanguageServerFeature, SoftWrap},
     Change, LineEnding, Position, Selection, NATIVE_LINE_ENDING,
 };
 use helix_dap as dap;
@@ -1477,6 +1477,10 @@ impl Editor {
                 self.config.clone(),
             )?;
 
+            let diagnostics =
+                Editor::doc_diagnostics(&self.language_servers, &self.diagnostics, &doc);
+            doc.replace_diagnostics(diagnostics, &[], None);
+
             if let Some(diff_base) = self.diff_providers.get_diff_base(&path) {
                 doc.set_diff_base(diff_base);
             }
@@ -1706,6 +1710,60 @@ impl Editor {
             .find(|doc| doc.path().map(|p| p == path.as_ref()).unwrap_or(false))
     }
 
+    /// Returns all supported diagnostics for the document
+    pub fn doc_diagnostics<'a>(
+        language_servers: &'a helix_lsp::Registry,
+        diagnostics: &'a BTreeMap<lsp::Url, Vec<(lsp::Diagnostic, usize)>>,
+        document: &Document,
+    ) -> impl Iterator<Item = helix_core::Diagnostic> + 'a {
+        Editor::doc_diagnostics_with_filter(language_servers, diagnostics, document, |_, _| true)
+    }
+
+    /// Returns all supported diagnostics for the document
+    /// 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<lsp::Url, Vec<(lsp::Diagnostic, usize)>>,
+
+        document: &Document,
+        filter: impl Fn(&lsp::Diagnostic, usize) -> bool + 'a,
+    ) -> impl Iterator<Item = helix_core::Diagnostic> + 'a {
+        let text = document.text().clone();
+        let language_config = document.language.clone();
+        document
+            .path()
+            .and_then(|path| url::Url::from_file_path(path).ok()) // TODO log error?
+            .and_then(|uri| diagnostics.get(&uri))
+            .map(|diags| {
+                diags.iter().filter_map(move |(diagnostic, lsp_id)| {
+                    let ls = language_servers.get_by_id(*lsp_id)?;
+                    language_config
+                        .as_ref()
+                        .and_then(|c| {
+                            c.language_servers.iter().find(|features| {
+                                features.name == ls.name()
+                                    && features.has_feature(LanguageServerFeature::Diagnostics)
+                            })
+                        })
+                        .and_then(|_| {
+                            if filter(diagnostic, *lsp_id) {
+                                Document::lsp_diagnostic_to_diagnostic(
+                                    &text,
+                                    language_config.as_deref(),
+                                    diagnostic,
+                                    *lsp_id,
+                                    ls.offset_encoding(),
+                                )
+                            } else {
+                                None
+                            }
+                        })
+                })
+            })
+            .into_iter()
+            .flatten()
+    }
+
     /// Gets the primary cursor position in screen coordinates,
     /// or `None` if the primary cursor is not visible on screen.
     pub fn cursor(&self) -> (Option<Position>, CursorKind) {