- Make ConvertError only appear in meta

- Reorder derives
- Remove `#[non_exhaustive]`
- Change error message for `WrongClass`
- Remove builder pattern from `ConvertError`

Author: lilizoey

godot-codegen/src/class_generator.rs

@@ -909,7 +909,7 @@ fn make_native_structure(
         }
 
         impl FromGodot for *mut #class_name {
-            fn try_from_godot(via: Self::Via) -> Result<Self, crate::builtin::ConvertError> {
+            fn try_from_godot(via: Self::Via) -> Result<Self, crate::builtin::meta::ConvertError> {
                 Ok(via as Self)
             }
         }
@@ -925,7 +925,7 @@ fn make_native_structure(
         }
 
         impl FromGodot for *const #class_name {
-            fn try_from_godot(via: Self::Via) -> Result<Self, crate::builtin::ConvertError> {
+            fn try_from_godot(via: Self::Via) -> Result<Self, crate::builtin::meta::ConvertError> {
                 Ok(via as Self)
             }
         }

godot-codegen/src/util.rs

@@ -337,9 +337,9 @@ pub fn make_enum_definition(enum_: &Enum) -> TokenStream {
         }
 
         impl crate::builtin::meta::FromGodot for #enum_name {
-            fn try_from_godot(via: Self::Via) -> std::result::Result<Self, crate::builtin::ConvertError> {
+            fn try_from_godot(via: Self::Via) -> std::result::Result<Self, crate::builtin::meta::ConvertError> {
                 <Self as crate::obj::EngineEnum>::try_from_ord(via)
-                    .ok_or_else(|| crate::builtin::ConvertError::from(crate::builtin::meta::FromGodotError::InvalidEnum).with_value(via))
+                    .ok_or_else(|| crate::builtin::meta::FromGodotError::InvalidEnum.into_error(via))
             }
         }
 

godot-core/src/builtin/array.rs

@@ -14,7 +14,8 @@ use std::marker::PhantomData;
 use sys::{ffi_methods, interface_fn, GodotFfi};
 
 use super::meta::{
-    FromGodot, FromGodotError, FromVariantError, GodotConvert, GodotFfiVariant, GodotType, ToGodot,
+    ConvertError, FromGodot, FromGodotError, FromVariantError, GodotConvert, GodotFfiVariant,
+    GodotType, ToGodot,
 };
 
 /// Godot's `Array` type.
@@ -346,11 +347,11 @@ impl<T: GodotType> Array<T> {
         if self_ty == target_ty {
             Ok(self)
         } else {
-            Err(ConvertError::from(FromGodotError::BadArrayType {
+            Err(FromGodotError::BadArrayType {
                 expected: target_ty,
                 got: self_ty,
-            })
-            .with_value(self))
+            }
+            .into_error(self))
         }
     }
 
