Skip to content

Commit dbfe3cc

Browse files
committed
feat(class): Getter/setter implementation #325
1 parent 32b0dd6 commit dbfe3cc

File tree

6 files changed

+282
-15
lines changed

6 files changed

+282
-15
lines changed

crates/macros/src/class.rs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -251,9 +251,25 @@ fn generate_registered_class_impl(
251251
&'static str, ::ext_php_rs::internal::property::PropertyInfo<'a, Self>
252252
> {
253253
use ::std::iter::FromIterator;
254-
::std::collections::HashMap::from_iter([
254+
use ::ext_php_rs::internal::class::PhpClassImpl;
255+
256+
// Start with field properties (instance fields only, not static)
257+
let mut props = ::std::collections::HashMap::from_iter([
255258
#(#instance_fields,)*
256-
])
259+
]);
260+
261+
// Add method properties (from #[php(getter)] and #[php(setter)])
262+
let method_props = ::ext_php_rs::internal::class::PhpClassImplCollector::<Self>::default()
263+
.get_method_props();
264+
for (name, prop) in method_props {
265+
props.insert(name, ::ext_php_rs::internal::property::PropertyInfo {
266+
prop,
267+
flags: ::ext_php_rs::flags::PropertyFlags::Public,
268+
docs: &[],
269+
});
270+
}
271+
272+
props
257273
}
258274

259275
#[must_use]

crates/macros/src/impl_.rs

Lines changed: 96 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,17 @@ impl MethodArgs {
116116
}
117117
}
118118

119+
/// A property getter or setter method.
120+
#[derive(Debug)]
121+
struct PropertyMethod<'a> {
122+
/// Property name in PHP (e.g., "name" for `get_name`/`set_name`).
123+
prop_name: String,
124+
/// The Rust method identifier.
125+
method_ident: &'a syn::Ident,
126+
/// Whether this is a getter (true) or setter (false).
127+
is_getter: bool,
128+
}
129+
119130
#[derive(Debug)]
120131
struct ParsedImpl<'a> {
121132
path: &'a syn::Path,
@@ -124,6 +135,8 @@ struct ParsedImpl<'a> {
124135
functions: Vec<FnBuilder>,
125136
constructor: Option<(Function<'a>, Option<Visibility>)>,
126137
constants: Vec<Constant<'a>>,
138+
/// Property getter/setter methods.
139+
properties: Vec<PropertyMethod<'a>>,
127140
}
128141

129142
#[derive(Debug, Eq, Hash, PartialEq)]
@@ -178,6 +191,7 @@ impl<'a> ParsedImpl<'a> {
178191
functions: Vec::default(),
179192
constructor: Option::default(),
180193
constants: Vec::default(),
194+
properties: Vec::default(),
181195
}
182196
}
183197

@@ -211,6 +225,32 @@ impl<'a> ParsedImpl<'a> {
211225
method.attrs.retain(|attr| !attr.path().is_ident("php"));
212226

213227
let opts = MethodArgs::new(name, attr);
228+
229+
// Handle getter/setter methods
230+
if matches!(opts.ty, MethodTy::Getter | MethodTy::Setter) {
231+
let is_getter = matches!(opts.ty, MethodTy::Getter);
232+
// Extract property name by stripping get_/set_ prefix
233+
let method_name = method.sig.ident.to_string();
234+
let prop_name = if is_getter {
235+
method_name
236+
.strip_prefix("get_")
237+
.unwrap_or(&method_name)
238+
.to_string()
239+
} else {
240+
method_name
241+
.strip_prefix("set_")
242+
.unwrap_or(&method_name)
243+
.to_string()
244+
};
245+
246+
self.properties.push(PropertyMethod {
247+
prop_name,
248+
method_ident: &method.sig.ident,
249+
is_getter,
250+
});
251+
continue;
252+
}
253+
214254
let args = Args::parse_from_fnargs(method.sig.inputs.iter(), opts.defaults)?;
215255
let mut func = Function::new(&method.sig, opts.name, args, opts.optional, docs);
216256

@@ -280,6 +320,59 @@ impl<'a> ParsedImpl<'a> {
280320
}
281321
});
282322

