Skip to content

Commit e97cc25

Browse files
committed
fix(class): Return Self ($this) #502
1 parent b511af6 commit e97cc25

File tree

4 files changed

+164
-22
lines changed

4 files changed

+164
-22
lines changed

crates/macros/src/function.rs

Lines changed: 96 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,21 @@ use crate::parsing::{PhpRename, RenameRule, Visibility};
1111
use crate::prelude::*;
1212
use crate::syn_ext::DropLifetimes;
1313

14+
/// Checks if the return type is a reference to Self (`&Self` or `&mut Self`).
15+
/// This is used to detect methods that return `$this` in PHP.
16+
fn returns_self_ref(output: Option<&Type>) -> bool {
17+
let Some(ty) = output else {
18+
return false;
19+
};
20+
if let Type::Reference(ref_) = ty
21+
&& let Type::Path(path) = &*ref_.elem
22+
&& let Some(segment) = path.path.segments.last()
23+
{
24+
return segment.ident == "Self";
25+
}
26+
false
27+
}
28+
1429
pub fn wrap(input: &syn::Path) -> Result<TokenStream> {
1530
let Some(func_name) = input.get_ident() else {
1631
bail!(input => "Pass a PHP function name into `wrap_function!()`.");
@@ -145,7 +160,7 @@ impl<'a> Function<'a> {
145160
.map(TypedArg::arg_builder)
146161
.collect::<Vec<_>>();
147162

148-
let returns = self.build_returns();
163+
let returns = self.build_returns(None);
149164
let docs = if self.docs.is_empty() {
150165
quote! {}
151166
} else {
@@ -166,7 +181,7 @@ impl<'a> Function<'a> {
166181
}
167182

168183
/// Generates the function builder for the function.
169-
pub fn function_builder(&self, call_type: CallType) -> TokenStream {
184+
pub fn function_builder(&self, call_type: &CallType) -> TokenStream {
170185
let name = &self.name;
171186
let (required, not_required) = self.args.split_args(self.optional.as_ref());
172187

@@ -188,7 +203,7 @@ impl<'a> Function<'a> {
188203
.map(TypedArg::arg_builder)
189204
.collect::<Vec<_>>();
190205

191-
let returns = self.build_returns();
206+
let returns = self.build_returns(Some(call_type));
192207
let result = self.build_result(call_type, required, not_required);
193208
let docs = if self.docs.is_empty() {
194209
quote! {}
@@ -199,24 +214,54 @@ impl<'a> Function<'a> {
199214
}
200215
};
201216

217+
// Check if this method returns &Self or &mut Self
218+
// In that case, we need to return `this` (the ZendClassObject) directly
219+
let returns_this = returns_self_ref(self.output)
220+
&& matches!(
221+
call_type,
222+
CallType::Method {
223+
receiver: MethodReceiver::Class | MethodReceiver::ZendClassObject,
224+
..
225+
}
226+
);
227+
228+
let handler_body = if returns_this {
229+
quote! {
230+
use ::ext_php_rs::convert::IntoZval;
231+
232+
#(#arg_declarations)*
233+
#result
234+
235+
// The method returns &Self or &mut Self, use `this` directly
236+
if let Err(e) = this.set_zval(retval, false) {
237+
let e: ::ext_php_rs::exception::PhpException = e.into();
238+
e.throw().expect("Failed to throw PHP exception.");
239+
}
240+
}
241+
} else {
242+
quote! {
243+
use ::ext_php_rs::convert::IntoZval;
244+
245+
#(#arg_declarations)*
246+
let result = {
247+
#result
248+
};
249+
250+
if let Err(e) = result.set_zval(retval, false) {
251+
let e: ::ext_php_rs::exception::PhpException = e.into();
252+
e.throw().expect("Failed to throw PHP exception.");
253+
}
254+
}
255+
};
256+
202257
quote! {
203258
::ext_php_rs::builders::FunctionBuilder::new(#name, {
204259
::ext_php_rs::zend_fastcall! {
205260
extern fn handler(
206261
ex: &mut ::ext_php_rs::zend::ExecuteData,
207262
retval: &mut ::ext_php_rs::types::Zval,
208263
) {
209-
use ::ext_php_rs::convert::IntoZval;
210-
211-
#(#arg_declarations)*
212-
let result = {
213-
#result
214-
};
215-
216-
if let Err(e) = result.set_zval(retval, false) {
217-
let e: ::ext_php_rs::exception::PhpException = e.into();
218-
e.throw().expect("Failed to throw PHP exception.");
219-
}
264+
#handler_body
220265
}
221266
}
222267
handler
@@ -229,9 +274,24 @@ impl<'a> Function<'a> {
229274
}
230275
}
231276

232-
fn build_returns(&self) -> Option<TokenStream> {
277+
fn build_returns(&self, call_type: Option<&CallType>) -> Option<TokenStream> {
233278
self.output.cloned().map(|mut output| {
234279
output.drop_lifetimes();
280+
281+
// If returning &Self or &mut Self from a method, use the class type
282+
// for return type information since we return `this` (ZendClassObject)
283+
if returns_self_ref(self.output)
284+
&& let Some(CallType::Method { class, .. }) = call_type
285+
{
286+
return quote! {
287+
.returns(
288+
<&mut ::ext_php_rs::types::ZendClassObject<#class> as ::ext_php_rs::convert::IntoZval>::TYPE,
289+
false,
290+
<&mut ::ext_php_rs::types::ZendClassObject<#class> as ::ext_php_rs::convert::IntoZval>::NULLABLE,
291+
)
292+
};
293+
}
294+
235295
quote! {
236296
.returns(
237297
<#output as ::ext_php_rs::convert::IntoZval>::TYPE,
@@ -244,7 +304,7 @@ impl<'a> Function<'a> {
244304

245305
fn build_result(
246306
&self,
247-
call_type: CallType,
307+
call_type: &CallType,
248308
required: &[TypedArg<'_>],
249309
not_required: &[TypedArg<'_>],
250310
) -> TokenStream {
@@ -274,6 +334,9 @@ impl<'a> Function<'a> {
274334
})
275335
});
276336

337+
// Check if this method returns &Self or &mut Self
338+
let returns_this = returns_self_ref(self.output);
339+
277340
match call_type {
278341
CallType::Function => quote! {
279342
let parse = ex.parser()
@@ -306,15 +369,27 @@ impl<'a> Function<'a> {
306369
};
307370
},
308371
};
309-
let call = match receiver {
310-
MethodReceiver::Static => {
372+
373+
// When returning &Self or &mut Self, discard the return value
374+
// (we'll use `this` directly in the handler)
375+
let call = match (receiver, returns_this) {
376+
(MethodReceiver::Static, _) => {
311377
quote! { #class::#ident(#({#arg_accessors}),*) }
312378
}
313-
MethodReceiver::Class => quote! { this.#ident(#({#arg_accessors}),*) },
314-
MethodReceiver::ZendClassObject => {
379+
(MethodReceiver::Class, true) => {
380+
quote! { let _ = this.#ident(#({#arg_accessors}),*); }
381+
}
382+
(MethodReceiver::Class, false) => {
383+
quote! { this.#ident(#({#arg_accessors}),*) }
384+
}
385+
(MethodReceiver::ZendClassObject, true) => {
386+
quote! { let _ = #class::#ident(this, #({#arg_accessors}),*); }
387+
}
388+
(MethodReceiver::ZendClassObject, false) => {
315389
quote! { #class::#ident(this, #({#arg_accessors}),*) }
316390
}
317391
};
392+
318393
quote! {
319394
#this
320395
let parse_result = parse
@@ -336,7 +411,7 @@ impl<'a> Function<'a> {
336411
/// Generates a struct and impl for the `PhpFunction` trait.
337412
pub fn php_function_impl(&self) -> TokenStream {
338413
let internal_ident = self.internal_ident();
339-
let builder = self.function_builder(CallType::Function);
414+
let builder = self.function_builder(&CallType::Function);
340415

341416
quote! {
342417
#[doc(hidden)]

crates/macros/src/impl_.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ impl<'a> ParsedImpl<'a> {
241241
modifiers.insert(MethodModifier::Abstract);
242242
}
243243

244-
let builder = func.function_builder(call_type);
244+
let builder = func.function_builder(&call_type);
245245

246246
self.functions.push(FnBuilder {
247247
builder,

tests/src/integration/class/class.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,3 +94,24 @@
9494

9595
TestStaticProps::setCounter(100);
9696
assert(TestStaticProps::$staticCounter === 100, 'PHP should see Rust-set value');
97+
98+
// Test FluentBuilder - returning $this for method chaining (Issue #502)
99+
$builder = new FluentBuilder();
100+
assert($builder->getValue() === 0);
101+
assert($builder->getName() === '');
102+
103+
// Test single method call returning $this
104+
$result = $builder->setValue(42);
105+
assert($result === $builder, 'setValue should return $this');
106+
assert($builder->getValue() === 42);
107+
108+
// Test fluent interface / method chaining
109+
$builder2 = new FluentBuilder();
110+
$chainResult = $builder2->setValue(100)->setName('test');
111+
assert($chainResult === $builder2, 'Chained methods should return $this');
112+
assert($builder2->getValue() === 100);
113+
assert($builder2->getName() === 'test');
114+
115+
// Test returning &Self (immutable reference)
116+
$selfRef = $builder2->getSelf();
117+
assert($selfRef === $builder2, 'getSelf should return $this');

tests/src/integration/class/mod.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,51 @@ impl TestStaticProps {
223223
}
224224
}
225225

226+
/// Test class for returning $this (Issue #502)
227+
/// This demonstrates returning &mut Self from methods for fluent interfaces
228+
#[php_class]
229+
pub struct FluentBuilder {
230+
value: i32,
231+
name: String,
232+
}
233+
234+
#[php_impl]
235+
impl FluentBuilder {
236+
pub fn __construct() -> Self {
237+
Self {
238+
value: 0,
239+
name: String::new(),
240+
}
241+
}
242+
243+
/// Set value and return $this for method chaining
244+
pub fn set_value(&mut self, value: i32) -> &mut Self {
245+
self.value = value;
246+
self
247+
}
248+
249+
/// Set name and return $this for method chaining
250+
pub fn set_name(&mut self, name: String) -> &mut Self {
251+
self.name = name;
252+
self
253+
}
254+
255+
/// Get the current value
256+
pub fn get_value(&self) -> i32 {
257+
self.value
258+
}
259+
260+
/// Get the current name
261+
pub fn get_name(&self) -> String {
262+
self.name.clone()
263+
}
264+
265+
/// Test returning &Self (immutable reference to self)
266+
pub fn get_self(&self) -> &Self {
267+
self
268+
}
269+
}
270+
226271
pub fn build_module(builder: ModuleBuilder) -> ModuleBuilder {
227272
builder
228273
.class::<TestClass>()
@@ -232,6 +277,7 @@ pub fn build_module(builder: ModuleBuilder) -> ModuleBuilder {
232277
.class::<TestClassMethodVisibility>()
233278
.class::<TestClassProtectedConstruct>()
234279
.class::<TestStaticProps>()
280+
.class::<FluentBuilder>()
235281
.function(wrap_function!(test_class))
236282
.function(wrap_function!(throw_exception))
237283
}

0 commit comments

Comments
 (0)