Replace unit errors with error enum (#29)

* WIP: Replacing unit type errors with actual error

* Tidied up errors

Author: davidcole1340

src/errors.rs

@@ -0,0 +1,49 @@
+use std::{error::Error as ErrorTrait, fmt::Display};
+
+use crate::php::enums::DataType;
+
+/// The main result type which is passed by the library.
+pub type Result<T> = std::result::Result<T, Error>;
+
+/// The main error type which is passed by the library inside the custom
+/// [`Result`] type.
+#[derive(Debug, Clone)]
+#[non_exhaustive]
+pub enum Error {
+    /// An incorrect number of arguments was given to a PHP function.
+    ///
+    /// The type carries two integers - the first representing the minimum
+    /// number of arguments expected, and the second representing the number of
+    /// arguments that were received.
+    IncorrectArguments(u32, u32),
+
+    /// There was an error converting a Zval into a primitive type.
+    ///
+    /// The type carries the data type of the Zval.
+    ZvalConversion(DataType),
+
+    /// The type of the Zval is unknown.
+    ///
+    /// The type carries the integer representation of the type of Zval.
+    UnknownDatatype(u8),
+}
+
+impl Display for Error {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Error::IncorrectArguments(n, expected) => write!(
+                f,
+                "Expected at least {} arguments, got {} arguments.",
+                expected, n
+            ),
+            Error::ZvalConversion(ty) => write!(
+                f,
+                "Could not convert Zval from type {} into primitive type.",
+                ty
+            ),
+            Error::UnknownDatatype(dt) => write!(f, "Unknown datatype {}.", dt),
+        }
+    }
+}
+
+impl ErrorTrait for Error {}

src/lib.rs

@@ -5,6 +5,7 @@
 #[macro_use]
 pub mod macros;
 pub mod bindings;
+pub mod errors;
 pub mod functions;
 pub mod php;
 

src/php/args.rs

@@ -4,12 +4,15 @@ use std::convert::{TryFrom, TryInto};
 
 use super::{enums::DataType, execution_data::ExecutionData, types::zval::Zval};
 
