fix(class): Return Self ($this) #502

Author: kakserpom

crates/macros/src/function.rs

@@ -11,6 +11,21 @@ use crate::parsing::{PhpRename, RenameRule, Visibility};
 use crate::prelude::*;
 use crate::syn_ext::DropLifetimes;
 
+/// Checks if the return type is a reference to Self (`&Self` or `&mut Self`).
+/// This is used to detect methods that return `$this` in PHP.
+fn returns_self_ref(output: Option<&Type>) -> bool {
+    let Some(ty) = output else {
+        return false;
+    };
+    if let Type::Reference(ref_) = ty
+        && let Type::Path(path) = &*ref_.elem
+        && let Some(segment) = path.path.segments.last()
+    {
+        return segment.ident == "Self";
+    }
+    false
+}
+
 pub fn wrap(input: &syn::Path) -> Result<TokenStream> {
     let Some(func_name) = input.get_ident() else {
         bail!(input => "Pass a PHP function name into `wrap_function!()`.");
@@ -145,7 +160,7 @@ impl<'a> Function<'a> {
             .map(TypedArg::arg_builder)
             .collect::<Vec<_>>();
 
-        let returns = self.build_returns();
+        let returns = self.build_returns(None);
         let docs = if self.docs.is_empty() {
             quote! {}
         } else {
@@ -166,7 +181,7 @@ impl<'a> Function<'a> {
     }
 
     /// Generates the function builder for the function.
-    pub fn function_builder(&self, call_type: CallType) -> TokenStream {
+    pub fn function_builder(&self, call_type: &CallType) -> TokenStream {
         let name = &self.name;
         let (required, not_required) = self.args.split_args(self.optional.as_ref());
 
@@ -188,7 +203,7 @@ impl<'a> Function<'a> {
             .map(TypedArg::arg_builder)
             .collect::<Vec<_>>();
 
-        let returns = self.build_returns();
+        let returns = self.build_returns(Some(call_type));
         let result = self.build_result(call_type, required, not_required);
         let docs = if self.docs.is_empty() {
             quote! {}
@@ -199,24 +214,54 @@ impl<'a> Function<'a> {
             }
         };
 
+        // Check if this method returns &Self or &mut Self
+        // In that case, we need to return `this` (the ZendClassObject) directly
+        let returns_this = returns_self_ref(self.output)
+            && matches!(
+                call_type,
+                CallType::Method {
+                    receiver: MethodReceiver::Class | MethodReceiver::ZendClassObject,
+                    ..
+                }
+            );
+
+        let handler_body = if returns_this {
+            quote! {
+                use ::ext_php_rs::convert::IntoZval;
+
+                #(#arg_declarations)*
+                #result
+
+                // The method returns &Self or &mut Self, use `this` directly
+                if let Err(e) = this.set_zval(retval, false) {
+                    let e: ::ext_php_rs::exception::PhpException = e.into();
+                    e.throw().expect("Failed to throw PHP exception.");
+                }
+            }
+        } else {
+            quote! {
+                use ::ext_php_rs::convert::IntoZval;
+
+                #(#arg_declarations)*
+                let result = {
+                    #result
+                };
+
+                if let Err(e) = result.set_zval(retval, false) {
+                    let e: ::ext_php_rs::exception::PhpException = e.into();
+                    e.throw().expect("Failed to throw PHP exception.");
+                }
+            }
+        };
+
         quote! {
             ::ext_php_rs::builders::FunctionBuilder::new(#name, {
                 ::ext_php_rs::zend_fastcall! {
                     extern fn handler(
                         ex: &mut ::ext_php_rs::zend::ExecuteData,
                         retval: &mut ::ext_php_rs::types::Zval,
                     ) {
-                        use ::ext_php_rs::convert::IntoZval;
-
-                        #(#arg_declarations)*
-                        let result = {
-                            #result
-                        };
-
-                        if let Err(e) = result.set_zval(retval, false) {
-                            let e: ::ext_php_rs::exception::PhpException = e.into();
-                            e.throw().expect("Failed to throw PHP exception.");
-                        }
+                        #handler_body
                     }
                 }
                 handler
@@ -229,9 +274,24 @@ impl<'a> Function<'a> {
         }
     }
 
-    fn build_returns(&self) -> Option<TokenStream> {
+    fn build_returns(&self, call_type: Option<&CallType>) -> Option<TokenStream> {
         self.output.cloned().map(|mut output| {
             output.drop_lifetimes();
+
+            // If returning &Self or &mut Self from a method, use the class type
+            // for return type information since we return `this` (ZendClassObject)
+            if returns_self_ref(self.output)
+                && let Some(CallType::Method { class, .. }) = call_type
+            {
+                return quote! {
+                    .returns(
+                        <&mut ::ext_php_rs::types::ZendClassObject<#class> as ::ext_php_rs::convert::IntoZval>::TYPE,
+                        false,
+                        <&mut ::ext_php_rs::types::ZendClassObject<#class> as ::ext_php_rs::convert::IntoZval>::NULLABLE,
+                    )
+                };
+            }
+
             quote! {
                 .returns(
                     <#output as ::ext_php_rs::convert::IntoZval>::TYPE,
@@ -244,7 +304,7 @@ impl<'a> Function<'a> {
 
     fn build_result(
         &self,
-        call_type: CallType,
+        call_type: &CallType,
         required: &[TypedArg<'_>],
         not_required: &[TypedArg<'_>],
     ) -> TokenStream {
@@ -274,6 +334,9 @@ impl<'a> Function<'a> {
             })
         });
 
+        // Check if this method returns &Self or &mut Self
+        let returns_this = returns_self_ref(self.output);
+
         match call_type {
             CallType::Function => quote! {
                 let parse = ex.parser()
@@ -306,15 +369,27 @@ impl<'a> Function<'a> {
                         };
                     },
                 };
-                let call = match receiver {
-                    MethodReceiver::Static => {
+
+                // When returning &Self or &mut Self, discard the return value
+                // (we'll use `this` directly in the handler)
+                let call = match (receiver, returns_this) {
+                    (MethodReceiver::Static, _) => {
                         quote! { #class::#ident(#({#arg_accessors}),*) }
                     }
-                    MethodReceiver::Class => quote! { this.#ident(#({#arg_accessors}),*) },
-                    MethodReceiver::ZendClassObject => {
+                    (MethodReceiver::Class, true) => {
+                        quote! { let _ = this.#ident(#({#arg_accessors}),*); }
+                    }
+                    (MethodReceiver::Class, false) => {
+                        quote! { this.#ident(#({#arg_accessors}),*) }
+                    }
+                    (MethodReceiver::ZendClassObject, true) => {
+                        quote! { let _ = #class::#ident(this, #({#arg_accessors}),*); }
+                    }
+                    (MethodReceiver::ZendClassObject, false) => {
                         quote! { #class::#ident(this, #({#arg_accessors}),*) }
                     }
                 };
+
                 quote! {
                     #this
                     let parse_result = parse
@@ -336,7 +411,7 @@ impl<'a> Function<'a> {
     /// Generates a struct and impl for the `PhpFunction` trait.
     pub fn php_function_impl(&self) -> TokenStream {
         let internal_ident = self.internal_ident();
-        let builder = self.function_builder(CallType::Function);
+        let builder = self.function_builder(&CallType::Function);
 
         quote! {
             #[doc(hidden)]

crates/macros/src/impl_.rs

@@ -241,7 +241,7 @@ impl<'a> ParsedImpl<'a> {
                             modifiers.insert(MethodModifier::Abstract);
                         }
 
-                        let builder = func.function_builder(call_type);
+                        let builder = func.function_builder(&call_type);
 
                         self.functions.push(FnBuilder {
                             builder,

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

@@ -51,3 +51,24 @@
 
 $classReflection = new ReflectionClass(TestClassProtectedConstruct::class);
 assert($classReflection->getMethod('__construct')->isProtected());
+
+// Test FluentBuilder - returning $this for method chaining (Issue #502)
+$builder = new FluentBuilder();
+assert($builder->getValue() === 0);
+assert($builder->getName() === '');
+
+// Test single method call returning $this
+$result = $builder->setValue(42);
+assert($result === $builder, 'setValue should return $this');
+assert($builder->getValue() === 42);
+
+// Test fluent interface / method chaining
+$builder2 = new FluentBuilder();
+$chainResult = $builder2->setValue(100)->setName('test');
+assert($chainResult === $builder2, 'Chained methods should return $this');
+assert($builder2->getValue() === 100);
+assert($builder2->getName() === 'test');
+
+// Test returning &Self (immutable reference)
+$selfRef = $builder2->getSelf();
+assert($selfRef === $builder2, 'getSelf should return $this');

tests/src/integration/class/mod.rs

@@ -175,6 +175,51 @@ impl TestClassProtectedConstruct {
     }
 }
 
+/// Test class for returning $this (Issue #502)
+/// This demonstrates returning &mut Self from methods for fluent interfaces
+#[php_class]
+pub struct FluentBuilder {
+    value: i32,
+    name: String,
+}
+
+#[php_impl]
+impl FluentBuilder {
+    pub fn __construct() -> Self {
+        Self {
+            value: 0,
+            name: String::new(),
+        }
+    }
+
+    /// Set value and return $this for method chaining
+    pub fn set_value(&mut self, value: i32) -> &mut Self {
+        self.value = value;
+        self
+    }
+
+    /// Set name and return $this for method chaining
+    pub fn set_name(&mut self, name: String) -> &mut Self {
+        self.name = name;
+        self
+    }
+
+    /// Get the current value
+    pub fn get_value(&self) -> i32 {
+        self.value
+    }
+
+    /// Get the current name
+    pub fn get_name(&self) -> String {
+        self.name.clone()
+    }
+
+    /// Test returning &Self (immutable reference to self)
+    pub fn get_self(&self) -> &Self {
+        self
+    }
+}
+
 pub fn build_module(builder: ModuleBuilder) -> ModuleBuilder {
     builder
         .class::<TestClass>()
@@ -183,6 +228,7 @@ pub fn build_module(builder: ModuleBuilder) -> ModuleBuilder {
         .class::<TestClassExtendsImpl>()
         .class::<TestClassMethodVisibility>()
         .class::<TestClassProtectedConstruct>()
+        .class::<FluentBuilder>()
         .function(wrap_function!(test_class))
         .function(wrap_function!(throw_exception))
 }