Merge pull request askama-rs#299 from Kijewski/pr-issue-283

derive: keep track of called macros

Author: Kijewski

rinja_derive/src/generator.rs

@@ -8,17 +8,17 @@ use std::path::Path;
 use std::str;
 use std::sync::Arc;
 
-use parser::node::Whitespace;
+use parser::node::{Macro, Whitespace};
 use parser::{
     CharLit, Expr, FloatKind, IntKind, MAX_RUST_KEYWORD_LEN, Num, RUST_KEYWORDS, StrLit, WithSpan,
 };
 use rustc_hash::FxBuildHasher;
 
-use crate::CompileError;
 use crate::heritage::{Context, Heritage};
 use crate::html::write_escaped_str;
 use crate::input::{Source, TemplateInput};
 use crate::integration::{Buffer, impl_everything, write_header};
+use crate::{CompileError, FileInfo};
 
 pub(crate) fn template_to_string(
     buf: &mut Buffer,
@@ -46,27 +46,29 @@ pub(crate) fn template_to_string(
 }
 
 struct Generator<'a, 'h> {
-    // The template input state: original struct AST and attributes
+    /// The template input state: original struct AST and attributes
     input: &'a TemplateInput<'a>,
-    // All contexts, keyed by the package-relative template path
+    /// All contexts, keyed by the package-relative template path
     contexts: &'a HashMap<&'a Arc<Path>, Context<'a>, FxBuildHasher>,
-    // The heritage contains references to blocks and their ancestry
+    /// The heritage contains references to blocks and their ancestry
     heritage: Option<&'h Heritage<'a, 'h>>,
-    // Variables accessible directly from the current scope (not redirected to context)
+    /// Variables accessible directly from the current scope (not redirected to context)
     locals: MapChain<'a>,
-    // Suffix whitespace from the previous literal. Will be flushed to the
-    // output buffer unless suppressed by whitespace suppression on the next
-    // non-literal.
+    /// Suffix whitespace from the previous literal. Will be flushed to the
+    /// output buffer unless suppressed by whitespace suppression on the next
+    /// non-literal.
     next_ws: Option<&'a str>,
-    // Whitespace suppression from the previous non-literal. Will be used to
-    // determine whether to flush prefix whitespace from the next literal.
+    /// Whitespace suppression from the previous non-literal. Will be used to
+    /// determine whether to flush prefix whitespace from the next literal.
     skip_ws: Whitespace,
-    // If currently in a block, this will contain the name of a potential parent block
+    /// If currently in a block, this will contain the name of a potential parent block
     super_block: Option<(&'a str, usize)>,
-    // Buffer for writable
+    /// Buffer for writable
     buf_writable: WritableBuffer<'a>,
-    // Used in blocks to check if we are inside a filter block.
+    /// Used in blocks to check if we are inside a filter block.
     is_in_filter_block: usize,
+    /// Set of called macros we are currently in. Used to prevent (indirect) recursions.
+    seen_macros: Vec<(&'a Macro<'a>, Option<FileInfo<'a>>)>,
 }
 
 impl<'a, 'h> Generator<'a, 'h> {
@@ -91,6 +93,7 @@ impl<'a, 'h> Generator<'a, 'h> {
                 ..Default::default()
             },
             is_in_filter_block,
+            seen_macros: Vec::new(),
         }
     }
 

rinja_derive/src/generator/node.rs

@@ -1,5 +1,6 @@
 use std::borrow::Cow;
 use std::collections::hash_map::{Entry, HashMap};
+use std::fmt::Write;
 use std::mem;
 
 use parser::node::{
@@ -616,14 +617,28 @@ impl<'a> Generator<'a, '_> {
                     call.span(),
                 )
             })?;
-            (def, mctx)
+            (*def, mctx)
         } else {
             let def = ctx.macros.get(name).ok_or_else(|| {
                 ctx.generate_error(format_args!("macro {name:?} not found"), call.span())
             })?;
-            (def, ctx)
+            (*def, ctx)
         };
 
+        if self.seen_macros.iter().any(|(s, _)| std::ptr::eq(*s, def)) {
+            let mut message = "Found recursion in macro calls:".to_owned();
+            for (m, f) in &self.seen_macros {
+                if let Some(f) = f {
+                    write!(message, "{f}").unwrap();
+                } else {
+                    write!(message, "\n`{}`", m.name.escape_debug()).unwrap();
+                }
+            }
+            return Err(ctx.generate_error(message, call.span()));
+        } else {
+            self.seen_macros.push((def, ctx.file_info_of(call.span())));
+        }
+
         self.flush_ws(ws); // Cannot handle_ws() here: whitespace from macro definition comes first
         let size_hint = self.push_locals(|this| {
             macro_call_ensure_arg_count(call, def, ctx)?;
@@ -746,6 +761,7 @@ impl<'a> Generator<'a, '_> {
             Ok(size_hint)
         })?;
         self.prepare_ws(ws);
+        self.seen_macros.pop();
         Ok(size_hint)
     }
 

rinja_derive/src/heritage.rs

@@ -51,7 +51,7 @@ pub(crate) struct Context<'a> {
     pub(crate) parsed: &'a Parsed,
 }
 
-impl Context<'_> {
+impl<'a> Context<'a> {
     pub(crate) fn empty(parsed: &Parsed) -> Context<'_> {
         Context {
             nodes: &[],
@@ -64,11 +64,11 @@ impl Context<'_> {
         }
     }
 
