Repository: mitsuhiko/minijinja (language: rust)

Issue:
Break/continue support
Hi, Matt from Hugging Face here! We were experimenting with adding `break` and `continue` via the `loopcontrols` extension in Jinja. Is this something that could be added to `minijinja` as well, or would that be very difficult?


Discussion / hints:
No, I'm open to adding this.  I think I would flag it behind a feature like we do with macros.  Not sure if this needs to be something that is controlled on the environment.

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6ce2f4e1..671ce0e4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,12 +2,14 @@
 
 All notable changes to MiniJinja are documented here.
 
-## 2.1.3
+## 2.2.0
 
 - Fixes a bug where some enums did not deserialize correctly when
   used with `ViaDeserialize`.  #554
 - Implemented `IntoDeserializer` for `Value` and `&Value`.  #555
 - Added `filesizeformat` to minijinja-contrib.  #556
+- Added support for the `loop_controls` feature which adds
+  `{% break %}` and `{% continue %}`.  #558
 
 ## 2.1.2
 
diff --git a/Makefile b/Makefile
index d927a1ab..65a2b4a8 100644
--- a/Makefile
+++ b/Makefile
@@ -1,5 +1,5 @@
 DOC_FEATURES=loader,json,urlencode,custom_syntax,fuel
-TEST_FEATURES=unstable_machinery,builtins,loader,json,urlencode,debug,internal_debug,macros,multi_template,adjacent_loop_items,custom_syntax,deserialization,serde
+TEST_FEATURES=unstable_machinery,builtins,loader,json,urlencode,debug,internal_debug,macros,multi_template,adjacent_loop_items,custom_syntax,deserialization,serde,loop_controls
 
 .PHONY: all
 all: test
@@ -40,7 +40,7 @@ snapshot-tests:
 run-tests:
 	@rustup component add rustfmt 2> /dev/null
 	@echo "CARGO TESTS"
-	@cd minijinja; cargo test --features=json,urlencode,internal_debug
+	@cd minijinja; cargo test --features=json,urlencode,internal_debug,loop_controls
 	@echo "CARGO TEST SPEEDUPS"
 	@cd minijinja; cargo test --no-default-features --features=speedups,$(TEST_FEATURES)
 	@echo "CARGO CHECK NO_DEFAULT_FEATURES"
diff --git a/minijinja-cli/Cargo.toml b/minijinja-cli/Cargo.toml
index 5a6f38fc..4c3d4ee4 100644
--- a/minijinja-cli/Cargo.toml
+++ b/minijinja-cli/Cargo.toml
@@ -42,6 +42,7 @@ minijinja = { version = "=2.1.2", path = "../minijinja", features = [
     "fuel",
     "unstable_machinery",
     "custom_syntax",
+    "loop_controls"
 ] }
 minijinja-contrib = { version = "=2.1.2", optional = true, path = "../minijinja-contrib", features = ["pycompat", "datetime", "timezone", "rand"] }
 rustyline = { version = "12.0.0", optional = true }
diff --git a/minijinja/Cargo.toml b/minijinja/Cargo.toml
index 10f6832f..e746bb2f 100644
--- a/minijinja/Cargo.toml
+++ b/minijinja/Cargo.toml
@@ -47,6 +47,7 @@ builtins = []
 macros = []
 multi_template = []
 adjacent_loop_items = []
+loop_controls = []
 fuel = []
 
 # Extra Filters
