diff --git a/crates/macros/src/function.rs b/crates/macros/src/function.rs index 6d5dd2cf4..853d6664b 100644 --- a/crates/macros/src/function.rs +++ b/crates/macros/src/function.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use darling::{FromAttributes, ToTokens}; use proc_macro2::{Ident, Span, TokenStream}; -use quote::{format_ident, quote}; +use quote::{format_ident, quote, quote_spanned}; use syn::spanned::Spanned as _; use syn::{Expr, FnArg, GenericArgument, ItemFn, PatType, PathArguments, Type, TypePath}; @@ -11,6 +11,37 @@ 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 + && path.path.segments.len() == 1 + && let Some(segment) = path.path.segments.last() + { + return segment.ident == "Self"; + } + false +} + +/// Checks if the return type is `Self` (not a reference). +/// This is used to detect methods that return a new instance of the same class. +fn returns_self(output: Option<&Type>) -> bool { + let Some(ty) = output else { + return false; + }; + if let Type::Path(path) = ty + && path.path.segments.len() == 1 + && let Some(segment) = path.path.segments.last() + { + return segment.ident == "Self"; + } + false +} + pub fn wrap(input: &syn::Path) -> Result { let Some(func_name) = input.get_ident() else { bail!(input => "Pass a PHP function name into `wrap_function!()`."); @@ -145,7 +176,7 @@ impl<'a> Function<'a> { .map(TypedArg::arg_builder) .collect::>(); - let returns = self.build_returns(); + let returns = self.build_returns(None); let docs = if self.docs.is_empty() { quote! {} } else { @@ -166,7 +197,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 +219,7 @@ impl<'a> Function<'a> { .map(TypedArg::arg_builder) .collect::>(); - 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,6 +230,62 @@ impl<'a> Function<'a> { } }; + // Static methods cannot return &Self or &mut Self + if returns_self_ref(self.output) + && let CallType::Method { + receiver: MethodReceiver::Static, + .. + } = call_type + && let Some(output) = self.output + { + return quote_spanned! { output.span() => + compile_error!( + "Static methods cannot return `&Self` or `&mut Self`. \ + Only instance methods can use fluent interface pattern returning `$this`." + ) + }; + } + + // 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! { @@ -206,17 +293,7 @@ impl<'a> Function<'a> { 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 +306,38 @@ impl<'a> Function<'a> { } } - fn build_returns(&self) -> Option { + fn build_returns(&self, call_type: Option<&CallType>) -> Option { 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, + ) + }; + } + + // If returning Self (new instance) from a method, replace Self with + // the actual class type since Self won't resolve in generated code + if returns_self(self.output) + && let Some(CallType::Method { class, .. }) = call_type + { + return quote! { + .returns( + <#class as ::ext_php_rs::convert::IntoZval>::TYPE, + false, + <#class as ::ext_php_rs::convert::IntoZval>::NULLABLE, + ) + }; + } + quote! { .returns( <#output as ::ext_php_rs::convert::IntoZval>::TYPE, @@ -244,7 +350,7 @@ impl<'a> Function<'a> { fn build_result( &self, - call_type: CallType, + call_type: &CallType, required: &[TypedArg<'_>], not_required: &[TypedArg<'_>], ) -> TokenStream { @@ -274,6 +380,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 +415,33 @@ 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) => { + // Explicit scope helps with mutable borrow lifetime when + // the method returns `&mut Self` + quote! { + { + let _ = #class::#ident(this, #({#arg_accessors}),*); + } + } + } + (MethodReceiver::ZendClassObject, false) => { quote! { #class::#ident(this, #({#arg_accessors}),*) } } }; + quote! { #this let parse_result = parse @@ -336,7 +463,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)] diff --git a/crates/macros/src/impl_.rs b/crates/macros/src/impl_.rs index 1d9687149..ce493369a 100644 --- a/crates/macros/src/impl_.rs +++ b/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, diff --git a/tests/src/integration/class/class.php b/tests/src/integration/class/class.php index 5a0005a1d..13b98b962 100644 --- a/tests/src/integration/class/class.php +++ b/tests/src/integration/class/class.php @@ -15,6 +15,13 @@ $class->selfMultiRef("bar"); assert($class->getString() === 'Changed to bar'); +// Test method returning Self (new instance) +$newClass = $class->withString('new string'); +assert($newClass instanceof TestClass, 'withString should return TestClass instance'); +assert($newClass->getString() === 'new string', 'new instance should have new string'); +assert($class->getString() === 'Changed to bar', 'original instance should be unchanged'); +assert($newClass !== $class, 'should be different instances'); + assert($class->getNumber() === 2022); $class->setNumber(2023); assert($class->getNumber() === 2023); @@ -94,3 +101,24 @@ TestStaticProps::setCounter(100); assert(TestStaticProps::$staticCounter === 100, 'PHP should see Rust-set value'); + +// 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'); diff --git a/tests/src/integration/class/mod.rs b/tests/src/integration/class/mod.rs index 293b5f6f1..6a547ca46 100644 --- a/tests/src/integration/class/mod.rs +++ b/tests/src/integration/class/mod.rs @@ -58,6 +58,15 @@ impl TestClass { self_.string = format!("Changed to {val}"); self_ } + + /// Returns a new instance with a different string (tests returning Self) + pub fn with_string(&self, string: String) -> Self { + Self { + string, + number: self.number, + boolean_prop: self.boolean_prop, + } + } } #[php_function] @@ -223,6 +232,51 @@ impl TestStaticProps { } } +/// 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::() @@ -232,6 +286,7 @@ pub fn build_module(builder: ModuleBuilder) -> ModuleBuilder { .class::() .class::() .class::() + .class::() .function(wrap_function!(test_class)) .function(wrap_function!(throw_exception)) }