Merge pull request #626 from zed-industries/inactive-code-diagnostics

Antonio Scandurra created

De-emphasize unnecessary code diagnostics

Change summary

crates/diagnostics/src/diagnostics.rs      |  9 +++++
crates/editor/src/display_map/block_map.rs |  3 +
crates/editor/src/display_map/fold_map.rs  |  3 +
crates/editor/src/editor.rs                |  1 
crates/editor/src/element.rs               | 31 ++++++++++++---------
crates/language/src/buffer.rs              | 27 ++++++++++++++++--
crates/language/src/proto.rs               |  2 +
crates/project/src/project.rs              | 34 ++++++++++++++---------
crates/rpc/proto/zed.proto                 |  1 
crates/rpc/src/rpc.rs                      |  2 
crates/theme/src/theme.rs                  |  1 
crates/zed/assets/themes/_base.toml        |  1 
12 files changed, 81 insertions(+), 34 deletions(-)

Detailed changes

crates/diagnostics/src/diagnostics.rs 🔗

@@ -218,7 +218,14 @@ impl ProjectDiagnosticsEditor {
         let mut first_excerpt_id = None;
         let excerpts_snapshot = self.excerpts.update(cx, |excerpts, excerpts_cx| {
             let mut old_groups = path_state.diagnostic_groups.iter().enumerate().peekable();
-            let mut new_groups = snapshot.diagnostic_groups().into_iter().peekable();
+            let mut new_groups = snapshot
+                .diagnostic_groups()
+                .into_iter()
+                .filter(|group| {
+                    group.entries[group.primary_ix].diagnostic.severity
+                        <= DiagnosticSeverity::WARNING
+                })
+                .peekable();
             loop {
                 let mut to_insert = None;
                 let mut to_remove = None;

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

@@ -819,7 +819,8 @@ impl<'a> Iterator for BlockChunks<'a> {
                 text: unsafe { std::str::from_utf8_unchecked(&NEWLINES[..line_count as usize]) },
                 syntax_highlight_id: None,
                 highlight_style: None,
-                diagnostic: None,
+                diagnostic_severity: None,
+                is_unnecessary: false,
             });
         }
 

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

@@ -1078,7 +1078,8 @@ impl<'a> Iterator for FoldChunks<'a> {
                 text: output_text,
                 syntax_highlight_id: None,
                 highlight_style: None,
-                diagnostic: None,
+                diagnostic_severity: None,
+                is_unnecessary: false,
             });
         }
 

crates/editor/src/editor.rs 🔗

@@ -4211,6 +4211,7 @@ impl Editor {
             };
             let group = diagnostics.find_map(|entry| {
                 if entry.diagnostic.is_primary
+                    && !entry.diagnostic.is_unnecessary
                     && !entry.range.is_empty()
                     && Some(entry.range.end) != active_primary_range.as_ref().map(|r| *r.end())
                 {

crates/editor/src/element.rs 🔗

@@ -665,22 +665,25 @@ impl EditorElement {
                     }
                 }
 
-                if let Some(severity) = chunk.diagnostic {
+                let mut diagnostic_highlight = HighlightStyle {
+                    ..Default::default()
+                };
+
+                if chunk.is_unnecessary {
+                    diagnostic_highlight.fade_out = Some(style.unnecessary_code_fade);
+                } else if let Some(severity) = chunk.diagnostic_severity {
                     let diagnostic_style = super::diagnostic_style(severity, true, style);
-                    let diagnostic_highlight = HighlightStyle {
-                        underline: Some(Underline {
-                            color: Some(diagnostic_style.message.text.color),
-                            thickness: 1.0.into(),
-                            squiggly: true,
-                        }),
-                        ..Default::default()
-                    };
+                    diagnostic_highlight.underline = Some(Underline {
+                        color: Some(diagnostic_style.message.text.color),
+                        thickness: 1.0.into(),
+                        squiggly: true,
+                    });
+                }
 
-                    if let Some(highlight_style) = highlight_style.as_mut() {
-                        highlight_style.highlight(diagnostic_highlight);
-                    } else {
-                        highlight_style = Some(diagnostic_highlight);
-                    }
+                if let Some(highlight_style) = highlight_style.as_mut() {
+                    highlight_style.highlight(diagnostic_highlight);
+                } else {
+                    highlight_style = Some(diagnostic_highlight);
                 }
 
                 (chunk.text, highlight_style)

crates/language/src/buffer.rs 🔗

@@ -105,6 +105,7 @@ pub struct Diagnostic {
     pub is_valid: bool,
     pub is_primary: bool,
     pub is_disk_based: bool,
+    pub is_unnecessary: bool,
 }
 
 #[derive(Clone, Debug)]
@@ -240,6 +241,7 @@ pub struct BufferChunks<'a> {
     warning_depth: usize,
     information_depth: usize,
     hint_depth: usize,
+    unnecessary_depth: usize,
     highlights: Option<BufferChunkHighlights<'a>>,
 }
 
@@ -248,7 +250,8 @@ pub struct Chunk<'a> {
     pub text: &'a str,
     pub syntax_highlight_id: Option<HighlightId>,
     pub highlight_style: Option<HighlightStyle>,
-    pub diagnostic: Option<DiagnosticSeverity>,
+    pub diagnostic_severity: Option<DiagnosticSeverity>,
+    pub is_unnecessary: bool,
 }
 
 pub(crate) struct Diff {
@@ -263,6 +266,7 @@ pub(crate) struct DiagnosticEndpoint {
     offset: usize,
     is_start: bool,
     severity: DiagnosticSeverity,
+    is_unnecessary: bool,
 }
 
 #[derive(Copy, Clone, Eq, PartialEq, PartialOrd, Ord, Debug)]
@@ -1580,11 +1584,13 @@ impl BufferSnapshot {
                     offset: entry.range.start,
                     is_start: true,
                     severity: entry.diagnostic.severity,
+                    is_unnecessary: entry.diagnostic.is_unnecessary,
                 });
                 diagnostic_endpoints.push(DiagnosticEndpoint {
                     offset: entry.range.end,
                     is_start: false,
                     severity: entry.diagnostic.severity,
+                    is_unnecessary: entry.diagnostic.is_unnecessary,
                 });
             }
             diagnostic_endpoints
