diff --git a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs index 7e5ae84620..3589cb9185 100644 --- a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs +++ b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs @@ -265,7 +265,14 @@ impl KotlinCodeOracle { } } - fn ffi_type_label(&self, ffi_type: &FfiType) -> String { + fn ffi_type_label_by_value(ffi_type: &FfiType) -> String { + match ffi_type { + FfiType::RustBuffer(_) => format!("{}.ByValue", Self::ffi_type_label(ffi_type)), + _ => Self::ffi_type_label(ffi_type), + } + } + + fn ffi_type_label(ffi_type: &FfiType) -> String { match ffi_type { // Note that unsigned integers in Kotlin are currently experimental, but java.nio.ByteBuffer does not // support them yet. Thus, we use the signed variants to represent both signed and unsigned @@ -277,16 +284,15 @@ impl KotlinCodeOracle { FfiType::Float32 => "Float".to_string(), FfiType::Float64 => "Double".to_string(), FfiType::RustArcPtr(_) => "Pointer".to_string(), - FfiType::RustBuffer(maybe_suffix) => match maybe_suffix { - Some(suffix) => format!("RustBuffer{suffix}.ByValue"), - None => "RustBuffer.ByValue".to_string(), - }, + FfiType::RustBuffer(maybe_suffix) => { + format!("RustBuffer{}", maybe_suffix.as_deref().unwrap_or_default()) + } FfiType::ForeignBytes => "ForeignBytes.ByValue".to_string(), FfiType::ForeignCallback => "ForeignCallback".to_string(), FfiType::ForeignExecutorHandle => "USize".to_string(), FfiType::ForeignExecutorCallback => "UniFfiForeignExecutorCallback".to_string(), FfiType::FutureCallback { return_type } => { - format!("UniFfiFutureCallback{}", return_type.canonical_name()) + format!("UniFfiFutureCallback{}", Self::ffi_type_label(return_type)) } FfiType::FutureCallbackData => "USize".to_string(), } @@ -394,11 +400,13 @@ pub mod filters { pub fn future_callback_handler(result_type: &ResultType) -> Result { let return_component = match &result_type.return_type { - Some(return_type) => return_type.canonical_name(), + Some(return_type) => KotlinCodeOracle.find(return_type).canonical_name(), None => "Void".into(), }; let throws_component = match &result_type.throws_type { - Some(throws_type) => format!("_{}", throws_type.canonical_name()), + Some(throws_type) => { + format!("_{}", KotlinCodeOracle.find(throws_type).canonical_name()) + } None => "".into(), }; Ok(format!( @@ -423,7 +431,25 @@ pub mod filters { /// Get the Kotlin syntax for representing a given low-level `FfiType`. pub fn ffi_type_name(type_: &FfiType) -> Result { - Ok(KotlinCodeOracle.ffi_type_label(type_)) + Ok(KotlinCodeOracle::ffi_type_label(type_)) + } + + pub fn ffi_type_name_by_value(type_: &FfiType) -> Result { + Ok(KotlinCodeOracle::ffi_type_label_by_value(type_)) + } + + // Some FfiTypes have the same ffi_type_label - this makes a vec of them unique. + pub fn unique_ffi_types( + types: impl Iterator, + ) -> Result, askama::Error> { + let mut seen = HashSet::new(); + let mut result = Vec::new(); + for t in types { + if seen.insert(KotlinCodeOracle::ffi_type_label(&t)) { + result.push(t) + } + } + Ok(result.into_iter()) } /// Get the idiomatic Kotlin rendering of a function name. diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/AsyncTypes.kt b/uniffi_bindgen/src/bindings/kotlin/templates/AsyncTypes.kt index e388e7e5ee..a22d53c3f4 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/AsyncTypes.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/AsyncTypes.kt @@ -11,11 +11,11 @@ // FFI type for callback handlers -{%- for callback_param in ci.iter_future_callback_params() %} -internal interface UniFfiFutureCallback{{ callback_param.canonical_name() }} : com.sun.jna.Callback { +{%- for callback_param in ci.iter_future_callback_params()|unique_ffi_types %} +internal interface UniFfiFutureCallback{{ callback_param|ffi_type_name }} : com.sun.jna.Callback { // Note: callbackData is always 0. We could pass Rust a pointer/usize to represent the // continuation, but with JNA it's easier to just store it in the callback handler. - fun invoke(_callbackData: USize, returnValue: {{ callback_param|ffi_type_name }}, callStatus: RustCallStatus.ByValue); + fun invoke(_callbackData: USize, returnValue: {{ callback_param|ffi_type_name_by_value }}, callStatus: RustCallStatus.ByValue); } {%- endfor %} @@ -25,8 +25,8 @@ internal interface UniFfiFutureCallback{{ callback_param.canonical_name() }} : c {%- let callback_param = result_type.future_callback_param() %} internal class {{ result_type|future_callback_handler }}(val continuation: {{ result_type|future_continuation_type }}) - : UniFfiFutureCallback{{ callback_param.canonical_name() }} { - override fun invoke(_callbackData: USize, returnValue: {{ callback_param|ffi_type_name }}, callStatus: RustCallStatus.ByValue) { + : UniFfiFutureCallback{{ callback_param|ffi_type_name }} { + override fun invoke(_callbackData: USize, returnValue: {{ callback_param|ffi_type_name_by_value }}, callStatus: RustCallStatus.ByValue) { try { checkCallStatus({{ result_type|error_handler }}, callStatus) {%- match result_type.return_type %} diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/CustomTypeTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/CustomTypeTemplate.kt index c7807f2f33..1fcc62956a 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/CustomTypeTemplate.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/CustomTypeTemplate.kt @@ -11,7 +11,7 @@ public typealias {{ ffi_converter_name }} = {{ builtin|ffi_converter_name }} {%- when Some with (config) %} -{%- let ffi_type_name=builtin.ffi_type().borrow()|ffi_type_name %} +{%- let ffi_type_name=builtin.ffi_type().borrow()|ffi_type_name_by_value %} {# When the config specifies a different type name, create a typealias for it #} {%- match config.type_name %} diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/NamespaceLibraryTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/NamespaceLibraryTemplate.kt index c8fcde3e34..6a3aeada35 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/NamespaceLibraryTemplate.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/NamespaceLibraryTemplate.kt @@ -33,7 +33,7 @@ internal interface _UniFFILib : Library { {% for func in ci.iter_ffi_function_definitions() -%} fun {{ func.name() }}( {%- call kt::arg_list_ffi_decl(func) %} - ): {% match func.return_type() %}{% when Some with (return_type) %}{{ return_type.borrow()|ffi_type_name }}{% when None %}Unit{% endmatch %} + ): {% match func.return_type() %}{% when Some with (return_type) %}{{ return_type.borrow()|ffi_type_name_by_value }}{% when None %}Unit{% endmatch %} {% endfor %} } diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/macros.kt b/uniffi_bindgen/src/bindings/kotlin/templates/macros.kt index 7e373c8c59..0c6d1f2616 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/macros.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/macros.kt @@ -63,7 +63,7 @@ -#} {%- macro arg_list_ffi_decl(func) %} {%- for arg in func.arguments() %} - {{- arg.name()|var_name }}: {{ arg.type_().borrow()|ffi_type_name -}}, + {{- arg.name()|var_name }}: {{ arg.type_().borrow()|ffi_type_name_by_value -}}, {%- endfor %} {%- if func.has_rust_call_status_arg() %}_uniffi_out_err: RustCallStatus, {% endif %} {%- endmacro -%} @@ -75,9 +75,3 @@ this.{{ field.name()|var_name }}{%- if !loop.last %}, {% endif -%} {% endfor -%}) {%- endmacro -%} - -{%- macro ffi_function_definition(func) %} -fun {{ func.name()|fn_name }}( - {%- call arg_list_ffi_decl(func) %} -){%- match func.return_type() -%}{%- when Some with (type_) %}: {{ type_|ffi_type_name }}{% when None %}: Unit{% endmatch %} -{% endmacro %} diff --git a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs index cccbc8b5dd..6ee702ba61 100644 --- a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs +++ b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs @@ -405,11 +405,11 @@ pub mod filters { // Name of the callback function we pass to Rust to complete an async call pub fn async_callback_fn(result_type: &ResultType) -> Result { let return_string = match &result_type.return_type { - Some(t) => t.canonical_name().to_snake_case(), + Some(t) => PythonCodeOracle.find(t).canonical_name().to_snake_case(), None => "void".into(), }; let throws_string = match &result_type.throws_type { - Some(t) => t.canonical_name().to_snake_case(), + Some(t) => PythonCodeOracle.find(t).canonical_name().to_snake_case(), None => "void".into(), }; Ok(format!( diff --git a/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs b/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs index da8f9c7b45..523a4b8a95 100644 --- a/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs +++ b/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs @@ -23,6 +23,61 @@ fn is_reserved_word(word: &str) -> bool { RESERVED_WORDS.contains(&word) } +impl Type { + /// Get the canonical, unique-within-this-component name for a type. + /// + /// When generating helper code for foreign language bindings, it's sometimes useful to be + /// able to name a particular type in order to e.g. call a helper function that is specific + /// to that type. We support this by defining a naming convention where each type gets a + /// unique canonical name, constructed recursively from the names of its component types (if any). + pub fn canonical_name(&self) -> String { + match self { + // Builtin primitive types, with plain old names. + Type::Int8 => "i8".into(), + Type::UInt8 => "u8".into(), + Type::Int16 => "i16".into(), + Type::UInt16 => "u16".into(), + Type::Int32 => "i32".into(), + Type::UInt32 => "u32".into(), + Type::Int64 => "i64".into(), + Type::UInt64 => "u64".into(), + Type::Float32 => "f32".into(), + Type::Float64 => "f64".into(), + Type::String => "string".into(), + Type::Bytes => "bytes".into(), + Type::Boolean => "bool".into(), + // API defined types. + // Note that these all get unique names, and the parser ensures that the names do not + // conflict with a builtin type. We add a prefix to the name to guard against pathological + // cases like a record named `SequenceRecord` interfering with `sequence`. + // 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}"), + Type::Timestamp => "Timestamp".into(), + Type::Duration => "Duration".into(), + Type::ForeignExecutor => "ForeignExecutor".into(), + // Recursive types. + // These add a prefix to the name of the underlying type. + // The component API definition cannot give names to recursive types, so as long as the + // prefixes we add here are all unique amongst themselves, then we have no chance of + // acccidentally generating name collisions. + Type::Optional(t) => format!("Optional{}", t.canonical_name()), + Type::Sequence(t) => format!("Sequence{}", t.canonical_name()), + Type::Map(k, v) => format!( + "Map{}{}", + k.canonical_name().to_upper_camel_case(), + v.canonical_name().to_upper_camel_case() + ), + // A type that exists externally. + Type::External { name, .. } | Type::Custom { name, .. } => format!("Type{name}"), + } + } +} + // Some config options for it the caller wants to customize the generated ruby. // Note that this can only be used to control details of the ruby *that do not affect the underlying component*, // since the details of the underlying component are entirely determined by the `ComponentInterface`. @@ -274,5 +329,26 @@ mod filters { } } +#[cfg(test)] +mod test_type { + use super::*; + + #[test] + fn test_canonical_names() { + // Non-exhaustive, but gives a bit of a flavour of what we want. + assert_eq!(Type::UInt8.canonical_name(), "u8"); + assert_eq!(Type::String.canonical_name(), "string"); + assert_eq!(Type::Bytes.canonical_name(), "bytes"); + assert_eq!( + Type::Optional(Box::new(Type::Sequence(Box::new(Type::Object { + name: "Example".into(), + imp: ObjectImpl::Struct, + })))) + .canonical_name(), + "OptionalSequenceTypeExample" + ); + } +} + #[cfg(test)] mod tests; diff --git a/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs b/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs index fdf4c9af51..3d3fcaca42 100644 --- a/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs +++ b/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs @@ -343,7 +343,7 @@ impl SwiftCodeOracle { format!("`{}`", nm.to_string().to_lower_camel_case()) } - fn ffi_type_label(&self, ffi_type: &FfiType) -> String { + fn ffi_type_label_raw(&self, ffi_type: &FfiType) -> String { match ffi_type { FfiType::Int8 => "Int8".into(), FfiType::UInt8 => "UInt8".into(), @@ -358,16 +358,30 @@ impl SwiftCodeOracle { FfiType::RustArcPtr(_) => "UnsafeMutableRawPointer".into(), FfiType::RustBuffer(_) => "RustBuffer".into(), FfiType::ForeignBytes => "ForeignBytes".into(), - FfiType::ForeignCallback => "ForeignCallback _Nonnull".into(), + FfiType::ForeignCallback => "ForeignCallback".into(), FfiType::ForeignExecutorHandle => "Int".into(), - FfiType::ForeignExecutorCallback => "ForeignExecutorCallback _Nonnull".into(), - FfiType::FutureCallback { return_type } => format!( - "UniFfiFutureCallback{} _Nonnull", - return_type.canonical_name() - ), + FfiType::ForeignExecutorCallback => "ForeignExecutorCallback".into(), + FfiType::FutureCallback { return_type } => { + format!("UniFfiFutureCallback{}", self.ffi_type_label(return_type)) + } FfiType::FutureCallbackData => "UnsafeMutableRawPointer".into(), } } + + fn ffi_type_label(&self, ffi_type: &FfiType) -> String { + match ffi_type { + FfiType::ForeignCallback + | FfiType::ForeignExecutorCallback + | FfiType::FutureCallback { .. } => { + format!("{} _Nonnull", self.ffi_type_label_raw(ffi_type)) + } + _ => self.ffi_type_label_raw(ffi_type), + } + } + + fn ffi_canonical_name(&self, ffi_type: &FfiType) -> String { + self.ffi_type_label_raw(ffi_type) + } } pub mod filters { @@ -417,6 +431,10 @@ pub mod filters { Ok(oracle().ffi_type_label(ffi_type)) } + pub fn ffi_canonical_name(ffi_type: &FfiType) -> Result { + Ok(oracle().ffi_canonical_name(ffi_type)) + } + /// Like `ffi_type_name`, but used in `BridgingHeaderTemplate.h` which uses a slightly different /// names. pub fn header_ffi_type_name(ffi_type: &FfiType) -> Result { @@ -439,7 +457,7 @@ pub mod filters { FfiType::ForeignExecutorHandle => "size_t".into(), FfiType::FutureCallback { return_type } => format!( "UniFfiFutureCallback{} _Nonnull", - return_type.canonical_name() + SwiftCodeOracle.ffi_type_label_raw(return_type) ), FfiType::FutureCallbackData => "void* _Nonnull".into(), }) @@ -477,11 +495,11 @@ pub mod filters { Ok(format!( "uniffiFutureCallbackHandler{}{}", match &result.return_type { - Some(t) => t.canonical_name(), + Some(t) => SwiftCodeOracle.find(t).canonical_name(), None => "Void".into(), }, match &result.throws_type { - Some(t) => t.canonical_name(), + Some(t) => SwiftCodeOracle.find(t).canonical_name(), None => "".into(), } )) diff --git a/uniffi_bindgen/src/bindings/swift/templates/BridgingHeaderTemplate.h b/uniffi_bindgen/src/bindings/swift/templates/BridgingHeaderTemplate.h index 5f0e2ee234..5f7e728202 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/BridgingHeaderTemplate.h +++ b/uniffi_bindgen/src/bindings/swift/templates/BridgingHeaderTemplate.h @@ -61,7 +61,7 @@ typedef struct RustCallStatus { // Callbacks for UniFFI Futures {%- for ffi_type in ci.iter_future_callback_params() %} -typedef void (*UniFfiFutureCallback{{ ffi_type.canonical_name() }})(const void * _Nonnull, {{ ffi_type|header_ffi_type_name }}, RustCallStatus); +typedef void (*UniFfiFutureCallback{{ ffi_type|ffi_canonical_name }})(const void * _Nonnull, {{ ffi_type|header_ffi_type_name }}, RustCallStatus); {%- endfor %} // Scaffolding functions diff --git a/uniffi_bindgen/src/interface/ffi.rs b/uniffi_bindgen/src/interface/ffi.rs index 19f64b92f8..dcbae68355 100644 --- a/uniffi_bindgen/src/interface/ffi.rs +++ b/uniffi_bindgen/src/interface/ffi.rs @@ -64,35 +64,6 @@ pub enum FfiType { // We don't need that yet and it's possible we never will, so it isn't here for now. } -impl FfiType { - pub fn canonical_name(&self) -> String { - match self { - Self::UInt8 => "UInt8".into(), - Self::Int8 => "Int8".into(), - Self::UInt16 => "UInt16".into(), - Self::Int16 => "Int16".into(), - Self::UInt32 => "UInt32".into(), - Self::Int32 => "Int32".into(), - Self::UInt64 => "UInt64".into(), - Self::Int64 => "Int64".into(), - Self::Float32 => "Float32".into(), - Self::Float64 => "Float64".into(), - Self::RustArcPtr(name) => format!("RustArcPtr{name}"), - Self::RustBuffer(maybe_suffix) => { - format!("RustBuffer{}", maybe_suffix.as_deref().unwrap_or_default()) - } - Self::ForeignBytes => "ForeignBytes".into(), - Self::ForeignCallback => "ForeignCallback".into(), - Self::ForeignExecutorHandle => "ForeignExecutorHandle".into(), - Self::ForeignExecutorCallback => "ForeignExecutorCallback".into(), - Self::FutureCallback { return_type } => { - format!("FutureCallback{}", return_type.canonical_name()) - } - Self::FutureCallbackData => "FutureCallbackData".into(), - } - } -} - /// Represents an "extern C"-style function that will be part of the FFI. /// /// These can't be declared explicitly in the UDL, but rather, are derived automatically diff --git a/uniffi_bindgen/src/interface/types/mod.rs b/uniffi_bindgen/src/interface/types/mod.rs index 52f4cfd6d9..c249bc97cf 100644 --- a/uniffi_bindgen/src/interface/types/mod.rs +++ b/uniffi_bindgen/src/interface/types/mod.rs @@ -24,7 +24,6 @@ use std::{collections::hash_map::Entry, collections::BTreeSet, collections::HashMap, iter}; use anyhow::{bail, Result}; -use heck::ToUpperCamelCase; use uniffi_meta::Checksum; use super::ffi::FfiType; @@ -122,59 +121,6 @@ pub enum ExternalKind { } impl Type { - /// Get the canonical, unique-within-this-component name for a type. - /// - /// When generating helper code for foreign language bindings, it's sometimes useful to be - /// able to name a particular type in order to e.g. call a helper function that is specific - /// to that type. We support this by defining a naming convention where each type gets a - /// unique canonical name, constructed recursively from the names of its component types (if any). - pub fn canonical_name(&self) -> String { - match self { - // Builtin primitive types, with plain old names. - Type::Int8 => "i8".into(), - Type::UInt8 => "u8".into(), - Type::Int16 => "i16".into(), - Type::UInt16 => "u16".into(), - Type::Int32 => "i32".into(), - Type::UInt32 => "u32".into(), - Type::Int64 => "i64".into(), - Type::UInt64 => "u64".into(), - Type::Float32 => "f32".into(), - Type::Float64 => "f64".into(), - Type::String => "string".into(), - Type::Bytes => "bytes".into(), - Type::Boolean => "bool".into(), - // API defined types. - // Note that these all get unique names, and the parser ensures that the names do not - // conflict with a builtin type. We add a prefix to the name to guard against pathological - // cases like a record named `SequenceRecord` interfering with `sequence`. - // 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}"), - Type::Timestamp => "Timestamp".into(), - Type::Duration => "Duration".into(), - Type::ForeignExecutor => "ForeignExecutor".into(), - // Recursive types. - // These add a prefix to the name of the underlying type. - // The component API definition cannot give names to recursive types, so as long as the - // prefixes we add here are all unique amongst themselves, then we have no chance of - // acccidentally generating name collisions. - Type::Optional(t) => format!("Optional{}", t.canonical_name()), - Type::Sequence(t) => format!("Sequence{}", t.canonical_name()), - Type::Map(k, v) => format!( - "Map{}{}", - k.canonical_name().to_upper_camel_case(), - v.canonical_name().to_upper_camel_case() - ), - // A type that exists externally. - Type::External { name, .. } | Type::Custom { name, .. } => format!("Type{name}"), - } - } - pub fn ffi_type(&self) -> FfiType { self.into() } @@ -305,10 +251,7 @@ impl TypeUniverse { /// This will fail if you try to add a name for which an existing type definition exists. pub fn add_type_definition(&mut self, name: &str, type_: Type) -> Result<()> { if resolve_builtin_type(name).is_some() { - bail!( - "please don't shadow builtin types ({name}, {})", - type_.canonical_name(), - ); + bail!("please don't shadow builtin types ({name}, {:?})", type_,); } self.add_known_type(&type_)?; match self.type_definitions.entry(name.to_string()) { @@ -390,27 +333,6 @@ impl TypeUniverse { /// use an `impl Iterator` on any method that yields types. pub type TypeIterator<'a> = Box + 'a>; -#[cfg(test)] -mod test_type { - use super::*; - - #[test] - fn test_canonical_names() { - // Non-exhaustive, but gives a bit of a flavour of what we want. - assert_eq!(Type::UInt8.canonical_name(), "u8"); - assert_eq!(Type::String.canonical_name(), "string"); - assert_eq!(Type::Bytes.canonical_name(), "bytes"); - assert_eq!( - Type::Optional(Box::new(Type::Sequence(Box::new(Type::Object { - name: "Example".into(), - imp: ObjectImpl::Struct, - })))) - .canonical_name(), - "OptionalSequenceTypeExample" - ); - } -} - #[cfg(test)] mod test_type_universe { // All the useful functionality of the `TypeUniverse` struct