Repository: mitsuhiko/minijinja
Pull request #686 (2025-01-28 11:43:30)

Fix undefineds in some places not reporting proper errors
MiniJinja did not report proper information about what is undefined

## Before

```
undefined value (in err_strict_undefined_if.txt:1)
------------------------- err_strict_undefined_if.txt -------------------------
   1 > {% if undefined_value %}
   2 |     ...
   3 | {% endif %}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
No referenced variables
-------------------------------------------------------------------------------
```

## After

```
undefined value (in err_strict_undefined_if.txt:1)
------------------------- err_strict_undefined_if.txt -------------------------
   1 > {% if undefined_value %}
     i       ^^^^^^^^^^^^^^^ undefined value
   2 |     ...
   3 | {% endif %}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
No referenced variables
-------------------------------------------------------------------------------
```



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index cbb418b..3ee4b6d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,8 @@ All notable changes to MiniJinja are documented here.
 - Fixed an issue that caused loop aliasing not to be supported for
   recursive loops.  #678
 - CLI moved from `serde_yml` to `serde_yaml`.  #684
+- Improved undefined error reporting.  Undefined values will now in most
+  cases point to exactly where the error happened.  #686
 
 ## 2.6.0
 
diff --git a/minijinja/src/compiler/ast.rs b/minijinja/src/compiler/ast.rs
index 9c269a7..313b788 100644
--- a/minijinja/src/compiler/ast.rs
+++ b/minijinja/src/compiler/ast.rs
@@ -186,6 +186,24 @@ impl Expr<'_> {
             Expr::Filter(_) => "filter expression",
         }
     }
+
+    pub fn span(&self) -> Span {
+        match self {
+            Expr::Var(s) => s.span(),
+            Expr::Const(s) => s.span(),
+            Expr::Slice(s) => s.span(),
+            Expr::UnaryOp(s) => s.span(),
+            Expr::BinOp(s) => s.span(),
+            Expr::IfExpr(s) => s.span(),
+            Expr::Filter(s) => s.span(),
+            Expr::Test(s) => s.span(),
+            Expr::GetAttr(s) => s.span(),
+            Expr::GetItem(s) => s.span(),
+            Expr::Call(s) => s.span(),
+            Expr::List(s) => s.span(),
+            Expr::Map(s) => s.span(),
+        }
+    }
 }
 
 /// Root template node.
