From 614a77b649b954b964938521879319d956cb5568 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Tue, 21 May 2024 09:53:26 +0200 Subject: [PATCH 1/3] parser: order inherent impl before trait impls --- askama_parser/src/lib.rs | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/askama_parser/src/lib.rs b/askama_parser/src/lib.rs index ce1393f2..afe9968d 100644 --- a/askama_parser/src/lib.rs +++ b/askama_parser/src/lib.rs @@ -166,6 +166,22 @@ pub(crate) struct ErrorContext<'a> { pub(crate) message: Option>, } +impl<'a> ErrorContext<'a> { + pub(crate) fn from_err(error: nom::Err>) -> nom::Err { + match error { + nom::Err::Incomplete(i) => nom::Err::Incomplete(i), + nom::Err::Failure(Error { input, .. }) => nom::Err::Failure(Self { + input, + message: None, + }), + nom::Err::Error(Error { input, .. }) => nom::Err::Error(Self { + input, + message: None, + }), + } + } +} + impl<'a> nom::error::ParseError<&'a str> for ErrorContext<'a> { fn from_error_kind(input: &'a str, _code: ErrorKind) -> Self { Self { @@ -188,22 +204,6 @@ impl<'a, E: std::fmt::Display> FromExternalError<&'a str, E> for ErrorContext<'a } } -impl<'a> ErrorContext<'a> { - pub(crate) fn from_err(error: nom::Err>) -> nom::Err { - match error { - nom::Err::Incomplete(i) => nom::Err::Incomplete(i), - nom::Err::Failure(Error { input, .. }) => nom::Err::Failure(Self { - input, - message: None, - }), - nom::Err::Error(Error { input, .. }) => nom::Err::Error(Self { - input, - message: None, - }), - } - } -} - fn is_ws(c: char) -> bool { matches!(c, ' ' | '\t' | '\r' | '\n') } From 57138f6922f53909266ba1d56d860dcaf1742c8e Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Tue, 21 May 2024 10:01:19 +0200 Subject: [PATCH 2/3] parser: attach unclosed error creation to ErrorContext --- askama_parser/src/lib.rs | 13 +++++++++++++ askama_parser/src/node.rs | 15 ++++----------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/askama_parser/src/lib.rs b/askama_parser/src/lib.rs index afe9968d..71832bc4 100644 --- a/askama_parser/src/lib.rs +++ b/askama_parser/src/lib.rs @@ -167,6 +167,13 @@ pub(crate) struct ErrorContext<'a> { } impl<'a> ErrorContext<'a> { + fn unclosed(kind: &str, tag: &str, i: &'a str) -> Self { + Self { + input: i, + message: Some(Cow::Owned(format!("unclosed {kind}, missing {tag:?}"))), + } + } + pub(crate) fn from_err(error: nom::Err>) -> nom::Err { match error { nom::Err::Incomplete(i) => nom::Err::Incomplete(i), @@ -204,6 +211,12 @@ impl<'a, E: std::fmt::Display> FromExternalError<&'a str, E> for ErrorContext<'a } } +impl<'a> From> for nom::Err> { + fn from(cx: ErrorContext<'a>) -> Self { + Self::Failure(cx) + } +} + fn is_ws(c: char) -> bool { matches!(c, ' ' | '\t' | '\r' | '\n') } diff --git a/askama_parser/src/node.rs b/askama_parser/src/node.rs index 1552a572..50a2fc7c 100644 --- a/askama_parser/src/node.rs +++ b/askama_parser/src/node.rs @@ -12,7 +12,7 @@ use nom::error_position; use nom::multi::{fold_many0, many0, many1, separated_list0, separated_list1}; use nom::sequence::{delimited, pair, preceded, terminated, tuple}; -use crate::{not_ws, ErrorContext, ParseErr, ParseResult}; +use crate::{not_ws, ErrorContext, ParseResult}; use super::{ bool_lit, char_lit, filter, identifier, is_ws, keyword, num_lit, path_or_identifier, skip_till, @@ -101,7 +101,7 @@ impl<'a> Node<'a> { )))(i)?; match closed { true => Ok((i, node)), - false => Err(fail_unclosed("block", s.syntax.block_end, i)), + false => Err(ErrorContext::unclosed("block", s.syntax.block_end, i).into()), } } @@ -152,7 +152,7 @@ impl<'a> Node<'a> { ))(i)?; match closed { true => Ok((i, Self::Expr(Ws(pws, nws), expr))), - false => Err(fail_unclosed("expression", s.syntax.expr_end, i)), + false => Err(ErrorContext::unclosed("expression", s.syntax.expr_end, i).into()), } } } @@ -1072,7 +1072,7 @@ impl<'a> Comment<'a> { loop { let (_, tag) = opt(skip_till(|i| tag(i, s)))(i)?; let Some((j, tag)) = tag else { - return Err(fail_unclosed("comment", s.syntax.comment_end, i)); + return Err(ErrorContext::unclosed("comment", s.syntax.comment_end, i).into()); }; match tag { Tag::Open => match depth.checked_add(1) { @@ -1123,10 +1123,3 @@ impl<'a> Comment<'a> { /// Second field is "minus/plus sign was used on the right part of the item". #[derive(Clone, Copy, Debug, PartialEq)] pub struct Ws(pub Option, pub Option); - -fn fail_unclosed<'a>(kind: &str, tag: &str, i: &'a str) -> ParseErr<'a> { - nom::Err::Failure(ErrorContext { - input: i, - message: Some(Cow::Owned(format!("unclosed {kind}, missing {tag:?}"))), - }) -} From 7d002e9df5e7b749cb9c4ef0820a1c3d6e686fdb Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Tue, 21 May 2024 10:14:33 +0200 Subject: [PATCH 3/3] 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) {