diff --git a/minijinja/src/compiler/ast.rs b/minijinja/src/compiler/ast.rs
index 4063eb9d..dd4b58fe 100644
--- a/minijinja/src/compiler/ast.rs
+++ b/minijinja/src/compiler/ast.rs
@@ -80,6 +80,10 @@ pub enum Stmt<'a> {
     Macro(Spanned<Macro<'a>>),
     #[cfg(feature = "macros")]
     CallBlock(Spanned<CallBlock<'a>>),
+    #[cfg(feature = "loop_controls")]
+    Continue(Spanned<Continue>),
+    #[cfg(feature = "loop_controls")]
+    Break(Spanned<Break>),
     Do(Spanned<Do<'a>>),
 }
 
@@ -111,6 +115,10 @@ impl<'a> fmt::Debug for Stmt<'a> {
             Stmt::Macro(s) => fmt::Debug::fmt(s, f),
             #[cfg(feature = "macros")]
             Stmt::CallBlock(s) => fmt::Debug::fmt(s, f),
+            #[cfg(feature = "loop_controls")]
+            Stmt::Continue(s) => fmt::Debug::fmt(s, f),
+            #[cfg(feature = "loop_controls")]
+            Stmt::Break(s) => fmt::Debug::fmt(s, f),
             Stmt::Do(s) => fmt::Debug::fmt(s, f),
         }
     }
@@ -298,6 +306,18 @@ pub struct CallBlock<'a> {
     pub macro_decl: Spanned<Macro<'a>>,
 }
 
+/// Continue
+#[cfg_attr(feature = "internal_debug", derive(Debug))]
+#[cfg(feature = "loop_controls")]
+#[cfg_attr(feature = "unstable_machinery_serde", derive(serde::Serialize))]
+pub struct Continue;
+
+/// Break
+#[cfg_attr(feature = "internal_debug", derive(Debug))]
+#[cfg(feature = "loop_controls")]
+#[cfg_attr(feature = "unstable_machinery_serde", derive(serde::Serialize))]
+pub struct Break;
+
 /// A call block
 #[cfg_attr(feature = "internal_debug", derive(Debug))]
 #[cfg_attr(feature = "unstable_machinery_serde", derive(serde::Serialize))]
diff --git a/minijinja/src/compiler/codegen.rs b/minijinja/src/compiler/codegen.rs
index 1e008bed..4c28d602 100644
--- a/minijinja/src/compiler/codegen.rs
+++ b/minijinja/src/compiler/codegen.rs
@@ -35,7 +35,7 @@ fn get_local_id<'source>(ids: &mut BTreeMap<&'source str, LocalId>, name: &'sour
 /// jump targets.
 enum PendingBlock {
     Branch(usize),
-    Loop(usize),
+    Loop(usize, Vec<usize>),
     ScBool(Vec<usize>),
 }
 
@@ -135,29 +135,30 @@ impl<'source> CodeGenerator<'source> {
             flags |= LOOP_FLAG_RECURSIVE;
         }
         self.add(Instruction::PushLoop(flags));
