Reimplement {% filter %} block

This PR reimplements the code generation for `{% filter %}` blocks, so
that the data is written directly into its destination writer, without
using a buffer. This way it behaves like a specialized
`{{ expr|filter1|filter2 }}` would, if the `expr` was a `Template` that
contained the body of the filter block.

rinja/src/helpers.rs

@@ -1,5 +1,7 @@
 #![doc(hidden)]
 
+use std::cell::Cell;
+use std::fmt;
 use std::iter::{Enumerate, Peekable};
 
 pub struct TemplateLoop<I>
@@ -48,3 +50,42 @@ pub struct LoopItem {
     pub first: bool,
     pub last: bool,
 }
+
+pub struct FmtCell<F> {
+    func: Cell<Option<F>>,
+    err: Cell<Option<crate::Error>>,
+}
+
+impl<F> FmtCell<F>
+where
+    F: for<'a, 'b> FnOnce(&'a mut fmt::Formatter<'b>) -> crate::Result<()>,
+{
+    #[inline]
+    pub fn new(f: F) -> Self {
+        Self {
+            func: Cell::new(Some(f)),
+            err: Cell::new(None),
+        }
+    }
+
+    #[inline]
+    pub fn take_err(&self) -> crate::Result<()> {
+        Err(self.err.take().unwrap_or(crate::Error::Fmt))
+    }
+}
+
+impl<F> fmt::Display for FmtCell<F>
+where
+    F: for<'a, 'b> FnOnce(&'a mut fmt::Formatter<'b>) -> crate::Result<()>,
+{
+    #[inline]
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        if let Some(func) = self.func.take() {
+            if let Err(err) = func(f) {
+                self.err.set(Some(err));
+                return Err(fmt::Error);
+            }
+        }
+        Ok(())
+    }
+}

rinja_derive/src/generator.rs

@@ -730,68 +730,48 @@ impl<'a> Generator<'a> {
         self.write_buf_writable(ctx, buf)?;
         self.flush_ws(filter.ws1);
         self.is_in_filter_block += 1;
-        let mut var_name = String::new();
-        for id in 0.. {
-            var_name = format!("__filter_block{id}");
-            if self.locals.get(&Cow::Borrowed(&var_name)).is_none() {
-                // No variable with this name exists, we're in the clear!
-                break;
-            }
-        }
-        buf.write(format_args!(
-            "let mut {var_name} = String::new(); {{ let writer = &mut {var_name};"
-        ));
-        let current_buf = mem::take(&mut self.buf_writable.buf);
+        self.write_buf_writable(ctx, buf)?;
+        buf.writeln("{");
 
+        // build `FmtCell` that contains the inner block
+        buf.writeln(format_args!(
+            "let {FILTER_SOURCE} = {CRATE}::helpers::FmtCell::new(\
+                |writer: &mut ::core::fmt::Formatter<'_>| -> {CRATE}::Result<()> {{"
+        ));
+        self.locals.push();
         self.prepare_ws(filter.ws1);
-        let mut size_hint = self.handle(ctx, &filter.nodes, buf, AstLevel::Nested)?;
+        let size_hint = self.handle(ctx, &filter.nodes, buf, AstLevel::Nested)?;
         self.flush_ws(filter.ws2);
+        self.write_buf_writable(ctx, buf)?;
+        self.locals.pop();
+        buf.writeln(format_args!("{CRATE}::Result::Ok(())"));
+        buf.writeln("});");
 
-        let WriteParts {
-            size_hint: write_size_hint,
-            buffers,
-        } = self.prepare_format(ctx)?;
-        self.buf_writable.buf = current_buf;
-        size_hint += write_size_hint;
-        match buffers {
-            None => {}
-            Some(WritePartsBuffers { format, expr: None }) => {
-                buf.writeln(format_args!("writer.write_str({:#?})?;", &format.buf));
-            }
-            Some(WritePartsBuffers {
-                format,
-                expr: Some(expr),
-            }) => {
-                buf.writeln(format_args!(
-                    "::std::write!(writer, {:#?}, {})?;",
-                    &format.buf,
-                    expr.buf.trim()
-                ));
-            }
-        };
-
+        // display the `FmtCell`
         let mut filter_buf = Buffer::new();
-        let Filter {
-            name: filter_name,
-            arguments,
-        } = &filter.filters;
-        let mut arguments = arguments.clone();
-
-        insert_first_filter_argument(&mut arguments, var_name.clone());
-
-        let wrap = self.visit_filter(ctx, &mut filter_buf, filter_name, &arguments, filter)?;
-
-        self.buf_writable
-            .push(Writable::Generated(filter_buf.buf, wrap));
-        self.prepare_ws(filter.ws2);
-
-        // We don't forget to add the created variable into the list of variables in the scope.
-        self.locals
-            .insert(Cow::Owned(var_name), LocalMeta::initialized());
+        let display_wrap = self.visit_filter(
+            ctx,
+            &mut filter_buf,
+            filter.filters.name,
+            &filter.filters.arguments,
+            filter,
+        )?;
+        let filter_buf = match display_wrap {
+            DisplayWrap::Wrapped => filter_buf.buf,
+            DisplayWrap::Unwrapped => format!(
+                "(&&{CRATE}::filters::AutoEscaper::new(&({}), {})).rinja_auto_escape()?",
+                filter_buf.buf, self.input.escaper,
+            ),
+        };
+        buf.writeln(format_args!(
+            "if ::core::write!(writer, \"{{}}\", {filter_buf}).is_err() {{\n\
+                return {FILTER_SOURCE}.take_err();\n\
+            }}"
+        ));
 
         buf.writeln("}");
         self.is_in_filter_block -= 1;
-
+        self.prepare_ws(filter.ws2);
         Ok(size_hint)
     }
 
