Better interactions with objects (#41)

davidcole1340 · web-flow · commit b23b678a3d6f · 2021-05-21T17:11:15.000+12:00
* Object functions now return references to objects

Added ability to get object class name as well as hashtable of
properties

* Return mutable reference to objects

* Optimized `unwrap_or_else` calls
diff --git a/example/skel/src/lib.rs b/example/skel/src/lib.rs
@@ -62,12 +62,8 @@ impl Test {
         let x = ZendClassObject::<Test>::get(execute_data).unwrap();
         //let param = execute_data.get_property("value");
         let obj = execute_data.get_self().unwrap();
-        dbg!(obj.get_property("value"));
-        obj.set_property("value", "Hello");
-        // dbg!(param);
-        //execute_data.set_property("new_value", "New value...");
-        //execute_data.set_property("value", "Hello, world!");
-        // dbg!(execute_data.set_property("value", "Hello, world!"));
+        obj.set_property("hello", "world");
+        dbg!(obj);
     }
 
     pub extern "C" fn call(execute_data: &mut ExecutionData, _retval: &mut Zval) {
@@ -86,6 +82,15 @@ impl Test {
 
         println!("Ready for call!");
     }
+
+    pub extern "C" fn debug(execute_data: &mut ExecutionData, _retval: &mut Zval) {
+        let mut val = Arg::new("val", DataType::Object);
+
+        parse_args!(execute_data, val);
+        let obj = val.zval().unwrap().object().unwrap();
+        obj.set_property("hello", "not irigianl");
+        dbg!(val.zval().map(|zv| zv.object()));
+    }
 }
 
 impl Default for Test {
@@ -111,13 +116,20 @@ pub extern "C" fn module_init(_type: i32, module_number: i32) -> i32 {
             FunctionBuilder::new("get", Test::get).build(),
             MethodFlags::Public,
         )
+        .method(
+            FunctionBuilder::new("debug", Test::debug)
+                .arg(Arg::new("val", DataType::Object))
+                .build(),
+            MethodFlags::Public,
+        )
         .method(
             FunctionBuilder::new("call", Test::call)
                 .arg(Arg::new("fn", DataType::Callable))
                 .build(),
             MethodFlags::Public,
         )
-        .property("value", "world", PropertyFlags::Public)
+        .property("asdf", "world", PropertyFlags::Public)
+        .property("jhki", 12345, PropertyFlags::Public)
         .constant("TEST", "Hello world")
         .object_override::<Test>()
         .build();
diff --git a/example/skel/test.php b/example/skel/test.php
@@ -8,6 +8,15 @@
 $x = new TestClass();
 var_dump($x);
 $x->get();
+$x->asdf = 10;
+$x->hello = 'asdf';
 var_dump($x);
-$x->value = null;
 $x->get();
+
+$y = new \stdClass;
+$y->hello = 'world';
+$y->world = 'hello';
+
+$x->debug($y);
+var_dump($y);
+
diff --git a/src/php/args.rs b/src/php/args.rs
@@ -200,27 +200,7 @@ impl<'a, 'b> ArgParser<'a, 'b> {
         }
 
         for (i, arg) in self.args.iter_mut().enumerate() {
-            let zval = unsafe { execute_data.zend_call_arg(i) };
-
-            if let Some(zval) = zval {
-                // if !arg.allow_null && zval.is_null() {
-                //     unsafe {
-                //         zend_wrong_parameter_error(
-                //             ZPP_ERROR_WRONG_CLASS_OR_NULL as i32,
-                //             i as u32,
-                //             c_str(arg.name) as *mut i8,
-                //             _zend_expected_type::from(**arg),
-                //             &mut *zval,
-                //         );
-                //     }
-                //     return Err(format!(
-                //         "Argument at index {} was null but is non-nullable.",
-                //         i
-                //     ));
-                // }
-
-                arg.zval = Some(zval);
-            }
+            arg.zval = unsafe { execute_data.zend_call_arg(i) };
         }
 
         Ok(())
diff --git a/src/php/class.rs b/src/php/class.rs
@@ -29,8 +29,8 @@ pub struct ClassBuilder<'a> {
     extends: *mut ClassEntry,
     methods: Vec<FunctionEntry>,
     object_override: Option<unsafe extern "C" fn(class_type: *mut ClassEntry) -> *mut ZendObject>,
-    properties: Vec<(&'a str, Zval, PropertyFlags)>,
-    constants: Vec<(&'a str, Zval)>,
+    properties: Vec<(String, Zval, PropertyFlags)>,
+    constants: Vec<(String, Zval)>,
 }
 
 impl<'a> ClassBuilder<'a> {
@@ -91,11 +91,12 @@ impl<'a> ClassBuilder<'a> {
     /// * `name` - The name of the property to add to the class.
     /// * `default` - The default value of the property.
     /// * `flags` - Flags relating to the property. See [`PropertyFlags`].
-    #[allow(unused_mut, unused_variables)]
-    pub fn property<T>(mut self, name: &'a str, default: T, flags: PropertyFlags) -> Self
-    where
-        T: Into<Zval>,
-    {
+    pub fn property(
+        mut self,
+        name: impl AsRef<str>,
+        default: impl Into<Zval>,
+        flags: PropertyFlags,
+    ) -> Self {
         let mut default = default.into();
 
         if default.is_string() {
@@ -104,7 +105,8 @@ impl<'a> ClassBuilder<'a> {
             default.set_persistent_string(val);
         }
 
-        self.properties.push((name, default, flags));
+        self.properties
+            .push((name.as_ref().to_string(), default, flags));
         self
     }
 
@@ -115,10 +117,7 @@ impl<'a> ClassBuilder<'a> {
     ///
     /// * `name` - The name of the constant to add to the class.
     /// * `value` - The value of the constant.
-    pub fn constant<T>(mut self, name: &'a str, value: T) -> Self
-    where
-        T: Into<Zval>,
-    {
+    pub fn constant(mut self, name: impl AsRef<str>, value: impl Into<Zval>) -> Self {
         let mut value = value.into();
 
         if value.is_string() {
@@ -127,7 +126,7 @@ impl<'a> ClassBuilder<'a> {
             value.set_persistent_string(val);
         }
 
-        self.constants.push((name, value));
+        self.constants.push((name.as_ref().to_string(), value));
         self
     }
 
@@ -175,7 +174,7 @@ impl<'a> ClassBuilder<'a> {
             unsafe {
                 zend_declare_property(
                     class,
-                    c_str(name),
+                    c_str(&name),
                     name.len() as _,
                     &mut default,
                     flags.bits() as _,
@@ -185,7 +184,7 @@ impl<'a> ClassBuilder<'a> {
 
         for (name, value) in self.constants {
             let value = Box::into_raw(Box::new(value));
-            unsafe { zend_declare_class_constant(class, c_str(name), name.len() as u64, value) };
+            unsafe { zend_declare_class_constant(class, c_str(&name), name.len() as u64, value) };
         }
 
         if let Some(object_override) = self.object_override {
diff --git a/src/php/types/array.rs b/src/php/types/array.rs
@@ -15,6 +15,7 @@ use crate::{
         zend_hash_next_index_insert, zend_hash_str_del, zend_hash_str_find, zend_hash_str_update,
         HT_MIN_SIZE,
     },
+    errors::{Error, Result},
     functions::c_str,
 };
 
@@ -50,8 +51,19 @@ impl ZendHashTable {
     /// # Parameters
     ///
     /// * `ptr` - The pointer of the actual hash table.
-    pub(crate) fn from_ptr(ptr: *mut HashTable) -> Self {
-        Self { ptr, free: false }
+    /// * `free` - Whether the pointer should be freed when the resulting [`ZendHashTable`]
+    /// goes out of scope.
+    ///
+    /// # Safety
+    ///
+    /// As a raw pointer is given this function is unsafe, you must ensure that the pointer is valid when calling
+    /// the function. A simple null check is done but this is not sufficient in most cases.
+    pub unsafe fn from_ptr(ptr: *mut HashTable, free: bool) -> Result<Self> {
+        if ptr.is_null() {
+            return Err(Error::InvalidPointer);
+        }
+
+        Ok(Self { ptr, free })
     }
 
     /// Returns the current number of elements in the array.
@@ -290,8 +302,9 @@ impl<'a> Iterator for Iter<'a> {
         let result = if let Some(val) = unsafe { self.pos.as_ref() } {
             // SAFETY: We can ensure safety further by checking if it is null before
             // converting it to a reference (val.key.as_ref() returns None if ptr == null)
-            let str_key =
-                unsafe { ZendString::from_ptr(val.key, false) }.and_then(|s| s.try_into().ok());
+            let str_key = unsafe { ZendString::from_ptr(val.key, false) }
+                .and_then(|s| s.try_into())
+                .ok();
 
             Some((val.h, str_key, &val.val))
         } else {
diff --git a/src/php/types/object.rs b/src/php/types/object.rs
@@ -2,6 +2,7 @@
 //! allowing users to store Rust data inside a PHP object.
 
 use std::{
+    convert::TryInto,
     fmt::Debug,
     mem,
     ops::{Deref, DerefMut},
@@ -17,7 +18,7 @@ use crate::{
     php::{class::ClassEntry, execution_data::ExecutionData, types::string::ZendString},
 };
 
-use super::zval::Zval;
+use super::{array::ZendHashTable, zval::Zval};
 
 pub type ZendObject = zend_object;
 pub type ZendObjectHandlers = zend_object_handlers;
@@ -35,6 +36,18 @@ pub enum PropertyQuery {
 }
 
 impl ZendObject {
+    /// Attempts to retrieve the class name of the object.
+    pub fn get_class_name(&self) -> Result<String> {
+        let name = unsafe {
+            ZendString::from_ptr(
+                self.handlers()?.get_class_name.ok_or(Error::InvalidScope)?(self),
+                false,
+            )
+        }?;
+
+        name.try_into()
+    }
+
     /// Attempts to read a property from the Object. Returns a result returning an
     /// immutable reference to the [`Zval`] if the property exists and can be read,
     /// and an [`Error`] otherwise.
@@ -104,16 +117,24 @@ impl ZendObject {
     pub fn has_property(&self, name: impl AsRef<str>, query: PropertyQuery) -> Result<bool> {
         let name = ZendString::new(name.as_ref(), false);
 
-        let x = unsafe {
+        Ok(unsafe {
             self.handlers()?.has_property.ok_or(Error::InvalidScope)?(
                 self.mut_ptr(),
                 name.borrow_ptr(),
                 query as _,
                 std::ptr::null_mut(),
             )
-        };
+        } > 0)
+    }
 
-        Ok(x > 0)
+    /// Attempts to retrieve the properties of the object. Returned inside a Zend Hashtable.
+    pub fn get_properties(&self) -> Result<ZendHashTable> {
+        unsafe {
+            ZendHashTable::from_ptr(
+                self.handlers()?.get_properties.ok_or(Error::InvalidScope)?(self.mut_ptr()),
+                false,
+            )
+        }
     }
 
     /// Attempts to retrieve a reference to the object handlers.
@@ -131,6 +152,24 @@ impl ZendObject {
     }
 }
 
+impl Debug for ZendObject {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let mut dbg = f.debug_struct(
+            self.get_class_name()
+                .unwrap_or_else(|_| "ZendObject".to_string())
+                .as_str(),
+        );
+
+        if let Ok(props) = self.get_properties() {
+            for (id, key, val) in props.into_iter() {
+                dbg.field(key.unwrap_or_else(|| id.to_string()).as_str(), val);
+            }
+        }
+
+        dbg.finish()
+    }
+}
+
 /// Implemented by the [`ZendObjectHandler`](ext_php_rs_derive::ZendObjectHandler) macro on a type T
 /// which is used as the T type for [`ZendClassObject`].
 /// Implements a function `create_object` which is passed to a PHP class entry to instantiate the
@@ -193,15 +232,15 @@ impl<T: Default> ZendClassObject<T> {
         &mut obj.std
     }
 
-    /// Attempts to retrieve the zend class object container from the
+    /// Attempts to retrieve the Zend class object container from the
     /// zend object contained in the execution data of a function.
     ///
     /// # Parameters
     ///
     /// * `ex` - The execution data of the function.
     pub fn get(ex: &ExecutionData) -> Option<&'static mut Self> {
         // cast to u8 to work in terms of bytes
-        let ptr = ex.This.object()? as *mut u8;
+        let ptr = (ex.This.object()? as *const ZendObject) as *mut u8;
         let offset = std::mem::size_of::<T>();
         unsafe {
             let ptr = ptr.offset(0 - offset as isize);
diff --git a/src/php/types/string.rs b/src/php/types/string.rs
@@ -65,12 +65,12 @@ impl ZendString {
     ///
     /// As a raw pointer is given this function is unsafe, you must ensure the pointer is valid when calling
     /// the function. A simple null check is done but this is not sufficient in most places.
-    pub unsafe fn from_ptr(ptr: *mut zend_string, free: bool) -> Option<Self> {
+    pub unsafe fn from_ptr(ptr: *mut zend_string, free: bool) -> Result<Self> {
         if ptr.is_null() {
-            return None;
+            return Err(Error::InvalidPointer);
         }
 
-        Some(Self { ptr, free })
+        Ok(Self { ptr, free })
     }
 
     /// Releases the Zend string, returning the raw pointer to the `zend_string` object
diff --git a/src/php/types/zval.rs b/src/php/types/zval.rs
@@ -7,8 +7,7 @@ use std::{convert::TryFrom, fmt::Debug, ptr};
 use crate::{
     bindings::{
         _call_user_function_impl, _zval_struct__bindgen_ty_1, _zval_struct__bindgen_ty_2,
-        ext_php_rs_zend_string_release, zend_is_callable, zend_object, zend_resource, zend_value,
-        zval,
+        ext_php_rs_zend_string_release, zend_is_callable, zend_resource, zend_value, zval,
     },
     errors::{Error, Result},
     php::pack::Pack,
@@ -20,7 +19,7 @@ use crate::php::{
     types::{long::ZendLong, string::ZendString},
 };
 
-use super::array::ZendHashTable;
+use super::{array::ZendHashTable, object::ZendObject};
 
 /// Zend value. Represents most data types that are in the Zend engine.
 pub type Zval = zval;
@@ -122,18 +121,16 @@ impl<'a> Zval {
     /// Returns the value of the zval if it is an array.
     pub fn array(&self) -> Option<ZendHashTable> {
         if self.is_array() {
-            Some(ZendHashTable::from_ptr(unsafe { self.value.arr }))
+            unsafe { ZendHashTable::from_ptr(self.value.arr, false) }.ok()
         } else {
             None
         }
     }
 
     /// Returns the value of the zval if it is an object.
-    pub fn object(&self) -> Option<*mut zend_object> {
-        // TODO: Can we improve this function? I haven't done much research into
-        // objects so I don't know if this is the optimal way to return this.
+    pub fn object(&self) -> Option<&mut ZendObject> {
         if self.is_object() {
-            Some(unsafe { self.value.obj })
+            unsafe { self.value.obj.as_mut() }
         } else {
             None
         }
@@ -379,9 +376,9 @@ impl<'a> Zval {
     ///
     /// * `val` - The value to set the zval as.
     /// * `copy` - Whether to copy the object or pass as a reference.
-    pub fn set_object(&mut self, val: *mut zend_object, _copy: bool) {
+    pub fn set_object(&mut self, val: &ZendObject, _copy: bool) {
         self.u1.type_info = ZvalTypeFlags::ObjectEx.bits();
-        self.value.obj = val;
+        self.value.obj = (val as *const ZendObject) as *mut ZendObject;
     }
 
     /// Sets the value of the zval as an array.

Original file line number	Diff line number	Diff line change
`@@ -65,12 +65,12 @@ impl ZendString {`
`65`	`65`	`///`
`66`	`66`	`/// As a raw pointer is given this function is unsafe, you must ensure the pointer is valid when calling`
`67`	`67`	`/// the function. A simple null check is done but this is not sufficient in most places.`
`68`		`- pub unsafe fn from_ptr(ptr: *mut zend_string, free: bool) -> Option<Self> {`
	`68`	`+ pub unsafe fn from_ptr(ptr: *mut zend_string, free: bool) -> Result<Self> {`
`69`	`69`	`if ptr.is_null() {`
`70`		`- return None;`
	`70`	`+ return Err(Error::InvalidPointer);`
`71`	`71`	`}`
`72`	`72`
`73`		`- Some(Self { ptr, free })`
	`73`	`+ Ok(Self { ptr, free })`
`74`	`74`	`}`
`75`	`75`
`76`	`76`	/// Releases the Zend string, returning the raw pointer to the `zend_string` object