-    pub(crate) fn new<'n>(
+    pub(crate) fn new(
         config: &Config,
-        path: &'n Path,
-        parsed: &'n Parsed,
-    ) -> Result<Context<'n>, CompileError> {
+        path: &'a Path,
+        parsed: &'a Parsed,
+    ) -> Result<Self, CompileError> {
         let mut extends = None;
         let mut blocks = HashMap::default();
         let mut macros = HashMap::default();
@@ -142,10 +142,11 @@ impl Context<'_> {
     }
 
     pub(crate) fn generate_error(&self, msg: impl fmt::Display, node: Span<'_>) -> CompileError {
-        CompileError::new(
-            msg,
-            self.path.map(|path| FileInfo::of(node, path, self.parsed)),
-        )
+        CompileError::new(msg, self.file_info_of(node))
+    }
+
+    pub(crate) fn file_info_of(&self, node: Span<'a>) -> Option<FileInfo<'a>> {
+        self.path.map(|path| FileInfo::of(node, path, self.parsed))
     }
 }
 

testing/templates/macro-recursion-1.html

@@ -0,0 +1,5 @@
+{% import "macro-recursion-2.html" as next %}
+
+{% macro some_macro %}
+    {% call next::some_macro %}
+{% endmacro %}

testing/templates/macro-recursion-2.html

@@ -0,0 +1,5 @@
+{% import "macro-recursion-3.html" as next %}
+
+{% macro some_macro %}
+    {% call next::some_macro %}
+{% endmacro %}

testing/templates/macro-recursion-3.html

@@ -0,0 +1,5 @@
+{% import "macro-recursion-1.html" as next %}
+
+{% macro some_macro %}
+    {% call next::some_macro %}
+{% endmacro %}

testing/tests/ui/macro-recursion.rs

@@ -0,0 +1,41 @@
+use rinja::Template;
+
+#[derive(Template)]
+#[template(
+    source = "
+        {% macro one %}{% call one %}{% endmacro %}
+        {% call one %}
+    ",
+    ext = "html"
+)]
+struct Direct;
+
+#[derive(Template)]
+#[template(
+    source = "
+        {% macro one %}{% call two %}{% endmacro %}
+        {% macro two %}{% call three %}{% endmacro %}
+        {% macro three %}{% call four %}{% endmacro %}
+        {% macro four %}{% call five %}{% endmacro %}
+        {% macro five %}{% call one %}{% endmacro %}
+        {% call one %}
+    ",
+    ext = "html"
+)]
+struct Indirect;
+
+#[derive(Template)]
+#[template(
+    source = r#"
+        {% import "macro-recursion-1.html" as next %}
+        {% macro some_macro %}
+            {% call next::some_macro %}
+        {% endmacro %}
+        {% call some_macro %}
+    "#,
+    ext = "html"
+)]
+struct AcrossImports;
+
+fn main() {
+}

testing/tests/ui/macro-recursion.stderr

@@ -0,0 +1,61 @@
+error: Found recursion in macro calls:
+ --> Direct.html:3:10
+       " call one %}\n    "
+         --> Direct.html:2:25
+       " call one %}{% endmacro %}\n        {% call one %}\n    "
+ --> tests/ui/macro-recursion.rs:5:14
+  |
+5 |       source = "
+  |  ______________^
+6 | |         {% macro one %}{% call one %}{% endmacro %}
+7 | |         {% call one %}
+8 | |     ",
+  | |_____^
+
+error: Found recursion in macro calls:
+ --> Indirect.html:7:10
+       " call one %}\n    "
+         --> Indirect.html:2:25
+       " call two %}{% endmacro %}\n        {% macro two %}{% call three %}{% endmacro %}"...
+         --> Indirect.html:3:25
+       " call three %}{% endmacro %}\n        {% macro three %}{% call four %}{% endmacro"...
+         --> Indirect.html:4:27
+       " call four %}{% endmacro %}\n        {% macro four %}{% call five %}{% endmacro %"...
+         --> Indirect.html:5:26
+       " call five %}{% endmacro %}\n        {% macro five %}{% call one %}{% endmacro %}"...
+         --> Indirect.html:6:26
+       " call one %}{% endmacro %}\n        {% call one %}\n    "
+  --> tests/ui/macro-recursion.rs:15:14
+   |
+15 |       source = "
+   |  ______________^
+16 | |         {% macro one %}{% call two %}{% endmacro %}
+17 | |         {% macro two %}{% call three %}{% endmacro %}
+18 | |         {% macro three %}{% call four %}{% endmacro %}
+...  |
+21 | |         {% call one %}
+22 | |     ",
+   | |_____^
+
+error: Found recursion in macro calls:
+ --> AcrossImports.html:6:10
+       " call some_macro %}\n    "
+         --> AcrossImports.html:4:14
+       " call next::some_macro %}\n        {% endmacro %}\n        {% call some_macro %}\n "...
+         --> testing/templates/macro-recursion-1.html:4:6
+       " call next::some_macro %}\n{% endmacro %}"
+         --> testing/templates/macro-recursion-2.html:4:6
+       " call next::some_macro %}\n{% endmacro %}"
+         --> testing/templates/macro-recursion-3.html:4:6
+       " call next::some_macro %}\n{% endmacro %}"
+  --> tests/ui/macro-recursion.rs:29:14
+   |
+29 |       source = r#"
+   |  ______________^
+30 | |         {% import "macro-recursion-1.html" as next %}
+31 | |         {% macro some_macro %}
+32 | |             {% call next::some_macro %}
+33 | |         {% endmacro %}
+34 | |         {% call some_macro %}
+35 | |     "#,
+   | |______^