From 2a93400d78d2f6a2cb1865209dc44af96dc5ec20 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Sun, 24 Mar 2024 10:00:30 +0100 Subject: [PATCH 1/2] improve error messages on parse errors see https://github.com/lovasoa/SQLpage/discussions/275#discussion-6413288 error messages not specifying what the error is make it hard to debug the problem this also renames some grammar rules to make them easier to understand --- src/error.rs | 4 +-- src/grammar.pest | 94 ++++++++++++++++++++++++------------------------ src/grammar.rs | 2 +- src/template.rs | 22 ++++++------ 4 files changed, 62 insertions(+), 60 deletions(-) diff --git a/src/error.rs b/src/error.rs index 5cb0e5ff7..cf33ee00f 100644 --- a/src/error.rs +++ b/src/error.rs @@ -192,8 +192,8 @@ pub enum TemplateErrorReason { MismatchingClosedHelper(String, String), #[error("decorator {0:?} was opened, but {1:?} is closing")] MismatchingClosedDecorator(String, String), - #[error("invalid handlebars syntax.")] - InvalidSyntax, + #[error("invalid handlebars syntax: {0}")] + InvalidSyntax(String), #[error("invalid parameter {0:?}")] InvalidParam(String), #[error("nested subexpression is not supported")] diff --git a/src/grammar.pest b/src/grammar.pest index 06c8f0abb..b4a21ccb7 100644 --- a/src/grammar.pest +++ b/src/grammar.pest @@ -43,66 +43,66 @@ reference = ${ path_inline } name = _{ subexpression | reference } -param = { !(keywords ~ !symbol_char) ~ (literal | reference | subexpression) } -hash = { identifier ~ "=" ~ param } +helper_parameter = { !(keywords ~ !symbol_char) ~ (literal | reference | subexpression) } +hash = { identifier ~ "=" ~ helper_parameter } block_param = { "as" ~ "|" ~ identifier ~ identifier? ~ "|"} -exp_line = _{ identifier ~ (hash|param)* ~ block_param?} -partial_exp_line = _{ ((partial_identifier|name) ~ (hash|param)*) } - -subexpression = { "(" ~ ((identifier ~ (hash|param)+) | reference) ~ ")" } - -pre_whitespace_omitter = { "~" } -pro_whitespace_omitter = { "~" } - -expression = { !(invert_tag|invert_chain_tag) ~ "{{" ~ pre_whitespace_omitter? ~ - ((identifier ~ (hash|param)+) | name ) - ~ pro_whitespace_omitter? ~ "}}" } -html_expression_triple_bracket_legacy = _{ "{{{" ~ pre_whitespace_omitter? ~ - ((identifier ~ (hash|param)+) | name ) ~ - pro_whitespace_omitter? ~ "}}}" } -html_expression_triple_bracket = _{ "{{" ~ pre_whitespace_omitter? ~ "{" ~ - ((identifier ~ (hash|param)+) | name ) ~ - "}" ~ pro_whitespace_omitter? ~ "}}" } - -amp_expression = _{ "{{" ~ pre_whitespace_omitter? ~ "&" ~ name ~ - pro_whitespace_omitter? ~ "}}" } +exp_line = _{ identifier ~ (hash|helper_parameter)* ~ block_param?} +partial_exp_line = _{ ((partial_identifier|name) ~ (hash|helper_parameter)*) } + +subexpression = { "(" ~ ((identifier ~ (hash|helper_parameter)+) | reference) ~ ")" } + +leading_tilde_to_omit_whitespace = { "~" } +trailing_tilde_to_omit_whitespace = { "~" } + +expression = { !(invert_tag|invert_chain_tag) ~ "{{" ~ leading_tilde_to_omit_whitespace? ~ + ((identifier ~ (hash|helper_parameter)+) | name ) + ~ trailing_tilde_to_omit_whitespace? ~ "}}" } +html_expression_triple_bracket_legacy = _{ "{{{" ~ leading_tilde_to_omit_whitespace? ~ + ((identifier ~ (hash|helper_parameter)+) | name ) ~ + trailing_tilde_to_omit_whitespace? ~ "}}}" } +html_expression_triple_bracket = _{ "{{" ~ leading_tilde_to_omit_whitespace? ~ "{" ~ + ((identifier ~ (hash|helper_parameter)+) | name ) ~ + "}" ~ trailing_tilde_to_omit_whitespace? ~ "}}" } + +amp_expression = _{ "{{" ~ leading_tilde_to_omit_whitespace? ~ "&" ~ name ~ + trailing_tilde_to_omit_whitespace? ~ "}}" } html_expression = { (html_expression_triple_bracket_legacy | html_expression_triple_bracket) | amp_expression } -decorator_expression = { "{{" ~ pre_whitespace_omitter? ~ "*" ~ exp_line ~ -pro_whitespace_omitter? ~ "}}" } -partial_expression = { "{{" ~ pre_whitespace_omitter? ~ ">" ~ partial_exp_line - ~ pro_whitespace_omitter? ~ "}}" } +decorator_expression = { "{{" ~ leading_tilde_to_omit_whitespace? ~ "*" ~ exp_line ~ +trailing_tilde_to_omit_whitespace? ~ "}}" } +partial_expression = { "{{" ~ leading_tilde_to_omit_whitespace? ~ ">" ~ partial_exp_line + ~ trailing_tilde_to_omit_whitespace? ~ "}}" } invert_tag_item = { "else"|"^" } -invert_tag = { !escape ~ "{{" ~ pre_whitespace_omitter? ~ invert_tag_item - ~ pro_whitespace_omitter? ~ "}}" } -invert_chain_tag = { !escape ~ "{{" ~ pre_whitespace_omitter? ~ invert_tag_item - ~ exp_line ~ pro_whitespace_omitter? ~ "}}" } -helper_block_start = { "{{" ~ pre_whitespace_omitter? ~ "#" ~ exp_line ~ - pro_whitespace_omitter? ~ "}}" } -helper_block_end = { "{{" ~ pre_whitespace_omitter? ~ "/" ~ identifier ~ - pro_whitespace_omitter? ~ "}}" } +invert_tag = { !escape ~ "{{" ~ leading_tilde_to_omit_whitespace? ~ invert_tag_item + ~ trailing_tilde_to_omit_whitespace? ~ "}}" } +invert_chain_tag = { !escape ~ "{{" ~ leading_tilde_to_omit_whitespace? ~ invert_tag_item + ~ exp_line ~ trailing_tilde_to_omit_whitespace? ~ "}}" } +helper_block_start = { "{{" ~ leading_tilde_to_omit_whitespace? ~ "#" ~ exp_line ~ + trailing_tilde_to_omit_whitespace? ~ "}}" } +helper_block_end = { "{{" ~ leading_tilde_to_omit_whitespace? ~ "/" ~ identifier ~ + trailing_tilde_to_omit_whitespace? ~ "}}" } helper_block = _{ helper_block_start ~ template ~ (invert_chain_tag ~ template)* ~ (invert_tag ~ template)? ~ helper_block_end } -decorator_block_start = { "{{" ~ pre_whitespace_omitter? ~ "#" ~ "*" - ~ exp_line ~ pro_whitespace_omitter? ~ "}}" } -decorator_block_end = { "{{" ~ pre_whitespace_omitter? ~ "/" ~ identifier ~ - pro_whitespace_omitter? ~ "}}" } +decorator_block_start = { "{{" ~ leading_tilde_to_omit_whitespace? ~ "#" ~ "*" + ~ exp_line ~ trailing_tilde_to_omit_whitespace? ~ "}}" } +decorator_block_end = { "{{" ~ leading_tilde_to_omit_whitespace? ~ "/" ~ identifier ~ + trailing_tilde_to_omit_whitespace? ~ "}}" } decorator_block = _{ decorator_block_start ~ template ~ decorator_block_end } -partial_block_start = { "{{" ~ pre_whitespace_omitter? ~ "#" ~ ">" - ~ partial_exp_line ~ pro_whitespace_omitter? ~ "}}" } -partial_block_end = { "{{" ~ pre_whitespace_omitter? ~ "/" ~ partial_identifier ~ - pro_whitespace_omitter? ~ "}}" } +partial_block_start = { "{{" ~ leading_tilde_to_omit_whitespace? ~ "#" ~ ">" + ~ partial_exp_line ~ trailing_tilde_to_omit_whitespace? ~ "}}" } +partial_block_end = { "{{" ~ leading_tilde_to_omit_whitespace? ~ "/" ~ partial_identifier ~ + trailing_tilde_to_omit_whitespace? ~ "}}" } partial_block = _{ partial_block_start ~ template ~ partial_block_end } -raw_block_start = { "{{{{" ~ pre_whitespace_omitter? ~ exp_line ~ - pro_whitespace_omitter? ~ "}}}}" } -raw_block_end = { "{{{{" ~ pre_whitespace_omitter? ~ "/" ~ identifier ~ - pro_whitespace_omitter? ~ "}}}}" } +raw_block_start = { "{{{{" ~ leading_tilde_to_omit_whitespace? ~ exp_line ~ + trailing_tilde_to_omit_whitespace? ~ "}}}}" } +raw_block_end = { "{{{{" ~ leading_tilde_to_omit_whitespace? ~ "/" ~ identifier ~ + trailing_tilde_to_omit_whitespace? ~ "}}}}" } raw_block = _{ raw_block_start ~ raw_block_text ~ raw_block_end } hbs_comment = { "{{!" ~ "--" ~ (!"--}}" ~ ANY)* ~ "--" ~ "}}" } @@ -121,7 +121,7 @@ template = { ( partial_expression | partial_block )* } -parameter = _{ param ~ EOI } +parameter = _{ helper_parameter ~ EOI } handlebars = _{ template ~ EOI } /// json path visitor diff --git a/src/grammar.rs b/src/grammar.rs index 187972c06..9545c8cac 100644 --- a/src/grammar.rs +++ b/src/grammar.rs @@ -103,7 +103,7 @@ mod test { fn test_param() { let s = vec!["hello", "\"json literal\"", "nullable", "truestory"]; for i in s.iter() { - assert_rule!(Rule::param, i); + assert_rule!(Rule::helper_parameter, i); } } diff --git a/src/template.rs b/src/template.rs index a81a43259..a4d4c7065 100644 --- a/src/template.rs +++ b/src/template.rs @@ -368,7 +368,7 @@ impl Template { I: Iterator>, { let mut param = it.next().unwrap(); - if param.as_rule() == Rule::param { + if param.as_rule() == Rule::helper_parameter { param = it.next().unwrap(); } let param_rule = param.as_rule(); @@ -482,7 +482,7 @@ impl Template { let mut omit_pro_ws = false; let mut block_param = None; - if it.peek().unwrap().as_rule() == Rule::pre_whitespace_omitter { + if it.peek().unwrap().as_rule() == Rule::leading_tilde_to_omit_whitespace { omit_pre_ws = true; it.next(); } @@ -507,7 +507,7 @@ impl Template { it.next(); match rule { - Rule::param => { + Rule::helper_parameter => { params.push(Template::parse_param(source, it.by_ref(), end)?); } Rule::hash => { @@ -517,7 +517,7 @@ impl Template { Rule::block_param => { block_param = Some(Template::parse_block_param(source, it.by_ref(), end)); } - Rule::pro_whitespace_omitter => { + Rule::trailing_tilde_to_omit_whitespace => { omit_pro_ws = true; } _ => {} @@ -640,7 +640,7 @@ impl Template { LineColLocation::Pos(line_col) => line_col, LineColLocation::Span(line_col, _) => line_col, }; - TemplateError::of(TemplateErrorReason::InvalidSyntax) + TemplateError::of(TemplateErrorReason::InvalidSyntax(e.variant.message().to_string())) .at(source, line_no, col_no) .in_template(options.name()) })?; @@ -1133,7 +1133,7 @@ mod test { let terr = Template::compile(source).unwrap_err(); - assert!(matches!(terr.reason(), TemplateErrorReason::InvalidSyntax)); + assert!(matches!(terr.reason(), TemplateErrorReason::InvalidSyntax(_))); assert_eq!(terr.pos(), Some((4, 5))); } @@ -1248,10 +1248,12 @@ mod test { let sources = ["{{invalid", "{{{invalid", "{{invalid}", "{{!hello"]; for s in sources.iter() { let result = Template::compile(s.to_owned()); - assert!(matches!( - *result.unwrap_err().reason(), - TemplateErrorReason::InvalidSyntax - )); + let err = result.expect_err("expected a syntax error"); + let syntax_error_msg = match err.reason() { + TemplateErrorReason::InvalidSyntax(s) => s, + _ => panic!("InvalidSyntax expected"), + }; + assert!(syntax_error_msg.contains("expected identifier"), "{}", syntax_error_msg); } } From 231a3f54edd2c0227e1bce879da3e6e99818add6 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Sun, 24 Mar 2024 10:01:24 +0100 Subject: [PATCH 2/2] fmt --- src/template.rs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/template.rs b/src/template.rs index a4d4c7065..065661cd8 100644 --- a/src/template.rs +++ b/src/template.rs @@ -640,9 +640,11 @@ impl Template { LineColLocation::Pos(line_col) => line_col, LineColLocation::Span(line_col, _) => line_col, }; - TemplateError::of(TemplateErrorReason::InvalidSyntax(e.variant.message().to_string())) - .at(source, line_no, col_no) - .in_template(options.name()) + TemplateError::of(TemplateErrorReason::InvalidSyntax( + e.variant.message().to_string(), + )) + .at(source, line_no, col_no) + .in_template(options.name()) })?; // dbg!(parser_queue.clone().flatten()); @@ -1133,7 +1135,10 @@ mod test { let terr = Template::compile(source).unwrap_err(); - assert!(matches!(terr.reason(), TemplateErrorReason::InvalidSyntax(_))); + assert!(matches!( + terr.reason(), + TemplateErrorReason::InvalidSyntax(_) + )); assert_eq!(terr.pos(), Some((4, 5))); } @@ -1253,7 +1258,11 @@ mod test { TemplateErrorReason::InvalidSyntax(s) => s, _ => panic!("InvalidSyntax expected"), }; - assert!(syntax_error_msg.contains("expected identifier"), "{}", syntax_error_msg); + assert!( + syntax_error_msg.contains("expected identifier"), + "{}", + syntax_error_msg + ); } }