@@ -1153,16 +1133,6 @@ impl<'a> Generator<'a> {
                         &mut expr_cache,
                     );
                 }
-                Writable::Generated(s, wrapped) => {
-                    size_hint += self.named_expression(
-                        &mut buf_expr,
-                        &mut buf_format,
-                        s,
-                        wrapped,
-                        false,
-                        &mut expr_cache,
-                    );
-                }
             }
         }
         Ok(WriteParts {
@@ -1288,7 +1258,7 @@ impl<'a> Generator<'a> {
             Expr::Try(ref expr) => self.visit_try(ctx, buf, expr)?,
             Expr::Tuple(ref exprs) => self.visit_tuple(ctx, buf, exprs)?,
             Expr::NamedArgument(_, ref expr) => self.visit_named_argument(ctx, buf, expr)?,
-            Expr::Generated(ref s) => self.visit_generated(buf, s),
+            Expr::FilterSource => self.visit_filter_source(buf),
         })
     }
 
@@ -1788,8 +1758,8 @@ impl<'a> Generator<'a> {
         DisplayWrap::Unwrapped
     }
 
-    fn visit_generated(&mut self, buf: &mut Buffer, s: &str) -> DisplayWrap {
-        buf.write(s);
+    fn visit_filter_source(&mut self, buf: &mut Buffer) -> DisplayWrap {
+        buf.write(FILTER_SOURCE);
         DisplayWrap::Unwrapped
     }
 
@@ -2203,10 +2173,13 @@ pub(crate) fn is_cacheable(expr: &WithSpan<'_, Expr<'_>>) -> bool {
         Expr::Call(_, _) => false,
         Expr::RustMacro(_, _) => false,
         Expr::Try(_) => false,
-        Expr::Generated(_) => true,
+        // Should never be encountered:
+        Expr::FilterSource => unreachable!("FilterSource in expression?"),
     }
 }
 
