feat(class): Getter/setter implementation #325

Author: kakserpom

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::<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]

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<FnBuilder>,
     constructor: Option<(Function<'a>, Option<Visibility>)>,
     constants: Vec<Constant<'a>>,
+    /// Property getter/setter methods.
+    properties: Vec<PropertyMethod<'a>>,
 }
 
 #[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<TokenStream> = 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>> {

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<

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<Test> = Property::method_getter(Test::get_prop);
+    /// ```
+    #[must_use]
+    pub fn method_getter<V>(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<dyn Fn(&T, &mut Zval) -> 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<Test> = Property::method_setter(Test::set_prop);
+    /// ```
+    #[must_use]
+    pub fn method_setter<V>(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<dyn Fn(&mut T, &Zval) -> 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_`.
     ///

tests/sapi.rs

@@ -166,7 +166,7 @@ fn test_sapi_multithread() {
                     Ok(zval) => {
                         assert!(zval.is_string());
                         let string = zval.string().unwrap();
-                        let output = string.to_string();
+                        let output = string.clone();
                         assert_eq!(output, format!("Hello, thread-{i}!"));
 
                         results.lock().unwrap().push((i, output));

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');

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::<TestClass>()
@@ -183,6 +201,7 @@ pub fn build_module(builder: ModuleBuilder) -> ModuleBuilder {
         .class::<TestClassExtendsImpl>()
         .class::<TestClassMethodVisibility>()
         .class::<TestClassProtectedConstruct>()
+        .class::<TestClassStaticStrGetter>()
         .function(wrap_function!(test_class))
         .function(wrap_function!(throw_exception))
 }