From f24f0a06e43b37a113860f8c5dd9dd0deac6c0ba Mon Sep 17 00:00:00 2001 From: Vasily Zorin Date: Sun, 21 Dec 2025 00:40:01 +0700 Subject: [PATCH] feat(class): Getter/setter implementation #325 --- crates/macros/src/class.rs | 20 +++- crates/macros/src/impl_.rs | 97 +++++++++++++++- crates/macros/tests/expand/class.expanded.rs | 19 ++- src/props.rs | 115 +++++++++++++++++++ tests/src/integration/class/class.php | 22 ++-- tests/src/integration/class/mod.rs | 19 +++ 6 files changed, 279 insertions(+), 13 deletions(-) diff --git a/crates/macros/src/class.rs b/crates/macros/src/class.rs index 4264d22c5f..72f631d7db 100644 --- a/crates/macros/src/class.rs +++ b/crates/macros/src/class.rs @@ -203,9 +203,25 @@ fn generate_registered_class_impl( &'static str, ::ext_php_rs::internal::property::PropertyInfo<'a, Self> > { use ::std::iter::FromIterator; - ::std::collections::HashMap::from_iter([ + use ::ext_php_rs::internal::class::PhpClassImpl; + + // Start with field properties + let mut props = ::std::collections::HashMap::from_iter([ #(#fields,)* - ]) + ]); + + // Add method properties (from #[php(getter)] and #[php(setter)]) + let method_props = ::ext_php_rs::internal::class::PhpClassImplCollector::::default() + .get_method_props(); + for (name, prop) in method_props { + props.insert(name, ::ext_php_rs::internal::property::PropertyInfo { + prop, + flags: ::ext_php_rs::flags::PropertyFlags::Public, + docs: &[], + }); + } + + props } #[inline] diff --git a/crates/macros/src/impl_.rs b/crates/macros/src/impl_.rs index 1d96871493..c94054d133 100644 --- a/crates/macros/src/impl_.rs +++ b/crates/macros/src/impl_.rs @@ -114,6 +114,17 @@ impl MethodArgs { } } +/// A property getter or setter method. +#[derive(Debug)] +struct PropertyMethod<'a> { + /// Property name in PHP (e.g., "name" for `get_name`/`set_name`). + prop_name: String, + /// The Rust method identifier. + method_ident: &'a syn::Ident, + /// Whether this is a getter (true) or setter (false). + is_getter: bool, +} + #[derive(Debug)] struct ParsedImpl<'a> { path: &'a syn::Path, @@ -122,6 +133,8 @@ struct ParsedImpl<'a> { functions: Vec, constructor: Option<(Function<'a>, Option)>, constants: Vec>, + /// Property getter/setter methods. + properties: Vec>, } #[derive(Debug, Eq, Hash, PartialEq)] @@ -176,6 +189,7 @@ impl<'a> ParsedImpl<'a> { functions: Vec::default(), constructor: Option::default(), constants: Vec::default(), + properties: Vec::default(), } } @@ -206,6 +220,32 @@ impl<'a> ParsedImpl<'a> { method.attrs.retain(|attr| !attr.path().is_ident("php")); let opts = MethodArgs::new(name, attr); + + // Handle getter/setter methods + if matches!(opts.ty, MethodTy::Getter | MethodTy::Setter) { + let is_getter = matches!(opts.ty, MethodTy::Getter); + // Extract property name by stripping get_/set_ prefix + let method_name = method.sig.ident.to_string(); + let prop_name = if is_getter { + method_name + .strip_prefix("get_") + .unwrap_or(&method_name) + .to_string() + } else { + method_name + .strip_prefix("set_") + .unwrap_or(&method_name) + .to_string() + }; + + self.properties.push(PropertyMethod { + prop_name, + method_ident: &method.sig.ident, + is_getter, + }); + continue; + } + let args = Args::parse_from_fnargs(method.sig.inputs.iter(), opts.defaults)?; let mut func = Function::new(&method.sig, opts.name, args, opts.optional, docs); @@ -275,6 +315,59 @@ impl<'a> ParsedImpl<'a> { } }); + // Group properties by name to combine getters and setters + let mut prop_groups: HashMap<&str, (Option<&syn::Ident>, Option<&syn::Ident>)> = + HashMap::new(); + for prop in &self.properties { + let entry = prop_groups.entry(&prop.prop_name).or_default(); + if prop.is_getter { + entry.0 = Some(prop.method_ident); + } else { + entry.1 = Some(prop.method_ident); + } + } + + // Generate property creation code + let property_inserts: Vec = prop_groups + .iter() + .map(|(prop_name, (getter, setter))| { + match (getter, setter) { + (Some(getter_ident), Some(setter_ident)) => { + // Both getter and setter - use combine + quote! { + props.insert( + #prop_name, + ::ext_php_rs::props::Property::method_getter(#path::#getter_ident) + .combine(::ext_php_rs::props::Property::method_setter(#path::#setter_ident)) + ); + } + } + (Some(getter_ident), None) => { + // Only getter + quote! { + props.insert( + #prop_name, + ::ext_php_rs::props::Property::method_getter(#path::#getter_ident) + ); + } + } + (None, Some(setter_ident)) => { + // Only setter + quote! { + props.insert( + #prop_name, + ::ext_php_rs::props::Property::method_setter(#path::#setter_ident) + ); + } + } + (None, None) => { + // Should not happen + quote! {} + } + } + }) + .collect(); + quote! { impl ::ext_php_rs::internal::class::PhpClassImpl<#path> for ::ext_php_rs::internal::class::PhpClassImplCollector<#path> @@ -286,7 +379,9 @@ impl<'a> ParsedImpl<'a> { } fn get_method_props<'a>(self) -> ::std::collections::HashMap<&'static str, ::ext_php_rs::props::Property<'a, #path>> { - todo!() + let mut props = ::std::collections::HashMap::new(); + #(#property_inserts)* + props } fn get_constructor(self) -> ::std::option::Option<::ext_php_rs::class::ConstructorMeta<#path>> { diff --git a/crates/macros/tests/expand/class.expanded.rs b/crates/macros/tests/expand/class.expanded.rs index f08ca30a61..a9e975c76b 100644 --- a/crates/macros/tests/expand/class.expanded.rs +++ b/crates/macros/tests/expand/class.expanded.rs @@ -25,7 +25,24 @@ impl ::ext_php_rs::class::RegisteredClass for MyClass { ::ext_php_rs::internal::property::PropertyInfo<'a, Self>, > { use ::std::iter::FromIterator; - ::std::collections::HashMap::from_iter([]) + use ::ext_php_rs::internal::class::PhpClassImpl; + let mut props = ::std::collections::HashMap::from_iter([]); + let method_props = ::ext_php_rs::internal::class::PhpClassImplCollector::< + Self, + >::default() + .get_method_props(); + for (name, prop) in method_props { + props + .insert( + name, + ::ext_php_rs::internal::property::PropertyInfo { + prop, + flags: ::ext_php_rs::flags::PropertyFlags::Public, + docs: &[], + }, + ); + } + props } #[inline] fn method_builders() -> ::std::vec::Vec< diff --git a/src/props.rs b/src/props.rs index 974eb197e8..39d69959fc 100644 --- a/src/props.rs +++ b/src/props.rs @@ -167,6 +167,121 @@ impl<'a, T: 'a> Property<'a, T> { Self::Method { get, set } } + /// Creates a getter-only method property. + /// + /// This is useful when the getter returns a type that only implements + /// [`IntoZval`] but not [`FromZval`] for all lifetimes, such as + /// `&'static str`. + /// + /// # Parameters + /// + /// * `get` - Function used to get the value of the property. + /// + /// # Examples + /// + /// ```no_run + /// # use ext_php_rs::props::Property; + /// struct Test; + /// + /// impl Test { + /// pub fn get_prop(&self) -> &'static str { + /// "Hello" + /// } + /// } + /// + /// let prop: Property = Property::method_getter(Test::get_prop); + /// ``` + #[must_use] + pub fn method_getter(get: fn(&T) -> V) -> Self + where + V: IntoZval + 'a, + { + let get = Box::new(move |self_: &T, retval: &mut Zval| -> PhpResult { + let value = get(self_); + value + .set_zval(retval, false) + .map_err(|e| format!("Failed to return property value to PHP: {e:?}"))?; + Ok(()) + }) as Box PhpResult + Send + Sync + 'a>; + + Self::Method { + get: Some(get), + set: None, + } + } + + /// Creates a setter-only method property. + /// + /// This is useful when the setter accepts a type that only implements + /// [`FromZval`] but not [`IntoZval`]. + /// + /// # Parameters + /// + /// * `set` - Function used to set the value of the property. + /// + /// # Examples + /// + /// ```no_run + /// # use ext_php_rs::props::Property; + /// struct Test { + /// value: String, + /// } + /// + /// impl Test { + /// pub fn set_prop(&mut self, val: String) { + /// self.value = val; + /// } + /// } + /// + /// let prop: Property = Property::method_setter(Test::set_prop); + /// ``` + #[must_use] + pub fn method_setter(set: fn(&mut T, V)) -> Self + where + for<'b> V: FromZval<'b> + 'a, + { + let set = Box::new(move |self_: &mut T, value: &Zval| -> PhpResult { + let val = V::from_zval(value) + .ok_or("Unable to convert property value into required type.")?; + set(self_, val); + Ok(()) + }) as Box PhpResult + Send + Sync + 'a>; + + Self::Method { + get: None, + set: Some(set), + } + } + + /// Adds a setter to an existing getter-only property, or combines with + /// another property. + /// + /// # Parameters + /// + /// * `other` - Another property to combine with this one. If `other` has a + /// getter and this property doesn't, the getter from `other` is used. If + /// `other` has a setter and this property doesn't, the setter from + /// `other` is used. + /// + /// # Returns + /// + /// A new property with the combined getters and setters. + #[must_use] + pub fn combine(self, other: Self) -> Self { + match (self, other) { + (Property::Method { get: g1, set: s1 }, Property::Method { get: g2, set: s2 }) => { + Property::Method { + get: g1.or(g2), + set: s1.or(s2), + } + } + // If either is a field property, prefer the method property + (Property::Field(_), other @ Property::Method { .. }) => other, + // If first is method or both are fields, prefer the first one + (this @ (Property::Method { .. } | Property::Field(_)), Property::Field(_)) => this, + } + } + /// Attempts to retrieve the value of the property from the given object /// `self_`. /// diff --git a/tests/src/integration/class/class.php b/tests/src/integration/class/class.php index d6c6ee6c1f..bbebc91b09 100644 --- a/tests/src/integration/class/class.php +++ b/tests/src/integration/class/class.php @@ -6,18 +6,18 @@ $class = test_class('lorem ipsum', 2022); assert($class instanceof TestClass); -// Tests getter/setter -assert($class->getString() === 'lorem ipsum'); -$class->setString('dolor et'); -assert($class->getString() === 'dolor et'); +// Tests getter/setter as properties (get_string -> $class->string property) +assert($class->string === 'lorem ipsum'); +$class->string = 'dolor et'; +assert($class->string === 'dolor et'); $class->selfRef("foo"); -assert($class->getString() === 'Changed to foo'); +assert($class->string === 'Changed to foo'); $class->selfMultiRef("bar"); -assert($class->getString() === 'Changed to bar'); +assert($class->string === 'Changed to bar'); -assert($class->getNumber() === 2022); -$class->setNumber(2023); -assert($class->getNumber() === 2023); +assert($class->number === 2022); +$class->number = 2023; +assert($class->number === 2023); var_dump($class); // Tests #prop decorator @@ -51,3 +51,7 @@ $classReflection = new ReflectionClass(TestClassProtectedConstruct::class); assert($classReflection->getMethod('__construct')->isProtected()); + +// Test issue #325 - returning &'static str from getter +$staticStrClass = new TestClassStaticStrGetter(); +assert($staticStrClass->static_value === 'Hello from static str'); diff --git a/tests/src/integration/class/mod.rs b/tests/src/integration/class/mod.rs index 3c3a2bb0c5..537dd1ee1a 100644 --- a/tests/src/integration/class/mod.rs +++ b/tests/src/integration/class/mod.rs @@ -175,6 +175,24 @@ impl TestClassProtectedConstruct { } } +/// Test class for issue #325 - returning &'static str from getter +#[php_class] +pub struct TestClassStaticStrGetter; + +#[php_impl] +impl TestClassStaticStrGetter { + pub fn __construct() -> Self { + Self + } + + /// This getter returns a &'static str which previously failed to compile + /// due to "implementation of `FromZval` is not general enough" error. + #[php(getter)] + pub fn get_static_value(&self) -> &'static str { + "Hello from static str" + } +} + pub fn build_module(builder: ModuleBuilder) -> ModuleBuilder { builder .class::() @@ -183,6 +201,7 @@ pub fn build_module(builder: ModuleBuilder) -> ModuleBuilder { .class::() .class::() .class::() + .class::() .function(wrap_function!(test_class)) .function(wrap_function!(throw_exception)) }