@@ -1976,6 +1982,7 @@ impl<'a> BufferChunks<'a> {
             warning_depth: 0,
             information_depth: 0,
             hint_depth: 0,
+            unnecessary_depth: 0,
             highlights,
         }
     }
@@ -2021,9 +2028,17 @@ impl<'a> BufferChunks<'a> {
         } else {
             *depth -= 1;
         }
+
+        if endpoint.is_unnecessary {
+            if endpoint.is_start {
+                self.unnecessary_depth += 1;
+            } else {
+                self.unnecessary_depth -= 1;
+            }
+        }
     }
 
-    fn current_diagnostic_severity(&mut self) -> Option<DiagnosticSeverity> {
+    fn current_diagnostic_severity(&self) -> Option<DiagnosticSeverity> {
         if self.error_depth > 0 {
             Some(DiagnosticSeverity::ERROR)
         } else if self.warning_depth > 0 {
@@ -2036,6 +2051,10 @@ impl<'a> BufferChunks<'a> {
             None
         }
     }
+
+    fn current_code_is_unnecessary(&self) -> bool {
+        self.unnecessary_depth > 0
+    }
 }
 
 impl<'a> Iterator for BufferChunks<'a> {
@@ -2107,7 +2126,8 @@ impl<'a> Iterator for BufferChunks<'a> {
                 text: slice,
                 syntax_highlight_id: highlight_id,
                 highlight_style: None,
-                diagnostic: self.current_diagnostic_severity(),
+                diagnostic_severity: self.current_diagnostic_severity(),
+                is_unnecessary: self.current_code_is_unnecessary(),
             })
         } else {
             None
@@ -2193,6 +2213,7 @@ impl Default for Diagnostic {
             is_primary: Default::default(),
             is_valid: true,
             is_disk_based: false,
+            is_unnecessary: false,
         }
     }
 }

crates/language/src/proto.rs 🔗

@@ -132,6 +132,7 @@ pub fn serialize_diagnostics<'a>(
             is_valid: entry.diagnostic.is_valid,
             code: entry.diagnostic.code.clone(),
             is_disk_based: entry.diagnostic.is_disk_based,
+            is_unnecessary: entry.diagnostic.is_unnecessary,
         })
         .collect()
 }
@@ -308,6 +309,7 @@ pub fn deserialize_diagnostics(
                     is_valid: diagnostic.is_valid,
                     is_primary: diagnostic.is_primary,
                     is_disk_based: diagnostic.is_disk_based,
+                    is_unnecessary: diagnostic.is_unnecessary,
                 },
             })
         })

crates/project/src/project.rs 🔗

@@ -21,7 +21,7 @@ use language::{
     LocalFile, OffsetRangeExt, Operation, PointUtf16, TextBufferSnapshot, ToLspPosition, ToOffset,
     ToPointUtf16, Transaction,
 };
-use lsp::{DiagnosticSeverity, DocumentHighlightKind, LanguageServer};
+use lsp::{DiagnosticSeverity, DiagnosticTag, DocumentHighlightKind, LanguageServer};
 use lsp_command::*;
 use parking_lot::Mutex;
 use postage::watch;