diff --git a/minijinja/src/compiler/codegen.rs b/minijinja/src/compiler/codegen.rs
index 89e9949..db718be 100644
--- a/minijinja/src/compiler/codegen.rs
+++ b/minijinja/src/compiler/codegen.rs
@@ -480,8 +480,10 @@ impl<'source> CodeGenerator<'source> {
 
     fn compile_if_stmt(&mut self, if_cond: &ast::Spanned<ast::IfCond<'source>>) {
         self.set_line_from_span(if_cond.span());
+        self.push_span(if_cond.expr.span());
         self.compile_expr(&if_cond.expr);
         self.start_if();
+        self.pop_span();
         for node in &if_cond.true_body {
             self.compile_stmt(node);
         }
@@ -495,8 +497,8 @@ impl<'source> CodeGenerator<'source> {
     }
 
     fn compile_emit_expr(&mut self, expr: &ast::Spanned<ast::EmitExpr<'source>>) {
-        self.set_line_from_span(expr.span());
         if let ast::Expr::Call(call) = &expr.expr {
+            self.set_line_from_span(expr.expr.span());
             match call.identify_call() {
                 ast::CallType::Function(name) => {
                     if name == "super" && call.args.is_empty() {
@@ -516,8 +518,10 @@ impl<'source> CodeGenerator<'source> {
                 _ => {}
             }
         }
+        self.push_span(expr.expr.span());
         self.compile_expr(&expr.expr);
         self.add(Instruction::Emit);
+        self.pop_span();
     }
 
     fn compile_for_loop(&mut self, for_loop: &ast::Spanned<ast::ForLoop<'source>>) {
@@ -529,6 +533,7 @@ impl<'source> CodeGenerator<'source> {
         // iterated over normally
         if let Some(ref filter_expr) = for_loop.filter_expr {
             self.add(Instruction::LoadConst(Value::from(0usize)));
+            self.push_span(filter_expr.span());
             self.compile_expr(&for_loop.iter);
             self.start_for_loop(false, false);
             self.add(Instruction::DupTop);
@@ -541,13 +546,17 @@ impl<'source> CodeGenerator<'source> {
             self.start_else();
             self.add(Instruction::DiscardTop);
             self.end_if();
+            self.pop_span();
             self.end_for_loop(false);
             self.add(Instruction::BuildList(None));
+            self.start_for_loop(true, for_loop.recursive);
         } else {
+            self.push_span(for_loop.iter.span());
             self.compile_expr(&for_loop.iter);
+            self.start_for_loop(true, for_loop.recursive);
+            self.pop_span();
         }
 
-        self.start_for_loop(true, for_loop.recursive);
         self.compile_assignment(&for_loop.target);
         for node in &for_loop.body {
             self.compile_stmt(node);


Test patch:
diff --git a/minijinja/tests/inputs/err_strict_undefined_for.txt b/minijinja/tests/inputs/err_strict_undefined_for.txt
new file mode 100644
index 0000000..ab4feba
--- /dev/null
+++ b/minijinja/tests/inputs/err_strict_undefined_for.txt
@@ -0,0 +1,5 @@
+{"$settings": {"undefined": "strict"}}
+---
+{% for item in undefined_value %}
+    ...
+{% endfor %}
\ No newline at end of file
diff --git a/minijinja/tests/inputs/err_strict_undefined_for_filter.txt b/minijinja/tests/inputs/err_strict_undefined_for_filter.txt
new file mode 100644
index 0000000..d7695d6
--- /dev/null
+++ b/minijinja/tests/inputs/err_strict_undefined_for_filter.txt
@@ -0,0 +1,5 @@
+{"$settings": {"undefined": "strict"}}
+---
+{% for item in [undefined_value] if item %}
+    ...
+{% endfor %}
\ No newline at end of file
diff --git a/minijinja/tests/inputs/err_strict_undefined_if.txt b/minijinja/tests/inputs/err_strict_undefined_if.txt
new file mode 100644
index 0000000..c76fbe6
--- /dev/null
+++ b/minijinja/tests/inputs/err_strict_undefined_if.txt
@@ -0,0 +1,5 @@
+{"$settings": {"undefined": "strict"}}
+---
+{% if undefined_value %}
+    ...
+{% endif %}
\ No newline at end of file
diff --git a/minijinja/tests/inputs/err_strict_undefined_print.txt b/minijinja/tests/inputs/err_strict_undefined_print.txt
new file mode 100644
index 0000000..cf416c8
--- /dev/null
+++ b/minijinja/tests/inputs/err_strict_undefined_print.txt
@@ -0,0 +1,3 @@
+{"$settings": {"undefined": "strict"}}
+---
+{{ undefined_value }}
\ No newline at end of file
diff --git a/minijinja/tests/snapshots/test_templates__vm@err_strict_undefined_for.txt.snap b/minijinja/tests/snapshots/test_templates__vm@err_strict_undefined_for.txt.snap
new file mode 100644
index 0000000..7de8374
--- /dev/null
+++ b/minijinja/tests/snapshots/test_templates__vm@err_strict_undefined_for.txt.snap
@@ -0,0 +1,25 @@
+---
+source: minijinja/tests/test_templates.rs
+description: "{% for item in undefined_value %}\n    ...\n{% endfor %}"
+info:
+  $settings:
+    undefined: strict
+input_file: minijinja/tests/inputs/err_strict_undefined_for.txt
+---
+!!!ERROR!!!
+
+Error {
+    kind: UndefinedError,
+    name: "err_strict_undefined_for.txt",
+    line: 1,
+}
+
+undefined value (in err_strict_undefined_for.txt:1)
+------------------------ err_strict_undefined_for.txt -------------------------
+   1 > {% for item in undefined_value %}
+     i                ^^^^^^^^^^^^^^^ undefined value
+   2 |     ...
+   3 | {% endfor %}
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+No referenced variables
+-------------------------------------------------------------------------------
diff --git a/minijinja/tests/snapshots/test_templates__vm@err_strict_undefined_for_filter.txt.snap b/minijinja/tests/snapshots/test_templates__vm@err_strict_undefined_for_filter.txt.snap
new file mode 100644
index 0000000..9cab3db
--- /dev/null
+++ b/minijinja/tests/snapshots/test_templates__vm@err_strict_undefined_for_filter.txt.snap
@@ -0,0 +1,27 @@
+---
+source: minijinja/tests/test_templates.rs
+description: "{% for item in [undefined_value] if item %}\n    ...\n{% endfor %}"
+info:
+  $settings:
+    undefined: strict
+input_file: minijinja/tests/inputs/err_strict_undefined_for_filter.txt
+---
+!!!ERROR!!!
+
+Error {
+    kind: UndefinedError,
+    name: "err_strict_undefined_for_filter.txt",
+    line: 1,
+}
+
+undefined value (in err_strict_undefined_for_filter.txt:1)
+--------------------- err_strict_undefined_for_filter.txt ---------------------
+   1 > {% for item in [undefined_value] if item %}
+     i                                     ^^^^ undefined value
+   2 |     ...
+   3 | {% endfor %}
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Referenced variables: {
+    item: undefined,
+}
+-------------------------------------------------------------------------------
diff --git a/minijinja/tests/snapshots/test_templates__vm@err_strict_undefined_if.txt.snap b/minijinja/tests/snapshots/test_templates__vm@err_strict_undefined_if.txt.snap
new file mode 100644
index 0000000..7053ffb
--- /dev/null
+++ b/minijinja/tests/snapshots/test_templates__vm@err_strict_undefined_if.txt.snap
@@ -0,0 +1,25 @@
+---
+source: minijinja/tests/test_templates.rs
+description: "{% if undefined_value %}\n    ...\n{% endif %}"
+info:
+  $settings:
+    undefined: strict
+input_file: minijinja/tests/inputs/err_strict_undefined_if.txt
+---
+!!!ERROR!!!
+
+Error {
+    kind: UndefinedError,
+    name: "err_strict_undefined_if.txt",
+    line: 1,
+}
+
+undefined value (in err_strict_undefined_if.txt:1)
+------------------------- err_strict_undefined_if.txt -------------------------
+   1 > {% if undefined_value %}
+     i       ^^^^^^^^^^^^^^^ undefined value
+   2 |     ...
+   3 | {% endif %}
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+No referenced variables
+-------------------------------------------------------------------------------
diff --git a/minijinja/tests/snapshots/test_templates__vm@err_strict_undefined_print.txt.snap b/minijinja/tests/snapshots/test_templates__vm@err_strict_undefined_print.txt.snap
new file mode 100644
index 0000000..facd1e8
--- /dev/null
+++ b/minijinja/tests/snapshots/test_templates__vm@err_strict_undefined_print.txt.snap
@@ -0,0 +1,23 @@
+---
+source: minijinja/tests/test_templates.rs
+description: "{{ undefined_value }}"
+info:
+  $settings:
+    undefined: strict
+input_file: minijinja/tests/inputs/err_strict_undefined_print.txt
+---
+!!!ERROR!!!
+
+Error {
+    kind: UndefinedError,
+    name: "err_strict_undefined_print.txt",
+    line: 1,
+}
+
+undefined value (in err_strict_undefined_print.txt:1)
+----------------------- err_strict_undefined_print.txt ------------------------
+   1 > {{ undefined_value }}
+     i    ^^^^^^^^^^^^^^^ undefined value
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+No referenced variables
+-------------------------------------------------------------------------------
diff --git a/minijinja/tests/snapshots/test_templates__vm@loop-recursion-error.txt.snap b/minijinja/tests/snapshots/test_templates__vm@loop-recursion-error.txt.snap
index a7bb042..54e6154 100644
--- a/minijinja/tests/snapshots/test_templates__vm@loop-recursion-error.txt.snap
+++ b/minijinja/tests/snapshots/test_templates__vm@loop-recursion-error.txt.snap
@@ -20,6 +20,7 @@ Error {
 invalid operation: recursion limit exceeded (in loop-recursion-error.txt:1)
 -------------------------- loop-recursion-error.txt ---------------------------
    1 > {% for item in seq recursive %}
+     i                ^^^ invalid operation
    2 |   {{ loop(seq) }}
    3 | {% endfor %}
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/minijinja/tests/snapshots/test_templates__vm@loop_over_non_iterable.txt.snap b/minijinja/tests/snapshots/test_templates__vm@loop_over_non_iterable.txt.snap
index f42cd05..43c2d4f 100644
--- a/minijinja/tests/snapshots/test_templates__vm@loop_over_non_iterable.txt.snap
+++ b/minijinja/tests/snapshots/test_templates__vm@loop_over_non_iterable.txt.snap
@@ -17,9 +17,9 @@ Error {
 invalid operation: number is not iterable (in loop_over_non_iterable.txt:1)
 ------------------------- loop_over_non_iterable.txt --------------------------
    1 > [{% for item in seq %}{% endfor %}]
+     i                 ^^^ invalid operation
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 Referenced variables: {
     seq: 42,
 }
 -------------------------------------------------------------------------------
-
diff --git a/minijinja/tests/test_templates.rs b/minijinja/tests/test_templates.rs
index 82e57b0..4cac3a7 100644
--- a/minijinja/tests/test_templates.rs
+++ b/minijinja/tests/test_templates.rs
@@ -3,6 +3,7 @@
     feature = "macros",
     feature = "builtins",
     feature = "adjacent_loop_items",
+    feature = "custom_syntax",
     feature = "deserialization"
 ))]
 use std::collections::BTreeMap;
@@ -11,11 +12,44 @@ use std::sync::Arc;
 use std::{env, fs};
 
 use insta::assert_snapshot;
+use minijinja::syntax::SyntaxConfig;
 use minijinja::value::{Enumerator, Object, ObjectRepr, Rest, Value};
-use minijinja::{context, render, Environment, Error, ErrorKind, State};
+use minijinja::{context, render, Environment, Error, ErrorKind, State, UndefinedBehavior};
 
+use serde::Deserialize;
 use similar_asserts::assert_eq;
 
+#[derive(Deserialize, Default)]
+#[serde(default)]
+struct TestSettings {
+    keep_trailing_newline: bool,
+    lstrip_blocks: bool,
+    trim_blocks: bool,
+    markers: Option<[String; 6]>,
+    line_statement_prefix: Option<String>,
+    line_comment_prefix: Option<String>,
+    undefined: Option<String>,
+}
+
+impl TestSettings {
+    pub fn into_syntax(self) -> SyntaxConfig {
+        let mut builder = SyntaxConfig::builder();
+        if let Some(ref markers) = self.markers {
+            builder
+                .block_delimiters(markers[0].to_string(), markers[1].to_string())
+                .variable_delimiters(markers[2].to_string(), markers[3].to_string())
+                .comment_delimiters(markers[4].to_string(), markers[5].to_string());
+        }
+        if let Some(prefix) = self.line_statement_prefix {
+            builder.line_statement_prefix(prefix);
+        }
+        if let Some(prefix) = self.line_comment_prefix {
+            builder.line_comment_prefix(prefix);
+        }
+        builder.build().unwrap()
+    }
+}
+
 #[test]
 fn test_vm() {
     let mut refs = Vec::new();
@@ -40,6 +74,22 @@ fn test_vm() {
             Value::from(args.0)
         });
         let ctx: Value = serde_json::from_str(iter.next().unwrap()).unwrap();
+        let settings =
+            if let Some(settings) = ctx.get_attr("$settings").ok().filter(|x| !x.is_undefined()) {
+                TestSettings::deserialize(settings).unwrap()
+            } else {
+                TestSettings::default()
+            };
+        env.set_undefined_behavior(match settings.undefined.as_deref() {
+            Some("strict") => UndefinedBehavior::Strict,
+            Some("lenient") | None => UndefinedBehavior::Lenient,
+            Some("chainable") => UndefinedBehavior::Chainable,
+            Some(other) => panic!("unknown undefined behavior '{}'", other),
+        });
+        env.set_keep_trailing_newline(settings.keep_trailing_newline);
+        env.set_trim_blocks(settings.trim_blocks);
+        env.set_lstrip_blocks(settings.lstrip_blocks);
+        env.set_syntax(settings.into_syntax());
 
         for (path, source) in &refs {
             let ref_filename = path.file_name().unwrap().to_str().unwrap();