@@ -801,11 +802,11 @@ impl<T: GodotType> GodotFfiVariant for Array<T> {
 
     fn ffi_from_variant(variant: &Variant) -> Result<Self, ConvertError> {
         if variant.get_type() != Self::variant_type() {
-            return Err(ConvertError::from(FromVariantError::BadType {
+            return Err(FromVariantError::BadType {
                 expected: Self::variant_type(),
                 got: variant.get_type(),
-            })
-            .with_value(variant.clone()));
+            }
+            .into_error(variant.clone()));
         }
 
         let array = unsafe {

godot-core/src/builtin/meta/godot_convert/convert_error.rs

@@ -22,37 +22,54 @@ pub struct ConvertError {
 }
 
 impl ConvertError {
-    /// Create a new error for a conversion.
-    fn new(kind: ErrorKind) -> Self {
+    /// Create a new custom error for a conversion.
+    pub fn new() -> Self {
         Self {
-            kind,
+            kind: ErrorKind::Custom,
             cause: None,
             value: None,
         }
     }
 
-    /// Create a new custom error for a conversion.
-    pub fn custom() -> Self {
-        Self::new(ErrorKind::Custom)
+    fn custom() -> Self {
+        Self {
+            kind: ErrorKind::Custom,
+            cause: None,
+            value: None,
+        }
     }
 
-    /// Add a rust-error as an underlying cause for the conversion error.
-    pub fn with_cause<C: Into<Cause>>(mut self, cause: C) -> Self {
-        self.cause = Some(cause.into());
-        self
+    /// Create a new custom error for a conversion with the value that failed to convert.
+    pub fn new_with_value<V: fmt::Debug + 'static>(value: V) -> Self {
+        let mut err = Self::custom();
+        err.value = Some(Box::new(value));
+        err
+    }
+
+    /// Create a new custom error with a rust-error as an underlying cause for the conversion error.
+    pub fn new_with_cause<C: Into<Cause>>(cause: C) -> Self {
+        let mut err = Self::custom();
+        err.cause = Some(cause.into());
+        err
+    }
+
+    /// Create a new custom error with a rust-error as an underlying cause for the conversion error, and the
+    /// value that failed to convert.
+    pub fn new_with_cause_value<C: Into<Cause>, V: fmt::Debug + 'static>(
+        cause: C,
+        value: V,
+    ) -> Self {
+        let mut err = Self::custom();
+        err.cause = Some(cause.into());
+        err.value = Some(Box::new(value));
+        err
     }
 
     /// Returns the rust-error that caused this error, if one exists.
     pub fn cause(&self) -> Option<&(dyn Error + Send + Sync)> {
         self.cause.as_deref()
     }
 
-    /// Add the value that failed to be converted.
-    pub fn with_value<V: fmt::Debug + 'static>(mut self, value: V) -> Self {
-        self.value = Some(Box::new(value));
-        self
-    }
-
     /// Returns the value that failed to convert, if one exists.
     pub fn value(&self) -> Option<&(dyn fmt::Debug)> {
         self.value.as_deref()
@@ -63,6 +80,12 @@ impl ConvertError {
     }
 }
 
+impl Default for ConvertError {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl fmt::Display for ConvertError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match (self.description(), self.cause.as_ref()) {
@@ -89,7 +112,6 @@ impl Error for ConvertError {
 }
 
 #[derive(Debug, PartialEq, Eq)]
-#[non_exhaustive]
 pub(crate) enum ErrorKind {
     FromGodot(FromGodotError),
     FromFfi(FromFfiError),
@@ -109,8 +131,7 @@ impl ErrorKind {
 }
 
 /// Conversion failed during a [`FromGodot`](crate::builtin::meta::FromGodot) call.
-#[derive(Debug, PartialEq, Eq)]
-#[non_exhaustive]
+#[derive(Eq, PartialEq, Debug)]
 pub(crate) enum FromGodotError {
     BadArrayType {
         expected: array_inner::TypeInfo,
@@ -121,6 +142,14 @@ pub(crate) enum FromGodotError {
 }
 
 impl FromGodotError {
+    pub fn into_error<V: fmt::Debug + 'static>(self, value: V) -> ConvertError {
+        ConvertError {
+            kind: ErrorKind::FromGodot(self),
+            cause: None,
+            value: Some(Box::new(value)),
+        }
+    }
+
     fn description(&self) -> String {
         match self {
             Self::BadArrayType { expected, got } => {
@@ -157,14 +186,8 @@ impl FromGodotError {
     }
 }
 
-impl From<FromGodotError> for ConvertError {
-    fn from(from_godot: FromGodotError) -> Self {
-        ConvertError::new(ErrorKind::FromGodot(from_godot))
-    }
-}
-
 /// Conversion failed during a [`GodotType::try_from_ffi()`](crate::builtin::meta::GodotType::try_from_ffi()) call.
-#[derive(Debug, PartialEq, Eq)]
+#[derive(Eq, PartialEq, Debug)]
 #[non_exhaustive]
 pub(crate) enum FromFfiError {
     NullRawGd,
@@ -178,6 +201,14 @@ pub(crate) enum FromFfiError {
 }
 
 impl FromFfiError {
+    pub fn into_error<V: fmt::Debug + 'static>(self, value: V) -> ConvertError {
+        ConvertError {
+            kind: ErrorKind::FromFfi(self),
+            cause: None,
+            value: Some(Box::new(value)),
+        }
+    }
+
     fn description(&self) -> String {
         let target = match self {
             Self::NullRawGd => return "`Gd` cannot be null".into(),
@@ -194,14 +225,7 @@ impl FromFfiError {
     }
 }
 
-impl From<FromFfiError> for ConvertError {
-    fn from(from_ffi: FromFfiError) -> Self {
-        ConvertError::new(ErrorKind::FromFfi(from_ffi))
-    }
-}
-
-#[derive(Debug, PartialEq, Eq)]
-#[non_exhaustive]
+#[derive(Eq, PartialEq, Debug)]
 pub(crate) enum FromVariantError {
     /// Variant type does not match expected type
     BadType {
@@ -212,27 +236,25 @@ pub(crate) enum FromVariantError {
     WrongClass {
         expected: ClassName,
     },
-
-    /// Variant value cannot be represented in target type
-    BadValue,
 }
 
 impl FromVariantError {
+    pub fn into_error<V: fmt::Debug + 'static>(self, value: V) -> ConvertError {
+        ConvertError {
+            kind: ErrorKind::FromVariant(self),
+            cause: None,
+            value: Some(Box::new(value)),
+        }
+    }
+
     fn description(&self) -> String {
         match self {
             Self::BadType { expected, got } => {
                 format!("expected Variant of type `{expected:?}` but got Variant of type `{got:?}`")
             }
             Self::WrongClass { expected } => {
-                format!("got variant of wrong class, expected class `{expected}`")
+                format!("expected class `{expected}`, got variant with wrong class")
             }
-            Self::BadValue => "Variant value cannot be represented in target type".into(),
         }
     }
 }
-
-impl From<FromVariantError> for ConvertError {
-    fn from(from_variant: FromVariantError) -> Self {
-        ConvertError::new(ErrorKind::FromVariant(from_variant))
-    }
-}

godot-core/src/builtin/meta/godot_convert/impls.rs

@@ -4,10 +4,10 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
-use crate::builtin::{
-    meta::{impl_godot_as_self, FromGodot, GodotConvert, GodotType, ToGodot},
-    ConvertError, Variant,
+use crate::builtin::meta::{
+    impl_godot_as_self, ConvertError, FromGodot, GodotConvert, GodotType, ToGodot,
 };
+use crate::builtin::Variant;
 use godot_ffi as sys;
 
 // The following ToGodot/FromGodot/Convert impls are auto-generated for each engine type, co-located with their definitions:
@@ -137,7 +137,7 @@ macro_rules! impl_godot_scalar {
             }
 
             fn try_from_ffi(ffi: Self::Ffi) -> Result<Self, ConvertError> {
-                Self::try_from(ffi).map_err(|_| ConvertError::from($err).with_value(ffi))
+                Self::try_from(ffi).map_err(|_| $err.into_error(ffi))
             }
 
             $(

godot-core/src/builtin/meta/godot_convert/mod.rs

@@ -116,7 +116,7 @@ macro_rules! impl_godot_as_self {
 
         impl $crate::builtin::meta::FromGodot for $T {
             #[inline]
-            fn try_from_godot(via: Self::Via) -> Result<Self, $crate::builtin::ConvertError> {
+            fn try_from_godot(via: Self::Via) -> Result<Self, $crate::builtin::meta::ConvertError> {
                 Ok(via)
             }
         }

godot-core/src/builtin/meta/return_marshal.rs

@@ -4,11 +4,10 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
-use crate::builtin::ConvertError;
 use crate::obj::{Gd, GodotClass};
 use crate::sys;
 
-use super::{FromGodot, GodotType};
+use super::{ConvertError, FromGodot, GodotType};
 
 /// Specifies how the return type is marshalled in a ptrcall.
 #[doc(hidden)]

godot-core/src/builtin/mod.rs

@@ -58,7 +58,6 @@ pub use vectors::*;
 
 /// Meta-information about variant types, properties and class names.
 pub mod meta;
-pub use meta::ConvertError;
 
 /// Math-related functions and traits like [`ApproxEq`][math::ApproxEq].
 pub mod math;

godot-core/src/builtin/string/mod.rs

@@ -16,8 +16,7 @@ pub use gstring::*;
 pub use node_path::*;
 pub use string_name::*;
 
-use super::meta::{FromGodot, GodotConvert, ToGodot};
-use super::ConvertError;
+use super::meta::{ConvertError, FromGodot, GodotConvert, ToGodot};
 
 impl GodotConvert for &str {
     type Via = GString;

godot-core/src/builtin/variant/impls.rs

@@ -37,11 +37,11 @@ macro_rules! impl_ffi_variant {
             fn ffi_from_variant(variant: &Variant) -> Result<Self, ConvertError> {
                 // Type check -- at the moment, a strict match is required.
                 if variant.get_type() != Self::variant_type() {
-                    return Err(ConvertError::from(FromVariantError::BadType {
+                    return Err(FromVariantError::BadType {
                         expected: Self::variant_type(),
                         got: variant.get_type(),
-                    })
-                    .with_value(variant.clone()));
+                    }
+                    .into_error(variant.clone()));
                 }
 
                 // For 4.0:
@@ -155,7 +155,11 @@ impl GodotFfiVariant for () {
             return Ok(());
         }
 
-        Err(ConvertError::from(FromVariantError::BadValue).with_value(variant.clone()))
+        Err(FromVariantError::BadType {
+            expected: VariantType::Nil,
+            got: variant.get_type(),
+        }
+        .into_error(variant.clone()))
     }
 }