323+
// Group properties by name to combine getters and setters
324+
let mut prop_groups: HashMap<&str, (Option<&syn::Ident>, Option<&syn::Ident>)> =
325+
HashMap::new();
326+
for prop in &self.properties {
327+
let entry = prop_groups.entry(&prop.prop_name).or_default();
328+
if prop.is_getter {
329+
entry.0 = Some(prop.method_ident);
330+
} else {
331+
entry.1 = Some(prop.method_ident);
332+
}
333+
}
334+
335+
// Generate property creation code
336+
let property_inserts: Vec<TokenStream> = prop_groups
337+
.iter()
338+
.map(|(prop_name, (getter, setter))| {
339+
match (getter, setter) {
340+
(Some(getter_ident), Some(setter_ident)) => {
341+
// Both getter and setter - use combine
342+
quote! {
343+
props.insert(
344+
#prop_name,
345+
::ext_php_rs::props::Property::method_getter(#path::#getter_ident)
346+
.combine(::ext_php_rs::props::Property::method_setter(#path::#setter_ident))
347+
);
348+
}
349+
}
350+
(Some(getter_ident), None) => {
351+
// Only getter
352+
quote! {
353+
props.insert(
354+
#prop_name,
355+
::ext_php_rs::props::Property::method_getter(#path::#getter_ident)
356+
);
357+
}
358+
}
359+
(None, Some(setter_ident)) => {
360+
// Only setter
361+
quote! {
362+
props.insert(
363+
#prop_name,
364+
::ext_php_rs::props::Property::method_setter(#path::#setter_ident)
365+
);
366+
}
367+
}
368+
(None, None) => {
369+
// Should not happen
370+
quote! {}
371+
}
372+
}
373+
})
374+
.collect();
375+
283376
quote! {
284377
impl ::ext_php_rs::internal::class::PhpClassImpl<#path>
285378
for ::ext_php_rs::internal::class::PhpClassImplCollector<#path>
@@ -291,7 +384,9 @@ impl<'a> ParsedImpl<'a> {
291384
}
292385

293386
fn get_method_props<'a>(self) -> ::std::collections::HashMap<&'static str, ::ext_php_rs::props::Property<'a, #path>> {
294-
todo!()
387+
let mut props = ::std::collections::HashMap::new();
388+
#(#property_inserts)*
389+
props
295390
}
296391

297392
fn get_constructor(self) -> ::std::option::Option<::ext_php_rs::class::ConstructorMeta<#path>> {

crates/macros/tests/expand/class.expanded.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,24 @@ impl ::ext_php_rs::class::RegisteredClass for MyClass {
2525
::ext_php_rs::internal::property::PropertyInfo<'a, Self>,
2626
> {
2727
use ::std::iter::FromIterator;
28-
::std::collections::HashMap::from_iter([])
28+
use ::ext_php_rs::internal::class::PhpClassImpl;
29+
let mut props = ::std::collections::HashMap::from_iter([]);
30+
let method_props = ::ext_php_rs::internal::class::PhpClassImplCollector::<
31+
Self,
32+
>::default()
33+
.get_method_props();
34+
for (name, prop) in method_props {
35+
props
36+
.insert(
37+
name,
38+
::ext_php_rs::internal::property::PropertyInfo {
39+
prop,
40+
flags: ::ext_php_rs::flags::PropertyFlags::Public,
41+
docs: &[],
42+
},
43+
);
44+
}
45+
props
2946
}
3047
#[must_use]
3148
fn static_properties() -> &'static [(

src/props.rs

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,121 @@ impl<'a, T: 'a> Property<'a, T> {
167167
Self::Method { get, set }
168168
}
169169

170+
/// Creates a getter-only method property.
171+
///
172+
/// This is useful when the getter returns a type that only implements
173+
/// [`IntoZval`] but not [`FromZval`] for all lifetimes, such as
174+
/// `&'static str`.
175+
///
176+
/// # Parameters
177+
///
178+
/// * `get` - Function used to get the value of the property.
179+
///
180+
/// # Examples
181+
///
182+
/// ```no_run
183+
/// # use ext_php_rs::props::Property;
184+
/// struct Test;
185+
///
186+
/// impl Test {
187+
/// pub fn get_prop(&self) -> &'static str {
188+
/// "Hello"
189+
/// }
190+
/// }
191+
///
192+
/// let prop: Property<Test> = Property::method_getter(Test::get_prop);
193+
/// ```
194+
#[must_use]
195+
pub fn method_getter<V>(get: fn(&T) -> V) -> Self
196+
where
197+
V: IntoZval + 'a,
198+
{
199+
let get = Box::new(move |self_: &T, retval: &mut Zval| -> PhpResult {
200+
let value = get(self_);
201+
value
202+
.set_zval(retval, false)
203+
.map_err(|e| format!("Failed to return property value to PHP: {e:?}"))?;
204+
Ok(())
205+
}) as Box<dyn Fn(&T, &mut Zval) -> PhpResult + Send + Sync + 'a>;
206+
207+
Self::Method {
208+
get: Some(get),
209+
set: None,
210+
}
211+
}
212+
213+
/// Creates a setter-only method property.
214+
///
215+
/// This is useful when the setter accepts a type that only implements
216+
/// [`FromZval`] but not [`IntoZval`].
217+
///
218+
/// # Parameters
219+
///
220+
/// * `set` - Function used to set the value of the property.
221+
///
222+
/// # Examples
223+
///
224+
/// ```no_run
225+
/// # use ext_php_rs::props::Property;
226+
/// struct Test {
227+
/// value: String,
228+
/// }
229+
///
230+
/// impl Test {
231+
/// pub fn set_prop(&mut self, val: String) {
232+
/// self.value = val;
233+
/// }
234+
/// }
235+
///
236+
/// let prop: Property<Test> = Property::method_setter(Test::set_prop);
237+
/// ```
238+
#[must_use]
239+
pub fn method_setter<V>(set: fn(&mut T, V)) -> Self
240+
where
241+
for<'b> V: FromZval<'b> + 'a,
242+
{
243+
let set = Box::new(move |self_: &mut T, value: &Zval| -> PhpResult {
244+
let val = V::from_zval(value)
245+
.ok_or("Unable to convert property value into required type.")?;
246+
set(self_, val);
247+
Ok(())
248+
}) as Box<dyn Fn(&mut T, &Zval) -> PhpResult + Send + Sync + 'a>;
249+
250+
Self::Method {
251+
get: None,
252+
set: Some(set),
253+
}
254+
}
255+
256+
/// Adds a setter to an existing getter-only property, or combines with
257+
/// another property.
258+
///
259+
/// # Parameters
260+
///
261+
/// * `other` - Another property to combine with this one. If `other` has a
262+
/// getter and this property doesn't, the getter from `other` is used. If
263+
/// `other` has a setter and this property doesn't, the setter from
264+
/// `other` is used.
265+
///
266+
/// # Returns
267+
///
268+
/// A new property with the combined getters and setters.
269+
#[must_use]
270+
pub fn combine(self, other: Self) -> Self {
271+
match (self, other) {
272+
(Property::Method { get: g1, set: s1 }, Property::Method { get: g2, set: s2 }) => {
273+
Property::Method {
274+
get: g1.or(g2),
275+
set: s1.or(s2),
276+
}
277+
}
278+
// If either is a field property, prefer the method property
279+
(Property::Field(_), other @ Property::Method { .. }) => other,
280+
// If first is method or both are fields, prefer the first one
281+
(this @ (Property::Method { .. } | Property::Field(_)), Property::Field(_)) => this,
282+
}
283+
}
284+
170285
/// Attempts to retrieve the value of the property from the given object
171286
/// `self_`.
172287
///

tests/src/integration/class/class.php

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,25 +6,26 @@
66
$class = test_class('lorem ipsum', 2022);
77
assert($class instanceof TestClass);
88

9-
// Tests getter/setter
10-
assert($class->getString() === 'lorem ipsum');
11-
$class->setString('dolor et');
12-
assert($class->getString() === 'dolor et');
9+
// Tests getter/setter as properties (get_string -> $class->string property)
10+
assert($class->string === 'lorem ipsum');
11+
$class->string = 'dolor et';
12+
assert($class->string === 'dolor et');
1313
$class->selfRef("foo");
14-
assert($class->getString() === 'Changed to foo');
14+
assert($class->string === 'Changed to foo');
1515
$class->selfMultiRef("bar");
16-
assert($class->getString() === 'Changed to bar');
16+
assert($class->string === 'Changed to bar');
1717

1818
// Test method returning Self (new instance)
1919
$newClass = $class->withString('new string');
2020
assert($newClass instanceof TestClass, 'withString should return TestClass instance');
21-
assert($newClass->getString() === 'new string', 'new instance should have new string');
22-
assert($class->getString() === 'Changed to bar', 'original instance should be unchanged');
21+
assert($newClass->string === 'new string', 'new instance should have new string');
22+
assert($class->string === 'Changed to bar', 'original instance should be unchanged');
2323
assert($newClass !== $class, 'should be different instances');
2424

25-
assert($class->getNumber() === 2022);
26-
$class->setNumber(2023);
27-
assert($class->getNumber() === 2023);
25+
// Test number getter/setter as property (from #[php(getter)]/[php(setter)])
26+
assert($class->number === 2022);
27+
$class->number = 2023;
28+
assert($class->number === 2023);
2829

2930
var_dump($class);
3031
// Tests #prop decorator
@@ -156,3 +157,7 @@
156157
assert(strpos($output, 'publicNum') !== false, 'var_dump should show public property');
157158
// Private properties should show as ClassName::propertyName in var_dump
158159
// Protected properties should show with * prefix
160+
161+
// Test issue #325 - returning &'static str from getter
162+
$staticStrClass = new TestClassStaticStrGetter();
163+
assert($staticStrClass->static_value === 'Hello from static str');

tests/src/integration/class/mod.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,24 @@ impl TestPropertyVisibility {
322322
}
323323
}
324324

325+
/// Test class for issue #325 - returning &'static str from getter
326+
#[php_class]
327+
pub struct TestClassStaticStrGetter;
328+
329+
#[php_impl]
330+
impl TestClassStaticStrGetter {
331+
pub fn __construct() -> Self {
332+
Self
333+
}
334+
335+
/// This getter returns a &'static str which previously failed to compile
336+
/// due to "implementation of `FromZval` is not general enough" error.
337+
#[php(getter)]
338+
pub fn get_static_value(&self) -> &'static str {
339+
"Hello from static str"
340+
}
341+
}
342+
325343
pub fn build_module(builder: ModuleBuilder) -> ModuleBuilder {
326344
builder
327345
.class::<TestClass>()
@@ -333,6 +351,7 @@ pub fn build_module(builder: ModuleBuilder) -> ModuleBuilder {
333351
.class::<TestStaticProps>()
334352
.class::<FluentBuilder>()
335353
.class::<TestPropertyVisibility>()
354+
.class::<TestClassStaticStrGetter>()
336355
.function(wrap_function!(test_class))
337356
.function(wrap_function!(throw_exception))
338357
}

0 commit comments

Comments
 (0)