From 627d58bc3b93c484f190d0711a4f9a9e26bc062c Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Tue, 21 May 2024 10:14:33 +0200 Subject: [PATCH] parser: simplify ErrorContext construction --- askama_parser/src/expr.rs | 21 +++++------ askama_parser/src/lib.rs | 38 ++++++++++---------- askama_parser/src/node.rs | 76 ++++++++++++++++++--------------------- 3 files changed, 62 insertions(+), 73 deletions(-) diff --git a/askama_parser/src/expr.rs b/askama_parser/src/expr.rs index 702c93f4..16069b2d 100644 --- a/askama_parser/src/expr.rs +++ b/askama_parser/src/expr.rs @@ -1,4 +1,3 @@ -use std::borrow::Cow; use std::collections::HashSet; use std::str; @@ -111,12 +110,10 @@ impl<'a> Expr<'a> { move |i| Self::parse(i, level), ))(i)?; if has_named_arguments && !matches!(expr, Self::NamedArgument(_, _)) { - Err(nom::Err::Failure(ErrorContext { - input: start, - message: Some(Cow::Borrowed( - "named arguments must always be passed last", - )), - })) + Err(nom::Err::Failure(ErrorContext::new( + "named arguments must always be passed last", + start, + ))) } else { Ok((i, expr)) } @@ -146,12 +143,10 @@ impl<'a> Expr<'a> { if named_arguments.insert(argument) { Ok((i, Self::NamedArgument(argument, Box::new(value)))) } else { - Err(nom::Err::Failure(ErrorContext { - input: start, - message: Some(Cow::Owned(format!( - "named argument `{argument}` was passed more than once" - ))), - })) + Err(nom::Err::Failure(ErrorContext::new( + format!("named argument `{argument}` was passed more than once"), + start, + ))) } } diff --git a/askama_parser/src/lib.rs b/askama_parser/src/lib.rs index 71832bc4..666c3fb5 100644 --- a/askama_parser/src/lib.rs +++ b/askama_parser/src/lib.rs @@ -168,9 +168,13 @@ pub(crate) struct ErrorContext<'a> { impl<'a> ErrorContext<'a> { fn unclosed(kind: &str, tag: &str, i: &'a str) -> Self { + Self::new(format!("unclosed {kind}, missing {tag:?}"), i) + } + + fn new(message: impl Into>, input: &'a str) -> Self { Self { - input: i, - message: Some(Cow::Owned(format!("unclosed {kind}, missing {tag:?}"))), + input, + message: Some(message.into()), } } @@ -373,19 +377,20 @@ fn char_lit(i: &str) -> ParseResult<'_> { opt(escaped(is_not("\\\'"), '\\', anychar)), char('\''), )(i)?; + let Some(s) = s else { - return Err(nom::Err::Failure(ErrorContext { - input: start, - // Same error as rustc. - message: Some(Cow::Borrowed("empty character literal")), - })); + return Err(nom::Err::Failure(ErrorContext::new( + "empty character literal", + start, + ))); }; let Ok(("", c)) = Char::parse(s) else { - return Err(nom::Err::Failure(ErrorContext { - input: start, - message: Some(Cow::Borrowed("invalid character")), - })); + return Err(nom::Err::Failure(ErrorContext::new( + "invalid character", + start, + ))); }; + let (nb, max_value, err1, err2) = match c { Char::Literal | Char::Escaped => return Ok((i, s)), Char::AsciiEscape(nb) => ( @@ -405,17 +410,12 @@ fn char_lit(i: &str) -> ParseResult<'_> { }; let Ok(nb) = u32::from_str_radix(nb, 16) else { - return Err(nom::Err::Failure(ErrorContext { - input: start, - message: Some(Cow::Borrowed(err1)), - })); + return Err(nom::Err::Failure(ErrorContext::new(err1, start))); }; if nb > max_value { - return Err(nom::Err::Failure(ErrorContext { - input: start, - message: Some(Cow::Borrowed(err2)), - })); + return Err(nom::Err::Failure(ErrorContext::new(err2, start))); } + Ok((i, s)) } diff --git a/askama_parser/src/node.rs b/askama_parser/src/node.rs index 50a2fc7c..feb13d04 100644 --- a/askama_parser/src/node.rs +++ b/askama_parser/src/node.rs @@ -1,4 +1,3 @@ -use std::borrow::Cow; use std::str; use nom::branch::alt; @@ -113,10 +112,10 @@ impl<'a> Node<'a> { )); let (j, (pws, _, nws)) = p(i)?; if !s.is_in_loop() { - return Err(nom::Err::Failure(ErrorContext { - input: i, - message: Some(Cow::Borrowed("you can only `break` inside a `for` loop")), - })); + return Err(nom::Err::Failure(ErrorContext::new( + "you can only `break` inside a `for` loop", + i, + ))); } Ok((j, Self::Break(Ws(pws, nws)))) } @@ -129,10 +128,10 @@ impl<'a> Node<'a> { )); let (j, (pws, _, nws)) = p(i)?; if !s.is_in_loop() { - return Err(nom::Err::Failure(ErrorContext { - input: i, - message: Some(Cow::Borrowed("you can only `continue` inside a `for` loop")), - })); + return Err(nom::Err::Failure(ErrorContext::new( + "you can only `continue` inside a `for` loop", + i, + ))); } Ok((j, Self::Continue(Ws(pws, nws)))) } @@ -297,10 +296,10 @@ impl<'a> Target<'a> { fn verify_name(input: &'a str, name: &'a str) -> Result>> { match name { - "self" | "writer" => Err(nom::Err::Failure(ErrorContext { + "self" | "writer" => Err(nom::Err::Failure(ErrorContext::new( + format!("cannot use `{name}` as a name"), input, - message: Some(Cow::Owned(format!("Cannot use `{name}` as a name"))), - })), + ))), _ => Ok(Self::Name(name)), } } @@ -375,12 +374,10 @@ impl<'a> Cond<'a> { opt(Whitespace::parse), ws(alt((keyword("else"), |i| { let _ = keyword("elif")(i)?; - Err(nom::Err::Failure(ErrorContext { - input: i, - message: Some(Cow::Borrowed( - "unknown `elif` keyword; did you mean `else if`?", - )), - })) + Err(nom::Err::Failure(ErrorContext::new( + "unknown `elif` keyword; did you mean `else if`?", + i, + ))) }))), cut(tuple(( opt(|i| CondTest::parse(i, s)), @@ -559,10 +556,10 @@ impl<'a> Macro<'a> { )); let (j, (pws1, _, (name, params, nws1, _))) = start(i)?; if name == "super" { - return Err(nom::Err::Failure(ErrorContext { - input: i, - message: Some(Cow::Borrowed("'super' is not a valid name for a macro")), - })); + return Err(nom::Err::Failure(ErrorContext::new( + "'super' is not a valid name for a macro", + i, + ))); } let mut end = cut(tuple(( @@ -831,15 +828,14 @@ fn check_end_name<'a>( if name == end_name { return Ok((after, end_name)); } - let message = if name.is_empty() && !end_name.is_empty() { - format!("unexpected name `{end_name}` in `end{kind}` tag for unnamed `{kind}`") - } else { - format!("expected name `{name}` in `end{kind}` tag, found `{end_name}`") - }; - Err(nom::Err::Failure(ErrorContext { - input: before, - message: Some(Cow::Owned(message)), - })) + + Err(nom::Err::Failure(ErrorContext::new( + match name.is_empty() && !end_name.is_empty() { + true => format!("unexpected name `{end_name}` in `end{kind}` tag for unnamed `{kind}`"), + false => format!("expected name `{name}` in `end{kind}` tag, found `{end_name}`"), + }, + before, + ))) } #[derive(Debug, PartialEq)] @@ -1036,12 +1032,10 @@ impl<'a> Extends<'a> { ))(i)?; match (pws, nws) { (None, None) => Ok((i, Self { path })), - (_, _) => Err(nom::Err::Failure(ErrorContext { - input: start, - message: Some(Cow::Borrowed( - "whitespace control is not allowed on `extends`", - )), - })), + (_, _) => Err(nom::Err::Failure(ErrorContext::new( + "whitespace control is not allowed on `extends`", + start, + ))), } } } @@ -1078,10 +1072,10 @@ impl<'a> Comment<'a> { Tag::Open => match depth.checked_add(1) { Some(new_depth) => depth = new_depth, None => { - return Err(nom::Err::Failure(ErrorContext { - input: i, - message: Some(Cow::Borrowed("too deeply nested comments")), - })) + return Err(nom::Err::Failure(ErrorContext::new( + "too deeply nested comments", + i, + ))); } }, Tag::Close => match depth.checked_sub(1) {