From 849a4bbaf3c1b839eb697d13c4311e5f862243ed Mon Sep 17 00:00:00 2001 From: konojunya Date: Thu, 3 Sep 2026 20:50:00 +0900 Subject: [PATCH 1/3] Add structured diagnostic expectations --- src/diagnostic.rs | 41 ++++++++++++++++++++++++++++++++++++ tests/specification-revision | 2 +- 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/diagnostic.rs b/src/diagnostic.rs index 331dda9..1ae599e 100644 --- a/src/diagnostic.rs +++ b/src/diagnostic.rs @@ -94,6 +94,8 @@ pub struct Diagnostic { pub message: String, /// Primary source span. pub span: Span, + /// Ordered source values or constructs valid at the primary span. + pub expected: Vec, /// Optional corrective guidance. pub help: Option, /// Other declarations or references involved in the problem. @@ -107,6 +109,7 @@ impl Diagnostic { severity: Severity::Error, message: message.into(), span, + expected: Vec::new(), help: None, related: Vec::new(), } @@ -118,6 +121,7 @@ impl Diagnostic { severity: Severity::Warning, message: message.into(), span, + expected: Vec::new(), help: None, related: Vec::new(), } @@ -128,6 +132,15 @@ impl Diagnostic { self } + pub(crate) fn with_expected(mut self, expected: I) -> Self + where + I: IntoIterator, + S: Into, + { + self.expected = expected.into_iter().map(Into::into).collect(); + self + } + pub(crate) fn with_related(mut self, message: impl Into, span: Span) -> Self { self.related.push(RelatedInformation { message: message.into(), @@ -136,3 +149,31 @@ impl Diagnostic { self } } + +#[cfg(test)] +mod tests { + use super::{Diagnostic, SourcePosition, Span}; + + #[test] + fn diagnostics_default_to_no_expected_values() { + let diagnostic = Diagnostic::error( + "STK2002", + "Unexpected value.", + Span::point(SourcePosition::start()), + ); + + assert!(diagnostic.expected.is_empty()); + } + + #[test] + fn diagnostics_preserve_expected_value_order() { + let diagnostic = Diagnostic::error( + "STK2002", + "Unknown direction.", + Span::point(SourcePosition::start()), + ) + .with_expected(["right", "down"]); + + assert_eq!(diagnostic.expected, ["right", "down"]); + } +} diff --git a/tests/specification-revision b/tests/specification-revision index 6da9620..20d6940 100644 --- a/tests/specification-revision +++ b/tests/specification-revision @@ -1 +1 @@ -f382069928c805fe69b7a192bfd6a877036bc036 +7f9154d22702ddf02f2713bbc06dde7bdf635806 From 0f0e6c00ca4e281f56303213a557ed1d5a014a07 Mon Sep 17 00:00:00 2001 From: konojunya Date: Thu, 3 Sep 2026 20:51:25 +0900 Subject: [PATCH 2/3] Report parser expectations --- src/parser.rs | 180 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 122 insertions(+), 58 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index 26fde4c..d6a4101 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -40,7 +40,7 @@ impl Parser { fn parse_version(&mut self) -> ParseResult { let start = self.expect_keyword("stack")?.span; let (major, _) = self.expect_integer("language major version")?; - self.expect_simple(|kind| matches!(kind, TokenKind::Dot), "'.'")?; + self.expect_simple(|kind| matches!(kind, TokenKind::Dot), "'.'", ".")?; let (minor, minor_span) = self.expect_integer("language minor version")?; Ok(Version { @@ -53,7 +53,7 @@ impl Parser { fn parse_diagram(&mut self) -> ParseResult { let start = self.expect_keyword("diagram")?.span; let title = self.expect_string("diagram title")?; - self.expect_simple(|kind| matches!(kind, TokenKind::LeftBrace), "'{'")?; + self.expect_simple(|kind| matches!(kind, TokenKind::LeftBrace), "'{'", "{")?; let mut members = Vec::new(); while !self.at_simple(|kind| matches!(kind, TokenKind::RightBrace)) { @@ -64,12 +64,17 @@ impl Parser { Some("edge") => DiagramMember::Edge(self.parse_edge()?), Some("theme") => DiagramMember::Theme(self.parse_theme()?), Some("layout") => DiagramMember::Layout(self.parse_layout()?), - _ => return Err(self.unexpected("a diagram declaration")), + _ => { + return Err(self.unexpected( + "a diagram declaration", + ["node", "group", "edge", "theme", "layout", "}"], + )); + } }); } let end = self - .expect_simple(|kind| matches!(kind, TokenKind::RightBrace), "'}'")? + .expect_simple(|kind| matches!(kind, TokenKind::RightBrace), "'}'", "}")? .span; Ok(Diagram { @@ -83,7 +88,7 @@ impl Parser { let start = self.expect_keyword("group")?.span; let identifier = self.expect_identifier("group identifier")?; let label = self.expect_string("group label")?; - self.expect_simple(|kind| matches!(kind, TokenKind::LeftBrace), "'{'")?; + self.expect_simple(|kind| matches!(kind, TokenKind::LeftBrace), "'{'", "{")?; let mut members = Vec::new(); while !self.at_simple(|kind| matches!(kind, TokenKind::RightBrace)) { @@ -92,12 +97,17 @@ impl Parser { Some("node") => GroupMember::Node(self.parse_node()?), Some("group") => GroupMember::Group(self.parse_group()?), Some("layout") => GroupMember::Layout(self.parse_layout()?), - _ => return Err(self.unexpected("a node, group, or layout declaration")), + _ => { + return Err(self.unexpected( + "a node, group, or layout declaration", + ["node", "group", "layout", "}"], + )); + } }); } let end = self - .expect_simple(|kind| matches!(kind, TokenKind::RightBrace), "'}'")? + .expect_simple(|kind| matches!(kind, TokenKind::RightBrace), "'}'", "}")? .span; Ok(Group { @@ -120,7 +130,9 @@ impl Parser { .is_some() { if self.at_simple(|kind| matches!(kind, TokenKind::RightBrace)) { - return Err(self.unexpected("at least one node property")); + return Err( + self.unexpected("at least one node property", ["kind", "icon", "detail"]) + ); } while !self.at_simple(|kind| matches!(kind, TokenKind::RightBrace)) { @@ -138,12 +150,16 @@ impl Parser { self.advance(); NodeProperty::Detail(self.expect_string("node detail")?) } - _ => return Err(self.unexpected("a node property")), + _ => { + return Err( + self.unexpected("a node property", ["kind", "icon", "detail", "}"]) + ); + } }); } end = self - .expect_simple(|kind| matches!(kind, TokenKind::RightBrace), "'}'")? + .expect_simple(|kind| matches!(kind, TokenKind::RightBrace), "'}'", "}")? .span; } @@ -163,7 +179,7 @@ impl Parser { TokenKind::ForwardArrow => EdgeOperator::Forward, TokenKind::BidirectionalArrow => EdgeOperator::Bidirectional, TokenKind::Association => EdgeOperator::Association, - _ => return Err(self.unexpected("an edge operator")), + _ => return Err(self.unexpected("an edge operator", ["->", "<->", "--"])), }; self.advance(); let operator = Spanned::new(operator, operator_token.span); @@ -181,7 +197,7 @@ impl Parser { .is_some() { if self.at_simple(|kind| matches!(kind, TokenKind::RightBrace)) { - return Err(self.unexpected("at least one edge property")); + return Err(self.unexpected("at least one edge property", ["kind"])); } while !self.at_simple(|kind| matches!(kind, TokenKind::RightBrace)) { @@ -191,12 +207,12 @@ impl Parser { self.advance(); EdgeProperty::Kind(self.expect_identifier("edge kind")?) } - _ => return Err(self.unexpected("an edge property")), + _ => return Err(self.unexpected("an edge property", ["kind", "}"])), }); } end = self - .expect_simple(|kind| matches!(kind, TokenKind::RightBrace), "'}'")? + .expect_simple(|kind| matches!(kind, TokenKind::RightBrace), "'}'", "}")? .span; } @@ -221,9 +237,12 @@ impl Parser { fn parse_layout(&mut self) -> ParseResult { let start = self.expect_keyword("layout")?.span; - self.expect_simple(|kind| matches!(kind, TokenKind::LeftBrace), "'{'")?; + self.expect_simple(|kind| matches!(kind, TokenKind::LeftBrace), "'{'", "{")?; if self.at_simple(|kind| matches!(kind, TokenKind::RightBrace)) { - return Err(self.unexpected("at least one layout statement")); + return Err(self.unexpected( + "at least one layout statement", + ["direction", "rank", "order"], + )); } let mut statements = Vec::new(); @@ -243,12 +262,16 @@ impl Parser { self.advance(); LayoutStatement::Order(self.parse_identifier_list()?) } - _ => return Err(self.unexpected("a layout statement")), + _ => { + return Err( + self.unexpected("a layout statement", ["direction", "rank", "order", "}"]) + ); + } }); } let end = self - .expect_simple(|kind| matches!(kind, TokenKind::RightBrace), "'}'")? + .expect_simple(|kind| matches!(kind, TokenKind::RightBrace), "'}'", "}")? .span; Ok(Layout { statements, @@ -258,10 +281,10 @@ impl Parser { fn parse_identifier_list(&mut self) -> ParseResult { let start = self - .expect_simple(|kind| matches!(kind, TokenKind::LeftBracket), "'['")? + .expect_simple(|kind| matches!(kind, TokenKind::LeftBracket), "'['", "[")? .span; let mut identifiers = vec![self.expect_identifier("layout identifier")?]; - self.expect_simple(|kind| matches!(kind, TokenKind::Comma), "','")?; + self.expect_simple(|kind| matches!(kind, TokenKind::Comma), "','", ",")?; identifiers.push(self.expect_identifier("layout identifier")?); while self @@ -272,7 +295,7 @@ impl Parser { } let end = self - .expect_simple(|kind| matches!(kind, TokenKind::RightBracket), "']'")? + .expect_simple(|kind| matches!(kind, TokenKind::RightBracket), "']'", "]")? .span; Ok(IdentifierList { identifiers, @@ -281,23 +304,32 @@ impl Parser { } fn expect_integer(&mut self, description: &str) -> ParseResult<(u32, Span)> { - let value = self.expect_identifier(description)?; - if value.value.len() > 1 && value.value.starts_with('0') { - return Err(Box::new(Diagnostic::error( - "STK2002", - format!("Expected {description} without leading zeroes."), - value.span, - ))); + let token = self.current_token().clone(); + let TokenKind::Bare(value) = token.kind else { + return Err(self.unexpected(description, [""])); + }; + self.advance(); + + if value.len() > 1 && value.starts_with('0') { + return Err(Box::new( + Diagnostic::error( + "STK2002", + format!("Expected {description} without leading zeroes."), + token.span, + ) + .with_expected([""]) + .with_help("Use an unsigned decimal integer without leading zeroes."), + )); } - let parsed = value.value.parse::().map_err(|_| { - Box::new(Diagnostic::error( - "STK2002", - format!("Expected {description}."), - value.span, - )) + let parsed = value.parse::().map_err(|_| { + Box::new( + Diagnostic::error("STK2002", format!("Expected {description}."), token.span) + .with_expected([""]) + .with_help("Use an unsigned decimal integer without leading zeroes."), + ) })?; - Ok((parsed, value.span)) + Ok((parsed, token.span)) } fn expect_identifier(&mut self, description: &str) -> ParseResult> { @@ -307,7 +339,7 @@ impl Parser { self.advance(); Ok(Spanned::new(value, token.span)) } - _ => Err(self.unexpected(description)), + _ => Err(self.unexpected(description, [""])), } } @@ -318,13 +350,13 @@ impl Parser { self.advance(); Ok(Spanned::new(value, token.span)) } - _ => Err(self.unexpected(description)), + _ => Err(self.unexpected(description, [""])), } } fn expect_keyword(&mut self, keyword: &str) -> ParseResult { if self.current_bare() != Some(keyword) { - return Err(self.unexpected(&format!("'{keyword}'"))); + return Err(self.unexpected(&format!("'{keyword}'"), [keyword])); } let token = self.current_token().clone(); self.advance(); @@ -335,9 +367,10 @@ impl Parser { &mut self, predicate: impl FnOnce(&TokenKind) -> bool, description: &str, + expected: &str, ) -> ParseResult { if !predicate(&self.current_token().kind) { - return Err(self.unexpected(description)); + return Err(self.unexpected(description, [expected])); } let token = self.current_token().clone(); self.advance(); @@ -357,35 +390,52 @@ impl Parser { if matches!(self.current_token().kind, TokenKind::End) { Ok(()) } else { - Err(self.unexpected("the end of the document")) + Err(self.unexpected("the end of the document", [""])) } } fn reject_end(&self, construct: &str) -> ParseResult<()> { if matches!(self.current_token().kind, TokenKind::End) { - Err(Box::new(Diagnostic::error( - "STK2003", - format!("Input ended before the {construct} was complete."), - self.current_token().span, - ))) + Err(Box::new( + Diagnostic::error( + "STK2003", + format!("Input ended before the {construct} was complete."), + self.current_token().span, + ) + .with_expected(["}"]) + .with_help("Add the closing '}' for this construct."), + )) } else { Ok(()) } } - fn unexpected(&self, expected: &str) -> Box { + fn unexpected( + &self, + description: &str, + expected: [&str; N], + ) -> Box { + let help = format!("Use one of: {}.", expected.join(", ")); if matches!(self.current_token().kind, TokenKind::End) { - Box::new(Diagnostic::error( - "STK2003", - format!("Input ended while expecting {expected}."), - self.current_token().span, - )) + Box::new( + Diagnostic::error( + "STK2003", + format!("Input ended while expecting {description}."), + self.current_token().span, + ) + .with_expected(expected) + .with_help(help), + ) } else { - Box::new(Diagnostic::error( - "STK2002", - format!("Expected {expected}."), - self.current_token().span, - )) + Box::new( + Diagnostic::error( + "STK2002", + format!("Expected {description}."), + self.current_token().span, + ) + .with_expected(expected) + .with_help(help), + ) } } @@ -487,10 +537,24 @@ diagram "System" { #[test] fn rejects_unknown_and_incomplete_syntax() { let unknown = parse("stack 1.0 diagram \"x\" { server api \"API\" }"); - assert!(matches!(unknown, Err(diagnostic) if diagnostic.code == "STK2002")); + assert!(matches!( + unknown, + Err(diagnostic) + if diagnostic.code == "STK2002" + && diagnostic.expected == ["node", "group", "edge", "theme", "layout", "}"] + && diagnostic.help.as_deref() + == Some("Use one of: node, group, edge, theme, layout, }.") + )); let incomplete = parse("stack 1.0 diagram \"x\" { node api \"API\""); - assert!(matches!(incomplete, Err(diagnostic) if diagnostic.code == "STK2003")); + assert!(matches!( + incomplete, + Err(diagnostic) + if diagnostic.code == "STK2003" + && diagnostic.expected == ["}"] + && diagnostic.help.as_deref() + == Some("Add the closing '}' for this construct.") + )); } #[test] From cdd0969a59fd011dc685367754591e263e253550 Mon Sep 17 00:00:00 2001 From: konojunya Date: Thu, 3 Sep 2026 20:54:36 +0900 Subject: [PATCH 3/3] Add actionable compiler guidance --- src/lexer.rs | 157 +++++++++++++-------- src/lib.rs | 1 + src/validation.rs | 306 ++++++++++++++++++++++++++++++---------- src/validation/tests.rs | 75 ++++++++++ tests/conformance.rs | 29 +++- 5 files changed, 427 insertions(+), 141 deletions(-) diff --git a/src/lexer.rs b/src/lexer.rs index 9177801..77f5304 100644 --- a/src/lexer.rs +++ b/src/lexer.rs @@ -32,11 +32,14 @@ pub(crate) fn tokenize(source: &str) -> LexResult> { line: 1, column: 2, }; - return Err(Box::new(Diagnostic::error( - "STK1002", - "A byte order mark is not permitted.", - Span { start, end }, - ))); + return Err(Box::new( + Diagnostic::error( + "STK1002", + "A byte order mark is not permitted.", + Span { start, end }, + ) + .with_help("Remove the UTF-8 byte order mark at the start of the file."), + )); } Lexer::new(source).tokenize() @@ -131,14 +134,18 @@ impl<'source> Lexer<'source> { loop { let Some(character) = self.peek() else { - return Err(Box::new(Diagnostic::error( - "STK2003", - "Input ended before the string was closed.", - Span { - start, - end: self.position, - }, - ))); + return Err(Box::new( + Diagnostic::error( + "STK2003", + "Input ended before the string was closed.", + Span { + start, + end: self.position, + }, + ) + .with_expected(["\""]) + .with_help("Add a closing double quote."), + )); }; match character { @@ -162,22 +169,30 @@ impl<'source> Lexer<'source> { start: self.position, end: self.position_after_current(), }; - return Err(Box::new(Diagnostic::error( - "STK1003", - "Strings cannot contain line breaks or tabs.", - span, - ))); + return Err(Box::new( + Diagnostic::error( + "STK1003", + "Strings cannot contain line breaks or tabs.", + span, + ) + .with_help( + "Remove the line break or tab; Stack strings must stay on one line.", + ), + )); } value_character if value_character.is_control() => { let span = Span { start: self.position, end: self.position_after_current(), }; - return Err(Box::new(Diagnostic::error( - "STK1003", - "Strings cannot contain control characters.", - span, - ))); + return Err(Box::new( + Diagnostic::error( + "STK1003", + "Strings cannot contain control characters.", + span, + ) + .with_help("Remove the control character from the string."), + )); } value_character => { value.push(value_character); @@ -257,14 +272,18 @@ impl<'source> Lexer<'source> { fn lex_escape(&mut self, value: &mut String, string_start: SourcePosition) -> LexResult<()> { let escape_start = self.position; let Some(character) = self.peek() else { - return Err(Box::new(Diagnostic::error( - "STK1003", - "Input ended inside a string escape.", - Span { - start: string_start, - end: self.position, - }, - ))); + return Err(Box::new( + Diagnostic::error( + "STK1003", + "Input ended inside a string escape.", + Span { + start: string_start, + end: self.position, + }, + ) + .with_expected([r#"\""#, r#"\\"#, r#"\uXXXX"#]) + .with_help("Complete the escape using a supported Stack string escape."), + )); }; match character { @@ -303,14 +322,19 @@ impl<'source> Lexer<'source> { return Err(self.invalid_escape(escape_start)); }; if decoded.is_control() || matches!(decoded, '\n' | '\r' | '\t') { - return Err(Box::new(Diagnostic::error( - "STK1003", - "A string escape decoded to a prohibited control value.", - Span { - start: escape_start, - end: self.position, - }, - ))); + return Err(Box::new( + Diagnostic::error( + "STK1003", + "A string escape decoded to a prohibited control value.", + Span { + start: escape_start, + end: self.position, + }, + ) + .with_help( + "Use a printable Unicode scalar that is not a line break or tab.", + ), + )); } value.push(decoded); } @@ -337,25 +361,32 @@ impl<'source> Lexer<'source> { } fn invalid_escape(&self, start: SourcePosition) -> Box { - Box::new(Diagnostic::error( - "STK1003", - "The string contains an invalid escape.", - Span { - start, - end: self.position, - }, - )) + Box::new( + Diagnostic::error( + "STK1003", + "The string contains an invalid escape.", + Span { + start, + end: self.position, + }, + ) + .with_expected([r#"\""#, r#"\\"#, r#"\uXXXX"#]) + .with_help("Use an escaped double quote, backslash, or Unicode code unit."), + ) } fn invalid_surrogate(&self, start: SourcePosition) -> Box { - Box::new(Diagnostic::error( - "STK1003", - "The string contains an unpaired Unicode surrogate.", - Span { - start, - end: self.position, - }, - )) + Box::new( + Diagnostic::error( + "STK1003", + "The string contains an unpaired Unicode surrogate.", + Span { + start, + end: self.position, + }, + ) + .with_help("Pair high and low surrogates or use a non-surrogate Unicode code unit."), + ) } fn lex_bare(&mut self) -> TokenKind { @@ -512,12 +543,26 @@ mod tests { let result = tokenize(source); assert!(matches!(result, Err(diagnostic) if diagnostic.code == "STK1003")); } + + let invalid_escape = tokenize(r#""\n""#); + assert!(matches!( + invalid_escape, + Err(diagnostic) + if diagnostic.expected == [r#"\""#, r#"\\"#, r#"\uXXXX"#] + && diagnostic.help.is_some() + )); } #[test] fn reports_an_unterminated_string() { let result = tokenize("\"unfinished"); - assert!(matches!(result, Err(diagnostic) if diagnostic.code == "STK2003")); + assert!(matches!( + result, + Err(diagnostic) + if diagnostic.code == "STK2003" + && diagnostic.expected == ["\""] + && diagnostic.help.as_deref() == Some("Add a closing double quote.") + )); } #[test] diff --git a/src/lib.rs b/src/lib.rs index 7d1cac8..b1cf150 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -226,6 +226,7 @@ fn invalid_utf8_diagnostic(source: &[u8], error: std::str::Utf8Error) -> Diagnos "Input is not valid UTF-8.", diagnostic::Span::point(position), ) + .with_help("Save the source as UTF-8 and replace the invalid byte sequence.") } #[cfg(test)] diff --git a/src/validation.rs b/src/validation.rs index efbb8d5..0654830 100644 --- a/src/validation.rs +++ b/src/validation.rs @@ -7,6 +7,12 @@ use crate::ir; const SUPPORTED_MAJOR: u32 = 1; const SUPPORTED_MINOR: u32 = 0; +const NODE_KINDS: [&str; 10] = [ + "actor", "client", "service", "function", "worker", "database", "cache", "queue", "storage", + "external", +]; +const EDGE_KINDS: [&str; 5] = ["flow", "request", "event", "data", "dependency"]; +const LAYOUT_DIRECTIONS: [&str; 2] = ["right", "down"]; pub(crate) fn validate(document: &ast::Document) -> CompileOutput { let mut validator = Validator::new(document); @@ -91,6 +97,7 @@ impl<'document> Validator<'document> { ), version.span, ) + .with_expected([format!("{SUPPORTED_MAJOR}.{SUPPORTED_MINOR}")]) .with_help(format!( "Use Stack {SUPPORTED_MAJOR}.{SUPPORTED_MINOR} or an older compatible minor version." )), @@ -123,11 +130,14 @@ impl<'document> Validator<'document> { self.validate_text(&group.label, 1, 60, "group label"); if depth > 3 { - self.diagnostics.push(Diagnostic::error( - "STK3010", - "Group nesting exceeds three levels below the diagram.", - group.identifier.span, - )); + self.diagnostics.push( + Diagnostic::error( + "STK3010", + "Group nesting exceeds three levels below the diagram.", + group.identifier.span, + ) + .with_help("Move this group to the third level or higher."), + ); } if descendant_node_count(group) == 0 { @@ -164,6 +174,7 @@ impl<'document> Validator<'document> { ), identifier.span, ) + .with_help("Rename or remove this duplicate declaration.") .with_related("The first declaration is here.", original.span), ); } else { @@ -191,11 +202,15 @@ impl<'document> Validator<'document> { NodeProperty::Kind(value) => { self.validate_identifier(value); if parse_node_kind(&value.value).is_none() { - self.diagnostics.push(Diagnostic::error( - "STK2002", - format!("Unknown node kind '{}'.", value.value), - value.span, - )); + self.diagnostics.push( + Diagnostic::error( + "STK2002", + format!("Unknown node kind '{}'.", value.value), + value.span, + ) + .with_expected(NODE_KINDS) + .with_help("Choose one of the supported node kinds."), + ); } } NodeProperty::Icon(value) => self.validate_icon_identifier(value), @@ -220,6 +235,7 @@ impl<'document> Validator<'document> { "A diagram may contain only one theme statement.", theme.span, ) + .with_help("Remove the duplicate theme statement.") .with_related("The first theme statement is here.", first_span), ); } else { @@ -281,6 +297,7 @@ impl<'document> Validator<'document> { "A layout scope may contain only one layout block.", duplicate.span, ) + .with_help("Remove the duplicate layout block.") .with_related("The first layout block is here.", first.span), ); } @@ -301,11 +318,17 @@ impl<'document> Validator<'document> { LayoutStatement::Direction(value) => { self.validate_identifier(value); if !matches!(value.value.as_str(), "right" | "down") { - self.diagnostics.push(Diagnostic::error( - "STK2002", - format!("Unknown layout direction '{}'.", value.value), - value.span, - )); + self.diagnostics.push( + Diagnostic::error( + "STK2002", + format!("Unknown layout direction '{}'.", value.value), + value.span, + ) + .with_expected(LAYOUT_DIRECTIONS) + .with_help( + "Use 'right' for horizontal flow or 'down' for vertical flow.", + ), + ); } self.reject_duplicate_singleton( "direction statement", @@ -326,6 +349,7 @@ impl<'document> Validator<'document> { ), identifier.span, ) + .with_help("Keep this child in only one same-rank statement.") .with_related("The child was first ranked here.", *original), ); } else { @@ -354,17 +378,38 @@ impl<'document> Validator<'document> { for identifier in &list.identifiers { let identifier_is_valid = self.validate_identifier(identifier); if identifier_is_valid && !direct_children.contains_key(identifier.value.as_str()) { - self.diagnostics.push( - Diagnostic::error( - "STK3011", - format!( - "Layout reference '{}' is not a direct child of this scope.", - identifier.value - ), - identifier.span, - ) - .with_help("Reference a node or group declared directly in this layout scope."), + let suggestions = identifier_suggestions( + &identifier.value, + direct_children.iter().map(|(name, span)| (*name, *span)), ); + let expected = suggestions + .iter() + .map(|(name, _)| name.clone()) + .collect::>(); + let mut diagnostic = Diagnostic::error( + "STK3011", + format!( + "Layout reference '{}' is not a direct child of this scope.", + identifier.value + ), + identifier.span, + ) + .with_expected(expected.clone()); + diagnostic = if expected.is_empty() { + diagnostic.with_help( + "Reference a node or group declared directly in this layout scope.", + ) + } else { + diagnostic.with_help(format!( + "Use a direct child such as {}.", + expected.join(", ") + )) + }; + for (name, span) in suggestions { + diagnostic = diagnostic + .with_related(format!("Direct child '{name}' is declared here."), span); + } + self.diagnostics.push(diagnostic); } if let Some(original) = seen.insert(identifier.value.as_str(), identifier.span) { @@ -377,6 +422,7 @@ impl<'document> Validator<'document> { ), identifier.span, ) + .with_help("Remove the repeated reference from this list.") .with_related("The first occurrence is here.", original), ); } @@ -405,11 +451,14 @@ impl<'document> Validator<'document> { } if from_is_node && to_is_node && edge.from.value == edge.to.value { - self.diagnostics.push(Diagnostic::error( - "STK3005", - format!("Edge connects node '{}' to itself.", edge.from.value), - edge.span, - )); + self.diagnostics.push( + Diagnostic::error( + "STK3005", + format!("Edge connects node '{}' to itself.", edge.from.value), + edge.span, + ) + .with_help("Connect two different nodes or remove this edge."), + ); } if let Some(edge_kind) = @@ -423,6 +472,7 @@ impl<'document> Validator<'document> { "An exact duplicate edge is declared.", edge.span, ) + .with_help("Remove this edge or change its endpoints, kind, or label.") .with_related("The first edge is here.", original), ); } @@ -460,11 +510,37 @@ impl<'document> Validator<'document> { false } None => { - self.diagnostics.push(Diagnostic::error( + let suggestions = identifier_suggestions( + &endpoint.value, + self.symbols.iter().filter_map(|(name, symbol)| { + (symbol.kind == SymbolKind::Node).then_some((*name, symbol.span)) + }), + ); + let expected = suggestions + .iter() + .map(|(name, _)| name.clone()) + .collect::>(); + let mut diagnostic = Diagnostic::error( "STK3003", format!("Unknown node '{}'.", endpoint.value), endpoint.span, - )); + ) + .with_expected(expected.clone()); + diagnostic = if expected.is_empty() { + diagnostic.with_help( + "Declare this node or replace it with an existing node identifier.", + ) + } else { + diagnostic.with_help(format!( + "Use a declared node such as {}.", + expected.join(", ") + )) + }; + for (name, span) in suggestions { + diagnostic = + diagnostic.with_related(format!("Node '{name}' is declared here."), span); + } + self.diagnostics.push(diagnostic); false } } @@ -480,11 +556,15 @@ impl<'document> Validator<'document> { if let Some(kind) = parse_edge_kind(&value.value) { effective = Some(kind.as_str()); } else { - self.diagnostics.push(Diagnostic::error( - "STK2002", - format!("Unknown edge kind '{}'.", value.value), - value.span, - )); + self.diagnostics.push( + Diagnostic::error( + "STK2002", + format!("Unknown edge kind '{}'.", value.value), + value.span, + ) + .with_expected(EDGE_KINDS) + .with_help("Choose one of the supported edge kinds."), + ); effective = None; } } @@ -520,36 +600,45 @@ impl<'document> Validator<'document> { .filter(|member| matches!(member, DiagramMember::Edge(_))) .count(); if !(1..=40).contains(&self.node_count) { - self.diagnostics.push(Diagnostic::error( - "STK4003", - format!( - "A diagram must contain between 1 and 40 nodes; found {}.", - self.node_count - ), - self.document.diagram.span, - )); + self.diagnostics.push( + Diagnostic::error( + "STK4003", + format!( + "A diagram must contain between 1 and 40 nodes; found {}.", + self.node_count + ), + self.document.diagram.span, + ) + .with_help("Add nodes or split the diagram to stay within 1 to 40 nodes."), + ); } if self.group_count > 12 { - self.diagnostics.push(Diagnostic::error( - "STK4003", - format!( - "A diagram may contain at most 12 groups; found {}.", - self.group_count - ), - self.document.diagram.span, - )); + self.diagnostics.push( + Diagnostic::error( + "STK4003", + format!( + "A diagram may contain at most 12 groups; found {}.", + self.group_count + ), + self.document.diagram.span, + ) + .with_help("Remove groups or split the diagram into focused views."), + ); } let maximum_edges = 80.min(self.node_count.saturating_mul(2)); if edge_count > maximum_edges { - self.diagnostics.push(Diagnostic::error( - "STK4003", - format!( - "A diagram with {} nodes may contain at most {maximum_edges} edges; found {edge_count}.", - self.node_count - ), - self.document.diagram.span, - )); + self.diagnostics.push( + Diagnostic::error( + "STK4003", + format!( + "A diagram with {} nodes may contain at most {maximum_edges} edges; found {edge_count}.", + self.node_count + ), + self.document.diagram.span, + ) + .with_help("Remove edges or split the diagram into focused views."), + ); } } @@ -557,22 +646,32 @@ impl<'document> Validator<'document> { if is_identifier(&identifier.value) { true } else { - self.diagnostics.push(Diagnostic::error( - "STK3001", - format!("Identifier '{}' is invalid.", identifier.value), - identifier.span, - )); + self.diagnostics.push( + Diagnostic::error( + "STK3001", + format!("Identifier '{}' is invalid.", identifier.value), + identifier.span, + ) + .with_help( + "Use 1 to 64 lowercase ASCII letters, digits, underscores, or hyphens, starting with a letter.", + ), + ); false } } fn validate_icon_identifier(&mut self, identifier: &Spanned) { if !is_icon_identifier(&identifier.value) { - self.diagnostics.push(Diagnostic::error( - "STK3013", - format!("Icon identifier '{}' is malformed.", identifier.value), - identifier.span, - )); + self.diagnostics.push( + Diagnostic::error( + "STK3013", + format!("Icon identifier '{}' is malformed.", identifier.value), + identifier.span, + ) + .with_help( + "Use 1 to 64 lowercase ASCII letters, digits, or hyphens, starting with a letter or digit.", + ), + ); } } @@ -603,13 +702,18 @@ impl<'document> Validator<'document> { .next_back() .is_some_and(char::is_whitespace); if !(minimum..=maximum).contains(&length) || boundary_whitespace { - self.diagnostics.push(Diagnostic::error( - "STK3008", - format!( - "The {description} must contain {minimum} to {maximum} Unicode scalar values without leading or trailing whitespace." - ), - value.span, - )); + self.diagnostics.push( + Diagnostic::error( + "STK3008", + format!( + "The {description} must contain {minimum} to {maximum} Unicode scalar values without leading or trailing whitespace." + ), + value.span, + ) + .with_help(format!( + "Trim the text and keep its length between {minimum} and {maximum}." + )), + ); } } @@ -626,6 +730,7 @@ impl<'document> Validator<'document> { format!("Property '{name}' occurs more than once in the same block."), span, ) + .with_help("Remove the duplicate property.") .with_related("The first property is here.", original), ); } @@ -644,6 +749,7 @@ impl<'document> Validator<'document> { format!("A layout block may contain only one {description}."), span, ) + .with_help("Remove the duplicate layout statement.") .with_related("The first occurrence is here.", *original), ); } else { @@ -681,6 +787,50 @@ fn edge_key(edge: &ast::Edge, kind: &'static str) -> EdgeKey { } } +fn identifier_suggestions<'candidate>( + authored: &str, + candidates: impl Iterator, +) -> Vec<(String, Span)> { + let authored_length = authored.chars().count(); + let mut suggestions = candidates + .filter_map(|(candidate, span)| { + let distance = levenshtein(authored, candidate); + let threshold = 1.max(authored_length.max(candidate.chars().count()) / 3); + (distance <= threshold).then_some((distance, candidate, span)) + }) + .collect::>(); + suggestions.sort_by(|left, right| { + left.0 + .cmp(&right.0) + .then_with(|| left.1.as_bytes().cmp(right.1.as_bytes())) + }); + suggestions.truncate(3); + suggestions + .into_iter() + .map(|(_, name, span)| (name.to_owned(), span)) + .collect() +} + +fn levenshtein(left: &str, right: &str) -> usize { + let right = right.chars().collect::>(); + let mut previous = (0..=right.len()).collect::>(); + let mut current = vec![0; right.len() + 1]; + + for (left_index, left_character) in left.chars().enumerate() { + current[0] = left_index + 1; + for (right_index, right_character) in right.iter().copied().enumerate() { + let substitution = + previous[right_index] + usize::from(left_character != right_character); + current[right_index + 1] = (current[right_index] + 1) + .min(previous[right_index + 1] + 1) + .min(substitution); + } + std::mem::swap(&mut previous, &mut current); + } + + previous[right.len()] +} + fn is_identifier(value: &str) -> bool { let bytes = value.as_bytes(); (1..=64).contains(&bytes.len()) diff --git a/src/validation/tests.rs b/src/validation/tests.rs index 9964881..582a29b 100644 --- a/src/validation/tests.rs +++ b/src/validation/tests.rs @@ -1,6 +1,8 @@ use crate::diagnostic::Severity; use crate::ir::{Direction, EdgeDirection, EdgeKind, ElementId, NodeKind}; +use super::levenshtein; + fn compile(source: &str) -> crate::CompileOutput { crate::compile(source) } @@ -236,6 +238,79 @@ diagram "Invalid variants" { } } +#[test] +fn validation_reports_closed_set_expectations() { + let output = compile( + r#"stack 1.0 +diagram "Expected values" { + node a "A" { kind process } + node b "B" + layout { direction sideways } + edge a -> b { kind command } +}"#, + ); + + let diagnostics = output + .diagnostics + .iter() + .filter(|diagnostic| diagnostic.code == "STK2002") + .collect::>(); + assert_eq!(diagnostics.len(), 3); + assert_eq!( + diagnostics[0].expected, + [ + "actor", "client", "service", "function", "worker", "database", "cache", "queue", + "storage", "external", + ] + ); + assert_eq!(diagnostics[1].expected, ["right", "down"]); + assert_eq!( + diagnostics[2].expected, + ["flow", "request", "event", "data", "dependency"] + ); + assert!( + diagnostics + .iter() + .all(|diagnostic| diagnostic.help.is_some()) + ); +} + +#[test] +fn validation_suggests_nearby_nodes_deterministically() { + let output = compile( + r#"stack 1.0 +diagram "Suggestions" { + node api "API" + node payment "Payment" + node paymant "Paymant" + node paymont "Paymont" + node database "Database" + edge api -> paymnt +}"#, + ); + + let Some(diagnostic) = output + .diagnostics + .iter() + .find(|diagnostic| diagnostic.code == "STK3003") + else { + return; + }; + assert_eq!(diagnostic.expected, ["paymant", "payment", "paymont"]); + assert_eq!(diagnostic.related.len(), 3); + assert_eq!( + diagnostic.help.as_deref(), + Some("Use a declared node such as paymant, payment, paymont.") + ); +} + +#[test] +fn suggestion_distance_counts_unicode_scalars() { + assert_eq!(levenshtein("café", "cafe"), 1); + assert_eq!(levenshtein("図", "図表"), 1); + assert_eq!(levenshtein("right", "down"), 5); +} + #[test] fn validation_rejects_excessive_group_depth() { let output = compile( diff --git a/tests/conformance.rs b/tests/conformance.rs index 8863ed9..d10a257 100644 --- a/tests/conformance.rs +++ b/tests/conformance.rs @@ -36,7 +36,7 @@ fn valid_cases_match_normalized_ir() -> Result<(), Box> { json!({ "schemaVersion": "1.0", "diagnostics": [] }) }; assert_eq!( - diagnostics_json(&output.diagnostics), + diagnostics_json(&output.diagnostics, &expected_diagnostics), expected_diagnostics, "diagnostic mismatch in {}", case.display() @@ -59,7 +59,7 @@ fn invalid_cases_match_portable_diagnostics() -> Result<(), Box> { case.display() ); assert_eq!( - diagnostics_json(&output.diagnostics), + diagnostics_json(&output.diagnostics, &expected), expected, "diagnostic mismatch in {}", case.display() @@ -94,22 +94,37 @@ fn test_error(message: String) -> Box { Box::new(std::io::Error::other(message)) } -fn diagnostics_json(diagnostics: &[diagnostic::Diagnostic]) -> Value { +fn diagnostics_json(diagnostics: &[diagnostic::Diagnostic], expected: &Value) -> Value { + let expected_diagnostics = expected.get("diagnostics").and_then(Value::as_array); json!({ "schemaVersion": "1.0", "diagnostics": diagnostics .iter() - .map(diagnostic_expectation_json) + .enumerate() + .map(|(index, diagnostic)| { + let compare_expected = expected_diagnostics + .and_then(|items| items.get(index)) + .and_then(|item| item.get("expected")) + .is_some(); + diagnostic_expectation_json(diagnostic, compare_expected) + }) .collect::>(), }) } -fn diagnostic_expectation_json(diagnostic: &diagnostic::Diagnostic) -> Value { - json!({ +fn diagnostic_expectation_json( + diagnostic: &diagnostic::Diagnostic, + compare_expected: bool, +) -> Value { + let mut value = json!({ "code": diagnostic.code, "severity": severity_name(diagnostic.severity), "range": range_json(diagnostic.span), - }) + }); + if compare_expected { + value["expected"] = json!(diagnostic.expected); + } + value } fn severity_name(severity: diagnostic::Severity) -> &'static str {