Allow LSP adapters to decide, which diagnostics to underline (#31450)

Kirill Bulatov created

Closes
https://github.com/zed-industries/zed/pull/31355#issuecomment-2910439798

<img width="1728" alt="image"
src="https://github.com/user-attachments/assets/2eaa8e9b-00bc-4e99-ac09-fceb2d932e41"
/>


Release Notes:

- N/A

Change summary

crates/editor/src/display_map.rs           |  7 ++-----
crates/editor/src/display_map/fold_map.rs  |  3 +++
crates/language/src/buffer.rs              | 12 ++++++++++++
crates/language/src/language.rs            | 14 ++++++++++++++
crates/language/src/proto.rs               |  2 ++
crates/languages/src/c.rs                  |  4 ++++
crates/project/src/lsp_store.rs            |  6 ++++++
crates/project/src/lsp_store/clangd_ext.rs |  9 +++++++++
crates/proto/proto/buffer.proto            |  1 +
9 files changed, 53 insertions(+), 5 deletions(-)

Detailed changes

crates/editor/src/display_map.rs 🔗

@@ -960,11 +960,8 @@ impl DisplaySnapshot {
             }) {
                 if chunk.is_unnecessary {
                     diagnostic_highlight.fade_out = Some(editor_style.unnecessary_code_fade);
-                // https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#diagnosticTag
-                // states that
-                // > Clients are allowed to render diagnostics with this tag faded out instead of having an error squiggle.
-                // for the unnecessary diagnostics, so do not underline them.
-                } else if editor_style.show_underlines {
+                }
+                if chunk.underline && editor_style.show_underlines {
                     let diagnostic_color = super::diagnostic_style(severity, &editor_style.status);
                     diagnostic_highlight.underline = Some(UnderlineStyle {
                         color: Some(diagnostic_color),

crates/editor/src/display_map/fold_map.rs 🔗

@@ -1255,6 +1255,8 @@ pub struct Chunk<'a> {
     pub diagnostic_severity: Option<lsp::DiagnosticSeverity>,
     /// Whether this chunk of text is marked as unnecessary.
     pub is_unnecessary: bool,
+    /// Whether this chunk of text should be underlined.
+    pub underline: bool,
     /// Whether this chunk of text was originally a tab character.
     pub is_tab: bool,
     /// An optional recipe for how the chunk should be presented.
@@ -1422,6 +1424,7 @@ impl<'a> Iterator for FoldChunks<'a> {
                 diagnostic_severity: chunk.diagnostic_severity,
                 is_unnecessary: chunk.is_unnecessary,
                 is_tab: chunk.is_tab,
+                underline: chunk.underline,
                 renderer: None,
             });
         }

crates/language/src/buffer.rs 🔗

@@ -231,6 +231,8 @@ pub struct Diagnostic {
     pub is_unnecessary: bool,
     /// Data from language server that produced this diagnostic. Passed back to the LS when we request code actions for this diagnostic.
     pub data: Option<Value>,
+    /// Whether to underline the corresponding text range in the editor.
+    pub underline: bool,
 }
 
 /// An operation used to synchronize this buffer with its other replicas.
@@ -462,6 +464,7 @@ pub struct BufferChunks<'a> {
     information_depth: usize,
     hint_depth: usize,
     unnecessary_depth: usize,
+    underline: bool,
     highlights: Option<BufferChunkHighlights<'a>>,
 }
 
@@ -482,6 +485,8 @@ pub struct Chunk<'a> {
     pub is_unnecessary: bool,
     /// Whether this chunk of text was originally a tab character.
     pub is_tab: bool,
+    /// Whether to underline the corresponding text range in the editor.
+    pub underline: bool,
 }
 
 /// A set of edits to a given version of a buffer, computed asynchronously.
@@ -496,6 +501,7 @@ pub struct Diff {
 pub(crate) struct DiagnosticEndpoint {
     offset: usize,
     is_start: bool,
+    underline: bool,
     severity: DiagnosticSeverity,
     is_unnecessary: bool,
 }
@@ -4388,6 +4394,7 @@ impl<'a> BufferChunks<'a> {
             information_depth: 0,
             hint_depth: 0,
             unnecessary_depth: 0,
+            underline: true,
             highlights,
         };
         this.initialize_diagnostic_endpoints();
@@ -4448,12 +4455,14 @@ impl<'a> BufferChunks<'a> {
                         is_start: true,
                         severity: entry.diagnostic.severity,
                         is_unnecessary: entry.diagnostic.is_unnecessary,
+                        underline: entry.diagnostic.underline,
                     });
                     diagnostic_endpoints.push(DiagnosticEndpoint {
                         offset: entry.range.end,
                         is_start: false,
                         severity: entry.diagnostic.severity,
                         is_unnecessary: entry.diagnostic.is_unnecessary,
+                        underline: entry.diagnostic.underline,
                     });
                 }
                 diagnostic_endpoints
