From 034f1676b53793c96b6c7d1ddc3580a7692c14b7 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 24 Aug 2025 17:22:13 +0200 Subject: [PATCH 1/4] Fix invalid "duplicated block call" warning --- askama_derive/src/generator.rs | 3 --- askama_derive/src/generator/node.rs | 39 ----------------------------- askama_derive/src/heritage.rs | 12 ++++++++- askama_derive/src/lib.rs | 35 ++++++++++++++++++++++++++ askama_parser/src/lib.rs | 4 +++ testing/Cargo.toml | 1 + testing/tests/book-examples.rs | 1 - 7 files changed, 51 insertions(+), 44 deletions(-) diff --git a/askama_derive/src/generator.rs b/askama_derive/src/generator.rs index 8336dbd6..b09a7d84 100644 --- a/askama_derive/src/generator.rs +++ b/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>)>, - /// Blocks already used. - called_blocks: Vec<(&'a str, Option>)>, /// 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(), } } diff --git a/askama_derive/src/generator/node.rs b/askama_derive/src/generator/node.rs index e3e0d711..e842913b 100644 --- a/askama_derive/src/generator/node.rs +++ b/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 { - "".to_string() - }; - let current = if let Some(node) = ctx.file_info_of(node) { - format!("{node:#}") - } else { - "".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 => "".to_string(), - }; - let current = if let Some(node) = ctx.file_info_of(node) { - format!("{node:#}") - } else { - "".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. diff --git a/askama_derive/src/heritage.rs b/askama_derive/src/heritage.rs index 08721417..408892e9 100644 --- a/askama_derive/src/heritage.rs +++ b/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 { let mut extends = None; let mut blocks: HashMap<&'a str, &'a WithSpan>> = 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); diff --git a/askama_derive/src/lib.rs b/askama_derive/src/lib.rs index 93d01872..3bfc23c4 100644 --- a/askama_derive/src/lib.rs +++ b/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>>, + 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)) diff --git a/askama_parser/src/lib.rs b/askama_parser/src/lib.rs index 1851ff1a..d3324950 100644 --- a/askama_parser/src/lib.rs +++ b/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 WithSpan { diff --git a/testing/Cargo.toml b/testing/Cargo.toml index a3e0e164..95d6c488 100644 --- a/testing/Cargo.toml +++ b/testing/Cargo.toml @@ -25,6 +25,7 @@ askama_parser = { path = "../askama_parser", version = "0.14.0" } assert_matches = "1.5.0" criterion = "0.7" pulldown-cmark = "0.13" +tempfile = "3.21" trybuild = "1.0.100" [features] diff --git a/testing/tests/book-examples.rs b/testing/tests/book-examples.rs index be0fff1c..bba93f59 100644 --- a/testing/tests/book-examples.rs +++ b/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, From a577f60d33f25c3f89fa358aa88b42e0590f8332 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 25 Aug 2025 00:40:58 +0200 Subject: [PATCH 2/4] Add new `custom_ui` test suite to allow checking askama warning messages --- testing/Cargo.toml | 1 - testing/tests/custom_ui.rs | 158 +++++++++++++++++++++++++++++++++++++ 2 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 testing/tests/custom_ui.rs diff --git a/testing/Cargo.toml b/testing/Cargo.toml index 95d6c488..a3e0e164 100644 --- a/testing/Cargo.toml +++ b/testing/Cargo.toml @@ -25,7 +25,6 @@ askama_parser = { path = "../askama_parser", version = "0.14.0" } assert_matches = "1.5.0" criterion = "0.7" pulldown-cmark = "0.13" -tempfile = "3.21" trybuild = "1.0.100" [features] diff --git a/testing/tests/custom_ui.rs b/testing/tests/custom_ui.rs new file mode 100644 index 00000000..18bc55c9 --- /dev/null +++ b/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(); + } +} From 129c3549d47160fde5ddfd7262f99f295466294f Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 24 Aug 2025 17:23:48 +0200 Subject: [PATCH 3/4] Add custom ui regression test for duplicated blocks calls --- .../tests/custom_ui/duplicated_block_calls.rs | 30 +++++++++++++++++++ .../custom_ui/duplicated_block_calls.stderr | 2 ++ 2 files changed, 32 insertions(+) create mode 100644 testing/tests/custom_ui/duplicated_block_calls.rs create mode 100644 testing/tests/custom_ui/duplicated_block_calls.stderr diff --git a/testing/tests/custom_ui/duplicated_block_calls.rs b/testing/tests/custom_ui/duplicated_block_calls.rs new file mode 100644 index 00000000..e026c071 --- /dev/null +++ b/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() {} diff --git a/testing/tests/custom_ui/duplicated_block_calls.stderr b/testing/tests/custom_ui/duplicated_block_calls.stderr new file mode 100644 index 00000000..82f4509d --- /dev/null +++ b/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 From 04e9241e4f25d275bb20504b97e03bb48b6db6a0 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 2 Sep 2025 15:13:46 +0200 Subject: [PATCH 4/4] Remove unwanted `print = "ast"` forgotten in tests --- testing/tests/extend.rs | 6 +----- testing/tests/ui/extend.rs | 1 - 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/testing/tests/extend.rs b/testing/tests/extend.rs index ce85d65b..0bd80b7b 100644 --- a/testing/tests/extend.rs +++ b/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, } diff --git a/testing/tests/ui/extend.rs b/testing/tests/ui/extend.rs index 871d501d..772993a8 100644 --- a/testing/tests/ui/extend.rs +++ b/testing/tests/ui/extend.rs @@ -6,7 +6,6 @@ use askama::Template; {% extends "base.html" %} "##, ext = "txt", - print = "ast" )] pub struct X;