-        let iter_instr = self.add(Instruction::Iterate(!0));
-        self.pending_block.push(PendingBlock::Loop(iter_instr));
+        let instr = self.add(Instruction::Iterate(!0));
+        self.pending_block.push(PendingBlock::Loop(instr, vec![]));
     }
 
     /// Ends the open for loop
     pub fn end_for_loop(&mut self, push_did_not_iterate: bool) {
-        match self.pending_block.pop() {
-            Some(PendingBlock::Loop(iter_instr)) => {
-                self.add(Instruction::Jump(iter_instr));
-                let loop_end = self.next_instruction();
-                if push_did_not_iterate {
-                    self.add(Instruction::PushDidNotIterate);
-                };
-                self.add(Instruction::PopFrame);
-                if let Some(Instruction::Iterate(ref mut jump_target)) =
-                    self.instructions.get_mut(iter_instr)
-                {
-                    *jump_target = loop_end;
-                } else {
-                    unreachable!();
+        if let Some(PendingBlock::Loop(iter_instr, breaks)) = self.pending_block.pop() {
+            self.add(Instruction::Jump(iter_instr));
+            let loop_end = self.next_instruction();
+            if push_did_not_iterate {
+                self.add(Instruction::PushDidNotIterate);
+            };
+            self.add(Instruction::PopFrame);
+            for instr in breaks.into_iter().chain(Some(iter_instr)) {
+                match self.instructions.get_mut(instr) {
+                    Some(Instruction::Iterate(ref mut jump_target))
+                    | Some(Instruction::Jump(ref mut jump_target)) => {
+                        *jump_target = loop_end;
+                    }
+                    _ => unreachable!(),
                 }
             }
-            _ => unreachable!(),
+        } else {
+            unreachable!()
         }
     }
 
@@ -347,6 +348,27 @@ impl<'source> CodeGenerator<'source> {
             ast::Stmt::CallBlock(call_block) => {
                 self.compile_call_block(call_block);
             }
+            #[cfg(feature = "loop_controls")]
+            ast::Stmt::Continue(cont) => {
+                self.set_line_from_span(cont.span());
+                for pending_block in self.pending_block.iter().rev() {
+                    if let PendingBlock::Loop(instr, _) = pending_block {
+                        self.add(Instruction::Jump(*instr));
+                        break;
+                    }
+                }
+            }
+            #[cfg(feature = "loop_controls")]
+            ast::Stmt::Break(brk) => {
+                self.set_line_from_span(brk.span());
+                let instr = self.add(Instruction::Jump(0));
+                for pending_block in self.pending_block.iter_mut().rev() {
+                    if let PendingBlock::Loop(_, ref mut breaks) = pending_block {
+                        breaks.push(instr);
+                        break;
+                    }
+                }
+            }
             ast::Stmt::Do(do_tag) => {
                 self.compile_do(do_tag);
             }
diff --git a/minijinja/src/compiler/meta.rs b/minijinja/src/compiler/meta.rs
index edd6aa06..2f5915aa 100644
--- a/minijinja/src/compiler/meta.rs
+++ b/minijinja/src/compiler/meta.rs
@@ -268,6 +268,8 @@ fn track_walk<'a>(node: &ast::Stmt<'a>, state: &mut AssignmentTracker<'a>) {
                 .for_each(|x| tracker_visit_expr(x, state));
             tracker_visit_macro(&stmt.macro_decl, state);
         }
+        #[cfg(feature = "loop_controls")]
+        ast::Stmt::Continue(_) | ast::Stmt::Break(_) => {}
         ast::Stmt::Do(stmt) => {
             tracker_visit_expr(&stmt.call.expr, state);
             stmt.call
diff --git a/minijinja/src/compiler/parser.rs b/minijinja/src/compiler/parser.rs
index 3bcdf207..c498bbea 100644
--- a/minijinja/src/compiler/parser.rs
+++ b/minijinja/src/compiler/parser.rs
@@ -166,6 +166,8 @@ struct Parser<'a> {
     #[allow(unused)]
     in_macro: bool,
     #[allow(unused)]
+    in_loop: bool,
+    #[allow(unused)]
     blocks: BTreeSet<&'a str>,
     depth: usize,
 }
@@ -236,6 +238,7 @@ impl<'a> Parser<'a> {
         Parser {
             stream: TokenStream::new(source, in_expr, syntax_config, whitespace_config),
             in_macro: false,
+            in_loop: false,
             blocks: BTreeSet::new(),
             depth: 0,
         }
@@ -692,6 +695,20 @@ impl<'a> Parser<'a> {
             "macro" => ast::Stmt::Macro(respan!(ok!(self.parse_macro()))),
             #[cfg(feature = "macros")]
             "call" => ast::Stmt::CallBlock(respan!(ok!(self.parse_call_block()))),
+            #[cfg(feature = "loop_controls")]
+            "continue" => {
+                if !self.in_loop {
+                    syntax_error!("'continue' must be placed inside a loop");
+                }
+                ast::Stmt::Continue(respan!(ast::Continue))
+            }
+            #[cfg(feature = "loop_controls")]
+            "break" => {
+                if !self.in_loop {
+                    syntax_error!("'break' must be placed inside a loop");
+                }
+                ast::Stmt::Break(respan!(ast::Break))
+            }
             "do" => ast::Stmt::Do(respan!(ok!(self.parse_do()))),
             name => syntax_error!("unknown statement {}", name),
         })
@@ -758,6 +775,7 @@ impl<'a> Parser<'a> {
     }
 
     fn parse_for_stmt(&mut self) -> Result<ast::ForLoop<'a>, Error> {
+        let old_in_loop = std::mem::replace(&mut self.in_loop, true);
         let target = ok!(self.parse_assignment());
         expect_token!(self, Token::Ident("in"), "in");
         let iter = ok!(self.parse_expr_noif());
@@ -776,6 +794,7 @@ impl<'a> Parser<'a> {
             Vec::new()
         };
         ok!(self.stream.next());
+        self.in_loop = old_in_loop;
         Ok(ast::ForLoop {
             target,
             iter,
@@ -872,6 +891,7 @@ impl<'a> Parser<'a> {
         if self.in_macro {
             syntax_error!("block tags in macros are not allowed");
         }
+        let old_in_loop = std::mem::replace(&mut self.in_loop, false);
         let (name, _) = expect_token!(self, Token::Ident(name) => name, "identifier");
         if !self.blocks.insert(name) {
             syntax_error!("block '{}' defined twice", name);
@@ -891,6 +911,7 @@ impl<'a> Parser<'a> {
             }
             ok!(self.stream.next());
         }
+        self.in_loop = old_in_loop;
 
         Ok(ast::Block { name, body })
     }
@@ -1035,6 +1056,7 @@ impl<'a> Parser<'a> {
         name: Option<&'a str>,
     ) -> Result<ast::Macro<'a>, Error> {
         expect_token!(self, Token::BlockEnd, "end of block");
+        let old_in_loop = std::mem::replace(&mut self.in_loop, false);
         let old_in_macro = std::mem::replace(&mut self.in_macro, true);
         let body = ok!(self.subparse(&|tok| match tok {
             Token::Ident("endmacro") if name.is_some() => true,
@@ -1042,6 +1064,7 @@ impl<'a> Parser<'a> {
             _ => false,
         }));
         self.in_macro = old_in_macro;
+        self.in_loop = old_in_loop;
         ok!(self.stream.next());
         Ok(ast::Macro {
             name: name.unwrap_or("caller"),
diff --git a/minijinja/src/lib.rs b/minijinja/src/lib.rs
index ee012174..0557aef9 100644
--- a/minijinja/src/lib.rs
+++ b/minijinja/src/lib.rs
@@ -177,6 +177,8 @@
 //! - `json`: When enabled the `tojson` filter is added as builtin filter as well as
 //!   the ability to auto escape via `AutoEscape::Json`.
 //! - `urlencode`: When enabled the `urlencode` filter is added as builtin filter.
+//! - `loop_controls`: enables the `{% break %}` and `{% continue %}` loop control flow
+//!    tags.
 //!
 //! Performance and memory related features:
 //!
diff --git a/minijinja/src/syntax.rs b/minijinja/src/syntax.rs
index 87f9c700..0f47e734 100644
--- a/minijinja/src/syntax.rs
+++ b/minijinja/src/syntax.rs
@@ -33,6 +33,7 @@
 //!   - [`{% do %}`](#-do-)
 //!   - [`{% autoescape %}`](#-autoescape-)
 //!   - [`{% raw %}`](#-raw-)
+//!   - [`{% break %} / {% continue %}`](#-break----continue-)
 #![cfg_attr(
     feature = "custom_syntax",
     doc = "- [Custom Delimiters](#custom-delimiters)"
@@ -721,6 +722,30 @@
 //! {% endraw %}
 //! ```
 //!
+//! ## `{% break %}` / `{% continue %}`
+//!
+//! If MiniJinja was compiled with the `loop_controls` feature, it’s possible to
+//! use `break`` and `continue`` in loops.  When break is reached, the loop is
+//! terminated; if continue is reached, the processing is stopped and continues
+//! with the next iteration.
+//!
+//! Here’s a loop that skips every second item:
+//!
+//! ```jinja
+//! {% for user in users %}
+//! {%- if loop.index is even %}{% continue %}{% endif %}
+//! ...
+//! {% endfor %}
+//! ```
+//!
+//! Likewise, a loop that stops processing after the 10th iteration:
+//!
+//! ```jinja
+//! {% for user in users %}
+//! {%- if loop.index >= 10 %}{% break %}{% endif %}
+//! {%- endfor %}
+//! ```
+//!
 #![cfg_attr(
     feature = "custom_syntax",
     doc = r###"


Test patch:
diff --git a/minijinja/tests/inputs/err_toplevel_break.txt b/minijinja/tests/inputs/err_toplevel_break.txt
new file mode 100644
index 00000000..a452e303
--- /dev/null
+++ b/minijinja/tests/inputs/err_toplevel_break.txt
@@ -0,0 +1,3 @@
+{}
+---
+{% break %}
diff --git a/minijinja/tests/inputs/err_toplevel_continue.txt b/minijinja/tests/inputs/err_toplevel_continue.txt
new file mode 100644
index 00000000..b8376a95
--- /dev/null
+++ b/minijinja/tests/inputs/err_toplevel_continue.txt
@@ -0,0 +1,3 @@
+{}
+---
+{% continue %}
diff --git a/minijinja/tests/inputs/loop_controls.txt b/minijinja/tests/inputs/loop_controls.txt
new file mode 100644
index 00000000..e3013521
--- /dev/null
+++ b/minijinja/tests/inputs/loop_controls.txt
@@ -0,0 +1,7 @@
+{}
+---
+{% for item in range(10) -%}
+  {%- if item is odd %}{% continue %}{% endif %}
+  {%- if item > 5 %}{% break %}{% endif %}
+  {{- item }}
+{%- endfor %}
diff --git a/minijinja/tests/snapshots/test_templates__vm@err_toplevel_break.txt.snap b/minijinja/tests/snapshots/test_templates__vm@err_toplevel_break.txt.snap
new file mode 100644
index 00000000..493d8373
--- /dev/null
+++ b/minijinja/tests/snapshots/test_templates__vm@err_toplevel_break.txt.snap
@@ -0,0 +1,22 @@
+---
+source: minijinja/tests/test_templates.rs
+description: "{% break %}"
+info: {}
+input_file: minijinja/tests/inputs/err_toplevel_break.txt
+---
+!!!SYNTAX ERROR!!!
+
+Error {
+    kind: SyntaxError,
+    detail: "'break' must be placed inside a loop",
+    name: "err_toplevel_break.txt",
+    line: 1,
+}
+
+syntax error: 'break' must be placed inside a loop (in err_toplevel_break.txt:1)
+--------------------------- err_toplevel_break.txt ----------------------------
+   1 > {% break %}
+     i    ^^^^^ syntax error
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+No referenced variables
+-------------------------------------------------------------------------------
diff --git a/minijinja/tests/snapshots/test_templates__vm@err_toplevel_continue.txt.snap b/minijinja/tests/snapshots/test_templates__vm@err_toplevel_continue.txt.snap
new file mode 100644
index 00000000..d80ecbae
--- /dev/null
+++ b/minijinja/tests/snapshots/test_templates__vm@err_toplevel_continue.txt.snap
@@ -0,0 +1,22 @@
+---
+source: minijinja/tests/test_templates.rs
+description: "{% continue %}"
+info: {}
+input_file: minijinja/tests/inputs/err_toplevel_continue.txt
+---
+!!!SYNTAX ERROR!!!
+
+Error {
+    kind: SyntaxError,
+    detail: "'continue' must be placed inside a loop",
+    name: "err_toplevel_continue.txt",
+    line: 1,
+}
+
+syntax error: 'continue' must be placed inside a loop (in err_toplevel_continue.txt:1)
+-------------------------- err_toplevel_continue.txt --------------------------
+   1 > {% continue %}
+     i    ^^^^^^^^ syntax error
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+No referenced variables
+-------------------------------------------------------------------------------
diff --git a/minijinja/tests/snapshots/test_templates__vm@loop_controls.txt.snap b/minijinja/tests/snapshots/test_templates__vm@loop_controls.txt.snap
new file mode 100644
index 00000000..7d49d09b
--- /dev/null
+++ b/minijinja/tests/snapshots/test_templates__vm@loop_controls.txt.snap
@@ -0,0 +1,7 @@
+---
+source: minijinja/tests/test_templates.rs
+description: "{% for item in range(10) -%}\n  {%- if item is odd %}{% continue %}{% endif %}\n  {%- if item > 5 %}{% break %}{% endif %}\n  {{- item }}\n{%- endfor %}"
+info: {}
+input_file: minijinja/tests/inputs/loop_controls.txt
+---
+024