Remove lifetime from PhpException (#80)

davidcole1340 · web-flow · commit daef57ba91c0 · 2021-09-28T02:23:13.000+13:00
* Remove lifetime from `PhpException`

All class entries are effectively static (module start to module end) so
it's just a hassle carrying the lifetime everywhere.

* Removed forgotten lifetimes

* Implement `IntoZval` for `Result&lt;T, E&gt;`

Fixes function return error with `PhpResult` as the return type

* Updated changelog
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,10 @@
 # Changelog
 
+- `PhpException` no longer requires a lifetime [#80].
+- Added `PhpException` and `PhpResult` to prelude [#80].
+
+[#80]: https://github.com/davidcole1340/ext-php-rs/pull/80
+
 ## Version 0.5.0
 
 ### Breaking changes
diff --git a/ext-php-rs-derive/src/function.rs b/ext-php-rs-derive/src/function.rs
@@ -60,7 +60,6 @@ pub fn parser(args: AttributeArgs, input: ItemFn) -> Result<(TokenStream, Functi
     let arg_parser = build_arg_parser(args.iter(), &optional)?;
     let arg_accessors = build_arg_accessors(&args);
 
-    let return_handler = build_return_handler(output);
     let return_type = get_return_type(output)?;
 
     let func = quote! {
@@ -75,7 +74,10 @@ pub fn parser(args: AttributeArgs, input: ItemFn) -> Result<(TokenStream, Functi
 
             let result = #ident(#(#arg_accessors, )*);
 
-            #return_handler
+            if let Err(e) = result.set_zval(retval, false) {
+                let e: ::ext_php_rs::php::exceptions::PhpException = e.into();
+                e.throw().expect("Failed to throw exception");
+            }
         }
     };
 
@@ -202,61 +204,6 @@ fn build_arg_accessors(args: &[Arg]) -> Vec<TokenStream> {
     args.iter().map(|arg| arg.get_accessor()).collect()
 }
 
-pub fn build_return_handler(output_type: &ReturnType) -> TokenStream {
-    let handler = match output_type {
-        ReturnType::Default => Some(quote! { retval.set_null(); }),
-        ReturnType::Type(_, ref ty) => match **ty {
-            Type::Path(ref path) => match path.path.segments.last() {
-                Some(path_seg) => match path_seg.ident.to_string().as_ref() {
-                    "Result" => Some(quote! {
-                        match result {
-                            Ok(result) => match result.set_zval(retval, false) {
-                                Ok(_) => {}
-                                Err(e) => {
-                                    let e: ::ext_php_rs::php::exceptions::PhpException = e.into();
-                                    e.throw().expect("Failed to throw exception: Failed to set return value.");
-                                },
-                            },
-                            Err(e) => {
-                                let e: ::ext_php_rs::php::exceptions::PhpException = e.into();
-                                e.throw().expect("Failed to throw exception: Error type returned from internal function.");
-                            }
-                        };
-                    }),
-                    "Option" => Some(quote! {
-                        match result {
-                            Some(result) => match result.set_zval(retval, false) {
-                                Ok(_) => {}
-                                Err(e) => {
-                                    let e: ::ext_php_rs::php::exceptions::PhpException = e.into();
-                                    e.throw().expect("Failed to throw exception: Failed to set return value.");
-                                },
-                            },
-                            None => retval.set_null(),
-                        };
-                    }),
-                    _ => None,
-                },
-                _ => None,
-            },
-            _ => None,
-        },
-    };
-
-    match handler {
-        Some(handler) => handler,
-        None => quote! {
-            match result.set_zval(retval, false) {
-                Ok(_) => {},
-                Err(e) => {
-                    let e: ::ext_php_rs::php::exceptions::PhpException = e.into();
-                    e.throw().expect("Failed to throw exception: Failed to set return value.");
-                }
-            }
-        },
-    }
-}
-
 pub fn get_return_type(output_type: &ReturnType) -> Result<Option<(String, bool)>> {
     Ok(match output_type {
         ReturnType::Default => None,
diff --git a/ext-php-rs-derive/src/method.rs b/ext-php-rs-derive/src/method.rs
@@ -114,7 +114,6 @@ pub fn parser(input: &mut ImplItemMethod, rename_rule: RenameRule) -> Result<Par
     let (arg_definitions, is_static) = build_arg_definitions(&args);
     let arg_parser = build_arg_parser(args.iter(), &optional)?;
     let arg_accessors = build_arg_accessors(&args);
-    let return_handler = function::build_return_handler(output);
     let this = if is_static {
         quote! { Self:: }
     } else {
@@ -133,7 +132,10 @@ pub fn parser(input: &mut ImplItemMethod, rename_rule: RenameRule) -> Result<Par
 
             let result = #this #ident(#(#arg_accessors, )*);
 
-            #return_handler
+            if let Err(e) = result.set_zval(retval, false) {
+                let e: ::ext_php_rs::php::exceptions::PhpException = e.into();
+                e.throw().expect("Failed to throw exception");
+            }
         }
     };
 
diff --git a/guide/src/macros/structs.md b/guide/src/macros/structs.md
@@ -75,7 +75,7 @@ pub struct RedisException;
 
 // Throw our newly created exception
 #[php_function]
-pub fn throw_exception() -> Result<i32, PhpException<'static>> {
+pub fn throw_exception() -> PhpResult<i32> {
     Err(PhpException::from_class::<RedisException>("Not good!".into()))
 }
 # #[php_module]
diff --git a/src/errors.rs b/src/errors.rs
@@ -95,7 +95,7 @@ impl From<NulError> for Error {
     }
 }
 
-impl<'a> From<Error> for PhpException<'a> {
+impl From<Error> for PhpException {
     fn from(err: Error) -> Self {
         Self::default(err.to_string())
     }
diff --git a/src/lib.rs b/src/lib.rs
@@ -398,7 +398,7 @@ pub use ext_php_rs_derive::php_module;
 /// pub struct Example;
 ///
 /// #[php_function]
-/// pub fn throw_exception() -> Result<i32, PhpException<'static>> {
+/// pub fn throw_exception() -> Result<i32, PhpException> {
 ///     Err(PhpException::from_class::<Example>("Bad things happen".into()))
 /// }
 ///
@@ -439,6 +439,7 @@ pub use ext_php_rs_derive::php_startup;
 
 /// A module typically glob-imported containing the typically required macros and imports.
 pub mod prelude {
+    pub use crate::php::exceptions::{PhpException, PhpResult};
     pub use crate::php::module::ModuleBuilder;
     pub use crate::php::types::callable::Callable;
     #[cfg(any(docs, feature = "closure"))]
diff --git a/src/php/exceptions.rs b/src/php/exceptions.rs
@@ -15,7 +15,7 @@ use crate::{
 };
 
 /// Result type with the error variant as a [`PhpException`].
-pub type PhpResult<T = ()> = std::result::Result<T, PhpException<'static>>;
+pub type PhpResult<T = ()> = std::result::Result<T, PhpException>;
 
 /// Represents a PHP exception which can be thrown using the `throw()` function. Primarily used to
 /// return from a [`Result<T, PhpException>`] which can immediately be thrown by the `ext-php-rs`
@@ -25,21 +25,21 @@ pub type PhpResult<T = ()> = std::result::Result<T, PhpException<'static>>;
 /// can also be returned from these functions. You can also implement [`From<T>`] for your custom
 /// error type.
 #[derive(Debug)]
-pub struct PhpException<'a> {
+pub struct PhpException {
     message: String,
     code: i32,
-    ex: &'a ClassEntry,
+    ex: &'static ClassEntry,
 }
 
-impl<'a> PhpException<'a> {
+impl PhpException {
     /// Creates a new exception instance.
     ///
     /// # Parameters
     ///
     /// * `message` - Message to contain in the exception.
     /// * `code` - Integer code to go inside the exception.
     /// * `ex` - Exception type to throw.
-    pub fn new(message: String, code: i32, ex: &'a ClassEntry) -> Self {
+    pub fn new(message: String, code: i32, ex: &'static ClassEntry) -> Self {
         Self { message, code, ex }
     }
 
@@ -69,13 +69,13 @@ impl<'a> PhpException<'a> {
     }
 }
 
-impl<'a> From<String> for PhpException<'a> {
+impl From<String> for PhpException {
     fn from(str: String) -> Self {
         Self::default(str)
     }
 }
 
-impl<'a> From<&str> for PhpException<'a> {
+impl From<&str> for PhpException {
     fn from(str: &str) -> Self {
         Self::default(str.into())
     }
diff --git a/src/php/types/object.rs b/src/php/types/object.rs
@@ -26,7 +26,7 @@ use crate::{
     php::{
         class::ClassEntry,
         enums::DataType,
-        exceptions::PhpException,
+        exceptions::PhpResult,
         flags::ZvalTypeFlags,
         types::{array::OwnedHashTable, string::ZendString},
     },
@@ -649,7 +649,7 @@ impl ZendObjectHandlers {
             type_: c_int,
             cache_slot: *mut *mut c_void,
             rv: *mut Zval,
-        ) -> std::result::Result<*mut Zval, PhpException<'static>> {
+        ) -> PhpResult<*mut Zval> {
             let obj = object
                 .as_ref()
                 .and_then(|obj| ZendClassObject::<T>::from_zend_obj_ptr(obj))
@@ -696,7 +696,7 @@ impl ZendObjectHandlers {
             member: *mut zend_string,
             value: *mut Zval,
             cache_slot: *mut *mut c_void,
-        ) -> std::result::Result<*mut Zval, PhpException<'static>> {
+        ) -> PhpResult<*mut Zval> {
             let obj = object
                 .as_ref()
                 .and_then(|obj| ZendClassObject::<T>::from_zend_obj_ptr(obj))
@@ -734,7 +734,7 @@ impl ZendObjectHandlers {
         unsafe fn internal<T: RegisteredClass>(
             object: *mut zend_object,
             props: &mut HashTable,
-        ) -> std::result::Result<(), PhpException<'static>> {
+        ) -> PhpResult {
             let obj = object
                 .as_ref()
                 .and_then(|obj| ZendClassObject::<T>::from_zend_obj_ptr(obj))
@@ -779,7 +779,7 @@ impl ZendObjectHandlers {
             member: *mut zend_string,
             has_set_exists: c_int,
             cache_slot: *mut *mut c_void,
-        ) -> std::result::Result<c_int, PhpException<'static>> {
+        ) -> PhpResult<c_int> {
             let obj = object
                 .as_ref()
                 .and_then(|obj| ZendClassObject::<T>::from_zend_obj_ptr(obj))
diff --git a/src/php/types/zval.rs b/src/php/types/zval.rs
@@ -14,7 +14,7 @@ use crate::{
         zend_value, zval, zval_ptr_dtor,
     },
     errors::{Error, Result},
-    php::pack::Pack,
+    php::{exceptions::PhpException, pack::Pack},
 };
 
 use crate::php::{
@@ -692,6 +692,24 @@ where
     }
 }
 
+impl<T, E> IntoZval for std::result::Result<T, E>
+where
+    T: IntoZval,
+    E: Into<PhpException>,
+{
+    const TYPE: DataType = T::TYPE;
+
+    fn set_zval(self, zv: &mut Zval, persistent: bool) -> Result<()> {
+        match self {
+            Ok(val) => val.set_zval(zv, persistent),
+            Err(e) => {
+                let ex: PhpException = e.into();
+                ex.throw()
+            }
+        }
+    }
+}
+
 /// Allows zvals to be converted into Rust types in a fallible way. Reciprocal of the [`IntoZval`]
 /// trait.
 pub trait FromZval<'a>: Sized {

Original file line number	Diff line number	Diff line change
`@@ -75,7 +75,7 @@ pub struct RedisException;`
`75`	`75`
`76`	`76`	`// Throw our newly created exception`
`77`	`77`	`#[php_function]`
`78`		`-pub fn throw_exception() -> Result<i32, PhpException<'static>> {`
	`78`	`+pub fn throw_exception() -> PhpResult<i32> {`
`79`	`79`	`Err(PhpException::from_class::<RedisException>("Not good!".into()))`
`80`	`80`	`}`
`81`	`81`	`# #[php_module]`
Original file line number	Diff line number	Diff line change
`@@ -95,7 +95,7 @@ impl From<NulError> for Error {`
`95`	`95`	`}`
`96`	`96`	`}`
`97`	`97`
`98`		`-impl<'a> From<Error> for PhpException<'a> {`
	`98`	`+impl From<Error> for PhpException {`
`99`	`99`	`fn from(err: Error) -> Self {`
`100`	`100`	`Self::default(err.to_string())`
`101`	`101`	`}`