Multiple Graphs with syntax error does not actually error #9

lcnbr commented 2025年07月28日 15:14:51 +02:00

When parsing multiple graphs (Thanks again!), the parser does not error on parsing errors if the first graph has no errors:

#[test]fn multiple(){assert_eq!(dot_parser::ast::Graphs::try_from("graph { A -> subgraph { B C } }\ngraph { A -> }").unwrap().graphs.len(),1);assert!(dot_parser::ast::Graph::try_from("graph { A -> }").is_err());}

this test passes when the first assert should not even get hit (it should raise an error) not just ignore the graph.

When parsing multiple graphs (Thanks again!), the parser does not error on parsing errors if the first graph has no errors: ```rust #[test] fn multiple() { assert_eq!( dot_parser::ast::Graphs::try_from("graph { A -> subgraph { B C } }\ngraph { A -> }") .unwrap() .graphs .len(), 1 ); assert!(dot_parser::ast::Graph::try_from("graph { A -> }").is_err()); } ``` this test passes when the first assert should not even get hit (it should raise an error) not just ignore the graph.

bromind commented 2025年07月30日 12:02:01 +02:00

I'll have a look at that. The fix does not seem to difficult, but that will probably change the error type of TryFrom. I expect the error type will look like a 2-tuple with an index (of the first error) and the associated parser error.

I'll have a look at that. The fix does not seem to difficult, but that will probably change the error type of TryFrom. I expect the error type will look like a 2-tuple with an index (of the first error) and the associated parser error.

lcnbr commented 2025年08月08日 12:47:38 +02:00

Ah nice thanks! Changing the error type is fine for me!

Ah nice thanks! Changing the error type is fine for me!

bromind commented 2025年08月11日 11:52:26 +02:00

What I imagined above turned out not to work: the way the parser worked was matching as many graph strings as possible (and at least one), until a non-matching string was found. Thus, the absence of error for the second graph (it wouldn't match the spec) while the first graph would generate an error (since I forced parsing at least one graph).

Instead of the solution above, I forced the parser to match the whole provided string, instead of parsing as much as correct. It seem to work on the example you provided, saying than an identifier or subgraph is missing when matching the last closing curly bracket. The good news is that it doesn't change the signature of TryFrom, so it should work out of the box. I'll push an updated version on crates.io shortly.

What I imagined above turned out not to work: the way the parser worked was matching as many graph strings as possible (and at least one), until a non-matching string was found. Thus, the absence of error for the second graph (it wouldn't match the spec) while the first graph would generate an error (since I forced parsing at least one graph). Instead of the solution above, I forced the parser to match the whole provided string, instead of parsing as much as correct. It seem to work on the example you provided, saying than an identifier or subgraph is missing when matching the last closing curly bracket. The good news is that it doesn't change the signature of TryFrom, so it should work out of the box. I'll push an updated version on crates.io shortly.

bromind commented 2025年08月19日 09:56:36 +02:00

It's now online, in v0.6.0. I'm closing the ticket, but don't hesitate to reopen if need be :-)

It's now online, in v0.6.0. I'm closing the ticket, but don't hesitate to reopen if need be :-)