From 290d51bb7b6c7913fb54739c0740ae2fa67b7367 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Wed, 3 Dec 2025 22:09:11 -0300 Subject: [PATCH 1/4] refactor: Diagnostics handling --- crates/plotnik-cli/src/commands/debug/mod.rs | 8 +- .../plotnik-lib/src/diagnostics/collection.rs | 85 ++++++ .../error.rs => diagnostics/message.rs} | 172 ++---------- crates/plotnik-lib/src/diagnostics/mod.rs | 15 ++ crates/plotnik-lib/src/diagnostics/printer.rs | 144 ++++++++++ crates/plotnik-lib/src/diagnostics/tests.rs | 248 ++++++++++++++++++ crates/plotnik-lib/src/lib.rs | 9 +- crates/plotnik-lib/src/parser/ast_tests.rs | 2 +- crates/plotnik-lib/src/parser/core.rs | 55 ++-- crates/plotnik-lib/src/parser/error_tests.rs | 222 ---------------- crates/plotnik-lib/src/parser/grammar.rs | 26 +- crates/plotnik-lib/src/parser/mod.rs | 40 ++- .../parser/tests/json_serialization_tests.rs | 8 +- .../parser/tests/recovery/coverage_tests.rs | 4 +- .../parser/tests/recovery/incomplete_tests.rs | 36 +-- .../parser/tests/recovery/unclosed_tests.rs | 24 +- .../parser/tests/recovery/unexpected_tests.rs | 48 ++-- .../parser/tests/recovery/validation_tests.rs | 124 ++++----- .../src/parser/tests/validation/naming.rs | 86 ------ crates/plotnik-lib/src/query/alt_kind.rs | 14 +- .../plotnik-lib/src/query/alt_kind_tests.rs | 8 +- crates/plotnik-lib/src/query/dump.rs | 5 +- crates/plotnik-lib/src/query/errors.rs | 74 +++--- crates/plotnik-lib/src/query/errors_tests.rs | 93 ------- crates/plotnik-lib/src/query/mod.rs | 35 ++- crates/plotnik-lib/src/query/mod_tests.rs | 6 +- crates/plotnik-lib/src/query/named_defs.rs | 47 ++-- .../plotnik-lib/src/query/named_defs_tests.rs | 10 +- crates/plotnik-lib/src/query/ref_cycles.rs | 12 +- .../plotnik-lib/src/query/ref_cycles_tests.rs | 26 +- .../src/query/shape_cardinalities.rs | 12 +- .../src/query/shape_cardinalities_tests.rs | 4 +- 32 files changed, 829 insertions(+), 873 deletions(-) create mode 100644 crates/plotnik-lib/src/diagnostics/collection.rs rename crates/plotnik-lib/src/{parser/error.rs => diagnostics/message.rs} (54%) create mode 100644 crates/plotnik-lib/src/diagnostics/mod.rs create mode 100644 crates/plotnik-lib/src/diagnostics/printer.rs create mode 100644 crates/plotnik-lib/src/diagnostics/tests.rs delete mode 100644 crates/plotnik-lib/src/parser/error_tests.rs delete mode 100644 crates/plotnik-lib/src/parser/tests/validation/naming.rs delete mode 100644 crates/plotnik-lib/src/query/errors_tests.rs diff --git a/crates/plotnik-cli/src/commands/debug/mod.rs b/crates/plotnik-cli/src/commands/debug/mod.rs index 0766f37b..7cb47a41 100644 --- a/crates/plotnik-cli/src/commands/debug/mod.rs +++ b/crates/plotnik-cli/src/commands/debug/mod.rs @@ -4,7 +4,6 @@ use std::fs; use std::io::{self, Read}; use plotnik_lib::Query; -use plotnik_lib::RenderOptions; use source::{dump_source, load_source, parse_tree, resolve_lang}; @@ -97,12 +96,7 @@ pub fn run(args: DebugArgs) { if let Some(ref q) = query && !q.is_valid() { - let options = if args.color { - RenderOptions::colored() - } else { - RenderOptions::plain() - }; - eprint!("{}", q.render_diagnostics(options)); + eprint!("{}", q.diagnostics_printer().colored(args.color).render()); } } diff --git a/crates/plotnik-lib/src/diagnostics/collection.rs b/crates/plotnik-lib/src/diagnostics/collection.rs new file mode 100644 index 00000000..03b616ea --- /dev/null +++ b/crates/plotnik-lib/src/diagnostics/collection.rs @@ -0,0 +1,85 @@ +//! Diagnostics collection for accumulating compiler messages. + +use super::message::{DiagnosticMessage, Severity}; + +/// Collection of diagnostic messages from parsing and analysis. +#[derive(Debug, Clone, Default)] +pub struct Diagnostics(Vec); + +impl Diagnostics { + pub fn new() -> Self { + Self(Vec::new()) + } + + pub fn push(&mut self, msg: DiagnosticMessage) { + self.0.push(msg); + } + + pub fn extend(&mut self, iter: impl IntoIterator) { + self.0.extend(iter); + } + + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + + pub fn len(&self) -> usize { + self.0.len() + } + + pub fn iter(&self) -> impl Iterator { + self.0.iter() + } + + pub fn has_errors(&self) -> bool { + self.0.iter().any(|d| d.is_error()) + } + + pub fn has_warnings(&self) -> bool { + self.0.iter().any(|d| d.is_warning()) + } + + pub fn as_slice(&self) -> &[DiagnosticMessage] { + &self.0 + } + + pub fn into_vec(self) -> Vec { + self.0 + } + + pub fn error_count(&self) -> usize { + self.0.iter().filter(|d| d.is_error()).count() + } + + pub fn warning_count(&self) -> usize { + self.0.iter().filter(|d| d.is_warning()).count() + } + + pub fn filter_by_severity(&self, severity: Severity) -> Vec<&DiagnosticMessage> { + self.0.iter().filter(|d| d.severity == severity).collect() + } +} + +impl IntoIterator for Diagnostics { + type Item = DiagnosticMessage; + type IntoIter = std::vec::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() + } +} + +impl<'a> IntoIterator for &'a Diagnostics { + type Item = &'a DiagnosticMessage; + type IntoIter = std::slice::Iter<'a, DiagnosticMessage>; + + fn into_iter(self) -> Self::IntoIter { + self.0.iter() + } +} + +impl FromIterator for Diagnostics { + fn from_iter>(iter: T) -> Self { + Self(iter.into_iter().collect()) + } +} diff --git a/crates/plotnik-lib/src/parser/error.rs b/crates/plotnik-lib/src/diagnostics/message.rs similarity index 54% rename from crates/plotnik-lib/src/parser/error.rs rename to crates/plotnik-lib/src/diagnostics/message.rs index 73135d9e..62218407 100644 --- a/crates/plotnik-lib/src/parser/error.rs +++ b/crates/plotnik-lib/src/diagnostics/message.rs @@ -1,6 +1,5 @@ -//! Diagnostic types and rendering utilities. +//! Diagnostic message types and related structures. -use annotate_snippets::{AnnotationKind, Group, Level, Patch, Renderer, Snippet}; use rowan::{TextRange, TextSize}; use serde::{Serialize, Serializer}; @@ -13,15 +12,6 @@ pub enum Severity { Warning, } -impl Severity { - fn to_level(self) -> Level<'static> { - match self { - Severity::Error => Level::ERROR, - Severity::Warning => Level::WARNING, - } - } -} - impl std::fmt::Display for Severity { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -31,28 +21,28 @@ impl std::fmt::Display for Severity { } } -/// The stage at which a diagnostic occurred (internal implementation detail). +/// The stage at which a diagnostic occurred. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Default)] #[serde(rename_all = "lowercase")] -pub enum ErrorStage { - /// Lexing/parsing errors (syntax structure) +pub enum DiagnosticStage { + /// Lexing/parsing diagnostics (syntax structure) #[default] Parse, - /// Semantic validation errors (mixed alternations, etc.) + /// Semantic validation diagnostics (mixed alternations, etc.) Validate, - /// Name resolution errors (undefined/duplicate references) + /// Name resolution diagnostics (undefined/duplicate references) Resolve, - /// Escape analysis errors (infinite recursion) + /// Escape analysis diagnostics (infinite recursion) Escape, } -impl std::fmt::Display for ErrorStage { +impl std::fmt::Display for DiagnosticStage { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - ErrorStage::Parse => write!(f, "parse"), - ErrorStage::Validate => write!(f, "validate"), - ErrorStage::Resolve => write!(f, "resolve"), - ErrorStage::Escape => write!(f, "escape"), + DiagnosticStage::Parse => write!(f, "parse"), + DiagnosticStage::Validate => write!(f, "validate"), + DiagnosticStage::Resolve => write!(f, "resolve"), + DiagnosticStage::Escape => write!(f, "escape"), } } } @@ -90,11 +80,19 @@ impl RelatedInfo { } } -/// A diagnostic with location, message, severity, and optional fix. +fn serialize_text_range(range: &TextRange, s: S) -> Result { + use serde::ser::SerializeStruct; + let mut state = s.serialize_struct("TextRange", 2)?; + state.serialize_field("start", &u32::from(range.start()))?; + state.serialize_field("end", &u32::from(range.end()))?; + state.end() +} + +/// A diagnostic message with location, message, severity, and optional fix. #[derive(Debug, Clone, PartialEq, Eq, Serialize)] -pub struct Diagnostic { +pub struct DiagnosticMessage { pub severity: Severity, - pub stage: ErrorStage, + pub stage: DiagnosticStage, #[serde(serialize_with = "serialize_text_range")] pub range: TextRange, pub message: String, @@ -104,20 +102,12 @@ pub struct Diagnostic { pub related: Vec, } -fn serialize_text_range(range: &TextRange, s: S) -> Result { - use serde::ser::SerializeStruct; - let mut state = s.serialize_struct("TextRange", 2)?; - state.serialize_field("start", &u32::from(range.start()))?; - state.serialize_field("end", &u32::from(range.end()))?; - state.end() -} - -impl Diagnostic { +impl DiagnosticMessage { /// Create an error diagnostic. pub fn error(range: TextRange, message: impl Into) -> Self { Self { severity: Severity::Error, - stage: ErrorStage::default(), + stage: DiagnosticStage::default(), range, message: message.into(), fix: None, @@ -129,7 +119,7 @@ impl Diagnostic { pub fn warning(range: TextRange, message: impl Into) -> Self { Self { severity: Severity::Warning, - stage: ErrorStage::default(), + stage: DiagnosticStage::default(), range, message: message.into(), fix: None, @@ -148,7 +138,7 @@ impl Diagnostic { } /// Set the pipeline stage. - pub fn with_stage(mut self, stage: ErrorStage) -> Self { + pub fn with_stage(mut self, stage: DiagnosticStage) -> Self { self.stage = stage; self } @@ -180,7 +170,7 @@ impl Diagnostic { } } -impl std::fmt::Display for Diagnostic { +impl std::fmt::Display for DiagnosticMessage { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!( f, @@ -206,108 +196,4 @@ impl std::fmt::Display for Diagnostic { } } -impl std::error::Error for Diagnostic {} - -/// Options for rendering diagnostics. -#[derive(Debug, Clone, Copy)] -pub struct RenderOptions { - pub colored: bool, -} - -impl Default for RenderOptions { - fn default() -> Self { - Self { colored: true } - } -} - -impl RenderOptions { - pub fn plain() -> Self { - Self { colored: false } - } - - pub fn colored() -> Self { - Self { colored: true } - } -} - -fn adjust_range(range: TextRange, limit: usize) -> std::ops::Range { - let start: usize = range.start().into(); - let end: usize = range.end().into(); - - if start == end { - return start..(start + 1).min(limit); - } - - start..end -} - -/// Render diagnostics using annotate-snippets. -pub fn render_diagnostics( - source: &str, - diagnostics: &[Diagnostic], - path: Option<&str>, - options: RenderOptions, -) -> String { - if diagnostics.is_empty() { - return String::new(); - } - - let renderer = if options.colored { - Renderer::styled() - } else { - Renderer::plain() - }; - - let mut output = String::new(); - - for (i, diag) in diagnostics.iter().enumerate() { - let range = adjust_range(diag.range, source.len()); - - let mut snippet = Snippet::source(source).line_start(1).annotation( - AnnotationKind::Primary - .span(range.clone()) - .label(&diag.message), - ); - - if let Some(p) = path { - snippet = snippet.path(p); - } - - for related in &diag.related { - snippet = snippet.annotation( - AnnotationKind::Context - .span(adjust_range(related.range, source.len())) - .label(&related.message), - ); - } - - let level = diag.severity.to_level(); - let title_group = level.primary_title(&diag.message).element(snippet); - - let mut report: Vec = vec![title_group]; - - if let Some(fix) = &diag.fix { - report.push( - Level::HELP.secondary_title(&fix.description).element( - Snippet::source(source) - .line_start(1) - .patch(Patch::new(range, &fix.replacement)), - ), - ); - } - - if i > 0 { - output.push('\n'); - } - output.push_str(&renderer.render(&report).to_string()); - } - - output -} - -// Backwards compatibility aliases -pub type SyntaxError = Diagnostic; - -pub fn render_errors(source: &str, errors: &[Diagnostic], path: Option<&str>) -> String { - render_diagnostics(source, errors, path, RenderOptions::plain()) -} +impl std::error::Error for DiagnosticMessage {} \ No newline at end of file diff --git a/crates/plotnik-lib/src/diagnostics/mod.rs b/crates/plotnik-lib/src/diagnostics/mod.rs new file mode 100644 index 00000000..43043810 --- /dev/null +++ b/crates/plotnik-lib/src/diagnostics/mod.rs @@ -0,0 +1,15 @@ +//! Compiler diagnostics infrastructure. +//! +//! This module provides types for collecting, filtering, and rendering +//! diagnostic messages from parsing and analysis stages. + +mod collection; +mod message; +mod printer; + +#[cfg(test)] +mod tests; + +pub use collection::Diagnostics; +pub use message::{DiagnosticMessage, DiagnosticStage, Fix, RelatedInfo, Severity}; +pub use printer::DiagnosticsPrinter; diff --git a/crates/plotnik-lib/src/diagnostics/printer.rs b/crates/plotnik-lib/src/diagnostics/printer.rs new file mode 100644 index 00000000..1f66762d --- /dev/null +++ b/crates/plotnik-lib/src/diagnostics/printer.rs @@ -0,0 +1,144 @@ + +//! Builder-pattern printer for rendering diagnostics. + +use std::fmt::Write; + +use annotate_snippets::{AnnotationKind, Group, Level, Patch, Renderer, Snippet}; +use rowan::TextRange; + +use super::collection::Diagnostics; +use super::message::Severity; + +/// Builder for rendering diagnostics with various options. +pub struct DiagnosticsPrinter<'d, 's> { + diagnostics: &'d Diagnostics, + source: Option<&'s str>, + path: Option<&'s str>, + colored: bool, +} + +impl<'d, 's> DiagnosticsPrinter<'d, 's> { + pub fn new(diagnostics: &'d Diagnostics) -> Self { + Self { + diagnostics, + source: None, + path: None, + colored: false, + } + } + + pub fn source(mut self, source: &'s str) -> Self { + self.source = Some(source); + self + } + + pub fn path(mut self, path: &'s str) -> Self { + self.path = Some(path); + self + } + + pub fn colored(mut self, value: bool) -> Self { + self.colored = value; + self + } + + pub fn render(&self) -> String { + let mut out = String::new(); + self.format(&mut out).expect("String write never fails"); + out + } + + pub fn format(&self, w: &mut impl Write) -> std::fmt::Result { + let Some(source) = self.source else { + return self.format_plain(w); + }; + + if self.diagnostics.is_empty() { + return Ok(()); + } + + let renderer = if self.colored { + Renderer::styled() + } else { + Renderer::plain() + }; + + for (i, diag) in self.diagnostics.iter().enumerate() { + let range = adjust_range(diag.range, source.len()); + + let mut snippet = Snippet::source(source).line_start(1).annotation( + AnnotationKind::Primary + .span(range.clone()) + .label(&diag.message), + ); + + if let Some(p) = self.path { + snippet = snippet.path(p); + } + + for related in &diag.related { + snippet = snippet.annotation( + AnnotationKind::Context + .span(adjust_range(related.range, source.len())) + .label(&related.message), + ); + } + + let level = severity_to_level(diag.severity); + let title_group = level.primary_title(&diag.message).element(snippet); + + let mut report: Vec = vec![title_group]; + + if let Some(fix) = &diag.fix { + report.push( + Level::HELP.secondary_title(&fix.description).element( + Snippet::source(source) + .line_start(1) + .patch(Patch::new(range, &fix.replacement)), + ), + ); + } + + if i > 0 { + w.write_char('\n')?; + } + write!(w, "{}", renderer.render(&report))?; + } + + Ok(()) + } + + fn format_plain(&self, w: &mut impl Write) -> std::fmt::Result { + for (i, diag) in self.diagnostics.iter().enumerate() { + if i > 0 { + w.write_char('\n')?; + } + write!(w, "{}", diag)?; + } + Ok(()) + } +} + +fn severity_to_level(severity: Severity) -> Level<'static> { + match severity { + Severity::Error => Level::ERROR, + Severity::Warning => Level::WARNING, + } +} + +fn adjust_range(range: TextRange, limit: usize) -> std::ops::Range { + let start: usize = range.start().into(); + let end: usize = range.end().into(); + + if start == end { + return start..(start + 1).min(limit); + } + + start..end +} + +impl Diagnostics { + pub fn printer(&self) -> DiagnosticsPrinter<'_, '_> { + DiagnosticsPrinter::new(self) + } +} diff --git a/crates/plotnik-lib/src/diagnostics/tests.rs b/crates/plotnik-lib/src/diagnostics/tests.rs new file mode 100644 index 00000000..a426262f --- /dev/null +++ b/crates/plotnik-lib/src/diagnostics/tests.rs @@ -0,0 +1,248 @@ +use super::*; +use rowan::TextRange; + +#[test] +fn severity_display() { + insta::assert_snapshot!(format!("{}", Severity::Error), @"error"); + insta::assert_snapshot!(format!("{}", Severity::Warning), @"warning"); +} + +#[test] +fn error_stage_display() { + insta::assert_snapshot!(format!("{}", DiagnosticStage::Parse), @"parse"); + insta::assert_snapshot!(format!("{}", DiagnosticStage::Validate), @"validate"); + insta::assert_snapshot!(format!("{}", DiagnosticStage::Resolve), @"resolve"); + insta::assert_snapshot!(format!("{}", DiagnosticStage::Escape), @"escape"); +} + +#[test] +fn diagnostic_warning_constructors() { + let warn = DiagnosticMessage::warning(TextRange::empty(0.into()), "test warning"); + assert!(warn.is_warning()); + assert!(!warn.is_error()); + + let warn_at = DiagnosticMessage::warning_at(5.into(), "warning at offset"); + assert!(warn_at.is_warning()); + assert_eq!(warn_at.range.start(), 5.into()); +} + +#[test] +fn diagnostic_error_at_constructor() { + let err = DiagnosticMessage::error_at(7.into(), "error at offset"); + assert!(err.is_error()); + assert!(!err.is_warning()); + assert_eq!(err.range.start(), 7.into()); + assert_eq!(err.range.end(), 7.into()); +} + +#[test] +fn diagnostic_builders() { + let diag = DiagnosticMessage::error(TextRange::empty(0.into()), "test") + .with_stage(DiagnosticStage::Resolve) + .with_fix(Fix::new("replacement", "description")) + .with_related(RelatedInfo::new(TextRange::empty(10.into()), "related")); + + assert_eq!(diag.stage, DiagnosticStage::Resolve); + assert!(diag.fix.is_some()); + assert_eq!(diag.related.len(), 1); + + let diag2 = + DiagnosticMessage::error(TextRange::empty(0.into()), "test").with_related_many(vec![ + RelatedInfo::new(TextRange::empty(1.into()), "first"), + RelatedInfo::new(TextRange::empty(2.into()), "second"), + ]); + assert_eq!(diag2.related.len(), 2); +} + +#[test] +fn diagnostic_display() { + let diag = DiagnosticMessage::error(TextRange::new(5.into(), 10.into()), "test message"); + insta::assert_snapshot!(format!("{}", diag), @"error at 5..10: test message"); + + let diag_with_fix = DiagnosticMessage::error(TextRange::empty(0.into()), "msg") + .with_fix(Fix::new("fix", "fix description")); + insta::assert_snapshot!(format!("{}", diag_with_fix), @"error at 0..0: msg (fix: fix description)"); + + let diag_with_related = DiagnosticMessage::error(TextRange::empty(0.into()), "msg") + .with_related(RelatedInfo::new(TextRange::new(1.into(), 2.into()), "rel")); + insta::assert_snapshot!(format!("{}", diag_with_related), @"error at 0..0: msg (related: rel at 1..2)"); +} + +#[test] +fn printer_colored() { + let diag = DiagnosticMessage::error(TextRange::new(0.into(), 5.into()), "test"); + let diagnostics = Diagnostics::from_iter([diag]); + let result = diagnostics.printer().source("hello").colored(true).render(); + assert!(result.contains("test")); + assert!(result.contains('\x1b')); +} + +#[test] +fn printer_empty_diagnostics() { + let diagnostics = Diagnostics::new(); + let result = diagnostics.printer().source("source").render(); + assert!(result.is_empty()); +} + +#[test] +fn printer_with_path() { + let diag = DiagnosticMessage::error(TextRange::new(0.into(), 5.into()), "test error"); + let diagnostics = Diagnostics::from_iter([diag]); + let result = diagnostics + .printer() + .source("hello world") + .path("test.pql") + .render(); + insta::assert_snapshot!(result, @r" + error: test error + --> test.pql:1:1 + | + 1 | hello world + | ^^^^^ test error + "); +} + +#[test] +fn printer_zero_width_span() { + let diag = DiagnosticMessage::error(TextRange::empty(0.into()), "zero width error"); + let diagnostics = Diagnostics::from_iter([diag]); + let result = diagnostics.printer().source("hello").render(); + insta::assert_snapshot!(result, @r" + error: zero width error + | + 1 | hello + | ^ zero width error + "); +} + +#[test] +fn printer_with_related() { + let diag = + DiagnosticMessage::error(TextRange::new(0.into(), 5.into()), "primary").with_related( + RelatedInfo::new(TextRange::new(6.into(), 10.into()), "related info"), + ); + let diagnostics = Diagnostics::from_iter([diag]); + let result = diagnostics.printer().source("hello world!").render(); + insta::assert_snapshot!(result, @r" + error: primary + | + 1 | hello world! + | ^^^^^ ---- related info + | | + | primary + "); +} + +#[test] +fn printer_related_zero_width() { + let diag = + DiagnosticMessage::error(TextRange::new(0.into(), 5.into()), "primary").with_related( + RelatedInfo::new(TextRange::empty(6.into()), "zero width related"), + ); + let diagnostics = Diagnostics::from_iter([diag]); + let result = diagnostics.printer().source("hello world!").render(); + insta::assert_snapshot!(result, @r" + error: primary + | + 1 | hello world! + | ^^^^^ - zero width related + | | + | primary + "); +} + +#[test] +fn printer_with_fix() { + let diag = DiagnosticMessage::error(TextRange::new(0.into(), 5.into()), "fixable") + .with_fix(Fix::new("fixed", "apply this fix")); + let diagnostics = Diagnostics::from_iter([diag]); + let result = diagnostics.printer().source("hello world").render(); + insta::assert_snapshot!(result, @r" + error: fixable + | + 1 | hello world + | ^^^^^ fixable + | + help: apply this fix + | + 1 - hello world + 1 + fixed world + | + "); +} + +#[test] +fn printer_multiple_diagnostics() { + let diag1 = DiagnosticMessage::error(TextRange::new(0.into(), 5.into()), "first error"); + let diag2 = DiagnosticMessage::error(TextRange::new(6.into(), 10.into()), "second error"); + let diagnostics = Diagnostics::from_iter([diag1, diag2]); + let result = diagnostics.printer().source("hello world!").render(); + insta::assert_snapshot!(result, @r" + error: first error + | + 1 | hello world! + | ^^^^^ first error + error: second error + | + 1 | hello world! + | ^^^^ second error + "); +} + +#[test] +fn printer_warning() { + let diag = DiagnosticMessage::warning(TextRange::new(0.into(), 5.into()), "a warning"); + let diagnostics = Diagnostics::from_iter([diag]); + let result = diagnostics.printer().source("hello").render(); + insta::assert_snapshot!(result, @r" + warning: a warning + | + 1 | hello + | ^^^^^ a warning + "); +} + +#[test] +fn printer_without_source_uses_plain_format() { + let diag = DiagnosticMessage::error(TextRange::new(0.into(), 3.into()), "test"); + let diagnostics = Diagnostics::from_iter([diag]); + let result = diagnostics.printer().render(); + insta::assert_snapshot!(result, @"error at 0..3: test"); +} + +#[test] +fn diagnostics_collection_methods() { + let diag1 = DiagnosticMessage::error(TextRange::empty(0.into()), "error"); + let diag2 = DiagnosticMessage::warning(TextRange::empty(1.into()), "warning"); + let mut diagnostics = Diagnostics::new(); + diagnostics.push(diag1); + diagnostics.push(diag2); + + assert!(!diagnostics.is_empty()); + assert_eq!(diagnostics.len(), 2); + assert!(diagnostics.has_errors()); + assert!(diagnostics.has_warnings()); + assert_eq!(diagnostics.error_count(), 1); + assert_eq!(diagnostics.warning_count(), 1); + assert_eq!(diagnostics.filter_by_severity(Severity::Error).len(), 1); + assert_eq!(diagnostics.filter_by_severity(Severity::Warning).len(), 1); +} + +#[test] +fn diagnostics_iteration() { + let diag1 = DiagnosticMessage::error(TextRange::empty(0.into()), "first"); + let diag2 = DiagnosticMessage::error(TextRange::empty(1.into()), "second"); + let diagnostics = Diagnostics::from_iter([diag1, diag2]); + + let messages: Vec<_> = diagnostics.iter().map(|d| d.message.as_str()).collect(); + assert_eq!(messages, vec!["first", "second"]); + + let messages: Vec<_> = (&diagnostics) + .into_iter() + .map(|d| d.message.as_str()) + .collect(); + assert_eq!(messages, vec!["first", "second"]); + + let vec = diagnostics.into_vec(); + assert_eq!(vec.len(), 2); +} diff --git a/crates/plotnik-lib/src/lib.rs b/crates/plotnik-lib/src/lib.rs index 0349b8ff..b4aa7b36 100644 --- a/crates/plotnik-lib/src/lib.rs +++ b/crates/plotnik-lib/src/lib.rs @@ -3,7 +3,7 @@ //! # Example //! //! ``` -//! use plotnik_lib::{Query, RenderOptions}; +//! use plotnik_lib::Query; //! //! let query = Query::new(r#" //! Expr = [(identifier) (number)] @@ -11,16 +11,19 @@ //! "#).expect("valid query"); //! //! if !query.is_valid() { -//! eprintln!("{}", query.render_diagnostics(RenderOptions::plain())); +//! eprintln!("{}", query.diagnostics_printer().render()); //! } //! ``` #![cfg_attr(coverage_nightly, feature(coverage_attribute))] +pub mod diagnostics; pub mod parser; pub mod query; -pub use parser::{Diagnostic, RenderOptions, Severity}; +pub use diagnostics::{ + DiagnosticMessage, DiagnosticStage, Diagnostics, DiagnosticsPrinter, Fix, RelatedInfo, Severity, +}; pub use query::{Query, QueryBuilder}; /// Errors that can occur during query parsing. diff --git a/crates/plotnik-lib/src/parser/ast_tests.rs b/crates/plotnik-lib/src/parser/ast_tests.rs index e69c94bb..cf974604 100644 --- a/crates/plotnik-lib/src/parser/ast_tests.rs +++ b/crates/plotnik-lib/src/parser/ast_tests.rs @@ -261,7 +261,7 @@ fn complex_example() { fn ast_with_errors() { let query = Query::new("(call (Undefined))").unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r#" + insta::assert_snapshot!(query.dump_diagnostics(), @r#" error: undefined reference: `Undefined` | 1 | (call (Undefined)) diff --git a/crates/plotnik-lib/src/parser/core.rs b/crates/plotnik-lib/src/parser/core.rs index 48095b45..0ecf2a0f 100644 --- a/crates/plotnik-lib/src/parser/core.rs +++ b/crates/plotnik-lib/src/parser/core.rs @@ -4,29 +4,29 @@ //! - Token access and lookahead //! - Trivia buffering and attachment //! - Tree construction via Rowan -//! - Error recording and recovery +//! - Diagnostic recording and recovery //! - Fuel-based limits (debug, execution, recursion) use rowan::{Checkpoint, GreenNode, GreenNodeBuilder, TextRange, TextSize}; use super::cst::token_sets::ROOT_EXPR_FIRST; use super::cst::{SyntaxKind, TokenSet}; -use super::error::{Diagnostic, Fix, RelatedInfo}; use super::lexer::{Token, token_text}; +use crate::diagnostics::{DiagnosticMessage, Diagnostics, Fix, RelatedInfo}; use crate::Error; const DEFAULT_EXEC_FUEL: u32 = 1_000_000; const DEFAULT_RECURSION_FUEL: u32 = 4096; -/// Parse result containing the green tree and any errors. +/// Parse result containing the green tree and any diagnostics. /// -/// The tree is always complete—errors are recorded separately and also +/// The tree is always complete—diagnostics are recorded separately and also /// represented as `SyntaxKind::Error` nodes in the tree itself. #[derive(Debug, Clone)] pub struct Parse { pub(super) cst: GreenNode, - pub(super) errors: Vec, + pub(super) diagnostics: Diagnostics, } /// Tracks an open delimiter for better error messages on unclosed constructs. @@ -51,12 +51,12 @@ pub struct Parser<'src> { /// Drained into tree at `start_node()` / `checkpoint()`. pub(super) trivia_buffer: Vec, pub(super) builder: GreenNodeBuilder<'static>, - pub(super) errors: Vec, + pub(super) diagnostics: Diagnostics, /// Current recursion depth. pub(super) depth: u32, - /// Last error position - used to suppress cascading errors at same span - pub(super) last_error_pos: Option, - /// Stack of open delimiters for "unclosed X started here" errors. + /// Last diagnostic position - used to suppress cascading diagnostics at same span + pub(super) last_diagnostic_pos: Option, + /// Stack of open delimiters for "unclosed X started here" messages. pub(super) delimiter_stack: Vec, // Fuel limits @@ -81,9 +81,9 @@ impl<'src> Parser<'src> { pos: 0, trivia_buffer: Vec::with_capacity(4), builder: GreenNodeBuilder::new(), - errors: Vec::new(), + diagnostics: Diagnostics::new(), depth: 0, - last_error_pos: None, + last_diagnostic_pos: None, delimiter_stack: Vec::with_capacity(8), debug_fuel: std::cell::Cell::new(256), exec_fuel_remaining: Some(DEFAULT_EXEC_FUEL), @@ -111,7 +111,7 @@ impl<'src> Parser<'src> { } Ok(Parse { cst: self.builder.finish(), - errors: self.errors, + diagnostics: self.diagnostics, }) } @@ -277,7 +277,7 @@ impl<'src> Parser<'src> { } } - /// Expect token. On mismatch: emit error but don't consume (allows parent recovery). + /// Expect token. On mismatch: emit diagnostic but don't consume (allows parent recovery). pub(super) fn expect(&mut self, kind: SyntaxKind, what: &str) -> bool { if self.eat(kind) { return true; @@ -289,11 +289,12 @@ impl<'src> Parser<'src> { pub(super) fn error(&mut self, message: impl Into) { let range = self.current_span(); let pos = range.start(); - if self.last_error_pos == Some(pos) { + if self.last_diagnostic_pos == Some(pos) { return; } - self.last_error_pos = Some(pos); - self.errors.push(Diagnostic::error(range, message)); + self.last_diagnostic_pos = Some(pos); + self.diagnostics + .push(DiagnosticMessage::error(range, message)); } /// Wrap unexpected token in Error node and consume it. @@ -308,7 +309,7 @@ impl<'src> Parser<'src> { } /// Skip tokens until we hit a recovery point. Wraps skipped tokens in Error node. - /// If already at recovery token, just emits error without consuming. + /// If already at recovery token, just emits diagnostic without consuming. #[allow(dead_code)] // Used by future grammar rules (named expressions) pub(super) fn error_recover(&mut self, message: &str, recovery: TokenSet) { if self.at_set(recovery) || self.should_stop() { @@ -395,16 +396,16 @@ impl<'src> Parser<'src> { self.delimiter_stack.pop() } - /// Record an error with a related location (e.g., where an unclosed delimiter started). + /// Record a diagnostic with a related location (e.g., where an unclosed delimiter started). pub(super) fn error_with_related(&mut self, message: impl Into, related: RelatedInfo) { let range = self.current_span(); let pos = range.start(); - if self.last_error_pos == Some(pos) { + if self.last_diagnostic_pos == Some(pos) { return; } - self.last_error_pos = Some(pos); - self.errors - .push(Diagnostic::error(range, message).with_related(related)); + self.last_diagnostic_pos = Some(pos); + self.diagnostics + .push(DiagnosticMessage::error(range, message).with_related(related)); } /// Get the end position of the last non-trivia token before current position. @@ -418,7 +419,7 @@ impl<'src> Parser<'src> { None } - /// Record an error with an associated fix suggestion. + /// Record a diagnostic with an associated fix suggestion. pub(super) fn error_with_fix( &mut self, range: TextRange, @@ -426,11 +427,11 @@ impl<'src> Parser<'src> { fix: Fix, ) { let pos = range.start(); - if self.last_error_pos == Some(pos) { + if self.last_diagnostic_pos == Some(pos) { return; } - self.last_error_pos = Some(pos); - self.errors - .push(Diagnostic::error(range, message).with_fix(fix)); + self.last_diagnostic_pos = Some(pos); + self.diagnostics + .push(DiagnosticMessage::error(range, message).with_fix(fix)); } } diff --git a/crates/plotnik-lib/src/parser/error_tests.rs b/crates/plotnik-lib/src/parser/error_tests.rs deleted file mode 100644 index 30123179..00000000 --- a/crates/plotnik-lib/src/parser/error_tests.rs +++ /dev/null @@ -1,222 +0,0 @@ -use super::error::*; -use rowan::TextRange; - -#[test] -fn severity_display() { - insta::assert_snapshot!(format!("{}", Severity::Error), @"error"); - insta::assert_snapshot!(format!("{}", Severity::Warning), @"warning"); -} - -#[test] -fn error_stage_display() { - insta::assert_snapshot!(format!("{}", ErrorStage::Parse), @"parse"); - insta::assert_snapshot!(format!("{}", ErrorStage::Validate), @"validate"); - insta::assert_snapshot!(format!("{}", ErrorStage::Resolve), @"resolve"); - insta::assert_snapshot!(format!("{}", ErrorStage::Escape), @"escape"); -} - -#[test] -fn diagnostic_warning_constructors() { - let warn = Diagnostic::warning(TextRange::empty(0.into()), "test warning"); - assert!(warn.is_warning()); - assert!(!warn.is_error()); - - let warn_at = Diagnostic::warning_at(5.into(), "warning at offset"); - assert!(warn_at.is_warning()); - assert_eq!(warn_at.range.start(), 5.into()); -} - -#[test] -fn diagnostic_error_at_constructor() { - let err = Diagnostic::error_at(7.into(), "error at offset"); - assert!(err.is_error()); - assert!(!err.is_warning()); - assert_eq!(err.range.start(), 7.into()); - assert_eq!(err.range.end(), 7.into()); -} - -#[test] -fn diagnostic_builders() { - let diag = Diagnostic::error(TextRange::empty(0.into()), "test") - .with_stage(ErrorStage::Resolve) - .with_fix(Fix::new("replacement", "description")) - .with_related(RelatedInfo::new(TextRange::empty(10.into()), "related")); - - assert_eq!(diag.stage, ErrorStage::Resolve); - assert!(diag.fix.is_some()); - assert_eq!(diag.related.len(), 1); - - let diag2 = Diagnostic::error(TextRange::empty(0.into()), "test").with_related_many(vec![ - RelatedInfo::new(TextRange::empty(1.into()), "first"), - RelatedInfo::new(TextRange::empty(2.into()), "second"), - ]); - assert_eq!(diag2.related.len(), 2); -} - -#[test] -fn diagnostic_display() { - let diag = Diagnostic::error(TextRange::new(5.into(), 10.into()), "test message"); - insta::assert_snapshot!(format!("{}", diag), @"error at 5..10: test message"); - - let diag_with_fix = Diagnostic::error(TextRange::empty(0.into()), "msg") - .with_fix(Fix::new("fix", "fix description")); - insta::assert_snapshot!(format!("{}", diag_with_fix), @"error at 0..0: msg (fix: fix description)"); - - let diag_with_related = Diagnostic::error(TextRange::empty(0.into()), "msg") - .with_related(RelatedInfo::new(TextRange::new(1.into(), 2.into()), "rel")); - insta::assert_snapshot!(format!("{}", diag_with_related), @"error at 0..0: msg (related: rel at 1..2)"); -} - -#[test] -fn render_options_constructors() { - let plain = RenderOptions::plain(); - assert!(!plain.colored); - - let colored = RenderOptions::colored(); - assert!(colored.colored); - - let default = RenderOptions::default(); - assert!(default.colored); -} - -#[test] -fn render_diagnostics_colored() { - let diag = Diagnostic::error(TextRange::new(0.into(), 5.into()), "test"); - let result = render_diagnostics("hello", &[diag], None, RenderOptions::colored()); - assert!(result.contains("test")); - assert!(result.contains('\x1b')); -} - -#[test] -fn render_diagnostics_empty() { - let result = render_diagnostics("source", &[], None, RenderOptions::plain()); - assert!(result.is_empty()); -} - -#[test] -fn render_diagnostics_with_path() { - let diag = Diagnostic::error(TextRange::new(0.into(), 5.into()), "test error"); - let result = render_diagnostics( - "hello world", - &[diag], - Some("test.pql"), - RenderOptions::plain(), - ); - insta::assert_snapshot!(result, @r" - error: test error - --> test.pql:1:1 - | - 1 | hello world - | ^^^^^ test error - "); -} - -#[test] -fn render_diagnostics_zero_width_span() { - let diag = Diagnostic::error(TextRange::empty(0.into()), "zero width error"); - let result = render_diagnostics("hello", &[diag], None, RenderOptions::plain()); - insta::assert_snapshot!(result, @r" - error: zero width error - | - 1 | hello - | ^ zero width error - "); -} - -#[test] -fn render_diagnostics_with_related() { - let diag = Diagnostic::error(TextRange::new(0.into(), 5.into()), "primary").with_related( - RelatedInfo::new(TextRange::new(6.into(), 10.into()), "related info"), - ); - let result = render_diagnostics("hello world!", &[diag], None, RenderOptions::plain()); - insta::assert_snapshot!(result, @r" - error: primary - | - 1 | hello world! - | ^^^^^ ---- related info - | | - | primary - "); -} - -#[test] -fn render_diagnostics_related_zero_width() { - let diag = Diagnostic::error(TextRange::new(0.into(), 5.into()), "primary").with_related( - RelatedInfo::new(TextRange::empty(6.into()), "zero width related"), - ); - let result = render_diagnostics("hello world!", &[diag], None, RenderOptions::plain()); - insta::assert_snapshot!(result, @r" - error: primary - | - 1 | hello world! - | ^^^^^ - zero width related - | | - | primary - "); -} - -#[test] -fn render_diagnostics_with_fix() { - let diag = Diagnostic::error(TextRange::new(0.into(), 5.into()), "fixable") - .with_fix(Fix::new("fixed", "apply this fix")); - let result = render_diagnostics("hello world", &[diag], None, RenderOptions::plain()); - insta::assert_snapshot!(result, @r" - error: fixable - | - 1 | hello world - | ^^^^^ fixable - | - help: apply this fix - | - 1 - hello world - 1 + fixed world - | - "); -} - -#[test] -fn render_diagnostics_multiple() { - let diag1 = Diagnostic::error(TextRange::new(0.into(), 5.into()), "first error"); - let diag2 = Diagnostic::error(TextRange::new(6.into(), 10.into()), "second error"); - let result = render_diagnostics( - "hello world!", - &[diag1, diag2], - None, - RenderOptions::plain(), - ); - insta::assert_snapshot!(result, @r" - error: first error - | - 1 | hello world! - | ^^^^^ first error - error: second error - | - 1 | hello world! - | ^^^^ second error - "); -} - -#[test] -fn render_diagnostics_warning() { - let diag = Diagnostic::warning(TextRange::new(0.into(), 5.into()), "a warning"); - let result = render_diagnostics("hello", &[diag], None, RenderOptions::plain()); - insta::assert_snapshot!(result, @r" - warning: a warning - | - 1 | hello - | ^^^^^ a warning - "); -} - -#[test] -fn render_errors_wrapper() { - let diag = Diagnostic::error(TextRange::new(0.into(), 3.into()), "test"); - let result = render_errors("abc", &[diag], Some("file.pql")); - insta::assert_snapshot!(result, @r" - error: test - --> file.pql:1:1 - | - 1 | abc - | ^^^ test - "); -} diff --git a/crates/plotnik-lib/src/parser/grammar.rs b/crates/plotnik-lib/src/parser/grammar.rs index a6e7e083..f86bff59 100644 --- a/crates/plotnik-lib/src/parser/grammar.rs +++ b/crates/plotnik-lib/src/parser/grammar.rs @@ -6,8 +6,8 @@ use rowan::{Checkpoint, TextRange}; use super::core::Parser; -use super::error::{Fix, RelatedInfo}; use super::invariants::assert_nonempty; +use crate::diagnostics::{Fix, RelatedInfo}; use super::cst::token_sets::{ ALT_RECOVERY, EXPR_FIRST, QUANTIFIERS, SEPARATORS, SEQ_RECOVERY, TREE_RECOVERY, @@ -48,13 +48,14 @@ impl Parser<'_> { if unnamed_def_spans.len() > 1 { for span in &unnamed_def_spans[..unnamed_def_spans.len() - 1] { let def_text = &self.source[usize::from(span.start())..usize::from(span.end())]; - self.errors.push(super::error::Diagnostic::error( - *span, - format!( - "unnamed definition must be last in file; add a name: `Name = {}`", - def_text.trim() - ), - )); + self.diagnostics + .push(crate::diagnostics::DiagnosticMessage::error( + *span, + format!( + "unnamed definition must be last in file; add a name: `Name = {}`", + def_text.trim() + ), + )); } } @@ -267,10 +268,11 @@ impl Parser<'_> { let children_span = TextRange::new(children_start, children_end); if let Some(name) = &ref_name { - self.errors.push(super::error::Diagnostic::error( - children_span, - format!("reference `{}` cannot contain children", name), - )); + self.diagnostics + .push(crate::diagnostics::DiagnosticMessage::error( + children_span, + format!("reference `{}` cannot contain children", name), + )); } } else if is_ref { self.start_node_at(checkpoint, SyntaxKind::Ref); diff --git a/crates/plotnik-lib/src/parser/mod.rs b/crates/plotnik-lib/src/parser/mod.rs index 6a762cea..683366bf 100644 --- a/crates/plotnik-lib/src/parser/mod.rs +++ b/crates/plotnik-lib/src/parser/mod.rs @@ -8,25 +8,24 @@ //! - Zero-copy parsing: tokens carry spans, text sliced only when building tree nodes //! - Trivia buffering: whitespace/comments collected, then attached as leading trivia //! - Checkpoint-based wrapping: retroactively wrap nodes for quantifiers `*+?` -//! - Explicit recovery sets: per-production sets determine when to bail vs consume errors +//! - Explicit recovery sets: per-production sets determine when to bail vs consume diagnostics //! -//! # Error Recovery Strategy +//! # Recovery Strategy //! -//! The parser never fails on syntax errors—it always produces a tree. Recovery follows these rules: +//! The parser is resilient—it always produces a tree. Recovery follows these rules: //! //! 1. Unknown tokens get wrapped in `SyntaxKind::Error` nodes and consumed -//! 2. Missing expected tokens emit an error but don't consume (parent may handle) +//! 2. Missing expected tokens emit a diagnostic but don't consume (parent may handle) //! 3. Recovery sets define "synchronization points" per production //! 4. On recursion limit, remaining input goes into single Error node //! -//! However, fuel exhaustion (exec_fuel, recursion_fuel) returns an error immediately. +//! However, fuel exhaustion (exec_fuel, recursion_fuel) returns an actual error immediately. pub mod ast; pub mod cst; pub mod lexer; mod core; -mod error; mod grammar; mod invariants; @@ -35,8 +34,6 @@ mod ast_tests; #[cfg(test)] mod cst_tests; #[cfg(test)] -mod error_tests; -#[cfg(test)] mod lexer_tests; #[cfg(test)] mod tests; @@ -50,21 +47,14 @@ pub use ast::{ Seq, Str, Tree, Type, Wildcard, }; -// Re-exports from error -pub use error::{ - Diagnostic, ErrorStage, Fix, RelatedInfo, RenderOptions, Severity, SyntaxError, - render_diagnostics, render_errors, -}; - -// Internal use -pub(crate) use core::Parser; +pub use core::Parser; -use crate::Result; +use crate::{Diagnostics, Result}; use lexer::lex; -/// Parse result containing the green tree and any errors. +/// Parse result containing the green tree and any diagnostics. /// -/// The tree is always complete—errors are recorded separately and also +/// The tree is always complete—diagnostics are recorded separately and also /// represented as `SyntaxKind::Error` nodes in the tree itself. #[derive(Debug, Clone)] pub struct Parse { @@ -83,18 +73,18 @@ impl Parse { SyntaxNode::new_root(self.inner.cst.clone()) } - pub fn errors(&self) -> &[Diagnostic] { - &self.inner.errors + pub fn diagnostics(&self) -> &Diagnostics { + &self.inner.diagnostics } #[allow(dead_code)] pub fn is_valid(&self) -> bool { - self.inner.errors.is_empty() + self.inner.diagnostics.is_empty() } - /// Render errors as a human-readable string using annotate-snippets. - pub fn render_errors(&self, source: &str) -> String { - render_errors(source, &self.inner.errors, None) + /// Render diagnostics as a human-readable string using annotate-snippets. + pub fn render_diagnostics(&self, source: &str) -> String { + self.inner.diagnostics.printer().source(source).render() } } diff --git a/crates/plotnik-lib/src/parser/tests/json_serialization_tests.rs b/crates/plotnik-lib/src/parser/tests/json_serialization_tests.rs index 97ef7d15..1a1f9a0f 100644 --- a/crates/plotnik-lib/src/parser/tests/json_serialization_tests.rs +++ b/crates/plotnik-lib/src/parser/tests/json_serialization_tests.rs @@ -4,10 +4,10 @@ use crate::parser::parse; fn error_json_serialization() { let input = "(identifier) @foo.bar"; let result = parse(input).unwrap(); - let errors = result.errors(); + let errors = result.diagnostics(); assert_eq!(errors.len(), 1); - let json = serde_json::to_string_pretty(&errors[0]).unwrap(); + let json = serde_json::to_string_pretty(errors.iter().next().unwrap()).unwrap(); insta::assert_snapshot!(json, @r#" { @@ -30,10 +30,10 @@ fn error_json_serialization() { fn error_json_serialization_no_fix() { let input = "(identifier) @"; let result = parse(input).unwrap(); - let errors = result.errors(); + let errors = result.diagnostics(); assert_eq!(errors.len(), 1); - let json = serde_json::to_string_pretty(&errors[0]).unwrap(); + let json = serde_json::to_string_pretty(errors.iter().next().unwrap()).unwrap(); insta::assert_snapshot!(json, @r#" { diff --git a/crates/plotnik-lib/src/parser/tests/recovery/coverage_tests.rs b/crates/plotnik-lib/src/parser/tests/recovery/coverage_tests.rs index 07ba1396..21a2bb91 100644 --- a/crates/plotnik-lib/src/parser/tests/recovery/coverage_tests.rs +++ b/crates/plotnik-lib/src/parser/tests/recovery/coverage_tests.rs @@ -136,7 +136,7 @@ fn named_def_missing_equals_with_garbage() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r#" + insta::assert_snapshot!(query.dump_diagnostics(), @r#" error: bare identifier not allowed; nodes must be enclosed in parentheses, e.g., (identifier) | 1 | Expr ^^^ (identifier) @@ -161,7 +161,7 @@ fn named_def_missing_equals_recovers_to_next_def() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r#" + insta::assert_snapshot!(query.dump_diagnostics(), @r#" error: bare identifier not allowed; nodes must be enclosed in parentheses, e.g., (identifier) | 1 | Broken ^^^ diff --git a/crates/plotnik-lib/src/parser/tests/recovery/incomplete_tests.rs b/crates/plotnik-lib/src/parser/tests/recovery/incomplete_tests.rs index 023ad38f..bf9dfab6 100644 --- a/crates/plotnik-lib/src/parser/tests/recovery/incomplete_tests.rs +++ b/crates/plotnik-lib/src/parser/tests/recovery/incomplete_tests.rs @@ -9,7 +9,7 @@ fn missing_capture_name() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: expected capture name after '@' | 1 | (identifier) @ @@ -25,7 +25,7 @@ fn missing_field_value() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: expected expression after field name | 1 | (call name:) @@ -39,7 +39,7 @@ fn named_def_eof_after_equals() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: expected expression after '=' in named definition | 1 | Expr = @@ -55,7 +55,7 @@ fn missing_type_name() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: expected type name after '::' (e.g., ::MyType or ::string) | 1 | (identifier) @name :: @@ -71,7 +71,7 @@ fn missing_negated_field_name() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: expected field name after '!' (e.g., !value) | 1 | (call !) @@ -87,7 +87,7 @@ fn missing_subtype() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: expected subtype after '/' (e.g., expression/binary_expression) | 1 | (expression/) @@ -103,7 +103,7 @@ fn tagged_branch_missing_expression() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: expected expression after branch label | 1 | [Label:] @@ -117,7 +117,7 @@ fn type_annotation_missing_name_at_eof() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: expected type name after '::' (e.g., ::MyType or ::string) | 1 | (a) @x :: @@ -131,7 +131,7 @@ fn type_annotation_missing_name_with_bracket() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: expected type name after '::' (e.g., ::MyType or ::string) | 1 | [(a) @x :: ] @@ -147,7 +147,7 @@ fn type_annotation_invalid_token_after() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: expected type name after '::' (e.g., ::MyType or ::string) | 1 | (identifier) @name :: ( @@ -173,7 +173,7 @@ fn field_value_is_garbage() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: expected expression after field name | 1 | (call name: %%%) @@ -189,7 +189,7 @@ fn capture_with_invalid_char() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: expected capture name after '@' | 1 | (identifier) @123 @@ -203,7 +203,7 @@ fn bare_capture_at_eof_triggers_sync() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: capture '@' must follow an expression to capture | 1 | @ @@ -219,7 +219,7 @@ fn bare_capture_at_root() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: capture '@' must follow an expression to capture | 1 | @name @@ -239,7 +239,7 @@ fn capture_at_start_of_alternation() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: unexpected token; expected a child expression or closing delimiter | 1 | [@x (a)] @@ -259,7 +259,7 @@ fn mixed_valid_invalid_captures() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: capture '@' must follow an expression to capture | 1 | (a) @ok @ @name @@ -283,7 +283,7 @@ fn field_equals_typo_missing_value() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: '=' is not valid for field constraints | 1 | (call name = ) @@ -307,7 +307,7 @@ fn lowercase_branch_label_missing_expression() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: tagged alternation labels must be Capitalized (they map to enum variants) | 1 | [label:] diff --git a/crates/plotnik-lib/src/parser/tests/recovery/unclosed_tests.rs b/crates/plotnik-lib/src/parser/tests/recovery/unclosed_tests.rs index c5328091..25b29b8d 100644 --- a/crates/plotnik-lib/src/parser/tests/recovery/unclosed_tests.rs +++ b/crates/plotnik-lib/src/parser/tests/recovery/unclosed_tests.rs @@ -9,7 +9,7 @@ fn missing_paren() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: unclosed tree; expected ')' | 1 | (identifier @@ -27,7 +27,7 @@ fn missing_bracket() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: unclosed alternation; expected ']' | 1 | [(identifier) (string) @@ -45,7 +45,7 @@ fn missing_brace() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: unclosed sequence; expected '}' | 1 | {(a) (b) @@ -63,7 +63,7 @@ fn nested_unclosed() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: unclosed tree; expected ')' | 1 | (a (b (c) @@ -81,7 +81,7 @@ fn deeply_nested_unclosed() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: unclosed tree; expected ')' | 1 | (a (b (c (d @@ -99,7 +99,7 @@ fn unclosed_alternation_nested() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: unclosed tree; expected ')' | 1 | [(a) (b @@ -117,7 +117,7 @@ fn empty_parens() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: empty tree expression - expected node type or children | 1 | () @@ -134,7 +134,7 @@ fn unclosed_tree_shows_open_location() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: unclosed tree; expected ')' | 1 | (call @@ -154,7 +154,7 @@ fn unclosed_alternation_shows_open_location() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: unclosed alternation; expected ']' | 1 | [ @@ -175,7 +175,7 @@ fn unclosed_sequence_shows_open_location() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: unclosed sequence; expected '}' | 1 | { @@ -192,7 +192,7 @@ fn unclosed_double_quote_string() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r#" + insta::assert_snapshot!(query.dump_diagnostics(), @r#" error: unexpected token; expected a child expression or closing delimiter | 1 | (call "foo) @@ -212,7 +212,7 @@ fn unclosed_single_quote_string() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: unexpected token; expected a child expression or closing delimiter | 1 | (call 'foo) diff --git a/crates/plotnik-lib/src/parser/tests/recovery/unexpected_tests.rs b/crates/plotnik-lib/src/parser/tests/recovery/unexpected_tests.rs index df38ff0d..8ec7c124 100644 --- a/crates/plotnik-lib/src/parser/tests/recovery/unexpected_tests.rs +++ b/crates/plotnik-lib/src/parser/tests/recovery/unexpected_tests.rs @@ -9,7 +9,7 @@ fn unexpected_token() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r#" + insta::assert_snapshot!(query.dump_diagnostics(), @r#" error: unexpected token; expected an expression like (node), [choice], {sequence}, "literal", or _ | 1 | (identifier) ^^^ (string) @@ -29,7 +29,7 @@ fn multiple_consecutive_garbage() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r#" + insta::assert_snapshot!(query.dump_diagnostics(), @r#" error: unexpected token; expected an expression like (node), [choice], {sequence}, "literal", or _ | 1 | ^^^ $$$ %%% (ok) @@ -45,7 +45,7 @@ fn garbage_at_start() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r#" + insta::assert_snapshot!(query.dump_diagnostics(), @r#" error: unexpected token; expected an expression like (node), [choice], {sequence}, "literal", or _ | 1 | ^^^ (a) @@ -61,7 +61,7 @@ fn only_garbage() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r#" + insta::assert_snapshot!(query.dump_diagnostics(), @r#" error: unexpected token; expected an expression like (node), [choice], {sequence}, "literal", or _ | 1 | ^^^ $$$ @@ -77,7 +77,7 @@ fn garbage_inside_alternation() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: unexpected token; expected a child expression or closing delimiter | 1 | [(a) ^^^ (b)] @@ -93,7 +93,7 @@ fn garbage_inside_node() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: expected capture name after '@' | 1 | (a (b) @@@ (c)) (d) @@ -117,7 +117,7 @@ fn xml_tag_garbage() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r#" + insta::assert_snapshot!(query.dump_diagnostics(), @r#" error: unexpected token; expected an expression like (node), [choice], {sequence}, "literal", or _ | 1 |
(identifier)
@@ -137,7 +137,7 @@ fn xml_self_closing() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r#" + insta::assert_snapshot!(query.dump_diagnostics(), @r#" error: unexpected token; expected an expression like (node), [choice], {sequence}, "literal", or _ | 1 |
(a) @@ -153,7 +153,7 @@ fn predicate_unsupported() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r#" + insta::assert_snapshot!(query.dump_diagnostics(), @r#" error: tree-sitter predicates (#eq?, #match?, #set!, etc.) are not supported | 1 | (a (#eq? @x "foo") b) @@ -181,7 +181,7 @@ fn predicate_match() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r#" + insta::assert_snapshot!(query.dump_diagnostics(), @r#" error: tree-sitter predicates (#eq?, #match?, #set!, etc.) are not supported | 1 | (identifier) #match? @name "test" @@ -207,7 +207,7 @@ fn predicate_in_tree() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r#" + insta::assert_snapshot!(query.dump_diagnostics(), @r#" error: tree-sitter predicates (#eq?, #match?, #set!, etc.) are not supported | 1 | (function #eq? @name "test") @@ -231,7 +231,7 @@ fn predicate_in_alternation() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: unexpected token; expected a child expression or closing delimiter | 1 | [(a) #eq? (b)] @@ -247,7 +247,7 @@ fn predicate_in_sequence() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: tree-sitter predicates (#eq?, #match?, #set!, etc.) are not supported | 1 | {(a) #set! (b)} @@ -265,7 +265,7 @@ fn multiline_garbage_recovery() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: unexpected token; expected a child expression or closing delimiter | 2 | ^^^ @@ -285,7 +285,7 @@ fn top_level_garbage_recovery() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r#" + insta::assert_snapshot!(query.dump_diagnostics(), @r#" error: unexpected token; expected an expression like (node), [choice], {sequence}, "literal", or _ | 1 | Expr = (a) ^^^ Expr2 = (b) @@ -305,7 +305,7 @@ fn multiple_definitions_with_garbage_between() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r#" + insta::assert_snapshot!(query.dump_diagnostics(), @r#" error: unexpected token; expected an expression like (node), [choice], {sequence}, "literal", or _ | 2 | ^^^ @@ -325,7 +325,7 @@ fn alternation_recovery_to_capture() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: unexpected token; expected a child expression or closing delimiter | 1 | [^^^ @name] @@ -349,7 +349,7 @@ fn comma_between_defs() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r#" + insta::assert_snapshot!(query.dump_diagnostics(), @r#" error: unexpected token; expected an expression like (node), [choice], {sequence}, "literal", or _ | 1 | A = (a), B = (b) @@ -363,7 +363,7 @@ fn bare_colon_in_tree() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: unexpected token; expected a child expression or closing delimiter | 1 | (a : (b)) @@ -377,7 +377,7 @@ fn paren_close_inside_alternation() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r#" + insta::assert_snapshot!(query.dump_diagnostics(), @r#" error: expected closing ']' for alternation | 1 | [(a) ) (b)] @@ -399,7 +399,7 @@ fn bracket_close_inside_sequence() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r#" + insta::assert_snapshot!(query.dump_diagnostics(), @r#" error: expected closing '}' for sequence | 1 | {(a) ] (b)} @@ -421,7 +421,7 @@ fn paren_close_inside_sequence() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r#" + insta::assert_snapshot!(query.dump_diagnostics(), @r#" error: expected closing '}' for sequence | 1 | {(a) ) (b)} @@ -443,7 +443,7 @@ fn single_colon_type_annotation_followed_by_non_id() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r#" + insta::assert_snapshot!(query.dump_diagnostics(), @r#" error: unexpected token; expected an expression like (node), [choice], {sequence}, "literal", or _ | 1 | (a) @x : (b) @@ -461,7 +461,7 @@ fn single_colon_type_annotation_at_eof() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r#" + insta::assert_snapshot!(query.dump_diagnostics(), @r#" error: unexpected token; expected an expression like (node), [choice], {sequence}, "literal", or _ | 1 | (a) @x : diff --git a/crates/plotnik-lib/src/parser/tests/recovery/validation_tests.rs b/crates/plotnik-lib/src/parser/tests/recovery/validation_tests.rs index 76c61ff7..f5e13c88 100644 --- a/crates/plotnik-lib/src/parser/tests/recovery/validation_tests.rs +++ b/crates/plotnik-lib/src/parser/tests/recovery/validation_tests.rs @@ -10,7 +10,7 @@ fn ref_with_children_error() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: reference `Expr` cannot contain children | 2 | (Expr (child)) @@ -27,7 +27,7 @@ fn ref_with_multiple_children_error() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: reference `Expr` cannot contain children | 2 | (Expr (a) (b) @cap) @@ -44,7 +44,7 @@ fn ref_with_field_children_error() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: reference `Expr` cannot contain children | 2 | (Expr name: (identifier)) @@ -58,7 +58,7 @@ fn reference_with_supertype_syntax_error() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: references cannot use supertype syntax (/) | 1 | (RefName/subtype) @@ -74,7 +74,7 @@ fn mixed_tagged_and_untagged() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: mixed tagged and untagged branches in alternation | 1 | [Tagged: (a) (b) Another: (c)] @@ -92,7 +92,7 @@ fn error_with_unexpected_content() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: (ERROR) takes no arguments | 1 | (ERROR (something)) @@ -108,7 +108,7 @@ fn bare_error_keyword() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: ERROR and MISSING must be inside parentheses: (ERROR) or (MISSING ...) | 1 | ERROR @@ -124,7 +124,7 @@ fn bare_missing_keyword() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: ERROR and MISSING must be inside parentheses: (ERROR) or (MISSING ...) | 1 | MISSING @@ -140,7 +140,7 @@ fn upper_ident_in_alternation_not_followed_by_colon() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: undefined reference: `Expr` | 1 | [(Expr) (Statement)] @@ -160,7 +160,7 @@ fn upper_ident_not_followed_by_equals_is_expression() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: undefined reference: `Expr` | 1 | (Expr) @@ -176,7 +176,7 @@ fn bare_upper_ident_not_followed_by_equals_is_error() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: bare identifier not allowed; nodes must be enclosed in parentheses, e.g., (identifier) | 1 | Expr @@ -192,7 +192,7 @@ fn named_def_missing_equals() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: bare identifier not allowed; nodes must be enclosed in parentheses, e.g., (identifier) | 1 | Expr (identifier) @@ -214,7 +214,7 @@ fn unnamed_def_not_allowed_in_middle() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: unnamed definition must be last in file; add a name: `Name = (first)` | 1 | (first) @@ -232,7 +232,7 @@ fn multiple_unnamed_defs_errors_for_all_but_last() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: unnamed definition must be last in file; add a name: `Name = (first)` | 1 | (first) @@ -252,7 +252,7 @@ fn capture_space_after_dot_is_anchor() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: unnamed definition must be last in file; add a name: `Name = (identifier) @foo` | 1 | (identifier) @foo . (other) @@ -270,7 +270,7 @@ fn def_name_lowercase_error() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: definition names must start with uppercase | 1 | lowercase = (x) @@ -292,7 +292,7 @@ fn def_name_snake_case_suggests_pascal() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: definition names must start with uppercase | 1 | my_expr = (identifier) @@ -314,7 +314,7 @@ fn def_name_kebab_case_suggests_pascal() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: definition names must start with uppercase | 1 | my-expr = (identifier) @@ -336,7 +336,7 @@ fn def_name_dotted_suggests_pascal() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: definition names must start with uppercase | 1 | my.expr = (identifier) @@ -356,7 +356,7 @@ fn def_name_with_underscores_error() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: definition names cannot contain separators | 1 | Some_Thing = (x) @@ -376,7 +376,7 @@ fn def_name_with_hyphens_error() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: definition names cannot contain separators | 1 | Some-Thing = (x) @@ -398,7 +398,7 @@ fn capture_name_pascal_case_error() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: capture names must start with lowercase | 1 | (a) @Name @@ -420,7 +420,7 @@ fn capture_name_pascal_case_with_hyphens_error() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: capture names cannot contain hyphens | 1 | (a) @My-Name @@ -442,7 +442,7 @@ fn capture_name_with_hyphens_error() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: capture names cannot contain hyphens | 1 | (a) @my-name @@ -464,7 +464,7 @@ fn capture_dotted_error() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: capture names cannot contain dots | 1 | (identifier) @foo.bar @@ -486,7 +486,7 @@ fn capture_dotted_multiple_parts() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: capture names cannot contain dots | 1 | (identifier) @foo.bar.baz @@ -508,7 +508,7 @@ fn capture_dotted_followed_by_field() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: capture names cannot contain dots | 1 | (node) @foo.bar name: (other) @@ -534,7 +534,7 @@ fn capture_space_after_dot_breaks_chain() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: capture names cannot contain dots | 1 | (identifier) @foo. bar @@ -564,7 +564,7 @@ fn capture_hyphenated_error() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: capture names cannot contain hyphens | 1 | (identifier) @foo-bar @@ -586,7 +586,7 @@ fn capture_hyphenated_multiple() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: capture names cannot contain hyphens | 1 | (identifier) @foo-bar-baz @@ -608,7 +608,7 @@ fn capture_mixed_dots_and_hyphens() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: capture names cannot contain dots | 1 | (identifier) @foo.bar-baz @@ -630,7 +630,7 @@ fn field_name_pascal_case_error() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: field names must start with lowercase | 1 | (call Name: (a)) @@ -650,7 +650,7 @@ fn field_name_with_dots_error() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: field names cannot contain dots | 1 | (call foo.bar: (x)) @@ -670,7 +670,7 @@ fn field_name_with_hyphens_error() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: field names cannot contain hyphens | 1 | (call foo-bar: (x)) @@ -692,7 +692,7 @@ fn negated_field_with_upper_ident_parses() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: field names must start with lowercase | 1 | (call !Arguments) @@ -714,7 +714,7 @@ fn branch_label_snake_case_suggests_pascal() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: branch labels cannot contain separators | 1 | [My_branch: (a) Other: (b)] @@ -736,7 +736,7 @@ fn branch_label_kebab_case_suggests_pascal() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: branch labels cannot contain separators | 1 | [My-branch: (a) Other: (b)] @@ -758,7 +758,7 @@ fn branch_label_dotted_suggests_pascal() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: branch labels cannot contain separators | 1 | [My.branch: (a) Other: (b)] @@ -778,7 +778,7 @@ fn branch_label_with_underscores_error() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: branch labels cannot contain separators | 1 | [Some_Label: (x)] @@ -798,7 +798,7 @@ fn branch_label_with_hyphens_error() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: branch labels cannot contain separators | 1 | [Some-Label: (x)] @@ -823,7 +823,7 @@ fn lowercase_branch_label() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: tagged alternation labels must be Capitalized (they map to enum variants) | 2 | left: (a) @@ -855,7 +855,7 @@ fn lowercase_branch_label_suggests_capitalized() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: tagged alternation labels must be Capitalized (they map to enum variants) | 1 | [first: (a) Second: (b)] @@ -875,7 +875,7 @@ fn mixed_case_branch_labels() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: tagged alternation labels must be Capitalized (they map to enum variants) | 1 | [foo: (a) Bar: (b)] @@ -897,7 +897,7 @@ fn type_annotation_dotted_suggests_pascal() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: type names cannot contain dots or hyphens | 1 | (a) @x :: My.Type @@ -919,7 +919,7 @@ fn type_annotation_kebab_suggests_pascal() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: type names cannot contain dots or hyphens | 1 | (a) @x :: My-Type @@ -939,7 +939,7 @@ fn type_name_with_dots_error() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: type names cannot contain dots or hyphens | 1 | (x) @name :: Some.Type @@ -959,7 +959,7 @@ fn type_name_with_hyphens_error() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: type names cannot contain dots or hyphens | 1 | (x) @name :: Some-Type @@ -979,7 +979,7 @@ fn comma_in_node_children() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: ',' is not valid syntax; plotnik uses whitespace for separation | 1 | (node (a), (b)) @@ -999,7 +999,7 @@ fn comma_in_alternation() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: ',' is not valid syntax; plotnik uses whitespace for separation | 1 | [(a), (b), (c)] @@ -1029,7 +1029,7 @@ fn comma_in_sequence() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: ',' is not valid syntax; plotnik uses whitespace for separation | 1 | {(a), (b)} @@ -1049,7 +1049,7 @@ fn pipe_in_alternation() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: '|' is not valid syntax; plotnik uses whitespace for separation | 1 | [(a) | (b) | (c)] @@ -1081,7 +1081,7 @@ fn pipe_between_branches() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: '|' is not valid syntax; plotnik uses whitespace for separation | 1 | [(a) | (b)] @@ -1101,7 +1101,7 @@ fn pipe_in_tree() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: '|' is not valid syntax; plotnik uses whitespace for separation | 1 | (a | b) @@ -1125,7 +1125,7 @@ fn pipe_in_sequence() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: '|' is not valid syntax; plotnik uses whitespace for separation | 1 | {(a) | (b)} @@ -1145,7 +1145,7 @@ fn field_equals_typo() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: '=' is not valid for field constraints | 1 | (node name = (identifier)) @@ -1165,7 +1165,7 @@ fn field_equals_typo_no_space() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: '=' is not valid for field constraints | 1 | (node name=(identifier)) @@ -1185,7 +1185,7 @@ fn field_equals_typo_no_expression() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: '=' is not valid for field constraints | 1 | (call name=) @@ -1211,7 +1211,7 @@ fn field_equals_typo_in_tree() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: '=' is not valid for field constraints | 1 | (call name = (identifier)) @@ -1231,7 +1231,7 @@ fn single_colon_type_annotation() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: single colon is not valid for type annotations | 1 | (identifier) @name : Type @@ -1250,7 +1250,7 @@ fn single_colon_type_annotation_no_space() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: single colon is not valid for type annotations | 1 | (identifier) @name:Type @@ -1271,7 +1271,7 @@ fn single_colon_type_annotation_with_space() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: single colon is not valid for type annotations | 1 | (a) @x : Type @@ -1290,7 +1290,7 @@ fn single_colon_primitive_type() { let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: capture '@' must follow an expression to capture | 1 | @val : string diff --git a/crates/plotnik-lib/src/parser/tests/validation/naming.rs b/crates/plotnik-lib/src/parser/tests/validation/naming.rs deleted file mode 100644 index 60274e92..00000000 --- a/crates/plotnik-lib/src/parser/tests/validation/naming.rs +++ /dev/null @@ -1,86 +0,0 @@ -//! Naming validation tests: capture names, definition names, branch labels, field names, type names. - -use crate::Query; -use indoc::indoc; - -// ============================================================================= -// Capture names -// ============================================================================= - -#[test] -fn capture_dotted_error() { - let input = indoc! {r#" - (identifier) @foo.bar - "#}; - - let query = Query::new(input).unwrap(); - assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @""); -} - -#[test] -fn capture_dotted_multiple_parts() { - let input = indoc! {r#" - (identifier) @a.b.c.d - "#}; - - let query = Query::new(input).unwrap(); - assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @""); -} - -#[test] -fn capture_dotted_followed_by_field() { - let input = indoc! {r#" - (call - (identifier) @foo.bar - name: (identifier)) - "#}; - - let query = Query::new(input).unwrap(); - assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @""); -} - -#[test] -fn capture_hyphenated_error() { - let input = indoc! {r#" - (identifier) @foo-bar - "#}; - - let query = Query::new(input).unwrap(); - assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @""); -} - -#[test] -fn capture_hyphenated_multiple() { - let input = indoc! {r#" - (identifier) @a-b-c-d - "#}; - - let query = Query::new(input).unwrap(); - assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @""); -} - -#[test] -fn capture_mixed_dots_and_hyphens() { - let input = indoc! {r#" - (identifier) @foo.bar-baz - "#}; - - let query = Query::new(input).unwrap(); - assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @""); -} - -#[test] -fn capture_name_pascal_case_error() { - let input = indoc! {r#" - (identifier) @MyCapture - "#}; - - let query = Query::new(input).unwrap(); - assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @""); \ No newline at end of file diff --git a/crates/plotnik-lib/src/query/alt_kind.rs b/crates/plotnik-lib/src/query/alt_kind.rs index 35a94bfa..415ad22e 100644 --- a/crates/plotnik-lib/src/query/alt_kind.rs +++ b/crates/plotnik-lib/src/query/alt_kind.rs @@ -8,11 +8,11 @@ use rowan::TextRange; use super::invariants::{ assert_alt_no_bare_exprs, assert_root_no_bare_exprs, ensure_both_branch_kinds, }; +use crate::diagnostics::{DiagnosticMessage, DiagnosticStage, Diagnostics, RelatedInfo}; use crate::parser::{Alt, AltKind, Branch, Expr, Root}; -use crate::parser::{Diagnostic, ErrorStage, RelatedInfo}; -pub fn validate(root: &Root) -> Vec { - let mut errors = Vec::new(); +pub fn validate(root: &Root) -> Diagnostics { + let mut errors = Diagnostics::new(); for def in root.defs() { if let Some(body) = def.body() { @@ -25,7 +25,7 @@ pub fn validate(root: &Root) -> Vec { errors } -fn validate_expr(expr: &Expr, errors: &mut Vec) { +fn validate_expr(expr: &Expr, errors: &mut Diagnostics) { match expr { Expr::Alt(alt) => { check_mixed_alternation(alt, errors); @@ -69,7 +69,7 @@ fn validate_expr(expr: &Expr, errors: &mut Vec) { } } -fn check_mixed_alternation(alt: &Alt, errors: &mut Vec) { +fn check_mixed_alternation(alt: &Alt, errors: &mut Diagnostics) { if alt.kind() != AltKind::Mixed { return; } @@ -102,12 +102,12 @@ fn check_mixed_alternation(alt: &Alt, errors: &mut Vec) { let untagged_range = branch_range(untagged_branch); - let error = Diagnostic::error( + let error = DiagnosticMessage::error( untagged_range, "mixed tagged and untagged branches in alternation", ) .with_related(RelatedInfo::new(tagged_range, "tagged branch here")) - .with_stage(ErrorStage::Validate); + .with_stage(DiagnosticStage::Validate); errors.push(error); } diff --git a/crates/plotnik-lib/src/query/alt_kind_tests.rs b/crates/plotnik-lib/src/query/alt_kind_tests.rs index 22842b41..4d1e7c95 100644 --- a/crates/plotnik-lib/src/query/alt_kind_tests.rs +++ b/crates/plotnik-lib/src/query/alt_kind_tests.rs @@ -34,7 +34,7 @@ fn untagged_alternation_valid() { fn mixed_alternation_tagged_first() { let query = Query::new("[A: (a) (b)]").unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: mixed tagged and untagged branches in alternation | 1 | [A: (a) (b)] @@ -56,7 +56,7 @@ fn mixed_alternation_untagged_first() { ) .unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: mixed tagged and untagged branches in alternation | 3 | (a) @@ -70,7 +70,7 @@ fn mixed_alternation_untagged_first() { fn nested_mixed_alternation() { let query = Query::new("(call [A: (a) (b)])").unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: mixed tagged and untagged branches in alternation | 1 | (call [A: (a) (b)]) @@ -84,7 +84,7 @@ fn nested_mixed_alternation() { fn multiple_mixed_alternations() { let query = Query::new("(foo [A: (a) (b)] [C: (c) (d)])").unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: mixed tagged and untagged branches in alternation | 1 | (foo [A: (a) (b)] [C: (c) (d)]) diff --git a/crates/plotnik-lib/src/query/dump.rs b/crates/plotnik-lib/src/query/dump.rs index 2e2eb384..c8f29a54 100644 --- a/crates/plotnik-lib/src/query/dump.rs +++ b/crates/plotnik-lib/src/query/dump.rs @@ -1,7 +1,6 @@ #[cfg(test)] mod test_helpers { use crate::Query; - use crate::parser::{RenderOptions, render_diagnostics}; impl Query<'_> { pub fn dump_cst(&self) -> String { @@ -24,8 +23,8 @@ mod test_helpers { self.printer().only_symbols(true).dump() } - pub fn dump_errors(&self) -> String { - render_diagnostics(self.source, &self.errors, None, RenderOptions::plain()) + pub fn dump_diagnostics(&self) -> String { + self.diagnostics_printer().render() } } } diff --git a/crates/plotnik-lib/src/query/errors.rs b/crates/plotnik-lib/src/query/errors.rs index f3e59425..795da2f1 100644 --- a/crates/plotnik-lib/src/query/errors.rs +++ b/crates/plotnik-lib/src/query/errors.rs @@ -1,87 +1,73 @@ -use crate::parser::{Diagnostic, ErrorStage, RenderOptions, Severity, render_diagnostics}; +use crate::diagnostics::{DiagnosticMessage, DiagnosticStage, DiagnosticsPrinter}; use super::Query; impl Query<'_> { - pub fn diagnostics(&self) -> &[Diagnostic] { - &self.errors + pub fn diagnostics(&self) -> &[DiagnosticMessage] { + self.diagnostics.as_slice() } - /// Backwards compatibility alias for diagnostics() - pub fn errors(&self) -> &[Diagnostic] { - &self.errors - } - - /// Query is valid if there are no errors (warnings are allowed). + /// Query is valid if there are no error-severity diagnostics (warnings are allowed). pub fn is_valid(&self) -> bool { - !self.errors.iter().any(|d| d.is_error()) + !self.diagnostics.iter().any(|d| d.is_error()) } pub fn has_errors(&self) -> bool { - self.errors.iter().any(|d| d.is_error()) + self.diagnostics.iter().any(|d| d.is_error()) } pub fn has_warnings(&self) -> bool { - self.errors.iter().any(|d| d.is_warning()) - } - - pub fn errors_only(&self) -> Vec<&Diagnostic> { - self.errors.iter().filter(|d| d.is_error()).collect() + self.diagnostics.iter().any(|d| d.is_warning()) } - pub fn warnings_only(&self) -> Vec<&Diagnostic> { - self.errors.iter().filter(|d| d.is_warning()).collect() + pub fn errors_only(&self) -> Vec<&DiagnosticMessage> { + self.diagnostics.iter().filter(|d| d.is_error()).collect() } - pub fn diagnostics_for_stage(&self, stage: ErrorStage) -> Vec<&Diagnostic> { - self.errors.iter().filter(|d| d.stage == stage).collect() + pub fn warnings_only(&self) -> Vec<&DiagnosticMessage> { + self.diagnostics.iter().filter(|d| d.is_warning()).collect() } - /// Backwards compatibility alias - pub fn errors_for_stage(&self, stage: ErrorStage) -> Vec<&Diagnostic> { - self.diagnostics_for_stage(stage) + pub fn diagnostics_for_stage(&self, stage: DiagnosticStage) -> Vec<&DiagnosticMessage> { + self.diagnostics + .iter() + .filter(|d| d.stage == stage) + .collect() } pub fn has_parse_errors(&self) -> bool { - self.errors + self.diagnostics .iter() - .any(|d| d.stage == ErrorStage::Parse && d.is_error()) + .any(|d| d.stage == DiagnosticStage::Parse && d.is_error()) } pub fn has_validate_errors(&self) -> bool { - self.errors + self.diagnostics .iter() - .any(|d| d.stage == ErrorStage::Validate && d.is_error()) + .any(|d| d.stage == DiagnosticStage::Validate && d.is_error()) } pub fn has_resolve_errors(&self) -> bool { - self.errors + self.diagnostics .iter() - .any(|d| d.stage == ErrorStage::Resolve && d.is_error()) + .any(|d| d.stage == DiagnosticStage::Resolve && d.is_error()) } pub fn has_escape_errors(&self) -> bool { - self.errors + self.diagnostics .iter() - .any(|d| d.stage == ErrorStage::Escape && d.is_error()) + .any(|d| d.stage == DiagnosticStage::Escape && d.is_error()) } - pub fn render_diagnostics(&self, options: RenderOptions) -> String { - render_diagnostics(self.source, &self.errors, None, options) + pub fn diagnostics_printer(&self) -> DiagnosticsPrinter<'_, '_> { + self.diagnostics.printer().source(self.source) } - pub fn error_count(&self) -> usize { - self.errors.iter().filter(|d| d.is_error()).count() - } - - pub fn warning_count(&self) -> usize { - self.errors.iter().filter(|d| d.is_warning()).count() + pub fn render_diagnostics_colored(&self, colored: bool) -> String { + self.diagnostics_printer().colored(colored).render() } - pub fn filter_by_severity(&self, severity: Severity) -> Vec<&Diagnostic> { - self.errors - .iter() - .filter(|d| d.severity == severity) - .collect() + pub fn error_count(&self) -> usize { + self.diagnostics.iter().filter(|d| d.is_error()).count() } } diff --git a/crates/plotnik-lib/src/query/errors_tests.rs b/crates/plotnik-lib/src/query/errors_tests.rs deleted file mode 100644 index 2e1bbb0b..00000000 --- a/crates/plotnik-lib/src/query/errors_tests.rs +++ /dev/null @@ -1,93 +0,0 @@ -use super::Query; -use crate::parser::{ErrorStage, RenderOptions, Severity}; - -#[test] -fn diagnostics_alias() { - let q = Query::new("(valid)").unwrap(); - assert_eq!(q.diagnostics().len(), q.errors().len()); -} - -#[test] -fn error_stage_filtering() { - let q = Query::new("(unclosed").unwrap(); - assert!(q.has_parse_errors()); - assert!(!q.has_resolve_errors()); - assert!(!q.has_escape_errors()); - assert_eq!(q.errors_for_stage(ErrorStage::Parse).len(), 1); - - let q = Query::new("(call (Undefined))").unwrap(); - assert!(!q.has_parse_errors()); - assert!(q.has_resolve_errors()); - assert!(!q.has_escape_errors()); - assert_eq!(q.errors_for_stage(ErrorStage::Resolve).len(), 1); - - let q = Query::new("[A: (a) (b)]").unwrap(); - assert!(!q.has_parse_errors()); - assert!(q.has_validate_errors()); - assert!(!q.has_resolve_errors()); - assert!(!q.has_escape_errors()); - assert_eq!(q.errors_for_stage(ErrorStage::Validate).len(), 1); - - let q = Query::new("Expr = (call (Expr))").unwrap(); - assert!(!q.has_parse_errors()); - assert!(!q.has_validate_errors()); - assert!(!q.has_resolve_errors()); - assert!(q.has_escape_errors()); - assert_eq!(q.errors_for_stage(ErrorStage::Escape).len(), 1); - - let q = Query::new("Expr = (call (Expr)) (unclosed").unwrap(); - assert!(q.has_parse_errors()); - assert!(!q.has_resolve_errors()); - assert!(q.has_escape_errors()); -} - -#[test] -fn is_valid_ignores_warnings() { - let q = Query::new("(valid)").unwrap(); - assert!(q.is_valid()); - assert!(!q.has_errors()); - assert!(!q.has_warnings()); - assert_eq!(q.error_count(), 0); - assert_eq!(q.warning_count(), 0); -} - -#[test] -fn error_and_warning_counts() { - let q = Query::new("(unclosed").unwrap(); - assert!(q.has_errors()); - assert!(!q.has_warnings()); - assert_eq!(q.error_count(), 1); - assert_eq!(q.warning_count(), 0); -} - -#[test] -fn errors_only_and_warnings_only() { - let q = Query::new("(unclosed").unwrap(); - let errors = q.errors_only(); - let warnings = q.warnings_only(); - assert_eq!(errors.len(), 1); - assert!(warnings.is_empty()); -} - -#[test] -fn render_diagnostics_method() { - let q = Query::new("(unclosed").unwrap(); - let rendered = q.render_diagnostics(RenderOptions::plain()); - insta::assert_snapshot!(rendered, @r" - error: unclosed tree; expected ')' - | - 1 | (unclosed - | - ^ unclosed tree; expected ')' - | | - | tree started here - "); -} - -#[test] -fn filter_by_severity() { - let q = Query::new("(unclosed").unwrap(); - let errors = q.filter_by_severity(Severity::Error); - let warnings = q.filter_by_severity(Severity::Warning); - assert_eq!(errors.len(), 1); - assert!(warnings.is_empty()); -} diff --git a/crates/plotnik-lib/src/query/mod.rs b/crates/plotnik-lib/src/query/mod.rs index 092c1035..371f8a02 100644 --- a/crates/plotnik-lib/src/query/mod.rs +++ b/crates/plotnik-lib/src/query/mod.rs @@ -14,8 +14,6 @@ pub mod shape_cardinalities; #[cfg(test)] mod alt_kind_tests; #[cfg(test)] -mod errors_tests; -#[cfg(test)] mod mod_tests; #[cfg(test)] mod named_defs_tests; @@ -29,9 +27,9 @@ mod shape_cardinalities_tests; use std::collections::HashMap; use crate::Result; +use crate::diagnostics::Diagnostics; use crate::parser::lexer::lex; -use crate::parser::{self, Parser}; -use crate::parser::{Diagnostic, Parse, Root, SyntaxNode}; +use crate::parser::{self, Parse, Parser, Root, SyntaxNode}; use named_defs::SymbolTable; use shape_cardinalities::ShapeCardinality; @@ -93,14 +91,14 @@ impl<'a> QueryBuilder<'a> { /// A parsed and analyzed query. /// /// Construction succeeds unless fuel limits are exceeded. -/// Check [`is_valid`](Self::is_valid) or [`errors`](Self::errors) -/// to determine if the query has syntax/semantic errors. +/// Check [`is_valid`](Self::is_valid) or [`diagnostics`](Self::diagnostics) +/// to determine if the query has syntax/semantic issues. #[derive(Debug, Clone)] pub struct Query<'a> { source: &'a str, parse: Parse, symbols: SymbolTable, - errors: Vec, + diagnostics: Diagnostics, shape_cardinalities: HashMap, } @@ -108,7 +106,7 @@ impl<'a> Query<'a> { /// Parse and analyze a query from source text. /// /// Returns `Err` if fuel limits are exceeded. - /// Syntax/semantic errors are collected and accessible via [`errors`](Self::errors). + /// Syntax/semantic diagnostics are collected and accessible via [`diagnostics`](Self::diagnostics). pub fn new(source: &'a str) -> Result { QueryBuilder::new(source).build() } @@ -122,22 +120,21 @@ impl<'a> Query<'a> { fn from_parse(source: &'a str, parse: Parse) -> Self { let root = Root::cast(parse.syntax()).expect("parser always produces Root"); - let mut errors = parse.errors().to_vec(); + let mut diagnostics = parse.diagnostics().clone(); - let alt_kind_errors = alt_kind::validate(&root); - errors.extend(alt_kind_errors); + let alt_kind_diags = alt_kind::validate(&root); + diagnostics.extend(alt_kind_diags); let resolve_result = named_defs::resolve(&root); - errors.extend(resolve_result.errors); + diagnostics.extend(resolve_result.diagnostics); - let ref_cycle_errors = ref_cycles::validate(&root, &resolve_result.symbols); - errors.extend(ref_cycle_errors); + let ref_cycle_diags = ref_cycles::validate(&root, &resolve_result.symbols); + diagnostics.extend(ref_cycle_diags); - let shape_cardinalities = if errors.is_empty() { + let shape_cardinalities = if diagnostics.is_empty() { let cards = shape_cardinalities::infer(&root, &resolve_result.symbols); - let shape_errors = - shape_cardinalities::validate(&root, &resolve_result.symbols, &cards); - errors.extend(shape_errors); + let shape_diags = shape_cardinalities::validate(&root, &resolve_result.symbols, &cards); + diagnostics.extend(shape_diags); cards } else { HashMap::new() @@ -147,7 +144,7 @@ impl<'a> Query<'a> { source, parse, symbols: resolve_result.symbols, - errors, + diagnostics, shape_cardinalities, } } diff --git a/crates/plotnik-lib/src/query/mod_tests.rs b/crates/plotnik-lib/src/query/mod_tests.rs index 4718f1bd..56ce056c 100644 --- a/crates/plotnik-lib/src/query/mod_tests.rs +++ b/crates/plotnik-lib/src/query/mod_tests.rs @@ -11,19 +11,19 @@ fn valid_query() { fn parse_error() { let q = Query::new("(unclosed").unwrap(); assert!(!q.is_valid()); - assert!(q.dump_errors().contains("expected")); + assert!(q.dump_diagnostics().contains("expected")); } #[test] fn resolution_error() { let q = Query::new("(call (Undefined))").unwrap(); assert!(!q.is_valid()); - assert!(q.dump_errors().contains("undefined reference")); + assert!(q.dump_diagnostics().contains("undefined reference")); } #[test] fn combined_errors() { let q = Query::new("(call (Undefined) extra)").unwrap(); assert!(!q.is_valid()); - assert!(!q.errors().is_empty()); + assert!(!q.diagnostics().is_empty()); } diff --git a/crates/plotnik-lib/src/query/named_defs.rs b/crates/plotnik-lib/src/query/named_defs.rs index cf3f771f..ad8740c5 100644 --- a/crates/plotnik-lib/src/query/named_defs.rs +++ b/crates/plotnik-lib/src/query/named_defs.rs @@ -7,7 +7,7 @@ use indexmap::{IndexMap, IndexSet}; use rowan::TextRange; -use crate::parser::{Diagnostic, ErrorStage}; +use crate::diagnostics::{DiagnosticMessage, DiagnosticStage, Diagnostics}; use crate::parser::{Expr, Ref, Root}; #[derive(Debug, Clone)] @@ -25,7 +25,7 @@ pub struct DefInfo { #[derive(Debug)] pub struct ResolveResult { pub symbols: SymbolTable, - pub errors: Vec, + pub diagnostics: Diagnostics, } impl SymbolTable { @@ -52,7 +52,7 @@ impl SymbolTable { pub fn resolve(root: &Root) -> ResolveResult { let mut defs = IndexMap::new(); - let mut errors = Vec::new(); + let mut diagnostics = Diagnostics::new(); // Pass 1: collect definitions for def in root.defs() { @@ -64,9 +64,9 @@ pub fn resolve(root: &Root) -> ResolveResult { let range = name_token.text_range(); if defs.contains_key(&name) { - errors.push( - Diagnostic::error(range, format!("duplicate definition: `{}`", name)) - .with_stage(ErrorStage::Resolve), + diagnostics.push( + DiagnosticMessage::error(range, format!("duplicate definition: `{}`", name)) + .with_stage(DiagnosticStage::Resolve), ); continue; } @@ -83,7 +83,7 @@ pub fn resolve(root: &Root) -> ResolveResult { // Pass 2: check references for def in root.defs() { let Some(body) = def.body() else { continue }; - collect_reference_errors(&body, &symbols, &mut errors); + collect_reference_diagnostics(&body, &symbols, &mut diagnostics); } // Parser wraps all top-level exprs in Def nodes, so this should be empty @@ -92,7 +92,10 @@ pub fn resolve(root: &Root) -> ResolveResult { "named_defs: unexpected bare Expr in Root (parser should wrap in Def)" ); - ResolveResult { symbols, errors } + ResolveResult { + symbols, + diagnostics, + } } fn collect_refs(expr: &Expr, refs: &mut IndexSet) { @@ -138,20 +141,24 @@ fn collect_refs(expr: &Expr, refs: &mut IndexSet) { } } -fn collect_reference_errors(expr: &Expr, symbols: &SymbolTable, errors: &mut Vec) { +fn collect_reference_diagnostics( + expr: &Expr, + symbols: &SymbolTable, + diagnostics: &mut Diagnostics, +) { match expr { Expr::Ref(r) => { - check_ref_reference(r, symbols, errors); + check_ref_diagnostic(r, symbols, diagnostics); } Expr::Tree(tree) => { for child in tree.children() { - collect_reference_errors(&child, symbols, errors); + collect_reference_diagnostics(&child, symbols, diagnostics); } } Expr::Alt(alt) => { for branch in alt.branches() { let Some(body) = branch.body() else { continue }; - collect_reference_errors(&body, symbols, errors); + collect_reference_diagnostics(&body, symbols, diagnostics); } // Parser wraps all alt children in Branch nodes assert!( @@ -161,26 +168,26 @@ fn collect_reference_errors(expr: &Expr, symbols: &SymbolTable, errors: &mut Vec } Expr::Seq(seq) => { for child in seq.children() { - collect_reference_errors(&child, symbols, errors); + collect_reference_diagnostics(&child, symbols, diagnostics); } } Expr::Capture(cap) => { let Some(inner) = cap.inner() else { return }; - collect_reference_errors(&inner, symbols, errors); + collect_reference_diagnostics(&inner, symbols, diagnostics); } Expr::Quantifier(q) => { let Some(inner) = q.inner() else { return }; - collect_reference_errors(&inner, symbols, errors); + collect_reference_diagnostics(&inner, symbols, diagnostics); } Expr::Field(f) => { let Some(value) = f.value() else { return }; - collect_reference_errors(&value, symbols, errors); + collect_reference_diagnostics(&value, symbols, diagnostics); } Expr::Str(_) | Expr::Wildcard(_) | Expr::Anchor(_) | Expr::NegatedField(_) => {} } } -fn check_ref_reference(r: &Ref, symbols: &SymbolTable, errors: &mut Vec) { +fn check_ref_diagnostic(r: &Ref, symbols: &SymbolTable, diagnostics: &mut Diagnostics) { let Some(name_token) = r.name() else { return }; let name = name_token.text(); @@ -188,11 +195,11 @@ fn check_ref_reference(r: &Ref, symbols: &SymbolTable, errors: &mut Vec Vec { +pub fn validate(root: &Root, symbols: &SymbolTable) -> Diagnostics { let sccs = find_sccs(symbols); - let mut errors = Vec::new(); + let mut errors = Diagnostics::new(); for scc in sccs { if scc.len() == 1 { @@ -293,7 +293,7 @@ fn build_cycle_chain(root: &Root, symbols: &SymbolTable, scc: &[String]) -> Vec< .collect() } -fn make_error(primary_name: &str, scc: &[String], related: Vec) -> Diagnostic { +fn make_error(primary_name: &str, scc: &[String], related: Vec) -> DiagnosticMessage { let cycle_str = if scc.len() == 1 { format!("`{}` → `{}`", primary_name, primary_name) } else { @@ -307,7 +307,7 @@ fn make_error(primary_name: &str, scc: &[String], related: Vec) -> .map(|r| r.range) .unwrap_or_else(|| TextRange::empty(0.into())); - Diagnostic::error( + DiagnosticMessage::error( range, format!( "recursive pattern can never match: cycle {} has no escape path", @@ -315,5 +315,5 @@ fn make_error(primary_name: &str, scc: &[String], related: Vec) -> ), ) .with_related_many(related) - .with_stage(ErrorStage::Escape) + .with_stage(DiagnosticStage::Escape) } diff --git a/crates/plotnik-lib/src/query/ref_cycles_tests.rs b/crates/plotnik-lib/src/query/ref_cycles_tests.rs index a4c145eb..0cc79421 100644 --- a/crates/plotnik-lib/src/query/ref_cycles_tests.rs +++ b/crates/plotnik-lib/src/query/ref_cycles_tests.rs @@ -23,7 +23,7 @@ fn escape_via_star() { fn no_escape_via_plus() { let query = Query::new("E = (call (E)+)").unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: recursive pattern can never match: cycle `E` → `E` has no escape path | 1 | E = (call (E)+) @@ -51,7 +51,7 @@ fn lazy_quantifiers_same_as_greedy() { fn recursion_in_tree_child() { let query = Query::new("E = (call (E))").unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: recursive pattern can never match: cycle `E` → `E` has no escape path | 1 | E = (call (E)) @@ -66,21 +66,21 @@ fn recursion_in_tree_child() { fn recursion_in_field() { let query = Query::new("E = (call body: (E))").unwrap(); assert!(!query.is_valid()); - assert!(query.dump_errors().contains("recursive pattern")); + assert!(query.dump_diagnostics().contains("recursive pattern")); } #[test] fn recursion_in_capture() { let query = Query::new("E = (call (E) @inner)").unwrap(); assert!(!query.is_valid()); - assert!(query.dump_errors().contains("recursive pattern")); + assert!(query.dump_diagnostics().contains("recursive pattern")); } #[test] fn recursion_in_sequence() { let query = Query::new("E = (call {(a) (E)})").unwrap(); assert!(!query.is_valid()); - assert!(query.dump_errors().contains("recursive pattern")); + assert!(query.dump_diagnostics().contains("recursive pattern")); } #[test] @@ -97,7 +97,7 @@ fn mutual_recursion_no_escape() { "#}; let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: recursive pattern can never match: cycle `B` → `A` → `B` has no escape path | 1 | A = (foo (B)) @@ -129,7 +129,7 @@ fn three_way_cycle_no_escape() { "#}; let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - assert!(query.dump_errors().contains("recursive pattern")); + assert!(query.dump_diagnostics().contains("recursive pattern")); } #[test] @@ -153,7 +153,7 @@ fn diamond_dependency() { "#}; let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - assert!(query.dump_errors().contains("recursive pattern")); + assert!(query.dump_diagnostics().contains("recursive pattern")); } #[test] @@ -164,7 +164,7 @@ fn cycle_ref_in_field() { "#}; let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: recursive pattern can never match: cycle `B` → `A` → `B` has no escape path | 1 | A = (foo body: (B)) @@ -185,7 +185,7 @@ fn cycle_ref_in_capture() { "#}; let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: recursive pattern can never match: cycle `B` → `A` → `B` has no escape path | 1 | A = (foo (B) @cap) @@ -206,7 +206,7 @@ fn cycle_ref_in_sequence() { "#}; let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: recursive pattern can never match: cycle `B` → `A` → `B` has no escape path | 1 | A = (foo {(x) (B)}) @@ -237,7 +237,7 @@ fn cycle_with_plus_no_escape() { "#}; let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - assert!(query.dump_errors().contains("recursive pattern")); + assert!(query.dump_diagnostics().contains("recursive pattern")); } #[test] @@ -309,7 +309,7 @@ fn no_escape_tree_all_recursive() { "#}; let query = Query::new(input).unwrap(); assert!(!query.is_valid()); - assert!(query.dump_errors().contains("recursive pattern")); + assert!(query.dump_diagnostics().contains("recursive pattern")); } #[test] diff --git a/crates/plotnik-lib/src/query/shape_cardinalities.rs b/crates/plotnik-lib/src/query/shape_cardinalities.rs index 197824b4..4ad477b7 100644 --- a/crates/plotnik-lib/src/query/shape_cardinalities.rs +++ b/crates/plotnik-lib/src/query/shape_cardinalities.rs @@ -11,8 +11,8 @@ use super::invariants::{ assert_ref_in_symbols, ensure_ref_body, ensure_ref_name, panic_unexpected_node, }; use super::named_defs::SymbolTable; +use crate::diagnostics::{DiagnosticMessage, DiagnosticStage, Diagnostics}; use crate::parser::{Branch, Def, Expr, Field, Ref, Root, Seq, SyntaxNode, Type}; -use crate::parser::{Diagnostic, ErrorStage}; use std::collections::HashMap; #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -162,8 +162,8 @@ pub fn validate( root: &Root, _symbols: &SymbolTable, cardinalities: &HashMap, -) -> Vec { - let mut errors = Vec::new(); +) -> Diagnostics { + let mut errors = Diagnostics::new(); validate_node(&root.syntax().clone(), cardinalities, &mut errors); errors } @@ -171,7 +171,7 @@ pub fn validate( fn validate_node( node: &SyntaxNode, cardinalities: &HashMap, - errors: &mut Vec, + errors: &mut Diagnostics, ) { if let Some(field) = Field::cast(node.clone()) && let Some(value) = field.value() @@ -188,14 +188,14 @@ fn validate_node( .unwrap_or_else(|| "field".to_string()); errors.push( - Diagnostic::error( + DiagnosticMessage::error( value.syntax().text_range(), format!( "field `{}` value must match a single node, not a sequence", field_name ), ) - .with_stage(ErrorStage::Validate), + .with_stage(DiagnosticStage::Validate), ); } } diff --git a/crates/plotnik-lib/src/query/shape_cardinalities_tests.rs b/crates/plotnik-lib/src/query/shape_cardinalities_tests.rs index 88c29c0e..d3c71d11 100644 --- a/crates/plotnik-lib/src/query/shape_cardinalities_tests.rs +++ b/crates/plotnik-lib/src/query/shape_cardinalities_tests.rs @@ -167,7 +167,7 @@ fn field_with_seq_error() { Tree¹ a Tree¹ b "); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: field `name` value must match a single node, not a sequence | 1 | (call name: {(a) (b)}) @@ -194,7 +194,7 @@ fn field_with_ref_to_seq_error() { Field¹ name: Ref⁺ X "); - insta::assert_snapshot!(query.dump_errors(), @r" + insta::assert_snapshot!(query.dump_diagnostics(), @r" error: field `name` value must match a single node, not a sequence | 2 | (call name: (X)) From 465a3ca36787ff1af65e6a7c596e5d6bf4a150aa Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Thu, 4 Dec 2025 00:00:43 -0300 Subject: [PATCH 2/4] Inline Diagnostics collection implementation in module and remove collection.rs --- .../plotnik-lib/src/diagnostics/collection.rs | 85 ------------------ crates/plotnik-lib/src/diagnostics/message.rs | 2 +- crates/plotnik-lib/src/diagnostics/mod.rs | 90 ++++++++++++++++++- crates/plotnik-lib/src/diagnostics/printer.rs | 38 +++----- crates/plotnik-lib/src/query/errors.rs | 62 ++----------- 5 files changed, 108 insertions(+), 169 deletions(-) delete mode 100644 crates/plotnik-lib/src/diagnostics/collection.rs diff --git a/crates/plotnik-lib/src/diagnostics/collection.rs b/crates/plotnik-lib/src/diagnostics/collection.rs deleted file mode 100644 index 03b616ea..00000000 --- a/crates/plotnik-lib/src/diagnostics/collection.rs +++ /dev/null @@ -1,85 +0,0 @@ -//! Diagnostics collection for accumulating compiler messages. - -use super::message::{DiagnosticMessage, Severity}; - -/// Collection of diagnostic messages from parsing and analysis. -#[derive(Debug, Clone, Default)] -pub struct Diagnostics(Vec); - -impl Diagnostics { - pub fn new() -> Self { - Self(Vec::new()) - } - - pub fn push(&mut self, msg: DiagnosticMessage) { - self.0.push(msg); - } - - pub fn extend(&mut self, iter: impl IntoIterator) { - self.0.extend(iter); - } - - pub fn is_empty(&self) -> bool { - self.0.is_empty() - } - - pub fn len(&self) -> usize { - self.0.len() - } - - pub fn iter(&self) -> impl Iterator { - self.0.iter() - } - - pub fn has_errors(&self) -> bool { - self.0.iter().any(|d| d.is_error()) - } - - pub fn has_warnings(&self) -> bool { - self.0.iter().any(|d| d.is_warning()) - } - - pub fn as_slice(&self) -> &[DiagnosticMessage] { - &self.0 - } - - pub fn into_vec(self) -> Vec { - self.0 - } - - pub fn error_count(&self) -> usize { - self.0.iter().filter(|d| d.is_error()).count() - } - - pub fn warning_count(&self) -> usize { - self.0.iter().filter(|d| d.is_warning()).count() - } - - pub fn filter_by_severity(&self, severity: Severity) -> Vec<&DiagnosticMessage> { - self.0.iter().filter(|d| d.severity == severity).collect() - } -} - -impl IntoIterator for Diagnostics { - type Item = DiagnosticMessage; - type IntoIter = std::vec::IntoIter; - - fn into_iter(self) -> Self::IntoIter { - self.0.into_iter() - } -} - -impl<'a> IntoIterator for &'a Diagnostics { - type Item = &'a DiagnosticMessage; - type IntoIter = std::slice::Iter<'a, DiagnosticMessage>; - - fn into_iter(self) -> Self::IntoIter { - self.0.iter() - } -} - -impl FromIterator for Diagnostics { - fn from_iter>(iter: T) -> Self { - Self(iter.into_iter().collect()) - } -} diff --git a/crates/plotnik-lib/src/diagnostics/message.rs b/crates/plotnik-lib/src/diagnostics/message.rs index 62218407..e3cd6f64 100644 --- a/crates/plotnik-lib/src/diagnostics/message.rs +++ b/crates/plotnik-lib/src/diagnostics/message.rs @@ -196,4 +196,4 @@ impl std::fmt::Display for DiagnosticMessage { } } -impl std::error::Error for DiagnosticMessage {} \ No newline at end of file +impl std::error::Error for DiagnosticMessage {} diff --git a/crates/plotnik-lib/src/diagnostics/mod.rs b/crates/plotnik-lib/src/diagnostics/mod.rs index 43043810..b7ead801 100644 --- a/crates/plotnik-lib/src/diagnostics/mod.rs +++ b/crates/plotnik-lib/src/diagnostics/mod.rs @@ -3,13 +3,99 @@ //! This module provides types for collecting, filtering, and rendering //! diagnostic messages from parsing and analysis stages. -mod collection; mod message; mod printer; #[cfg(test)] mod tests; -pub use collection::Diagnostics; pub use message::{DiagnosticMessage, DiagnosticStage, Fix, RelatedInfo, Severity}; pub use printer::DiagnosticsPrinter; + +/// Collection of diagnostic messages from parsing and analysis. +#[derive(Debug, Clone, Default)] +pub struct Diagnostics(Vec); + +impl Diagnostics { + pub fn new() -> Self { + Self(Vec::new()) + } + + pub fn push(&mut self, msg: DiagnosticMessage) { + self.0.push(msg); + } + + pub fn extend(&mut self, iter: impl IntoIterator) { + self.0.extend(iter); + } + + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + + pub fn len(&self) -> usize { + self.0.len() + } + + pub fn iter(&self) -> impl Iterator + Clone { + self.0.iter() + } + + pub fn has_errors(&self) -> bool { + self.0.iter().any(|d| d.is_error()) + } + + pub fn has_warnings(&self) -> bool { + self.0.iter().any(|d| d.is_warning()) + } + + pub fn as_slice(&self) -> &[DiagnosticMessage] { + &self.0 + } + + pub fn into_vec(self) -> Vec { + self.0 + } + + pub fn error_count(&self) -> usize { + self.0.iter().filter(|d| d.is_error()).count() + } + + pub fn warning_count(&self) -> usize { + self.0.iter().filter(|d| d.is_warning()).count() + } + + pub fn filter_by_severity(&self, severity: Severity) -> Vec<&DiagnosticMessage> { + self.0.iter().filter(|d| d.severity == severity).collect() + } + + pub fn printer( + &self, + ) -> DiagnosticsPrinter<'_, impl Iterator + Clone> { + DiagnosticsPrinter::new(self.iter()) + } +} + +impl IntoIterator for Diagnostics { + type Item = DiagnosticMessage; + type IntoIter = std::vec::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() + } +} + +impl<'a> IntoIterator for &'a Diagnostics { + type Item = &'a DiagnosticMessage; + type IntoIter = std::slice::Iter<'a, DiagnosticMessage>; + + fn into_iter(self) -> Self::IntoIter { + self.0.iter() + } +} + +impl FromIterator for Diagnostics { + fn from_iter>(iter: T) -> Self { + Self(iter.into_iter().collect()) + } +} diff --git a/crates/plotnik-lib/src/diagnostics/printer.rs b/crates/plotnik-lib/src/diagnostics/printer.rs index 1f66762d..fe7179ce 100644 --- a/crates/plotnik-lib/src/diagnostics/printer.rs +++ b/crates/plotnik-lib/src/diagnostics/printer.rs @@ -1,4 +1,3 @@ - //! Builder-pattern printer for rendering diagnostics. use std::fmt::Write; @@ -6,19 +5,20 @@ use std::fmt::Write; use annotate_snippets::{AnnotationKind, Group, Level, Patch, Renderer, Snippet}; use rowan::TextRange; -use super::collection::Diagnostics; -use super::message::Severity; +use super::message::{DiagnosticMessage, Severity}; -/// Builder for rendering diagnostics with various options. -pub struct DiagnosticsPrinter<'d, 's> { - diagnostics: &'d Diagnostics, - source: Option<&'s str>, - path: Option<&'s str>, +pub struct DiagnosticsPrinter<'a, I> { + diagnostics: I, + source: Option<&'a str>, + path: Option<&'a str>, colored: bool, } -impl<'d, 's> DiagnosticsPrinter<'d, 's> { - pub fn new(diagnostics: &'d Diagnostics) -> Self { +impl<'a, I> DiagnosticsPrinter<'a, I> +where + I: Iterator + Clone, +{ + pub fn new(diagnostics: I) -> Self { Self { diagnostics, source: None, @@ -27,12 +27,12 @@ impl<'d, 's> DiagnosticsPrinter<'d, 's> { } } - pub fn source(mut self, source: &'s str) -> Self { + pub fn source(mut self, source: &'a str) -> Self { self.source = Some(source); self } - pub fn path(mut self, path: &'s str) -> Self { + pub fn path(mut self, path: &'a str) -> Self { self.path = Some(path); self } @@ -53,17 +53,13 @@ impl<'d, 's> DiagnosticsPrinter<'d, 's> { return self.format_plain(w); }; - if self.diagnostics.is_empty() { - return Ok(()); - } - let renderer = if self.colored { Renderer::styled() } else { Renderer::plain() }; - for (i, diag) in self.diagnostics.iter().enumerate() { + for (i, diag) in self.diagnostics.clone().enumerate() { let range = adjust_range(diag.range, source.len()); let mut snippet = Snippet::source(source).line_start(1).annotation( @@ -109,7 +105,7 @@ impl<'d, 's> DiagnosticsPrinter<'d, 's> { } fn format_plain(&self, w: &mut impl Write) -> std::fmt::Result { - for (i, diag) in self.diagnostics.iter().enumerate() { + for (i, diag) in self.diagnostics.clone().enumerate() { if i > 0 { w.write_char('\n')?; } @@ -136,9 +132,3 @@ fn adjust_range(range: TextRange, limit: usize) -> std::ops::Range { start..end } - -impl Diagnostics { - pub fn printer(&self) -> DiagnosticsPrinter<'_, '_> { - DiagnosticsPrinter::new(self) - } -} diff --git a/crates/plotnik-lib/src/query/errors.rs b/crates/plotnik-lib/src/query/errors.rs index 795da2f1..b5f15ced 100644 --- a/crates/plotnik-lib/src/query/errors.rs +++ b/crates/plotnik-lib/src/query/errors.rs @@ -1,4 +1,4 @@ -use crate::diagnostics::{DiagnosticMessage, DiagnosticStage, DiagnosticsPrinter}; +use crate::diagnostics::DiagnosticMessage; use super::Query; @@ -12,62 +12,10 @@ impl Query<'_> { !self.diagnostics.iter().any(|d| d.is_error()) } - pub fn has_errors(&self) -> bool { - self.diagnostics.iter().any(|d| d.is_error()) - } - - pub fn has_warnings(&self) -> bool { - self.diagnostics.iter().any(|d| d.is_warning()) - } - - pub fn errors_only(&self) -> Vec<&DiagnosticMessage> { - self.diagnostics.iter().filter(|d| d.is_error()).collect() - } - - pub fn warnings_only(&self) -> Vec<&DiagnosticMessage> { - self.diagnostics.iter().filter(|d| d.is_warning()).collect() - } - - pub fn diagnostics_for_stage(&self, stage: DiagnosticStage) -> Vec<&DiagnosticMessage> { - self.diagnostics - .iter() - .filter(|d| d.stage == stage) - .collect() - } - - pub fn has_parse_errors(&self) -> bool { - self.diagnostics - .iter() - .any(|d| d.stage == DiagnosticStage::Parse && d.is_error()) - } - - pub fn has_validate_errors(&self) -> bool { - self.diagnostics - .iter() - .any(|d| d.stage == DiagnosticStage::Validate && d.is_error()) - } - - pub fn has_resolve_errors(&self) -> bool { - self.diagnostics - .iter() - .any(|d| d.stage == DiagnosticStage::Resolve && d.is_error()) - } - - pub fn has_escape_errors(&self) -> bool { - self.diagnostics - .iter() - .any(|d| d.stage == DiagnosticStage::Escape && d.is_error()) - } - - pub fn diagnostics_printer(&self) -> DiagnosticsPrinter<'_, '_> { + pub fn diagnostics_printer( + &self, + ) -> crate::diagnostics::DiagnosticsPrinter<'_, impl Iterator + Clone> + { self.diagnostics.printer().source(self.source) } - - pub fn render_diagnostics_colored(&self, colored: bool) -> String { - self.diagnostics_printer().colored(colored).render() - } - - pub fn error_count(&self) -> usize { - self.diagnostics.iter().filter(|d| d.is_error()).count() - } } From 22ae2baf60896fc23f0ff22c491ab45f317dffea Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Thu, 4 Dec 2025 01:25:43 -0300 Subject: [PATCH 3/4] refactor: Diagnostics handling --- crates/plotnik-cli/src/commands/debug/mod.rs | 2 +- crates/plotnik-lib/src/diagnostics/message.rs | 119 ++------ crates/plotnik-lib/src/diagnostics/mod.rs | 98 +++---- crates/plotnik-lib/src/diagnostics/printer.rs | 44 +-- crates/plotnik-lib/src/diagnostics/tests.rs | 274 +++++++++--------- crates/plotnik-lib/src/lib.rs | 12 +- crates/plotnik-lib/src/parser/core.rs | 40 ++- crates/plotnik-lib/src/parser/grammar.rs | 118 ++++---- crates/plotnik-lib/src/parser/mod.rs | 38 +-- .../parser/tests/json_serialization_tests.rs | 49 ---- crates/plotnik-lib/src/parser/tests/mod.rs | 1 - crates/plotnik-lib/src/query/alt_kind.rs | 22 +- crates/plotnik-lib/src/query/dump.rs | 6 +- crates/plotnik-lib/src/query/errors.rs | 21 -- crates/plotnik-lib/src/query/invariants.rs | 42 +-- crates/plotnik-lib/src/query/mod.rs | 103 +++++-- crates/plotnik-lib/src/query/named_defs.rs | 32 +- crates/plotnik-lib/src/query/printer.rs | 1 + crates/plotnik-lib/src/query/ref_cycles.rs | 51 ++-- .../src/query/shape_cardinalities.rs | 53 ++-- .../src/query/shape_cardinalities_tests.rs | 44 +++ 21 files changed, 521 insertions(+), 649 deletions(-) delete mode 100644 crates/plotnik-lib/src/parser/tests/json_serialization_tests.rs delete mode 100644 crates/plotnik-lib/src/query/errors.rs diff --git a/crates/plotnik-cli/src/commands/debug/mod.rs b/crates/plotnik-cli/src/commands/debug/mod.rs index 7cb47a41..6aad3358 100644 --- a/crates/plotnik-cli/src/commands/debug/mod.rs +++ b/crates/plotnik-cli/src/commands/debug/mod.rs @@ -96,7 +96,7 @@ pub fn run(args: DebugArgs) { if let Some(ref q) = query && !q.is_valid() { - eprint!("{}", q.diagnostics_printer().colored(args.color).render()); + eprint!("{}", q.render_diagnostics_colored(args.color)); } } diff --git a/crates/plotnik-lib/src/diagnostics/message.rs b/crates/plotnik-lib/src/diagnostics/message.rs index e3cd6f64..2b95bd88 100644 --- a/crates/plotnik-lib/src/diagnostics/message.rs +++ b/crates/plotnik-lib/src/diagnostics/message.rs @@ -1,11 +1,9 @@ //! Diagnostic message types and related structures. -use rowan::{TextRange, TextSize}; -use serde::{Serialize, Serializer}; +use rowan::TextRange; /// Severity level of a diagnostic. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Default)] -#[serde(rename_all = "lowercase")] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] pub enum Severity { #[default] Error, @@ -21,37 +19,11 @@ impl std::fmt::Display for Severity { } } -/// The stage at which a diagnostic occurred. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Default)] -#[serde(rename_all = "lowercase")] -pub enum DiagnosticStage { - /// Lexing/parsing diagnostics (syntax structure) - #[default] - Parse, - /// Semantic validation diagnostics (mixed alternations, etc.) - Validate, - /// Name resolution diagnostics (undefined/duplicate references) - Resolve, - /// Escape analysis diagnostics (infinite recursion) - Escape, -} - -impl std::fmt::Display for DiagnosticStage { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - DiagnosticStage::Parse => write!(f, "parse"), - DiagnosticStage::Validate => write!(f, "validate"), - DiagnosticStage::Resolve => write!(f, "resolve"), - DiagnosticStage::Escape => write!(f, "escape"), - } - } -} - /// A suggested fix for a diagnostic. -#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct Fix { - pub replacement: String, - pub description: String, + pub(crate) replacement: String, + pub(crate) description: String, } impl Fix { @@ -64,11 +36,10 @@ impl Fix { } /// Related location information for a diagnostic. -#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct RelatedInfo { - #[serde(serialize_with = "serialize_text_range")] - pub range: TextRange, - pub message: String, + pub(crate) range: TextRange, + pub(crate) message: String, } impl RelatedInfo { @@ -80,34 +51,20 @@ impl RelatedInfo { } } -fn serialize_text_range(range: &TextRange, s: S) -> Result { - use serde::ser::SerializeStruct; - let mut state = s.serialize_struct("TextRange", 2)?; - state.serialize_field("start", &u32::from(range.start()))?; - state.serialize_field("end", &u32::from(range.end()))?; - state.end() -} - /// A diagnostic message with location, message, severity, and optional fix. -#[derive(Debug, Clone, PartialEq, Eq, Serialize)] -pub struct DiagnosticMessage { - pub severity: Severity, - pub stage: DiagnosticStage, - #[serde(serialize_with = "serialize_text_range")] - pub range: TextRange, - pub message: String, - #[serde(skip_serializing_if = "Option::is_none")] - pub fix: Option, - #[serde(skip_serializing_if = "Vec::is_empty")] - pub related: Vec, +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct DiagnosticMessage { + pub(crate) severity: Severity, + pub(crate) range: TextRange, + pub(crate) message: String, + pub(crate) fix: Option, + pub(crate) related: Vec, } impl DiagnosticMessage { - /// Create an error diagnostic. - pub fn error(range: TextRange, message: impl Into) -> Self { + pub(crate) fn error(range: TextRange, message: impl Into) -> Self { Self { severity: Severity::Error, - stage: DiagnosticStage::default(), range, message: message.into(), fix: None, @@ -115,11 +72,9 @@ impl DiagnosticMessage { } } - /// Create a warning diagnostic. - pub fn warning(range: TextRange, message: impl Into) -> Self { + pub(crate) fn warning(range: TextRange, message: impl Into) -> Self { Self { severity: Severity::Warning, - stage: DiagnosticStage::default(), range, message: message.into(), fix: None, @@ -127,45 +82,11 @@ impl DiagnosticMessage { } } - /// Create an error at a zero-width position. - pub fn error_at(offset: TextSize, message: impl Into) -> Self { - Self::error(TextRange::empty(offset), message) - } - - /// Create a warning at a zero-width position. - pub fn warning_at(offset: TextSize, message: impl Into) -> Self { - Self::warning(TextRange::empty(offset), message) - } - - /// Set the pipeline stage. - pub fn with_stage(mut self, stage: DiagnosticStage) -> Self { - self.stage = stage; - self - } - - /// Add a fix suggestion. - pub fn with_fix(mut self, fix: Fix) -> Self { - self.fix = Some(fix); - self - } - - /// Add a related location. - pub fn with_related(mut self, related: RelatedInfo) -> Self { - self.related.push(related); - self - } - - /// Add multiple related locations. - pub fn with_related_many(mut self, related: impl IntoIterator) -> Self { - self.related.extend(related); - self - } - - pub fn is_error(&self) -> bool { + pub(crate) fn is_error(&self) -> bool { self.severity == Severity::Error } - pub fn is_warning(&self) -> bool { + pub(crate) fn is_warning(&self) -> bool { self.severity == Severity::Warning } } @@ -195,5 +116,3 @@ impl std::fmt::Display for DiagnosticMessage { Ok(()) } } - -impl std::error::Error for DiagnosticMessage {} diff --git a/crates/plotnik-lib/src/diagnostics/mod.rs b/crates/plotnik-lib/src/diagnostics/mod.rs index b7ead801..496d051d 100644 --- a/crates/plotnik-lib/src/diagnostics/mod.rs +++ b/crates/plotnik-lib/src/diagnostics/mod.rs @@ -1,7 +1,6 @@ //! Compiler diagnostics infrastructure. //! -//! This module provides types for collecting, filtering, and rendering -//! diagnostic messages from parsing and analysis stages. +//! This module provides types for collecting and rendering diagnostic messages. mod message; mod printer; @@ -9,93 +8,92 @@ mod printer; #[cfg(test)] mod tests; -pub use message::{DiagnosticMessage, DiagnosticStage, Fix, RelatedInfo, Severity}; +use rowan::TextRange; + +pub use message::Severity; pub use printer::DiagnosticsPrinter; +use message::{DiagnosticMessage, Fix, RelatedInfo}; + /// Collection of diagnostic messages from parsing and analysis. #[derive(Debug, Clone, Default)] -pub struct Diagnostics(Vec); +pub struct Diagnostics { + messages: Vec, +} + +/// Builder for constructing a diagnostic message. +#[must_use = "diagnostic not emitted, call .emit()"] +pub struct DiagnosticBuilder<'a> { + diagnostics: &'a mut Diagnostics, + message: DiagnosticMessage, +} impl Diagnostics { pub fn new() -> Self { - Self(Vec::new()) + Self { + messages: Vec::new(), + } } - pub fn push(&mut self, msg: DiagnosticMessage) { - self.0.push(msg); + pub fn error(&mut self, msg: impl Into, range: TextRange) -> DiagnosticBuilder<'_> { + DiagnosticBuilder { + diagnostics: self, + message: DiagnosticMessage::error(range, msg), + } } - pub fn extend(&mut self, iter: impl IntoIterator) { - self.0.extend(iter); + pub fn warning(&mut self, msg: impl Into, range: TextRange) -> DiagnosticBuilder<'_> { + DiagnosticBuilder { + diagnostics: self, + message: DiagnosticMessage::warning(range, msg), + } } pub fn is_empty(&self) -> bool { - self.0.is_empty() + self.messages.is_empty() } pub fn len(&self) -> usize { - self.0.len() - } - - pub fn iter(&self) -> impl Iterator + Clone { - self.0.iter() + self.messages.len() } pub fn has_errors(&self) -> bool { - self.0.iter().any(|d| d.is_error()) + self.messages.iter().any(|d| d.is_error()) } pub fn has_warnings(&self) -> bool { - self.0.iter().any(|d| d.is_warning()) - } - - pub fn as_slice(&self) -> &[DiagnosticMessage] { - &self.0 - } - - pub fn into_vec(self) -> Vec { - self.0 + self.messages.iter().any(|d| d.is_warning()) } pub fn error_count(&self) -> usize { - self.0.iter().filter(|d| d.is_error()).count() + self.messages.iter().filter(|d| d.is_error()).count() } pub fn warning_count(&self) -> usize { - self.0.iter().filter(|d| d.is_warning()).count() + self.messages.iter().filter(|d| d.is_warning()).count() } - pub fn filter_by_severity(&self, severity: Severity) -> Vec<&DiagnosticMessage> { - self.0.iter().filter(|d| d.severity == severity).collect() + pub fn printer<'a>(&'a self, source: &'a str) -> DiagnosticsPrinter<'a> { + DiagnosticsPrinter::new(&self.messages, source) } - pub fn printer( - &self, - ) -> DiagnosticsPrinter<'_, impl Iterator + Clone> { - DiagnosticsPrinter::new(self.iter()) + pub fn extend(&mut self, other: Diagnostics) { + self.messages.extend(other.messages); } } -impl IntoIterator for Diagnostics { - type Item = DiagnosticMessage; - type IntoIter = std::vec::IntoIter; - - fn into_iter(self) -> Self::IntoIter { - self.0.into_iter() +impl<'a> DiagnosticBuilder<'a> { + pub fn related_to(mut self, msg: impl Into, range: TextRange) -> Self { + self.message.related.push(RelatedInfo::new(range, msg)); + self } -} - -impl<'a> IntoIterator for &'a Diagnostics { - type Item = &'a DiagnosticMessage; - type IntoIter = std::slice::Iter<'a, DiagnosticMessage>; - fn into_iter(self) -> Self::IntoIter { - self.0.iter() + pub fn fix(mut self, description: impl Into, replacement: impl Into) -> Self { + self.message.fix = Some(Fix::new(replacement, description)); + self } -} -impl FromIterator for Diagnostics { - fn from_iter>(iter: T) -> Self { - Self(iter.into_iter().collect()) + pub fn emit(self) { + self.diagnostics.messages.push(self.message); } } diff --git a/crates/plotnik-lib/src/diagnostics/printer.rs b/crates/plotnik-lib/src/diagnostics/printer.rs index fe7179ce..fe05e936 100644 --- a/crates/plotnik-lib/src/diagnostics/printer.rs +++ b/crates/plotnik-lib/src/diagnostics/printer.rs @@ -7,31 +7,23 @@ use rowan::TextRange; use super::message::{DiagnosticMessage, Severity}; -pub struct DiagnosticsPrinter<'a, I> { - diagnostics: I, - source: Option<&'a str>, +pub struct DiagnosticsPrinter<'a> { + diagnostics: &'a [DiagnosticMessage], + source: &'a str, path: Option<&'a str>, colored: bool, } -impl<'a, I> DiagnosticsPrinter<'a, I> -where - I: Iterator + Clone, -{ - pub fn new(diagnostics: I) -> Self { +impl<'a> DiagnosticsPrinter<'a> { + pub(crate) fn new(diagnostics: &'a [DiagnosticMessage], source: &'a str) -> Self { Self { diagnostics, - source: None, + source, path: None, colored: false, } } - pub fn source(mut self, source: &'a str) -> Self { - self.source = Some(source); - self - } - pub fn path(mut self, path: &'a str) -> Self { self.path = Some(path); self @@ -49,20 +41,16 @@ where } pub fn format(&self, w: &mut impl Write) -> std::fmt::Result { - let Some(source) = self.source else { - return self.format_plain(w); - }; - let renderer = if self.colored { Renderer::styled() } else { Renderer::plain() }; - for (i, diag) in self.diagnostics.clone().enumerate() { - let range = adjust_range(diag.range, source.len()); + for (i, diag) in self.diagnostics.iter().enumerate() { + let range = adjust_range(diag.range, self.source.len()); - let mut snippet = Snippet::source(source).line_start(1).annotation( + let mut snippet = Snippet::source(self.source).line_start(1).annotation( AnnotationKind::Primary .span(range.clone()) .label(&diag.message), @@ -75,7 +63,7 @@ where for related in &diag.related { snippet = snippet.annotation( AnnotationKind::Context - .span(adjust_range(related.range, source.len())) + .span(adjust_range(related.range, self.source.len())) .label(&related.message), ); } @@ -88,7 +76,7 @@ where if let Some(fix) = &diag.fix { report.push( Level::HELP.secondary_title(&fix.description).element( - Snippet::source(source) + Snippet::source(self.source) .line_start(1) .patch(Patch::new(range, &fix.replacement)), ), @@ -103,16 +91,6 @@ where Ok(()) } - - fn format_plain(&self, w: &mut impl Write) -> std::fmt::Result { - for (i, diag) in self.diagnostics.clone().enumerate() { - if i > 0 { - w.write_char('\n')?; - } - write!(w, "{}", diag)?; - } - Ok(()) - } } fn severity_to_level(severity: Severity) -> Level<'static> { diff --git a/crates/plotnik-lib/src/diagnostics/tests.rs b/crates/plotnik-lib/src/diagnostics/tests.rs index a426262f..37e68726 100644 --- a/crates/plotnik-lib/src/diagnostics/tests.rs +++ b/crates/plotnik-lib/src/diagnostics/tests.rs @@ -1,6 +1,7 @@ -use super::*; use rowan::TextRange; +use super::*; + #[test] fn severity_display() { insta::assert_snapshot!(format!("{}", Severity::Error), @"error"); @@ -8,71 +9,108 @@ fn severity_display() { } #[test] -fn error_stage_display() { - insta::assert_snapshot!(format!("{}", DiagnosticStage::Parse), @"parse"); - insta::assert_snapshot!(format!("{}", DiagnosticStage::Validate), @"validate"); - insta::assert_snapshot!(format!("{}", DiagnosticStage::Resolve), @"resolve"); - insta::assert_snapshot!(format!("{}", DiagnosticStage::Escape), @"escape"); -} +fn error_builder() { + let mut diagnostics = Diagnostics::new(); + diagnostics + .error("test error", TextRange::new(0.into(), 5.into())) + .emit(); -#[test] -fn diagnostic_warning_constructors() { - let warn = DiagnosticMessage::warning(TextRange::empty(0.into()), "test warning"); - assert!(warn.is_warning()); - assert!(!warn.is_error()); - - let warn_at = DiagnosticMessage::warning_at(5.into(), "warning at offset"); - assert!(warn_at.is_warning()); - assert_eq!(warn_at.range.start(), 5.into()); + assert_eq!(diagnostics.len(), 1); + assert!(diagnostics.has_errors()); + assert!(!diagnostics.has_warnings()); } #[test] -fn diagnostic_error_at_constructor() { - let err = DiagnosticMessage::error_at(7.into(), "error at offset"); - assert!(err.is_error()); - assert!(!err.is_warning()); - assert_eq!(err.range.start(), 7.into()); - assert_eq!(err.range.end(), 7.into()); +fn warning_builder() { + let mut diagnostics = Diagnostics::new(); + diagnostics + .warning("test warning", TextRange::new(0.into(), 5.into())) + .emit(); + + assert_eq!(diagnostics.len(), 1); + assert!(!diagnostics.has_errors()); + assert!(diagnostics.has_warnings()); } #[test] -fn diagnostic_builders() { - let diag = DiagnosticMessage::error(TextRange::empty(0.into()), "test") - .with_stage(DiagnosticStage::Resolve) - .with_fix(Fix::new("replacement", "description")) - .with_related(RelatedInfo::new(TextRange::empty(10.into()), "related")); - - assert_eq!(diag.stage, DiagnosticStage::Resolve); - assert!(diag.fix.is_some()); - assert_eq!(diag.related.len(), 1); - - let diag2 = - DiagnosticMessage::error(TextRange::empty(0.into()), "test").with_related_many(vec![ - RelatedInfo::new(TextRange::empty(1.into()), "first"), - RelatedInfo::new(TextRange::empty(2.into()), "second"), - ]); - assert_eq!(diag2.related.len(), 2); +fn builder_with_related() { + let mut diagnostics = Diagnostics::new(); + diagnostics + .error("primary", TextRange::new(0.into(), 5.into())) + .related_to("related info", TextRange::new(6.into(), 10.into())) + .emit(); + + assert_eq!(diagnostics.len(), 1); + let result = diagnostics.printer("hello world!").render(); + insta::assert_snapshot!(result, @r" + error: primary + | + 1 | hello world! + | ^^^^^ ---- related info + | | + | primary + "); } #[test] -fn diagnostic_display() { - let diag = DiagnosticMessage::error(TextRange::new(5.into(), 10.into()), "test message"); - insta::assert_snapshot!(format!("{}", diag), @"error at 5..10: test message"); +fn builder_with_fix() { + let mut diagnostics = Diagnostics::new(); + diagnostics + .error("fixable", TextRange::new(0.into(), 5.into())) + .fix("apply this fix", "fixed") + .emit(); - let diag_with_fix = DiagnosticMessage::error(TextRange::empty(0.into()), "msg") - .with_fix(Fix::new("fix", "fix description")); - insta::assert_snapshot!(format!("{}", diag_with_fix), @"error at 0..0: msg (fix: fix description)"); + let result = diagnostics.printer("hello world").render(); + insta::assert_snapshot!(result, @r" + error: fixable + | + 1 | hello world + | ^^^^^ fixable + | + help: apply this fix + | + 1 - hello world + 1 + fixed world + | + "); +} - let diag_with_related = DiagnosticMessage::error(TextRange::empty(0.into()), "msg") - .with_related(RelatedInfo::new(TextRange::new(1.into(), 2.into()), "rel")); - insta::assert_snapshot!(format!("{}", diag_with_related), @"error at 0..0: msg (related: rel at 1..2)"); +#[test] +fn builder_with_all_options() { + let mut diagnostics = Diagnostics::new(); + diagnostics + .error("main error", TextRange::new(0.into(), 5.into())) + .related_to("see also", TextRange::new(6.into(), 11.into())) + .related_to("and here", TextRange::new(12.into(), 17.into())) + .fix("try this", "HELLO") + .emit(); + + let result = diagnostics.printer("hello world stuff!").render(); + insta::assert_snapshot!(result, @r" + error: main error + | + 1 | hello world stuff! + | ^^^^^ ----- ----- and here + | | | + | | see also + | main error + | + help: try this + | + 1 - hello world stuff! + 1 + HELLO world stuff! + | + "); } #[test] fn printer_colored() { - let diag = DiagnosticMessage::error(TextRange::new(0.into(), 5.into()), "test"); - let diagnostics = Diagnostics::from_iter([diag]); - let result = diagnostics.printer().source("hello").colored(true).render(); + let mut diagnostics = Diagnostics::new(); + diagnostics + .error("test", TextRange::new(0.into(), 5.into())) + .emit(); + + let result = diagnostics.printer("hello").colored(true).render(); assert!(result.contains("test")); assert!(result.contains('\x1b')); } @@ -80,19 +118,18 @@ fn printer_colored() { #[test] fn printer_empty_diagnostics() { let diagnostics = Diagnostics::new(); - let result = diagnostics.printer().source("source").render(); + let result = diagnostics.printer("source").render(); assert!(result.is_empty()); } #[test] fn printer_with_path() { - let diag = DiagnosticMessage::error(TextRange::new(0.into(), 5.into()), "test error"); - let diagnostics = Diagnostics::from_iter([diag]); - let result = diagnostics - .printer() - .source("hello world") - .path("test.pql") - .render(); + let mut diagnostics = Diagnostics::new(); + diagnostics + .error("test error", TextRange::new(0.into(), 5.into())) + .emit(); + + let result = diagnostics.printer("hello world").path("test.pql").render(); insta::assert_snapshot!(result, @r" error: test error --> test.pql:1:1 @@ -104,9 +141,12 @@ fn printer_with_path() { #[test] fn printer_zero_width_span() { - let diag = DiagnosticMessage::error(TextRange::empty(0.into()), "zero width error"); - let diagnostics = Diagnostics::from_iter([diag]); - let result = diagnostics.printer().source("hello").render(); + let mut diagnostics = Diagnostics::new(); + diagnostics + .error("zero width error", TextRange::empty(0.into())) + .emit(); + + let result = diagnostics.printer("hello").render(); insta::assert_snapshot!(result, @r" error: zero width error | @@ -115,32 +155,15 @@ fn printer_zero_width_span() { "); } -#[test] -fn printer_with_related() { - let diag = - DiagnosticMessage::error(TextRange::new(0.into(), 5.into()), "primary").with_related( - RelatedInfo::new(TextRange::new(6.into(), 10.into()), "related info"), - ); - let diagnostics = Diagnostics::from_iter([diag]); - let result = diagnostics.printer().source("hello world!").render(); - insta::assert_snapshot!(result, @r" - error: primary - | - 1 | hello world! - | ^^^^^ ---- related info - | | - | primary - "); -} - #[test] fn printer_related_zero_width() { - let diag = - DiagnosticMessage::error(TextRange::new(0.into(), 5.into()), "primary").with_related( - RelatedInfo::new(TextRange::empty(6.into()), "zero width related"), - ); - let diagnostics = Diagnostics::from_iter([diag]); - let result = diagnostics.printer().source("hello world!").render(); + let mut diagnostics = Diagnostics::new(); + diagnostics + .error("primary", TextRange::new(0.into(), 5.into())) + .related_to("zero width related", TextRange::empty(6.into())) + .emit(); + + let result = diagnostics.printer("hello world!").render(); insta::assert_snapshot!(result, @r" error: primary | @@ -151,32 +174,17 @@ fn printer_related_zero_width() { "); } -#[test] -fn printer_with_fix() { - let diag = DiagnosticMessage::error(TextRange::new(0.into(), 5.into()), "fixable") - .with_fix(Fix::new("fixed", "apply this fix")); - let diagnostics = Diagnostics::from_iter([diag]); - let result = diagnostics.printer().source("hello world").render(); - insta::assert_snapshot!(result, @r" - error: fixable - | - 1 | hello world - | ^^^^^ fixable - | - help: apply this fix - | - 1 - hello world - 1 + fixed world - | - "); -} - #[test] fn printer_multiple_diagnostics() { - let diag1 = DiagnosticMessage::error(TextRange::new(0.into(), 5.into()), "first error"); - let diag2 = DiagnosticMessage::error(TextRange::new(6.into(), 10.into()), "second error"); - let diagnostics = Diagnostics::from_iter([diag1, diag2]); - let result = diagnostics.printer().source("hello world!").render(); + let mut diagnostics = Diagnostics::new(); + diagnostics + .error("first error", TextRange::new(0.into(), 5.into())) + .emit(); + diagnostics + .error("second error", TextRange::new(6.into(), 10.into())) + .emit(); + + let result = diagnostics.printer("hello world!").render(); insta::assert_snapshot!(result, @r" error: first error | @@ -191,9 +199,12 @@ fn printer_multiple_diagnostics() { #[test] fn printer_warning() { - let diag = DiagnosticMessage::warning(TextRange::new(0.into(), 5.into()), "a warning"); - let diagnostics = Diagnostics::from_iter([diag]); - let result = diagnostics.printer().source("hello").render(); + let mut diagnostics = Diagnostics::new(); + diagnostics + .warning("a warning", TextRange::new(0.into(), 5.into())) + .emit(); + + let result = diagnostics.printer("hello").render(); insta::assert_snapshot!(result, @r" warning: a warning | @@ -202,21 +213,15 @@ fn printer_warning() { "); } -#[test] -fn printer_without_source_uses_plain_format() { - let diag = DiagnosticMessage::error(TextRange::new(0.into(), 3.into()), "test"); - let diagnostics = Diagnostics::from_iter([diag]); - let result = diagnostics.printer().render(); - insta::assert_snapshot!(result, @"error at 0..3: test"); -} - #[test] fn diagnostics_collection_methods() { - let diag1 = DiagnosticMessage::error(TextRange::empty(0.into()), "error"); - let diag2 = DiagnosticMessage::warning(TextRange::empty(1.into()), "warning"); let mut diagnostics = Diagnostics::new(); - diagnostics.push(diag1); - diagnostics.push(diag2); + diagnostics + .error("error", TextRange::empty(0.into())) + .emit(); + diagnostics + .warning("warning", TextRange::empty(1.into())) + .emit(); assert!(!diagnostics.is_empty()); assert_eq!(diagnostics.len(), 2); @@ -224,25 +229,4 @@ fn diagnostics_collection_methods() { assert!(diagnostics.has_warnings()); assert_eq!(diagnostics.error_count(), 1); assert_eq!(diagnostics.warning_count(), 1); - assert_eq!(diagnostics.filter_by_severity(Severity::Error).len(), 1); - assert_eq!(diagnostics.filter_by_severity(Severity::Warning).len(), 1); -} - -#[test] -fn diagnostics_iteration() { - let diag1 = DiagnosticMessage::error(TextRange::empty(0.into()), "first"); - let diag2 = DiagnosticMessage::error(TextRange::empty(1.into()), "second"); - let diagnostics = Diagnostics::from_iter([diag1, diag2]); - - let messages: Vec<_> = diagnostics.iter().map(|d| d.message.as_str()).collect(); - assert_eq!(messages, vec!["first", "second"]); - - let messages: Vec<_> = (&diagnostics) - .into_iter() - .map(|d| d.message.as_str()) - .collect(); - assert_eq!(messages, vec!["first", "second"]); - - let vec = diagnostics.into_vec(); - assert_eq!(vec.len(), 2); } diff --git a/crates/plotnik-lib/src/lib.rs b/crates/plotnik-lib/src/lib.rs index b4aa7b36..0750c095 100644 --- a/crates/plotnik-lib/src/lib.rs +++ b/crates/plotnik-lib/src/lib.rs @@ -11,7 +11,7 @@ //! "#).expect("valid query"); //! //! if !query.is_valid() { -//! eprintln!("{}", query.diagnostics_printer().render()); +//! eprintln!("{}", query.render_diagnostics()); //! } //! ``` @@ -21,9 +21,13 @@ pub mod diagnostics; pub mod parser; pub mod query; -pub use diagnostics::{ - DiagnosticMessage, DiagnosticStage, Diagnostics, DiagnosticsPrinter, Fix, RelatedInfo, Severity, -}; +/// Result type for analysis passes that produce both output and diagnostics. +/// +/// Each pass returns its typed output alongside any diagnostics it collected. +/// Fatal errors (like fuel exhaustion) use the outer `Result`. +pub type PassResult = std::result::Result<(T, Diagnostics), Error>; + +pub use diagnostics::{Diagnostics, DiagnosticsPrinter, Severity}; pub use query::{Query, QueryBuilder}; /// Errors that can occur during query parsing. diff --git a/crates/plotnik-lib/src/parser/core.rs b/crates/plotnik-lib/src/parser/core.rs index 0ecf2a0f..03ab2132 100644 --- a/crates/plotnik-lib/src/parser/core.rs +++ b/crates/plotnik-lib/src/parser/core.rs @@ -12,23 +12,13 @@ use rowan::{Checkpoint, GreenNode, GreenNodeBuilder, TextRange, TextSize}; use super::cst::token_sets::ROOT_EXPR_FIRST; use super::cst::{SyntaxKind, TokenSet}; use super::lexer::{Token, token_text}; -use crate::diagnostics::{DiagnosticMessage, Diagnostics, Fix, RelatedInfo}; +use crate::diagnostics::Diagnostics; use crate::Error; const DEFAULT_EXEC_FUEL: u32 = 1_000_000; const DEFAULT_RECURSION_FUEL: u32 = 4096; -/// Parse result containing the green tree and any diagnostics. -/// -/// The tree is always complete—diagnostics are recorded separately and also -/// represented as `SyntaxKind::Error` nodes in the tree itself. -#[derive(Debug, Clone)] -pub struct Parse { - pub(super) cst: GreenNode, - pub(super) diagnostics: Diagnostics, -} - /// Tracks an open delimiter for better error messages on unclosed constructs. #[derive(Debug, Clone, Copy)] pub(super) struct OpenDelimiter { @@ -104,15 +94,12 @@ impl<'src> Parser<'src> { self } - pub fn finish(mut self) -> Result { + pub fn finish(mut self) -> Result<(GreenNode, Diagnostics), Error> { self.drain_trivia(); if let Some(err) = self.fatal_error { return Err(err); } - Ok(Parse { - cst: self.builder.finish(), - diagnostics: self.diagnostics, - }) + Ok((self.builder.finish(), self.diagnostics)) } /// Check if a fatal error has occurred. @@ -293,8 +280,7 @@ impl<'src> Parser<'src> { return; } self.last_diagnostic_pos = Some(pos); - self.diagnostics - .push(DiagnosticMessage::error(range, message)); + self.diagnostics.error(message, range).emit(); } /// Wrap unexpected token in Error node and consume it. @@ -397,7 +383,12 @@ impl<'src> Parser<'src> { } /// Record a diagnostic with a related location (e.g., where an unclosed delimiter started). - pub(super) fn error_with_related(&mut self, message: impl Into, related: RelatedInfo) { + pub(super) fn error_with_related( + &mut self, + message: impl Into, + related_msg: impl Into, + related_range: TextRange, + ) { let range = self.current_span(); let pos = range.start(); if self.last_diagnostic_pos == Some(pos) { @@ -405,7 +396,9 @@ impl<'src> Parser<'src> { } self.last_diagnostic_pos = Some(pos); self.diagnostics - .push(DiagnosticMessage::error(range, message).with_related(related)); + .error(message, range) + .related_to(related_msg, related_range) + .emit(); } /// Get the end position of the last non-trivia token before current position. @@ -424,7 +417,8 @@ impl<'src> Parser<'src> { &mut self, range: TextRange, message: impl Into, - fix: Fix, + fix_description: impl Into, + fix_replacement: impl Into, ) { let pos = range.start(); if self.last_diagnostic_pos == Some(pos) { @@ -432,6 +426,8 @@ impl<'src> Parser<'src> { } self.last_diagnostic_pos = Some(pos); self.diagnostics - .push(DiagnosticMessage::error(range, message).with_fix(fix)); + .error(message, range) + .fix(fix_description, fix_replacement) + .emit(); } } diff --git a/crates/plotnik-lib/src/parser/grammar.rs b/crates/plotnik-lib/src/parser/grammar.rs index f86bff59..af198028 100644 --- a/crates/plotnik-lib/src/parser/grammar.rs +++ b/crates/plotnik-lib/src/parser/grammar.rs @@ -7,7 +7,6 @@ use rowan::{Checkpoint, TextRange}; use super::core::Parser; use super::invariants::assert_nonempty; -use crate::diagnostics::{Fix, RelatedInfo}; use super::cst::token_sets::{ ALT_RECOVERY, EXPR_FIRST, QUANTIFIERS, SEPARATORS, SEQ_RECOVERY, TREE_RECOVERY, @@ -49,13 +48,14 @@ impl Parser<'_> { for span in &unnamed_def_spans[..unnamed_def_spans.len() - 1] { let def_text = &self.source[usize::from(span.start())..usize::from(span.end())]; self.diagnostics - .push(crate::diagnostics::DiagnosticMessage::error( - *span, + .error( format!( "unnamed definition must be last in file; add a name: `Name = {}`", def_text.trim() ), - )); + *span, + ) + .emit(); } } @@ -269,10 +269,11 @@ impl Parser<'_> { if let Some(name) = &ref_name { self.diagnostics - .push(crate::diagnostics::DiagnosticMessage::error( - children_span, + .error( format!("reference `{}` cannot contain children", name), - )); + children_span, + ) + .emit(); } } else if is_ref { self.start_node_at(checkpoint, SyntaxKind::Ref); @@ -311,8 +312,7 @@ impl Parser<'_> { (caller must push delimiter before calling)" ) }); - let related = RelatedInfo::new(open.span, format!("{construct} started here")); - self.error_with_related(msg, related); + self.error_with_related(msg, format!("{construct} started here"), open.span); break; } if self.has_fatal_error() { @@ -369,8 +369,7 @@ impl Parser<'_> { (caller must push delimiter before calling)" ) }); - let related = RelatedInfo::new(open.span, "alternation started here"); - self.error_with_related(msg, related); + self.error_with_related(msg, "alternation started here", open.span); break; } if self.has_fatal_error() { @@ -440,14 +439,11 @@ impl Parser<'_> { let label_text = token_text(self.source, &self.tokens[self.pos]); let capitalized = capitalize_first(label_text); - let fix = Fix::new( - capitalized.clone(), - format!("capitalize as `{}`", capitalized), - ); self.error_with_fix( span, "tagged alternation labels must be Capitalized (they map to enum variants)", - fix, + format!("capitalize as `{}`", capitalized), + capitalized, ); self.bump(); @@ -557,8 +553,12 @@ impl Parser<'_> { self.start_node(SyntaxKind::Type); let span = self.current_span(); - let fix = Fix::new("::", "use '::'"); - self.error_with_fix(span, "single colon is not valid for type annotations", fix); + self.error_with_fix( + span, + "single colon is not valid for type annotations", + "use '::'", + "::", + ); self.bump(); // colon @@ -642,8 +642,12 @@ impl Parser<'_> { self.bump(); self.peek(); let span = self.current_span(); - let fix = Fix::new(":", "use ':'"); - self.error_with_fix(span, "'=' is not valid for field constraints", fix); + self.error_with_fix( + span, + "'=' is not valid for field constraints", + "use ':'", + ":", + ); self.bump(); if EXPR_FIRST.contains(self.peek()) { @@ -668,14 +672,14 @@ impl Parser<'_> { kind ), }; - let fix = Fix::new("", "remove separator"); self.error_with_fix( span, format!( "'{}' is not valid syntax; plotnik uses whitespace for separation", char_name ), - fix, + "remove separator", + "", ); self.skip_token(); } @@ -703,35 +707,38 @@ impl Parser<'_> { if name.contains('.') { let suggested = name.replace(['.', '-'], "_"); let suggested = to_snake_case(&suggested); - let fix = Fix::new( - suggested.clone(), + self.error_with_fix( + span, + "capture names cannot contain dots", format!("captures become struct fields; use @{} instead", suggested), + suggested, ); - self.error_with_fix(span, "capture names cannot contain dots", fix); return; } if name.contains('-') { let suggested = name.replace('-', "_"); let suggested = to_snake_case(&suggested); - let fix = Fix::new( - suggested.clone(), + self.error_with_fix( + span, + "capture names cannot contain hyphens", format!("captures become struct fields; use @{} instead", suggested), + suggested, ); - self.error_with_fix(span, "capture names cannot contain hyphens", fix); return; } if name.chars().next().is_some_and(|c| c.is_ascii_uppercase()) { let suggested = to_snake_case(name); - let fix = Fix::new( - suggested.clone(), + self.error_with_fix( + span, + "capture names must start with lowercase", format!( "capture names must be snake_case; use @{} instead", suggested ), + suggested, ); - self.error_with_fix(span, "capture names must start with lowercase", fix); } } @@ -739,27 +746,29 @@ impl Parser<'_> { fn validate_def_name(&mut self, name: &str, span: TextRange) { if !name.chars().next().is_some_and(|c| c.is_ascii_uppercase()) { let suggested = to_pascal_case(name); - let fix = Fix::new( - suggested.clone(), + self.error_with_fix( + span, + "definition names must start with uppercase", format!( "definition names must be PascalCase; use {} instead", suggested ), + suggested, ); - self.error_with_fix(span, "definition names must start with uppercase", fix); return; } if name.contains('_') || name.contains('-') || name.contains('.') { let suggested = to_pascal_case(name); - let fix = Fix::new( - suggested.clone(), + self.error_with_fix( + span, + "definition names cannot contain separators", format!( "definition names must be PascalCase; use {} instead", suggested ), + suggested, ); - self.error_with_fix(span, "definition names cannot contain separators", fix); } } @@ -767,14 +776,15 @@ impl Parser<'_> { fn validate_branch_label(&mut self, name: &str, span: TextRange) { if name.contains('_') || name.contains('-') || name.contains('.') { let suggested = to_pascal_case(name); - let fix = Fix::new( - format!("{}:", suggested), + self.error_with_fix( + span, + "branch labels cannot contain separators", format!( "branch labels must be PascalCase; use {}: instead", suggested ), + format!("{}:", suggested), ); - self.error_with_fix(span, "branch labels cannot contain separators", fix); } } @@ -783,32 +793,35 @@ impl Parser<'_> { if name.contains('.') { let suggested = name.replace(['.', '-'], "_"); let suggested = to_snake_case(&suggested); - let fix = Fix::new( - format!("{}:", suggested), + self.error_with_fix( + span, + "field names cannot contain dots", format!("field names must be snake_case; use {}: instead", suggested), + format!("{}:", suggested), ); - self.error_with_fix(span, "field names cannot contain dots", fix); return; } if name.contains('-') { let suggested = name.replace('-', "_"); let suggested = to_snake_case(&suggested); - let fix = Fix::new( - format!("{}:", suggested), + self.error_with_fix( + span, + "field names cannot contain hyphens", format!("field names must be snake_case; use {}: instead", suggested), + format!("{}:", suggested), ); - self.error_with_fix(span, "field names cannot contain hyphens", fix); return; } if name.chars().next().is_some_and(|c| c.is_ascii_uppercase()) { let suggested = to_snake_case(name); - let fix = Fix::new( - format!("{}:", suggested), + self.error_with_fix( + span, + "field names must start with lowercase", format!("field names must be snake_case; use {}: instead", suggested), + format!("{}:", suggested), ); - self.error_with_fix(span, "field names must start with lowercase", fix); } } @@ -816,14 +829,15 @@ impl Parser<'_> { fn validate_type_name(&mut self, name: &str, span: TextRange) { if name.contains('.') || name.contains('-') { let suggested = to_pascal_case(name); - let fix = Fix::new( - format!("::{}", suggested), + self.error_with_fix( + span, + "type names cannot contain dots or hyphens", format!( "type names cannot contain separators; use ::{} instead", suggested ), + format!("::{}", suggested), ); - self.error_with_fix(span, "type names cannot contain dots or hyphens", fix); } } } diff --git a/crates/plotnik-lib/src/parser/mod.rs b/crates/plotnik-lib/src/parser/mod.rs index 683366bf..1d9ef9a0 100644 --- a/crates/plotnik-lib/src/parser/mod.rs +++ b/crates/plotnik-lib/src/parser/mod.rs @@ -49,54 +49,38 @@ pub use ast::{ pub use core::Parser; -use crate::{Diagnostics, Result}; +use crate::PassResult; use lexer::lex; -/// Parse result containing the green tree and any diagnostics. +/// Parse result containing the green tree. /// -/// The tree is always complete—diagnostics are recorded separately and also -/// represented as `SyntaxKind::Error` nodes in the tree itself. +/// The tree is always complete—diagnostics are returned separately. +/// Error nodes in the tree represent recovery points. #[derive(Debug, Clone)] pub struct Parse { - inner: core::Parse, + cst: rowan::GreenNode, } impl Parse { - #[allow(dead_code)] pub fn as_cst(&self) -> &rowan::GreenNode { - &self.inner.cst + &self.cst } /// Creates a typed view over the immutable green tree. /// This is cheap—SyntaxNode is a thin wrapper with parent pointers. pub fn syntax(&self) -> SyntaxNode { - SyntaxNode::new_root(self.inner.cst.clone()) - } - - pub fn diagnostics(&self) -> &Diagnostics { - &self.inner.diagnostics - } - - #[allow(dead_code)] - pub fn is_valid(&self) -> bool { - self.inner.diagnostics.is_empty() - } - - /// Render diagnostics as a human-readable string using annotate-snippets. - pub fn render_diagnostics(&self, source: &str) -> String { - self.inner.diagnostics.printer().source(source).render() + SyntaxNode::new_root(self.cst.clone()) } } /// Main entry point. Returns Err on fuel exhaustion. -pub fn parse(source: &str) -> Result { +pub fn parse(source: &str) -> PassResult { parse_with_parser(Parser::new(source, lex(source))) } /// Parse with a pre-configured parser (for custom fuel limits). -pub(crate) fn parse_with_parser(mut parser: Parser) -> Result { +pub(crate) fn parse_with_parser(mut parser: Parser) -> PassResult { parser.parse_root(); - Ok(Parse { - inner: parser.finish()?, - }) + let (cst, diagnostics) = parser.finish()?; + Ok((Parse { cst }, diagnostics)) } diff --git a/crates/plotnik-lib/src/parser/tests/json_serialization_tests.rs b/crates/plotnik-lib/src/parser/tests/json_serialization_tests.rs deleted file mode 100644 index 1a1f9a0f..00000000 --- a/crates/plotnik-lib/src/parser/tests/json_serialization_tests.rs +++ /dev/null @@ -1,49 +0,0 @@ -use crate::parser::parse; - -#[test] -fn error_json_serialization() { - let input = "(identifier) @foo.bar"; - let result = parse(input).unwrap(); - let errors = result.diagnostics(); - - assert_eq!(errors.len(), 1); - let json = serde_json::to_string_pretty(errors.iter().next().unwrap()).unwrap(); - - insta::assert_snapshot!(json, @r#" - { - "severity": "error", - "stage": "parse", - "range": { - "start": 14, - "end": 21 - }, - "message": "capture names cannot contain dots", - "fix": { - "replacement": "foo_bar", - "description": "captures become struct fields; use @foo_bar instead" - } - } - "#); -} - -#[test] -fn error_json_serialization_no_fix() { - let input = "(identifier) @"; - let result = parse(input).unwrap(); - let errors = result.diagnostics(); - - assert_eq!(errors.len(), 1); - let json = serde_json::to_string_pretty(errors.iter().next().unwrap()).unwrap(); - - insta::assert_snapshot!(json, @r#" - { - "severity": "error", - "stage": "parse", - "range": { - "start": 14, - "end": 14 - }, - "message": "expected capture name after '@'" - } - "#); -} diff --git a/crates/plotnik-lib/src/parser/tests/mod.rs b/crates/plotnik-lib/src/parser/tests/mod.rs index 96015bd3..8de6845a 100644 --- a/crates/plotnik-lib/src/parser/tests/mod.rs +++ b/crates/plotnik-lib/src/parser/tests/mod.rs @@ -1,3 +1,2 @@ mod grammar; -mod json_serialization_tests; mod recovery; diff --git a/crates/plotnik-lib/src/query/alt_kind.rs b/crates/plotnik-lib/src/query/alt_kind.rs index 415ad22e..2e6100a8 100644 --- a/crates/plotnik-lib/src/query/alt_kind.rs +++ b/crates/plotnik-lib/src/query/alt_kind.rs @@ -8,10 +8,11 @@ use rowan::TextRange; use super::invariants::{ assert_alt_no_bare_exprs, assert_root_no_bare_exprs, ensure_both_branch_kinds, }; -use crate::diagnostics::{DiagnosticMessage, DiagnosticStage, Diagnostics, RelatedInfo}; +use crate::PassResult; +use crate::diagnostics::Diagnostics; use crate::parser::{Alt, AltKind, Branch, Expr, Root}; -pub fn validate(root: &Root) -> Diagnostics { +pub fn validate(root: &Root) -> PassResult<()> { let mut errors = Diagnostics::new(); for def in root.defs() { @@ -22,7 +23,7 @@ pub fn validate(root: &Root) -> Diagnostics { assert_root_no_bare_exprs(root); - errors + Ok(((), errors)) } fn validate_expr(expr: &Expr, errors: &mut Diagnostics) { @@ -102,14 +103,13 @@ fn check_mixed_alternation(alt: &Alt, errors: &mut Diagnostics) { let untagged_range = branch_range(untagged_branch); - let error = DiagnosticMessage::error( - untagged_range, - "mixed tagged and untagged branches in alternation", - ) - .with_related(RelatedInfo::new(tagged_range, "tagged branch here")) - .with_stage(DiagnosticStage::Validate); - - errors.push(error); + errors + .error( + "mixed tagged and untagged branches in alternation", + untagged_range, + ) + .related_to("tagged branch here", tagged_range) + .emit(); } fn branch_range(branch: &Branch) -> TextRange { diff --git a/crates/plotnik-lib/src/query/dump.rs b/crates/plotnik-lib/src/query/dump.rs index c8f29a54..5e03811b 100644 --- a/crates/plotnik-lib/src/query/dump.rs +++ b/crates/plotnik-lib/src/query/dump.rs @@ -19,12 +19,16 @@ mod test_helpers { self.printer().with_cardinalities(true).dump() } + pub fn dump_cst_with_cardinalities(&self) -> String { + self.printer().raw(true).with_cardinalities(true).dump() + } + pub fn dump_symbols(&self) -> String { self.printer().only_symbols(true).dump() } pub fn dump_diagnostics(&self) -> String { - self.diagnostics_printer().render() + self.render_diagnostics() } } } diff --git a/crates/plotnik-lib/src/query/errors.rs b/crates/plotnik-lib/src/query/errors.rs deleted file mode 100644 index b5f15ced..00000000 --- a/crates/plotnik-lib/src/query/errors.rs +++ /dev/null @@ -1,21 +0,0 @@ -use crate::diagnostics::DiagnosticMessage; - -use super::Query; - -impl Query<'_> { - pub fn diagnostics(&self) -> &[DiagnosticMessage] { - self.diagnostics.as_slice() - } - - /// Query is valid if there are no error-severity diagnostics (warnings are allowed). - pub fn is_valid(&self) -> bool { - !self.diagnostics.iter().any(|d| d.is_error()) - } - - pub fn diagnostics_printer( - &self, - ) -> crate::diagnostics::DiagnosticsPrinter<'_, impl Iterator + Clone> - { - self.diagnostics.printer().source(self.source) - } -} diff --git a/crates/plotnik-lib/src/query/invariants.rs b/crates/plotnik-lib/src/query/invariants.rs index c29867f6..19a9c6d8 100644 --- a/crates/plotnik-lib/src/query/invariants.rs +++ b/crates/plotnik-lib/src/query/invariants.rs @@ -2,7 +2,7 @@ #![cfg_attr(coverage_nightly, coverage(off))] -use crate::parser::{Alt, Branch, Ref, Root, SyntaxNode, SyntaxToken}; +use crate::parser::{Alt, Branch, Root}; #[inline] pub fn assert_root_no_bare_exprs(root: &Root) { @@ -33,43 +33,3 @@ pub fn ensure_both_branch_kinds<'a>( ), } } - -#[inline] -pub fn panic_unexpected_node(node: &SyntaxNode) -> ! { - panic!( - "shape_cardinality: unexpected non-Expr node kind {:?} at {:?}", - node.kind(), - node.text_range() - ); -} - -#[inline] -pub fn ensure_ref_name(r: &Ref) -> SyntaxToken { - r.name().unwrap_or_else(|| { - panic!( - "shape_cardinality: Ref node missing name token at {:?} (should be caught by parser)", - r.syntax().text_range() - ) - }) -} - -#[inline] -pub fn assert_ref_in_symbols(name: &str, r: &Ref, found: bool) { - assert!( - found, - "shape_cardinality: Ref `{}` not in symbol table at {:?} (should be caught by resolution)", - name, - r.syntax().text_range() - ); -} - -#[inline] -pub fn ensure_ref_body<'a>(name: &str, r: &Ref, body: Option<&'a SyntaxNode>) -> &'a SyntaxNode { - body.unwrap_or_else(|| { - panic!( - "shape_cardinality: Ref `{}` in symbol table but no body found at {:?}", - name, - r.syntax().text_range() - ) - }) -} diff --git a/crates/plotnik-lib/src/query/mod.rs b/crates/plotnik-lib/src/query/mod.rs index 371f8a02..6e37fc96 100644 --- a/crates/plotnik-lib/src/query/mod.rs +++ b/crates/plotnik-lib/src/query/mod.rs @@ -1,7 +1,6 @@ //! Query processing: parsing, analysis, and validation pipeline. mod dump; -mod errors; mod invariants; mod printer; pub use printer::QueryPrinter; @@ -83,8 +82,8 @@ impl<'a> QueryBuilder<'a> { parser = parser.with_recursion_fuel(limit); } - let parse = parser::parse_with_parser(parser)?; - Ok(Query::from_parse(self.source, parse)) + let (parse, parse_diagnostics) = parser::parse_with_parser(parser)?; + Ok(Query::from_parse(self.source, parse, parse_diagnostics)) } } @@ -98,8 +97,13 @@ pub struct Query<'a> { source: &'a str, parse: Parse, symbols: SymbolTable, - diagnostics: Diagnostics, shape_cardinalities: HashMap, + // Diagnostics per pass + parse_diagnostics: Diagnostics, + alt_kind_diagnostics: Diagnostics, + resolve_diagnostics: Diagnostics, + ref_cycle_diagnostics: Diagnostics, + shape_diagnostics: Diagnostics, } impl<'a> Query<'a> { @@ -116,36 +120,32 @@ impl<'a> Query<'a> { QueryBuilder::new(source) } - /// Internal: create Query from already-parsed input. - fn from_parse(source: &'a str, parse: Parse) -> Self { + fn from_parse(source: &'a str, parse: Parse, parse_diagnostics: Diagnostics) -> Self { let root = Root::cast(parse.syntax()).expect("parser always produces Root"); - let mut diagnostics = parse.diagnostics().clone(); + let ((), alt_kind_diagnostics) = + alt_kind::validate(&root).expect("alt_kind::validate is infallible"); - let alt_kind_diags = alt_kind::validate(&root); - diagnostics.extend(alt_kind_diags); + let (symbols, resolve_diagnostics) = + named_defs::resolve(&root).expect("named_defs::resolve is infallible"); - let resolve_result = named_defs::resolve(&root); - diagnostics.extend(resolve_result.diagnostics); + let ((), ref_cycle_diagnostics) = + ref_cycles::validate(&root, &symbols).expect("ref_cycles::validate is infallible"); - let ref_cycle_diags = ref_cycles::validate(&root, &resolve_result.symbols); - diagnostics.extend(ref_cycle_diags); - - let shape_cardinalities = if diagnostics.is_empty() { - let cards = shape_cardinalities::infer(&root, &resolve_result.symbols); - let shape_diags = shape_cardinalities::validate(&root, &resolve_result.symbols, &cards); - diagnostics.extend(shape_diags); - cards - } else { - HashMap::new() - }; + let (shape_cardinalities, shape_diagnostics) = + shape_cardinalities::analyze(&root, &symbols) + .expect("shape_cardinalities::analyze is infallible"); Self { source, parse, - symbols: resolve_result.symbols, - diagnostics, + symbols, shape_cardinalities, + parse_diagnostics, + alt_kind_diagnostics, + resolve_diagnostics, + ref_cycle_diagnostics, + shape_diagnostics, } } @@ -172,4 +172,59 @@ impl<'a> Query<'a> { .copied() .unwrap_or(ShapeCardinality::One) } + + /// All diagnostics combined from all passes. + pub fn all_diagnostics(&self) -> Diagnostics { + let mut all = Diagnostics::new(); + all.extend(self.parse_diagnostics.clone()); + all.extend(self.alt_kind_diagnostics.clone()); + all.extend(self.resolve_diagnostics.clone()); + all.extend(self.ref_cycle_diagnostics.clone()); + all.extend(self.shape_diagnostics.clone()); + all + } + + pub fn parse_diagnostics(&self) -> &Diagnostics { + &self.parse_diagnostics + } + + pub fn alt_kind_diagnostics(&self) -> &Diagnostics { + &self.alt_kind_diagnostics + } + + pub fn resolve_diagnostics(&self) -> &Diagnostics { + &self.resolve_diagnostics + } + + pub fn ref_cycle_diagnostics(&self) -> &Diagnostics { + &self.ref_cycle_diagnostics + } + + pub fn shape_diagnostics(&self) -> &Diagnostics { + &self.shape_diagnostics + } + + pub fn diagnostics(&self) -> Diagnostics { + self.all_diagnostics() + } + + /// Query is valid if there are no error-severity diagnostics (warnings are allowed). + pub fn is_valid(&self) -> bool { + !self.parse_diagnostics.has_errors() + && !self.alt_kind_diagnostics.has_errors() + && !self.resolve_diagnostics.has_errors() + && !self.ref_cycle_diagnostics.has_errors() + && !self.shape_diagnostics.has_errors() + } + + pub fn render_diagnostics(&self) -> String { + self.all_diagnostics().printer(self.source).render() + } + + pub fn render_diagnostics_colored(&self, colored: bool) -> String { + self.all_diagnostics() + .printer(self.source) + .colored(colored) + .render() + } } diff --git a/crates/plotnik-lib/src/query/named_defs.rs b/crates/plotnik-lib/src/query/named_defs.rs index ad8740c5..18ed6cfe 100644 --- a/crates/plotnik-lib/src/query/named_defs.rs +++ b/crates/plotnik-lib/src/query/named_defs.rs @@ -7,7 +7,8 @@ use indexmap::{IndexMap, IndexSet}; use rowan::TextRange; -use crate::diagnostics::{DiagnosticMessage, DiagnosticStage, Diagnostics}; +use crate::PassResult; +use crate::diagnostics::Diagnostics; use crate::parser::{Expr, Ref, Root}; #[derive(Debug, Clone)] @@ -22,12 +23,6 @@ pub struct DefInfo { pub refs: IndexSet, } -#[derive(Debug)] -pub struct ResolveResult { - pub symbols: SymbolTable, - pub diagnostics: Diagnostics, -} - impl SymbolTable { pub fn get(&self, name: &str) -> Option<&DefInfo> { self.defs.get(name) @@ -50,7 +45,7 @@ impl SymbolTable { } } -pub fn resolve(root: &Root) -> ResolveResult { +pub fn resolve(root: &Root) -> PassResult { let mut defs = IndexMap::new(); let mut diagnostics = Diagnostics::new(); @@ -64,10 +59,9 @@ pub fn resolve(root: &Root) -> ResolveResult { let range = name_token.text_range(); if defs.contains_key(&name) { - diagnostics.push( - DiagnosticMessage::error(range, format!("duplicate definition: `{}`", name)) - .with_stage(DiagnosticStage::Resolve), - ); + diagnostics + .error(format!("duplicate definition: `{}`", name), range) + .emit(); continue; } @@ -92,10 +86,7 @@ pub fn resolve(root: &Root) -> ResolveResult { "named_defs: unexpected bare Expr in Root (parser should wrap in Def)" ); - ResolveResult { - symbols, - diagnostics, - } + Ok((symbols, diagnostics)) } fn collect_refs(expr: &Expr, refs: &mut IndexSet) { @@ -195,11 +186,10 @@ fn check_ref_diagnostic(r: &Ref, symbols: &SymbolTable, diagnostics: &mut Diagno return; } - diagnostics.push( - DiagnosticMessage::error( - name_token.text_range(), + diagnostics + .error( format!("undefined reference: `{}`", name), + name_token.text_range(), ) - .with_stage(DiagnosticStage::Resolve), - ); + .emit(); } diff --git a/crates/plotnik-lib/src/query/printer.rs b/crates/plotnik-lib/src/query/printer.rs index cb9ab63d..2d88d90b 100644 --- a/crates/plotnik-lib/src/query/printer.rs +++ b/crates/plotnik-lib/src/query/printer.rs @@ -318,6 +318,7 @@ impl<'q, 'src> QueryPrinter<'q, 'src> { match self.query.shape_cardinality(node) { ShapeCardinality::One => "¹", ShapeCardinality::Many => "⁺", + ShapeCardinality::Invalid => "⁻", } } diff --git a/crates/plotnik-lib/src/query/ref_cycles.rs b/crates/plotnik-lib/src/query/ref_cycles.rs index 205d1be6..c8bd8da8 100644 --- a/crates/plotnik-lib/src/query/ref_cycles.rs +++ b/crates/plotnik-lib/src/query/ref_cycles.rs @@ -7,10 +7,11 @@ use indexmap::{IndexMap, IndexSet}; use rowan::TextRange; use super::named_defs::SymbolTable; -use crate::diagnostics::{DiagnosticMessage, DiagnosticStage, Diagnostics, RelatedInfo}; +use crate::PassResult; +use crate::diagnostics::Diagnostics; use crate::parser::{Def, Expr, Root, SyntaxKind}; -pub fn validate(root: &Root, symbols: &SymbolTable) -> Diagnostics { +pub fn validate(root: &Root, symbols: &SymbolTable) -> PassResult<()> { let sccs = find_sccs(symbols); let mut errors = Diagnostics::new(); @@ -36,7 +37,7 @@ pub fn validate(root: &Root, symbols: &SymbolTable) -> Diagnostics { let scc_set: IndexSet<&str> = std::iter::once(name.as_str()).collect(); if !expr_has_escape(&body, &scc_set) { let chain = build_self_ref_chain(root, name); - errors.push(make_error(name, &scc, chain)); + emit_error(&mut errors, name, &scc, chain); } continue; } @@ -56,11 +57,11 @@ pub fn validate(root: &Root, symbols: &SymbolTable) -> Diagnostics { if !any_has_escape { let chain = build_cycle_chain(root, symbols, &scc); - errors.push(make_error(&scc[0], &scc, chain)); + emit_error(&mut errors, &scc[0], &scc, chain); } } - errors + Ok(((), errors)) } fn expr_has_escape(expr: &Expr, scc: &IndexSet<&str>) -> bool { @@ -230,18 +231,17 @@ fn find_ref_in_expr(expr: &Expr, target: &str) -> Option { } } -fn build_self_ref_chain(root: &Root, name: &str) -> Vec { +fn build_self_ref_chain(root: &Root, name: &str) -> Vec<(TextRange, String)> { find_reference_location(root, name, name) - .map(|range| { - vec![RelatedInfo::new( - range, - format!("`{}` references itself", name), - )] - }) + .map(|range| vec![(range, format!("`{}` references itself", name))]) .unwrap_or_default() } -fn build_cycle_chain(root: &Root, symbols: &SymbolTable, scc: &[String]) -> Vec { +fn build_cycle_chain( + root: &Root, + symbols: &SymbolTable, + scc: &[String], +) -> Vec<(TextRange, String)> { let scc_set: IndexSet<&str> = scc.iter().map(|s| s.as_str()).collect(); let mut visited = IndexSet::new(); let mut path = Vec::new(); @@ -287,13 +287,18 @@ fn build_cycle_chain(root: &Root, symbols: &SymbolTable, scc: &[String]) -> Vec< } else { format!("`{}` references `{}`", from, to) }; - RelatedInfo::new(range, msg) + (range, msg) }) }) .collect() } -fn make_error(primary_name: &str, scc: &[String], related: Vec) -> DiagnosticMessage { +fn emit_error( + errors: &mut Diagnostics, + primary_name: &str, + scc: &[String], + related: Vec<(TextRange, String)>, +) { let cycle_str = if scc.len() == 1 { format!("`{}` → `{}`", primary_name, primary_name) } else { @@ -304,16 +309,20 @@ fn make_error(primary_name: &str, scc: &[String], related: Vec) -> let range = related .first() - .map(|r| r.range) + .map(|(r, _)| *r) .unwrap_or_else(|| TextRange::empty(0.into())); - DiagnosticMessage::error( - range, + let mut builder = errors.error( format!( "recursive pattern can never match: cycle {} has no escape path", cycle_str ), - ) - .with_related_many(related) - .with_stage(DiagnosticStage::Escape) + range, + ); + + for (rel_range, rel_msg) in related { + builder = builder.related_to(rel_msg, rel_range); + } + + builder.emit(); } diff --git a/crates/plotnik-lib/src/query/shape_cardinalities.rs b/crates/plotnik-lib/src/query/shape_cardinalities.rs index 4ad477b7..84efa094 100644 --- a/crates/plotnik-lib/src/query/shape_cardinalities.rs +++ b/crates/plotnik-lib/src/query/shape_cardinalities.rs @@ -4,14 +4,15 @@ //! or multiple sequential positions (`Many`). Used to validate field constraints: //! `field: expr` requires `expr` to have `ShapeCardinality::One`. //! +//! `Invalid` marks nodes where cardinality cannot be determined (error nodes, +//! undefined refs, etc.). +//! //! Root node cardinality indicates definition count (one vs multiple subqueries), //! not node matching semantics. -use super::invariants::{ - assert_ref_in_symbols, ensure_ref_body, ensure_ref_name, panic_unexpected_node, -}; use super::named_defs::SymbolTable; -use crate::diagnostics::{DiagnosticMessage, DiagnosticStage, Diagnostics}; +use crate::PassResult; +use crate::diagnostics::Diagnostics; use crate::parser::{Branch, Def, Expr, Field, Ref, Root, Seq, SyntaxNode, Type}; use std::collections::HashMap; @@ -19,10 +20,15 @@ use std::collections::HashMap; pub enum ShapeCardinality { One, Many, + Invalid, } -pub fn infer(root: &Root, symbols: &SymbolTable) -> HashMap { +pub fn analyze( + root: &Root, + symbols: &SymbolTable, +) -> PassResult> { let mut result = HashMap::new(); + let mut errors = Diagnostics::new(); let mut def_bodies: HashMap = HashMap::new(); for def in root.defs() { @@ -32,8 +38,9 @@ pub fn infer(root: &Root, symbols: &SymbolTable) -> HashMap, cache: &mut HashMap, ) -> ShapeCardinality { - let name_tok = ensure_ref_name(r); + let Some(name_tok) = r.name() else { + return ShapeCardinality::Invalid; + }; let name = name_tok.text(); - assert_ref_in_symbols(name, r, symbols.get(name).is_some()); + if symbols.get(name).is_none() { + return ShapeCardinality::Invalid; + } - let body_node = ensure_ref_body(name, r, def_bodies.get(name)); + let Some(body_node) = def_bodies.get(name) else { + return ShapeCardinality::Invalid; + }; get_or_compute(body_node, symbols, def_bodies, cache) } -pub fn validate( - root: &Root, - _symbols: &SymbolTable, - cardinalities: &HashMap, -) -> Diagnostics { - let mut errors = Diagnostics::new(); - validate_node(&root.syntax().clone(), cardinalities, &mut errors); - errors -} - fn validate_node( node: &SyntaxNode, cardinalities: &HashMap, @@ -187,16 +191,15 @@ fn validate_node( .map(|t| t.text().to_string()) .unwrap_or_else(|| "field".to_string()); - errors.push( - DiagnosticMessage::error( - value.syntax().text_range(), + errors + .error( format!( "field `{}` value must match a single node, not a sequence", field_name ), + value.syntax().text_range(), ) - .with_stage(DiagnosticStage::Validate), - ); + .emit(); } } diff --git a/crates/plotnik-lib/src/query/shape_cardinalities_tests.rs b/crates/plotnik-lib/src/query/shape_cardinalities_tests.rs index d3c71d11..0c251e8d 100644 --- a/crates/plotnik-lib/src/query/shape_cardinalities_tests.rs +++ b/crates/plotnik-lib/src/query/shape_cardinalities_tests.rs @@ -357,3 +357,47 @@ fn literal_is_one() { Str¹ "if" "#); } + +#[test] +fn invalid_error_node() { + let query = Query::new("(foo %)").unwrap(); + assert!(!query.is_valid()); + insta::assert_snapshot!(query.dump_cst_with_cardinalities(), @r#" + Root¹ + Def¹ + Tree¹ + ParenOpen "(" + Id "foo" + Error⁻ + Garbage "%" + ParenClose ")" + "#); +} + +#[test] +fn invalid_undefined_ref() { + let query = Query::new("(Undefined)").unwrap(); + assert!(!query.is_valid()); + insta::assert_snapshot!(query.dump_with_cardinalities(), @r" + Root¹ + Def⁻ + Ref⁻ Undefined + "); +} + +#[test] +fn invalid_ref_to_bodyless_def() { + let input = indoc! {r#" + X = % + (X) + "#}; + let query = Query::new(input).unwrap(); + assert!(!query.is_valid()); + insta::assert_snapshot!(query.dump_with_cardinalities(), @r" + Root⁺ + Def¹ X + Def¹ + Def⁻ + Ref⁻ X + "); +} From 0c612507db0dacbfbbf4cb18bafaabc24061aea8 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Thu, 4 Dec 2025 01:38:34 -0300 Subject: [PATCH 4/4] Add invariant functions for shape cardinality checks --- crates/plotnik-lib/src/query/invariants.rs | 24 +++++++++++++++++ .../src/query/shape_cardinalities.rs | 27 ++++++++++--------- .../src/query/shape_cardinalities_tests.rs | 16 +++++++++-- 3 files changed, 52 insertions(+), 15 deletions(-) diff --git a/crates/plotnik-lib/src/query/invariants.rs b/crates/plotnik-lib/src/query/invariants.rs index 19a9c6d8..3c47d051 100644 --- a/crates/plotnik-lib/src/query/invariants.rs +++ b/crates/plotnik-lib/src/query/invariants.rs @@ -33,3 +33,27 @@ pub fn ensure_both_branch_kinds<'a>( ), } } + +#[inline] +pub fn ensure_capture_has_inner(inner: Option) -> T { + inner.expect( + "shape_cardinalities: Capture without inner expression \ + (parser uses checkpoint, inner is guaranteed)", + ) +} + +#[inline] +pub fn ensure_quantifier_has_inner(inner: Option) -> T { + inner.expect( + "shape_cardinalities: Quantifier without inner expression \ + (parser uses checkpoint, inner is guaranteed)", + ) +} + +#[inline] +pub fn ensure_ref_has_name(name: Option) -> T { + name.expect( + "shape_cardinalities: Ref without name token \ + (parser only creates Ref for PascalCase Id)", + ) +} diff --git a/crates/plotnik-lib/src/query/shape_cardinalities.rs b/crates/plotnik-lib/src/query/shape_cardinalities.rs index 84efa094..8dc1749e 100644 --- a/crates/plotnik-lib/src/query/shape_cardinalities.rs +++ b/crates/plotnik-lib/src/query/shape_cardinalities.rs @@ -10,6 +10,9 @@ //! Root node cardinality indicates definition count (one vs multiple subqueries), //! not node matching semantics. +use super::invariants::{ + ensure_capture_has_inner, ensure_quantifier_has_inner, ensure_ref_has_name, +}; use super::named_defs::SymbolTable; use crate::PassResult; use crate::diagnostics::Diagnostics; @@ -76,14 +79,14 @@ fn compute_single( return def .body() .map(|b| get_or_compute(b.syntax(), symbols, def_bodies, cache)) - .unwrap_or(ShapeCardinality::One); + .unwrap_or(ShapeCardinality::Invalid); } if let Some(branch) = Branch::cast(node.clone()) { return branch .body() .map(|b| get_or_compute(b.syntax(), symbols, def_bodies, cache)) - .unwrap_or(ShapeCardinality::One); + .unwrap_or(ShapeCardinality::Invalid); } // Type annotations are metadata, not matching expressions @@ -107,15 +110,15 @@ fn compute_single( Expr::Seq(ref seq) => seq_cardinality(seq, symbols, def_bodies, cache), - Expr::Capture(ref cap) => cap - .inner() - .map(|inner| get_or_compute(inner.syntax(), symbols, def_bodies, cache)) - .unwrap_or(ShapeCardinality::One), + Expr::Capture(ref cap) => { + let inner = ensure_capture_has_inner(cap.inner()); + get_or_compute(inner.syntax(), symbols, def_bodies, cache) + } - Expr::Quantifier(ref q) => q - .inner() - .map(|inner| get_or_compute(inner.syntax(), symbols, def_bodies, cache)) - .unwrap_or(ShapeCardinality::One), + Expr::Quantifier(ref q) => { + let inner = ensure_quantifier_has_inner(q.inner()); + get_or_compute(inner.syntax(), symbols, def_bodies, cache) + } Expr::Ref(ref r) => ref_cardinality(r, symbols, def_bodies, cache), } @@ -156,9 +159,7 @@ fn ref_cardinality( def_bodies: &HashMap, cache: &mut HashMap, ) -> ShapeCardinality { - let Some(name_tok) = r.name() else { - return ShapeCardinality::Invalid; - }; + let name_tok = ensure_ref_has_name(r.name()); let name = name_tok.text(); if symbols.get(name).is_none() { diff --git a/crates/plotnik-lib/src/query/shape_cardinalities_tests.rs b/crates/plotnik-lib/src/query/shape_cardinalities_tests.rs index 0c251e8d..b2817621 100644 --- a/crates/plotnik-lib/src/query/shape_cardinalities_tests.rs +++ b/crates/plotnik-lib/src/query/shape_cardinalities_tests.rs @@ -385,6 +385,18 @@ fn invalid_undefined_ref() { "); } +#[test] +fn invalid_branch_without_body() { + let query = Query::new("[A:]").unwrap(); + assert!(!query.is_valid()); + insta::assert_snapshot!(query.dump_with_cardinalities(), @r" + Root¹ + Def¹ + Alt¹ + Branch⁻ A: + "); +} + #[test] fn invalid_ref_to_bodyless_def() { let input = indoc! {r#" @@ -395,8 +407,8 @@ fn invalid_ref_to_bodyless_def() { assert!(!query.is_valid()); insta::assert_snapshot!(query.dump_with_cardinalities(), @r" Root⁺ - Def¹ X - Def¹ + Def⁻ X + Def⁻ Def⁻ Ref⁻ X ");