-use crate::bindings::{
-    _zend_expected_type, _zend_expected_type_Z_EXPECTED_ARRAY, _zend_expected_type_Z_EXPECTED_BOOL,
-    _zend_expected_type_Z_EXPECTED_DOUBLE, _zend_expected_type_Z_EXPECTED_LONG,
-    _zend_expected_type_Z_EXPECTED_OBJECT, _zend_expected_type_Z_EXPECTED_RESOURCE,
-    _zend_expected_type_Z_EXPECTED_STRING, zend_internal_arg_info,
-    zend_wrong_parameters_count_error,
+use crate::{
+    bindings::{
+        _zend_expected_type, _zend_expected_type_Z_EXPECTED_ARRAY,
+        _zend_expected_type_Z_EXPECTED_BOOL, _zend_expected_type_Z_EXPECTED_DOUBLE,
+        _zend_expected_type_Z_EXPECTED_LONG, _zend_expected_type_Z_EXPECTED_OBJECT,
+        _zend_expected_type_Z_EXPECTED_RESOURCE, _zend_expected_type_Z_EXPECTED_STRING,
+        zend_internal_arg_info, zend_wrong_parameters_count_error,
+    },
+    errors::{Error, Result},
 };
 
 /// Represents an argument to a function.
@@ -180,7 +183,7 @@ impl<'a, 'b> ArgParser<'a, 'b> {
     /// * `Err(String)` - There were too many or too little arguments
     /// passed to the function. The user has already been notified so you
     /// can discard and return from the function if an `Err` is received.
-    pub fn parse(mut self) -> Result<(), String> {
+    pub fn parse(mut self) -> Result<()> {
         let execute_data = unsafe { self.execute_data.as_ref() }.unwrap();
         let num_args = unsafe { execute_data.This.u2.num_args };
         let max_num_args = self.args.len() as u32;
@@ -192,10 +195,7 @@ impl<'a, 'b> ArgParser<'a, 'b> {
         if num_args < min_num_args || num_args > max_num_args {
             unsafe { zend_wrong_parameters_count_error(min_num_args, max_num_args) };
 
-            return Err(format!(
-                "Expected at least {} arguments, got {} arguments.",
-                min_num_args, num_args,
-            ));
+            return Err(Error::IncorrectArguments(num_args, min_num_args));
         }
 
         for (i, arg) in self.args.iter_mut().enumerate() {

src/php/enums.rs

@@ -1,11 +1,14 @@
 //! Wrapper for enums introduced in C.
 
-use crate::bindings::{
-    IS_ARRAY, IS_CALLABLE, IS_CONSTANT_AST, IS_DOUBLE, IS_FALSE, IS_LONG, IS_NULL, IS_OBJECT,
-    IS_REFERENCE, IS_RESOURCE, IS_STRING, IS_TRUE, IS_UNDEF, IS_VOID,
-};
+use std::{convert::TryFrom, fmt::Display};
 
-use super::types::long::ZendLong;
+use crate::{
+    bindings::{
+        IS_ARRAY, IS_CALLABLE, IS_CONSTANT_AST, IS_DOUBLE, IS_FALSE, IS_LONG, IS_NULL, IS_OBJECT,
+        IS_REFERENCE, IS_RESOURCE, IS_STRING, IS_TRUE, IS_UNDEF, IS_VOID,
+    },
+    errors::{Error, Result},
+};
 
 /// Valid data types for PHP.
 #[derive(Clone, Copy, Debug)]
@@ -29,30 +32,48 @@ pub enum DataType {
     Void = IS_VOID,
 }
 
-impl From<ZendLong> for DataType {
-    fn from(_: ZendLong) -> Self {
-        Self::Long
-    }
-}
+impl TryFrom<u8> for DataType {
+    type Error = Error;
 
-impl From<bool> for DataType {
-    fn from(x: bool) -> Self {
-        if x {
-            Self::True
-        } else {
-            Self::False
-        }
-    }
-}
+    fn try_from(value: u8) -> Result<Self> {
+        match value as u32 {
+            IS_UNDEF => Ok(DataType::Undef),
+            IS_NULL => Ok(DataType::Null),
+            IS_FALSE => Ok(DataType::False),
+            IS_TRUE => Ok(DataType::True),
+            IS_LONG => Ok(DataType::Long),
+            IS_DOUBLE => Ok(DataType::Double),
+            IS_STRING => Ok(DataType::String),
+            IS_ARRAY => Ok(DataType::Array),
+            IS_OBJECT => Ok(DataType::Object),
+            IS_RESOURCE => Ok(DataType::Resource),
+            IS_REFERENCE => Ok(DataType::Reference),
+            IS_CALLABLE => Ok(DataType::Callable),
+            IS_CONSTANT_AST => Ok(DataType::ConstantExpression),
+            IS_VOID => Ok(DataType::Void),
 
-impl From<f64> for DataType {
-    fn from(_: f64) -> Self {
-        Self::Double
+            _ => Err(Error::UnknownDatatype(value)),
+        }
     }
 }
 
-impl From<String> for DataType {
-    fn from(_: String) -> Self {
-        Self::String
+impl Display for DataType {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            DataType::Undef => write!(f, "Undefined"),
+            DataType::Null => write!(f, "Null"),
+            DataType::False => write!(f, "False"),
+            DataType::True => write!(f, "True"),
+            DataType::Long => write!(f, "Long"),
+            DataType::Double => write!(f, "Double"),
+            DataType::String => write!(f, "String"),
+            DataType::Array => write!(f, "Array"),
+            DataType::Object => write!(f, "Object"),
+            DataType::Resource => write!(f, "Resource"),
+            DataType::Reference => write!(f, "Reference"),
+            DataType::Callable => write!(f, "Callable"),
+            DataType::ConstantExpression => write!(f, "Constant Expression"),
+            DataType::Void => write!(f, "Void"),
+        }
     }
 }

src/php/types/array.rs

@@ -135,7 +135,7 @@ impl ZendHashTable {
     /// # Returns
     ///
     /// * `Ok(())` - Key was successfully removed.
-    /// * `Err(())` - No key was removed, did not exist.
+    /// * `None` - No key was removed, did not exist.
     pub fn remove_index(&self, key: u64) -> Option<()> {
         let result = unsafe { zend_hash_index_del(self.ptr, key) };
 
@@ -191,7 +191,7 @@ impl ZendHashTable {
     ///
     /// # Returns
     ///
-    /// * `Some(Zval)` - The existing value in the hash table that was overriden.
+    /// * `Some(&Zval)` - The existing value in the hash table that was overriden.
     /// * `None` - The element was inserted.
     pub fn insert_at_index<V>(&mut self, key: u64, val: V) -> Option<&Zval>
     where

src/php/types/zval.rs

@@ -4,10 +4,13 @@
 use core::slice;
 use std::{convert::TryFrom, 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,
-    IS_INTERNED_STRING_EX, IS_STRING_EX,
+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, IS_INTERNED_STRING_EX, IS_STRING_EX,
+    },
+    errors::{Error, Result},
 };
 
 use crate::php::{
@@ -178,6 +181,11 @@ impl<'a> Zval {
         }
     }
 
+    /// Returns the type of the Zval.
+    pub fn get_type(&self) -> Result<DataType> {
+        DataType::try_from(unsafe { self.u1.v.type_ })
+    }
+
     /// Returns true if the zval is a long, false otherwise.
     pub fn is_long(&self) -> bool {
         unsafe { self.u1.v.type_ == DataType::Long as u8 }
@@ -361,11 +369,14 @@ impl<'a> Zval {
 macro_rules! try_from_zval {
     ($type: ty, $fn: ident) => {
         impl TryFrom<&Zval> for $type {
-            type Error = ();
-            fn try_from(value: &Zval) -> Result<Self, Self::Error> {
+            type Error = Error;
+            fn try_from(value: &Zval) -> Result<Self> {
                 match value.$fn() {
-                    Some(v) => <$type>::try_from(v).map_err(|_| ()),
-                    _ => Err(()),
+                    Some(v) => match <$type>::try_from(v) {
+                        Ok(v) => Ok(v),
+                        Err(_) => Err(Error::ZvalConversion(value.get_type()?)),
+                    },
+                    _ => Err(Error::ZvalConversion(value.get_type()?)),
                 }
             }
         }