Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions crates/macros/src/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
97 changes: 96 additions & 1 deletion crates/macros/src/impl_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)]
Expand Down Expand Up @@ -176,6 +189,7 @@ impl<'a> ParsedImpl<'a> {
functions: Vec::default(),
constructor: Option::default(),
constants: Vec::default(),
properties: Vec::default(),
}
}

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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>
Expand All @@ -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>> {
Expand Down
19 changes: 18 additions & 1 deletion crates/macros/tests/expand/class.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<
Expand Down
115 changes: 115 additions & 0 deletions src/props.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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_`.
///
Expand Down
22 changes: 13 additions & 9 deletions tests/src/integration/class/class.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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');
19 changes: 19 additions & 0 deletions tests/src/integration/class/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>()
Expand All @@ -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))
}
Expand Down