diff --git a/derive/src/from_pest/field.rs b/derive/src/from_pest/field.rs index f8d9a20..b794424 100644 --- a/derive/src/from_pest/field.rs +++ b/derive/src/from_pest/field.rs @@ -93,12 +93,12 @@ impl ConversionStrategy { } ConversionStrategy::Default(span, default_expr) => { // For default strategy, we try to parse as normal FromPest, - // but if it fails (NoMatch), we use the default value + // but if it fails (NoMatch or NoMatchWithInfo), we use the default value quote_spanned! {span=> { // Try to parse using FromPest first match ::from_pest::FromPest::from_pest(inner) { Ok(value) => value, - Err(::from_pest::ConversionError::NoMatch) => #default_expr, + Err(ref e) if e.is_no_match() => #default_expr, Err(e) => return Err(e), } }} diff --git a/derive/src/from_pest/mod.rs b/derive/src/from_pest/mod.rs index 8b56f08..c3d3009 100644 --- a/derive/src/from_pest/mod.rs +++ b/derive/src/from_pest/mod.rs @@ -151,12 +151,17 @@ fn derive_for_struct( #extraneous Err(::from_pest::ConversionError::Extraneous { current_node: stringify!(#name), + extraneous: format!("{:?}", inner.clone().map(|p| p.as_rule()).collect::>()), })?; } *pest = clone; Ok(this) } else { - Err(::from_pest::ConversionError::NoMatch) + Err(::from_pest::ConversionError::NoMatchWithInfo { + current_node: stringify!(#name), + expected: stringify!(#rule_variant), + actual: format!("{:?}", pair.as_rule()), + }) } }) } @@ -192,6 +197,7 @@ fn derive_for_enum( #extraneous Err(::from_pest::ConversionError::Extraneous { current_node: stringify!(#variant_name), + extraneous: format!("{:?}", inner.clone().map(|p| p.as_rule()).collect::>()), })?; } Ok(this) @@ -210,7 +216,11 @@ fn derive_for_enum( *pest = clone; Ok(this) } else { - Err(::from_pest::ConversionError::NoMatch) + Err(::from_pest::ConversionError::NoMatchWithInfo { + current_node: stringify!(#name), + expected: stringify!(#rule_variant), + actual: format!("{:?}", pair.as_rule()), + }) } }) } diff --git a/examples/csv.rs b/examples/csv.rs index 135c6ef..37e1d76 100644 --- a/examples/csv.rs +++ b/examples/csv.rs @@ -19,7 +19,7 @@ mod ast { use super::csv::Rule; use pest::Span; - fn span_into_str(span: Span) -> &str { + fn span_into_str(span: Span<'_>) -> &str { span.as_str() } diff --git a/src/lib.rs b/src/lib.rs index 65d6eba..e825afc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,14 +18,38 @@ use { }; /// An error that occurs during conversion. -#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)] +#[derive(Clone, Debug, Eq, PartialEq, Hash)] pub enum ConversionError { /// No match occurred: this node is not present here NoMatch, + /// No match occurred with detailed information. + /// `current_node` is the AST node being parsed. + /// `expected` is the rule that was expected. + /// `actual` is the rule that was actually found. + NoMatchWithInfo { + current_node: &'static str, + expected: &'static str, + actual: String, + }, /// Fatal error: this node is present but malformed Malformed(FatalError), - /// Found unexpected tokens at the end - Extraneous { current_node: &'static str }, + /// Found unexpected tokens at the end. + /// `current_node` is the AST node being parsed. + /// `extraneous` is a description of the extraneous tokens found. + Extraneous { + current_node: &'static str, + extraneous: String, + }, +} + +impl ConversionError { + /// Returns true if this error represents a "no match" situation. + pub fn is_no_match(&self) -> bool { + matches!( + self, + ConversionError::NoMatch | ConversionError::NoMatchWithInfo { .. } + ) + } } use std::fmt; @@ -37,9 +61,23 @@ where fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { ConversionError::NoMatch => write!(f, "Rule did not match, failed to convert node"), + ConversionError::NoMatchWithInfo { + current_node, + expected, + actual, + } => write!( + f, + "when converting {current_node}, expected {expected}, but found {actual}" + ), ConversionError::Malformed(fatalerror) => write!(f, "Malformed node: {fatalerror}"), - ConversionError::Extraneous { current_node, .. } => { - write!(f, "when converting {current_node}, found extraneous tokens") + ConversionError::Extraneous { + current_node, + extraneous, + } => { + write!( + f, + "when converting {current_node}, found extraneous tokens: {extraneous}" + ) } } } @@ -54,6 +92,7 @@ where fn source(&self) -> Option<&(dyn error::Error + 'static)> { match self { ConversionError::NoMatch => None, + ConversionError::NoMatchWithInfo { .. } => None, ConversionError::Extraneous { .. } => None, ConversionError::Malformed(ref fatalerror) => Some(fatalerror), } @@ -102,7 +141,7 @@ impl<'pest, Rule: RuleType, T: FromPest<'pest, Rule = Rule>> FromPest<'pest> for type FatalError = T::FatalError; fn from_pest(pest: &mut Pairs<'pest, Rule>) -> Result> { match T::from_pest(pest) { - Err(ConversionError::NoMatch) => Ok(None), + Err(ref e) if e.is_no_match() => Ok(None), result => result.map(Some), } } @@ -117,7 +156,7 @@ impl<'pest, Rule: RuleType, T: FromPest<'pest, Rule = Rule>> FromPest<'pest> for loop { match T::from_pest(pest) { Ok(t) => acc.push(t), - Err(ConversionError::NoMatch) => break, + Err(ref e) if e.is_no_match() => break, Err(error) => return Err(error), } } diff --git a/tests/error_messages.rs b/tests/error_messages.rs new file mode 100644 index 0000000..4ead1ff --- /dev/null +++ b/tests/error_messages.rs @@ -0,0 +1,120 @@ +//! Test for improved error messages. + +#![allow(dead_code)] + +#[macro_use] +extern crate pest_derive; +extern crate from_pest; +#[macro_use] +extern crate pest_ast; +extern crate pest; + +mod grammar { + #[derive(Parser)] + #[grammar_inline = r#" + value = { "number" | "string" } + pair = { name ~ ":" ~ value } + name = { ASCII_ALPHA+ } + WHITESPACE = _{ " " } + "#] + pub struct Parser; +} + +mod ast { + use super::grammar::Rule; + + #[derive(Debug, FromPest)] + #[pest_ast(rule(Rule::name))] + pub struct Name<'pest> { + #[pest_ast(outer())] + pub span: pest::Span<'pest>, + } + + #[derive(Debug, FromPest)] + #[pest_ast(rule(Rule::value))] + pub struct Value<'pest> { + #[pest_ast(outer())] + pub span: pest::Span<'pest>, + } + + #[derive(Debug, FromPest)] + #[pest_ast(rule(Rule::pair))] + pub struct Pair<'pest> { + pub name: Name<'pest>, + pub value: Value<'pest>, + } +} + +#[test] +fn test_no_match_error_message() { + use from_pest::ConversionError; + use from_pest::FromPest; + use pest::Parser; + + // Parse "name: number" as a value rule (wrong rule type) + let source = "name: number"; + let mut pairs = grammar::Parser::parse(grammar::Rule::pair, source).expect("parse success"); + + // Try to parse as Value (should fail with informative error) + let result: Result, _> = ast::Value::from_pest(&mut pairs); + + assert!(result.is_err()); + let error = result.unwrap_err(); + + match &error { + ConversionError::NoMatchWithInfo { + current_node, + expected, + actual, + } => { + assert_eq!(*current_node, "Value"); + assert_eq!(*expected, "value"); + assert!(actual.contains("pair")); // The actual rule is 'pair' + } + _ => panic!("Expected NoMatchWithInfo error, got: {:?}", error), + } + + // Verify the Display implementation + let error_message = error.to_string(); + assert!(error_message.contains("Value")); + assert!(error_message.contains("value")); + assert!(error_message.contains("pair")); +} + +#[test] +fn test_is_no_match_helper() { + use from_pest::ConversionError; + + let no_match: ConversionError<()> = ConversionError::NoMatch; + assert!(no_match.is_no_match()); + + let no_match_with_info: ConversionError<()> = ConversionError::NoMatchWithInfo { + current_node: "Test", + expected: "expected", + actual: "actual".to_string(), + }; + assert!(no_match_with_info.is_no_match()); + + let extraneous: ConversionError<()> = ConversionError::Extraneous { + current_node: "Test", + extraneous: "[extra]".to_string(), + }; + assert!(!extraneous.is_no_match()); +} + +#[test] +fn test_extraneous_error_message() { + use from_pest::ConversionError; + use from_pest::Void; + + let error: ConversionError = ConversionError::Extraneous { + current_node: "TestNode", + extraneous: "[extra_rule1, extra_rule2]".to_string(), + }; + + let message = error.to_string(); + assert!(message.contains("TestNode")); + assert!(message.contains("extra_rule1")); + assert!(message.contains("extra_rule2")); + assert!(message.contains("extraneous tokens")); +}