From 0f2759e83f636811a9cd57887f40effbcb5e0162 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Fri, 1 Jul 2022 22:58:57 +0200 Subject: [PATCH] Added support for `{% set %}` (#70) --- CHANGELOG.md | 1 + minijinja/src/ast.rs | 9 + minijinja/src/compiler.rs | 5 + minijinja/src/meta.rs | 6 +- minijinja/src/parser.rs | 19 ++ minijinja/src/syntax.rs | 14 ++ minijinja/src/value.rs | 2 +- minijinja/src/vm.rs | 210 ++++++++---------- minijinja/tests/inputs/set.txt | 22 ++ minijinja/tests/parser-inputs/set.txt | 2 + .../test_parser__parser@set.txt.snap | 44 ++++ .../snapshots/test_templates__vm@set.txt.snap | 34 +++ 12 files changed, 254 insertions(+), 114 deletions(-) create mode 100644 minijinja/tests/inputs/set.txt create mode 100644 minijinja/tests/parser-inputs/set.txt create mode 100644 minijinja/tests/snapshots/test_parser__parser@set.txt.snap create mode 100644 minijinja/tests/snapshots/test_templates__vm@set.txt.snap diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ae1307a..8a5fdbf5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ All notable changes to MiniJinja are documented here. - Added support for `{% raw %}`. (#67) - Minimum Rust version moved up to 1.45. +- Added support for `{% set %}`. (#70) # 0.16.0 diff --git a/minijinja/src/ast.rs b/minijinja/src/ast.rs index 4ad943cb..405892a7 100644 --- a/minijinja/src/ast.rs +++ b/minijinja/src/ast.rs @@ -56,6 +56,7 @@ pub enum Stmt<'a> { ForLoop(Spanned>), IfCond(Spanned>), WithBlock(Spanned>), + Set(Spanned>), Block(Spanned>), Extends(Spanned>), Include(Spanned>), @@ -73,6 +74,7 @@ impl<'a> fmt::Debug for Stmt<'a> { Stmt::ForLoop(s) => fmt::Debug::fmt(s, f), Stmt::IfCond(s) => fmt::Debug::fmt(s, f), Stmt::WithBlock(s) => fmt::Debug::fmt(s, f), + Stmt::Set(s) => fmt::Debug::fmt(s, f), Stmt::Block(s) => fmt::Debug::fmt(s, f), Stmt::Extends(s) => fmt::Debug::fmt(s, f), Stmt::Include(s) => fmt::Debug::fmt(s, f), @@ -151,6 +153,13 @@ pub struct WithBlock<'a> { pub body: Vec>, } +/// A set statement. +#[cfg_attr(feature = "internal_debug", derive(Debug))] +pub struct Set<'a> { + pub target: Expr<'a>, + pub expr: Expr<'a>, +} + /// A block for inheritance elements. #[cfg_attr(feature = "internal_debug", derive(Debug))] pub struct Block<'a> { diff --git a/minijinja/src/compiler.rs b/minijinja/src/compiler.rs index f965e747..92db05f6 100644 --- a/minijinja/src/compiler.rs +++ b/minijinja/src/compiler.rs @@ -261,6 +261,11 @@ impl<'source> Compiler<'source> { } self.add(Instruction::PopFrame); } + ast::Stmt::Set(set) => { + self.set_location_from_span(set.span()); + self.compile_expr(&set.expr)?; + self.compile_assignment(&set.target)?; + } ast::Stmt::Block(block) => { self.set_location_from_span(block.span()); let mut sub_compiler = diff --git a/minijinja/src/meta.rs b/minijinja/src/meta.rs index bea70683..648a7afe 100644 --- a/minijinja/src/meta.rs +++ b/minijinja/src/meta.rs @@ -172,6 +172,10 @@ pub fn find_undeclared_variables(source: &str) -> Result, Error> stmt.body.iter().for_each(|x| walk(x, state)); state.pop(); } + ast::Stmt::Set(stmt) => { + assign_nested(&stmt.target, state); + visit_expr(&stmt.expr, state); + } ast::Stmt::Block(stmt) => { state.push(); state.assign("super"); @@ -233,7 +237,7 @@ pub fn find_referenced_templates(source: &str) -> Result, Error> fn walk(node: &ast::Stmt, out: &mut HashSet) { match node { ast::Stmt::Template(stmt) => stmt.children.iter().for_each(|x| walk(x, out)), - ast::Stmt::EmitExpr(_) | ast::Stmt::EmitRaw(_) => {} + ast::Stmt::EmitExpr(_) | ast::Stmt::EmitRaw(_) | ast::Stmt::Set(_) => {} ast::Stmt::ForLoop(stmt) => stmt .body .iter() diff --git a/minijinja/src/parser.rs b/minijinja/src/parser.rs index a009eef6..e58c27f5 100644 --- a/minijinja/src/parser.rs +++ b/minijinja/src/parser.rs @@ -547,6 +547,10 @@ impl<'a> Parser<'a> { self.parse_with_block()?, self.stream.expand_span(span), ))), + Token::Ident("set") => Ok(ast::Stmt::Set(Spanned::new( + self.parse_set()?, + self.stream.expand_span(span), + ))), Token::Ident("block") => Ok(ast::Stmt::Block(Spanned::new( self.parse_block()?, self.stream.expand_span(span), @@ -718,6 +722,21 @@ impl<'a> Parser<'a> { Ok(ast::WithBlock { assignments, body }) } + fn parse_set(&mut self) -> Result, Error> { + let target = if matches!(self.stream.current()?, Some((Token::ParenOpen, _))) { + self.stream.next()?; + let assign = self.parse_assignment()?; + expect_token!(self, Token::ParenClose, "`)`")?; + assign + } else { + self.parse_assign_name()? + }; + expect_token!(self, Token::Assign, "assignment operator")?; + let expr = self.parse_expr()?; + + Ok(ast::Set { target, expr }) + } + fn parse_block(&mut self) -> Result, Error> { let (name, _) = expect_token!(self, Token::Ident(name) => name, "identifier")?; expect_token!(self, Token::BlockEnd(..), "end of block")?; diff --git a/minijinja/src/syntax.rs b/minijinja/src/syntax.rs index a1aa9f43..7e81cc20 100644 --- a/minijinja/src/syntax.rs +++ b/minijinja/src/syntax.rs @@ -18,6 +18,7 @@ //! - [`{% block %}`](#-block-) //! - [`{% include %}`](#-include-) //! - [`{% with %}`](#-with-) +//! - [`{% set %}`](#-set-) //! - [`{% filter %}`](#-filter-) //! - [`{% autoescape %}`](#-autoescape-) //! - [`{% raw %}`](#-raw-) @@ -405,6 +406,19 @@ //! {% endwith %} //! ``` //! +//! ## `{% set %}` +//! +//! The `set` statement can be used to assign to variables on the same scope. This is +//! similar to how `with` works but it won't introduce a new scope. +//! +//! ```jinja +//! {% set navigation = [('index.html', 'Index'), ('about.html', 'About')] %} +//! ``` +//! +//! Please keep in mind that it is not possible to set variables inside a block +//! and have them show up outside of it. This also applies to loops. The only +//! exception to that rule are if statements which do not introduce a scope. +//! //! ## `{% filter %}` //! //! Filter sections allow you to apply regular [filters](crate::filters) on a diff --git a/minijinja/src/value.rs b/minijinja/src/value.rs index 5adb8113..dd43f88a 100644 --- a/minijinja/src/value.rs +++ b/minijinja/src/value.rs @@ -1601,7 +1601,7 @@ impl Iterator for ValueIterator { impl ExactSizeIterator for ValueIterator {} -impl<'a> fmt::Debug for ValueIterator { +impl fmt::Debug for ValueIterator { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("ValueIterator").finish() } diff --git a/minijinja/src/vm.rs b/minijinja/src/vm.rs index 20b201b3..8e6a9a12 100644 --- a/minijinja/src/vm.rs +++ b/minijinja/src/vm.rs @@ -97,8 +97,7 @@ impl fmt::Display for LoopState { type Locals<'env> = BTreeMap<&'env str, Value>; #[cfg_attr(feature = "internal_debug", derive(Debug))] -pub struct Loop<'env> { - locals: Locals<'env>, +pub struct Loop { with_loop_var: bool, recurse_jump_target: Option, // if we're popping the frame, do we want to jump somewhere? The @@ -110,39 +109,46 @@ pub struct Loop<'env> { } #[cfg_attr(feature = "internal_debug", derive(Debug))] -pub struct With<'env> { +pub enum FrameBase<'env, 'vm> { + None, + Context(&'vm Context<'env, 'vm>), + Value(Value), +} + +pub struct Frame<'env, 'vm> { locals: Locals<'env>, + base: FrameBase<'env, 'vm>, + current_loop: Option, } -pub enum Frame<'env, 'vm> { - // This layer dispatches to another context - Chained { base: &'vm Context<'env, 'vm> }, - // this layer isolates - Isolate { value: Value }, - // this layer is a with statement. - With(With<'env>), - // this layer is a for loop - Loop(Loop<'env>), +impl<'env, 'vm> Default for Frame<'env, 'vm> { + fn default() -> Frame<'env, 'vm> { + Frame::new(FrameBase::None) + } +} + +impl<'env, 'vm> Frame<'env, 'vm> { + pub fn new(base: FrameBase<'env, 'vm>) -> Frame<'env, 'vm> { + Frame { + locals: Locals::new(), + base, + current_loop: None, + } + } } #[cfg(feature = "internal_debug")] impl<'env, 'vm> fmt::Debug for Frame<'env, 'vm> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::Chained { base } => fmt::Debug::fmt(base, f), - Self::Isolate { value } => fmt::Debug::fmt(value, f), - Self::With(w) => { - let mut m = f.debug_map(); - m.entries(w.locals.iter()); - m.finish() - } - Self::Loop(l) => { - let mut m = f.debug_map(); - m.entries(l.locals.iter()); - m.entry(&"loop", &l.controller); - m.finish() - } + let mut m = f.debug_map(); + m.entries(self.locals.iter()); + if let Some(Loop { ref controller, .. }) = self.current_loop { + m.entry(&"loop", controller); + } + if let FrameBase::Value(ref value) = self.base { + m.entries(value.iter_as_str_map()); } + m.finish() } } @@ -182,39 +188,31 @@ impl<'env, 'vm> Context<'env, 'vm> { /// Since it's only used for the debug support changing this is not too /// critical. #[cfg(feature = "debug")] - fn freeze<'a>(&'a self, env: &'a Environment) -> BTreeMap<&'a str, Value> { - let mut rv = BTreeMap::new(); - - for ctx in self.stack.iter() { - let (lookup_base, cont) = match ctx { - // if we hit a chain frame we dispatch there and never - // recurse - Frame::Chained { base } => return base.freeze(env), - Frame::Isolate { value } => (value, false), - Frame::With(With { locals }) => { - rv.extend(locals.iter().map(|(k, v)| (*k, v.clone()))); - continue; - } - Frame::Loop(Loop { - locals, - controller, - with_loop_var, - .. - }) => { - rv.extend(locals.iter().map(|(k, v)| (*k, v.clone()))); - if *with_loop_var { - rv.insert("loop", Value::from_rc_object(controller.clone())); - } - continue; - } - }; + fn freeze<'a>(&'a self, env: &'a Environment) -> Locals { + let mut rv = Locals::new(); - if !cont { - rv.clear(); - rv.extend(env.globals.iter().map(|(k, v)| (*k, v.clone()))); + rv.extend(env.globals.iter().map(|(k, v)| (*k, v.clone()))); + + for frame in self.stack.iter().rev() { + // look at locals first + rv.extend(frame.locals.iter().map(|(k, v)| (*k, v.clone()))); + + // if we are a loop, check if we are looking up the special loop var. + if let Some(ref l) = frame.current_loop { + if l.with_loop_var { + rv.insert("loop", Value::from_rc_object(l.controller.clone())); + } } - rv.extend(lookup_base.iter_as_str_map()); + match frame.base { + FrameBase::Context(ctx) => { + rv.extend(ctx.freeze(env)); + } + FrameBase::Value(ref value) => { + rv.extend(value.iter_as_str_map()); + } + FrameBase::None => continue, + } } rv @@ -222,23 +220,33 @@ impl<'env, 'vm> Context<'env, 'vm> { /// Stores a variable in the context. pub fn store(&mut self, key: &'env str, value: Value) { - if let Frame::Loop(Loop { locals, .. }) | Frame::With(With { locals }) = - self.stack.last_mut().expect("cannot store on empty stack") - { - locals.insert(key, value); - } else { - panic!("can only assign to a loop or with statement") - } + self.stack + .last_mut() + .expect("cannot store on empty stack") + .locals + .insert(key, value); } /// Looks up a variable in the context. pub fn load(&self, env: &Environment, key: &str) -> Option { - for ctx in self.stack.iter().rev() { - match ctx { - // if we hit a chain frame we dispatch there and never - // recurse - Frame::Chained { base } => return base.load(env, key), - Frame::Isolate { value } => { + for frame in self.stack.iter().rev() { + // look at locals first + if let Some(value) = frame.locals.get(key) { + if !value.is_undefined() { + return Some(value.clone()); + } + } + + // if we are a loop, check if we are looking up the special loop var. + if let Some(ref l) = frame.current_loop { + if l.with_loop_var && key == "loop" { + return Some(Value::from_rc_object(l.controller.clone())); + } + } + + match frame.base { + FrameBase::Context(ctx) => return ctx.load(env, key), + FrameBase::Value(ref value) => { let rv = value.get_attr(key); if let Ok(rv) = rv { if !rv.is_undefined() { @@ -248,28 +256,9 @@ impl<'env, 'vm> Context<'env, 'vm> { if let Some(value) = env.get_global(key) { return Some(value); } - break; } - Frame::With(With { locals }) => { - if let Some(value) = locals.get(key) { - return Some(value.clone()); - } - continue; - } - Frame::Loop(Loop { - locals, - controller, - with_loop_var, - .. - }) => { - if *with_loop_var && key == "loop" { - return Some(Value::from_rc_object(controller.clone())); - } else if let Some(value) = locals.get(key) { - return Some(value.clone()); - } - continue; - } - }; + FrameBase::None => continue, + } } None } @@ -285,14 +274,11 @@ impl<'env, 'vm> Context<'env, 'vm> { } /// Returns the current innermost loop. - pub fn current_loop(&mut self) -> Option<&mut Loop<'env>> { + pub fn current_loop(&mut self) -> Option<&mut Loop> { self.stack .iter_mut() .rev() - .filter_map(|x| match *x { - Frame::Loop(ref mut x) => Some(x), - _ => None, - }) + .filter_map(|x| x.current_loop.as_mut()) .next() } } @@ -423,7 +409,7 @@ impl<'env> Vm<'env> { output: &mut String, ) -> Result, Error> { let mut ctx = Context::default(); - ctx.push_frame(Frame::Isolate { value: root }); + ctx.push_frame(Frame::new(FrameBase::Value(root))); let mut referenced_blocks = BTreeMap::new(); for (&name, instr) in blocks.iter() { referenced_blocks.insert(name, vec![instr]); @@ -532,7 +518,7 @@ impl<'env> Vm<'env> { }}; ($instructions:expr, $blocks:expr, $current_block:expr, $auto_escape:expr) => {{ let mut sub_context = Context::default(); - sub_context.push_frame(Frame::Chained { base: &state.ctx }); + sub_context.push_frame(Frame::new(FrameBase::Context(&state.ctx))); let mut sub_state = State { env: self.env, ctx: sub_context, @@ -704,12 +690,10 @@ impl<'env> Vm<'env> { stack.push(try_ctx!(value::neg(&a))); } Instruction::PushWith => { - state.ctx.push_frame(Frame::With(With { - locals: Locals::new(), - })); + state.ctx.push_frame(Frame::new(FrameBase::None)); } Instruction::PopFrame => { - if let Frame::Loop(mut loop_ctx) = state.ctx.pop_frame() { + if let Some(mut loop_ctx) = state.ctx.pop_frame().current_loop { if let Some((target, end_capture)) = loop_ctx.current_recursion_jump.take() { pc = target; @@ -730,18 +714,20 @@ impl<'env> Vm<'env> { .filter(|x| x.recurse_jump_target.is_some()) .map_or(0, |x| x.controller.depth + 1); let recursive = *flags & LOOP_FLAG_RECURSIVE != 0; - state.ctx.push_frame(Frame::Loop(Loop { - locals: Locals::new(), - iterator, - with_loop_var: *flags & LOOP_FLAG_WITH_LOOP_VAR != 0, - recurse_jump_target: if recursive { Some(pc) } else { None }, - current_recursion_jump: next_loop_recursion_jump.take(), - controller: RcType::new(LoopState { - idx: AtomicUsize::new(!0usize), - len: AtomicUsize::new(len), - depth, + state.ctx.push_frame(Frame { + current_loop: Some(Loop { + iterator, + with_loop_var: *flags & LOOP_FLAG_WITH_LOOP_VAR != 0, + recurse_jump_target: if recursive { Some(pc) } else { None }, + current_recursion_jump: next_loop_recursion_jump.take(), + controller: RcType::new(LoopState { + idx: AtomicUsize::new(!0usize), + len: AtomicUsize::new(len), + depth, + }), }), - })); + ..Frame::default() + }); } Instruction::Iterate(jump_target) => { let l = state.ctx.current_loop().expect("not inside a loop"); diff --git a/minijinja/tests/inputs/set.txt b/minijinja/tests/inputs/set.txt new file mode 100644 index 00000000..03fdefb2 --- /dev/null +++ b/minijinja/tests/inputs/set.txt @@ -0,0 +1,22 @@ +foo: "root value" +--- +Basic: +{{ foo }} +{% set foo = "new value" %} +{{ foo }} +{% with %} + {% set foo = "new value 2" %} + {{ foo }} +{% endwith %} +{{ foo }} + +Into Loop: +{% for item in [1, 2, 3] %} + {{ item }} + {% set item = item * 2 %} + {{ item }} +{% endfor %} + +Conditional: +{% if true %}{% set foo = "was true" %}{% endif %} +{{ foo }} \ No newline at end of file diff --git a/minijinja/tests/parser-inputs/set.txt b/minijinja/tests/parser-inputs/set.txt new file mode 100644 index 00000000..c266ac00 --- /dev/null +++ b/minijinja/tests/parser-inputs/set.txt @@ -0,0 +1,2 @@ +{% set variable = value %} +{% set (a, b) = (1, 2) %} \ No newline at end of file diff --git a/minijinja/tests/snapshots/test_parser__parser@set.txt.snap b/minijinja/tests/snapshots/test_parser__parser@set.txt.snap new file mode 100644 index 00000000..e6c8118e --- /dev/null +++ b/minijinja/tests/snapshots/test_parser__parser@set.txt.snap @@ -0,0 +1,44 @@ +--- +source: minijinja/tests/test_parser.rs +expression: "&ast" +input_file: minijinja/tests/parser-inputs/set.txt +--- +Ok( + Template { + children: [ + Set { + target: Var { + id: "variable", + } @ 1:7-1:15, + expr: Var { + id: "value", + } @ 1:18-1:23, + } @ 1:3-1:23, + EmitRaw { + raw: "\n", + } @ 1:26-2:0, + Set { + target: List { + items: [ + Var { + id: "a", + } @ 2:8-2:9, + Var { + id: "b", + } @ 2:11-2:12, + ], + } @ 2:7-2:12, + expr: List { + items: [ + Const { + value: 1, + } @ 2:17-2:18, + Const { + value: 2, + } @ 2:20-2:21, + ], + } @ 2:16-2:21, + } @ 2:3-2:22, + ], + } @ 0:0-2:25, +) diff --git a/minijinja/tests/snapshots/test_templates__vm@set.txt.snap b/minijinja/tests/snapshots/test_templates__vm@set.txt.snap new file mode 100644 index 00000000..89a4cb46 --- /dev/null +++ b/minijinja/tests/snapshots/test_templates__vm@set.txt.snap @@ -0,0 +1,34 @@ +--- +source: minijinja/tests/test_templates.rs +expression: "&rendered" +input_file: minijinja/tests/inputs/set.txt +--- +Basic: +root value + +new value + + + new value 2 + +new value + +Into Loop: + + 1 + + 2 + + 2 + + 4 + + 3 + + 6 + + +Conditional: + +was true +