+const FILTER_SOURCE: &str = "__rinja_filter_block";
+
 fn median(sizes: &mut [usize]) -> usize {
     sizes.sort_unstable();
     if sizes.len() % 2 == 1 {
@@ -2216,51 +2189,6 @@ fn median(sizes: &mut [usize]) -> usize {
     }
 }
 
-/// In `FilterBlock`, we have a recursive `Expr::Filter` entry, where the more you go "down",
-/// the sooner you are called in the Rust code. Example:
-///
-/// ```text
-/// {% filter a|b|c %}bla{% endfilter %}
-/// ```
-///
-/// Will be translated as:
-///
-/// ```text
-/// FilterBlock {
-///    filters: Filter {
-///        name: "c",
-///        arguments: vec![
-///            Filter {
-///                name: "b",
-///                arguments: vec![
-///                    Filter {
-///                        name: "a",
-///                        arguments: vec![],
-///                    }.
-///                ],
-///            }
-///        ],
-///    },
-///    // ...
-/// }
-/// ```
-///
-/// So in here, we want to insert the variable containing the content of the filter block inside
-/// the call to `"a"`. To do so, we recursively go through all `Filter` and finally insert our
-/// variable as the first argument to the `"a"` call.
-fn insert_first_filter_argument(args: &mut Vec<WithSpan<'_, Expr<'_>>>, var_name: String) {
-    if let Some(expr) = args.first_mut() {
-        if let Expr::Filter(Filter {
-            ref mut arguments, ..
-        }) = **expr
-        {
-            insert_first_filter_argument(arguments, var_name);
-            return;
-        }
-    }
-    args.insert(0, WithSpan::new(Expr::Generated(var_name), ""));
-}
-
 #[derive(Clone, Copy, PartialEq)]
 enum AstLevel {
     Top,
@@ -2300,7 +2228,6 @@ impl<'a> Deref for WritableBuffer<'a> {
 enum Writable<'a> {
     Lit(&'a str),
     Expr(&'a WithSpan<'a, Expr<'a>>),
-    Generated(String, DisplayWrap),
 }
 
 struct WriteParts {

rinja_parser/src/expr.rs

@@ -63,7 +63,7 @@ pub enum Expr<'a> {
     RustMacro(Vec<&'a str>, &'a str),
     Try(Box<WithSpan<'a, Expr<'a>>>),
     /// This variant should never be used directly. It is created when generating filter blocks.
-    Generated(String),
+    FilterSource,
 }
 
 impl<'a> Expr<'a> {

rinja_parser/src/node.rs

@@ -502,9 +502,11 @@ impl<'a> FilterBlock<'a> {
         ));
         let (i, (pws1, _, (filter_name, params, extra_filters, _, nws1, _))) = start(i)?;
 
+        let mut arguments = params.unwrap_or_default();
+        arguments.insert(0, WithSpan::new(Expr::FilterSource, start_s));
         let mut filters = Filter {
             name: filter_name,
-            arguments: params.unwrap_or_default(),
+            arguments,
         };
         for (filter_name, args, span) in extra_filters {
             filters = Filter {

testing/tests/filter_block.rs

@@ -233,7 +233,7 @@ fn filter_block_include() {
     <body class=""><h1>Metadata</h1>
         
 
-    100</body>
+    99</body>
 </html>"#
     );
 }
@@ -272,3 +272,62 @@ fn filter_block_conditions() {
     };
     assert_eq!(s.render().unwrap(), "21x Is Big\n\n    V Is Hoho",);
 }
+
+// The output of `|upper` is not marked as `|safe`, so the output of `|paragraphbreaks` gets
+// escaped. The '&' in the input is is not marked as `|safe`, so it should get escaped, twice.
+#[derive(Template)]
+#[template(
+    source = r#"
+    {%- let count = 1 -%}
+    {%- let canary = 2 -%}
+    {%- filter upper -%}
+        {%- let canary = 3 -%}
+        [
+            {%- for _ in 0..=count %}
+                {%~ filter paragraphbreaks|safe -%}
+                    {{v}}
+                {%~ endfilter -%}
+            {%- endfor -%}
+        ]
+    {%~ endfilter %}{{ canary }}"#,
+    ext = "html"
+)]
+struct NestedFilterBlocks2 {
+    v: &'static str,
+}
+
+#[test]
+fn filter_nested_filter_blocks() {
+    let template = NestedFilterBlocks2 {
+        v: "Hello &\n\ngoodbye!",
+    };
+    assert_eq!(
+        template.render().unwrap(),
+        r#"[
+&#60;P&#62;HELLO &#38;#38;&#60;/P&#62;&#60;P&#62;GOODBYE!
+&#60;/P&#62;
+&#60;P&#62;HELLO &#38;#38;&#60;/P&#62;&#60;P&#62;GOODBYE!
+&#60;/P&#62;]
+2"#
+    );
+}
+
+#[derive(Template)]
+#[template(
+    source = r#"
+    {%- filter urlencode|urlencode|urlencode|urlencode -%}
+        {{ msg.clone()? }}
+    {%~ endfilter %}"#,
+    ext = "html"
+)]
+struct FilterBlockCustomErrors {
+    msg: Result<String, String>,
+}
+
+#[test]
+fn filter_block_custom_errors() {
+    let template = FilterBlockCustomErrors {
+        msg: Err("🐢".to_owned()),
+    };
+    assert_eq!(template.render().unwrap_err().to_string(), "🐢");
+}