From 2d69843583e449da6a492f41da3de6c7d9c422ee Mon Sep 17 00:00:00 2001 From: Kegan Dougal <7190048+kegsay@users.noreply.github.com> Date: Thu, 14 Mar 2024 15:48:20 +0000 Subject: [PATCH] Use pointers to nested variant errors to avoid compilation errors Should fix https://github.com/NordSecurity/uniffi-bindgen-go/issues/36 Definitely fixes an issue I've been having where the given test case fails with: ``` generated/errors/errors.go:1399:12: cannot use FfiConverterTypeValidationErrorINSTANCE.Read(reader) (value of type error) as ValidationError value in struct literal: need type assertion generated/errors/errors.go:1410:57: cannot use variantValue.Source (variable of type ValidationError) as *ValidationError value in argument to FfiConverterTypeValidationErrorINSTANCE.Write ``` This seems to be because the code sometimes expected `*ValidationError` and sometimes `ValidationError`. This patch makes the code use `*ValidationError` everywhere. Tests should hopefully prove they don't nil deference. Signed-off-by: Kegan Dougal <7190048+kegsay@users.noreply.github.com> --- bindgen/src/gen_go/mod.rs | 20 ++++++++++++++++++++ bindgen/templates/ErrorTemplate.go | 6 +++--- binding_tests/errors_test.go | 10 ++++++++++ fixtures/errors/src/errors.udl | 6 ++++++ fixtures/errors/src/lib.rs | 17 +++++++++++++++++ 5 files changed, 56 insertions(+), 3 deletions(-) diff --git a/bindgen/src/gen_go/mod.rs b/bindgen/src/gen_go/mod.rs index 181827f..a1e0c44 100644 --- a/bindgen/src/gen_go/mod.rs +++ b/bindgen/src/gen_go/mod.rs @@ -444,10 +444,30 @@ pub mod filters { Ok(nm.to_string().to_upper_camel_case()) } + // Return the runtime type cast of this field if it is an Enum type. In most cases + // we want to pass around the `error` interface and let the caller type cast, but in + // some cases (e.g when writing nested errors) we need to work with concrete error types + // which involve type casting from `error` to `ConcreteError`. + pub fn error_type_cast(type_: &impl AsType) -> Result { + let result = match type_.as_type() { + Type::Enum { .. } => format!(".(*{})", oracle().find(type_).type_label()), + _ => String::from(""), + }; + Ok(result) + } + pub fn type_name(type_: &impl AsType) -> Result { Ok(oracle().find(type_).type_label()) } + pub fn variant_type_name(type_: &impl AsType) -> Result { + let result = match type_.as_type() { + Type::Enum { .. } => format!("*{}", oracle().find(type_).type_label()), + _ => oracle().find(type_).type_label(), + }; + Ok(result) + } + pub fn canonical_name(type_: &impl AsType) -> Result { Ok(oracle().find(type_).canonical_name()) } diff --git a/bindgen/templates/ErrorTemplate.go b/bindgen/templates/ErrorTemplate.go index 3a58c13..2ea583e 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 }} {{ field|variant_type_name}} {%- 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 }} {{ field|variant_type_name}}, {%- endfor %} {%- endif %} ) *{{ type_name.clone() }} { @@ -110,7 +110,7 @@ func (c {{ e|ffi_converter_name }}) Read(reader io.Reader) error { case {{ loop.index }}: return &{{ type_name|class_name }}{&{{ type_name|class_name }}{{ variant.name()|class_name }}{ {%- for field in variant.fields() %} - {{ field.name()|error_field_name }}: {{ field|read_fn }}(reader), + {{ field.name()|error_field_name }}: {{ field|read_fn }}(reader){{field|error_type_cast}}, {%- endfor %} }} {%- endfor %} 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..6c1d839 100644 --- a/fixtures/errors/src/errors.udl +++ b/fixtures/errors/src/errors.udl @@ -19,6 +19,12 @@ enum BoobyTrapError { "HotDoorKnob", }; +[Error] +interface NestedError { + Nested(ValidationError source); +}; + + [Error] interface ValidationError { InvalidUser(i32 user_id); diff --git a/fixtures/errors/src/lib.rs b/fixtures/errors/src/lib.rs index 2c60467..91ccf79 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,17 @@ impl Vec2 { } } +#[uniffi::export] +fn try_nested(trip: bool) -> Result<(), NestedError> { + if trip { + Err(NestedError::Nested { + source: ValidationError::UnknownError, + }) + } else { + Ok(()) + } +} + fn try_void(trip: bool) -> Result<(), BoobyTrapError> { if trip { Err(BoobyTrapError::IceSlip)