Fix several issues dealing with the error interface

[kegsay]

This PR fixes 2 issues:

• fix an issue where nested variants did not work correctly when the nested variant was an error.
• fix an issue where a callback could not be used with an error (Error as callback parameter does not compile #36)

Both have tests added.

There's two main changes to fix these issues:

• when dealing with errors, sometimes you can't use the error interface and need to type cast to .(*LiteralError). This PR does this in tactical places to prevent compile errors. Tests should confirm that these type assertions are safe.
• when dealing with nested errors, ensure we're always working with pointers to concrete errors rather than a mix of both, which causes compilation errors.

[Savolro]

Hi! I thought of a bit different approach. This also uses pointers always. But instead of casting do all the changes during code generation. The tests from #36 and #47 pass. What do you think? Could be applied on 62738f9

diff --git a/bindgen/src/gen_go/mod.rs b/bindgen/src/gen_go/mod.rs
index 181827f..61d0766 100644
--- a/bindgen/src/gen_go/mod.rs
+++ b/bindgen/src/gen_go/mod.rs
@@ -370,7 +370,7 @@ impl GoCodeOracle {
 pub mod filters {
     use super::*;
 
-    fn oracle() -> &'static GoCodeOracle {
+    pub fn oracle() -> &'static GoCodeOracle {
         &GoCodeOracle
     }
 
@@ -578,4 +578,12 @@ impl<'a> TypeRenderer<'a> {
     pub fn cgo_callback_fn(&self, name: &str, module_path: &str) -> String {
         format!("{module_path}_cgo_{name}")
     }
+
+    pub fn field_type_name(&self, field: &Field) -> String {
+        let name = filters::oracle().find(&field.as_type()).type_label();
+        match self.ci.is_name_used_as_error(&name) {
+            true => format!("*{name}"),
+            false => name.to_string(),
+        }
+    }
 }
diff --git a/bindgen/templates/ErrorTemplate.go b/bindgen/templates/ErrorTemplate.go
index 3a58c13..69a7e66 100644
--- a/bindgen/templates/ErrorTemplate.go
+++ b/bindgen/templates/ErrorTemplate.go
@@ -30,7 +30,7 @@ type {{ variant_class_name }} struct {
 	message string
 	{%- else %}
 	{%- for field in variant.fields() %}
-	{{ field.name()|error_field_name }} {{ field|type_name}}
+	{{ field.name()|error_field_name }} {{ self.field_type_name(field) }}
 	{%- endfor %}
 	{%- endif %}
 }
@@ -39,7 +39,7 @@ type {{ variant_class_name }} struct {
 func New{{ variant_class_name }}(
 	{%- if !e.is_flat() %}
 	{%- for field in variant.fields() %}
-	{{ field.name()|var_name }} {{ field|type_name}},
+	{{ field.name()|var_name }} {{ self.field_type_name(field) }},
 	{%- endfor %}
 	{%- endif %}
 ) *{{ type_name.clone() }} {
@@ -81,14 +81,14 @@ type {{ e|ffi_converter_name }} struct{}
 var {{ e|ffi_converter_name }}INSTANCE = {{ e|ffi_converter_name }}{}
 
 func (c {{ e|ffi_converter_name }}) Lift(eb RustBufferI) error {
-	return LiftFromRustBuffer[error](c, eb)
+	return LiftFromRustBuffer[*{{ type_name|class_name }}](c, eb)
 }
 
 func (c {{ e|ffi_converter_name }}) Lower(value *{{ type_name|class_name }}) RustBuffer {
 	return LowerIntoRustBuffer[*{{ type_name|class_name }}](c, value)
 }
 
-func (c {{ e|ffi_converter_name }}) Read(reader io.Reader) error {
+func (c {{ e|ffi_converter_name }}) Read(reader io.Reader) *{{ type_name|class_name }} {
 	errorID := readUint32(reader)
 
 	{%- if e.is_flat() %}
diff --git a/binding_tests/errors_test.go b/binding_tests/errors_test.go
index 478f927..f39a5ae 100644
--- a/binding_tests/errors_test.go
+++ b/binding_tests/errors_test.go
@@ -151,3 +151,13 @@ func TestErrorNamedError(t *testing.T) {
 	assert.ErrorAs(t, err, &expectedError)
 	assert.Equal(t, "it's an error", expectedError.Unwrap().(*errors.ErrorNamedErrorError).Error_)
 }
+
+func TestNestedError(t *testing.T) {
+	assert.Equal(t, nil, errors.TryNested(false))
+	err := errors.TryNested(true)
+	var expectedError *errors.NestedError
+	assert.ErrorAs(t, err, &expectedError)
+	var expectedNestedError *errors.NestedErrorNested
+	assert.ErrorAs(t, expectedError.Unwrap(), &expectedNestedError)
+	assert.Equal(t, "ValidationError: UnknownError", expectedNestedError.Source.Error())
+}
diff --git a/fixtures/errors/src/errors.udl b/fixtures/errors/src/errors.udl
index fcbd984..dba01d3 100644
--- a/fixtures/errors/src/errors.udl
+++ b/fixtures/errors/src/errors.udl
@@ -19,6 +19,11 @@ enum BoobyTrapError {
   "HotDoorKnob",
 };
 
+[Error]
+interface NestedError {
+  Nested(ValidationError source);
+};
+
 [Error]
 interface ValidationError {
   InvalidUser(i32 user_id);
@@ -40,6 +45,10 @@ interface ComplexError {
     Option(i32? id_a, i32? id_b);
 };
 
+callback interface Callback {
+    void do_something(BoobyTrapError error);
+};
+
 dictionary Vec2 {
     double x;
     double y;
diff --git a/fixtures/errors/src/lib.rs b/fixtures/errors/src/lib.rs
index 2c60467..87b0d55 100644
--- a/fixtures/errors/src/lib.rs
+++ b/fixtures/errors/src/lib.rs
@@ -45,6 +45,12 @@ pub enum ComplexError {
     },
 }
 
+#[derive(Debug, thiserror::Error)]
+pub enum NestedError {
+    #[error(transparent)]
+    Nested { source: ValidationError },
+}
+
 #[derive(Debug)]
 pub struct Vec2 {
     x: f64,
@@ -57,6 +63,21 @@ impl Vec2 {
     }
 }
 
+#[uniffi::export]
+fn try_nested(trip: bool) -> Result<(), NestedError> {
+    if trip {
+        Err(NestedError::Nested {
+            source: ValidationError::UnknownError,
+        })
+    } else {
+        Ok(())
+    }
+}
+
+pub trait Callback {
+    fn do_something(&self, error: BoobyTrapError);
+}
+
 fn try_void(trip: bool) -> Result<(), BoobyTrapError> {
     if trip {
         Err(BoobyTrapError::IceSlip)

[kegsay]

This looks good on my end, the project I work with compiles fine with your diff. It's a better solution as it doesn't rely on runtime type assertions :D

[arg0d]

I think neither of these approaches address the underlying problems, enum/error types are ambiguous. While this PR fixes referencing errors inside an error (nested error), it does not fixed the issue of referencing error type in other declarations, i.e. function arguments, structs, associated enums. A dictionary that references an enum still generates invalid code.

If this PR fixes your immediate use case, I don't see a problem with merging it. I would prefer to have the compile time solution proposed by @Savolro, but I think even the runtime solution is fine for the time being, until the underlying issue can be addressed.

diff --git a/fixtures/errors/src/errors.udl b/fixtures/errors/src/errors.udl
index d1dda93..0a6a709 100644
--- a/fixtures/errors/src/errors.udl
+++ b/fixtures/errors/src/errors.udl
@@ -19,6 +19,10 @@ enum BoobyTrapError {
   "HotDoorKnob",
 };
 
+dictionary StructWithError {
+    ValidationError e;
+};
+
 [Error]
 interface NestedError {
   Nested(ValidationError source);
diff --git a/fixtures/errors/src/lib.rs b/fixtures/errors/src/lib.rs
index 4c51ea4..11772b0 100644
--- a/fixtures/errors/src/lib.rs
+++ b/fixtures/errors/src/lib.rs
@@ -4,6 +4,10 @@
 
 use std::collections::HashMap;
 
+struct StructWithError {
+    e: ValidationError,
+}
+
 #[derive(Debug, thiserror::Error)]
 pub enum BoobyTrapError {
     #[error("You slipped on deliberately poured ice")]

generated/errors/errors.go:952:2: undefined: FfiDestroyerTypeValidationError
generated/errors/errors.go:965:3: cannot use FfiConverterTypeValidationErrorINSTANCE.Read(reader) (value of type error) as type ValidationError in struct literal:
	need type assertion
generated/errors/errors.go:974:56: cannot use value.E (variable of type ValidationError) as type *ValidationError in argument to FfiConverterTypeValidationErrorINSTANCE.Write

[kegsay]

If this PR fixes your immediate use case, I don't see a problem with merging it.

It does solve my immediate problem, so it would be helpful to land @Savolro's patch.

[arg0d]

Can you apply his changes in this PR?

[kegsay]

Done.

[arg0d]

Is tihs a breaking change for anyone using the current main bindings? If they update to this version, will they have to update their code to make it work with these changes?

[arg0d]

Looks good, thanks!