Merge pull request #586 from GuillaumeGomez/fix-block-duplication-warning

Fix block duplication warning

askama_derive/src/generator.rs

@@ -87,8 +87,6 @@ struct Generator<'a, 'h> {
     is_in_filter_block: usize,
     /// Set of called macros we are currently in. Used to prevent (indirect) recursions.
     seen_callers: Vec<(&'a Macro<'a>, Option<FileInfo<'a>>)>,
-    /// Blocks already used.
-    called_blocks: Vec<(&'a str, Option<FileInfo<'a>>)>,
     /// The directory path of the calling file.
     caller_dir: CallerDir,
 }
@@ -123,7 +121,6 @@ impl<'a, 'h> Generator<'a, 'h> {
             is_in_filter_block,
             seen_callers: Vec::new(),
             caller_dir: CallerDir::Unresolved,
-            called_blocks: Vec::new(),
         }
     }
 

askama_derive/src/generator/node.rs

@@ -983,45 +983,6 @@ impl<'a> Generator<'a, '_> {
             )
         })?;
 
-        if let Some(name) = name {
-            // This checks if the same block is called in a "parent".
-            if let Some((_, first_call)) = self.called_blocks.iter().find(|block| block.0 == *name)
-            {
-                let first_call = if let Some(first_call) = first_call {
-                    format!("{first_call:#}")
-                } else {
-                    "<inline source>".to_string()
-                };
-                let current = if let Some(node) = ctx.file_info_of(node) {
-                    format!("{node:#}")
-                } else {
-                    "<inline source>".to_string()
-                };
-
-                eprintln!(
-                    "⚠️ {}: block `{}` was already called at `{}` so the previous one will be ignored",
-                    current, cur.0, first_call,
-                );
-            } else if child_ctx.blocks.contains_key(*name) {
-                let first = match child_ctx.path {
-                    Some(p) => p.display().to_string(),
-                    None => "<inline source>".to_string(),
-                };
-                let current = if let Some(node) = ctx.file_info_of(node) {
-                    format!("{node:#}")
-                } else {
-                    "<inline source>".to_string()
-                };
-
-                eprintln!(
-                    "⚠️ {}: block `{}` was already called at `{first}` so the previous one will be ignored",
-                    current, cur.0,
-                );
-            } else {
-                self.called_blocks.push((*name, ctx.file_info_of(node)));
-            }
-        }
-
         // We clone the context of the child in order to preserve their macros and imports.
         // But also add all the imports and macros from this template that don't override the
         // child's ones to preserve this template's context.

askama_derive/src/heritage.rs

