Remove Type::Error (#1590)

mozilla · Jun 14, 2023 · 2eb3975 · 2eb3975
1 parent 45d572d
commit 2eb3975
Show file tree

Hide file tree

Showing 44 changed files with 613 additions and 640 deletions.
diff --git a/fixtures/metadata/src/tests.rs b/fixtures/metadata/src/tests.rs
@@ -252,20 +252,22 @@ mod test_metadata {
     fn test_simple_error() {
         check_metadata(
             &error::UNIFFI_META_UNIFFI_FIXTURE_METADATA_ERROR_FLATERROR,
-            ErrorMetadata {
-                module_path: "uniffi_fixture_metadata".into(),
-                name: "FlatError".into(),
-                flat: true,
-                variants: vec![
-                    VariantMetadata {
-                        name: "Overflow".into(),
-                        fields: vec![],
-                    },
-                    VariantMetadata {
-                        name: "DivideByZero".into(),
-                        fields: vec![],
-                    },
-                ],
+            ErrorMetadata::Enum {
+                enum_: EnumMetadata {
+                    module_path: "uniffi_fixture_metadata".into(),
+                    name: "FlatError".into(),
+                    variants: vec![
+                        VariantMetadata {
+                            name: "Overflow".into(),
+                            fields: vec![],
+                        },
+                        VariantMetadata {
+                            name: "DivideByZero".into(),
+                            fields: vec![],
+                        },
+                    ],
+                },
+                is_flat: true,
             },
         );
     }
@@ -274,34 +276,36 @@ mod test_metadata {
     fn test_complex_error() {
         check_metadata(
             &error::UNIFFI_META_UNIFFI_FIXTURE_METADATA_ERROR_COMPLEXERROR,
-            ErrorMetadata {
-                module_path: "uniffi_fixture_metadata".into(),
-                name: "ComplexError".into(),
-                flat: false,
-                variants: vec![
-                    VariantMetadata {
-                        name: "NotFound".into(),
-                        fields: vec![],
-                    },
-                    VariantMetadata {
-                        name: "PermissionDenied".into(),
-                        fields: vec![FieldMetadata {
-                            name: "reason".into(),
-                            ty: Type::String,
-                            default: None,
-                        }],
-                    },
-                    VariantMetadata {
-                        name: "InvalidWeapon".into(),
-                        fields: vec![FieldMetadata {
-                            name: "weapon".into(),
-                            ty: Type::Enum {
-                                name: "Weapon".into(),
-                            },
-                            default: None,
-                        }],
-                    },
-                ],
+            ErrorMetadata::Enum {
+                enum_: EnumMetadata {
+                    module_path: "uniffi_fixture_metadata".into(),
+                    name: "ComplexError".into(),
+                    variants: vec![
+                        VariantMetadata {
+                            name: "NotFound".into(),
+                            fields: vec![],
+                        },
+                        VariantMetadata {
+                            name: "PermissionDenied".into(),
+                            fields: vec![FieldMetadata {
+                                name: "reason".into(),
+                                ty: Type::String,
+                                default: None,
+                            }],
+                        },
+                        VariantMetadata {
+                            name: "InvalidWeapon".into(),
+                            fields: vec![FieldMetadata {
+                                name: "weapon".into(),
+                                ty: Type::Enum {
+                                    name: "Weapon".into(),
+                                },
+                                default: None,
+                            }],
+                        },
+                    ],
+                },
+                is_flat: false,
             },
         );
     }
@@ -436,7 +440,7 @@ mod test_function_metadata {
                 return_type: Some(Type::Enum {
                     name: "State".into(),
                 }),
-                throws: Some(Type::Error {
+                throws: Some(Type::Enum {
                     name: "FlatError".into(),
                 }),
                 checksum: UNIFFI_META_CONST_UNIFFI_FIXTURE_METADATA_FUNC_TEST_FUNC_THAT_THROWS
@@ -455,7 +459,7 @@ mod test_function_metadata {
                 is_async: false,
                 inputs: vec![],
                 return_type: None,
-                throws: Some(Type::Error {
+                throws: Some(Type::Enum {
                     name: "FlatError".into(),
                 }),
                 checksum:
@@ -534,7 +538,7 @@ mod test_function_metadata {
                 return_type: Some(Type::Enum {
                     name: "State".into(),
                 }),
-                throws: Some(Type::Error {
+                throws: Some(Type::Enum {
                     name: "FlatError".into(),
                 }),
                 checksum:

diff --git a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/error.rs b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/error.rs
@@ -4,6 +4,7 @@
 
 use crate::backend::{CodeType, Literal};
 
+// When a type is used as an error it gets a special CodeType.
 #[derive(Debug)]
 pub struct ErrorCodeType {
     id: String,

diff --git a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs
@@ -231,6 +231,14 @@ impl KotlinCodeOracle {
         type_.clone().as_type().as_codetype()
     }
 
+    fn find_as_error(&self, type_: &Type) -> Box<dyn CodeType> {
+        match type_ {
+            Type::Enum(id) => Box::new(error::ErrorCodeType::new(id.clone())),
+            // XXX - not sure how we are supposed to return askama::Error?
+            _ => panic!("unsupported type for error: {type_:?}"),
+        }
+    }
+
     /// Get the idiomatic Kotlin rendering of a class name (for enums, records, errors, etc).
     fn class_name(&self, nm: &str) -> String {
         nm.to_string().to_upper_camel_case()
@@ -333,7 +341,6 @@ impl<T: AsType> AsCodeType for T {
             Type::Enum(id) => Box::new(enum_::EnumCodeType::new(id)),
             Type::Object { name, .. } => Box::new(object::ObjectCodeType::new(name)),
             Type::Record(id) => Box::new(record::RecordCodeType::new(id)),
-            Type::Error(id) => Box::new(error::ErrorCodeType::new(id)),
             Type::CallbackInterface(id) => {
                 Box::new(callback_interface::CallbackInterfaceCodeType::new(id))
             }
@@ -393,7 +400,7 @@ pub mod filters {
 
     pub fn error_handler(result_type: &ResultType) -> Result<String, askama::Error> {
         match &result_type.throws_type {
-            Some(error_type) => type_name(error_type),
+            Some(error_type) => Ok(KotlinCodeOracle.error_name(&type_name(error_type)?)),
             None => Ok("NullCallStatusErrorHandler".into()),
         }
     }
@@ -477,6 +484,26 @@ pub mod filters {
         Ok(variant::ErrorVariantCodeTypeProvider { v: v.clone() })
     }
 
+    /// Some of the above filters have different versions to help when the type
+    /// is used as an error.
+    pub fn error_type_name(as_type: &impl AsType) -> Result<String, askama::Error> {
+        Ok(KotlinCodeOracle
+            .find_as_error(&as_type.as_type())
+            .type_label())
+    }
+
+    pub fn error_canonical_name(as_type: &impl AsType) -> Result<String, askama::Error> {
+        Ok(KotlinCodeOracle
+            .find_as_error(&as_type.as_type())
+            .canonical_name())
+    }
+
+    pub fn error_ffi_converter_name(as_type: &impl AsType) -> Result<String, askama::Error> {
+        Ok(KotlinCodeOracle
+            .find_as_error(&as_type.as_type())
+            .ffi_converter_name())
+    }
+
     /// Remove the "`" chars we put around function/variable names
     ///
     /// These are used to avoid name clashes with kotlin identifiers, but sometimes you want to

diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceTemplate.kt
@@ -104,7 +104,7 @@ internal class {{ foreign_callback }} : ForeignCallback {
         {%- when Some(error_type) %}
         fun makeCallAndHandleError()  : Int = try {
             makeCall()
-        } catch (e: {{ error_type|type_name }}) {
+        } catch (e: {{ error_type|error_type_name }}) {
             // Expected error, serialize it into outBuf
             outBuf.setValue({{ error_type|ffi_converter_name }}.lowerIntoRustBuffer(e))
             UNIFFI_CALLBACK_ERROR

diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/EnumTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/EnumTemplate.kt
@@ -4,7 +4,6 @@
 // So, we switch here, using `enum class` for enums with no associated data
 // and `sealed class` for the general case.
 #}
-{%- let e = ci.get_enum_definition(name).unwrap() %}
 
 {%- if e.is_flat() %}
 

diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/ErrorTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/ErrorTemplate.kt
@@ -1,5 +1,6 @@
-{%- let e = ci.get_error_definition(name).unwrap() %}
-
+{%- let type_name = type_|error_type_name %}
+{%- let ffi_converter_name = type_|error_ffi_converter_name %}
+{%- let canonical_type_name = type_|error_canonical_name %}
 {% if e.is_flat() %}
 sealed class {{ type_name }}(message: String): Exception(message){% if contains_object_references %}, Disposable {% endif %} {
         // Each variant is a nested class
@@ -9,7 +10,7 @@ sealed class {{ type_name }}(message: String): Exception(message){% if contains_
         {% endfor %}
 
     companion object ErrorHandler : CallStatusErrorHandler<{{ type_name }}> {
-        override fun lift(error_buf: RustBuffer.ByValue): {{ type_name }} = {{ e|lift_fn }}(error_buf)
+        override fun lift(error_buf: RustBuffer.ByValue): {{ type_name }} = {{ ffi_converter_name }}.lift(error_buf)
     }
 }
 {%- else %}
@@ -28,7 +29,7 @@ sealed class {{ type_name }}: Exception(){% if contains_object_references %}, Di
     {% endfor %}
 
     companion object ErrorHandler : CallStatusErrorHandler<{{ type_name }}> {
-        override fun lift(error_buf: RustBuffer.ByValue): {{ type_name }} = {{ e|lift_fn }}(error_buf)
+        override fun lift(error_buf: RustBuffer.ByValue): {{ type_name }} = {{ ffi_converter_name }}.lift(error_buf)
     }
 
     {% if contains_object_references %}

diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt
@@ -7,7 +7,7 @@ public interface {{ type_name }}Interface {
     {% for meth in obj.methods() -%}
     {%- match meth.throws_type() -%}
     {%- when Some with (throwable) -%}
-    @Throws({{ throwable|type_name }}::class)
+    @Throws({{ throwable|error_type_name }}::class)
     {%- when None -%}
     {%- endmatch %}
     {% if meth.is_async() -%}
@@ -51,7 +51,7 @@ class {{ type_name }}(
     {% for meth in obj.methods() -%}
     {%- match meth.throws_type() -%}
     {%- when Some with (throwable) %}
-    @Throws({{ throwable|type_name }}::class)
+    @Throws({{ throwable|error_type_name }}::class)
     {%- else -%}
     {%- endmatch -%}
     {%- if meth.is_async() %}

diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/TopLevelFunctionTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/TopLevelFunctionTemplate.kt
@@ -1,7 +1,7 @@
 {%- if func.is_async() %}
 {%- match func.throws_type() -%}
 {%- when Some with (throwable) %}
-@Throws({{ throwable|type_name }}::class)
+@Throws({{ throwable|error_type_name }}::class)
 {%- else -%}
 {%- endmatch %}
 
@@ -38,7 +38,7 @@ suspend fun {{ func.name()|fn_name }}({%- call kt::arg_list_decl(func) -%}){% ma
 {%- else %}
 {%- match func.throws_type() -%}
 {%- when Some with (throwable) %}
-@Throws({{ throwable|type_name }}::class)
+@Throws({{ throwable|error_type_name }}::class)
 {%- else -%}
 {%- endmatch -%}
 

diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/Types.kt b/uniffi_bindgen/src/bindings/kotlin/templates/Types.kt
@@ -57,10 +57,12 @@
 {%- include "ByteArrayHelper.kt" %}
 
 {%- when Type::Enum(name) %}
+{%- let e = ci.get_enum_definition(name).unwrap() %}
+{%- if !ci.is_name_used_as_error(name) %}
 {% include "EnumTemplate.kt" %}
-
-{%- when Type::Error(name) %}
+{%- else %}
 {% include "ErrorTemplate.kt" %}
+{%- endif -%}
 
 {%- when Type::Object { name, imp } %}
 {% include "ObjectTemplate.kt" %}

diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/macros.kt b/uniffi_bindgen/src/bindings/kotlin/templates/macros.kt
@@ -7,7 +7,7 @@
 {%- macro to_ffi_call(func) -%}
     {%- match func.throws_type() %}
     {%- when Some with (e) %}
-    rustCallWithError({{ e|type_name}})
+    rustCallWithError({{ e|error_type_name }})
     {%- else %}
     rustCall()
     {%- endmatch %} { _status ->
@@ -18,7 +18,7 @@
 {%- macro to_ffi_call_with_prefix(prefix, func) %}
     {%- match func.throws_type() %}
     {%- when Some with (e) %}
-    rustCallWithError({{ e|type_name}})
+    rustCallWithError({{ e|error_type_name }})
     {%- else %}
     rustCall()
     {%- endmatch %} { _status ->

diff --git a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs
@@ -19,7 +19,6 @@ mod callback_interface;
 mod compounds;
 mod custom;
 mod enum_;
-mod error;
 mod executor;
 mod external;
 mod miscellany;
@@ -357,7 +356,6 @@ impl<T: AsType> AsCodeType for T {
             Type::Enum(id) => Box::new(enum_::EnumCodeType::new(id)),
             Type::Object { name, .. } => Box::new(object::ObjectCodeType::new(name)),
             Type::Record(id) => Box::new(record::RecordCodeType::new(id)),
-            Type::Error(id) => Box::new(error::ErrorCodeType::new(id)),
             Type::CallbackInterface(id) => {
                 Box::new(callback_interface::CallbackInterfaceCodeType::new(id))
             }

diff --git a/uniffi_bindgen/src/bindings/python/templates/EnumTemplate.py b/uniffi_bindgen/src/bindings/python/templates/EnumTemplate.py
@@ -4,7 +4,6 @@
 # when none of the variants have associated data, or a generic nested-class
 # construct when they do.
 #}
-{%- let e = ci.get_enum_definition(name).unwrap() %}
 {% if e.is_flat() %}
 
 class {{ type_name }}(enum.Enum):

diff --git a/uniffi_bindgen/src/bindings/python/templates/ErrorTemplate.py b/uniffi_bindgen/src/bindings/python/templates/ErrorTemplate.py
@@ -1,5 +1,3 @@
-{%- let e = ci.get_error_definition(name).unwrap() %}
-
 # {{ type_name }}
 # We want to define each variant as a nested class that's also a subclass,
 # which is tricky in Python.  To accomplish this we're going to create each

diff --git a/uniffi_bindgen/src/bindings/python/templates/Types.py b/uniffi_bindgen/src/bindings/python/templates/Types.py
@@ -56,10 +56,13 @@
 {%- include "BytesHelper.py" %}
 
 {%- when Type::Enum(name) %}
-{%- include "EnumTemplate.py" %}
-
-{%- when Type::Error(name) %}
+{%- let e = ci.get_enum_definition(name).unwrap() %}
+{# For enums, there are either an error *or* an enum, they can't be both. #}
+{%- if ci.is_name_used_as_error(name) %}
 {%- include "ErrorTemplate.py" %}
+{%- else %}
+{%- include "EnumTemplate.py" %}
+{% endif %}
 
 {%- when Type::Record(name) %}
 {%- include "RecordTemplate.py" %}

diff --git a/uniffi_bindgen/src/bindings/python/templates/wrapper.py b/uniffi_bindgen/src/bindings/python/templates/wrapper.py
@@ -67,9 +67,6 @@
     {%- for obj in ci.object_definitions() %}
     "{{ obj|type_name }}",
     {%- endfor %}
-    {%- for e in ci.error_definitions() %}
-    "{{ e|type_name }}",
-    {%- endfor %}
     {%- for c in ci.callback_interface_definitions() %}
     "{{ c.name()|class_name }}",
     {%- endfor %}