@@ -1621,7 +1621,7 @@ impl Project {
         let mut diagnostics = Vec::default();
         let mut primary_diagnostic_group_ids = HashMap::default();
         let mut sources_by_group_id = HashMap::default();
-        let mut supporting_diagnostic_severities = HashMap::default();
+        let mut supporting_diagnostics = HashMap::default();
         for diagnostic in &params.diagnostics {
             let source = diagnostic.source.as_ref();
             let code = diagnostic.code.as_ref().map(|code| match code {
@@ -1642,11 +1642,15 @@ impl Project {
                     })
                 });
 
+            let is_unnecessary = diagnostic.tags.as_ref().map_or(false, |tags| {
+                tags.iter().any(|tag| *tag == DiagnosticTag::UNNECESSARY)
+            });
+
             if is_supporting {
-                if let Some(severity) = diagnostic.severity {
-                    supporting_diagnostic_severities
-                        .insert((source, code.clone(), range), severity);
-                }
+                supporting_diagnostics.insert(
+                    (source, code.clone(), range),
+                    (diagnostic.severity, is_unnecessary),
+                );
             } else {
                 let group_id = post_inc(&mut next_group_id);
                 let is_disk_based =
@@ -1666,6 +1670,7 @@ impl Project {
                         is_primary: true,
                         is_valid: true,
                         is_disk_based,
+                        is_unnecessary,
                     },
                 });
                 if let Some(infos) = &diagnostic.related_information {
@@ -1682,6 +1687,7 @@ impl Project {
                                     is_primary: false,
                                     is_valid: true,
                                     is_disk_based,
+                                    is_unnecessary: false,
                                 },
                             });
                         }
@@ -1694,12 +1700,15 @@ impl Project {
             let diagnostic = &mut entry.diagnostic;
             if !diagnostic.is_primary {
                 let source = *sources_by_group_id.get(&diagnostic.group_id).unwrap();
-                if let Some(&severity) = supporting_diagnostic_severities.get(&(
+                if let Some(&(severity, is_unnecessary)) = supporting_diagnostics.get(&(
                     source,
                     diagnostic.code.clone(),
                     entry.range.clone(),
                 )) {
-                    diagnostic.severity = severity;
+                    if let Some(severity) = severity {
+                        diagnostic.severity = severity;
+                    }
+                    diagnostic.is_unnecessary = is_unnecessary;
                 }
             }
         }
@@ -5568,13 +5577,12 @@ mod tests {
     ) -> Vec<(String, Option<DiagnosticSeverity>)> {
         let mut chunks: Vec<(String, Option<DiagnosticSeverity>)> = Vec::new();
         for chunk in buffer.snapshot().chunks(range, true) {
-            if chunks
-                .last()
-                .map_or(false, |prev_chunk| prev_chunk.1 == chunk.diagnostic)
-            {
+            if chunks.last().map_or(false, |prev_chunk| {
+                prev_chunk.1 == chunk.diagnostic_severity
+            }) {
                 chunks.last_mut().unwrap().0.push_str(chunk.text);
             } else {
-                chunks.push((chunk.text.to_string(), chunk.diagnostic));
+                chunks.push((chunk.text.to_string(), chunk.diagnostic_severity));
             }
         }
         chunks

crates/rpc/proto/zed.proto 🔗

@@ -630,6 +630,7 @@ message Diagnostic {
     bool is_primary = 7;
     bool is_valid = 8;
     bool is_disk_based = 9;
+    bool is_unnecessary = 10;
 
     enum Severity {
         None = 0;

crates/rpc/src/rpc.rs 🔗

@@ -5,4 +5,4 @@ pub mod proto;
 pub use conn::Connection;
 pub use peer::*;
 
-pub const PROTOCOL_VERSION: u32 = 10;
+pub const PROTOCOL_VERSION: u32 = 11;

crates/theme/src/theme.rs 🔗

@@ -303,6 +303,7 @@ pub struct Editor {
     pub invalid_hint_diagnostic: DiagnosticStyle,
     pub autocomplete: AutocompleteStyle,
     pub code_actions_indicator: Color,
+    pub unnecessary_code_fade: f32,
 }
 
 #[derive(Clone, Deserialize, Default)]

crates/zed/assets/themes/_base.toml 🔗

@@ -250,6 +250,7 @@ gutter_padding_factor = 2.5
 active_line_background = "$state.active_line"
 highlighted_line_background = "$state.highlighted_line"
 rename_fade = 0.6
+unnecessary_code_fade = 0.5
 document_highlight_read_background = "#99999920"
 document_highlight_write_background = "#99999916"
 diff_background_deleted = "$state.deleted_line"