@@ -72,6 +72,7 @@ impl<'a> Context<'a> {
         parsed: &'a Parsed,
         literal: SourceSpan,
         template_span: proc_macro2::Span,
+        called_blocks: &mut crate::CalledBlocks<'a>,
     ) -> Result<Self, CompileError> {
         let mut extends = None;
         let mut blocks: HashMap<&'a str, &'a WithSpan<BlockDef<'a>>> = HashMap::default();
@@ -103,14 +104,23 @@ impl<'a> Context<'a> {
                         imports.push(import);
                     }
                     Node::BlockDef(b) => {
+                        let current = Self::file_info_of_inner(b.span(), path, parsed);
                         // This checks if the same block is called in a file.
                         if let Some(prev) = blocks.get(&*b.name) {
                             let prev = Self::file_info_of_inner(prev.span(), path, parsed);
-                            let current = Self::file_info_of_inner(b.span(), path, parsed);
                             eprintln!(
                                 "⚠️ {:#}: block `{}` was already called at `{:#}` so the previous one will be ignored",
                                 current, &*b.name, prev,
                             );
+                        } else if extends.is_none() {
+                            called_blocks.check_if_already_called(*b.name, current);
+                            called_blocks
+                                .called_blocks
+                                .entry(*b.name)
+                                .or_default()
+                                .push(current);
+                        } else {
+                            called_blocks.unprocessed.push((*b.name, current));
                         }
                         blocks.insert(*b.name, b);
                         nested.push(&b.nodes);

askama_derive/src/lib.rs

@@ -350,6 +350,26 @@ pub(crate) fn build_template(
     result
 }
 
+#[derive(Default)]
+pub(crate) struct CalledBlocks<'a> {
+    pub(crate) called_blocks: HashMap<&'a str, Vec<FileInfo<'a>>>,
+    pub(crate) unprocessed: Vec<(&'a str, FileInfo<'a>)>,
+}
+
+impl CalledBlocks<'_> {
+    fn check_if_already_called(&self, block_name: &str, current: FileInfo<'_>) {
+        if let Some(calls) = self.called_blocks.get(&block_name)
+            // The first one is always the definition so we skip it.
+            && let Some(prev) = calls.iter().skip(1).last()
+        {
+            eprintln!(
+                "⚠️ {:#}: block `{}` was already called at `{:#}` so the previous one will be ignored",
+                current, block_name, prev,
+            );
+        }
+    }
+}
+
 fn build_template_item(
     buf: &mut Buffer,
     ast: &syn::DeriveInput,
@@ -372,6 +392,8 @@ fn build_template_item(
     input.find_used_templates(&mut templates)?;
 
     let mut contexts = HashMap::default();
+    let mut called_blocks = CalledBlocks::default();
+
     for (path, parsed) in &templates {
         contexts.insert(
             path,
@@ -381,10 +403,23 @@ fn build_template_item(
                 parsed,
                 input.source_span.clone(),
                 input.template_span,
+                &mut called_blocks,
             )?,
         );
     }
 
+    // Now that all `extends` have been processed, we can finish to check for duplicated block
+    // calls.
+    let mut unprocessed_items = std::mem::take(&mut called_blocks.unprocessed);
+    while let Some((name, file_info)) = unprocessed_items.pop() {
+        called_blocks.check_if_already_called(name, file_info);
+        called_blocks
+            .called_blocks
+            .entry(name)
+            .or_default()
+            .push(file_info);
+    }
+
     let ctx = &contexts[&input.path];
     let heritage = if !ctx.blocks.is_empty() || ctx.extends.is_some() {
         Some(Heritage::new(ctx, &contexts))

askama_parser/src/lib.rs

@@ -264,6 +264,10 @@ impl Span {
         // `source` cannot be longer than `isize::MAX`, cf. [`std::alloc`].
         source.get(self.start..)
     }
+
+    pub fn is_overlapping(&self, other: Span) -> bool {
+        (self.start < other.end) & (other.start < self.end)
+    }
 }
 
 impl<T> WithSpan<T> {

testing/tests/book-examples.rs

@@ -102,7 +102,6 @@ fn test_book_examples() {
         panic!(">> cannot get `CARGO_MANIFEST_DIR` env variable");
     };
     let mut errors = 0;
-    eprintln!("{cargo_home:?}");
     go_through_book(
         &Path::new(&cargo_home).parent().unwrap().join("book/src"),
         &mut errors,

testing/tests/custom_ui.rs

@@ -0,0 +1,158 @@
+// Test askama outputs that is not handled by `trybuild`.
+
+use std::ffi::OsStr;
+use std::fs::{create_dir_all, read_dir, read_to_string, remove_dir_all};
+use std::path::{Path, PathBuf};
+use std::process::Command;
+
+#[test]
+fn test_custom_ui() {
+    if !cfg!(unix) {
+        return;
+    }
+    let Ok(cargo_home) = std::env::var("CARGO_MANIFEST_DIR") else {
+        panic!(">> cannot get `CARGO_MANIFEST_DIR` env variable");
+    };
+    let bless = std::env::var("TRYBUILD").as_deref() == Ok("overwrite");
+    let mut errors = 0;
+
+    go_through_entries(&cargo_home, bless, &mut errors);
+    assert!(errors == 0);
+}
+
+fn go_through_entries(cargo_home: &str, bless: bool, errors: &mut usize) {
+    let cargo_home_path = Path::new(cargo_home).parent().unwrap();
+    let test_dir = cargo_home_path.join("target/tests/custom_ui");
+
+    // We don't check whether it succeeds or not.
+    let _ = remove_dir_all(&test_dir);
+    create_dir_all(&test_dir).expect("failed to create test dir");
+
+    make_link(
+        &cargo_home_path.join("testing/templates"),
+        &test_dir.join("templates"),
+    );
+    make_link(&cargo_home_path.join("target"), &test_dir.join("target"));
+
+    let custom_ui_folder = cargo_home_path.join("testing/tests/custom_ui");
+    std::fs::write(
+        test_dir.join("Cargo.toml"),
+        format!(
+            r#"
+[package]
+name = "askama_test"
+version = "0.0.1"
+edition = "2024"
+
+[workspace]
+
+[dependencies]
+askama = {{ path = {:?} }}
+
+[[bin]]
+name = "main"
+path = "main.rs"
+"#,
+            cargo_home_path.join("askama").display(),
+        ),
+    )
+    .unwrap();
+
+    let mut nb_run_tests = 0;
+    for entry in read_dir(custom_ui_folder).unwrap() {
+        let entry = entry.unwrap();
+        let test_path = entry.path();
+        if !test_path.is_dir() && test_path.extension() == Some(OsStr::new("rs")) {
+            if nb_run_tests == 0 {
+                // To prevent having build logs in tests output, we run the build a first time.
+                run_cargo(&test_dir);
+            }
+            nb_run_tests += 1;
+            print!("> {}...", test_path.file_name().unwrap().display());
+            if !run_test(bless, &test_path, &test_dir) {
+                *errors += 1;
+            }
+        }
+    }
+}
+
+fn run_test(bless: bool, test_path: &Path, test_dir: &Path) -> bool {
+    let tmp_file = test_dir.join("main.rs");
+    std::fs::copy(test_path, &tmp_file).unwrap();
+
+    let stderr = run_cargo(test_dir);
+
+    let mut stderr_file_path = PathBuf::from(test_path);
+    stderr_file_path.set_extension("stderr");
+    match read_to_string(&stderr_file_path) {
+        Ok(content) if content == stderr => {
+            println!(" OK");
+            true
+        }
+        _ if bless => {
+            println!(" FAILED");
+            std::fs::write(&stderr_file_path, stderr.as_bytes()).unwrap();
+            true
+        }
+        content => {
+            eprintln!(
+                " FAILED
+======== {} ========
+Output differs from expected:
+
+=== FOUND ===
+{}
+
+{}",
+                test_path.file_name().unwrap().display(),
+                stderr,
+                match content {
+                    Ok(content) => format!("=== EXPECTED ===\n{}", content),
+                    _ => "No stderr exist yet. Use `TRYBUILD=overwrite` to bless the output"
+                        .to_string(),
+                },
+            );
+            false
+        }
+    }
+}
+
+fn run_cargo(test_dir: &Path) -> String {
+    let out = Command::new(env!("CARGO"))
+        .args(["check", "--bin", "main", "--color", "never"])
+        .current_dir(test_dir)
+        .output()
+        .expect("failed to execute process");
+    let stderr = String::from_utf8_lossy(&out.stderr);
+    let mut index = 0;
+    let mut start = None;
+    let mut end = None;
+
+    for line in stderr.split('\n') {
+        if start.is_none() && line.trim_start().starts_with("Checking askama_test v0.0.1") {
+            start = Some(index + line.len() + 1);
+        } else if end.is_none() && line.trim_start().starts_with("Finished `dev` profile [") {
+            end = Some(index);
+        }
+        if start.is_some() && end.is_some() {
+            break;
+        }
+        index += line.len() + 1; // +1 is for the '\n'.
+    }
+    match (start, end) {
+        (Some(start), None) => stderr[start..].to_string(),
+        (Some(start), Some(end)) => stderr[start..end].to_string(),
+        _ => panic!("didn't find `askama_test` start line, full output:\n{stderr}"),
+    }
+}
+
+fn make_link(original: &Path, destination: &Path) {
+    #[cfg(unix)]
+    {
+        std::os::unix::fs::symlink(original, destination).unwrap();
+    }
+    #[cfg(windows)]
+    {
+        std::os::windows::fs::symlink_dir(original, destination).unwrap();
+    }
+}

testing/tests/custom_ui/duplicated_block_calls.rs

@@ -0,0 +1,30 @@
+use askama::Template;
+
+// Check if the block call is duplicated in the current template.
+#[derive(Template)]
+#[template(
+    source = r##"{% extends "base.html" %}
+
+{% block content %}{% endblock %}
+{% block content %}{% endblock %}
+"##,
+    ext = "txt",
+)]
+struct X {
+    title: &'static str,
+}
+
+// Check if the block call is called in the extended template and in the current one.
+#[derive(Template)]
+#[template(
+    source = r##"{% extends "child.html" %}
+
+{% block content %}another{% endblock %}
+"##,
+    ext = "txt",
+)]
+struct X2 {
+    title: &'static str,
+}
+
+fn main() {}

testing/tests/custom_ui/duplicated_block_calls.stderr

@@ -0,0 +1,2 @@
+⚠️ X.txt:4:3: block `content` was already called at `X.txt:3:3` so the previous one will be ignored
+⚠️ X2.txt:3:3: block `content` was already called at `testing/templates/child.html:2:3` so the previous one will be ignored

testing/tests/extend.rs

@@ -30,11 +30,7 @@ fn test_macro_in_block_inheritance() {
 #[test]
 fn test_comment_before_extend() {
     #[derive(Template)]
-    #[template(
-        source = r##"{# comment #}{% extends "base.html" %}"##,
-        ext = "txt",
-        print = "ast"
-    )]
+    #[template(source = r##"{# comment #}{% extends "base.html" %}"##, ext = "txt")]
     pub struct X {
         title: &'static str,
     }

testing/tests/ui/extend.rs

@@ -6,7 +6,6 @@ use askama::Template;
 {% extends "base.html" %}
 "##,
     ext = "txt",
-    print = "ast"
 )]
 pub struct X;