Repository: djc/askama
Pull request #590 (2022-01-04 20:14:15)

 Implement error propagation expression: `?`
This PR consists of ~three~ ~two~ a change~s~:

* ~Right now almost every expression needs to be parsed twice: `expr_any()` first parses the left-hand side of a range expression, and if no `..` or `..=` was found the left-hand expression is parsed again, this time as the result of the function. This diff removes the second parsing step by first looking for `.. (opt rhs)`, then for `lhs .. (opt rhs)`.~
* ~Instead of having `Expr::VarCall`, `Expr::PathCall` and `Expr::MethodCall`, this diff unifies the handling of calls by removed the former three variants, and introducing `Expr::Call`. This makes parsing postfix operators easier.~
* This change allows using the operator `?` in askama expressions. It works like the same operator in Rust: if a `Result` is `Ok`, it is unwrapped. If it is an error, then the `render()` method fails with this error value.

djc: Having thought about this some more, and I'm not sure I'm comfortable with the addition of the try operator. It seems like a bunch of complexity with a goal that I'm not sure meshes well with the design of Askama -- in my opinion, templates should generally be written not to yield errors. Did you have some concrete use cases where you wanted this behavior? I don't think fully matching the Rust grammar should be a goal unto itself.

(The changes to introduce a unified `Call` AST type seems nice, though.)
Kijewski: Right now the use of serde_json is expected at it's errors are caught in a nice way, but I guess [serde_qs](https://docs.rs/serde_qs/0.8.5/serde_qs/fn.to_string.html) is equally as useful. Well, at least I am using serde_qs to generate URLs.

Right now I am formatting the query string outside or render(), and supply it in a field. But I would like it if I could use serde_qs::to_string() directly in the template without resorting to using unwrap().

Patch:
diff --git a/askama_shared/src/error.rs b/askama_shared/src/error.rs
index 4394eea..98f2703 100644
--- a/askama_shared/src/error.rs
+++ b/askama_shared/src/error.rs
@@ -1,6 +1,6 @@
 use std::fmt::{self, Display};
 
-pub type Result<I> = ::std::result::Result<I, Error>;
+pub type Result<I, E = Error> = ::std::result::Result<I, E>;
 
 /// askama error type
 ///
