feat: show LSP diagnostic related information

noir-lang · Oct 10, 2024 · 3aed172 · 3aed172
1 parent a8bcae2
commit 3aed172
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 17 deletions.
diff --git a/compiler/noirc_errors/src/reporter.rs b/compiler/noirc_errors/src/reporter.rs
@@ -10,7 +10,7 @@ use codespan_reporting::term::termcolor::{ColorChoice, StandardStream};
 pub struct CustomDiagnostic {
     pub message: String,
     pub secondaries: Vec<CustomLabel>,
-    notes: Vec<String>,
+    pub notes: Vec<String>,
     pub kind: DiagnosticKind,
     pub deprecated: bool,
     pub unnecessary: bool,

diff --git a/tooling/lsp/src/notifications/mod.rs b/tooling/lsp/src/notifications/mod.rs
@@ -8,7 +8,7 @@ use crate::{
 use async_lsp::{ErrorCode, LanguageClient, ResponseError};
 use fm::{FileManager, FileMap};
 use fxhash::FxHashMap as HashMap;
-use lsp_types::{DiagnosticTag, Url};
+use lsp_types::{DiagnosticRelatedInformation, DiagnosticTag, Url};
 use noirc_driver::check_crate;
 use noirc_errors::{DiagnosticKind, FileDiagnostic};
 
@@ -195,12 +195,18 @@ fn publish_diagnostics(
 
     for file_diagnostic in file_diagnostics.into_iter() {
         let file_id = file_diagnostic.file_id;
-        let diagnostic = file_diagnostic_to_diagnostic(file_diagnostic, files);
-
         let path = fm.path(file_id).expect("file must exist to have emitted diagnostic");
-        if let Ok(uri) = Url::from_file_path(path) {
-            diagnostics_per_url.entry(uri).or_default().push(diagnostic);
-        }
+        let Ok(uri) = Url::from_file_path(path) else {
+            continue;
+        };
+
+        let Some(diagnostic) =
+            file_diagnostic_to_diagnostic(file_diagnostic, files, fm, uri.clone())
+        else {
+            continue;
+        };
+
+        diagnostics_per_url.entry(uri).or_default().push(diagnostic);
     }
 
     let new_files_with_errors: HashSet<_> = diagnostics_per_url.keys().cloned().collect();
@@ -228,17 +234,21 @@ fn publish_diagnostics(
     state.files_with_errors.insert(package_root_dir.clone(), new_files_with_errors);
 }
 
-fn file_diagnostic_to_diagnostic(file_diagnostic: FileDiagnostic, files: &FileMap) -> Diagnostic {
+fn file_diagnostic_to_diagnostic(
+    file_diagnostic: FileDiagnostic,
+    files: &FileMap,
+    fm: &FileManager,
+    uri: Url,
+) -> Option<Diagnostic> {
     let file_id = file_diagnostic.file_id;
     let diagnostic = file_diagnostic.diagnostic;
 
-    // TODO: Should this be the first item in secondaries? Should we bail when we find a range?
-    let range = diagnostic
-        .secondaries
-        .into_iter()
-        .filter_map(|sec| byte_span_to_range(files, file_id, sec.span.into()))
-        .last()
-        .unwrap_or_default();
+    if diagnostic.secondaries.is_empty() {
+        return None;
+    }
+
+    let span = diagnostic.secondaries.first().unwrap().span;
+    let range = byte_span_to_range(files, file_id, span.into())?;
 
     let severity = match diagnostic.kind {
         DiagnosticKind::Error => DiagnosticSeverity::ERROR,
@@ -255,13 +265,45 @@ fn file_diagnostic_to_diagnostic(file_diagnostic: FileDiagnostic, files: &FileMa
         tags.push(DiagnosticTag::DEPRECATED);
     }
 
-    Diagnostic {
+    let mut related_information = Vec::new();
+
+    for secondary in diagnostic.secondaries {
+        let secondary_file = secondary.file.unwrap_or(file_id);
+        let Some(path) = fm.path(secondary_file) else {
+            continue;
+        };
+        let Ok(uri) = Url::from_file_path(path) else {
+            continue;
+        };
+        let Some(range) = byte_span_to_range(files, file_id, secondary.span.into()) else {
+            continue;
+        };
+
+        related_information.push(DiagnosticRelatedInformation {
+            location: lsp_types::Location { uri, range },
+            message: secondary.message,
+        });
+    }
+
+    for note in diagnostic.notes {
+        related_information.push(DiagnosticRelatedInformation {
+            location: lsp_types::Location { uri: uri.clone(), range },
+            message: note,
+        });
+    }
+
+    Some(Diagnostic {
         range,
         severity: Some(severity),
         message: diagnostic.message,
         tags: if tags.is_empty() { None } else { Some(tags) },
+        related_information: if related_information.is_empty() {
+            None
+        } else {
+            Some(related_information)
+        },
         ..Default::default()
-    }
+    })
 }
 
 pub(super) fn on_exit(