@@ -4559,6 +4568,7 @@ impl<'a> Iterator for BufferChunks<'a> {
                 if endpoint.offset <= self.range.start {
                     self.update_diagnostic_depths(endpoint);
                     diagnostic_endpoints.next();
+                    self.underline = endpoint.underline;
                 } else {
                     next_diagnostic_endpoint = endpoint.offset;
                     break;
@@ -4590,6 +4600,7 @@ impl<'a> Iterator for BufferChunks<'a> {
             Some(Chunk {
                 text: slice,
                 syntax_highlight_id: highlight_id,
+                underline: self.underline,
                 diagnostic_severity: self.current_diagnostic_severity(),
                 is_unnecessary: self.current_code_is_unnecessary(),
                 ..Chunk::default()
@@ -4632,6 +4643,7 @@ impl Default for Diagnostic {
             is_primary: false,
             is_disk_based: false,
             is_unnecessary: false,
+            underline: true,
             data: None,
         }
     }

crates/language/src/language.rs 🔗

@@ -245,6 +245,10 @@ impl CachedLspAdapter {
         self.adapter.retain_old_diagnostic(previous_diagnostic, cx)
     }
 
+    pub fn underline_diagnostic(&self, diagnostic: &lsp::Diagnostic) -> bool {
+        self.adapter.underline_diagnostic(diagnostic)
+    }
+
     pub fn diagnostic_message_to_markdown(&self, message: &str) -> Option<String> {
         self.adapter.diagnostic_message_to_markdown(message)
     }
@@ -470,6 +474,16 @@ pub trait LspAdapter: 'static + Send + Sync {
         false
     }
 
+    /// Whether to underline a given diagnostic or not, when rendering in the editor.
+    ///
+    /// https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#diagnosticTag
+    /// states that
+    /// > Clients are allowed to render diagnostics with this tag faded out instead of having an error squiggle.
+    /// for the unnecessary diagnostics, so do not underline them.
+    fn underline_diagnostic(&self, _diagnostic: &lsp::Diagnostic) -> bool {
+        true
+    }
+
     /// Post-processes completions provided by the language server.
     async fn process_completions(&self, _: &mut [lsp::CompletionItem]) {}
 

crates/language/src/proto.rs 🔗

@@ -213,6 +213,7 @@ pub fn serialize_diagnostics<'a>(
             } as i32,
             group_id: entry.diagnostic.group_id as u64,
             is_primary: entry.diagnostic.is_primary,
+            underline: entry.diagnostic.underline,
             code: entry.diagnostic.code.as_ref().map(|s| s.to_string()),
             code_description: entry
                 .diagnostic
@@ -429,6 +430,7 @@ pub fn deserialize_diagnostics(
                     is_primary: diagnostic.is_primary,
                     is_disk_based: diagnostic.is_disk_based,
                     is_unnecessary: diagnostic.is_unnecessary,
+                    underline: diagnostic.underline,
                     data,
                 },
             })

crates/languages/src/c.rs 🔗

@@ -285,6 +285,10 @@ impl super::LspAdapter for CLspAdapter {
     fn retain_old_diagnostic(&self, previous_diagnostic: &Diagnostic, _: &App) -> bool {
         clangd_ext::is_inactive_region(previous_diagnostic)
     }
+
+    fn underline_diagnostic(&self, diagnostic: &lsp::Diagnostic) -> bool {
+        !clangd_ext::is_lsp_inactive_region(diagnostic)
+    }
 }
 
 async fn get_cached_server_binary(container_dir: PathBuf) -> Option<LanguageServerBinary> {

crates/project/src/lsp_store.rs 🔗

@@ -8782,6 +8782,10 @@ impl LspStore {
                 .as_ref()
                 .map_or(false, |tags| tags.contains(&DiagnosticTag::UNNECESSARY));
 
+            let underline = self
+                .language_server_adapter_for_id(language_server_id)
+                .map_or(true, |adapter| adapter.underline_diagnostic(diagnostic));
+
             if is_supporting {
                 supporting_diagnostics.insert(
                     (source, diagnostic.code.clone(), range),
@@ -8814,6 +8818,7 @@ impl LspStore {
                         is_primary: true,
                         is_disk_based,
                         is_unnecessary,
+                        underline,
                         data: diagnostic.data.clone(),
                     },
                 });
@@ -8839,6 +8844,7 @@ impl LspStore {
                                     is_primary: false,
                                     is_disk_based,
                                     is_unnecessary: false,
+                                    underline,
                                     data: diagnostic.data.clone(),
                                 },
                             });

crates/project/src/lsp_store/clangd_ext.rs 🔗

@@ -37,6 +37,15 @@ pub fn is_inactive_region(diag: &Diagnostic) -> bool {
             .is_some_and(|v| v == CLANGD_SERVER_NAME)
 }
 
+pub fn is_lsp_inactive_region(diag: &lsp::Diagnostic) -> bool {
+    diag.severity == Some(INACTIVE_DIAGNOSTIC_SEVERITY)
+        && diag.message == INACTIVE_REGION_MESSAGE
+        && diag
+            .source
+            .as_ref()
+            .is_some_and(|v| v == CLANGD_SERVER_NAME)
+}
+
 pub fn register_notifications(
     lsp_store: WeakEntity<LspStore>,
     language_server: &LanguageServer,

crates/proto/proto/buffer.proto 🔗

@@ -261,6 +261,7 @@ message Diagnostic {
 
     bool is_disk_based = 10;
     bool is_unnecessary = 11;
+    bool underline = 15;
 
     enum Severity {
         None = 0;