From d25361d34d97b71dd2237b721980583cb2577b7d Mon Sep 17 00:00:00 2001 From: slhx agent Date: Mon, 25 May 2026 22:17:08 +0200 Subject: [PATCH] feat(derive): check generated handler params Emit handle_param facts for handled data-* attributes and reject handlers that omit generated param arguments. Cover the generated param boundary with unit and compile-fail tests. req: derive_handler/001 req: test/003 --- slhx-build/src/lib.rs | 40 ++++++++++++++- slhx-derive/src/lib.rs | 81 +++++++++++++++++++++++++++++-- slhx-derive/tests/compile_fail.rs | 52 ++++++++++++++++++++ 3 files changed, 168 insertions(+), 5 deletions(-) diff --git a/slhx-build/src/lib.rs b/slhx-build/src/lib.rs index 45365c4..210d383 100644 --- a/slhx-build/src/lib.rs +++ b/slhx-build/src/lib.rs @@ -72,6 +72,7 @@ struct Resources { slots: BTreeMap, handles: BTreeMap, handle_forms: BTreeMap, + handle_params: BTreeMap>, forms: BTreeMap, atoms: BTreeMap, classes: BTreeMap, @@ -139,6 +140,7 @@ impl Resources { reject_unkeyed_loop(surface, node.scope, path, "handle", &name)?; let canonical = canonical_symbol(root, path, &name); self.insert_handle(canonical, name.clone(), component.clone())?; + self.insert_handle_params(&name, &node.attrs)?; if tag == "form" { let form_name = static_attr(&node.attrs, "data-slhx-form").unwrap_or_else(|| name.clone()); @@ -211,6 +213,25 @@ impl Resources { } } + fn insert_handle_params(&mut self, handle: &str, attrs: &[SurfaceAttribute]) -> io::Result<()> { + let Some(handle_ident) = rust_ident(handle) else { + return Ok(()); + }; + for attr in attrs { + if !is_handle_param_attr(attr) { + continue; + } + let Some(param) = data_param_ident(&attr.name) else { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!("invalid handler param attribute `{}`; expected data-* name usable from Rust", attr.name), + )); + }; + self.handle_params.entry(handle_ident.clone()).or_default().insert(param); + } + Ok(()) + } + fn insert_form( &mut self, symbol: String, @@ -518,6 +539,11 @@ fn __slhx_attr(tag: &str, attr: &str) -> Option<::std::string::String> { for (handle_ident, form_ident) in &self.handle_forms { out.push_str(&format!("handle_form\t{handle_ident}\t{form_ident}\n")); } + for (handle_ident, params) in &self.handle_params { + for param in params { + out.push_str(&format!("handle_param\t{handle_ident}\t{param}\n")); + } + } for res in self.atoms.values() { out.push_str(&format!("atom\t{}\t{}\t{}\n", res.symbol, res.ident, res.id)); } @@ -750,6 +776,17 @@ fn class_ident(token: &str) -> Option { rust_ident(&ident) } +fn is_handle_param_attr(attr: &SurfaceAttribute) -> bool { + matches!(attr.origin, AttributeOrigin::Static | AttributeOrigin::Dynamic) + && attr.name.starts_with("data-") + && !attr.name.starts_with("data-slhx-") +} + +fn data_param_ident(name: &str) -> Option { + let data_name = name.strip_prefix("data-")?; + rust_ident(&data_name.replace('-', "_")) +} + fn is_inside_keyed_for(surface: &SurfaceDocument, mut scope: ScopeId) -> bool { loop { let Some(current) = surface.scopes.get(scope.0 as usize) else { @@ -890,7 +927,7 @@ mod tests { std::fs::create_dir_all(&templates).unwrap(); std::fs::write( templates.join("todo.heml"), - r#"
    "#, + r#"
      "#, ) .unwrap(); @@ -918,6 +955,7 @@ mod tests { assert!(syms.contains("atom\t")); assert!(syms.contains("\tfilter\t")); assert!(syms.contains("handle_form\tcreate\tnew_todo\n")); + assert!(syms.contains("handle_param\tdelete\ttodo_id\n")); let _ = std::fs::remove_dir_all(&base); } diff --git a/slhx-derive/src/lib.rs b/slhx-derive/src/lib.rs index 6698ddc..ddded58 100644 --- a/slhx-derive/src/lib.rs +++ b/slhx-derive/src/lib.rs @@ -1,7 +1,7 @@ use proc_macro::TokenStream; use quote::quote; use std::path::PathBuf; -use syn::{parse_macro_input, FnArg, ItemFn, ReturnType, Type}; +use syn::{parse_macro_input, FnArg, ItemFn, Pat, ReturnType, Type}; #[proc_macro_attribute] pub fn handler(_attr: TokenStream, item: TokenStream) -> TokenStream { @@ -57,6 +57,18 @@ pub fn handler(_attr: TokenStream, item: TokenStream) -> TokenStream { ) .into(); } + let missing_params = missing_handle_params(&syms_path, &name, &function); + if !missing_params.is_empty() { + let message = format!( + "slhx handler `{name}` is missing generated param argument(s): {}", + missing_params.join(", ") + ); + return quote!( + #function + compile_error!(#message); + ) + .into(); + } quote!(#function).into() } @@ -161,6 +173,51 @@ fn handle_requires_form(path: &PathBuf, ident: &str) -> bool { }) } +fn missing_handle_params(path: &PathBuf, ident: &str, function: &ItemFn) -> Vec { + let required = handle_params(path, ident); + if required.is_empty() { + return Vec::new(); + } + let args = handler_arg_names(function); + required + .into_iter() + .filter(|param| !args.contains(param)) + .collect() +} + +fn handle_params(path: &PathBuf, ident: &str) -> Vec { + let Ok(syms) = std::fs::read_to_string(path) else { + return Vec::new(); + }; + syms.lines() + .filter_map(|line| { + let mut fields = line.split('\t'); + if !matches!(fields.next(), Some("handle_param")) { + return None; + } + if fields.next()? != ident { + return None; + } + fields.next().map(ToOwned::to_owned) + }) + .collect() +} + +fn handler_arg_names(function: &ItemFn) -> Vec { + function + .sig + .inputs + .iter() + .filter_map(|arg| match arg { + FnArg::Typed(arg) => match arg.pat.as_ref() { + Pat::Ident(ident) => Some(ident.ident.to_string()), + _ => None, + }, + FnArg::Receiver(_) => None, + }) + .collect() +} + fn compile_error(message: &str) -> TokenStream { format!("compile_error!({message:?});") .parse() @@ -169,15 +226,15 @@ fn compile_error(message: &str) -> TokenStream { #[cfg(test)] mod tests { - use super::{handle_requires_form, has_form_param, has_non_unit_return, is_form_type, syms_contains_handle}; - use syn::{parse_quote, Type}; + use super::{handle_params, handle_requires_form, has_form_param, has_non_unit_return, is_form_type, missing_handle_params, syms_contains_handle}; + use syn::{parse_quote, ItemFn, Type}; #[test] fn syms_lookup_matches_handle_ident() { let path = std::env::temp_dir().join("slhx-derive-syms-test.syms"); std::fs::write( &path, - "slhx-syms-v1\nslot\ttemplates/a.heml::count\tcount\t1\nhandle\ttemplates/a.heml::create\tcreate\t2\nhandle_form\tcreate\tnew_todo\n", + "slhx-syms-v1\nslot\ttemplates/a.heml::count\tcount\t1\nhandle\ttemplates/a.heml::create\tcreate\t2\nhandle_form\tcreate\tnew_todo\nhandle_param\tcreate\ttodo_id\n", ) .unwrap(); @@ -185,6 +242,8 @@ mod tests { assert!(!syms_contains_handle(&path, "missing")); assert!(handle_requires_form(&path, "create")); assert!(!handle_requires_form(&path, "missing")); + assert_eq!(handle_params(&path, "create"), vec!["todo_id"]); + assert!(handle_params(&path, "missing").is_empty()); let _ = std::fs::remove_file(path); } @@ -210,4 +269,18 @@ mod tests { assert!(is_form_type(&real_form)); assert!(!is_form_type(&impostor)); } + + #[test] + fn handler_params_match_generated_param_names() { + let path = std::env::temp_dir().join("slhx-derive-param-test.syms"); + std::fs::write(&path, "slhx-syms-v1\nhandle_param\tshow\ttodo_id\nhandle_param\tshow\tmode\n") + .unwrap(); + let complete: ItemFn = parse_quote!(fn show(todo_id: String, mode: String) -> impl slhx::IntoEffect { slhx::EffectBatch::default() }); + let missing: ItemFn = parse_quote!(fn show(todo_id: String) -> impl slhx::IntoEffect { slhx::EffectBatch::default() }); + + assert!(missing_handle_params(&path, "show", &complete).is_empty()); + assert_eq!(missing_handle_params(&path, "show", &missing), vec!["mode"]); + + let _ = std::fs::remove_file(path); + } } diff --git a/slhx-derive/tests/compile_fail.rs b/slhx-derive/tests/compile_fail.rs index 3278b88..88b5d25 100644 --- a/slhx-derive/tests/compile_fail.rs +++ b/slhx-derive/tests/compile_fail.rs @@ -111,6 +111,58 @@ fn create() -> impl slhx::IntoEffect { ); } +#[test] +fn handler_requires_generated_param_arguments() { + // req: derive_handler/001 req: test/003 + let fixture = Fixture::new("slhx-derive-param-handler-fail"); + fixture.write( + "Cargo.toml", + &format!( + r#"[package] +name = "slhx-derive-param-handler-fail" +version = "0.0.0" +edition = "2021" + +[lib] +path = "src/lib.rs" + +[dependencies] +slhx = {{ path = {:?} }} +"#, + repo_path("slhx") + ), + ); + fixture.write( + "build.rs", + r#"fn main() { + let out = std::path::PathBuf::from(std::env::var_os("OUT_DIR").unwrap()); + std::fs::write( + out.join("slhx.syms"), + "slhx-syms-v1\nhandle\ttemplates/app.heml::show\tshow\t1\nhandle_param\tshow\ttodo_id\nhandle_param\tshow\tmode\n", + ) + .unwrap(); +} +"#, + ); + fixture.write( + "src/lib.rs", + r#"#[slhx::handler] +fn show(todo_id: String) -> impl slhx::IntoEffect { + slhx::EffectBatch::default() +} +"#, + ); + + let output = check_fixture(&fixture); + + assert!(!output.status.success(), "fixture unexpectedly compiled"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("slhx handler `show` is missing generated param argument(s): mode"), + "missing param diagnostic in stderr:\n{stderr}" + ); +} + #[test] fn form_handle_requires_form_parameter() { // req: form/004 req: form/006 req: test/003