Repository: djc/askama
Pull request #519 (2021-07-18 16:52:33)

Add {% break %} and {% continue %}
This PR adds `{% break %}` and `{% continue %}` statements to break out
of a loop, or continue with the next element of the iterator.

djc: This looks like a nice straightforward implementation, but it looks like this would not throw any error if `{% break %}` or `{% continue %}` are added outside of a loop context -- I think it probably should. Any ideas? (I think it might be easier to do it at the codegen stage than in the parser.)
Kijewski: I implemented the check inside of the parsing step. It was easier to implement, and it has the added bonus that the user gets informed about the exact position of the offending {%break%}.

This was the first time I had to use a Cell. Does this mean I can call myself a Rust programmer now? ;)
djc: Why do we need to use `Cell` here? Could we just pass `&mut State<'a>` around here? (Also I think we could pass `State<'_>` rather than `State<'a>`, would you mind checking that?)
djc: Note that Jinja does not allow this:

> Unlike in Python, it’s not possible to break or continue in a loop.
Kijewski: Replaced `State<'a>` with `State<'_>`. I didn't think much about it because all the functions already used lifetime argument, and replaced `Syntax<'a>` did so, too.

State cannot be passed mutable because it's captured by multiple closures. 

> Here, too, I wonder: do you have use cases for these constructs?

I used it before in Flask apps. You don't have to use break or continue in templates, since you can always put more logic into the Rust code and less in the template code, but I guess that's a matter of taste.

> Note that Jinja does not allow this:
>
>>  Unlike in Python, it’s not possible to break or continue in a loop.

After [13 years](https://github.com/pallets/jinja/blame/55848d36c0a589139e5003a5ad263fce0d95aa2f/src/jinja2/ext.py#L588), can we call an extension part of the standard? :)
djc: If you rebase this, I'm happy to merge this. Sorry for the long review cycle!
Kijewski: No problem. Rebased.
djc: Thanks!

Patch:
diff --git a/askama_shared/src/generator.rs b/askama_shared/src/generator.rs
index ec468e5..3bcb5d9 100644
--- a/askama_shared/src/generator.rs
+++ b/askama_shared/src/generator.rs
@@ -459,6 +459,16 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> {
                     // No whitespace handling: child template top-level is not used,
                     // except for the blocks defined in it.
                 }
+                Node::Break(ws) => {
+                    self.handle_ws(ws);
+                    self.write_buf_writable(buf)?;
+                    buf.writeln("break;")?;
+                }
+                Node::Continue(ws) => {
+                    self.handle_ws(ws);
+                    self.write_buf_writable(buf)?;
+                    buf.writeln("continue;")?;
+                }
             }
         }
 
diff --git a/askama_shared/src/parser.rs b/askama_shared/src/parser.rs
index 66f2ec1..eba4370 100644
--- a/askama_shared/src/parser.rs
+++ b/askama_shared/src/parser.rs
@@ -1,3 +1,6 @@
+use std::cell::Cell;
+use std::str;
+
 use nom::branch::alt;
 use nom::bytes::complete::{escaped, is_not, tag, take_until};
 use nom::character::complete::{anychar, char, digit1};
@@ -6,7 +9,6 @@ use nom::error::{Error, ParseError};
 use nom::multi::{fold_many0, many0, many1, separated_list0, separated_list1};
 use nom::sequence::{delimited, pair, preceded, terminated, tuple};
 use nom::{self, error_position, Compare, IResult, InputLength, InputTake};
-use std::str;
 
 use crate::{CompileError, Syntax};
 
@@ -27,6 +29,8 @@ pub enum Node<'a> {
     Import(Ws, &'a str, &'a str),
     Macro(&'a str, Macro<'a>),
     Raw(Ws, &'a str, Ws),
+    Break(Ws),
+    Continue(Ws),
 }
 
 #[derive(Debug, PartialEq)]
@@ -178,14 +182,19 @@ enum ContentState {
     End(usize),
 }
 
-fn take_content<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> ParserError<'a, Node<'a>> {
+struct State<'a> {
+    syntax: &'a Syntax<'a>,
+    loop_depth: Cell<usize>,
+}
+
+fn take_content<'a>(i: &'a [u8], s: &State<'_>) -> ParserError<'a, Node<'a>> {
     use crate::parser::ContentState::*;
-    let bs = s.block_start.as_bytes()[0];
-    let be = s.block_start.as_bytes()[1];
-    let cs = s.comment_start.as_bytes()[0];
-    let ce = s.comment_start.as_bytes()[1];
-    let es = s.expr_start.as_bytes()[0];
-    let ee = s.expr_start.as_bytes()[1];
+    let bs = s.syntax.block_start.as_bytes()[0];
+    let be = s.syntax.block_start.as_bytes()[1];
+    let cs = s.syntax.comment_start.as_bytes()[0];
+    let ce = s.syntax.comment_start.as_bytes()[1];
+    let es = s.syntax.expr_start.as_bytes()[0];
+    let ee = s.syntax.expr_start.as_bytes()[1];
 
     let mut state = Start;
     for (idx, c) in i.iter().enumerate() {
@@ -683,7 +692,7 @@ fn expr_any(i: &[u8]) -> IResult<&[u8], Expr<'_>> {
     alt((range_right, compound, expr_or))(i)
 }
 
-fn expr_node<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], Node<'a>> {
+fn expr_node<'a>(i: &'a [u8], s: &State<'_>) -> IResult<&'a [u8], Node<'a>> {
     let mut p = tuple((
         |i| tag_expr_start(i, s),
         cut(tuple((opt(tag("-")), ws(expr_any), opt(tag("-")), |i| {
@@ -729,7 +738,7 @@ fn cond_if(i: &[u8]) -> IResult<&[u8], CondTest<'_>> {
     Ok((i, CondTest { target, expr }))
 }
 
-fn cond_block<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], Cond<'a>> {
+fn cond_block<'a>(i: &'a [u8], s: &State<'_>) -> IResult<&'a [u8], Cond<'a>> {
     let mut p = tuple((
         |i| tag_block_start(i, s),
         opt(tag("-")),
@@ -745,7 +754,7 @@ fn cond_block<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], Cond<'a>>
     Ok((i, (Ws(pws.is_some(), nws.is_some()), cond, block)))
 }
 
-fn block_if<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], Node<'a>> {
+fn block_if<'a>(i: &'a [u8], s: &State<'_>) -> IResult<&'a [u8], Node<'a>> {
     let mut p = tuple((
         opt(tag("-")),
         cond_if,
@@ -771,7 +780,7 @@ fn block_if<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], Node<'a>> {
     Ok((i, Node::Cond(res, Ws(pws2.is_some(), nws2.is_some()))))
 }
 
-fn match_else_block<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], When<'a>> {
+fn match_else_block<'a>(i: &'a [u8], s: &State<'_>) -> IResult<&'a [u8], When<'a>> {
     let mut p = tuple((
         |i| tag_block_start(i, s),
         opt(tag("-")),
@@ -789,7 +798,7 @@ fn match_else_block<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], Whe
     ))
 }
 
-fn when_block<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], When<'a>> {
+fn when_block<'a>(i: &'a [u8], s: &State<'_>) -> IResult<&'a [u8], When<'a>> {
     let mut p = tuple((
         |i| tag_block_start(i, s),
         opt(tag("-")),
@@ -805,7 +814,7 @@ fn when_block<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], When<'a>>
     Ok((i, (Ws(pws.is_some(), nws.is_some()), target, block)))
 }
 
-fn block_match<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], Node<'a>> {
+fn block_match<'a>(i: &'a [u8], s: &State<'_>) -> IResult<&'a [u8], Node<'a>> {
     let mut p = tuple((
         opt(tag("-")),
         ws(tag("match")),
@@ -885,7 +894,14 @@ fn block_let(i: &[u8]) -> IResult<&[u8], Node<'_>> {
     ))
 }
 
-fn block_for<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], Node<'a>> {
+fn parse_loop_content<'a>(i: &'a [u8], s: &State<'_>) -> IResult<&'a [u8], Vec<Node<'a>>> {
+    s.loop_depth.set(s.loop_depth.get() + 1);
+    let (i, node) = parse_template(i, s)?;
+    s.loop_depth.set(s.loop_depth.get() - 1);
+    Ok((i, node))
+}
+
+fn block_for<'a>(i: &'a [u8], s: &State<'_>) -> IResult<&'a [u8], Node<'a>> {
     let mut p = tuple((
         opt(tag("-")),
         ws(tag("for")),
@@ -897,7 +913,7 @@ fn block_for<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], Node<'a>>
                 opt(tag("-")),
                 |i| tag_block_end(i, s),
                 cut(tuple((
-                    |i| parse_template(i, s),
+                    |i| parse_loop_content(i, s),
                     cut(tuple((
                         |i| tag_block_start(i, s),
                         opt(tag("-")),
@@ -926,7 +942,7 @@ fn block_extends(i: &[u8]) -> IResult<&[u8], Node<'_>> {
     Ok((i, Node::Extends(name)))
 }
 
-fn block_block<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], Node<'a>> {
+fn block_block<'a>(i: &'a [u8], s: &State<'_>) -> IResult<&'a [u8], Node<'a>> {
     let mut start = tuple((
         opt(tag("-")),
         ws(tag("block")),
@@ -1001,7 +1017,7 @@ fn block_import(i: &[u8]) -> IResult<&[u8], Node<'_>> {
     ))
 }
 
-fn block_macro<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], Node<'a>> {
+fn block_macro<'a>(i: &'a [u8], s: &State<'_>) -> IResult<&'a [u8], Node<'a>> {
     let mut p = tuple((
         opt(tag("-")),
         ws(tag("macro")),
@@ -1041,7 +1057,7 @@ fn block_macro<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], Node<'a>
     ))
 }
 
-fn block_raw<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], Node<'a>> {
+fn block_raw<'a>(i: &'a [u8], s: &State<'_>) -> IResult<&'a [u8], Node<'a>> {
     let mut p = tuple((
         opt(tag("-")),
         ws(tag("raw")),
@@ -1068,7 +1084,31 @@ fn block_raw<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], Node<'a>>
     ))
 }
 
-fn block_node<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], Node<'a>> {
+fn break_statement<'a>(i: &'a [u8], s: &State<'_>) -> IResult<&'a [u8], Node<'a>> {
+    let mut p = tuple((opt(tag("-")), ws(tag("break")), opt(tag("-"))));
+    let (j, (pws, _, nws)) = p(i)?;
+    if s.loop_depth.get() == 0 {
+        return Err(nom::Err::Failure(error_position!(
+            i,
+            nom::error::ErrorKind::Tag
+        )));
+    }
+    Ok((j, Node::Break(Ws(pws.is_some(), nws.is_some()))))
+}
+
+fn continue_statement<'a>(i: &'a [u8], s: &State<'_>) -> IResult<&'a [u8], Node<'a>> {
+    let mut p = tuple((opt(tag("-")), ws(tag("continue")), opt(tag("-"))));
+    let (j, (pws, _, nws)) = p(i)?;
+    if s.loop_depth.get() == 0 {
+        return Err(nom::Err::Failure(error_position!(
+            i,
+            nom::error::ErrorKind::Tag
+        )));
+    }
+    Ok((j, Node::Continue(Ws(pws.is_some(), nws.is_some()))))
+}
+
+fn block_node<'a>(i: &'a [u8], s: &State<'_>) -> IResult<&'a [u8], Node<'a>> {
     let mut p = tuple((
         |i| tag_block_start(i, s),
         alt((
@@ -1083,6 +1123,8 @@ fn block_node<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], Node<'a>>
             |i| block_block(i, s),
             |i| block_macro(i, s),
             |i| block_raw(i, s),
+            |i| break_statement(i, s),
+            |i| continue_statement(i, s),
         )),
         cut(|i| tag_block_end(i, s)),
     ));
@@ -1090,11 +1132,11 @@ fn block_node<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], Node<'a>>
     Ok((i, contents))
 }
 
-fn block_comment_body<'a>(mut i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], &'a [u8]> {
+fn block_comment_body<'a>(mut i: &'a [u8], s: &State<'_>) -> IResult<&'a [u8], &'a [u8]> {
     let mut level = 0;
     loop {
-        let (end, tail) = take_until(s.comment_end)(i)?;
-        match take_until::<_, _, Error<_>>(s.comment_start)(i) {
+        let (end, tail) = take_until(s.syntax.comment_end)(i)?;
+        match take_until::<_, _, Error<_>>(s.syntax.comment_start)(i) {
             Ok((start, _)) if start.as_ptr() < end.as_ptr() => {
                 level += 1;
                 i = &start[2..];
@@ -1108,7 +1150,7 @@ fn block_comment_body<'a>(mut i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8
     }
 }
 
-fn block_comment<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], Node<'a>> {
+fn block_comment<'a>(i: &'a [u8], s: &State<'_>) -> IResult<&'a [u8], Node<'a>> {
     let mut p = tuple((
         |i| tag_comment_start(i, s),
         cut(tuple((
@@ -1121,7 +1163,7 @@ fn block_comment<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], Node<'
     Ok((i, Node::Comment(Ws(pws.is_some(), tail.ends_with(b"-")))))
 }
 
-fn parse_template<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], Vec<Node<'a>>> {
+fn parse_template<'a>(i: &'a [u8], s: &State<'_>) -> IResult<&'a [u8], Vec<Node<'a>>> {
     many0(alt((
         complete(|i| take_content(i, s)),
         complete(|i| block_comment(i, s)),
@@ -1130,27 +1172,31 @@ fn parse_template<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], Vec<N
     )))(i)
 }
 
-fn tag_block_start<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], &'a [u8]> {
-    tag(s.block_start)(i)
+fn tag_block_start<'a>(i: &'a [u8], s: &State<'_>) -> IResult<&'a [u8], &'a [u8]> {
+    tag(s.syntax.block_start)(i)
 }
-fn tag_block_end<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], &'a [u8]> {
-    tag(s.block_end)(i)
+fn tag_block_end<'a>(i: &'a [u8], s: &State<'_>) -> IResult<&'a [u8], &'a [u8]> {
+    tag(s.syntax.block_end)(i)
 }
-fn tag_comment_start<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], &'a [u8]> {
-    tag(s.comment_start)(i)
+fn tag_comment_start<'a>(i: &'a [u8], s: &State<'_>) -> IResult<&'a [u8], &'a [u8]> {
+    tag(s.syntax.comment_start)(i)
 }
-fn tag_comment_end<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], &'a [u8]> {
-    tag(s.comment_end)(i)
+fn tag_comment_end<'a>(i: &'a [u8], s: &State<'_>) -> IResult<&'a [u8], &'a [u8]> {
+    tag(s.syntax.comment_end)(i)
 }
-fn tag_expr_start<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], &'a [u8]> {
-    tag(s.expr_start)(i)
+fn tag_expr_start<'a>(i: &'a [u8], s: &State<'_>) -> IResult<&'a [u8], &'a [u8]> {
+    tag(s.syntax.expr_start)(i)
 }
-fn tag_expr_end<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], &'a [u8]> {
-    tag(s.expr_end)(i)
+fn tag_expr_end<'a>(i: &'a [u8], s: &State<'_>) -> IResult<&'a [u8], &'a [u8]> {
+    tag(s.syntax.expr_end)(i)
 }
 
 pub fn parse<'a>(src: &'a str, syntax: &'a Syntax<'a>) -> Result<Vec<Node<'a>>, CompileError> {
-    match parse_template(src.as_bytes(), syntax) {
+    let state = State {
+        syntax,
+        loop_depth: Cell::new(0),
+    };
+    match parse_template(src.as_bytes(), &state) {
         Ok((left, res)) => {
             if !left.is_empty() {
                 let s = str::from_utf8(left).unwrap();


Test patch:
diff --git a/testing/templates/for-break-continue.html b/testing/templates/for-break-continue.html
new file mode 100644
index 0000000..c7a948f
--- /dev/null
+++ b/testing/templates/for-break-continue.html
@@ -0,0 +1,11 @@
+{%- for v in values -%}
+    x {{- v -}}
+    {%- if matches!(v, x if *x > 9) -%}
+        {%- if matches!(v, x if *x % 2 == 0) -%}
+            {%- break -%}
+        {%- else -%}
+            {%- continue -%}
+        {%- endif -%}
+    {%- endif -%}
+    y
+{%- endfor -%}
diff --git a/testing/tests/loops.rs b/testing/tests/loops.rs
index fffb39d..49c37e0 100644
--- a/testing/tests/loops.rs
+++ b/testing/tests/loops.rs
@@ -304,3 +304,71 @@ fn test_for_enumerate() {
     };
     assert_eq!(t.render().unwrap(), "0=hello-1=world-2=!-");
 }
+
+#[derive(Template)]
+#[template(
+    source = "{% for v in values.iter() %}x{{v}}{% if matches!(v, x if *x==3) %}{% break %}{% endif %}y{% endfor %}",
+    ext = "txt"
+)]
+struct Break<'a> {
+    values: &'a [i32],
+}
+
+#[test]
+fn test_loop_break() {
+    let t = Break {
+        values: &[1, 2, 3, 4, 5],
+    };
+    assert_eq!(t.render().unwrap(), "x1yx2yx3");
+
+    let t = Break {
+        values: &[1, 2, 4, 5],
+    };
+    assert_eq!(t.render().unwrap(), "x1yx2yx4yx5y");
+}
+
+#[derive(Template)]
+#[template(
+    source = "{% for v in values %}x{{v}}{% if matches!(v, x if *x==3) %}{% continue %}{% endif %}y{% endfor %}",
+    ext = "txt"
+)]
+struct Continue<'a> {
+    values: &'a [i32],
+}
+
+#[test]
+fn test_loop_continue() {
+    let t = Continue {
+        values: &[1, 2, 3, 4, 5],
+    };
+    assert_eq!(t.render().unwrap(), "x1yx2yx3x4yx5y");
+
+    let t = Continue {
+        values: &[1, 2, 4, 5],
+    };
+    assert_eq!(t.render().unwrap(), "x1yx2yx4yx5y");
+}
+
+#[derive(Template)]
+#[template(path = "for-break-continue.html")]
+struct BreakContinue<'a> {
+    values: &'a [i32],
+}
+
+#[test]
+fn test_loop_break_continue() {
+    let t = BreakContinue {
+        values: &[1, 2, 3, 4, 5],
+    };
+    assert_eq!(t.render().unwrap(), "x1yx2yx3yx4yx5y");
+
+    let t = BreakContinue {
+        values: &[1, 2, 3, 10, 4, 5],
+    };
+    assert_eq!(t.render().unwrap(), "x1yx2yx3yx10");
+
+    let t = BreakContinue {
+        values: &[1, 2, 3, 11, 4, 5],
+    };
+    assert_eq!(t.render().unwrap(), "x1yx2yx3yx11x4yx5y");
+}
diff --git a/testing/tests/ui/break_outside_of_loop.rs b/testing/tests/ui/break_outside_of_loop.rs
new file mode 100644
index 0000000..ccbb0a9
--- /dev/null
+++ b/testing/tests/ui/break_outside_of_loop.rs
@@ -0,0 +1,11 @@
+use askama::Template;
+
+#[derive(Template)]
+#[template(
+    source = "Have a {%break%}, have a parsing error!",
+    ext = "txt"
+)]
+struct MyTemplate;
+
+fn main() {
+}
diff --git a/testing/tests/ui/break_outside_of_loop.stderr b/testing/tests/ui/break_outside_of_loop.stderr
new file mode 100644
index 0000000..99d1d25
--- /dev/null
+++ b/testing/tests/ui/break_outside_of_loop.stderr
@@ -0,0 +1,8 @@
+error: problems parsing template source at row 1, column 9 near:
+"break%}, have a parsing error!"
+ --> $DIR/break_outside_of_loop.rs:3:10
+  |
+3 | #[derive(Template)]
+  |          ^^^^^^^^
+  |
+  = note: this error originates in the derive macro `Template` (in Nightly builds, run with -Z macro-backtrace for more info)