From 1aab2a91eb4df4f94ce231ff92c8d05f0787b1c1 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 8 Nov 2022 15:25:01 +0100 Subject: [PATCH 1/3] Replicate extending logic from Jinja2 --- minijinja/src/compiler/ast.rs | 17 +++- minijinja/src/compiler/codegen.rs | 36 +++++-- minijinja/src/compiler/instructions.rs | 16 +++- minijinja/src/compiler/parser.rs | 5 +- minijinja/src/output.rs | 46 ++++++--- minijinja/src/vm/context.rs | 2 + minijinja/src/vm/mod.rs | 96 ++++++++++++++----- minijinja/src/vm/state.rs | 1 + minijinja/tests/inputs/err_extends_twice.txt | 6 ++ minijinja/tests/inputs/extends_set.txt | 6 ++ minijinja/tests/inputs/macro_extends.txt | 8 ++ .../tests/inputs/refs/layout_with_var.txt | 1 + .../test_templates__vm@debug.txt.snap | 1 + ...t_templates__vm@err_extends_twice.txt.snap | 27 ++++++ ...st_templates__vm@err_self_extends.txt.snap | 1 + .../test_templates__vm@extends_set.txt.snap | 9 ++ .../test_templates__vm@macro_extends.txt.snap | 10 ++ 17 files changed, 231 insertions(+), 57 deletions(-) create mode 100644 minijinja/tests/inputs/err_extends_twice.txt create mode 100644 minijinja/tests/inputs/extends_set.txt create mode 100644 minijinja/tests/inputs/macro_extends.txt create mode 100644 minijinja/tests/inputs/refs/layout_with_var.txt create mode 100644 minijinja/tests/snapshots/test_templates__vm@err_extends_twice.txt.snap create mode 100644 minijinja/tests/snapshots/test_templates__vm@extends_set.txt.snap create mode 100644 minijinja/tests/snapshots/test_templates__vm@macro_extends.txt.snap diff --git a/minijinja/src/compiler/ast.rs b/minijinja/src/compiler/ast.rs index 533d0d63..165ada4a 100644 --- a/minijinja/src/compiler/ast.rs +++ b/minijinja/src/compiler/ast.rs @@ -59,10 +59,11 @@ pub enum Stmt<'a> { WithBlock(Spanned>), Set(Spanned>), SetBlock(Spanned>), - Block(Spanned>), AutoEscape(Spanned>), FilterBlock(Spanned>), #[cfg(feature = "multi-template")] + Block(Spanned>), + #[cfg(feature = "multi-template")] Import(Spanned>), #[cfg(feature = "multi-template")] FromImport(Spanned>), @@ -86,10 +87,11 @@ impl<'a> fmt::Debug for Stmt<'a> { Stmt::WithBlock(s) => fmt::Debug::fmt(s, f), Stmt::Set(s) => fmt::Debug::fmt(s, f), Stmt::SetBlock(s) => fmt::Debug::fmt(s, f), - Stmt::Block(s) => fmt::Debug::fmt(s, f), Stmt::AutoEscape(s) => fmt::Debug::fmt(s, f), Stmt::FilterBlock(s) => fmt::Debug::fmt(s, f), #[cfg(feature = "multi-template")] + Stmt::Block(s) => fmt::Debug::fmt(s, f), + #[cfg(feature = "multi-template")] Stmt::Extends(s) => fmt::Debug::fmt(s, f), #[cfg(feature = "multi-template")] Stmt::Include(s) => fmt::Debug::fmt(s, f), @@ -193,6 +195,7 @@ pub struct SetBlock<'a> { /// A block for inheritance elements. #[cfg_attr(feature = "internal_debug", derive(Debug))] +#[cfg(feature = "multi-template")] pub struct Block<'a> { pub name: &'a str, pub body: Vec>, @@ -458,6 +461,7 @@ impl<'a> Map<'a> { pub enum CallType<'ast, 'source> { Function(&'source str), Method(&'ast Expr<'source>, &'source str), + #[cfg(feature = "multi-template")] Block(&'source str), Object(&'ast Expr<'source>), } @@ -472,9 +476,12 @@ impl<'a> Call<'a> { match self.expr { Expr::Var(ref var) => CallType::Function(var.id), Expr::GetAttr(ref attr) => { - if let Expr::Var(ref var) = attr.expr { - if var.id == "self" { - return CallType::Block(attr.name); + #[cfg(feature = "multi-template")] + { + if let Expr::Var(ref var) = attr.expr { + if var.id == "self" { + return CallType::Block(attr.name); + } } } CallType::Method(&attr.expr, attr.name) diff --git a/minijinja/src/compiler/codegen.rs b/minijinja/src/compiler/codegen.rs index 42d41989..bebbb61d 100644 --- a/minijinja/src/compiler/codegen.rs +++ b/minijinja/src/compiler/codegen.rs @@ -6,6 +6,7 @@ use crate::compiler::instructions::{ }; use crate::compiler::tokens::Span; use crate::error::Error; +use crate::output::CaptureMode; use crate::value::Value; #[cfg(test)] @@ -41,6 +42,8 @@ pub struct CodeGenerator<'source> { span_stack: Vec, filter_local_ids: BTreeMap<&'source str, LocalId>, test_local_ids: BTreeMap<&'source str, LocalId>, + #[cfg(feature = "multi-template")] + has_extends: bool, } impl<'source> CodeGenerator<'source> { @@ -54,6 +57,8 @@ impl<'source> CodeGenerator<'source> { span_stack: Vec::new(), filter_local_ids: BTreeMap::new(), test_local_ids: BTreeMap::new(), + #[cfg(feature = "multi-template")] + has_extends: false, } } @@ -99,6 +104,7 @@ impl<'source> CodeGenerator<'source> { } /// Creats a sub generator. + #[cfg(feature = "multi-template")] fn new_subgenerator(&self) -> CodeGenerator<'source> { let mut sub = CodeGenerator::new(self.instructions.name(), self.instructions.source()); sub.current_line = self.current_line; @@ -107,6 +113,7 @@ impl<'source> CodeGenerator<'source> { } /// Finishes a sub generator and syncs it back. + #[cfg(feature = "multi-template")] fn finish_subgenerator(&mut self, sub: CodeGenerator<'source>) -> Instructions<'source> { self.current_line = sub.current_line; let (instructions, blocks) = sub.finish(); @@ -226,6 +233,12 @@ impl<'source> CodeGenerator<'source> { for node in &t.children { ok!(self.compile_stmt(node)); } + #[cfg(feature = "multi-template")] + { + if self.has_extends { + self.add(Instruction::RenderParent); + } + } } ast::Stmt::EmitExpr(expr) => { ok!(self.compile_emit_expr(expr)); @@ -259,7 +272,7 @@ impl<'source> CodeGenerator<'source> { } ast::Stmt::SetBlock(set_block) => { self.set_line_from_span(set_block.span()); - self.add(Instruction::BeginCapture); + self.add(Instruction::BeginCapture(CaptureMode::Capture)); for node in &set_block.body { ok!(self.compile_stmt(node)); } @@ -269,9 +282,6 @@ impl<'source> CodeGenerator<'source> { } ok!(self.compile_assignment(&set_block.target)); } - ast::Stmt::Block(block) => { - ok!(self.compile_block(block)); - } ast::Stmt::AutoEscape(auto_escape) => { self.set_line_from_span(auto_escape.span()); ok!(self.compile_expr(&auto_escape.enabled)); @@ -283,7 +293,7 @@ impl<'source> CodeGenerator<'source> { } ast::Stmt::FilterBlock(filter_block) => { self.set_line_from_span(filter_block.span()); - self.add(Instruction::BeginCapture); + self.add(Instruction::BeginCapture(CaptureMode::Capture)); for node in &filter_block.body { ok!(self.compile_stmt(node)); } @@ -292,8 +302,12 @@ impl<'source> CodeGenerator<'source> { self.add(Instruction::Emit); } #[cfg(feature = "multi-template")] + ast::Stmt::Block(block) => { + ok!(self.compile_block(block)); + } + #[cfg(feature = "multi-template")] ast::Stmt::Import(import) => { - self.add(Instruction::BeginCapture); + self.add(Instruction::BeginCapture(CaptureMode::Discard)); self.add(Instruction::PushWith); ok!(self.compile_expr(&import.expr)); self.add_with_span(Instruction::Include(false), import.span()); @@ -304,7 +318,7 @@ impl<'source> CodeGenerator<'source> { } #[cfg(feature = "multi-template")] ast::Stmt::FromImport(from_import) => { - self.add(Instruction::BeginCapture); + self.add(Instruction::BeginCapture(CaptureMode::Discard)); self.add(Instruction::PushWith); ok!(self.compile_expr(&from_import.expr)); self.add_with_span(Instruction::Include(false), from_import.span()); @@ -321,7 +335,8 @@ impl<'source> CodeGenerator<'source> { ast::Stmt::Extends(extends) => { self.set_line_from_span(extends.span()); ok!(self.compile_expr(&extends.name)); - self.add(Instruction::LoadBlocks); + self.add_with_span(Instruction::LoadBlocks, extends.span()); + self.has_extends = true; } #[cfg(feature = "multi-template")] ast::Stmt::Include(include) => { @@ -337,6 +352,7 @@ impl<'source> CodeGenerator<'source> { Ok(()) } + #[cfg(feature = "multi-template")] fn compile_block(&mut self, block: &ast::Spanned>) -> Result<(), Error> { self.set_line_from_span(block.span()); let mut sub = self.new_subgenerator(); @@ -450,6 +466,7 @@ impl<'source> CodeGenerator<'source> { return Ok(()); } } + #[cfg(feature = "multi-template")] ast::CallType::Block(name) => { self.add(Instruction::CallBlock(name)); return Ok(()); @@ -664,8 +681,9 @@ impl<'source> CodeGenerator<'source> { } self.add(Instruction::CallFunction(name, c.args.len())); } + #[cfg(feature = "multi-template")] ast::CallType::Block(name) => { - self.add(Instruction::BeginCapture); + self.add(Instruction::BeginCapture(CaptureMode::Capture)); self.add(Instruction::CallBlock(name)); self.add(Instruction::EndCapture); } diff --git a/minijinja/src/compiler/instructions.rs b/minijinja/src/compiler/instructions.rs index 280c057b..d4df9de6 100644 --- a/minijinja/src/compiler/instructions.rs +++ b/minijinja/src/compiler/instructions.rs @@ -5,6 +5,7 @@ use std::fmt; use similar_asserts::assert_eq; use crate::compiler::tokens::Span; +use crate::output::CaptureMode; use crate::value::Value; /// This loop has the loop var. @@ -152,17 +153,14 @@ pub enum Instruction<'source> { /// Jump if the stack top evaluates to true or pops the value JumpIfTrueOrPop(usize), - /// Call into a block. - CallBlock(&'source str), - /// Sets the auto escape flag to the current value. PushAutoEscape, /// Resets the auto escape flag to the previous value. PopAutoEscape, - /// Begins capturing of output. - BeginCapture, + /// Begins capturing of output (false) or discard (true). + BeginCapture(CaptureMode), /// Ends capturing of output. EndCapture, @@ -188,10 +186,18 @@ pub enum Instruction<'source> { /// A fast loop recurse instruction without intermediate capturing. FastRecurse, + /// Call into a block. + #[cfg(feature = "multi-template")] + CallBlock(&'source str), + /// Loads block from a template with name on stack ("extends") #[cfg(feature = "multi-template")] LoadBlocks, + /// Renders the parent template + #[cfg(feature = "multi-template")] + RenderParent, + /// Includes another template. #[cfg(feature = "multi-template")] Include(bool), diff --git a/minijinja/src/compiler/parser.rs b/minijinja/src/compiler/parser.rs index 271a4378..0127ba50 100644 --- a/minijinja/src/compiler/parser.rs +++ b/minijinja/src/compiler/parser.rs @@ -150,6 +150,7 @@ impl<'a> TokenStream<'a> { struct Parser<'a> { stream: TokenStream<'a>, + #[allow(unused)] in_macro: bool, depth: usize, } @@ -638,7 +639,6 @@ impl<'a> Parser<'a> { SetParseResult::Set(rv) => ast::Stmt::Set(respan!(rv)), SetParseResult::SetBlock(rv) => ast::Stmt::SetBlock(respan!(rv)), }, - Token::Ident("block") => ast::Stmt::Block(respan!(ok!(self.parse_block()))), Token::Ident("autoescape") => { ast::Stmt::AutoEscape(respan!(ok!(self.parse_auto_escape()))) } @@ -646,6 +646,8 @@ impl<'a> Parser<'a> { ast::Stmt::FilterBlock(respan!(ok!(self.parse_filter_block()))) } #[cfg(feature = "multi-template")] + Token::Ident("block") => ast::Stmt::Block(respan!(ok!(self.parse_block()))), + #[cfg(feature = "multi-template")] Token::Ident("extends") => ast::Stmt::Extends(respan!(ok!(self.parse_extends()))), #[cfg(feature = "multi-template")] Token::Ident("include") => ast::Stmt::Include(respan!(ok!(self.parse_include()))), @@ -820,6 +822,7 @@ impl<'a> Parser<'a> { } } + #[cfg(feature = "multi-template")] fn parse_block(&mut self) -> Result, Error> { if self.in_macro { syntax_error!("block tags in macros are not allowed"); diff --git a/minijinja/src/output.rs b/minijinja/src/output.rs index dc621c1c..73755e1f 100644 --- a/minijinja/src/output.rs +++ b/minijinja/src/output.rs @@ -3,6 +3,14 @@ use std::{fmt, io}; use crate::utils::AutoEscape; use crate::value::Value; +/// How should output be captured? +#[derive(Debug, Clone, Copy)] +pub enum CaptureMode { + Capture, + #[allow(unused)] + Discard, +} + /// An abstraction over [`Write`](std::fmt::Write) for the rendering. /// /// This is a utility type used in the engine which can be written into like one @@ -10,7 +18,7 @@ use crate::value::Value; /// in the engine but it's also passed to the custom formatter function. pub struct Output<'a> { w: &'a mut (dyn fmt::Write + 'a), - capture_stack: Vec, + capture_stack: Vec>, } impl<'a> Output<'a> { @@ -31,33 +39,38 @@ impl<'a> Output<'a> { /// Creates a null output that writes nowhere. pub(crate) fn null() -> Self { - static mut NULL_WRITER: NullWriter = NullWriter; Self { - // SAFETY: this is safe as the null writer is a ZST - w: unsafe { &mut NULL_WRITER }, + w: NullWriter::get_mut(), capture_stack: Vec::new(), } } - /// Begins capturing into a string. - pub(crate) fn begin_capture(&mut self) { - self.capture_stack.push(String::new()); + /// Begins capturing into a string or discard. + pub(crate) fn begin_capture(&mut self, mode: CaptureMode) { + self.capture_stack.push(match mode { + CaptureMode::Capture => Some(String::new()), + CaptureMode::Discard => None, + }); } /// Ends capturing and returns the captured string as value. pub(crate) fn end_capture(&mut self, auto_escape: AutoEscape) -> Value { - let captured = self.capture_stack.pop().unwrap(); - if !matches!(auto_escape, AutoEscape::None) { - Value::from_safe_string(captured) + if let Some(captured) = self.capture_stack.pop().unwrap() { + if !matches!(auto_escape, AutoEscape::None) { + Value::from_safe_string(captured) + } else { + Value::from(captured) + } } else { - Value::from(captured) + Value::UNDEFINED } } #[inline(always)] fn target(&mut self) -> &mut dyn fmt::Write { match self.capture_stack.last_mut() { - Some(stream) => stream as _, + Some(Some(stream)) => stream as _, + Some(None) => NullWriter::get_mut(), None => self.w, } } @@ -94,6 +107,15 @@ impl fmt::Write for Output<'_> { pub struct NullWriter; +impl NullWriter { + /// Returns a reference to the null writer. + pub fn get_mut() -> &'static mut NullWriter { + static mut NULL_WRITER: NullWriter = NullWriter; + // SAFETY: this is safe as the null writer is a ZST + unsafe { &mut NULL_WRITER } + } +} + impl fmt::Write for NullWriter { #[inline] fn write_str(&mut self, _s: &str) -> fmt::Result { diff --git a/minijinja/src/vm/context.rs b/minijinja/src/vm/context.rs index cc3606d4..7d5c42e2 100644 --- a/minijinja/src/vm/context.rs +++ b/minijinja/src/vm/context.rs @@ -231,6 +231,7 @@ impl<'env> Context<'env> { } /// Increase the stack depth. + #[allow(unused)] pub fn incr_depth(&mut self, delta: usize) -> Result<(), Error> { self.check_depth()?; self.outer_stack_depth += delta; @@ -238,6 +239,7 @@ impl<'env> Context<'env> { } /// Decrease the stack depth. + #[allow(unused)] pub fn decr_depth(&mut self, delta: usize) { self.outer_stack_depth -= delta; } diff --git a/minijinja/src/vm/mod.rs b/minijinja/src/vm/mod.rs index 8163b846..397e352a 100644 --- a/minijinja/src/vm/mod.rs +++ b/minijinja/src/vm/mod.rs @@ -8,7 +8,7 @@ use crate::compiler::instructions::{ }; use crate::environment::Environment; use crate::error::{Error, ErrorKind}; -use crate::output::Output; +use crate::output::{CaptureMode, Output}; use crate::utils::AutoEscape; use crate::value::{self, ops, MapType, Value, ValueMap, ValueRepr}; use crate::vm::context::{Context, Frame, LoopState, Stack}; @@ -27,6 +27,7 @@ mod macro_object; mod state; // the cost of a single include against the stack limit. +#[cfg(feature = "multi-template")] const INCLUDE_RECURSION_COST: usize = 10; /// Helps to evaluate something. @@ -151,6 +152,12 @@ impl<'env> Vm<'env> { let mut loaded_filters = [None; MAX_LOCALS]; let mut loaded_tests = [None; MAX_LOCALS]; + // If we are extending we are holding the instructions of the target parent + // template here. This is used to detect multiple extends and the evaluation + // uses these instructions when RenderParent is evaluated. + #[cfg(feature = "multi-template")] + let mut parent_instructions = None; + macro_rules! recurse_loop { ($capture:expr) => {{ let jump_target = ctx_ok!(self.prepare_loop_recursion(state)); @@ -160,7 +167,7 @@ impl<'env> Vm<'env> { // to. next_loop_recursion_jump = Some((pc + 1, $capture)); if $capture { - out.begin_capture(); + out.begin_capture(CaptureMode::Capture); } pc = jump_target; continue; @@ -377,24 +384,27 @@ impl<'env> Vm<'env> { stack.pop(); } } + #[cfg(feature = "multi-template")] Instruction::CallBlock(name) => { - let old_block = state.current_block; - state.current_block = Some(name); - if let Some(block_stack) = state.blocks.get(name) { - let old_instructions = - mem::replace(&mut state.instructions, block_stack.instructions()); - ctx_ok!(state.ctx.push_frame(Frame::default())); - let rv = self.eval_state(state, out); - state.ctx.pop_frame(); - state.instructions = old_instructions; - ctx_ok!(rv); - } else { - bail!(Error::new( - ErrorKind::InvalidOperation, - "tried to invoke unknown block" - )); + if parent_instructions.is_none() { + let old_block = state.current_block; + state.current_block = Some(name); + if let Some(block_stack) = state.blocks.get(name) { + let old_instructions = + mem::replace(&mut state.instructions, block_stack.instructions()); + ctx_ok!(state.ctx.push_frame(Frame::default())); + let rv = self.eval_state(state, out); + state.ctx.pop_frame(); + state.instructions = old_instructions; + ctx_ok!(rv); + } else { + bail!(Error::new( + ErrorKind::InvalidOperation, + "tried to invoke unknown block" + )); + } + state.current_block = old_block; } - state.current_block = old_block; } Instruction::PushAutoEscape => { a = stack.pop(); @@ -404,8 +414,8 @@ impl<'env> Vm<'env> { Instruction::PopAutoEscape => { state.auto_escape = auto_escape_stack.pop().unwrap(); } - Instruction::BeginCapture => { - out.begin_capture(); + Instruction::BeginCapture(mode) => { + out.begin_capture(*mode); } Instruction::EndCapture => { stack.push(out.end_capture(state.auto_escape)); @@ -494,10 +504,43 @@ impl<'env> Vm<'env> { Instruction::FastRecurse => { recurse_loop!(false); } + // Explanation on the behavior of `LoadBlocks` and `RenderParent`. + // MiniJinja inherits the behavior from Jinja2 where extending + // loads the blocks (`LoadBlocks`) and the rest of the template + // keeps executing but with output disabled, only at the end the + // parent template is then invoked (`RenderParent`). This has the + // effect that you can still set variables or declare macros and + // that they become visible in the blocks. + // + // This behavior has a few downsides. First of all what happens + // in the parent template overrides what happens in the child. + // For instance if you declare a macro named `foo` after `{% + // extends %}` and then a variable with that named is also set + // in the parent template, then you won't be able to call that + // macro in the body. + // + // The reason for this is that blocks unlike macros do not have + // closures in Jinja2/MiniJinja. + // + // However for the common case this is convenient because it + // lets you put some imports there and for as long as you do not + // create name clashes this works fine. #[cfg(feature = "multi-template")] Instruction::LoadBlocks => { a = stack.pop(); - ctx_ok!(self.load_blocks(a, state)); + if parent_instructions.is_some() { + bail!(Error::new( + ErrorKind::InvalidOperation, + "tried to extend a second time in a template" + )); + } + parent_instructions = Some(ctx_ok!(self.load_blocks(a, state))); + out.begin_capture(CaptureMode::Discard); + } + #[cfg(feature = "multi-template")] + Instruction::RenderParent => { + out.end_capture(AutoEscape::None); + state.instructions = parent_instructions.take().unwrap(); // then replace the instructions and set the pc to 0 again. // this effectively means that the template engine will now @@ -622,7 +665,7 @@ impl<'env> Vm<'env> { } if capture { - out.begin_capture(); + out.begin_capture(CaptureMode::Capture); } let old_instructions = mem::replace(&mut state.instructions, block_stack.instructions()); @@ -661,7 +704,11 @@ impl<'env> Vm<'env> { } #[cfg(feature = "multi-template")] - fn load_blocks(&self, name: Value, state: &mut State<'_, 'env>) -> Result<(), Error> { + fn load_blocks( + &self, + name: Value, + state: &mut State<'_, 'env>, + ) -> Result<&'env Instructions<'env>, Error> { let name = match name.as_str() { Some(name) => name, None => { @@ -689,8 +736,7 @@ impl<'env> Vm<'env> { .or_insert_with(BlockStack::default) .append_instructions(instr); } - state.instructions = tmpl.instructions(); - Ok(()) + Ok(tmpl.instructions()) } fn derive_auto_escape( diff --git a/minijinja/src/vm/state.rs b/minijinja/src/vm/state.rs index af5c2ba2..29f80d13 100644 --- a/minijinja/src/vm/state.rs +++ b/minijinja/src/vm/state.rs @@ -27,6 +27,7 @@ pub struct State<'vm, 'env> { pub(crate) auto_escape: AutoEscape, pub(crate) instructions: &'vm Instructions<'env>, pub(crate) blocks: BTreeMap<&'env str, BlockStack<'vm, 'env>>, + #[allow(unused)] pub(crate) loaded_templates: BTreeSet<&'env str>, #[cfg(feature = "macros")] pub(crate) macros: std::sync::Arc, usize)>>, diff --git a/minijinja/tests/inputs/err_extends_twice.txt b/minijinja/tests/inputs/err_extends_twice.txt new file mode 100644 index 00000000..b26b0765 --- /dev/null +++ b/minijinja/tests/inputs/err_extends_twice.txt @@ -0,0 +1,6 @@ +{ + "template": "simple_layout.txt" +} +--- +{% extends template %} +{% extends template %} diff --git a/minijinja/tests/inputs/extends_set.txt b/minijinja/tests/inputs/extends_set.txt new file mode 100644 index 00000000..fb4d3c6c --- /dev/null +++ b/minijinja/tests/inputs/extends_set.txt @@ -0,0 +1,6 @@ +{ + "template": "layout_with_var.txt" +} +--- +{% extends template %} +{% set var = "the value from the child" %} diff --git a/minijinja/tests/inputs/macro_extends.txt b/minijinja/tests/inputs/macro_extends.txt new file mode 100644 index 00000000..1f44acda --- /dev/null +++ b/minijinja/tests/inputs/macro_extends.txt @@ -0,0 +1,8 @@ +{ + "template": "simple_layout.txt" +} +--- +{%- extends template %} +{%- macro foo() %}inside foo{% endmacro %} +{%- block title %}{{ foo() }}{% endblock %} +{%- block body %}new body{% endblock %} \ No newline at end of file diff --git a/minijinja/tests/inputs/refs/layout_with_var.txt b/minijinja/tests/inputs/refs/layout_with_var.txt new file mode 100644 index 00000000..c0714033 --- /dev/null +++ b/minijinja/tests/inputs/refs/layout_with_var.txt @@ -0,0 +1 @@ +{% block body %}{{ var }}{% endblock %} diff --git a/minijinja/tests/snapshots/test_templates__vm@debug.txt.snap b/minijinja/tests/snapshots/test_templates__vm@debug.txt.snap index 84539252..dbbe26b3 100644 --- a/minijinja/tests/snapshots/test_templates__vm@debug.txt.snap +++ b/minijinja/tests/snapshots/test_templates__vm@debug.txt.snap @@ -78,6 +78,7 @@ State { "debug.txt", "example_macro.txt", "include_with_var_and_macro.txt", + "layout_with_var.txt", "self-extends.txt", "self-include.txt", "simple2_layout.txt", diff --git a/minijinja/tests/snapshots/test_templates__vm@err_extends_twice.txt.snap b/minijinja/tests/snapshots/test_templates__vm@err_extends_twice.txt.snap new file mode 100644 index 00000000..42312250 --- /dev/null +++ b/minijinja/tests/snapshots/test_templates__vm@err_extends_twice.txt.snap @@ -0,0 +1,27 @@ +--- +source: minijinja/tests/test_templates.rs +description: "{% extends template %}\n{% extends template %}" +info: + template: simple_layout.txt +input_file: minijinja/tests/inputs/err_extends_twice.txt +--- +!!!ERROR!!! + +Error { + kind: InvalidOperation, + detail: "tried to extend a second time in a template", + name: "err_extends_twice.txt", + line: 2, +} + +invalid operation: tried to extend a second time in a template (in err_extends_twice.txt:2) +---------------------------- err_extends_twice.txt ---------------------------- + 1 | {% extends template %} + 2 > {% extends template %} + i ^^^^^^^^^^^^^^^^ invalid operation +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Referenced variables: { + template: "simple_layout.txt", +} +------------------------------------------------------------------------------- + diff --git a/minijinja/tests/snapshots/test_templates__vm@err_self_extends.txt.snap b/minijinja/tests/snapshots/test_templates__vm@err_self_extends.txt.snap index 821e4675..63c0ca3c 100644 --- a/minijinja/tests/snapshots/test_templates__vm@err_self_extends.txt.snap +++ b/minijinja/tests/snapshots/test_templates__vm@err_self_extends.txt.snap @@ -16,6 +16,7 @@ Error { invalid operation: cycle in template inheritance. "self-extends.txt" was referenced more than once (in self-extends.txt:1) ------------------------------ self-extends.txt ------------------------------- 1 > {% extends "self-extends.txt" %} + i ^^^^^^^^^^^^^^^^^^^^^^^^^^ invalid operation 2 | {% block wat %}{% endblock %} ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ No referenced variables diff --git a/minijinja/tests/snapshots/test_templates__vm@extends_set.txt.snap b/minijinja/tests/snapshots/test_templates__vm@extends_set.txt.snap new file mode 100644 index 00000000..82497b90 --- /dev/null +++ b/minijinja/tests/snapshots/test_templates__vm@extends_set.txt.snap @@ -0,0 +1,9 @@ +--- +source: minijinja/tests/test_templates.rs +description: "{% extends template %}\n{% set var = \"the value from the child\" %}" +info: + template: layout_with_var.txt +input_file: minijinja/tests/inputs/extends_set.txt +--- +the value from the child + diff --git a/minijinja/tests/snapshots/test_templates__vm@macro_extends.txt.snap b/minijinja/tests/snapshots/test_templates__vm@macro_extends.txt.snap new file mode 100644 index 00000000..9e2d8ec2 --- /dev/null +++ b/minijinja/tests/snapshots/test_templates__vm@macro_extends.txt.snap @@ -0,0 +1,10 @@ +--- +source: minijinja/tests/test_templates.rs +description: "{%- extends template %}\n{%- macro foo() %}inside foo{% endmacro %}\n{%- block title %}{{ foo() }}{% endblock %}\n{%- block body %}new body{% endblock %}" +info: + template: simple_layout.txt +input_file: minijinja/tests/inputs/macro_extends.txt +--- +inside foo +new body + From c6bdc311fb3df49d7c2d7bbd0c421d2f358de6f2 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 8 Nov 2022 15:55:20 +0100 Subject: [PATCH 2/3] Added test for block scopes --- minijinja/tests/inputs/block_scope_extends.txt | 6 ++++++ .../tests/inputs/refs/var_referencing_layout.txt | 3 +++ ...est_templates__vm@block_scope_extends.txt.snap | 15 +++++++++++++++ .../snapshots/test_templates__vm@debug.txt.snap | 1 + 4 files changed, 25 insertions(+) create mode 100644 minijinja/tests/inputs/block_scope_extends.txt create mode 100644 minijinja/tests/inputs/refs/var_referencing_layout.txt create mode 100644 minijinja/tests/snapshots/test_templates__vm@block_scope_extends.txt.snap diff --git a/minijinja/tests/inputs/block_scope_extends.txt b/minijinja/tests/inputs/block_scope_extends.txt new file mode 100644 index 00000000..7bbe396a --- /dev/null +++ b/minijinja/tests/inputs/block_scope_extends.txt @@ -0,0 +1,6 @@ +{ + "template": "var_referencing_layout.txt" +} +--- +{% extends template %} +{% block item_block %}[{{ item }}]{% endblock %} diff --git a/minijinja/tests/inputs/refs/var_referencing_layout.txt b/minijinja/tests/inputs/refs/var_referencing_layout.txt new file mode 100644 index 00000000..e7e3739d --- /dev/null +++ b/minijinja/tests/inputs/refs/var_referencing_layout.txt @@ -0,0 +1,3 @@ +{% for item in [1, 2, 3] %} + {% block item_block %}{{ item }}{% endblock %} +{% endfor %} \ No newline at end of file diff --git a/minijinja/tests/snapshots/test_templates__vm@block_scope_extends.txt.snap b/minijinja/tests/snapshots/test_templates__vm@block_scope_extends.txt.snap new file mode 100644 index 00000000..7565fd70 --- /dev/null +++ b/minijinja/tests/snapshots/test_templates__vm@block_scope_extends.txt.snap @@ -0,0 +1,15 @@ +--- +source: minijinja/tests/test_templates.rs +description: "{% extends template %}\n{% block item_block %}[{{ item }}]{% endblock %}" +info: + template: var_referencing_layout.txt +input_file: minijinja/tests/inputs/block_scope_extends.txt +--- + + [1] + + [2] + + [3] + + diff --git a/minijinja/tests/snapshots/test_templates__vm@debug.txt.snap b/minijinja/tests/snapshots/test_templates__vm@debug.txt.snap index dbbe26b3..0a6efd62 100644 --- a/minijinja/tests/snapshots/test_templates__vm@debug.txt.snap +++ b/minijinja/tests/snapshots/test_templates__vm@debug.txt.snap @@ -85,6 +85,7 @@ State { "simple_include.txt", "simple_layout.txt", "super_with_html.html", + "var_referencing_layout.txt", "var_setting_layout.txt", ], }, From 6f341b34a25ac20361b96a619ea0b322454d1d0e Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 8 Nov 2022 22:00:53 +0100 Subject: [PATCH 3/3] Fixed a broken test --- minijinja/src/compiler/meta.rs | 13 +++++++------ minijinja/src/vm/mod.rs | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/minijinja/src/compiler/meta.rs b/minijinja/src/compiler/meta.rs index d7950e34..6fe832aa 100644 --- a/minijinja/src/compiler/meta.rs +++ b/minijinja/src/compiler/meta.rs @@ -133,12 +133,6 @@ pub fn find_macro_closure<'a>(m: &ast::Macro<'a>) -> HashSet<&'a str> { assign_nested(&stmt.target, state); visit_expr(&stmt.expr, state); } - ast::Stmt::Block(stmt) => { - state.push(); - state.assign("super"); - stmt.body.iter().for_each(|x| walk(x, state)); - state.pop(); - } ast::Stmt::AutoEscape(stmt) => { state.push(); stmt.body.iter().for_each(|x| walk(x, state)); @@ -156,6 +150,13 @@ pub fn find_macro_closure<'a>(m: &ast::Macro<'a>) -> HashSet<&'a str> { state.pop(); } #[cfg(feature = "multi-template")] + ast::Stmt::Block(stmt) => { + state.push(); + state.assign("super"); + stmt.body.iter().for_each(|x| walk(x, state)); + state.pop(); + } + #[cfg(feature = "multi-template")] ast::Stmt::Extends(_) | ast::Stmt::Include(_) => {} #[cfg(feature = "multi-template")] ast::Stmt::Import(stmt) => { diff --git a/minijinja/src/vm/mod.rs b/minijinja/src/vm/mod.rs index 397e352a..68e71684 100644 --- a/minijinja/src/vm/mod.rs +++ b/minijinja/src/vm/mod.rs @@ -511,7 +511,7 @@ impl<'env> Vm<'env> { // parent template is then invoked (`RenderParent`). This has the // effect that you can still set variables or declare macros and // that they become visible in the blocks. - // + // // This behavior has a few downsides. First of all what happens // in the parent template overrides what happens in the child. // For instance if you declare a macro named `foo` after `{%