diff --git a/fixtures/metadata/src/tests.rs b/fixtures/metadata/src/tests.rs index 0a3f7d49d2..29519fc54c 100644 --- 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 index 8e691244e8..eb16aa5ce2 100644 --- 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 index 3589cb9185..040bb2c8a7 100644 --- 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 { + 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 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 { 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 { + Ok(KotlinCodeOracle + .find_as_error(&as_type.as_type()) + .type_label()) + } + + pub fn error_canonical_name(as_type: &impl AsType) -> Result { + Ok(KotlinCodeOracle + .find_as_error(&as_type.as_type()) + .canonical_name()) + } + + pub fn error_ffi_converter_name(as_type: &impl AsType) -> Result { + 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 index 7ea7fe25d8..f3cc097239 100644 --- 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 index a4c0699ff9..798abf2b21 100644 --- 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 index 0ddfe7183f..6dc16e989a 100644 --- 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 index 520fa4f20f..95621f850e 100644 --- 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 index a4dfb6bcdc..5defe5a586 100644 --- 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 index 0ad00dce5d..2d51eb5b04 100644 --- 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 index 0c6d1f2616..d7f2cb7aba 100644 --- 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 index 6ee702ba61..c3831a35c1 100644 --- 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 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 index b24448be46..d84b08a0d8 100644 --- 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 index c10f095a13..e4bb1c6e89 100644 --- 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 index ab2767158c..ef02d8367b 100644 --- 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 index 061a967c66..a7e5a5b5ec 100644 --- 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 %} diff --git a/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs b/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs index 523a4b8a95..1bc3f1e5eb 100644 --- a/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs +++ b/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs @@ -53,7 +53,6 @@ impl Type { // However, types that support importing all end up with the same prefix of "Type", so // that the import handling code knows how to find the remote reference. Type::Object { name, .. } => format!("Type{name}"), - Type::Error(nm) => format!("Type{nm}"), Type::Enum(nm) => format!("Type{nm}"), Type::Record(nm) => format!("Type{nm}"), Type::CallbackInterface(nm) => format!("CallbackInterface{nm}"), @@ -226,9 +225,7 @@ mod filters { Type::UInt64 => format!("{ns}::uniffi_in_range({nm}, \"u64\", 0, 2**64)"), Type::Float32 | Type::Float64 => format!("{nm}.to_f"), Type::Boolean => format!("{nm} ? true : false"), - Type::Object { .. } | Type::Enum(_) | Type::Error(_) | Type::Record(_) => { - nm.to_string() - } + Type::Object { .. } | Type::Enum(_) | Type::Record(_) => nm.to_string(), Type::String | Type::Bytes => format!("{nm}.to_s"), Type::Timestamp | Type::Duration => nm.to_string(), Type::CallbackInterface(_) => panic!("No support for coercing callback interfaces yet"), @@ -276,7 +273,6 @@ mod filters { Type::Bytes => format!("RustBuffer.allocFromBytes({nm})"), Type::Object { name, .. } => format!("({}._uniffi_lower {nm})", class_name_rb(name)?), Type::CallbackInterface(_) => panic!("No support for lowering callback interfaces yet"), - Type::Error(_) => panic!("No support for lowering errors, yet"), Type::Enum(_) | Type::Record(_) | Type::Optional(_) @@ -310,9 +306,14 @@ mod filters { Type::Bytes => format!("{nm}.consumeIntoBytes"), Type::Object { name, .. } => format!("{}._uniffi_allocate({nm})", class_name_rb(name)?), Type::CallbackInterface(_) => panic!("No support for lifting callback interfaces, yet"), - Type::Error(_) => panic!("No support for lowering errors, yet"), - Type::Enum(_) - | Type::Record(_) + Type::Enum(_) => { + format!( + "{}.consumeInto{}", + nm, + class_name_rb(&type_.canonical_name())? + ) + } + Type::Record(_) | Type::Optional(_) | Type::Sequence(_) | Type::Timestamp diff --git a/uniffi_bindgen/src/bindings/ruby/templates/ErrorTemplate.rb b/uniffi_bindgen/src/bindings/ruby/templates/ErrorTemplate.rb index 3a64e9ffeb..a23324ab03 100644 --- a/uniffi_bindgen/src/bindings/ruby/templates/ErrorTemplate.rb +++ b/uniffi_bindgen/src/bindings/ruby/templates/ErrorTemplate.rb @@ -19,7 +19,8 @@ def to_s CALL_SUCCESS = 0 CALL_ERROR = 1 CALL_PANIC = 2 -{%- for e in ci.error_definitions() %} +{%- for e in ci.enum_definitions() %} +{% if ci.is_name_used_as_error(e.name()) %} {% if e.is_flat() %} class {{ e.name()|class_name_rb }} {%- for variant in e.variants() %} @@ -47,14 +48,17 @@ def to_s {%- endfor %} {% endif %} end +{% endif %} {%- endfor %} # Map error modules to the RustBuffer method name that reads them ERROR_MODULE_TO_READER_METHOD = { -{%- for e in ci.error_definitions() %} +{%- for e in ci.enum_definitions() %} +{% if ci.is_name_used_as_error(e.name()) %} {%- let typ=ci.get_type(e.name()).unwrap() %} {%- let canonical_type_name = typ.canonical_name().borrow()|class_name_rb %} {{ e.name()|class_name_rb }} => :read{{ canonical_type_name }}, +{% endif %} {%- endfor %} } diff --git a/uniffi_bindgen/src/bindings/ruby/templates/RustBufferBuilder.rb b/uniffi_bindgen/src/bindings/ruby/templates/RustBufferBuilder.rb index e1bdb85e38..9035563ade 100644 --- a/uniffi_bindgen/src/bindings/ruby/templates/RustBufferBuilder.rb +++ b/uniffi_bindgen/src/bindings/ruby/templates/RustBufferBuilder.rb @@ -168,6 +168,7 @@ def write_{{ canonical_type_name }}(obj) end {% when Type::Enum with (enum_name) -%} + {% if !ci.is_name_used_as_error(enum_name) %} {%- let e = ci.get_enum_definition(enum_name).unwrap() -%} # The Enum type {{ enum_name }}. @@ -185,6 +186,7 @@ def write_{{ canonical_type_name }}(v) {%- endfor %} {%- endif %} end + {% endif %} {% when Type::Record with (record_name) -%} {%- let rec = ci.get_record_definition(record_name).unwrap() -%} diff --git a/uniffi_bindgen/src/bindings/ruby/templates/RustBufferStream.rb b/uniffi_bindgen/src/bindings/ruby/templates/RustBufferStream.rb index 941a018f78..04cf09f7d0 100644 --- a/uniffi_bindgen/src/bindings/ruby/templates/RustBufferStream.rb +++ b/uniffi_bindgen/src/bindings/ruby/templates/RustBufferStream.rb @@ -158,8 +158,10 @@ def read{{ canonical_type_name }} return {{ object_name|class_name_rb }}._uniffi_allocate(pointer) end - {% when Type::Enum with (enum_name) -%} - {%- let e = ci.get_enum_definition(enum_name).unwrap() -%} + {% when Type::Enum with (name) -%} + {%- let e = ci.get_enum_definition(name).unwrap() -%} + {% if !ci.is_name_used_as_error(name) %} + {% let enum_name = name %} # The Enum type {{ enum_name }}. def read{{ canonical_type_name }} @@ -190,8 +192,9 @@ def read{{ canonical_type_name }} {%- endif %} end - {% when Type::Error with (error_name) -%} - {%- let e = ci.get_error_definition(error_name).unwrap().wrapped_enum() %} + {% else %} + + {% let error_name = name %} # The Error type {{ error_name }} @@ -225,6 +228,7 @@ def read{{ canonical_type_name }} raise InternalError, 'Unexpected variant tag for {{ canonical_type_name }}' {%- endif %} end + {% endif %} {% when Type::Record with (record_name) -%} {%- let rec = ci.get_record_definition(record_name).unwrap() -%} diff --git a/uniffi_bindgen/src/bindings/ruby/templates/RustBufferTemplate.rb b/uniffi_bindgen/src/bindings/ruby/templates/RustBufferTemplate.rb index e2dcb1db0f..b7ea8a0300 100644 --- a/uniffi_bindgen/src/bindings/ruby/templates/RustBufferTemplate.rb +++ b/uniffi_bindgen/src/bindings/ruby/templates/RustBufferTemplate.rb @@ -142,6 +142,7 @@ def consumeInto{{ canonical_type_name }} end {% when Type::Enum with (enum_name) -%} + {% if !ci.is_name_used_as_error(enum_name) %} {%- let e = ci.get_enum_definition(enum_name).unwrap() -%} # The Enum type {{ enum_name }}. @@ -157,6 +158,7 @@ def consumeInto{{ canonical_type_name }} return stream.read{{ canonical_type_name }} end end + {% endif %} {% when Type::Optional with (inner_type) -%} # The Optional type for {{ inner_type.canonical_name() }}. diff --git a/uniffi_bindgen/src/bindings/ruby/templates/wrapper.rb b/uniffi_bindgen/src/bindings/ruby/templates/wrapper.rb index 3511a9b2c4..22c3c18b30 100644 --- a/uniffi_bindgen/src/bindings/ruby/templates/wrapper.rb +++ b/uniffi_bindgen/src/bindings/ruby/templates/wrapper.rb @@ -31,7 +31,9 @@ module {{ ci.namespace()|class_name_rb }} # Public interface members begin here. {% for e in ci.enum_definitions() %} + {% if !ci.is_name_used_as_error(e.name()) %} {% include "EnumTemplate.rb" %} + {% endif %} {%- endfor -%} {%- for rec in ci.record_definitions() %} diff --git a/uniffi_bindgen/src/bindings/swift/gen_swift/error.rs b/uniffi_bindgen/src/bindings/swift/gen_swift/error.rs deleted file mode 100644 index 366be003fa..0000000000 --- a/uniffi_bindgen/src/bindings/swift/gen_swift/error.rs +++ /dev/null @@ -1,26 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -use crate::backend::CodeType; - -#[derive(Debug)] -pub struct ErrorCodeType { - id: String, -} - -impl ErrorCodeType { - pub fn new(id: String) -> Self { - Self { id } - } -} - -impl CodeType for ErrorCodeType { - fn type_label(&self) -> String { - super::SwiftCodeOracle.class_name(&self.id) - } - - fn canonical_name(&self) -> String { - format!("Type{}", self.id) - } -} diff --git a/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs b/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs index 3d3fcaca42..741bf8729b 100644 --- a/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs +++ b/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs @@ -20,7 +20,6 @@ mod callback_interface; mod compounds; mod custom; mod enum_; -mod error; mod executor; mod external; mod miscellany; @@ -306,7 +305,6 @@ impl SwiftCodeOracle { 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/swift/templates/EnumTemplate.swift b/uniffi_bindgen/src/bindings/swift/templates/EnumTemplate.swift index 74f0a3effe..bc6a547867 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/EnumTemplate.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/EnumTemplate.swift @@ -1,6 +1,5 @@ // Note that we don't yet support `indirect` for enums. // See https://github.com/mozilla/uniffi-rs/issues/396 for further discussion. -{%- let e = ci.get_enum_definition(name).unwrap() %} public enum {{ type_name }} { {% for variant in e.variants() %} case {{ variant.name()|enum_variant_swift }}{% if variant.fields().len() > 0 %}({% call swift::field_list_decl(variant) %}){% endif -%} diff --git a/uniffi_bindgen/src/bindings/swift/templates/ErrorTemplate.swift b/uniffi_bindgen/src/bindings/swift/templates/ErrorTemplate.swift index 5c6a9ac839..c7b87df9ca 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/ErrorTemplate.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/ErrorTemplate.swift @@ -1,4 +1,3 @@ -{%- let e = ci.get_error_definition(name).unwrap() %} public enum {{ type_name }} { {% if e.is_flat() %} diff --git a/uniffi_bindgen/src/bindings/swift/templates/Types.swift b/uniffi_bindgen/src/bindings/swift/templates/Types.swift index b869e28428..9bbf8e4493 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/Types.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/Types.swift @@ -71,10 +71,12 @@ {%- include "CustomType.swift" %} {%- when Type::Enum(name) %} -{%- include "EnumTemplate.swift" %} - -{%- when Type::Error(name) %} +{%- let e = ci.get_enum_definition(name).unwrap() %} +{%- if ci.is_name_used_as_error(name) %} {%- include "ErrorTemplate.swift" %} +{%- else %} +{%- include "EnumTemplate.swift" %} +{% endif %} {%- when Type::Object{ name, imp } %} {%- include "ObjectTemplate.swift" %} diff --git a/uniffi_bindgen/src/interface/enum_.rs b/uniffi_bindgen/src/interface/enum_.rs index 9c627c859e..69f02678c6 100644 --- a/uniffi_bindgen/src/interface/enum_.rs +++ b/uniffi_bindgen/src/interface/enum_.rs @@ -75,6 +75,85 @@ //! assert_eq!(e.variants()[1].fields()[0].name(), "first"); //! # Ok::<(), anyhow::Error>(()) //! ``` +//! +//! # Enums are also used to represent error definitions for a `ComponentInterface`. +//! +//! ``` +//! # let ci = uniffi_bindgen::interface::ComponentInterface::from_webidl(r##" +//! # namespace example {}; +//! [Error] +//! enum Example { +//! "one", +//! "two" +//! }; +//! # "##)?; +//! # Ok::<(), anyhow::Error>(()) +//! ``` +//! +//! Will result in an [`Enum`] member with fieldless variants being added to the resulting [`ComponentInterface`]: +//! +//! ``` +//! # let ci = uniffi_bindgen::interface::ComponentInterface::from_webidl(r##" +//! # namespace example {}; +//! # [Error] +//! # enum Example { +//! # "one", +//! # "two" +//! # }; +//! # "##)?; +//! let err = ci.get_enum_definition("Example").unwrap(); +//! assert_eq!(err.name(), "Example"); +//! assert_eq!(err.variants().len(), 2); +//! assert_eq!(err.variants()[0].name(), "one"); +//! assert_eq!(err.variants()[1].name(), "two"); +//! assert_eq!(err.is_flat(), true); +//! assert!(ci.is_name_used_as_error(&err.name())); +//! # Ok::<(), anyhow::Error>(()) +//! ``` +//! +//! A declaration in the UDL like this: +//! +//! ``` +//! # let ci = uniffi_bindgen::interface::ComponentInterface::from_webidl(r##" +//! # namespace example {}; +//! [Error] +//! interface Example { +//! one(i16 code); +//! two(string reason); +//! three(i32 x, i32 y); +//! }; +//! # "##)?; +//! # Ok::<(), anyhow::Error>(()) +//! ``` +//! +//! Will result in an [`Enum`] member with variants that have fields being added to the resulting [`ComponentInterface`]: +//! +//! ``` +//! # let ci = uniffi_bindgen::interface::ComponentInterface::from_webidl(r##" +//! # namespace example {}; +//! # [Error] +//! # interface Example { +//! # one(); +//! # two(string reason); +//! # three(i32 x, i32 y); +//! # }; +//! # "##)?; +//! let err = ci.get_enum_definition("Example").unwrap(); +//! assert_eq!(err.name(), "Example"); +//! assert_eq!(err.variants().len(), 3); +//! assert_eq!(err.variants()[0].name(), "one"); +//! assert_eq!(err.variants()[1].name(), "two"); +//! assert_eq!(err.variants()[2].name(), "three"); +//! assert_eq!(err.variants()[0].fields().len(), 0); +//! assert_eq!(err.variants()[1].fields().len(), 1); +//! assert_eq!(err.variants()[1].fields()[0].name(), "reason"); +//! assert_eq!(err.variants()[2].fields().len(), 2); +//! assert_eq!(err.variants()[2].fields()[0].name(), "x"); +//! assert_eq!(err.variants()[2].fields()[1].name(), "y"); +//! assert_eq!(err.is_flat(), false); +//! assert!(ci.is_name_used_as_error(err.name())); +//! # Ok::<(), anyhow::Error>(()) +//! ``` use anyhow::{bail, Result}; use uniffi_meta::Checksum; @@ -128,19 +207,14 @@ impl Enum { pub fn iter_types(&self) -> TypeIterator<'_> { Box::new(self.variants.iter().flat_map(Variant::iter_types)) } -} - -impl AsType for Enum { - fn as_type(&self) -> Type { - Type::Enum(self.name.clone()) - } -} -impl TryFrom for Enum { - type Error = anyhow::Error; - - fn try_from(meta: uniffi_meta::EnumMetadata) -> Result { - let flat = meta.variants.iter().all(|v| v.fields.is_empty()); + // Sadly can't use TryFrom due to the 'is_flat' complication. + pub fn try_from_meta(meta: uniffi_meta::EnumMetadata, flat: bool) -> Result { + // This is messy - error enums are considered "flat" if the user + // opted in via a special attribute, regardless of whether the enum + // is actually flat. + // Real enums are considered flat iff they are actually flat. + // We don't have that context here, so this is handled by our caller. Ok(Self { name: meta.name, variants: meta @@ -153,6 +227,12 @@ impl TryFrom for Enum { } } +impl AsType for Enum { + fn as_type(&self) -> Type { + Type::Enum(self.name.clone()) + } +} + // Note that we have two `APIConverter` impls here - one for the `enum` case // and one for the `[Enum] interface` case. @@ -441,7 +521,9 @@ mod test { FfiType::RustBuffer(None) ); let fret = ci.get_function_definition("returns_an_enum").unwrap(); - assert!(matches!(fret.return_type(), Some(Type::Enum(nm)) if nm == "TestEnum")); + assert!( + matches!(fret.return_type(), Some(Type::Enum(name)) if name == "TestEnum" && !ci.is_name_used_as_error(name)) + ); assert!(matches!( fret.ffi_func().return_type(), Some(FfiType::RustBuffer(None)) @@ -462,10 +544,78 @@ mod test { let fret = ci .get_function_definition("returns_an_enum_with_data") .unwrap(); - assert!(matches!(fret.return_type(), Some(Type::Enum(nm)) if nm == "TestEnumWithData")); + assert!( + matches!(fret.return_type(), Some(Type::Enum(name)) if name == "TestEnumWithData" && !ci.is_name_used_as_error(name)) + ); assert!(matches!( fret.ffi_func().return_type(), Some(FfiType::RustBuffer(None)) )); } + + // Tests for [Error], which are represented as `Enum` + #[test] + fn test_variants() { + const UDL: &str = r#" + namespace test{}; + [Error] + enum Testing { "one", "two", "three" }; + "#; + let ci = ComponentInterface::from_webidl(UDL).unwrap(); + assert_eq!(ci.enum_definitions().count(), 1); + let error = ci.get_enum_definition("Testing").unwrap(); + assert_eq!( + error + .variants() + .iter() + .map(|v| v.name()) + .collect::>(), + vec!("one", "two", "three") + ); + assert!(error.is_flat()); + assert!(ci.is_name_used_as_error(&error.name)); + } + + #[test] + fn test_duplicate_error_variants() { + const UDL: &str = r#" + namespace test{}; + // Weird, but currently allowed! + // We should probably disallow this... + [Error] + enum Testing { "one", "two", "one" }; + "#; + let ci = ComponentInterface::from_webidl(UDL).unwrap(); + assert_eq!(ci.enum_definitions().count(), 1); + assert_eq!( + ci.get_enum_definition("Testing").unwrap().variants().len(), + 3 + ); + } + + #[test] + fn test_variant_data() { + const UDL: &str = r#" + namespace test{}; + + [Error] + interface Testing { + One(string reason); + Two(u8 code); + }; + "#; + let ci = ComponentInterface::from_webidl(UDL).unwrap(); + assert_eq!(ci.enum_definitions().count(), 1); + let error: &Enum = ci.get_enum_definition("Testing").unwrap(); + assert_eq!( + error + .variants() + .iter() + .map(|v| v.name()) + .collect::>(), + vec!("One", "Two") + ); + assert!(!error.is_flat()); + assert!(ci.is_name_used_as_error(&error.name)); + } } diff --git a/uniffi_bindgen/src/interface/error.rs b/uniffi_bindgen/src/interface/error.rs deleted file mode 100644 index 45e2540659..0000000000 --- a/uniffi_bindgen/src/interface/error.rs +++ /dev/null @@ -1,235 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -//! # Error definitions for a `ComponentInterface`. -//! -//! This module converts error definition from UDL into structures that can be -//! added to a `ComponentInterface`. A declaration in the UDL like this: -//! -//! ``` -//! # let ci = uniffi_bindgen::interface::ComponentInterface::from_webidl(r##" -//! # namespace example {}; -//! [Error] -//! enum Example { -//! "one", -//! "two" -//! }; -//! # "##)?; -//! # Ok::<(), anyhow::Error>(()) -//! ``` -//! -//! Will result in an [`Error`] member with fieldless variants being added to the resulting [`ComponentInterface`]: -//! -//! ``` -//! # let ci = uniffi_bindgen::interface::ComponentInterface::from_webidl(r##" -//! # namespace example {}; -//! # [Error] -//! # enum Example { -//! # "one", -//! # "two" -//! # }; -//! # "##)?; -//! let err = ci.get_error_definition("Example").unwrap(); -//! assert_eq!(err.name(), "Example"); -//! assert_eq!(err.variants().len(), 2); -//! assert_eq!(err.variants()[0].name(), "one"); -//! assert_eq!(err.variants()[1].name(), "two"); -//! assert_eq!(err.is_flat(), true); -//! # Ok::<(), anyhow::Error>(()) -//! ``` -//! -//! A declaration in the UDL like this: -//! -//! ``` -//! # let ci = uniffi_bindgen::interface::ComponentInterface::from_webidl(r##" -//! # namespace example {}; -//! [Error] -//! interface Example { -//! one(i16 code); -//! two(string reason); -//! three(i32 x, i32 y); -//! }; -//! # "##)?; -//! # Ok::<(), anyhow::Error>(()) -//! ``` -//! -//! Will result in an [`Error`] member with variants that have fields being added to the resulting [`ComponentInterface`]: -//! -//! ``` -//! # let ci = uniffi_bindgen::interface::ComponentInterface::from_webidl(r##" -//! # namespace example {}; -//! # [Error] -//! # interface Example { -//! # one(); -//! # two(string reason); -//! # three(i32 x, i32 y); -//! # }; -//! # "##)?; -//! let err = ci.get_error_definition("Example").unwrap(); -//! assert_eq!(err.name(), "Example"); -//! assert_eq!(err.variants().len(), 3); -//! assert_eq!(err.variants()[0].name(), "one"); -//! assert_eq!(err.variants()[1].name(), "two"); -//! assert_eq!(err.variants()[2].name(), "three"); -//! assert_eq!(err.variants()[0].fields().len(), 0); -//! assert_eq!(err.variants()[1].fields().len(), 1); -//! assert_eq!(err.variants()[1].fields()[0].name(), "reason"); -//! assert_eq!(err.variants()[2].fields().len(), 2); -//! assert_eq!(err.variants()[2].fields()[0].name(), "x"); -//! assert_eq!(err.variants()[2].fields()[1].name(), "y"); -//! assert_eq!(err.is_flat(), false); -//! # Ok::<(), anyhow::Error>(()) -//! ``` - -use anyhow::Result; -use uniffi_meta::Checksum; - -use super::enum_::{Enum, Variant}; -use super::types::{Type, TypeIterator}; -use super::{APIConverter, AsType, ComponentInterface}; - -/// Represents an Error that might be thrown by functions/methods in the component interface. -/// -/// Errors are represented in the UDL as enums with the special `[Error]` attribute, but -/// they're handled in the FFI very differently. Defining an error means that exported functions -/// can return a `Result` where `T` is a UniFFI type and `E` is the error type. -#[derive(Debug, Clone, PartialEq, Eq, Checksum)] -pub struct Error { - pub name: String, - enum_: Enum, -} - -impl Error { - pub fn from_enum(enum_: Enum) -> Self { - Self { - name: enum_.name.clone(), - enum_, - } - } - - pub fn name(&self) -> &str { - &self.name - } - - pub fn wrapped_enum(&self) -> &Enum { - &self.enum_ - } - - pub fn variants(&self) -> &[Variant] { - self.enum_.variants() - } - - pub fn is_flat(&self) -> bool { - self.enum_.is_flat() - } - - pub fn iter_types(&self) -> TypeIterator<'_> { - self.wrapped_enum().iter_types() - } -} - -impl AsType for Error { - fn as_type(&self) -> Type { - Type::Error(self.name.clone()) - } -} - -impl TryFrom for Error { - type Error = anyhow::Error; - - fn try_from(meta: uniffi_meta::ErrorMetadata) -> Result { - Ok(Self { - name: meta.name.clone(), - enum_: Enum { - name: meta.name, - variants: meta - .variants - .into_iter() - .map(TryInto::try_into) - .collect::>()?, - flat: meta.flat, - }, - }) - } -} - -impl APIConverter for weedle::EnumDefinition<'_> { - fn convert(&self, ci: &mut ComponentInterface) -> Result { - Ok(Error::from_enum(APIConverter::::convert(self, ci)?)) - } -} - -impl APIConverter for weedle::InterfaceDefinition<'_> { - fn convert(&self, ci: &mut ComponentInterface) -> Result { - Ok(Error::from_enum(APIConverter::::convert(self, ci)?)) - } -} - -#[cfg(test)] -mod test { - use super::*; - - #[test] - fn test_variants() { - const UDL: &str = r#" - namespace test{}; - [Error] - enum Testing { "one", "two", "three" }; - "#; - let ci = ComponentInterface::from_webidl(UDL).unwrap(); - assert_eq!(ci.error_definitions().count(), 1); - let error = ci.get_error_definition("Testing").unwrap(); - assert_eq!( - error - .variants() - .iter() - .map(|v| v.name()) - .collect::>(), - vec!("one", "two", "three") - ); - assert!(error.is_flat()); - } - - #[test] - fn test_duplicate_variants() { - const UDL: &str = r#" - namespace test{}; - // Weird, but currently allowed! - // We should probably disallow this... - [Error] - enum Testing { "one", "two", "one" }; - "#; - let ci = ComponentInterface::from_webidl(UDL).unwrap(); - assert_eq!(ci.error_definitions().count(), 1); - assert_eq!( - ci.get_error_definition("Testing").unwrap().variants().len(), - 3 - ); - } - - #[test] - fn test_variant_data() { - const UDL: &str = r#" - namespace test{}; - - [Error] - interface Testing { - One(string reason); - Two(u8 code); - }; - "#; - let ci = ComponentInterface::from_webidl(UDL).unwrap(); - assert_eq!(ci.error_definitions().count(), 1); - let error: &Error = ci.get_error_definition("Testing").unwrap(); - assert_eq!( - error - .variants() - .iter() - .map(|v| v.name()) - .collect::>(), - vec!("One", "Two") - ); - assert!(!error.is_flat()); - } -} diff --git a/uniffi_bindgen/src/interface/function.rs b/uniffi_bindgen/src/interface/function.rs index c8f06609dd..476ce8f2f2 100644 --- a/uniffi_bindgen/src/interface/function.rs +++ b/uniffi_bindgen/src/interface/function.rs @@ -36,11 +36,11 @@ use std::convert::TryFrom; use anyhow::{bail, Result}; use uniffi_meta::Checksum; -use super::attributes::{ArgumentAttributes, Attribute, FunctionAttributes}; +use super::attributes::{ArgumentAttributes, FunctionAttributes}; use super::ffi::{FfiArgument, FfiFunction, FfiType}; use super::literal::{convert_default_value, Literal}; use super::types::{ObjectImpl, Type, TypeIterator}; -use super::{convert_type, APIConverter, AsType, ComponentInterface}; +use super::{APIConverter, AsType, ComponentInterface}; /// Represents a standalone function. /// @@ -62,7 +62,7 @@ pub struct Function { // avoids a weird circular dependency in the calculation. #[checksum_ignore] pub(super) ffi_func: FfiFunction, - pub(super) attributes: FunctionAttributes, + pub(super) throws: Option, pub(super) checksum_fn_name: String, // Force a checksum value. This is used for functions from the proc-macro code, which uses a // different checksum method. @@ -105,17 +105,15 @@ impl Function { } pub fn throws(&self) -> bool { - self.attributes.get_throws_err().is_some() + self.throws.is_some() } pub fn throws_name(&self) -> Option<&str> { - self.attributes.get_throws_err() + super::throws_name(&self.throws) } - pub fn throws_type(&self) -> Option { - self.attributes - .get_throws_err() - .map(|name| Type::Error(name.to_owned())) + pub fn throws_type(&self) -> Option<&Type> { + self.throws.as_ref() } pub fn derive_ffi_func(&mut self, ci_namespace: &str) -> Result<()> { @@ -136,7 +134,7 @@ impl From for Argument { fn from(meta: uniffi_meta::FnParamMetadata) -> Self { Argument { name: meta.name, - type_: convert_type(&meta.ty), + type_: meta.ty.into(), by_ref: false, optional: false, default: None, @@ -149,7 +147,7 @@ impl From for Function { let ffi_name = meta.ffi_symbol_name(); let checksum_fn_name = meta.checksum_symbol_name(); let is_async = meta.is_async; - let return_type = meta.return_type.map(|out| convert_type(&out)); + let return_type = meta.return_type.map(Into::into); let arguments = meta.inputs.into_iter().map(Into::into).collect(); let ffi_func = FfiFunction { @@ -158,19 +156,19 @@ impl From for Function { ..FfiFunction::default() }; + let throws = match meta.throws { + None => None, + Some(uniffi_meta::Type::Enum { name, .. }) => Some(Type::Enum(name)), + _ => panic!("unsupported error type {:?}", meta.throws), + }; + Self { name: meta.name, is_async, arguments, return_type, ffi_func, - attributes: match meta.throws { - None => FunctionAttributes::from_iter(vec![]), - Some(uniffi_meta::Type::Error { name }) => { - FunctionAttributes::from_iter(vec![Attribute::Throws(name)]) - } - Some(ty) => panic!("Invalid throws type: {ty:?}"), - }, + throws, checksum_fn_name, checksum_override: Some(meta.checksum), } @@ -194,13 +192,24 @@ impl APIConverter for weedle::namespace::OperationNamespaceMember<'_> Some(id) => id.0.to_string(), }; let checksum_fn_name = uniffi_meta::fn_checksum_symbol_name(ci.namespace(), &name); + let attrs = FunctionAttributes::try_from(self.attributes.as_ref())?; + let throws = match attrs.get_throws_err() { + None => None, + Some(name) => match ci.get_type(name) { + Some(t) => { + ci.note_name_used_as_error(name); + Some(t) + } + None => bail!("unknown type for error: {name}"), + }, + }; Ok(Function { name, is_async: false, return_type, arguments: self.args.body.list.convert(ci)?, ffi_func: Default::default(), - attributes: FunctionAttributes::try_from(self.attributes.as_ref())?, + throws, checksum_fn_name, checksum_override: None, }) @@ -323,7 +332,7 @@ impl Callable for Function { } fn throws_type(&self) -> Option { - self.throws_type() + self.throws_type().cloned() } } @@ -375,7 +384,9 @@ mod test { func2.return_type().unwrap().canonical_name(), "SequenceOptionalstring" ); - assert!(matches!(func2.throws_type(), Some(Type::Error(s)) if s == "TestError")); + assert!( + matches!(func2.throws_type(), Some(Type::Enum(name)) if name == "TestError" && ci.is_name_used_as_error(name)) + ); assert_eq!(func2.arguments().len(), 2); assert_eq!(func2.arguments()[0].name(), "arg1"); assert_eq!(func2.arguments()[0].as_type().canonical_name(), "u32"); diff --git a/uniffi_bindgen/src/interface/mod.rs b/uniffi_bindgen/src/interface/mod.rs index 7ab90f8eeb..60bacde09a 100644 --- a/uniffi_bindgen/src/interface/mod.rs +++ b/uniffi_bindgen/src/interface/mod.rs @@ -61,8 +61,6 @@ mod callbacks; pub use callbacks::CallbackInterface; mod enum_; pub use enum_::{Enum, Variant}; -mod error; -pub use error::Error; mod function; pub use function::{Argument, Callable, Function, ResultType}; mod literal; @@ -100,7 +98,8 @@ pub struct ComponentInterface { functions: Vec, objects: Vec, callback_interfaces: Vec, - errors: BTreeMap, + // Type names which were seen used as an error. + errors: HashSet, } impl ComponentInterface { @@ -206,16 +205,6 @@ impl ComponentInterface { self.callback_interfaces.iter().find(|o| o.name == name) } - /// Get the definitions for every Error type in the interface. - pub fn error_definitions(&self) -> impl Iterator { - self.errors.values() - } - - /// Get an Error definition by name, or None if no such Error is defined. - pub fn get_error_definition(&self, name: &str) -> Option<&Error> { - self.errors.get(name) - } - /// Get the definitions for every Method type in the interface. pub fn iter_callables(&self) -> impl Iterator { // Each of the `as &dyn Callable` casts is a trivial cast, but it seems like the clearest @@ -237,18 +226,18 @@ impl ComponentInterface { /// This is a workaround for the fact that lower/write can't be generated for some errors, /// specifically errors that are defined as flat in the UDL, but actually have fields in the /// Rust source. - pub fn should_generate_error_read(&self, error: &Error) -> bool { + pub fn should_generate_error_read(&self, e: &Enum) -> bool { // We can and should always generate read() methods for fielded errors - let fielded = !error.is_flat(); + let fielded = !e.is_flat(); // For flat errors, we should only generate read() methods if we need them to support // callback interface errors let used_in_callback_interface = self .callback_interface_definitions() .iter() .flat_map(|cb| cb.methods()) - .any(|m| m.throws_type() == Some(error.as_type())); + .any(|m| m.throws_type() == Some(&e.as_type())); - fielded || used_in_callback_interface + self.is_name_used_as_error(&e.name) && (fielded || used_in_callback_interface) } /// Get details about all `Type::External` types @@ -718,34 +707,20 @@ impl ComponentInterface { self.objects.push(defn); } + pub(super) fn note_name_used_as_error(&mut self, name: &str) { + self.errors.insert(name.to_string()); + } + + pub fn is_name_used_as_error(&self, name: &str) -> bool { + self.errors.contains(name) + } + /// Called by `APIBuilder` impls to add a newly-parsed callback interface definition to the `ComponentInterface`. fn add_callback_interface_definition(&mut self, defn: CallbackInterface) { // Note that there will be no duplicates thanks to the previous type-finding pass. self.callback_interfaces.push(defn); } - /// Called by `APIBuilder` impls to add a newly-parsed error definition to the `ComponentInterface`. - pub(super) fn add_error_definition(&mut self, defn: Error) -> Result<()> { - match self.errors.entry(defn.name().to_owned()) { - Entry::Vacant(v) => { - v.insert(defn); - } - Entry::Occupied(o) => { - let existing_def = o.get(); - if defn != *existing_def { - bail!( - "Mismatching definition for error `{}`!\n\ - existing definition: {existing_def:#?},\n\ - new definition: {defn:#?}", - defn.name(), - ); - } - } - } - - Ok(()) - } - /// Perform global consistency checks on the declared interface. /// /// This method checks for consistency problems in the declared interface @@ -758,12 +733,16 @@ impl ComponentInterface { // To keep codegen tractable, enum variant names must not shadow type names. for e in self.enums.values() { - for variant in &e.variants { - if self.types.get_type_definition(variant.name()).is_some() { - bail!( - "Enum variant names must not shadow type names: \"{}\"", - variant.name() - ) + // Not sure why this was deemed important for "normal" enums but + // not those used as errors, but this is what we've always done. + if !self.is_name_used_as_error(&e.name) { + for variant in &e.variants { + if self.types.get_type_definition(variant.name()).is_some() { + bail!( + "Enum variant names must not shadow type names: \"{}\"", + variant.name() + ) + } } } } @@ -878,7 +857,6 @@ impl<'a> RecursiveTypeIterator<'a> { match type_ { Type::Record(nm) | Type::Enum(nm) - | Type::Error(nm) | Type::Object { name: nm, .. } | Type::CallbackInterface(nm) => { if !self.seen.contains(nm.as_str()) { @@ -903,8 +881,7 @@ impl<'a> RecursiveTypeIterator<'a> { // call to `next()` to try again with the next pending type. let next_iter = match next_type { Type::Record(nm) => self.ci.get_record_definition(nm).map(Record::iter_types), - Type::Enum(nm) => self.ci.get_enum_definition(nm).map(Enum::iter_types), - Type::Error(nm) => self.ci.get_error_definition(nm).map(Error::iter_types), + Type::Enum(name) => self.ci.get_enum_definition(name).map(Enum::iter_types), Type::Object { name: nm, .. } => { self.ci.get_object_definition(nm).map(Object::iter_types) } @@ -970,8 +947,9 @@ impl APIBuilder for weedle::Definition<'_> { // We check if the enum represents an error... let attrs = attributes::EnumAttributes::try_from(d.attributes.as_ref())?; if attrs.contains_error_attr() { - let err = d.convert(ci)?; - ci.add_error_definition(err)?; + let e = d.convert(ci)?; + ci.note_name_used_as_error(&e.name); + ci.add_enum_definition(e)?; } else { let e = d.convert(ci)?; ci.add_enum_definition(e)?; @@ -987,8 +965,9 @@ impl APIBuilder for weedle::Definition<'_> { let e = d.convert(ci)?; ci.add_enum_definition(e)?; } else if attrs.contains_error_attr() { - let e = d.convert(ci)?; - ci.add_error_definition(e)?; + let e: Enum = d.convert(ci)?; + ci.note_name_used_as_error(&e.name); + ci.add_enum_definition(e)?; } else { let obj = d.convert(ci)?; ci.add_object_definition(obj); @@ -1029,49 +1008,13 @@ impl> APIConverter> for Vec { } } -fn convert_type(s: &uniffi_meta::Type) -> Type { - use uniffi_meta::Type as Ty; - - match s { - Ty::U8 => Type::UInt8, - Ty::U16 => Type::UInt16, - Ty::U32 => Type::UInt32, - Ty::U64 => Type::UInt64, - Ty::I8 => Type::Int8, - Ty::I16 => Type::Int16, - Ty::I32 => Type::Int32, - Ty::I64 => Type::Int64, - Ty::F32 => Type::Float32, - Ty::F64 => Type::Float64, - Ty::Bool => Type::Boolean, - Ty::String => Type::String, - Ty::SystemTime => Type::Timestamp, - Ty::Duration => Type::Duration, - Ty::ForeignExecutor => Type::ForeignExecutor, - Ty::Record { name } => Type::Record(name.clone()), - Ty::Enum { name } => Type::Enum(name.clone()), - Ty::ArcObject { - object_name, - is_trait, - } => Type::Object { - name: object_name.clone(), - imp: ObjectImpl::from_is_trait(*is_trait), - }, - Ty::Error { name } => Type::Error(name.clone()), - Ty::CallbackInterface { name } => Type::CallbackInterface(name.clone()), - Ty::Custom { name, builtin } => Type::Custom { - name: name.clone(), - builtin: convert_type(builtin).into(), - }, - Ty::Option { inner_type } => Type::Optional(convert_type(inner_type).into()), - Ty::Vec { inner_type } => Type::Sequence(convert_type(inner_type).into()), - Ty::HashMap { - key_type, - value_type, - } => Type::Map( - convert_type(key_type).into(), - convert_type(value_type).into(), - ), +// Helpers for functions/methods/constructors which all have the same "throws" semantics. +fn throws_name(throws: &Option) -> Option<&str> { + // Type has no `name()` method, just `canonical_name()` which isn't what we want. + match throws { + None => None, + Some(Type::Enum(name)) => Some(name), + _ => panic!("unknown throw type: {throws:?}"), } } @@ -1112,9 +1055,34 @@ mod test { let err = ComponentInterface::from_webidl(UDL2).unwrap_err(); assert_eq!( err.to_string(), - "Conflicting type definition for `Testing`! \ - existing definition: Enum(\"Testing\"), \ - new definition: Error(\"Testing\")" + "Mismatching definition for enum `Testing`!\nexisting definition: Enum { + name: \"Testing\", + variants: [ + Variant { + name: \"one\", + fields: [], + }, + Variant { + name: \"two\", + fields: [], + }, + ], + flat: true, +}, +new definition: Enum { + name: \"Testing\", + variants: [ + Variant { + name: \"three\", + fields: [], + }, + Variant { + name: \"four\", + fields: [], + }, + ], + flat: true, +}", ); const UDL3: &str = r#" diff --git a/uniffi_bindgen/src/interface/object.rs b/uniffi_bindgen/src/interface/object.rs index c15f1a1b34..aaf1261339 100644 --- a/uniffi_bindgen/src/interface/object.rs +++ b/uniffi_bindgen/src/interface/object.rs @@ -63,11 +63,11 @@ use std::{collections::HashSet, iter}; use anyhow::{bail, Result}; use uniffi_meta::Checksum; -use super::attributes::{Attribute, ConstructorAttributes, InterfaceAttributes, MethodAttributes}; +use super::attributes::{ConstructorAttributes, InterfaceAttributes, MethodAttributes}; use super::ffi::{FfiArgument, FfiFunction, FfiType}; use super::function::{Argument, Callable}; use super::types::{ObjectImpl, Type, TypeIterator}; -use super::{convert_type, APIConverter, AsType, ComponentInterface}; +use super::{APIConverter, AsType, ComponentInterface}; /// An "object" is an opaque type that is passed around by reference, can /// have methods called on it, and so on - basically your classic Object Oriented Programming @@ -287,7 +287,8 @@ impl APIConverter for weedle::InterfaceDefinition<'_> { arguments, return_type, ffi_func: Default::default(), - attributes: Default::default(), + throws: None, + takes_self_by_arc: false, checksum_override: None, }) }; @@ -367,7 +368,7 @@ pub struct Constructor { // avoids a weird circular dependency in the calculation. #[checksum_ignore] pub(super) ffi_func: FfiFunction, - pub(super) attributes: ConstructorAttributes, + pub(super) throws: Option, pub(super) checksum_fn_name: String, // Force a checksum value. This is used for functions from the proc-macro code, which uses a // different checksum method. @@ -402,17 +403,15 @@ impl Constructor { } pub fn throws(&self) -> bool { - self.attributes.get_throws_err().is_some() + self.throws.is_some() } pub fn throws_name(&self) -> Option<&str> { - self.attributes.get_throws_err() + super::throws_name(&self.throws) } - pub fn throws_type(&self) -> Option { - self.attributes - .get_throws_err() - .map(|name| Type::Error(name.to_owned())) + pub fn throws_type(&self) -> Option<&Type> { + self.throws.as_ref() } pub fn is_primary_constructor(&self) -> bool { @@ -459,19 +458,12 @@ impl From for Constructor { name: ffi_name, ..FfiFunction::default() }; - Self { name: meta.name, object_name: meta.self_name, arguments, ffi_func, - attributes: match meta.throws { - None => ConstructorAttributes::from_iter(vec![]), - Some(uniffi_meta::Type::Error { name }) => { - ConstructorAttributes::from_iter(vec![Attribute::Throws(name)]) - } - Some(ty) => panic!("Invalid throws type: {ty:?}"), - }, + throws: meta.throws.map(Into::into), checksum_fn_name, checksum_override: Some(meta.checksum), } @@ -484,6 +476,9 @@ impl APIConverter for weedle::interface::ConstructorInterfaceMember Some(attr) => ConstructorAttributes::try_from(attr)?, None => Default::default(), }; + let throws = attributes + .get_throws_err() + .map(|name| ci.get_type(name).expect("invalid throws type")); Ok(Constructor { name: String::from(attributes.get_name().unwrap_or("new")), // We don't know the name of the containing `Object` at this point, fill it in later. @@ -492,7 +487,7 @@ impl APIConverter for weedle::interface::ConstructorInterfaceMember checksum_fn_name: Default::default(), arguments: self.args.body.list.convert(ci)?, ffi_func: Default::default(), - attributes, + throws, checksum_override: None, }) } @@ -518,7 +513,8 @@ pub struct Method { // avoids a weird circular dependency in the calculation. #[checksum_ignore] pub(super) ffi_func: FfiFunction, - pub(super) attributes: MethodAttributes, + pub(super) throws: Option, + pub(super) takes_self_by_arc: bool, pub(super) checksum_fn_name: String, // Force a checksum value. This is used for functions from the proc-macro code, which uses a // different checksum method. @@ -550,7 +546,7 @@ impl Method { name: self.object_name.clone(), imp: self.object_impl, }, - by_ref: !self.attributes.get_self_by_arc(), + by_ref: !self.takes_self_by_arc, optional: false, default: None, }] @@ -577,21 +573,19 @@ impl Method { } pub fn throws(&self) -> bool { - self.attributes.get_throws_err().is_some() + self.throws.is_some() } pub fn throws_name(&self) -> Option<&str> { - self.attributes.get_throws_err() + super::throws_name(&self.throws) } - pub fn throws_type(&self) -> Option { - self.attributes - .get_throws_err() - .map(|name| Type::Error(name.to_owned())) + pub fn throws_type(&self) -> Option<&Type> { + self.throws.as_ref() } pub fn takes_self_by_arc(&self) -> bool { - self.attributes.get_self_by_arc() + self.takes_self_by_arc } pub fn derive_ffi_func(&mut self, ci_namespace: &str, obj_name: &str) -> Result<()> { @@ -634,7 +628,7 @@ impl From for Method { let ffi_name = meta.ffi_symbol_name(); let checksum_fn_name = meta.checksum_symbol_name(); let is_async = meta.is_async; - let return_type = meta.return_type.map(|out| convert_type(&out)); + let return_type = meta.return_type.map(Into::into); let arguments = meta.inputs.into_iter().map(Into::into).collect(); let ffi_func = FfiFunction { @@ -651,13 +645,8 @@ impl From for Method { arguments, return_type, ffi_func, - attributes: match meta.throws { - None => MethodAttributes::from_iter(vec![]), - Some(uniffi_meta::Type::Error { name }) => { - MethodAttributes::from_iter(vec![Attribute::Throws(name)]) - } - Some(ty) => panic!("Invalid throws type: {ty:?}"), - }, + throws: meta.throws.map(Into::into), + takes_self_by_arc: false, // not yet supported by procmacros? checksum_fn_name, checksum_override: Some(meta.checksum), } @@ -673,6 +662,20 @@ impl APIConverter for weedle::interface::OperationInterfaceMember<'_> { bail!("method modifiers are not supported") } let return_type = ci.resolve_return_type_expression(&self.return_type)?; + let attributes = MethodAttributes::try_from(self.attributes.as_ref())?; + + let throws = match attributes.get_throws_err() { + Some(name) => match ci.get_type(name) { + Some(t) => { + ci.note_name_used_as_error(name); + Some(t) + } + None => bail!("unknown type for error: {name}"), + }, + None => None, + }; + + let takes_self_by_arc = attributes.get_self_by_arc(); Ok(Method { name: match self.identifier { None => bail!("anonymous methods are not supported {:?}", self), @@ -693,7 +696,8 @@ impl APIConverter for weedle::interface::OperationInterfaceMember<'_> { arguments: self.args.body.list.convert(ci)?, return_type, ffi_func: Default::default(), - attributes: MethodAttributes::try_from(self.attributes.as_ref())?, + throws, + takes_self_by_arc, checksum_override: None, }) } @@ -751,7 +755,7 @@ impl Callable for Constructor { } fn throws_type(&self) -> Option { - self.throws_type() + self.throws_type().cloned() } } @@ -765,7 +769,7 @@ impl Callable for Method { } fn throws_type(&self) -> Option { - self.throws_type() + self.throws_type().cloned() } } diff --git a/uniffi_bindgen/src/interface/record.rs b/uniffi_bindgen/src/interface/record.rs index 2d176bfa06..810e4ec4a0 100644 --- a/uniffi_bindgen/src/interface/record.rs +++ b/uniffi_bindgen/src/interface/record.rs @@ -47,11 +47,8 @@ use anyhow::{bail, Result}; use uniffi_meta::Checksum; +use super::literal::{convert_default_value, Literal}; use super::types::{Type, TypeIterator}; -use super::{ - convert_type, - literal::{convert_default_value, Literal}, -}; use super::{APIConverter, AsType, ComponentInterface}; /// Represents a "data class" style object, for passing around complex values. @@ -148,7 +145,7 @@ impl TryFrom for Field { fn try_from(meta: uniffi_meta::FieldMetadata) -> Result { let name = meta.name; - let type_ = convert_type(&meta.ty); + let type_ = meta.ty.into(); let default = meta .default .map(|d| Literal::from_metadata(&name, &type_, d)) diff --git a/uniffi_bindgen/src/interface/types/finder.rs b/uniffi_bindgen/src/interface/types/finder.rs index 6ee94f0abf..243cdf2b6b 100644 --- a/uniffi_bindgen/src/interface/types/finder.rs +++ b/uniffi_bindgen/src/interface/types/finder.rs @@ -21,7 +21,7 @@ use std::convert::TryFrom; use anyhow::{bail, Result}; -use super::super::attributes::{EnumAttributes, InterfaceAttributes, TypedefAttributes}; +use super::super::attributes::{InterfaceAttributes, TypedefAttributes}; use super::{AsType, Type, TypeUniverse}; /// Trait to help with an early "type discovery" phase when processing the UDL. @@ -60,10 +60,8 @@ impl TypeFinder for weedle::InterfaceDefinition<'_> { let name = self.identifier.0.to_string(); let attrs = InterfaceAttributes::try_from(self.attributes.as_ref())?; // Some enum types are defined using an `interface` with a special attribute. - if attrs.contains_enum_attr() { + if attrs.contains_enum_attr() || attrs.contains_error_attr() { types.add_type_definition(self.identifier.0, Type::Enum(name)) - } else if attrs.contains_error_attr() { - types.add_type_definition(self.identifier.0, Type::Error(name)) } else { let obj = crate::interface::Object::new(name, attrs.object_impl()); types.add_type_definition(self.identifier.0, obj.as_type()) @@ -82,11 +80,7 @@ impl TypeFinder for weedle::EnumDefinition<'_> { fn add_type_definitions_to(&self, types: &mut TypeUniverse) -> Result<()> { let name = self.identifier.0.to_string(); // Our error types are defined using an `enum` with a special attribute. - if EnumAttributes::try_from(self.attributes.as_ref())?.contains_error_attr() { - types.add_type_definition(self.identifier.0, Type::Error(name)) - } else { - types.add_type_definition(self.identifier.0, Type::Enum(name)) - } + types.add_type_definition(self.identifier.0, Type::Enum(name)) } } @@ -192,7 +186,7 @@ mod test { matches!(types.get_type_definition("TestItems").unwrap(), Type::Enum(nm) if nm == "TestItems") ); assert!( - matches!(types.get_type_definition("TestError").unwrap(), Type::Error(nm) if nm == "TestError") + matches!(types.get_type_definition("TestError").unwrap(), Type::Enum(nm) if nm == "TestError") ); }, ); diff --git a/uniffi_bindgen/src/interface/types/mod.rs b/uniffi_bindgen/src/interface/types/mod.rs index c249bc97cf..c544079261 100644 --- a/uniffi_bindgen/src/interface/types/mod.rs +++ b/uniffi_bindgen/src/interface/types/mod.rs @@ -94,7 +94,6 @@ pub enum Type { // Types defined in the component API, each of which has a string name. Record(String), Enum(String), - Error(String), CallbackInterface(String), // Structurally recursive types. Optional(Box), @@ -169,7 +168,6 @@ impl From<&Type> for FfiType { Type::ForeignExecutor => FfiType::ForeignExecutorHandle, // Other types are serialized into a bytebuffer and deserialized on the other side. Type::Enum(_) - | Type::Error(_) | Type::Record(_) | Type::Optional(_) | Type::Sequence(_) @@ -220,6 +218,62 @@ where } } +impl From for Type { + fn from(ty: uniffi_meta::Type) -> Self { + use uniffi_meta::Type as Ty; + + match ty { + Ty::U8 => Type::UInt8, + Ty::U16 => Type::UInt16, + Ty::U32 => Type::UInt32, + Ty::U64 => Type::UInt64, + Ty::I8 => Type::Int8, + Ty::I16 => Type::Int16, + Ty::I32 => Type::Int32, + Ty::I64 => Type::Int64, + Ty::F32 => Type::Float32, + Ty::F64 => Type::Float64, + Ty::Bool => Type::Boolean, + Ty::String => Type::String, + Ty::SystemTime => Type::Timestamp, + Ty::Duration => Type::Duration, + Ty::ForeignExecutor => Type::ForeignExecutor, + Ty::Record { name } => Type::Record(name), + Ty::Enum { name, .. } => Type::Enum(name), + Ty::ArcObject { + object_name, + is_trait, + } => Type::Object { + name: object_name, + imp: ObjectImpl::from_is_trait(is_trait), + }, + Ty::CallbackInterface { name } => Type::CallbackInterface(name), + Ty::Custom { name, builtin } => Type::Custom { + name, + builtin: builtin.into(), + }, + Ty::Option { inner_type } => Type::Optional(inner_type.into()), + Ty::Vec { inner_type } => Type::Sequence(inner_type.into()), + Ty::HashMap { + key_type, + value_type, + } => Type::Map(key_type.into(), value_type.into()), + } + } +} + +impl From for Box { + fn from(ty: uniffi_meta::Type) -> Self { + Box::new(ty.into()) + } +} + +impl From> for Box { + fn from(ty: Box) -> Self { + Box::new((*ty).into()) + } +} + /// The set of all possible types used in a particular component interface. /// /// Every component API uses a finite number of types, including primitive types, API-defined @@ -257,9 +311,7 @@ impl TypeUniverse { match self.type_definitions.entry(name.to_string()) { Entry::Occupied(o) => { let existing_def = o.get(); - if type_ == *existing_def - && matches!(type_, Type::Record(_) | Type::Enum(_) | Type::Error(_)) - { + if type_ == *existing_def && matches!(type_, Type::Record(_) | Type::Enum(_)) { // UDL and proc-macro metadata are allowed to define the same record, enum and // error types, if the definitions match (fields and variants are checked in // add_{record,enum,error}_definition) diff --git a/uniffi_bindgen/src/macro_metadata/ci.rs b/uniffi_bindgen/src/macro_metadata/ci.rs index 8b5271fe7e..83d0a0ce36 100644 --- a/uniffi_bindgen/src/macro_metadata/ci.rs +++ b/uniffi_bindgen/src/macro_metadata/ci.rs @@ -2,9 +2,9 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use crate::interface::{ComponentInterface, Enum, Error, Record, Type}; +use crate::interface::{ComponentInterface, Enum, Record, Type}; use anyhow::{bail, Context}; -use uniffi_meta::{group_metadata, Metadata, MetadataGroup}; +use uniffi_meta::{group_metadata, EnumMetadata, ErrorMetadata, Metadata, MetadataGroup}; /// Add Metadata items to the ComponentInterface /// @@ -41,47 +41,9 @@ pub fn add_group_to_ci(iface: &mut ComponentInterface, group: MetadataGroup) -> iface.namespace() ); } - for item in group.items { - match item { - Metadata::Namespace(_) => unreachable!(), - Metadata::UdlFile(_) => (), - Metadata::Func(meta) => { - iface.add_function_definition(meta.into())?; - } - Metadata::Constructor(meta) => { - iface.add_constructor_meta(meta)?; - } - Metadata::Method(meta) => { - iface.add_method_meta(meta)?; - } - Metadata::Record(meta) => { - let ty = Type::Record(meta.name.clone()); - iface.types.add_known_type(&ty)?; - iface.types.add_type_definition(&meta.name, ty)?; - - let record: Record = meta.try_into()?; - iface.add_record_definition(record)?; - } - Metadata::Enum(meta) => { - let ty = Type::Enum(meta.name.clone()); - iface.types.add_known_type(&ty)?; - iface.types.add_type_definition(&meta.name, ty)?; - - let enum_: Enum = meta.try_into()?; - iface.add_enum_definition(enum_)?; - } - Metadata::Object(meta) => { - iface.add_object_free_fn(meta)?; - } - Metadata::Error(meta) => { - let ty = Type::Error(meta.name.clone()); - iface.types.add_known_type(&ty)?; - iface.types.add_type_definition(&meta.name, ty)?; - let error: Error = meta.try_into()?; - iface.add_error_definition(error)?; - } - } + for item in group.items { + add_item_to_ci(iface, item)? } iface @@ -90,6 +52,59 @@ pub fn add_group_to_ci(iface: &mut ComponentInterface, group: MetadataGroup) -> iface .check_consistency() .context("ComponentInterface consistency error")?; + Ok(()) +} + +fn add_enum_to_ci( + iface: &mut ComponentInterface, + meta: EnumMetadata, + is_flat: bool, +) -> anyhow::Result<()> { + let ty = Type::Enum(meta.name.clone()); + iface.types.add_known_type(&ty)?; + iface.types.add_type_definition(&meta.name, ty)?; + + let enum_ = Enum::try_from_meta(meta, is_flat)?; + iface.add_enum_definition(enum_)?; + Ok(()) +} + +fn add_item_to_ci(iface: &mut ComponentInterface, item: Metadata) -> anyhow::Result<()> { + match item { + Metadata::Namespace(_) => unreachable!(), + Metadata::UdlFile(_) => (), + Metadata::Func(meta) => { + iface.add_function_definition(meta.into())?; + } + Metadata::Constructor(meta) => { + iface.add_constructor_meta(meta)?; + } + Metadata::Method(meta) => { + iface.add_method_meta(meta)?; + } + Metadata::Record(meta) => { + let ty = Type::Record(meta.name.clone()); + iface.types.add_known_type(&ty)?; + iface.types.add_type_definition(&meta.name, ty)?; + let record: Record = meta.try_into()?; + iface.add_record_definition(record)?; + } + Metadata::Enum(meta) => { + let flat = meta.variants.iter().all(|v| v.fields.is_empty()); + add_enum_to_ci(iface, meta, flat)?; + } + Metadata::Object(meta) => { + iface.add_object_free_fn(meta)?; + } + Metadata::Error(meta) => { + iface.note_name_used_as_error(meta.name()); + match meta { + ErrorMetadata::Enum { enum_, is_flat } => { + add_enum_to_ci(iface, enum_, is_flat)?; + } + }; + } + } Ok(()) } diff --git a/uniffi_bindgen/src/scaffolding/mod.rs b/uniffi_bindgen/src/scaffolding/mod.rs index d56560d7f2..7818106cca 100644 --- a/uniffi_bindgen/src/scaffolding/mod.rs +++ b/uniffi_bindgen/src/scaffolding/mod.rs @@ -43,7 +43,7 @@ mod filters { Type::Bytes => "Vec".into(), Type::Timestamp => "std::time::SystemTime".into(), Type::Duration => "std::time::Duration".into(), - Type::Enum(name) | Type::Record(name) | Type::Error(name) => format!("r#{name}"), + Type::Enum(name) | Type::Record(name) => format!("r#{name}"), Type::Object { name, imp } => format!("std::sync::Arc<{}>", imp.rust_name_for(name)), Type::CallbackInterface(name) => format!("Box"), Type::ForeignExecutor => "::uniffi::ForeignExecutor".into(), diff --git a/uniffi_bindgen/src/scaffolding/templates/scaffolding_template.rs b/uniffi_bindgen/src/scaffolding/templates/scaffolding_template.rs index 14a4e6a92e..b6a0dfaed8 100644 --- a/uniffi_bindgen/src/scaffolding/templates/scaffolding_template.rs +++ b/uniffi_bindgen/src/scaffolding/templates/scaffolding_template.rs @@ -38,14 +38,14 @@ uniffi::deps::static_assertions::assert_impl_all!({{ k|type_rs }}: ::std::cmp::E {% include "RustBuffer.rs" %} +{% for e in ci.enum_definitions() %} +{% if ci.is_name_used_as_error(e.name()) %} // Error definitions, corresponding to `error` in the UDL. -{% for e in ci.error_definitions() %} {% include "ErrorTemplate.rs" %} -{% endfor %} - +{% else %} // Enum definitions, corresponding to `enum` in UDL. -{% for e in ci.enum_definitions() %} {% include "EnumTemplate.rs" %} +{% endif %} {% endfor %} // Record definitions, implemented as method-less structs, corresponding to `dictionary` objects. diff --git a/uniffi_core/src/lib.rs b/uniffi_core/src/lib.rs index 03a78733c6..048a1bd716 100644 --- a/uniffi_core/src/lib.rs +++ b/uniffi_core/src/lib.rs @@ -295,15 +295,12 @@ pub fn check_remaining(buf: &[u8], num_bytes: usize) -> Result<()> { } /// Helper function to lower an `anyhow::Error` that's wrapping an error type -pub fn lower_anyhow_error_or_panic( - err: anyhow::Error, - arg_name: &str, -) -> >::FfiType +pub fn lower_anyhow_error_or_panic(err: anyhow::Error, arg_name: &str) -> RustBuffer where E: 'static + FfiConverter + Sync + Send + std::fmt::Debug + std::fmt::Display, { match err.downcast::() { - Ok(actual_error) => E::lower(actual_error), + Ok(actual_error) => lower_into_rust_buffer(actual_error), Err(ohno) => panic!("Failed to convert arg '{arg_name}': {ohno}"), } } diff --git a/uniffi_core/src/metadata.rs b/uniffi_core/src/metadata.rs index be4d4c60b4..9fc906e658 100644 --- a/uniffi_core/src/metadata.rs +++ b/uniffi_core/src/metadata.rs @@ -54,7 +54,7 @@ pub mod codes { pub const TYPE_OPTION: u8 = 12; pub const TYPE_RECORD: u8 = 13; pub const TYPE_ENUM: u8 = 14; - pub const TYPE_ERROR: u8 = 15; + // 15 no longer used. pub const TYPE_INTERFACE: u8 = 16; pub const TYPE_VEC: u8 = 17; pub const TYPE_HASH_MAP: u8 = 18; diff --git a/uniffi_macros/src/enum_.rs b/uniffi_macros/src/enum_.rs index 19c836c4b0..62b0dc74cd 100644 --- a/uniffi_macros/src/enum_.rs +++ b/uniffi_macros/src/enum_.rs @@ -68,7 +68,7 @@ pub(crate) fn rich_error_ffi_converter_impl( ident, enum_, tag, - quote! { ::uniffi::metadata::codes::TYPE_ERROR }, + quote! { ::uniffi::metadata::codes::TYPE_ENUM }, ) } diff --git a/uniffi_macros/src/error.rs b/uniffi_macros/src/error.rs index 6ad4f47819..2b8cb1c1f7 100644 --- a/uniffi_macros/src/error.rs +++ b/uniffi_macros/src/error.rs @@ -142,7 +142,7 @@ fn flat_error_ffi_converter_impl( #try_read_impl } - const TYPE_ID_META: ::uniffi::MetadataBuffer = ::uniffi::MetadataBuffer::from_code(::uniffi::metadata::codes::TYPE_ERROR) + const TYPE_ID_META: ::uniffi::MetadataBuffer = ::uniffi::MetadataBuffer::from_code(::uniffi::metadata::codes::TYPE_ENUM) .concat_str(#name); } } @@ -155,15 +155,16 @@ pub(crate) fn error_meta_static_var( ) -> syn::Result { let name = ident_to_string(ident); let module_path = mod_path()?; - let flat_code = u8::from(flat); let mut metadata_expr = quote! { ::uniffi::MetadataBuffer::from_code(::uniffi::metadata::codes::ERROR) + // first our is-flat flag + .concat_bool(#flat) + // followed by an enum .concat_str(#module_path) .concat_str(#name) - .concat_value(#flat_code) }; if flat { - metadata_expr.extend(flat_error_variant_metadata(enum_)?); + metadata_expr.extend(flat_error_variant_metadata(enum_)?) } else { metadata_expr.extend(variant_metadata(enum_)?); } diff --git a/uniffi_meta/src/group.rs b/uniffi_meta/src/group.rs index c45aac4842..b019c2eda3 100644 --- a/uniffi_meta/src/group.rs +++ b/uniffi_meta/src/group.rs @@ -39,7 +39,7 @@ pub fn group_metadata(items: Vec) -> Result> { Metadata::Record(meta) => (format!("record `{}`", meta.name), &meta.module_path), Metadata::Enum(meta) => (format!("enum `{}`", meta.name), &meta.module_path), Metadata::Object(meta) => (format!("object `{}`", meta.name), &meta.module_path), - Metadata::Error(meta) => (format!("error `{}`", meta.name), &meta.module_path), + Metadata::Error(meta) => (format!("error `{}`", meta.name()), meta.module_path()), }; let crate_name = module_path.split("::").next().unwrap(); diff --git a/uniffi_meta/src/lib.rs b/uniffi_meta/src/lib.rs index 3fc812d28f..d64904e9b8 100644 --- a/uniffi_meta/src/lib.rs +++ b/uniffi_meta/src/lib.rs @@ -220,9 +220,6 @@ pub enum Type { object_name: String, is_trait: bool, }, - Error { - name: String, - }, CallbackInterface { name: String, }, @@ -295,11 +292,22 @@ impl ObjectMetadata { } #[derive(Clone, Debug, PartialEq, Eq, Serialize)] -pub struct ErrorMetadata { - pub module_path: String, - pub name: String, - pub variants: Vec, - pub flat: bool, +pub enum ErrorMetadata { + Enum { enum_: EnumMetadata, is_flat: bool }, +} + +impl ErrorMetadata { + pub fn name(&self) -> &String { + match self { + Self::Enum { enum_, .. } => &enum_.name, + } + } + + pub fn module_path(&self) -> &String { + match self { + Self::Enum { enum_, .. } => &enum_.module_path, + } + } } /// Returns the last 16 bits of the value's hash as computed with [`SipHasher13`]. @@ -374,14 +382,14 @@ impl From for Metadata { } } -impl From for Metadata { - fn from(v: ObjectMetadata) -> Self { - Self::Object(v) +impl From for Metadata { + fn from(e: ErrorMetadata) -> Self { + Self::Error(e) } } -impl From for Metadata { - fn from(v: ErrorMetadata) -> Self { - Self::Error(v) +impl From for Metadata { + fn from(v: ObjectMetadata) -> Self { + Self::Object(v) } } diff --git a/uniffi_meta/src/reader.rs b/uniffi_meta/src/reader.rs index b69886f8c3..9657e5372a 100644 --- a/uniffi_meta/src/reader.rs +++ b/uniffi_meta/src/reader.rs @@ -51,7 +51,7 @@ impl<'a> MetadataReader<'a> { codes::CONSTRUCTOR => self.read_constructor()?.into(), codes::METHOD => self.read_method()?.into(), codes::RECORD => self.read_record()?.into(), - codes::ENUM => self.read_enum()?.into(), + codes::ENUM => self.read_enum(false)?.into(), codes::ERROR => self.read_error()?.into(), codes::INTERFACE => self.read_object()?.into(), _ => bail!("Unexpected metadata code: {value:?}"), @@ -111,9 +111,6 @@ impl<'a> MetadataReader<'a> { codes::TYPE_ENUM => Type::Enum { name: self.read_string()?, }, - codes::TYPE_ERROR => Type::Error { - name: self.read_string()?, - }, codes::TYPE_INTERFACE => Type::ArcObject { object_name: self.read_string()?, is_trait: self.read_bool()?, @@ -237,30 +234,28 @@ impl<'a> MetadataReader<'a> { }) } - fn read_enum(&mut self) -> Result { - Ok(EnumMetadata { - module_path: self.read_string()?, - name: self.read_string()?, - variants: self.read_variants()?, - }) - } - - fn read_error(&mut self) -> Result { + fn read_enum(&mut self, is_flat_error: bool) -> Result { let module_path = self.read_string()?; let name = self.read_string()?; - let flat = self.read_bool()?; - Ok(ErrorMetadata { + let variants = if is_flat_error { + self.read_flat_variants()? + } else { + self.read_variants()? + }; + + Ok(EnumMetadata { module_path, name, - flat, - variants: if flat { - self.read_flat_variants()? - } else { - self.read_variants()? - }, + variants, }) } + fn read_error(&mut self) -> Result { + let is_flat = self.read_bool()?; + let enum_ = self.read_enum(is_flat)?; + Ok(ErrorMetadata::Enum { enum_, is_flat }) + } + fn read_object(&mut self) -> Result { Ok(ObjectMetadata { module_path: self.read_string()?,