From 57c9d26f9a2862dd3c717200253a2670a0057c46 Mon Sep 17 00:00:00 2001 From: myypo Date: Sat, 26 Sep 2026 20:29:26 +0300 Subject: [PATCH] fix: allow `(@&...)` to borrow template-local bindings --- crates/ast/src/component.rs | 6 +- crates/ast/src/control.rs | 100 +++++++++++++++++------- crates/ast/src/generate.rs | 58 +++++++++++--- crates/ast/src/lib.rs | 37 ++++++++- crates/cheers/tests/main.rs | 151 ++++++++++++++++++++++++++++++++++++ 5 files changed, 308 insertions(+), 44 deletions(-) diff --git a/crates/ast/src/component.rs b/crates/ast/src/component.rs index 02dcbf5..13e30c1 100644 --- a/crates/ast/src/component.rs +++ b/crates/ast/src/component.rs @@ -234,9 +234,9 @@ impl ComponentAttribute { tokens } - ParenExprMode::Ref => g - .hoist_ref_expr(expr.paren_token, &expr.body) - .to_token_stream(), + ParenExprMode::Ref => { + g.hoist_ref_expr(expr.paren_token, &expr.body, expr.body.ref_root()) + } }, Some(ComponentAttributeValue::Ident(ident)) => ident.to_token_stream(), None => self.name.to_token_stream(), diff --git a/crates/ast/src/control.rs b/crates/ast/src/control.rs index e9306a3..6d103ce 100644 --- a/crates/ast/src/control.rs +++ b/crates/ast/src/control.rs @@ -5,7 +5,7 @@ use proc_macro2::{Ident, Span, TokenStream, TokenTree}; use quote::{ToTokens, quote}; use rustc_hash::FxHasher; use syn::{ - Expr, LitStr, Local, Pat, Stmt, Token, braced, + Expr, LitStr, Local, Pat, PatIdent, Stmt, Token, braced, parse::{Parse, ParseStream}, spanned::Spanned as _, token::Brace, @@ -33,47 +33,42 @@ fn tokens_contain_any_ident(tokens: &TokenStream, needles: &[Ident]) -> bool { }) } -fn collect_pat_bindings(pat: &Pat, bindings: &mut Vec<(TokenStream, Ident)>) { +fn for_each_pat_ident(pat: &Pat, f: &mut impl FnMut(&PatIdent)) { match pat { Pat::Ident(pat) => { - let mut binding = TokenStream::new(); - pat.mutability.to_tokens(&mut binding); - pat.ident.to_tokens(&mut binding); - - if let Some(existing) = bindings.iter_mut().find(|(_, ident)| ident == &pat.ident) { - *existing = (binding, pat.ident.clone()); - } else { - bindings.push((binding, pat.ident.clone())); + f(pat); + if let Some((_, subpat)) = &pat.subpat { + for_each_pat_ident(subpat, f); } } Pat::Or(pat) => { for case in &pat.cases { - collect_pat_bindings(case, bindings); + for_each_pat_ident(case, f); } } - Pat::Paren(pat) => collect_pat_bindings(&pat.pat, bindings), - Pat::Reference(pat) => collect_pat_bindings(&pat.pat, bindings), + Pat::Paren(pat) => for_each_pat_ident(&pat.pat, f), + Pat::Reference(pat) => for_each_pat_ident(&pat.pat, f), Pat::Slice(pat) => { for elem in &pat.elems { - collect_pat_bindings(elem, bindings); + for_each_pat_ident(elem, f); } } Pat::Struct(pat) => { for field in &pat.fields { - collect_pat_bindings(&field.pat, bindings); + for_each_pat_ident(&field.pat, f); } } Pat::Tuple(pat) => { for elem in &pat.elems { - collect_pat_bindings(elem, bindings); + for_each_pat_ident(elem, f); } } Pat::TupleStruct(pat) => { for elem in &pat.elems { - collect_pat_bindings(elem, bindings); + for_each_pat_ident(elem, f); } } - Pat::Type(pat) => collect_pat_bindings(&pat.pat, bindings), + Pat::Type(pat) => for_each_pat_ident(&pat.pat, f), Pat::Const(_) | Pat::Lit(_) | Pat::Macro(_) @@ -86,6 +81,43 @@ fn collect_pat_bindings(pat: &Pat, bindings: &mut Vec<(TokenStream, Ident)>) { } } +fn collect_pat_bindings(pat: &Pat, bindings: &mut Vec<(TokenStream, Ident)>) { + for_each_pat_ident(pat, &mut |pat| { + let mut binding = TokenStream::new(); + pat.mutability.to_tokens(&mut binding); + pat.ident.to_tokens(&mut binding); + + if let Some(existing) = bindings.iter_mut().find(|(_, ident)| ident == &pat.ident) { + *existing = (binding, pat.ident.clone()); + } else { + bindings.push((binding, pat.ident.clone())); + } + }); +} + +fn pat_binding_idents(pat: &Pat) -> Vec { + let mut idents = Vec::new(); + for_each_pat_ident(pat, &mut |pat| idents.push(pat.ident.clone())); + idents +} + +fn cond_binding_idents(cond: &Expr) -> Vec { + fn collect(cond: &Expr, idents: &mut Vec) { + match cond { + Expr::Let(let_) => idents.append(&mut pat_binding_idents(&let_.pat)), + Expr::Binary(binary) if matches!(binary.op, syn::BinOp::And(_)) => { + collect(&binary.left, idents); + collect(&binary.right, idents); + } + _ => {} + } + } + + let mut idents = Vec::new(); + collect(cond, &mut idents); + idents +} + fn leading_let_bindings( nodes: &[ElementNode], leading_let_count: usize, @@ -379,6 +411,7 @@ impl Generate for Let { fn generate(&mut self, g: &mut Generator<'_>) { g.push_stmt(&self.0); + g.declare_local_bindings(pat_binding_idents(&self.0.pat)); } } @@ -389,7 +422,11 @@ pub struct ControlBlock { impl ControlBlock { fn block(&mut self, g: &mut Generator<'_>) -> AnyBlock { - self.nodes.block(g, self.brace_token) + self.block_with_bindings(g, Vec::new()) + } + + fn block_with_bindings(&mut self, g: &mut Generator<'_>, bindings: Vec) -> AnyBlock { + self.nodes.block(g, self.brace_token, bindings) } } @@ -439,7 +476,9 @@ impl Generate for If { fn to_expr(if_: &mut If, g: &mut Generator<'_>) -> TokenStream { let if_token = if_.if_token; let cond = &if_.cond; - let then_block = if_.then_block.block(g); + let then_block = if_ + .then_block + .block_with_bindings(g, cond_binding_idents(cond)); let else_branch = if_ .else_branch .as_mut() @@ -515,7 +554,7 @@ impl Generate for For { let pat = &self.pat; let in_token = self.in_token; let expr = &self.expr; - let block = self.block.block(g); + let block = self.block.block_with_bindings(g, pat_binding_idents(pat)); g.push_stmt(quote! { #for_token #pat #in_token #expr @@ -546,7 +585,7 @@ impl Generate for While { fn generate(&mut self, g: &mut Generator<'_>) { let while_token = self.while_token; let cond = &self.cond; - let block = self.block.block(g); + let block = self.block.block_with_bindings(g, cond_binding_idents(cond)); g.push_stmt(quote! { #while_token #cond @@ -597,11 +636,17 @@ impl Generate for Match { .as_ref() .map(|(if_token, guard)| quote!(#if_token #guard)); let fat_arrow_token = arm.fat_arrow_token; + let bindings = pat_binding_idents(&pat); let block = match &mut arm.body { - MatchNodeArmBody::Block(block) => block.block(g), - MatchNodeArmBody::Node(node) => { - g.block_with(Brace::default(), |g| g.push(node), true) - } + MatchNodeArmBody::Block(block) => block.block_with_bindings(g, bindings), + MatchNodeArmBody::Node(node) => g.block_with( + Brace::default(), + |g| { + g.declare_local_bindings(bindings); + g.push(node); + }, + true, + ), }; let comma = arm.comma_token; @@ -976,6 +1021,9 @@ impl Generate for Async { g.block_with( self.async_block.brace_token, |g| { + g.declare_local_bindings( + leading_bindings.iter().map(|(_, ident)| ident.clone()), + ); for node in self.async_block.nodes.0.iter_mut().skip(leading_let_count) { g.push(node); } diff --git a/crates/ast/src/generate.rs b/crates/ast/src/generate.rs index 6e290a5..a645fa7 100644 --- a/crates/ast/src/generate.rs +++ b/crates/ast/src/generate.rs @@ -209,6 +209,9 @@ impl ToTokens for ValidationModule { struct BorrowState { captures: Vec, counter: usize, + /// Names bound by template control flow (`@let`, `@for`, `@if let`, ...) that are in scope + /// at the current generation point. They do not exist where borrows are hoisted to. + local_bindings: Vec, } impl BorrowState { @@ -216,24 +219,35 @@ impl BorrowState { Self { captures: Vec::new(), counter: 0, + local_bindings: Vec::new(), } } - fn hoist_ref_expr(&mut self, paren_token: Paren, expr: impl ToTokens) -> Ident { + fn hoist_ref_expr( + &mut self, + paren_token: Paren, + expr: impl ToTokens, + root: Option<&Ident>, + ) -> TokenStream { + let mut ref_expr = TokenStream::new(); + paren_token.surround(&mut ref_expr, |tokens| expr.to_tokens(tokens)); + + let reference = quote_spanned!(paren_token.span=> &); + + if root.is_some_and(|root| self.local_bindings.contains(root)) { + return quote!(#reference #ref_expr); + } + let ref_idx = self.counter; self.counter += 1; let ref_ident = format_ident!("__cheers_ref_{ref_idx}", span = Span::mixed_site()); - let mut ref_expr = TokenStream::new(); - paren_token.surround(&mut ref_expr, |tokens| expr.to_tokens(tokens)); - - let reference = quote_spanned!(paren_token.span=> &); self.captures.push(quote! { let #ref_ident = #reference #ref_expr; }); - ref_ident + ref_ident.into_token_stream() } } @@ -367,6 +381,7 @@ impl<'a> Generator<'a> { f: impl for<'b> FnOnce(&mut Generator<'b>), append_async: bool, ) -> AnyBlock { + let local_bindings_len = self.borrow_state.local_bindings.len(); let (mut child_checks, mut block) = { let mut g = self.new_child_with_brace(brace_token, flavour); @@ -377,6 +392,9 @@ impl<'a> Generator<'a> { (child_checks, block) }; + self.borrow_state + .local_bindings + .truncate(local_bindings_len); self.checks.append(&mut child_checks); if append_async { @@ -511,13 +529,31 @@ impl<'a> Generator<'a> { self.with_context_override(Context::DatastarSource, |g| g.push(node)); } - pub fn hoist_ref_expr(&mut self, paren_token: Paren, expr: impl ToTokens) -> Ident { - self.borrow_state.hoist_ref_expr(paren_token, expr) + /// Borrows `expr` outside the render closure so the closure captures only the reference. + /// Expressions rooted at a template-local binding are borrowed in place instead, since the + /// binding does not exist outside the closure. `root` is the variable `expr` borrows from. + pub fn hoist_ref_expr( + &mut self, + paren_token: Paren, + expr: impl ToTokens, + root: Option<&Ident>, + ) -> TokenStream { + self.borrow_state.hoist_ref_expr(paren_token, expr, root) + } + + pub fn push_ref_expr( + &mut self, + paren_token: Paren, + context: Context, + expr: impl ToTokens, + root: Option<&Ident>, + ) { + let ref_expr = self.hoist_ref_expr(paren_token, expr, root); + self.push_expr(Paren::default(), context, ref_expr); } - pub fn push_ref_expr(&mut self, paren_token: Paren, context: Context, expr: impl ToTokens) { - let ref_ident = self.hoist_ref_expr(paren_token, expr); - self.push_expr(Paren::default(), context, ref_ident); + pub fn declare_local_bindings(&mut self, bindings: impl IntoIterator) { + self.borrow_state.local_bindings.extend(bindings); } pub fn push_async_stmt(&mut self, async_stmt: impl ToTokens) { diff --git a/crates/ast/src/lib.rs b/crates/ast/src/lib.rs index 328cfcd..b8f067e 100644 --- a/crates/ast/src/lib.rs +++ b/crates/ast/src/lib.rs @@ -156,6 +156,25 @@ pub enum ParenExprBody { Tuple(Punctuated), } +impl ParenExprBody { + pub(crate) fn ref_root(&self) -> Option<&Ident> { + match self { + Self::Expr(expr) => ref_expr_root(expr), + Self::Unit | Self::Tuple(_) => None, + } + } +} + +/// The local variable a validated `(@&...)` path or field expression borrows from, if any. +fn ref_expr_root(expr: &Expr) -> Option<&Ident> { + match expr { + Expr::Path(path) if path.qself.is_none() => path.path.get_ident(), + Expr::Field(field) => ref_expr_root(&field.base), + Expr::Paren(paren) => ref_expr_root(&paren.expr), + _ => None, + } +} + impl ToTokens for ParenExprBody { fn to_tokens(&self, tokens: &mut TokenStream) { match self { @@ -342,7 +361,12 @@ impl Generate for ParenExpr { fn generate(&mut self, g: &mut Generator<'_>) { match self.mode { ParenExprMode::Normal => g.push_expr(self.paren_token, Self::CONTEXT, &self.body), - ParenExprMode::Ref => g.push_ref_expr(self.paren_token, Self::CONTEXT, &self.body), + ParenExprMode::Ref => g.push_ref_expr( + self.paren_token, + Self::CONTEXT, + &self.body, + self.body.ref_root(), + ), } } } @@ -398,10 +422,16 @@ impl SyntaxStatic for Nodes { } impl Nodes { - fn block(&mut self, g: &mut Generator<'_>, brace_token: Brace) -> AnyBlock { + fn block( + &mut self, + g: &mut Generator<'_>, + brace_token: Brace, + bindings: Vec, + ) -> AnyBlock { g.block_with( brace_token, |g| { + g.declare_local_bindings(bindings); g.push_all(&mut self.0); }, true, @@ -1036,8 +1066,7 @@ impl BorrowExpr { quote!(&#expr) } ParenExprMode::Ref => { - let ref_ident = g.hoist_ref_expr(Paren::default(), &self.expr); - quote!(#ref_ident) + g.hoist_ref_expr(Paren::default(), &self.expr, ref_expr_root(&self.expr)) } } } diff --git a/crates/cheers/tests/main.rs b/crates/cheers/tests/main.rs index 9caeb63..ced0925 100644 --- a/crates/cheers/tests/main.rs +++ b/crates/cheers/tests/main.rs @@ -656,6 +656,130 @@ fn ref_expr_keeps_outer_values_available_across_nested_blocks() { assert_eq!(subtitle, "World"); } +#[test] +fn ref_expr_can_borrow_template_local_bindings() { + enum Rank { + Named(String), + Unranked, + } + + let title = "Hall".to_owned(); + let names = vec!["Dwalin".to_owned(), "Balin".to_owned()]; + let owner = Some("Thorin".to_owned()); + let ranks = [Rank::Named("Elder".to_owned()), Rank::Unranked]; + let result = html! { + @let heading = names.join(" & "); + h1 { (@&heading) } + ul { + @for name in &names { + li title=(@&name) { (@&name) " of " (@&title) } + } + } + @if let Some(owner) = &owner { + p { (@&owner) } + } + @for rank in &ranks { + @match rank { + Rank::Named(rank) => b { (@&rank) } + Rank::Unranked => i { "none" } + } + } + @let mut remaining = names.iter().rev(); + @while let Some(next) = remaining.next() { + span { (@&next) } + } + }; + + assert_eq!( + result.render().into_inner(), + concat!( + "

Dwalin & Balin

", + r#"
  • Dwalin of Hall
  • Balin of Hall
"#, + "

Thorin

", + "Eldernone", + "BalinDwalin", + ) + ); + assert_eq!(title, "Hall"); +} + +#[test] +fn ref_expr_can_borrow_template_local_bindings_in_component_props() { + #[derive(Cheers)] + struct Badge<'a> { + label: &'a str, + } + + impl<'a> Render for Badge<'a> { + fn render_to(&self, buffer: &mut Buffer) { + html! { + span { (self.label) } + } + .render_to(buffer); + } + } + + let names = vec!["Dwalin".to_owned(), "Balin".to_owned()]; + let result = html! { + @for name in names.iter().cloned() { + Badge label=(@&name); + } + }; + + assert_eq!( + result.render().into_inner(), + "DwalinBalin" + ); +} + +#[test] +fn ref_expr_hoists_field_names_that_match_template_local_bindings() { + struct User { + name: String, + } + + let user = User { + name: "Dwalin".to_owned(), + }; + let result = html! { + @for name in ["a"] { + i { (@&user.name) " " (name) } + } + }; + + assert_eq!(result.render().into_inner(), "Dwalin a"); + assert_eq!(user.name, "Dwalin"); +} + +#[test] +fn ref_expr_can_borrow_bindings_of_at_subpatterns() { + let values = [Some(7), None]; + let result = html! { + @for value in values { + @match value { + _whole @ Some(inner) => b { (@&inner) } + None => i { "none" } + } + } + }; + + assert_eq!(result.render().into_inner(), "7none"); +} + +#[test] +fn template_local_bindings_do_not_leak_out_of_their_block() { + let name = "outer".to_owned(); + let result = html! { + @for name in ["inner"] { + i { (name) } + } + b { (@&name) } + }; + + assert_eq!(result.render().into_inner(), "innerouter"); + assert_eq!(name, "outer"); +} + #[test] fn ref_expr_keeps_outer_value_available_in_component_prop_builders() { #[derive(Cheers)] @@ -1123,6 +1247,33 @@ async fn page_is_rendered() { ); } +#[tokio::test] +async fn ref_expr_can_borrow_async_leading_let_bindings() { + async fn main_page() -> cheers::prelude::AsyncLazy> + { + html! { + Base { + @async { + @let data = async { "Here!".to_owned() }.await; + div { (@&data) } + } @else { + div { "Wait for it..." } + } + } + } + } + + let mut result = main_page() + .await + .into_response() + .into_body() + .into_data_stream(); + let got = next_axum_chunk(&mut result).await; + assert!(got.contains("Wait for it..."), "{got}"); + let got = next_axum_chunk(&mut result).await; + assert!(got.contains("
Here!
"), "{got}"); +} + #[tokio::test] async fn page_async_block_is_streamed() { async fn main_page() -> cheers::prelude::AsyncLazy>