fix(grit): improved diagnostics

crates/biome_grit_patterns/src/errors.rs

@@ -7,34 +7,10 @@ use biome_parser::diagnostic::ParseDiagnostic;
 use biome_rowan::SyntaxError;
 use grit_util::ByteRange;
 
-#[derive(Debug, Diagnostic)]
-#[diagnostic(
-    category = "parse",
-    severity = Error,
-    message = "Error(s) parsing pattern",
-)]
-pub struct ParsePatternError {
-    pub diagnostics: Vec<ParseDiagnostic>,
-}
-
-#[derive(Debug, Diagnostic)]
-#[diagnostic(
-    category = "parse",
-    severity = Error,
-    message = "Error(s) parsing pattern snippet",
-)]
-pub struct ParseSnippetError {
-    diagnostics: Vec<ParseDiagnostic>,
-}
-
-// TODO: We definitely need to improve diagnostics.
 #[derive(Debug)]
 pub enum CompileError {
     /// Indicates the (top-level) pattern could not be parsed.
-    ParsePatternError(ParsePatternError),
-
-    /// Indicates one of the pattern's snippets could not be parsed.
-    ParseSnippetError(ParseSnippetError),
+    ParsePatternError(ParseDiagnostic),
 
     /// Used for missing syntax nodes.
     MissingSyntaxNode,
@@ -91,25 +67,9 @@ impl Diagnostic for CompileError {
 
     fn message(&self, fmt: &mut Formatter<'_>) -> std::io::Result<()> {
         match self {
-            CompileError::ParsePatternError(error) => {
-                fmt.write_markup(markup! { "Error parsing pattern" })?;
-                match error.diagnostics.first() {
-                    Some(diag) => {
-                        fmt.write_str(": ")?;
-                        diag.message(fmt)
-                    }
-                    None => Ok(()),
-                }
-            }
-            CompileError::ParseSnippetError(error) => {
-                fmt.write_markup(markup! { "Error parsing snippet" })?;
-                match error.diagnostics.first() {
-                    Some(diag) => {
-                        fmt.write_str(": ")?;
-                        diag.message(fmt)
-                    }
-                    None => Ok(()),
-                }
+            CompileError::ParsePatternError(diagnostic) => {
+                fmt.write_markup(markup! { "Error parsing pattern: " })?;
+                diagnostic.message(fmt)
             }
             CompileError::MissingSyntaxNode => {
                 fmt.write_markup(markup! { "A syntax node was missing" })
@@ -159,25 +119,14 @@ impl Diagnostic for CompileError {
 
     fn location(&self) -> Location<'_> {
         match self {
-            CompileError::ParsePatternError(error) => error
-                .diagnostics
-                .first()
-                .map(Diagnostic::location)
-                .unwrap_or_default(),
+            CompileError::ParsePatternError(diagnostic) => diagnostic.location(),
             _ => Location::default(),
         }
     }
 
     fn description(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
-            CompileError::ParsePatternError(error) => match error.diagnostics.first() {
-                Some(diag) => diag.description(fmt),
-                None => Ok(()),
-            },
-            CompileError::ParseSnippetError(error) => match error.diagnostics.first() {
-                Some(diag) => diag.description(fmt),
-                None => Ok(()),
-            },
+            CompileError::ParsePatternError(diagnostic) => diagnostic.description(fmt),
             CompileError::FunctionArgument(error) => error.fmt(fmt),
             _ => Ok(()),
         }

crates/biome_grit_patterns/src/lib.rs

@@ -34,9 +34,10 @@ pub fn compile_pattern(
 ) -> Result<GritQuery, CompileError> {
     let parsed = parse_grit(source);
     if parsed.has_errors() {
-        return Err(CompileError::ParsePatternError(ParsePatternError {
-            diagnostics: parsed.into_diagnostics(),
-        }));
+        return Err(CompileError::ParsePatternError(
+            // TODO: We may want to preserve other diagnostics too.
+            parsed.into_diagnostics().remove(0),
+        ));
     }
 
     GritQuery::from_node(parsed.tree(), path, language)

crates/biome_grit_patterns/tests/spec_tests.rs

@@ -1,6 +1,8 @@
+use biome_diagnostics::Diagnostic;
 use biome_grit_parser::parse_grit;
 use biome_grit_patterns::{
-    GritQuery, GritQueryResult, GritTargetFile, GritTargetLanguage, Message, OutputFile,
+    GritQuery, GritQueryResult, GritTargetFile, GritTargetLanguage, JsTargetLanguage, Message,
+    OutputFile,
 };
 use biome_js_parser::{parse, JsParserOptions};
 use biome_js_syntax::JsFileSource;
@@ -20,6 +22,13 @@ fn run_test(input: &'static str, _: &str, _: &str, _: &str) {
         panic!("the test file must be placed in the specs/<target-lang-ext>/ directory");
     }
 
+    // We have one exception: The `specs/error/` directory
+    // is for testing error diagnostics.
+    if target_lang_ext == "error" {
+        run_error_test(query_path, test_name);
+        return;
+    }
+
     let Some(target_lang) = GritTargetLanguage::from_extension(target_lang_ext) else {
         panic!("the test file must be placed in the specs/<target-lang-ext>/ directory, unrecognized extension: {target_lang_ext}");
     };
@@ -73,6 +82,47 @@ fn run_test(input: &'static str, _: &str, _: &str, _: &str) {
     });
 }
 
+fn run_error_test(query_path: &Path, test_name: &str) {
+    let snapshot_result = {
+        let query = read_to_string(query_path)
+            .unwrap_or_else(|err| panic!("cannot read query from {query_path:?}: {err:?}"));
+
+        let parse_grit_result = parse_grit(&query);
+        if parse_grit_result.diagnostics().is_empty() {
+            match GritQuery::from_node(
+                parse_grit_result.tree(),
+                None,
+                GritTargetLanguage::JsTargetLanguage(JsTargetLanguage),
+            ) {
+                Ok(_) => panic!("an error was expected when compiling query from {query_path:?}"),
+                Err(error) => ErrorSnapshotResult {
+                    diagnostics: vec![Box::new(error)],
+                },
+            }
+        } else {
+            ErrorSnapshotResult {
+                diagnostics: parse_grit_result
+                    .diagnostics()
+                    .iter()
+                    .map(|diagnostic| {
+                        let boxed: Box<dyn Diagnostic> = Box::new(diagnostic.clone());
+                        boxed
+                    })
+                    .collect(),
+            }
+        }
+    };
+
+    let snapshot = format!("{snapshot_result:#?}");
+
+    insta::with_settings!({
+        prepend_module_to_snapshot => false,
+        snapshot_path => query_path.parent().unwrap(),
+    }, {
+        insta::assert_snapshot!(test_name, snapshot, test_name);
+    });
+}
+
 /// Tests should be in a `specs/<target-lang-extension>` directory, and each
 /// test should have a `.grit` file and a matching `.<target-lang-extension>`
 /// file.
@@ -137,3 +187,9 @@ fn format_range(range: Range) -> String {
         range.start.line, range.start.column, range.end.line, range.end.column
     )
 }
+
+#[derive(Debug, Default)]
+struct ErrorSnapshotResult {
+    #[allow(unused)]
+    diagnostics: Vec<Box<dyn Diagnostic>>,
+}

crates/biome_grit_patterns/tests/specs/error/missingQuote.grit

@@ -0,0 +1 @@
+`

crates/biome_grit_patterns/tests/specs/error/missingQuote.snap

@@ -0,0 +1,26 @@
+---
+source: crates/biome_grit_patterns/tests/spec_tests.rs
+expression: invalidQuery
+---
+ErrorSnapshotResult {
+    diagnostics: [
+        ParseDiagnostic {
+            span: Some(
+                0..2,
+            ),
+            message: Missing closing quote,
+            advice: ParserAdvice {
+                advice_list: [
+                    Detail(
+                        ParserAdviceDetail {
+                            message: "file ends here",
+                            span: Some(
+                                2..2,
+                            ),
+                        },
+                    ),
+                ],
+            },
+        },
+    ],
+}

crates/biome_grit_patterns/tests/specs/error/missingQuote.ts

@@ -0,0 +1,2 @@
+
+// Not going to match anything.

crates/biome_parser/src/diagnostic.rs

@@ -26,7 +26,7 @@ pub struct ParseDiagnostic {
     span: Option<TextRange>,
     #[message]
     #[description]
-    message: MessageAndDescription,
+    pub message: MessageAndDescription,
     #[advice]
     advice: ParserAdvice,
 }

crates/biome_service/src/diagnostics.rs

@@ -4,13 +4,14 @@ use biome_configuration::diagnostics::{ConfigurationDiagnostic, EditorConfigDiag
 use biome_configuration::{BiomeDiagnostic, CantLoadExtendFile};
 use biome_console::fmt::Bytes;
 use biome_console::markup;
-use biome_diagnostics::serde::Diagnostic as SerdeDiagnostic;
+use biome_css_parser::ParseDiagnostic;
 use biome_diagnostics::{
-    category, Advices, Category, Diagnostic, DiagnosticTags, Location, LogCategory, Severity, Visit,
+    category, Advices, Category, Diagnostic, DiagnosticTags, Location, LogCategory,
+    MessageAndDescription, Severity, Visit,
 };
 use biome_formatter::{FormatError, PrintError};
 use biome_fs::{BiomePath, FileSystemDiagnostic};
-use biome_grit_patterns::{CompileError, ParsePatternError};
+use biome_grit_patterns::CompileError;
 use biome_js_analyze::utils::rename::RenameError;
 use serde::{Deserialize, Serialize};
 use std::error::Error;
@@ -336,7 +337,17 @@ pub enum SearchError {
 
 #[derive(Debug, Deserialize, Diagnostic, Serialize)]
 pub struct PatternCompilationError {
-    pub diagnostics: Vec<SerdeDiagnostic>,
+    #[message]
+    #[description]
+    message: MessageAndDescription,
+}
+
+impl From<ParseDiagnostic> for PatternCompilationError {
+    fn from(diagnostic: ParseDiagnostic) -> Self {
+        Self {
+            message: diagnostic.message,
+        }
+    }
 }
 
 #[derive(Debug, Serialize, Deserialize, Diagnostic)]
@@ -463,18 +474,10 @@ impl From<VcsDiagnostic> for WorkspaceError {
 
 impl From<CompileError> for WorkspaceError {
     fn from(value: CompileError) -> Self {
-        match &value {
-            CompileError::ParsePatternError(ParsePatternError { diagnostics }) => {
-                Self::SearchError(SearchError::PatternCompilationError(
-                    PatternCompilationError {
-                        diagnostics: diagnostics
-                            .iter()
-                            .cloned()
-                            .map(SerdeDiagnostic::new)
-                            .collect(),
-                    },
-                ))
-            }
+        match value {
+            CompileError::ParsePatternError(diagnostic) => Self::SearchError(
+                SearchError::PatternCompilationError(PatternCompilationError::from(diagnostic)),
+            ),
             // FIXME: This really needs proper diagnostics
             _ => Self::SearchError(SearchError::QueryError(QueryDiagnostic(format!(
                 "{value:?}"