feat: LSP diagnostics now have "unnecessary" and "deprecated" tags (#…

…5878) # Description ## Problem Browsing the LSP docs I found out that diagnostics can have tags, like "unnecessary" or "deprecated". I thought it would be nice to use those. ## Summary Before: ![before](https://github.com/user-attachments/assets/487400f9-d0fc-4818-bf0e-f31e2617320f) After: ![after](https://github.com/user-attachments/assets/33112342-795b-4b53-8051-4654ae14beeb) ## Additional Context None ## Documentation Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings.
noir-lang · Sep 2, 2024 · 2f0d4e0 · 2f0d4e0
1 parent 410c1f6
commit 2f0d4e0
Show file tree

Hide file tree

Showing 5 changed files with 67 additions and 33 deletions.
diff --git a/compiler/noirc_errors/src/reporter.rs b/compiler/noirc_errors/src/reporter.rs
@@ -12,6 +12,8 @@ pub struct CustomDiagnostic {
     pub secondaries: Vec<CustomLabel>,
     notes: Vec<String>,
     pub kind: DiagnosticKind,
+    pub deprecated: bool,
+    pub unnecessary: bool,
 }
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
@@ -35,6 +37,8 @@ impl CustomDiagnostic {
             secondaries: Vec::new(),
             notes: Vec::new(),
             kind: DiagnosticKind::Error,
+            deprecated: false,
+            unnecessary: false,
         }
     }
 
@@ -49,6 +53,8 @@ impl CustomDiagnostic {
             secondaries: vec![CustomLabel::new(secondary_message, secondary_span, None)],
             notes: Vec::new(),
             kind,
+            deprecated: false,
+            unnecessary: false,
         }
     }
 
@@ -101,6 +107,8 @@ impl CustomDiagnostic {
             secondaries: vec![CustomLabel::new(secondary_message, secondary_span, None)],
             notes: Vec::new(),
             kind: DiagnosticKind::Bug,
+            deprecated: false,
+            unnecessary: false,
         }
     }
 

diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs
@@ -150,20 +150,24 @@ impl<'a> From<&'a ResolverError> for Diagnostic {
             ResolverError::UnusedVariable { ident } => {
                 let name = &ident.0.contents;
 
-                Diagnostic::simple_warning(
+                let mut diagnostic = Diagnostic::simple_warning(
                     format!("unused variable {name}"),
                     "unused variable ".to_string(),
                     ident.span(),
-                )
+                );
+                diagnostic.unnecessary = true;
+                diagnostic
             }
             ResolverError::UnusedImport { ident } => {
                 let name = &ident.0.contents;
 
-                Diagnostic::simple_warning(
+                let mut diagnostic = Diagnostic::simple_warning(
                     format!("unused import {name}"),
                     "unused import ".to_string(),
                     ident.span(),
-                )
+                );
+                diagnostic.unnecessary = true;
+                diagnostic
             }
             ResolverError::VariableNotDeclared { name, span } => Diagnostic::simple_error(
                 format!("cannot find `{name}` in this scope "),

diff --git a/compiler/noirc_frontend/src/hir/type_check/errors.rs b/compiler/noirc_frontend/src/hir/type_check/errors.rs
@@ -357,7 +357,9 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic {
                 let primary_message = error.to_string();
                 let secondary_message = note.clone().unwrap_or_default();
 
-                Diagnostic::simple_warning(primary_message, secondary_message, *span)
+                let mut diagnostic = Diagnostic::simple_warning(primary_message, secondary_message, *span);
+                diagnostic.deprecated = true;
+                diagnostic
             }
             TypeCheckError::UnusedResultError { expr_type, expr_span } => {
                 let msg = format!("Unused expression result of type {expr_type}");

diff --git a/compiler/noirc_frontend/src/parser/errors.rs b/compiler/noirc_frontend/src/parser/errors.rs
@@ -165,42 +165,51 @@ impl std::fmt::Display for ParserError {
 impl<'a> From<&'a ParserError> for Diagnostic {
     fn from(error: &'a ParserError) -> Diagnostic {
         match &error.reason {
-            Some(reason) => {
-                match reason {
-                    ParserErrorReason::ConstrainDeprecated => Diagnostic::simple_error(
+            Some(reason) => match reason {
+                ParserErrorReason::ConstrainDeprecated => {
+                    let mut diagnostic = Diagnostic::simple_error(
                         "Use of deprecated keyword 'constrain'".into(),
                         "The 'constrain' keyword is deprecated. Please use the 'assert' function instead.".into(),
                         error.span,
-                    ),
-                    ParserErrorReason::ComptimeDeprecated => Diagnostic::simple_warning(
+                    );
+                    diagnostic.deprecated = true;
+                    diagnostic
+                }
+                ParserErrorReason::ComptimeDeprecated => {
+                    let mut diagnostic = Diagnostic::simple_warning(
                         "Use of deprecated keyword 'comptime'".into(),
                         "The 'comptime' keyword has been deprecated. It can be removed without affecting your program".into(),
                         error.span,
+                    ) ;
+                    diagnostic.deprecated = true;
+                    diagnostic
+                }
+                ParserErrorReason::InvalidBitSize(bit_size) => Diagnostic::simple_error(
+                    format!("Use of invalid bit size {}", bit_size),
+                    format!(
+                        "Allowed bit sizes for integers are {}",
+                        IntegerBitSize::allowed_sizes()
+                            .iter()
+                            .map(|n| n.to_string())
+                            .collect::<Vec<_>>()
+                            .join(", ")
                     ),
-                    ParserErrorReason::InvalidBitSize(bit_size) => Diagnostic::simple_error(
-                        format!("Use of invalid bit size {}", bit_size),
-                        format!("Allowed bit sizes for integers are {}", IntegerBitSize::allowed_sizes().iter().map(|n| n.to_string()).collect::<Vec<_>>().join(", ")),
-                        error.span,
-                    ),
-                    ParserErrorReason::ExperimentalFeature(_) => Diagnostic::simple_warning(
-                        reason.to_string(),
-                        "".into(),
-                        error.span,
-                    ),
-                    ParserErrorReason::TraitImplFunctionModifiers => Diagnostic::simple_warning(
-                        reason.to_string(),
-                        "".into(),
-                        error.span,
-                    ),
-                    ParserErrorReason::ExpectedPatternButFoundType(ty) => {
-                        Diagnostic::simple_error("Expected a ; separating these two statements".into(), format!("{ty} is a type and cannot be used as a variable name"), error.span)
-                    }
-                    ParserErrorReason::Lexer(error) => error.into(),
-                    other => {
-                        Diagnostic::simple_error(format!("{other}"), String::new(), error.span)
-                    }
+                    error.span,
+                ),
+                ParserErrorReason::ExperimentalFeature(_) => {
+                    Diagnostic::simple_warning(reason.to_string(), "".into(), error.span)
                 }
-            }
+                ParserErrorReason::TraitImplFunctionModifiers => {
+                    Diagnostic::simple_warning(reason.to_string(), "".into(), error.span)
+                }
+                ParserErrorReason::ExpectedPatternButFoundType(ty) => Diagnostic::simple_error(
+                    "Expected a ; separating these two statements".into(),
+                    format!("{ty} is a type and cannot be used as a variable name"),
+                    error.span,
+                ),
+                ParserErrorReason::Lexer(error) => error.into(),
+                other => Diagnostic::simple_error(format!("{other}"), String::new(), error.span),
+            },
             None => {
                 let primary = error.to_string();
                 Diagnostic::simple_error(primary, String::new(), error.span)

diff --git a/tooling/lsp/src/notifications/mod.rs b/tooling/lsp/src/notifications/mod.rs
@@ -2,6 +2,7 @@ use std::ops::ControlFlow;
 
 use crate::insert_all_files_for_workspace_into_file_manager;
 use async_lsp::{ErrorCode, LanguageClient, ResponseError};
+use lsp_types::DiagnosticTag;
 use noirc_driver::{check_crate, file_manager_with_stdlib, CheckOptions};
 use noirc_errors::{DiagnosticKind, FileDiagnostic};
 
@@ -189,10 +190,20 @@ pub(crate) fn process_workspace_for_noir_document(
                             DiagnosticKind::Info => DiagnosticSeverity::INFORMATION,
                             DiagnosticKind::Bug => DiagnosticSeverity::WARNING,
                         };
+
+                        let mut tags = Vec::new();
+                        if diagnostic.unnecessary {
+                            tags.push(DiagnosticTag::UNNECESSARY);
+                        }
+                        if diagnostic.deprecated {
+                            tags.push(DiagnosticTag::DEPRECATED);
+                        }
+
                         Some(Diagnostic {
                             range,
                             severity: Some(severity),
                             message: diagnostic.message,
+                            tags: if tags.is_empty() { None } else { Some(tags) },
                             ..Default::default()
                         })
                     })