@@ -28,6 +28,9 @@ pub enum Error {
     /// formatting error
     Fmt(fmt::Error),
 
+    /// an error raised by using `?` in a template
+    Custom(Box<dyn std::error::Error + Send + Sync>),
+
     /// json conversion error
     #[cfg(feature = "serde_json")]
     Json(::serde_json::Error),
@@ -41,6 +44,7 @@ impl std::error::Error for Error {
     fn cause(&self) -> Option<&dyn std::error::Error> {
         match *self {
             Error::Fmt(ref err) => err.source(),
+            Error::Custom(ref err) => Some(err.as_ref()),
             #[cfg(feature = "serde_json")]
             Error::Json(ref err) => err.source(),
             #[cfg(feature = "serde_yaml")]
@@ -51,12 +55,13 @@ impl std::error::Error for Error {
 
 impl Display for Error {
     fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match *self {
-            Error::Fmt(ref err) => write!(formatter, "formatting error: {}", err),
+        match self {
+            Error::Fmt(err) => write!(formatter, "formatting error: {}", err),
+            Error::Custom(err) => write!(formatter, "{}", err),
             #[cfg(feature = "serde_json")]
-            Error::Json(ref err) => write!(formatter, "json conversion error: {}", err),
+            Error::Json(err) => write!(formatter, "json conversion error: {}", err),
             #[cfg(feature = "serde_yaml")]
-            Error::Yaml(ref err) => write!(formatter, "yaml conversion error: {}", err),
+            Error::Yaml(err) => write!(formatter, "yaml conversion error: {}", err),
         }
     }
 }
diff --git a/askama_shared/src/generator.rs b/askama_shared/src/generator.rs
index fabe277..de2fdcb 100644
--- a/askama_shared/src/generator.rs
+++ b/askama_shared/src/generator.rs
@@ -1060,9 +1060,21 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> {
             Expr::Group(ref inner) => self.visit_group(buf, inner)?,
             Expr::Call(ref obj, ref args) => self.visit_call(buf, obj, args)?,
             Expr::RustMacro(name, args) => self.visit_rust_macro(buf, name, args),
+            Expr::Try(ref expr) => self.visit_try(buf, expr.as_ref())?,
         })
     }
 
+    fn visit_try(
+        &mut self,
+        buf: &mut Buffer,
+        expr: &Expr<'_>,
+    ) -> Result<DisplayWrap, CompileError> {
+        buf.write("::core::result::Result::map_err(");
+        self.visit_expr(buf, expr)?;
+        buf.write(", |err| ::askama::shared::Error::Custom(::core::convert::Into::into(err)))?");
+        Ok(DisplayWrap::Unwrapped)
+    }
+
     fn visit_rust_macro(&mut self, buf: &mut Buffer, name: &str, args: &str) -> DisplayWrap {
         buf.write(name);
         buf.write("!(");
diff --git a/askama_shared/src/parser.rs b/askama_shared/src/parser.rs
index f429628..63cea0c 100644
--- a/askama_shared/src/parser.rs
+++ b/askama_shared/src/parser.rs
@@ -63,6 +63,7 @@ pub enum Expr<'a> {
     Group(Box<Expr<'a>>),
     Call(Box<Expr<'a>>, Vec<Expr<'a>>),
     RustMacro(&'a str, &'a str),
+    Try(Box<Expr<'a>>),
 }
 
 impl Expr<'_> {
@@ -510,6 +511,7 @@ enum Suffix<'a> {
     Attr(&'a str),
     Index(Expr<'a>),
     Call(Vec<Expr<'a>>),
+    Try,
 }
 
 fn expr_attr(i: &str) -> IResult<&str, Suffix<'_>> {
@@ -533,6 +535,10 @@ fn expr_call(i: &str) -> IResult<&str, Suffix<'_>> {
     map(arguments, Suffix::Call)(i)
 }
 
+fn expr_try(i: &str) -> IResult<&str, Suffix<'_>> {
+    map(preceded(take_till(not_ws), char('?')), |_| Suffix::Try)(i)
+}
+
 fn filter(i: &str) -> IResult<&str, (&str, Option<Vec<Expr<'_>>>)> {
     let (i, (_, fname, args)) = tuple((char('|'), ws(identifier), opt(arguments)))(i)?;
     Ok((i, (fname, args)))
@@ -567,12 +573,13 @@ fn expr_prefix(i: &str) -> IResult<&str, Expr<'_>> {
 fn expr_suffix(i: &str) -> IResult<&str, Expr<'_>> {
     let (mut i, mut expr) = expr_single(i)?;
     loop {
-        let (j, suffix) = opt(alt((expr_attr, expr_index, expr_call)))(i)?;
+        let (j, suffix) = opt(alt((expr_attr, expr_index, expr_call, expr_try)))(i)?;
         i = j;
         match suffix {
             Some(Suffix::Attr(attr)) => expr = Expr::Attr(expr.into(), attr),
             Some(Suffix::Index(index)) => expr = Expr::Index(expr.into(), index.into()),
             Some(Suffix::Call(args)) => expr = Expr::Call(expr.into(), args),
+            Some(Suffix::Try) => expr = Expr::Try(expr.into()),
             None => break,
         }
     }


Test patch:
diff --git a/testing/tests/try.rs b/testing/tests/try.rs
new file mode 100644
index 0000000..e199f9c
--- /dev/null
+++ b/testing/tests/try.rs
@@ -0,0 +1,69 @@
+use askama::Template;
+
+#[derive(Template)]
+#[template(source = "{% let v = self.parse()? %}{{s}}={{v}}", ext = "txt")]
+struct IntParserTemplate<'a> {
+    s: &'a str,
+}
+
+impl IntParserTemplate<'_> {
+    fn parse(&self) -> Result<i32, std::num::ParseIntError> {
+        self.s.parse()
+    }
+}
+
+#[test]
+fn test_int_parser() {
+    let template = IntParserTemplate { s: "💯" };
+    assert!(matches!(template.render(), Err(askama::Error::Custom(_))));
+    assert_eq!(
+        format!("{}", &template.render().unwrap_err()),
+        "invalid digit found in string"
+    );
+
+    let template = IntParserTemplate { s: "100" };
+    assert_eq!(template.render().unwrap(), "100=100");
+}
+
+#[derive(Template)]
+#[template(source = "{{ value()? }}", ext = "txt")]
+struct FailFmt {
+    value: fn() -> Result<&'static str, std::fmt::Error>,
+}
+
+#[test]
+fn fail_fmt() {
+    let template = FailFmt {
+        value: || Err(std::fmt::Error),
+    };
+    assert!(matches!(template.render(), Err(askama::Error::Custom(_))));
+    assert_eq!(
+        format!("{}", &template.render().unwrap_err()),
+        format!("{}", std::fmt::Error)
+    );
+
+    let template = FailFmt {
+        value: || Ok("hello world"),
+    };
+    assert_eq!(template.render().unwrap(), "hello world");
+}
+
+#[derive(Template)]
+#[template(source = "{{ value()? }}", ext = "txt")]
+struct FailStr {
+    value: fn() -> Result<&'static str, &'static str>,
+}
+
+#[test]
+fn fail_str() {
+    let template = FailStr {
+        value: || Err("FAIL"),
+    };
+    assert!(matches!(template.render(), Err(askama::Error::Custom(_))));
+    assert_eq!(format!("{}", &template.render().unwrap_err()), "FAIL");
+
+    let template = FailStr {
+        value: || Ok("hello world"),
+    };
+    assert_eq!(template.render().unwrap(), "hello world");
+}