diff --git a/plantuml/parser/puml_cli/BUILD b/plantuml/parser/puml_cli/BUILD index 4a71a499..f8318a6f 100644 --- a/plantuml/parser/puml_cli/BUILD +++ b/plantuml/parser/puml_cli/BUILD @@ -10,7 +10,7 @@ # # SPDX-License-Identifier: Apache-2.0 # ******************************************************************************* -load("@rules_rust//rust:defs.bzl", "rust_binary") +load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_test") rust_binary( name = "puml_cli", @@ -24,6 +24,26 @@ rust_binary( "//plantuml/parser/puml_serializer", "//plantuml/parser/puml_utils", "//tools/metamodel:class_diagram", + "//tools/metamodel:sequence_diagram", + "@crates//:clap", + "@crates//:env_logger", + "@crates//:log", + "@crates//:serde", + ], +) + +rust_test( + name = "puml_cli_test", + srcs = ["src/main.rs"], + crate_root = "src/main.rs", + deps = [ + "//plantuml/parser/puml_lobster", + "//plantuml/parser/puml_parser", + "//plantuml/parser/puml_resolver", + "//plantuml/parser/puml_serializer", + "//plantuml/parser/puml_utils", + "//tools/metamodel:class_diagram", + "//tools/metamodel:sequence_diagram", "@crates//:clap", "@crates//:env_logger", "@crates//:log", diff --git a/plantuml/parser/puml_cli/src/main.rs b/plantuml/parser/puml_cli/src/main.rs index 816b1534..140b310a 100644 --- a/plantuml/parser/puml_cli/src/main.rs +++ b/plantuml/parser/puml_cli/src/main.rs @@ -25,7 +25,9 @@ use puml_lobster::{write_lobster_to_file, LobsterModel}; use puml_parser::{ DiagramParser, Preprocessor, PumlClassParser, PumlComponentParser, PumlSequenceParser, }; -use puml_resolver::{ClassResolver, ComponentResolver, DiagramResolver}; +use puml_resolver::{ + ClassResolver, ComponentResolver, DiagramResolver, SequenceResolver, SequenceTree, +}; use puml_serializer::{ClassSerializer, ComponentSerializer}; use puml_utils::{write_fbs_to_file, write_json_to_file, write_placeholder_file, LogLevel}; @@ -185,6 +187,7 @@ fn main() -> Result<(), Box> { let lobster_model = match &logic_result { ResolvedDiagram::Component(model) => LobsterModel::Component(model), ResolvedDiagram::Class(model) => LobsterModel::Class(model), + ResolvedDiagram::Sequence(_) => LobsterModel::Empty, }; write_lobster_to_file(lobster_model, path, ldir)?; } @@ -217,9 +220,15 @@ fn serialize_resolved_diagram(resolved_content: &ResolvedDiagram, source_file: & } ResolvedDiagram::Class(resolved_content) => { ClassSerializer::serialize(resolved_content, source_file) - } // ResolvedDiagram::Sequence(_) => { // placeholder - // /* sequence serializer */ - // } + } + ResolvedDiagram::Sequence(_) => { + log::warn!( + "Sequence diagram serialization is not yet implemented; \ + no output will be written for '{}'", + source_file + ); + vec![] + } } } @@ -227,7 +236,7 @@ fn serialize_resolved_diagram(resolved_content: &ResolvedDiagram, source_file: & pub enum ResolvedDiagram { Component(HashMap), Class(class_diagram::ClassDiagram), - // Sequence(SequenceLogic), // placeholder + Sequence(SequenceTree), } fn resolve_parsed_diagram( @@ -242,9 +251,9 @@ fn resolve_parsed_diagram( let mut resolver = ClassResolver::new(); puml_resolver(&mut resolver, &parsed_content).map(ResolvedDiagram::Class) } - ParsedDiagram::Sequence(_) => { - /* sequence resolver */ - Err("Sequence diagrams not implemented".into()) + ParsedDiagram::Sequence(parsed_content) => { + let mut resolver = SequenceResolver; + puml_resolver(&mut resolver, &parsed_content).map(ResolvedDiagram::Sequence) } } } @@ -259,7 +268,7 @@ where Resolver::Error: std::error::Error + 'static, { let logic_result = resolver - .visit_document(parsed_content) + .resolve(parsed_content) .map_err(|e| Box::new(e) as Box)?; Ok(logic_result) @@ -416,3 +425,32 @@ fn collect_puml_files( } Ok(()) } + +#[cfg(test)] +mod sequence_pipeline_tests { + use super::*; + + /// Parsing a sequence diagram must succeed end-to-end (parse → resolve). + /// Before the fix this returned Err("Sequence diagrams not implemented"). + #[test] + fn test_sequence_diagram_resolves_without_error() { + let content = "\ +@startuml +participant A +participant B +A -> B : call +B --> A : reply +@enduml"; + + let path = Rc::new(PathBuf::from("test.puml")); + let parsed = parse_puml_file(&path, content, LogLevel::Info, DiagramType::Sequence) + .expect("sequence parse must succeed"); + + let resolved = resolve_parsed_diagram(parsed); + assert!( + resolved.is_ok(), + "sequence diagram must resolve without error; got: {:?}", + resolved.err() + ); + } +} diff --git a/plantuml/parser/puml_parser/src/class_diagram/BUILD b/plantuml/parser/puml_parser/src/class_diagram/BUILD index 51de9283..cc6f6f92 100644 --- a/plantuml/parser/puml_parser/src/class_diagram/BUILD +++ b/plantuml/parser/puml_parser/src/class_diagram/BUILD @@ -39,6 +39,7 @@ rust_library( deps = [ "//plantuml/parser/puml_parser:parser_core", "//plantuml/parser/puml_utils", + "@crates//:log", "@crates//:pest", "@crates//:serde", "@crates//:thiserror", diff --git a/plantuml/parser/puml_parser/src/class_diagram/src/class_parser.rs b/plantuml/parser/puml_parser/src/class_diagram/src/class_parser.rs index 46ca19db..0659dcc3 100644 --- a/plantuml/parser/puml_parser/src/class_diagram/src/class_parser.rs +++ b/plantuml/parser/puml_parser/src/class_diagram/src/class_parser.rs @@ -19,8 +19,9 @@ use crate::class_traits::{TypeDef, WritableName}; use crate::source_map::{ normalize_multiline_member_signatures, remap_syntax_error_to_original_source, NormalizedContent, }; +use log::{debug, trace}; use parser_core::common_parser::{parse_arrow, PlantUmlCommonParser, Rule}; -use parser_core::{pest_to_syntax_error, BaseParseError, DiagramParser}; +use parser_core::{format_parse_tree, pest_to_syntax_error, BaseParseError, DiagramParser}; use pest::Parser; use puml_utils::LogLevel; use std::collections::HashSet; @@ -806,17 +807,50 @@ fn parse_enum_value(pair: pest::iterators::Pair) -> EnumValue { } } -fn visit_top_level(pair: pest::iterators::Pair, visitor: &mut F) -where - F: FnMut(pest::iterators::Pair), -{ +fn flatten_top_level(pair: pest::iterators::Pair) -> Vec> { match pair.as_rule() { Rule::top_level | Rule::together_def => { - for inner in pair.into_inner() { - visit_top_level(inner, visitor); - } + pair.into_inner().flat_map(flatten_top_level).collect() } - _ => visitor(pair), + _ => vec![pair], + } +} + +fn parse_top_level_element( + pair: pest::iterators::Pair, + normalized_content: &NormalizedContent, + ignored_objects: &mut IgnoredObjectRegistry, + relationships: &mut Vec, +) -> Result, ClassError> { + match pair.as_rule() { + Rule::type_def => { + let type_def = parse_type_def(pair, normalized_content)?; + Ok(vec![ClassUmlTopLevel::Types(type_def)]) + } + Rule::unsupported_object_def => { + let ignored = parse_ignored_object_name(pair); + ignored_objects.register(&ignored, &None); + Ok(vec![]) + } + Rule::enum_def => Ok(vec![ClassUmlTopLevel::Enum(parse_enum_def( + pair, + normalized_content, + ))]), + Rule::namespace_def => { + let (namespace, nested_ignored_objects) = parse_namespace(pair, normalized_content)?; + ignored_objects.merge(nested_ignored_objects); + Ok(vec![ClassUmlTopLevel::Namespace(namespace)]) + } + Rule::relationship => { + relationships.push(parse_relationship(pair)); + Ok(vec![]) + } + Rule::package_def => { + let (package, nested_ignored_objects) = parse_package(pair, normalized_content)?; + ignored_objects.merge(nested_ignored_objects); + Ok(vec![ClassUmlTopLevel::Package(package)]) + } + _ => Ok(vec![]), } } @@ -833,28 +867,17 @@ fn parse_namespace( parse_named(inner, &mut namespace.name); } Rule::top_level => { - let mut visit_result = Ok(()); - visit_top_level(inner, &mut |top_level_inner| { - if visit_result.is_err() { - return; - } - - visit_result = match top_level_inner.as_rule() { + for top_level_inner in flatten_top_level(inner) { + match top_level_inner.as_rule() { Rule::type_def => { - let mut type_def = - match parse_type_def(top_level_inner, normalized_content) { - Ok(type_def) => type_def, - Err(error) => return visit_result = Err(error), - }; + let mut type_def = parse_type_def(top_level_inner, normalized_content)?; type_def.set_namespace(namespace.name.internal.clone()); namespace.types.push(type_def); - Ok(()) } Rule::unsupported_object_def => { let ignored = parse_ignored_object_name(top_level_inner); ignored_objects .register(&ignored, &Some(namespace.name.internal.clone())); - Ok(()) } Rule::enum_def => { let mut enum_def = Element::EnumDef(parse_enum_def( @@ -863,22 +886,16 @@ fn parse_namespace( )); enum_def.set_namespace(namespace.name.internal.clone()); namespace.types.push(enum_def); - Ok(()) } Rule::namespace_def => { let (nested_namespace, nested_ignored_objects) = - match parse_namespace(top_level_inner, normalized_content) { - Ok(parsed) => parsed, - Err(error) => return visit_result = Err(error), - }; + parse_namespace(top_level_inner, normalized_content)?; ignored_objects.merge(nested_ignored_objects); namespace.namespaces.push(nested_namespace); - Ok(()) } - _ => Ok(()), - }; - }); - visit_result?; + _ => (), + } + } } _ => (), } @@ -902,53 +919,36 @@ fn parse_package( } Rule::top_level => { - let mut visit_result = Ok(()); - visit_top_level(inner, &mut |t| { - if visit_result.is_err() { - return; - } - - visit_result = match t.as_rule() { + for t in flatten_top_level(inner) { + match t.as_rule() { Rule::type_def => { - let mut r#type = match parse_type_def(t, normalized_content) { - Ok(r#type) => r#type, - Err(error) => return visit_result = Err(error), - }; + let mut r#type = parse_type_def(t, normalized_content)?; r#type.set_package(package.name.internal.clone()); package.types.push(r#type); - Ok(()) } Rule::unsupported_object_def => { let ignored = parse_ignored_object_name(t); ignored_objects .register(&ignored, &Some(package.name.internal.clone())); - Ok(()) } Rule::enum_def => { let mut enum_def = Element::EnumDef(parse_enum_def(t, normalized_content)); enum_def.set_package(package.name.internal.clone()); package.types.push(enum_def); - Ok(()) } Rule::relationship => { relationships.push(parse_relationship(t)); - Ok(()) } Rule::package_def => { let (subpackage, nested_ignored_objects) = - match parse_package(t, normalized_content) { - Ok(parsed) => parsed, - Err(error) => return visit_result = Err(error), - }; + parse_package(t, normalized_content)?; ignored_objects.merge(nested_ignored_objects); package.packages.push(subpackage); - Ok(()) } - _ => Ok(()), - }; - }); - visit_result?; + _ => (), + } + } } _ => {} } @@ -1038,7 +1038,7 @@ impl DiagramParser for PumlClassParser { // Log file content at trace level if matches!(log_level, LogLevel::Trace) { - eprintln!( + trace!( "{}:\n{}\n{}", path.display(), normalized_content.as_str(), @@ -1051,7 +1051,20 @@ impl DiagramParser for PumlClassParser { let mut relationships = Vec::new(); match PlantUmlCommonParser::parse(Rule::class_start, normalized_content.as_str()) { - Ok(mut pairs) => { + Ok(pairs) => { + // Debug-only, excluded to keep coverage focused on parser logic. + #[cfg(not(coverage))] + if matches!(log_level, LogLevel::Debug | LogLevel::Trace) { + let mut tree_output = String::new(); + format_parse_tree(pairs.clone(), 0, &mut tree_output); + debug!( + "\n=== Parse Tree for {} ===\n{}=== End Parse Tree ===", + path.display(), + tree_output + ); + } + + let mut pairs = pairs; let file_pair = pairs.next().unwrap(); let inner = file_pair.into_inner(); @@ -1059,63 +1072,15 @@ impl DiagramParser for PumlClassParser { for pair in inner { match pair.as_rule() { Rule::top_level => { - let mut visit_result = Ok(()); - visit_top_level(pair, &mut |inner_pair| { - if visit_result.is_err() { - return; - } - - visit_result = match inner_pair.as_rule() { - Rule::type_def => { - let type_def = - match parse_type_def(inner_pair, &normalized_content) { - Ok(type_def) => type_def, - Err(error) => return visit_result = Err(error), - }; - uml_file.elements.push(ClassUmlTopLevel::Types(type_def)); - Ok(()) - } - Rule::unsupported_object_def => { - let ignored = parse_ignored_object_name(inner_pair); - ignored_objects.register(&ignored, &None); - Ok(()) - } - Rule::enum_def => { - uml_file.elements.push(ClassUmlTopLevel::Enum( - parse_enum_def(inner_pair, &normalized_content), - )); - Ok(()) - } - Rule::namespace_def => { - let (namespace, nested_ignored_objects) = - match parse_namespace(inner_pair, &normalized_content) { - Ok(parsed) => parsed, - Err(error) => return visit_result = Err(error), - }; - ignored_objects.merge(nested_ignored_objects); - uml_file - .elements - .push(ClassUmlTopLevel::Namespace(namespace)); - Ok(()) - } - Rule::relationship => { - relationships.push(parse_relationship(inner_pair)); - Ok(()) - } - Rule::package_def => { - let (package, nested_ignored_objects) = - match parse_package(inner_pair, &normalized_content) { - Ok(parsed) => parsed, - Err(error) => return visit_result = Err(error), - }; - ignored_objects.merge(nested_ignored_objects); - uml_file.elements.push(ClassUmlTopLevel::Package(package)); - Ok(()) - } - _ => Ok(()), - }; - }); - visit_result?; + for inner_pair in flatten_top_level(pair) { + let mut elements = parse_top_level_element( + inner_pair, + &normalized_content, + &mut ignored_objects, + &mut relationships, + )?; + uml_file.elements.append(&mut elements); + } } Rule::startuml => { let text = pair.as_str(); diff --git a/plantuml/parser/puml_parser/src/component_diagram/BUILD b/plantuml/parser/puml_parser/src/component_diagram/BUILD index fb4f7221..56f1bfac 100644 --- a/plantuml/parser/puml_parser/src/component_diagram/BUILD +++ b/plantuml/parser/puml_parser/src/component_diagram/BUILD @@ -40,6 +40,15 @@ rust_library( "@crates//:log", "@crates//:pest", "@crates//:serde", + "@crates//:thiserror", + ], +) + +rust_test( + name = "component_unit_test", + crate = ":puml_parser_component", + proc_macro_deps = [ + "@crates//:pest_derive", ], ) diff --git a/plantuml/parser/puml_parser/src/component_diagram/src/component_parser.rs b/plantuml/parser/puml_parser/src/component_diagram/src/component_parser.rs index 4e4f82ca..c6fa9fff 100644 --- a/plantuml/parser/puml_parser/src/component_diagram/src/component_parser.rs +++ b/plantuml/parser/puml_parser/src/component_diagram/src/component_parser.rs @@ -13,16 +13,25 @@ use log::debug; use std::path::PathBuf; use std::rc::Rc; +use thiserror::Error; use crate::{ Arrow, CompPumlDocument, Component, ComponentStyle, Port, PortType, Relation, Statement, }; -use parser_core::{pest_to_syntax_error, BaseParseError, DiagramParser}; +use parser_core::{format_parse_tree, pest_to_syntax_error, BaseParseError, DiagramParser}; use puml_utils::LogLevel; use parser_core::common_parser::parse_arrow as common_parse_arrow; use parser_core::common_parser::{PlantUmlCommonParser, Rule}; +#[derive(Debug, Error)] +pub enum ComponentError { + #[error(transparent)] + Base(#[from] BaseParseError), + #[error("invalid component statement: {0}")] + InvalidStatement(String), +} + pub struct PumlComponentParser; // lobster-trace: Tools.ArchitectureModelingSyntax @@ -34,27 +43,9 @@ pub struct PumlComponentParser; // lobster-trace: Tools.ArchitectureModelingComponentHierarchyComponent // lobster-trace: Tools.ArchitectureModelingComponentInteract impl PumlComponentParser { - // Debug-only, excluded to keep coverage focused on parser logic. - #[cfg(not(coverage))] - fn format_parse_tree(pairs: pest::iterators::Pairs, indent: usize, output: &mut String) { - for pair in pairs { - let indent_str = " ".repeat(indent); - - output.push_str(&format!( - "{}Rule::{:?} -> \"{}\"\n", - indent_str, - pair.as_rule(), - pair.as_str() - )); - - // Recursively print inner pairs - Self::format_parse_tree(pair.into_inner(), indent + 1, output); - } - } - fn parse_statement( pair: pest::iterators::Pair, - ) -> Result, Box> { + ) -> Result, ComponentError> { for inner in pair.into_inner() { match inner.as_rule() { Rule::component => { @@ -77,7 +68,7 @@ impl PumlComponentParser { Ok(vec![]) } - fn parse_port(pair: pest::iterators::Pair) -> Result> { + fn parse_port(pair: pest::iterators::Pair) -> Result { let mut port_type = PortType::Port; let mut name = String::new(); let mut alias = None; @@ -110,7 +101,7 @@ impl PumlComponentParser { fn parse_together_block( pair: pest::iterators::Pair, - ) -> Result, Box> { + ) -> Result, ComponentError> { let mut stmts = Vec::new(); for inner in pair.into_inner() { if inner.as_rule() == Rule::component_statement { @@ -120,9 +111,7 @@ impl PumlComponentParser { Ok(stmts) } - fn parse_component( - pair: pest::iterators::Pair, - ) -> Result> { + fn parse_component(pair: pest::iterators::Pair) -> Result { let mut component = Component { component_type: "".to_string(), name: None, @@ -181,9 +170,7 @@ impl PumlComponentParser { Ok(component) } - fn parse_relation( - pair: pest::iterators::Pair, - ) -> Result> { + fn parse_relation(pair: pest::iterators::Pair) -> Result { let mut lhs = String::new(); let mut rhs = String::new(); let mut arrow = Arrow::default(); @@ -223,11 +210,9 @@ impl PumlComponentParser { .map(|p| p.as_str().trim().to_string()) } - fn parse_arrow(pair: pest::iterators::Pair) -> Result> { - let arrow = common_parse_arrow(pair).map_err(|e| { - Box::new(std::io::Error::new(std::io::ErrorKind::InvalidData, e)) - as Box - })?; + fn parse_arrow(pair: pest::iterators::Pair) -> Result { + let arrow = common_parse_arrow(pair) + .map_err(|e| ComponentError::InvalidStatement(format!("invalid arrow: {}", e)))?; Ok(arrow) } @@ -261,7 +246,7 @@ impl PumlComponentParser { fn parse_default_component( pair: pest::iterators::Pair, - ) -> Result<(String, Option), Box> { + ) -> Result<(String, Option), ComponentError> { let mut comp_type = String::new(); let mut name = None; @@ -289,7 +274,7 @@ impl PumlComponentParser { fn parse_bracket_component( pair: pest::iterators::Pair, - ) -> Result, Box> { + ) -> Result, ComponentError> { let mut name: Option = None; for inner in pair.into_inner() { @@ -303,7 +288,7 @@ impl PumlComponentParser { fn parse_component_style( _pair: pest::iterators::Pair, - ) -> Result> { + ) -> Result { // Simplified implementation Ok(ComponentStyle { color: None, @@ -313,15 +298,14 @@ impl PumlComponentParser { fn parse_statement_block( pair: pest::iterators::Pair, - ) -> Result, Box> { + ) -> Result, ComponentError> { let mut statements = Vec::new(); for inner in pair.into_inner() { match inner.as_rule() { Rule::component_statement => { - if let Ok(mut stmts) = Self::parse_statement(inner) { - statements.append(&mut stmts); - } + let mut stmts = Self::parse_statement(inner)?; + statements.append(&mut stmts); } _ => { // Skip empty lines and other rules like braces @@ -335,7 +319,7 @@ impl PumlComponentParser { impl DiagramParser for PumlComponentParser { type Output = CompPumlDocument; - type Error = BaseParseError; + type Error = ComponentError; fn parse_file( &mut self, @@ -353,7 +337,7 @@ impl DiagramParser for PumlComponentParser { if matches!(log_level, LogLevel::Debug | LogLevel::Trace) { let mut tree_output = String::new(); - Self::format_parse_tree(pairs.clone(), 0, &mut tree_output); + format_parse_tree(pairs.clone(), 0, &mut tree_output); debug!( "\n=== Parse Tree for {} ===\n{}=== End Parse Tree ===", @@ -379,9 +363,8 @@ impl DiagramParser for PumlComponentParser { } } Rule::component_statement => { - if let Ok(mut stmts) = Self::parse_statement(inner_pair) { - document.statements.append(&mut stmts); - } + let mut stmts = Self::parse_statement(inner_pair)?; + document.statements.append(&mut stmts); } _ => { // Skip empty lines and other rules like enduml @@ -393,3 +376,66 @@ impl DiagramParser for PumlComponentParser { Ok(document) } } + +#[cfg(test)] +mod error_handling_tests { + use super::*; + use parser_core::DiagramParser; + use puml_utils::LogLevel; + use std::path::PathBuf; + use std::rc::Rc; + + fn path() -> Rc { + Rc::new(PathBuf::from("test.puml")) + } + + /// A valid diagram must still parse successfully – no regression. + #[test] + fn test_valid_document_succeeds() { + let input = "@startuml\ncomponent A\ncomponent B\nA --> B\n@enduml"; + let mut parser = PumlComponentParser; + let result = parser.parse_file(&path(), input, LogLevel::Info); + assert!(result.is_ok()); + let doc = result.unwrap(); + assert_eq!(doc.statements.len(), 3); + } + + /// A relation that references a component which has no name and no alias + /// must not silently yield a document with fewer statements than expected. + #[test] + fn test_statement_count_matches_source() { + // Two explicit components + one relation = 3 statements. + let input = + "@startuml\ncomponent \"Alpha\" as A\ncomponent \"Beta\" as B\nA --> B : link\n@enduml"; + let mut parser = PumlComponentParser; + let doc = parser + .parse_file(&path(), input, LogLevel::Info) + .expect("valid diagram must parse"); + assert_eq!( + doc.statements.len(), + 3, + "all statements must be present; none may be silently dropped" + ); + } +} + +#[cfg(test)] +mod dispatch_style_tests { + use super::*; + use parser_core::DiagramParser; + use puml_utils::LogLevel; + use std::path::PathBuf; + use std::rc::Rc; + + /// Smoke test: the statement count from a two-component, one-relation diagram + /// must be exactly 3 for the component parser. + #[test] + fn test_component_statement_count() { + let input = "@startuml\ncomponent A\ncomponent B\nA --> B\n@enduml"; + let mut parser = PumlComponentParser; + let doc = parser + .parse_file(&Rc::new(PathBuf::from("t.puml")), input, LogLevel::Info) + .expect("valid input must parse"); + assert_eq!(doc.statements.len(), 3); + } +} diff --git a/plantuml/parser/puml_parser/src/component_diagram/src/lib.rs b/plantuml/parser/puml_parser/src/component_diagram/src/lib.rs index e9c311e5..61c6023c 100644 --- a/plantuml/parser/puml_parser/src/component_diagram/src/lib.rs +++ b/plantuml/parser/puml_parser/src/component_diagram/src/lib.rs @@ -17,4 +17,4 @@ mod component_parser; pub use component_ast::{ Arrow, CompPumlDocument, Component, ComponentStyle, Port, PortType, Relation, Statement, }; -pub use component_parser::PumlComponentParser; +pub use component_parser::{ComponentError, PumlComponentParser}; diff --git a/plantuml/parser/puml_parser/src/lib.rs b/plantuml/parser/puml_parser/src/lib.rs index e7e635bd..c8faf42a 100644 --- a/plantuml/parser/puml_parser/src/lib.rs +++ b/plantuml/parser/puml_parser/src/lib.rs @@ -13,10 +13,10 @@ // Re-export commonly used items that don't have name conflicts pub use class_parser::{ClassError, ClassUmlFile, PumlClassParser}; -pub use component_parser::{CompPumlDocument, PumlComponentParser}; +pub use component_parser::{CompPumlDocument, ComponentError, PumlComponentParser}; pub use parser_core::{common_ast, common_parser, Arrow, BaseParseError, DiagramParser}; pub use preprocessor::{ IncludeExpandError, IncludeParseError, PreprocessError, Preprocessor, ProcedureExpandError, ProcedureParseError, }; -pub use sequence_parser::{PumlSequenceParser, SeqPumlDocument}; +pub use sequence_parser::{PumlSequenceParser, SeqPumlDocument, SequenceError}; diff --git a/plantuml/parser/puml_parser/src/parser_core/BUILD b/plantuml/parser/puml_parser/src/parser_core/BUILD index 2c50b398..61f0830a 100644 --- a/plantuml/parser/puml_parser/src/parser_core/BUILD +++ b/plantuml/parser/puml_parser/src/parser_core/BUILD @@ -31,6 +31,7 @@ rust_library( visibility = ["//plantuml/parser:__subpackages__"], deps = [ "//plantuml/parser/puml_utils", + "@crates//:log", "@crates//:pest", "@crates//:serde", "@crates//:thiserror", diff --git a/plantuml/parser/puml_parser/src/parser_core/src/lib.rs b/plantuml/parser/puml_parser/src/parser_core/src/lib.rs index 597fe822..cdf8e683 100644 --- a/plantuml/parser/puml_parser/src/parser_core/src/lib.rs +++ b/plantuml/parser/puml_parser/src/parser_core/src/lib.rs @@ -18,6 +18,28 @@ pub use common_ast::*; pub use common_parser::*; pub use error::{pest_to_syntax_error, BaseParseError}; +/// Recursively format a Pest parse tree into an indented string for diagnostic output. +/// +/// Excluded from coverage instrumentation so that enabling `LogLevel::Debug` in tests +/// does not distort coverage metrics. +#[cfg(not(coverage))] +pub fn format_parse_tree( + pairs: pest::iterators::Pairs, + indent: usize, + output: &mut String, +) { + for pair in pairs { + let indent_str = " ".repeat(indent); + output.push_str(&format!( + "{}Rule::{:?} -> \"{}\"\n", + indent_str, + pair.as_rule(), + pair.as_str() + )); + format_parse_tree(pair.into_inner(), indent + 1, output); + } +} + pub trait DiagramParser { type Output; type Error; diff --git a/plantuml/parser/puml_parser/src/sequence_diagram/BUILD b/plantuml/parser/puml_parser/src/sequence_diagram/BUILD index 43fa1912..e66eb2a3 100644 --- a/plantuml/parser/puml_parser/src/sequence_diagram/BUILD +++ b/plantuml/parser/puml_parser/src/sequence_diagram/BUILD @@ -10,7 +10,7 @@ # # SPDX-License-Identifier: Apache-2.0 # ******************************************************************************* -load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_library") +load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_library", "rust_test") rust_library( name = "puml_parser_sequence", @@ -29,9 +29,19 @@ rust_library( deps = [ "//plantuml/parser/puml_parser:parser_core", "//plantuml/parser/puml_utils", + "@crates//:log", "@crates//:pest", "@crates//:serde", "@crates//:serde_json", + "@crates//:thiserror", + ], +) + +rust_test( + name = "sequence_unit_test", + crate = ":puml_parser_sequence", + proc_macro_deps = [ + "@crates//:pest_derive", ], ) diff --git a/plantuml/parser/puml_parser/src/sequence_diagram/src/lib.rs b/plantuml/parser/puml_parser/src/sequence_diagram/src/lib.rs index d9957c81..6a7fb4d5 100644 --- a/plantuml/parser/puml_parser/src/sequence_diagram/src/lib.rs +++ b/plantuml/parser/puml_parser/src/sequence_diagram/src/lib.rs @@ -20,7 +20,7 @@ pub use syntax_ast::{ Statement, }; -pub use syntax_parser::PumlSequenceParser; +pub use syntax_parser::{PumlSequenceParser, SequenceError}; /// Parse a PlantUML sequence diagram and return the document name and statements /// This is a convenience function for backwards compatibility with tests diff --git a/plantuml/parser/puml_parser/src/sequence_diagram/src/syntax_parser.rs b/plantuml/parser/puml_parser/src/sequence_diagram/src/syntax_parser.rs index cd754ac5..47f99f19 100644 --- a/plantuml/parser/puml_parser/src/sequence_diagram/src/syntax_parser.rs +++ b/plantuml/parser/puml_parser/src/sequence_diagram/src/syntax_parser.rs @@ -10,15 +10,25 @@ // // SPDX-License-Identifier: Apache-2.0 // ******************************************************************************* +use log::{debug, trace}; use parser_core::common_parser::parse_arrow as common_parse_arrow; use parser_core::common_parser::{PlantUmlCommonParser, Rule}; -use parser_core::{pest_to_syntax_error, BaseParseError, DiagramParser}; +use parser_core::{format_parse_tree, pest_to_syntax_error, BaseParseError, DiagramParser}; use puml_utils::LogLevel; use std::path::PathBuf; use std::rc::Rc; +use thiserror::Error; use crate::syntax_ast::*; +#[derive(Debug, Error)] +pub enum SequenceError { + #[error(transparent)] + Base(#[from] BaseParseError), + #[error("invalid sequence statement: {0}")] + InvalidStatement(String), +} + pub struct PumlSequenceParser; // lobster-trace: Tools.ArchitectureModelingSyntax @@ -36,25 +46,33 @@ impl PumlSequenceParser { None } - fn parse_statement(pair: pest::iterators::Pair) -> Option { - let inner = pair.into_inner().next()?; + fn parse_statement(pair: pest::iterators::Pair) -> Result, SequenceError> { + let inner = pair + .into_inner() + .next() + .ok_or_else(|| SequenceError::InvalidStatement("empty statement".to_string()))?; match inner.as_rule() { - Rule::participant_def => Some(Statement::ParticipantDef(Self::parse_participant_def( + Rule::participant_def => Ok(vec![Statement::ParticipantDef( + Self::parse_participant_def(inner)?, + )]), + Rule::message => Ok(vec![Statement::Message(Self::parse_message(inner)?)]), + Rule::group_cmd => Ok(vec![Statement::GroupCmd(Self::parse_group_cmd(inner)?)]), + Rule::destroy_cmd => Ok(vec![Statement::DestroyCmd(Self::parse_destroy_cmd(inner)?)]), + Rule::create_cmd => Ok(vec![Statement::CreateCmd(Self::parse_create_cmd(inner)?)]), + Rule::activate_cmd => Ok(vec![Statement::ActivateCmd(Self::parse_activate_cmd( inner, - )?)), - Rule::message => Some(Statement::Message(Self::parse_message(inner)?)), - Rule::group_cmd => Some(Statement::GroupCmd(Self::parse_group_cmd(inner)?)), - Rule::destroy_cmd => Some(Statement::DestroyCmd(Self::parse_destroy_cmd(inner)?)), - Rule::create_cmd => Some(Statement::CreateCmd(Self::parse_create_cmd(inner)?)), - Rule::activate_cmd => Some(Statement::ActivateCmd(Self::parse_activate_cmd(inner)?)), - Rule::deactivate_cmd => { - Some(Statement::DeactivateCmd(Self::parse_deactivate_cmd(inner)?)) - } - _ => None, + )?)]), + Rule::deactivate_cmd => Ok(vec![Statement::DeactivateCmd(Self::parse_deactivate_cmd( + inner, + )?)]), + // Grammar-valid directives that are intentionally not modeled as statements + _ => Ok(vec![]), } } - fn parse_participant_def(pair: pest::iterators::Pair) -> Option { + fn parse_participant_def( + pair: pest::iterators::Pair, + ) -> Result { let mut participant_type: Option = None; let mut identifier: Option = None; let mut stereotype: Option = None; @@ -71,9 +89,18 @@ impl PumlSequenceParser { let mut parts = inner.into_inner(); let quoted = parts .next() - .map(|p| Self::extract_quoted_string(p.as_str()))?; - let alias_clause = parts.next()?; // alias_clause - let id_pair = alias_clause.into_inner().next()?; + .map(|p| Self::extract_quoted_string(p.as_str())) + .ok_or_else(|| { + SequenceError::InvalidStatement( + "missing quoted participant".to_string(), + ) + })?; + let alias_clause = parts.next().ok_or_else(|| { + SequenceError::InvalidStatement("missing alias clause".to_string()) + })?; + let id_pair = alias_clause.into_inner().next().ok_or_else(|| { + SequenceError::InvalidStatement("missing alias id".to_string()) + })?; let id = match id_pair.as_rule() { Rule::quoted_string => Self::extract_quoted_string(id_pair.as_str()), _ => id_pair.as_str().trim().to_string(), @@ -82,17 +109,39 @@ impl PumlSequenceParser { } Rule::participant_id_as_quoted => { let mut parts = inner.into_inner(); - let id = parts.next()?.as_str().trim().to_string(); - let alias_clause = parts.next()?; // alias_clause - let quoted_pair = alias_clause.into_inner().next()?; + let id = parts + .next() + .ok_or_else(|| { + SequenceError::InvalidStatement("missing participant id".to_string()) + })? + .as_str() + .trim() + .to_string(); + let alias_clause = parts.next().ok_or_else(|| { + SequenceError::InvalidStatement("missing alias clause".to_string()) + })?; + let quoted_pair = alias_clause.into_inner().next().ok_or_else(|| { + SequenceError::InvalidStatement("missing quoted alias".to_string()) + })?; let quoted = Self::extract_quoted_string(quoted_pair.as_str()); identifier = Some(ParticipantIdentifier::IdAsQuoted { id, quoted }); } Rule::participant_id_as_id => { let mut parts = inner.into_inner(); - let id1 = parts.next()?.as_str().trim().to_string(); - let alias_clause = parts.next()?; // alias_clause - let id2_pair = alias_clause.into_inner().next()?; + let id1 = parts + .next() + .ok_or_else(|| { + SequenceError::InvalidStatement("missing participant id1".to_string()) + })? + .as_str() + .trim() + .to_string(); + let alias_clause = parts.next().ok_or_else(|| { + SequenceError::InvalidStatement("missing alias clause".to_string()) + })?; + let id2_pair = alias_clause.into_inner().next().ok_or_else(|| { + SequenceError::InvalidStatement("missing alias id2".to_string()) + })?; let id2 = id2_pair.as_str().trim().to_string(); identifier = Some(ParticipantIdentifier::IdAsId { id1, id2 }); } @@ -114,9 +163,13 @@ impl PumlSequenceParser { } } - Some(ParticipantDef { - participant_type: participant_type?, - identifier: identifier?, + Ok(ParticipantDef { + participant_type: participant_type.ok_or_else(|| { + SequenceError::InvalidStatement("missing participant type".to_string()) + })?, + identifier: identifier.ok_or_else(|| { + SequenceError::InvalidStatement("missing participant identifier".to_string()) + })?, stereotype, }) } @@ -136,7 +189,7 @@ impl PumlSequenceParser { } } - fn parse_message(pair: pest::iterators::Pair) -> Option { + fn parse_message(pair: pest::iterators::Pair) -> Result { let mut left: Option = None; let mut arrow: Option = None; let mut right: Option = None; @@ -155,13 +208,16 @@ impl PumlSequenceParser { } } Rule::sequence_arrow => { - arrow = Self::parse_arrow(inner); + arrow = Some(Self::parse_arrow(inner)?); } Rule::activation_marker => { activation_marker = Some(inner.as_str().to_string()); } Rule::sequence_description => { - description = Some(inner.into_inner().next()?.as_str().trim().to_string()); + description = inner + .into_inner() + .next() + .map(|p| p.as_str().trim().to_string()); } _ => {} } @@ -169,24 +225,25 @@ impl PumlSequenceParser { let content = MessageContent::WithTargets { left: left.unwrap_or_default(), - arrow: arrow?, + arrow: arrow.ok_or_else(|| { + SequenceError::InvalidStatement("missing arrow in message".to_string()) + })?, right: right.unwrap_or_default(), }; - Some(Message { + Ok(Message { content, activation_marker, description, }) } - fn parse_arrow(pair: pest::iterators::Pair) -> Option { - let arrow = common_parse_arrow(pair).ok()?; - - Some(arrow) + fn parse_arrow(pair: pest::iterators::Pair) -> Result { + common_parse_arrow(pair) + .map_err(|e| SequenceError::InvalidStatement(format!("invalid arrow: {}", e))) } - fn parse_group_cmd(pair: pest::iterators::Pair) -> Option { + fn parse_group_cmd(pair: pest::iterators::Pair) -> Result { let mut group_type: Option = None; let mut text: Option = None; @@ -202,8 +259,9 @@ impl PumlSequenceParser { } } - Some(GroupCmd { - group_type: group_type?, + Ok(GroupCmd { + group_type: group_type + .ok_or_else(|| SequenceError::InvalidStatement("missing group type".to_string()))?, text, }) } @@ -226,7 +284,7 @@ impl PumlSequenceParser { } } - fn parse_destroy_cmd(pair: pest::iterators::Pair) -> Option { + fn parse_destroy_cmd(pair: pest::iterators::Pair) -> Result { let mut participant: Option = None; for inner in pair.into_inner() { @@ -235,12 +293,14 @@ impl PumlSequenceParser { } } - Some(DestroyCmd { - participant: participant?, + Ok(DestroyCmd { + participant: participant.ok_or_else(|| { + SequenceError::InvalidStatement("missing participant in destroy".to_string()) + })?, }) } - fn parse_create_cmd(pair: pest::iterators::Pair) -> Option { + fn parse_create_cmd(pair: pest::iterators::Pair) -> Result { let mut participant: Option = None; for inner in pair.into_inner() { @@ -249,12 +309,14 @@ impl PumlSequenceParser { } } - Some(CreateCmd { - participant: participant?, + Ok(CreateCmd { + participant: participant.ok_or_else(|| { + SequenceError::InvalidStatement("missing participant in create".to_string()) + })?, }) } - fn parse_activate_cmd(pair: pest::iterators::Pair) -> Option { + fn parse_activate_cmd(pair: pest::iterators::Pair) -> Result { let mut participant: Option = None; for inner in pair.into_inner() { @@ -263,12 +325,16 @@ impl PumlSequenceParser { } } - Some(ActivateCmd { - participant: participant?, + Ok(ActivateCmd { + participant: participant.ok_or_else(|| { + SequenceError::InvalidStatement("missing participant in activate".to_string()) + })?, }) } - fn parse_deactivate_cmd(pair: pest::iterators::Pair) -> Option { + fn parse_deactivate_cmd( + pair: pest::iterators::Pair, + ) -> Result { let mut participant: Option = None; for inner in pair.into_inner() { @@ -277,7 +343,7 @@ impl PumlSequenceParser { } } - Some(DeactivateCmd { participant }) + Ok(DeactivateCmd { participant }) } // Helper functions @@ -340,7 +406,7 @@ impl PumlSequenceParser { impl DiagramParser for PumlSequenceParser { type Output = SeqPumlDocument; - type Error = BaseParseError; + type Error = SequenceError; fn parse_file( &mut self, @@ -352,12 +418,24 @@ impl DiagramParser for PumlSequenceParser { // Log file content at trace level if matches!(log_level, LogLevel::Trace) { - eprintln!("{}:\n{}\n{}", path.display(), content, "=".repeat(30)); + trace!("{}:\n{}\n{}", path.display(), content, "=".repeat(30)); } let pairs = PlantUmlCommonParser::parse(Rule::sequence_start, content) .map_err(|e| pest_to_syntax_error(e, path.as_ref().clone(), content))?; + // Debug-only, excluded to keep coverage focused on parser logic. + #[cfg(not(coverage))] + if matches!(log_level, LogLevel::Debug | LogLevel::Trace) { + let mut tree_output = String::new(); + format_parse_tree(pairs.clone(), 0, &mut tree_output); + debug!( + "\n=== Parse Tree for {} ===\n{}=== End Parse Tree ===", + path.display(), + tree_output + ); + } + let mut document = SeqPumlDocument { name: None, statements: Vec::new(), @@ -371,9 +449,8 @@ impl DiagramParser for PumlSequenceParser { document.name = Self::parse_startuml(inner_pair); } Rule::sequence_statement => { - if let Some(stmt) = Self::parse_statement(inner_pair) { - document.statements.push(stmt); - } + let mut stmts = Self::parse_statement(inner_pair)?; + document.statements.append(&mut stmts); } Rule::empty_line => { // Skip empty lines @@ -387,3 +464,69 @@ impl DiagramParser for PumlSequenceParser { Ok(document) } } + +#[cfg(test)] +mod error_handling_tests { + use super::*; + use parser_core::DiagramParser; + use puml_utils::LogLevel; + use std::path::PathBuf; + use std::rc::Rc; + + fn path() -> Rc { + Rc::new(PathBuf::from("test.puml")) + } + + /// A diagram with a known-good participant type must not lose the definition. + #[test] + fn test_valid_participant_is_present_in_output() { + let input = "@startuml\nparticipant Alice\nparticipant Bob\nAlice -> Bob : hello\n@enduml"; + let mut parser = PumlSequenceParser; + let doc = parser + .parse_file(&path(), input, LogLevel::Info) + .expect("valid diagram must parse"); + + // 2 participant defs + 1 message = 3 statements + assert_eq!( + doc.statements.len(), + 3, + "all statements must be present; none may be silently dropped" + ); + } + + /// parse_file must return Err (or log a warning) rather than return an + /// empty document when the content is semantically malformed. + #[test] + fn test_empty_document_on_grammar_failure_is_not_silently_ok() { + // Completely invalid PlantUML – the grammar must reject it. + let input = "@startuml\n$$$$invalid$$$$\n@enduml"; + let mut parser = PumlSequenceParser; + let result = parser.parse_file(&path(), input, LogLevel::Info); + // Grammar-level rejection must surface as Err, not Ok(empty doc). + assert!( + result.is_err(), + "invalid syntax must produce an error, not a silently-empty document" + ); + } +} + +#[cfg(test)] +mod dispatch_style_tests { + use super::*; + use parser_core::DiagramParser; + use puml_utils::LogLevel; + use std::path::PathBuf; + use std::rc::Rc; + + /// Smoke test: the statement count from a two-participant, one-message diagram + /// must be exactly 3 for the sequence parser. + #[test] + fn test_sequence_statement_count() { + let input = "@startuml\nparticipant A\nparticipant B\nA -> B : call\n@enduml"; + let mut parser = PumlSequenceParser; + let doc = parser + .parse_file(&Rc::new(PathBuf::from("t.puml")), input, LogLevel::Info) + .expect("valid input must parse"); + assert_eq!(doc.statements.len(), 3); + } +} diff --git a/plantuml/parser/puml_resolver/BUILD b/plantuml/parser/puml_resolver/BUILD index 53adcda8..aebb5197 100644 --- a/plantuml/parser/puml_resolver/BUILD +++ b/plantuml/parser/puml_resolver/BUILD @@ -20,6 +20,7 @@ rust_library( ":class_resolver", ":component_resolver", ":resolver_traits", + ":sequence_resolver", ], ) @@ -35,6 +36,12 @@ alias( visibility = ["//plantuml/parser:__subpackages__"], ) +alias( + name = "sequence_resolver", + actual = "//plantuml/parser/puml_resolver/src/sequence_diagram:puml_resolver_sequence", + visibility = ["//plantuml/parser:__subpackages__"], +) + rust_library( name = "resolver_traits", srcs = [ diff --git a/plantuml/parser/puml_resolver/src/class_diagram/src/class_resolver.rs b/plantuml/parser/puml_resolver/src/class_diagram/src/class_resolver.rs index b5d24331..2a79a5ff 100644 --- a/plantuml/parser/puml_resolver/src/class_diagram/src/class_resolver.rs +++ b/plantuml/parser/puml_resolver/src/class_diagram/src/class_resolver.rs @@ -694,11 +694,12 @@ impl ClassResolver { impl DiagramResolver for ClassResolver { type Document = ClassUmlFile; - type Statement = (); type Output = ClassDiagram; type Error = ClassPumlResolverError; - fn visit_document(&mut self, document: &Self::Document) -> Result { + // DESIGN: single-pass resolver — all logic lives in analyze(); there is no + // per-statement visitor step. + fn resolve(&mut self, document: &Self::Document) -> Result { self.name_map.clear(); self.logic.name = document.name.clone(); @@ -1030,7 +1031,7 @@ mod tests { } // ---------------------------- - // visit_document integration + // resolve integration // ---------------------------- #[test] fn test_visit_document_simple() { @@ -1042,7 +1043,7 @@ mod tests { relationships: vec![], }; - let logic = resolver.visit_document(&file).unwrap(); + let logic = resolver.resolve(&file).unwrap(); assert_eq!(logic.name, "test"); assert_eq!(logic.entities.len(), 1); assert_eq!(logic.entities[0].id, "User"); diff --git a/plantuml/parser/puml_resolver/src/class_diagram/test/class_resolver_test.rs b/plantuml/parser/puml_resolver/src/class_diagram/test/class_resolver_test.rs index e9aaf047..26665958 100644 --- a/plantuml/parser/puml_resolver/src/class_diagram/test/class_resolver_test.rs +++ b/plantuml/parser/puml_resolver/src/class_diagram/test/class_resolver_test.rs @@ -44,7 +44,7 @@ impl DiagramProcessor for ClassResolverRunner { let parsed_ast = parser .parse_file(path, &puml_file, LogLevel::Error) .expect("Class Resolver: Failed to parse test file"); - let logic_ast = resolver.visit_document(&parsed_ast)?; + let logic_ast = resolver.resolve(&parsed_ast)?; results.insert(Rc::clone(path), logic_ast); } diff --git a/plantuml/parser/puml_resolver/src/component_diagram/src/component_resolver.rs b/plantuml/parser/puml_resolver/src/component_diagram/src/component_resolver.rs index 68a96575..96c8d578 100644 --- a/plantuml/parser/puml_resolver/src/component_diagram/src/component_resolver.rs +++ b/plantuml/parser/puml_resolver/src/component_diagram/src/component_resolver.rs @@ -157,11 +157,10 @@ impl ComponentResolver { impl DiagramResolver for ComponentResolver { type Document = CompPumlDocument; - type Statement = Statement; type Output = HashMap; type Error = ComponentResolverError; - fn visit_document(&mut self, document: &CompPumlDocument) -> Result { + fn resolve(&mut self, document: &CompPumlDocument) -> Result { self.scope.clear(); for stmt in &document.statements { @@ -173,8 +172,10 @@ impl DiagramResolver for ComponentResolver { Ok(self.components.clone()) } +} - fn visit_statement(&mut self, statement: &Statement) -> Result<(), Self::Error> { +impl ComponentResolver { + fn visit_statement(&mut self, statement: &Statement) -> Result<(), ComponentResolverError> { match statement { Statement::Component(component) => { self.visit_component(component)?; diff --git a/plantuml/parser/puml_resolver/src/component_diagram/tests/component_resolver_test.rs b/plantuml/parser/puml_resolver/src/component_diagram/tests/component_resolver_test.rs index 4dae9322..4c199e54 100644 --- a/plantuml/parser/puml_resolver/src/component_diagram/tests/component_resolver_test.rs +++ b/plantuml/parser/puml_resolver/src/component_diagram/tests/component_resolver_test.rs @@ -41,7 +41,7 @@ impl DiagramProcessor for ComponentResolverRunner { let parsed_ast = parser .parse_file(path, &puml_file, LogLevel::Error) .expect("Failed to parse test file"); - let logic_ast = resolver.visit_document(&parsed_ast)?; + let logic_ast = resolver.resolve(&parsed_ast)?; results.insert(Rc::clone(path), logic_ast); } diff --git a/plantuml/parser/puml_resolver/src/lib.rs b/plantuml/parser/puml_resolver/src/lib.rs index 59a5f03f..07c4890f 100644 --- a/plantuml/parser/puml_resolver/src/lib.rs +++ b/plantuml/parser/puml_resolver/src/lib.rs @@ -14,3 +14,4 @@ pub use class_resolver::*; pub use component_resolver::*; pub use resolver_traits::DiagramResolver; +pub use sequence_resolver::{SequenceResolver, SequenceResolverError, SequenceTree}; diff --git a/plantuml/parser/puml_resolver/src/resolver_traits.rs b/plantuml/parser/puml_resolver/src/resolver_traits.rs index 77d3749f..fae6ef5a 100644 --- a/plantuml/parser/puml_resolver/src/resolver_traits.rs +++ b/plantuml/parser/puml_resolver/src/resolver_traits.rs @@ -11,16 +11,28 @@ // SPDX-License-Identifier: Apache-2.0 // ******************************************************************************* -/// Resolver trait for PlantUML diagrams +/// Resolver trait for PlantUML diagrams. +/// +/// Implementations convert a parsed document (`Self::Document`) into a logic model +/// (`Self::Output`). Two patterns are used across the three diagram resolvers: +/// +/// * **Per-statement visitor** (`ComponentResolver`): `resolve` iterates the +/// statement list and delegates each entry to a private helper, maintaining a +/// mutable scope stack between calls. This suits diagrams that can be processed +/// one statement at a time with carrying state (e.g. nested component scopes). +/// +/// * **Single-pass analysis** (`ClassResolver`, `SequenceResolver`): `resolve` +/// delegates to a private `analyze()` / `build_tree()` function that processes +/// the whole document at once. Use this pattern when the resolver needs multiple +/// passes or sees the whole statement list before making decisions (e.g. the class +/// resolver registers all type names before resolving member types). +/// +/// In both cases only `resolve` is part of the public trait contract. Per-statement +/// helpers are ordinary private methods; callers always go through `resolve`. pub trait DiagramResolver { type Document; - type Statement; type Output; type Error; - fn visit_document(&mut self, document: &Self::Document) -> Result; - - fn visit_statement(&mut self, _statement: &Self::Statement) -> Result<(), Self::Error> { - Ok(()) - } + fn resolve(&mut self, document: &Self::Document) -> Result; } diff --git a/plantuml/parser/puml_resolver/src/sequence_diagram/BUILD b/plantuml/parser/puml_resolver/src/sequence_diagram/BUILD index e5ceb348..1e84afc6 100644 --- a/plantuml/parser/puml_resolver/src/sequence_diagram/BUILD +++ b/plantuml/parser/puml_resolver/src/sequence_diagram/BUILD @@ -17,11 +17,13 @@ rust_library( srcs = [ "src/lib.rs", "src/logic_parser.rs", + "src/sequence_resolver.rs", ], crate_name = "sequence_resolver", crate_root = "src/lib.rs", visibility = ["//plantuml/parser:__subpackages__"], deps = [ + "//plantuml/parser/puml_parser:parser_core", "//plantuml/parser/puml_parser:sequence_diagram", "//plantuml/parser/puml_resolver:resolver_traits", "//tools/metamodel:sequence_diagram", @@ -30,6 +32,11 @@ rust_library( ], ) +rust_test( + name = "puml_resolver_sequence_unit_test", + crate = ":puml_resolver_sequence", +) + # Job 2: Build hierarchical tree from syntax JSON rust_binary( name = "logic_parse", diff --git a/plantuml/parser/puml_resolver/src/sequence_diagram/src/lib.rs b/plantuml/parser/puml_resolver/src/sequence_diagram/src/lib.rs index f657c2a1..ad32df0d 100644 --- a/plantuml/parser/puml_resolver/src/sequence_diagram/src/lib.rs +++ b/plantuml/parser/puml_resolver/src/sequence_diagram/src/lib.rs @@ -12,7 +12,9 @@ // ******************************************************************************* pub mod logic_parser; +pub mod sequence_resolver; pub use sequence_logic::{ Condition, ConditionType, Event, Interaction, Return, SequenceNode, SequenceTree, }; +pub use sequence_resolver::{SequenceResolver, SequenceResolverError}; diff --git a/plantuml/parser/puml_resolver/src/sequence_diagram/src/logic_parser.rs b/plantuml/parser/puml_resolver/src/sequence_diagram/src/logic_parser.rs index 61ccb08e..d4705ebf 100644 --- a/plantuml/parser/puml_resolver/src/sequence_diagram/src/logic_parser.rs +++ b/plantuml/parser/puml_resolver/src/sequence_diagram/src/logic_parser.rs @@ -288,3 +288,44 @@ fn is_return_arrow_from_arrow(arrow: &Arrow) -> bool { // Return arrows are typically dashed: "-->" arrow.line.raw.contains("--") } + +#[cfg(test)] +mod return_arrow_detection_tests { + use super::*; + use parser_core::common_ast::{Arrow, ArrowDecor, ArrowLine}; + + fn arrow(line: &str, right: Option<&str>) -> Arrow { + Arrow { + left: None, + line: ArrowLine { + raw: line.to_string(), + }, + middle: None, + right: right.map(|r| ArrowDecor { raw: r.to_string() }), + } + } + + /// "->" is a solid call arrow and must NOT be classified as a return. + #[test] + fn test_solid_call_arrow_is_not_return() { + assert!(!is_return_arrow_from_arrow(&arrow("-", Some(">")))); + } + + /// "-->" is a dashed return arrow and MUST be classified as a return. + #[test] + fn test_dashed_return_arrow_is_return() { + assert!(is_return_arrow_from_arrow(&arrow("--", Some(">")))); + } + + /// "->>" (solid with double-headed arrow) must NOT be classified as a return. + #[test] + fn test_solid_double_headed_arrow_is_not_return() { + assert!(!is_return_arrow_from_arrow(&arrow("-", Some(">>")))); + } + + /// "-->>" (dashed with double-headed arrow) MUST be classified as a return. + #[test] + fn test_dashed_double_headed_arrow_is_return() { + assert!(is_return_arrow_from_arrow(&arrow("--", Some(">>")))); + } +} diff --git a/plantuml/parser/puml_resolver/src/sequence_diagram/src/sequence_resolver.rs b/plantuml/parser/puml_resolver/src/sequence_diagram/src/sequence_resolver.rs new file mode 100644 index 00000000..e051f819 --- /dev/null +++ b/plantuml/parser/puml_resolver/src/sequence_diagram/src/sequence_resolver.rs @@ -0,0 +1,172 @@ +// ******************************************************************************* +// Copyright (c) 2026 Contributors to the Eclipse Foundation +// +// See the NOTICE file(s) distributed with this work for additional +// information regarding copyright ownership. +// +// This program and the accompanying materials are made available under the +// terms of the Apache License Version 2.0 which is available at +// +// +// SPDX-License-Identifier: Apache-2.0 +// ******************************************************************************* + +use crate::logic_parser::build_tree; +use resolver_traits::DiagramResolver; +use sequence_logic::SequenceTree; +use sequence_parser::SeqPumlDocument; + +/// Resolver for sequence diagrams. +/// +/// Uses the single-pass pattern: `resolve` delegates entirely to `build_tree`, +/// which converts the flat statement list into a `SequenceTree`. The resolver +/// carries no mutable state, so calling `resolve` multiple times is safe. +pub struct SequenceResolver; + +/// Error type for `SequenceResolver`. +/// +/// `build_tree` is currently infallible, so this enum has no variants. +/// It satisfies the `std::error::Error` bound required by the CLI's generic +/// `puml_resolver` helper. +#[derive(Debug)] +pub enum SequenceResolverError {} + +impl std::fmt::Display for SequenceResolverError { + fn fmt(&self, _f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match *self {} + } +} + +impl std::error::Error for SequenceResolverError {} + +impl DiagramResolver for SequenceResolver { + type Document = SeqPumlDocument; + type Output = SequenceTree; + type Error = SequenceResolverError; + + fn resolve(&mut self, document: &SeqPumlDocument) -> Result { + let root_interactions = build_tree(&document.statements); + Ok(SequenceTree { + name: document.name.clone(), + root_interactions, + }) + } +} + +#[cfg(test)] +mod sequence_resolver_tests { + use super::*; + use parser_core::common_ast::{Arrow, ArrowDecor, ArrowLine}; + use resolver_traits::DiagramResolver; + use sequence_parser::syntax_ast::{Message, MessageContent, Statement}; + + fn solid_arrow() -> Arrow { + Arrow { + left: None, + line: ArrowLine { + raw: "-".to_string(), + }, + middle: None, + right: Some(ArrowDecor { + raw: ">".to_string(), + }), + } + } + + fn dashed_arrow() -> Arrow { + Arrow { + left: None, + line: ArrowLine { + raw: "--".to_string(), + }, + middle: None, + right: Some(ArrowDecor { + raw: ">".to_string(), + }), + } + } + + fn make_call(from: &str, to: &str, label: &str) -> Statement { + Statement::Message(Message { + content: MessageContent::WithTargets { + left: from.to_string(), + arrow: solid_arrow(), + right: to.to_string(), + }, + activation_marker: None, + description: Some(label.to_string()), + }) + } + + fn make_return(from: &str, to: &str, label: &str) -> Statement { + Statement::Message(Message { + content: MessageContent::WithTargets { + left: from.to_string(), + arrow: dashed_arrow(), + right: to.to_string(), + }, + activation_marker: None, + description: Some(label.to_string()), + }) + } + + /// SequenceResolver must implement DiagramResolver — compile-time check. + #[test] + fn test_implements_diagram_resolver_trait() { + fn assert_is_diagram_resolver() {} + assert_is_diagram_resolver::(); + } + + /// An empty diagram produces an empty SequenceTree. + #[test] + fn test_empty_document_yields_empty_tree() { + let mut resolver = SequenceResolver; + let doc = SeqPumlDocument { + name: Some("empty".to_string()), + statements: vec![], + }; + let tree = resolver.resolve(&doc).expect("must not fail"); + assert!(tree.root_interactions.is_empty()); + assert_eq!(tree.name.as_deref(), Some("empty")); + } + + /// A single call with its matching return produces one Interaction node. + #[test] + fn test_call_and_return_produce_one_interaction_node() { + let stmts = vec![ + make_call("A", "B", "doWork"), + make_return("B", "A", "result"), + ]; + let mut resolver = SequenceResolver; + let doc = SeqPumlDocument { + name: Some("test".to_string()), + statements: stmts, + }; + let tree = resolver.resolve(&doc).expect("must not fail"); + assert_eq!( + tree.root_interactions.len(), + 1, + "one call + matching return = one Interaction node at root level" + ); + } + + /// resolve must be callable multiple times without carrying state from a previous call. + #[test] + fn test_resolver_is_stateless_across_calls() { + let stmts = vec![make_call("A", "B", "ping")]; + let doc1 = SeqPumlDocument { + name: Some("first".to_string()), + statements: stmts.clone(), + }; + let doc2 = SeqPumlDocument { + name: Some("second".to_string()), + statements: stmts, + }; + + let mut resolver = SequenceResolver; + let tree1 = resolver.resolve(&doc1).unwrap(); + let tree2 = resolver.resolve(&doc2).unwrap(); + + assert_eq!(tree1.root_interactions.len(), tree2.root_interactions.len()); + } +}