From d79ebbea6f3fb312d99b268d3f9ed53cd35d067d Mon Sep 17 00:00:00 2001 From: Martijn Faassen Date: Mon, 21 Oct 2024 15:34:06 +0200 Subject: [PATCH 01/11] Don't use symbolic links for git. Since git 2.23 .gitignore that is a symbolic link is ignored. I made them explicit now (though I'm not sure they're even needed) --- crates/cli/.gitignore | 7 ++++++- crates/macros/.gitignore | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) mode change 120000 => 100644 crates/cli/.gitignore mode change 120000 => 100644 crates/macros/.gitignore diff --git a/crates/cli/.gitignore b/crates/cli/.gitignore deleted file mode 120000 index 6ef08f9d15..0000000000 --- a/crates/cli/.gitignore +++ /dev/null @@ -1 +0,0 @@ -../../.gitignore \ No newline at end of file diff --git a/crates/cli/.gitignore b/crates/cli/.gitignore new file mode 100644 index 0000000000..89e3a707f6 --- /dev/null +++ b/crates/cli/.gitignore @@ -0,0 +1,6 @@ +/target +Cargo.lock +/.vscode +/.idea +/tmp +expand.rs \ No newline at end of file diff --git a/crates/macros/.gitignore b/crates/macros/.gitignore deleted file mode 120000 index 6ef08f9d15..0000000000 --- a/crates/macros/.gitignore +++ /dev/null @@ -1 +0,0 @@ -../../.gitignore \ No newline at end of file diff --git a/crates/macros/.gitignore b/crates/macros/.gitignore new file mode 100644 index 0000000000..89e3a707f6 --- /dev/null +++ b/crates/macros/.gitignore @@ -0,0 +1,6 @@ +/target +Cargo.lock +/.vscode +/.idea +/tmp +expand.rs \ No newline at end of file From 3a66ba3217f8b4623d674e3a8ff83be3487677ce Mon Sep 17 00:00:00 2001 From: Martijn Faassen Date: Mon, 21 Oct 2024 15:54:55 +0200 Subject: [PATCH 02/11] Preparatory: factor out generate_class so we can later call it earlier. --- crates/macros/src/startup_function.rs | 220 +++++++++++++------------- 1 file changed, 114 insertions(+), 106 deletions(-) diff --git a/crates/macros/src/startup_function.rs b/crates/macros/src/startup_function.rs index 695753d6d9..c7c6304eb9 100644 --- a/crates/macros/src/startup_function.rs +++ b/crates/macros/src/startup_function.rs @@ -61,120 +61,128 @@ pub fn parser(args: Option, input: ItemFn) -> Result Ok(func) } +fn generate_class(name: &str, class: &Class) -> Result { + let class_name = &class.class_name; + let ident = Ident::new(name, Span::call_site()); + let methods = class.methods.iter().map(|method| { + let builder = method.get_builder(&ident); + let flags = method.get_flags(); + quote! { .method(#builder.unwrap(), #flags) } + }); + let constants = class.constants.iter().map(|constant| { + let name = &constant.name; + let val = constant.val_tokens(); + quote! { .constant(#name, #val).unwrap() } + }); + let parent = { + if let Some(parent) = &class.parent { + let expr: Expr = syn::parse_str(parent) + .map_err(|_| anyhow!("Invalid expression given for `{}` parent", class_name))?; + Some(quote! { .extends(#expr) }) + } else { + None + } + }; + let interfaces = class + .interfaces + .iter() + .map(|interface| { + let expr: Expr = syn::parse_str(interface).map_err(|_| { + anyhow!( + "Invalid expression given for `{}` interface: `{}`", + class_name, + interface + ) + })?; + Ok(quote! { .implements(#expr) }) + }) + .collect::>>()?; + // TODO(david): register properties for reflection (somehow) + // let properties = class + // .properties + // .iter() + // .map(|(name, (default, flags))| { + // let default_expr: Expr = syn::parse_str(default).map_err(|_| { + // anyhow!( + // "Invalid default value given for property `{}` type: `{}`", + // name, + // default + // ) + // })?; + // let flags_expr: Expr = syn::parse_str( + // flags + // .as_ref() + // .map(|flags| flags.as_str()) + // .unwrap_or("PropertyFlags::Public"), + // ) + // .map_err(|_| { + // anyhow!( + // "Invalid default value given for property `{}` type: `{}`", + // name, + // default + // ) + // })?; + + // Ok(quote! { .property(#name, #default_expr, #flags_expr) }) + // }) + // .collect::>>()?; + let class_modifier = class.modifier.as_ref().map(|modifier| { + let modifier = Ident::new(modifier, Span::call_site()); + quote! { + let builder = #modifier(builder).expect(concat!("Failed to build class ", #class_name)); + } + }); + + let flags = { + if let Some(flags) = &class.flags { + let mut name = "::ext_php_rs::flags::ClassFlags::".to_owned(); + name.push_str(flags); + let expr: Expr = syn::parse_str(&name) + .map_err(|_| anyhow!("Invalid expression given for `{}` flags", class_name))?; + Some(quote! { .flags(#expr) }) + } else { + None + } + }; + + let object_override = { + if let Some(flags) = &class.flags { + if flags == "Interface" { + None + } else { + Some(quote! { .object_override::<#ident>() }) + } + } else { + Some(quote! { .object_override::<#ident>() }) + } + }; + + Ok(quote! { + let builder = ::ext_php_rs::builders::ClassBuilder::new(#class_name) + #(#methods)* + #(#constants)* + #(#interfaces)* + // #(#properties)* + #parent + #flags + #object_override + ; + #class_modifier + }) +} + /// Returns a vector of `ClassBuilder`s for each class. fn build_classes(classes: &HashMap) -> Result> { classes .iter() .map(|(name, class)| { - let Class { class_name, .. } = &class; - let ident = Ident::new(name, Span::call_site()); let meta = Ident::new(&format!("_{name}_META"), Span::call_site()); - let methods = class.methods.iter().map(|method| { - let builder = method.get_builder(&ident); - let flags = method.get_flags(); - quote! { .method(#builder.unwrap(), #flags) } - }); - let constants = class.constants.iter().map(|constant| { - let name = &constant.name; - let val = constant.val_tokens(); - quote! { .constant(#name, #val).unwrap() } - }); - let parent = { - if let Some(parent) = &class.parent { - let expr: Expr = syn::parse_str(parent).map_err(|_| { - anyhow!("Invalid expression given for `{}` parent", class_name) - })?; - Some(quote! { .extends(#expr) }) - } else { - None - } - }; - let interfaces = class - .interfaces - .iter() - .map(|interface| { - let expr: Expr = syn::parse_str(interface).map_err(|_| { - anyhow!( - "Invalid expression given for `{}` interface: `{}`", - class_name, - interface - ) - })?; - Ok(quote! { .implements(#expr) }) - }) - .collect::>>()?; - // TODO(david): register properties for reflection (somehow) - // let properties = class - // .properties - // .iter() - // .map(|(name, (default, flags))| { - // let default_expr: Expr = syn::parse_str(default).map_err(|_| { - // anyhow!( - // "Invalid default value given for property `{}` type: `{}`", - // name, - // default - // ) - // })?; - // let flags_expr: Expr = syn::parse_str( - // flags - // .as_ref() - // .map(|flags| flags.as_str()) - // .unwrap_or("PropertyFlags::Public"), - // ) - // .map_err(|_| { - // anyhow!( - // "Invalid default value given for property `{}` type: `{}`", - // name, - // default - // ) - // })?; - - // Ok(quote! { .property(#name, #default_expr, #flags_expr) }) - // }) - // .collect::>>()?; - let class_modifier = class.modifier.as_ref().map(|modifier| { - let modifier = Ident::new(modifier, Span::call_site()); - quote! { - let builder = #modifier(builder).expect(concat!("Failed to build class ", #class_name)); - } - }); - - let flags = { - if let Some(flags) = &class.flags { - let mut name = "::ext_php_rs::flags::ClassFlags::".to_owned(); - name.push_str(flags); - let expr: Expr = syn::parse_str(&name).map_err(|_| { - anyhow!("Invalid expression given for `{}` flags", class_name) - })?; - Some(quote! { .flags(#expr) }) - } else { - None - } - }; - - let object_override = { - if let Some(flags) = &class.flags { - if flags == "Interface" { - None - } else { - Some(quote! { .object_override::<#ident>() }) - } - } else { - Some(quote! { .object_override::<#ident>() }) - } - }; + + let class_name = &class.class_name; + let generated = generate_class(name, class)?; Ok(quote! {{ - let builder = ::ext_php_rs::builders::ClassBuilder::new(#class_name) - #(#methods)* - #(#constants)* - #(#interfaces)* - // #(#properties)* - #parent - #flags - #object_override - ; - #class_modifier + #generated let class = builder.build() .expect(concat!("Unable to build class `", #class_name, "`")); From 624146fc5435df3b6cc4f89c7de2ce12de4e2764 Mon Sep 17 00:00:00 2001 From: Martijn Faassen Date: Tue, 22 Oct 2024 13:32:19 +0200 Subject: [PATCH 03/11] function::parse doesn't need to return a tuple So simplify the return value. --- crates/macros/src/function.rs | 4 ++-- crates/macros/src/lib.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/macros/src/function.rs b/crates/macros/src/function.rs index 670d32fedc..4423d65bcb 100644 --- a/crates/macros/src/function.rs +++ b/crates/macros/src/function.rs @@ -40,7 +40,7 @@ pub struct Function { pub output: Option<(String, bool)>, } -pub fn parser(args: AttributeArgs, input: ItemFn) -> Result<(TokenStream, Function)> { +pub fn parser(args: AttributeArgs, input: ItemFn) -> Result { let attr_args = match AttrArgs::from_list(&args) { Ok(args) => args, Err(e) => bail!("Unable to parse attribute arguments: {:?}", e), @@ -106,7 +106,7 @@ pub fn parser(args: AttributeArgs, input: ItemFn) -> Result<(TokenStream, Functi state.functions.push(function.clone()); - Ok((func, function)) + Ok(func) } fn build_args( diff --git a/crates/macros/src/lib.rs b/crates/macros/src/lib.rs index 2e6deaf8ab..828ab91c27 100644 --- a/crates/macros/src/lib.rs +++ b/crates/macros/src/lib.rs @@ -69,7 +69,7 @@ pub fn php_function(args: TokenStream, input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as ItemFn); match function::parser(args, input) { - Ok((parsed, _)) => parsed, + Ok(parsed) => parsed, Err(e) => syn::Error::new(Span::call_site(), e).to_compile_error(), } .into() From 470de0ea96bdb791949c6b56b92b4c5290c27b6c Mon Sep 17 00:00:00 2001 From: Martijn Faassen Date: Tue, 22 Oct 2024 13:36:32 +0200 Subject: [PATCH 04/11] Call it class_tokenstream which is a bit more clear. --- crates/macros/src/startup_function.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/macros/src/startup_function.rs b/crates/macros/src/startup_function.rs index c7c6304eb9..418e277d05 100644 --- a/crates/macros/src/startup_function.rs +++ b/crates/macros/src/startup_function.rs @@ -61,7 +61,7 @@ pub fn parser(args: Option, input: ItemFn) -> Result Ok(func) } -fn generate_class(name: &str, class: &Class) -> Result { +fn class_tokenstream(name: &str, class: &Class) -> Result { let class_name = &class.class_name; let ident = Ident::new(name, Span::call_site()); let methods = class.methods.iter().map(|method| { @@ -179,7 +179,7 @@ fn build_classes(classes: &HashMap) -> Result> { let meta = Ident::new(&format!("_{name}_META"), Span::call_site()); let class_name = &class.class_name; - let generated = generate_class(name, class)?; + let generated = class_tokenstream(name, class)?; Ok(quote! {{ #generated From ccd81ebbffc25851d9065154952346b1853ddb59 Mon Sep 17 00:00:00 2001 From: Martijn Faassen Date: Tue, 22 Oct 2024 13:54:27 +0200 Subject: [PATCH 05/11] Factor out a stateless prepare function from the parser function. Our goal is to get pure functions that don't affect STATE. --- crates/macros/src/function.rs | 36 +++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/crates/macros/src/function.rs b/crates/macros/src/function.rs index 4423d65bcb..0479d5dc43 100644 --- a/crates/macros/src/function.rs +++ b/crates/macros/src/function.rs @@ -40,12 +40,7 @@ pub struct Function { pub output: Option<(String, bool)>, } -pub fn parser(args: AttributeArgs, input: ItemFn) -> Result { - let attr_args = match AttrArgs::from_list(&args) { - Ok(args) => args, - Err(e) => bail!("Unable to parse attribute arguments: {:?}", e), - }; - +fn prepare(attr_args: AttrArgs, input: ItemFn) -> Result<(TokenStream, Function)> { let ItemFn { sig, .. } = &input; let Signature { ident, @@ -89,14 +84,14 @@ pub fn parser(args: AttributeArgs, input: ItemFn) -> Result { } }; - let mut state = STATE.lock(); - - if state.built_module && !attr_args.ignore_module { - bail!("The `#[php_module]` macro must be called last to ensure functions are registered. To ignore this error, pass the `ignore_module` option into this attribute invocation: `#[php_function(ignore_module)]`"); - } + let name = if let Some(name) = attr_args.name { + name + } else { + ident.to_string() + }; let function = Function { - name: attr_args.name.unwrap_or_else(|| ident.to_string()), + name, docs: get_docs(&input.attrs), ident: internal_ident.to_string(), args, @@ -104,6 +99,23 @@ pub fn parser(args: AttributeArgs, input: ItemFn) -> Result { output: return_type, }; + Ok((func, function)) +} + +pub fn parser(args: AttributeArgs, input: ItemFn) -> Result { + let attr_args = match AttrArgs::from_list(&args) { + Ok(args) => args, + Err(e) => bail!("Unable to parse attribute arguments: {:?}", e), + }; + let ignore_module = attr_args.ignore_module; + let (func, function) = prepare(attr_args, input)?; + + let mut state = STATE.lock(); + + if state.built_module && !ignore_module { + bail!("The `#[php_module]` macro must be called last to ensure functions are registered. To ignore this error, pass the `ignore_module` option into this attribute invocation: `#[php_function(ignore_module)]`"); + } + state.functions.push(function.clone()); Ok(func) From ec5735a64cac3ee2c74de54cd462b0b44c6ddbd6 Mon Sep 17 00:00:00 2001 From: Martijn Faassen Date: Tue, 22 Oct 2024 14:02:33 +0200 Subject: [PATCH 06/11] Pull stateless prepare out of class construction. --- crates/macros/src/class.rs | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/crates/macros/src/class.rs b/crates/macros/src/class.rs index c10a015429..a88740bafd 100644 --- a/crates/macros/src/class.rs +++ b/crates/macros/src/class.rs @@ -41,10 +41,7 @@ pub struct AttrArgs { flags: Option, } -pub fn parser(args: AttributeArgs, mut input: ItemStruct) -> Result { - let args = AttrArgs::from_list(&args) - .map_err(|e| anyhow!("Unable to parse attribute arguments: {:?}", e))?; - +fn prepare(args: AttrArgs, mut input: ItemStruct) -> Result<(TokenStream, Class)> { let mut parent = None; let mut interfaces = vec![]; let mut properties = HashMap::new(); @@ -132,6 +129,23 @@ pub fn parser(args: AttributeArgs, mut input: ItemStruct) -> Result ..Default::default() }; + Ok(( + quote! { + #input + + ::ext_php_rs::class_derives!(#ident); + }, + class, + )) +} + +pub fn parser(args: AttributeArgs, input: ItemStruct) -> Result { + let args = AttrArgs::from_list(&args) + .map_err(|e| anyhow!("Unable to parse attribute arguments: {:?}", e))?; + + let ident = &input.ident.clone(); + let (tokens, class) = prepare(args, input)?; + let mut state = STATE.lock(); if state.built_module { @@ -144,11 +158,7 @@ pub fn parser(args: AttributeArgs, mut input: ItemStruct) -> Result state.classes.insert(ident.to_string(), class); - Ok(quote! { - #input - - ::ext_php_rs::class_derives!(#ident); - }) + Ok(tokens) } #[derive(Debug)] From f1ba2715f673d518bbfed39d60377e255df55972 Mon Sep 17 00:00:00 2001 From: Martijn Faassen Date: Tue, 22 Oct 2024 15:09:24 +0200 Subject: [PATCH 07/11] A tricky refactor to pull out state. It turns out the dropping and relocking state in the right order is essential for the test suite to even complete... --- crates/macros/src/module.rs | 92 +++++++++++++++++++++---------------- 1 file changed, 53 insertions(+), 39 deletions(-) diff --git a/crates/macros/src/module.rs b/crates/macros/src/module.rs index f350fd472b..5a18416fc4 100644 --- a/crates/macros/src/module.rs +++ b/crates/macros/src/module.rs @@ -1,5 +1,3 @@ -use std::sync::MutexGuard; - use anyhow::{anyhow, bail, Result}; use proc_macro2::{Ident, Span, TokenStream}; use quote::quote; @@ -11,56 +9,36 @@ use crate::{ startup_function, State, STATE, }; -pub fn parser(input: ItemFn) -> Result { +fn prepare<'a>( + input: ItemFn, + functions: &[Function], + startup_fn: Option<&TokenStream>, + startup_function_name: Option<&str>, + classes: impl Iterator, + describe: &TokenStream, +) -> Result { let ItemFn { sig, block, .. } = input; let Signature { output, inputs, .. } = sig; let stmts = &block.stmts; - let mut state = STATE.lock(); - - if state.built_module { - bail!("You may only define a module with the `#[php_module]` attribute once."); - } - - state.built_module = true; - // Generate startup function if one hasn't already been tagged with the macro. - let startup_fn = if (!state.classes.is_empty() || !state.constants.is_empty()) - && state.startup_function.is_none() - { - drop(state); - - let parsed = syn::parse2(quote! { - fn php_module_startup() {} - }) - .map_err(|_| anyhow!("Unable to generate PHP module startup function."))?; - let startup = startup_function::parser(None, parsed)?; - state = STATE.lock(); - Some(startup) - } else { - None - }; - - let functions = state - .functions + let functions = functions .iter() .map(|func| func.get_builder()) .collect::>(); - let startup = state.startup_function.as_ref().map(|ident| { + let startup = startup_function_name.as_ref().map(|ident| { let ident = Ident::new(ident, Span::call_site()); quote! { .startup_function(#ident) } }); - let registered_classes_impls = state - .classes - .values() + let registered_classes_impls = classes .map(generate_registered_class_impl) .collect::>>()?; - let describe_fn = generate_stubs(&state); + let describe_fn = generate_stubs(describe); - let result = quote! { + Ok(quote! { #(#registered_classes_impls)* #startup_fn @@ -90,8 +68,46 @@ pub fn parser(input: ItemFn) -> Result { } #describe_fn + }) +} + +fn prepare_startup() -> Result { + let parsed = syn::parse2(quote! { + fn php_module_startup() {} + }) + .map_err(|_| anyhow!("Unable to generate PHP module startup function."))?; + startup_function::parser(None, parsed) +} + +pub fn parser(input: ItemFn) -> Result { + let mut state = STATE.lock(); + + if state.built_module { + bail!("You may only define a module with the `#[php_module]` attribute once."); + } + + state.built_module = true; + + // Generate startup function if one hasn't already been tagged with the macro. + let startup_fn = if (!state.classes.is_empty() || !state.constants.is_empty()) + && state.startup_function.is_none() + { + drop(state); + let prepared = prepare_startup()?; + state = STATE.lock(); + Some(prepared) + } else { + None }; - Ok(result) + + prepare( + input, + &state.functions, + startup_fn.as_ref(), + state.startup_function.as_deref(), + state.classes.values(), + &state.describe(), + ) } /// Generates an implementation for `RegisteredClass` on the given class. @@ -151,9 +167,7 @@ pub trait Describe { fn describe(&self) -> TokenStream; } -fn generate_stubs(state: &MutexGuard) -> TokenStream { - let module = state.describe(); - +fn generate_stubs(module: &TokenStream) -> TokenStream { quote! { #[cfg(debug_assertions)] #[no_mangle] From fbddbe1c306b416e36f9b82d7eb9bef955d5fad5 Mon Sep 17 00:00:00 2001 From: Martijn Faassen Date: Tue, 22 Oct 2024 16:01:32 +0200 Subject: [PATCH 08/11] Factor out stateless stuff from startup function. --- crates/macros/src/startup_function.rs | 36 +++++++++++++++++---------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/crates/macros/src/startup_function.rs b/crates/macros/src/startup_function.rs index 418e277d05..dee7a14cef 100644 --- a/crates/macros/src/startup_function.rs +++ b/crates/macros/src/startup_function.rs @@ -14,23 +14,18 @@ struct StartupArgs { before: bool, } -pub fn parser(args: Option, input: ItemFn) -> Result { - let args = if let Some(args) = args { - StartupArgs::from_list(&args) - .map_err(|e| anyhow!("Unable to parse attribute arguments: {:?}", e))? - } else { - StartupArgs::default() - }; - +fn prepare( + args: StartupArgs, + input: ItemFn, + classes: &HashMap, + constants: &[Constant], +) -> Result { let ItemFn { sig, block, .. } = input; let Signature { ident, .. } = sig; let stmts = &block.stmts; - let mut state = STATE.lock(); - state.startup_function = Some(ident.to_string()); - - let classes = build_classes(&state.classes)?; - let constants = build_constants(&state.constants); + let classes = build_classes(classes)?; + let constants = build_constants(constants); let (before, after) = if args.before { (Some(quote! { internal(ty, module_number); }), None) } else { @@ -61,6 +56,21 @@ pub fn parser(args: Option, input: ItemFn) -> Result Ok(func) } +pub fn parser(args: Option, input: ItemFn) -> Result { + let args = if let Some(args) = args { + StartupArgs::from_list(&args) + .map_err(|e| anyhow!("Unable to parse attribute arguments: {:?}", e))? + } else { + StartupArgs::default() + }; + + let mut state = STATE.lock(); + let ident = input.sig.ident.to_string(); + state.startup_function = Some(ident); + + prepare(args, input, &state.classes, &state.constants) +} + fn class_tokenstream(name: &str, class: &Class) -> Result { let class_name = &class.class_name; let ident = Ident::new(name, Span::call_site()); From 41eced918385a30be99f19484db15d1944412f66 Mon Sep 17 00:00:00 2001 From: Martijn Faassen Date: Tue, 22 Oct 2024 16:06:11 +0200 Subject: [PATCH 09/11] Factor out prepare from impl. Note that prepare isn't pure here as it mutates a class. We may want to rearrange things later. --- crates/macros/src/impl_.rs | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/crates/macros/src/impl_.rs b/crates/macros/src/impl_.rs index 49fb9dc3ea..8d1cc3df27 100644 --- a/crates/macros/src/impl_.rs +++ b/crates/macros/src/impl_.rs @@ -5,6 +5,7 @@ use quote::quote; use std::collections::HashMap; use syn::{Attribute, AttributeArgs, ItemImpl, Lit, Meta, NestedMeta}; +use crate::class::Class; use crate::helpers::get_docs; use crate::{ class::{Property, PropertyAttr}, @@ -94,10 +95,12 @@ pub enum PropAttrTy { Setter, } -pub fn parser(args: AttributeArgs, input: ItemImpl) -> Result { - let args = AttrArgs::from_list(&args) - .map_err(|e| anyhow!("Unable to parse attribute arguments: {:?}", e))?; - +// note: this takes a mutable argument as it mutates a class +fn prepare( + args: AttrArgs, + input: ItemImpl, + classes: &mut HashMap, +) -> Result { let ItemImpl { self_ty, items, .. } = input; let class_name = self_ty.to_token_stream().to_string(); @@ -105,15 +108,7 @@ pub fn parser(args: AttributeArgs, input: ItemImpl) -> Result { bail!("This macro cannot be used on trait implementations."); } - let mut state = crate::STATE.lock(); - - if state.startup_function.is_some() { - bail!( - "Impls must be declared before you declare your startup function and module function." - ); - } - - let class = state.classes.get_mut(&class_name).ok_or_else(|| { + let class = classes.get_mut(&class_name).ok_or_else(|| { anyhow!( "You must use `#[php_class]` on the struct before using this attribute on the impl." ) @@ -178,6 +173,21 @@ pub fn parser(args: AttributeArgs, input: ItemImpl) -> Result { Ok(output) } +pub fn parser(args: AttributeArgs, input: ItemImpl) -> Result { + let args = AttrArgs::from_list(&args) + .map_err(|e| anyhow!("Unable to parse attribute arguments: {:?}", e))?; + + let mut state = crate::STATE.lock(); + + if state.startup_function.is_some() { + bail!( + "Impls must be declared before you declare your startup function and module function." + ); + } + + prepare(args, input, &mut state.classes) +} + pub fn parse_attribute(attr: &Attribute) -> Result> { let name = attr.path.to_token_stream().to_string(); let meta = attr From 6e804b6874c754c7f01439d0ed43be70207d19a2 Mon Sep 17 00:00:00 2001 From: Martijn Faassen Date: Tue, 22 Oct 2024 16:09:26 +0200 Subject: [PATCH 10/11] It's nicer to have the check that the class is there yet earlier as it's state related. It's still going to need revision later as it depends on state modification to work. The class should only be modified once it is explicitly registered. --- crates/macros/src/impl_.rs | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/crates/macros/src/impl_.rs b/crates/macros/src/impl_.rs index 8d1cc3df27..474a21bfb6 100644 --- a/crates/macros/src/impl_.rs +++ b/crates/macros/src/impl_.rs @@ -96,24 +96,13 @@ pub enum PropAttrTy { } // note: this takes a mutable argument as it mutates a class -fn prepare( - args: AttrArgs, - input: ItemImpl, - classes: &mut HashMap, -) -> Result { +fn prepare(args: AttrArgs, input: ItemImpl, class: &mut Class) -> Result { let ItemImpl { self_ty, items, .. } = input; - let class_name = self_ty.to_token_stream().to_string(); if input.trait_.is_some() { bail!("This macro cannot be used on trait implementations."); } - let class = classes.get_mut(&class_name).ok_or_else(|| { - anyhow!( - "You must use `#[php_class]` on the struct before using this attribute on the impl." - ) - })?; - let tokens = items .into_iter() .map(|item| { @@ -185,7 +174,15 @@ pub fn parser(args: AttributeArgs, input: ItemImpl) -> Result { ); } - prepare(args, input, &mut state.classes) + let class_name = input.self_ty.to_token_stream().to_string(); + + let class = state.classes.get_mut(&class_name).ok_or_else(|| { + anyhow!( + "You must use `#[php_class]` on the struct before using this attribute on the impl." + ) + })?; + + prepare(args, input, class) } pub fn parse_attribute(attr: &Attribute) -> Result> { From c8a363d76363999967191f6d37ace1b4b17df20a Mon Sep 17 00:00:00 2001 From: Martijn Faassen Date: Tue, 22 Oct 2024 16:12:30 +0200 Subject: [PATCH 11/11] It's almost not worth it here, but factor out preparation from token stream generation. --- crates/macros/src/constant.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/crates/macros/src/constant.rs b/crates/macros/src/constant.rs index 3a9516c26b..ebc4bf1c2b 100644 --- a/crates/macros/src/constant.rs +++ b/crates/macros/src/constant.rs @@ -15,6 +15,13 @@ pub struct Constant { pub value: String, } +fn prepare(input: ItemConst) -> Result { + Ok(quote! { + #[allow(dead_code)] + #input + }) +} + pub fn parser(input: ItemConst) -> Result { let mut state = STATE.lock(); @@ -28,10 +35,7 @@ pub fn parser(input: ItemConst) -> Result { value: input.expr.to_token_stream().to_string(), }); - Ok(quote! { - #[allow(dead_code)] - #input - }) + prepare(input) } impl Constant {