Kotlin classes can inherit from traits.

Builds on #2204 which landed the metadata and Python support.

Also fixes minor issues with `CallbackInterface` types.

CHANGELOG.md

@@ -17,6 +17,11 @@
 
 - Fixed bug in metadata extraction with large ELF files.
 
+### What's new?
+
+- Kotlin: Proc-macros exporting an `impl Trait for Struct` block now has a class inheritance
+  hierarcy to reflect that. [#2297](https://github.com/mozilla/uniffi-rs/pull/2297)
+
 [All changes in [[UnreleasedUniFFIVersion]]](https://github.com/mozilla/uniffi-rs/compare/v0.28.2...HEAD).
 
 ### What's changed?

fixtures/coverall/tests/bindings/test_coverall.kts

@@ -415,6 +415,17 @@ getTraits().let { traits ->
     // not possible through the `NodeTrait` interface (see #1787).
 }
 
+// Structs which implement traits.
+Node("node").let { n ->
+    assert(n.describeParent() == "Some(Node { name: Some(\"via node\"), parent: Mutex { data: None, poisoned: false, .. } })")
+    // NOTE same re-wrap problem described in the Python tests.
+    n.setParent(n.getParent())
+    // Expect: as above
+    // Get: `Some(UniFFICallbackHandlerNodeTrait { handle: 18 })`
+    // assert(n.describeParent() == "Some(Node { name: Some(\"via node\"), parent: Mutex { data: None, poisoned: false, .. } })")
+    n.setParent(Node("parent"))
+}
+
 makeRustGetters().let { rustGetters ->
     // Check that these don't cause use-after-free bugs
     testRoundTripThroughRust(rustGetters)

fixtures/keywords/kotlin/src/keywords.udl

@@ -15,6 +15,10 @@ callback interface continue {
     record<u8, sequence<class>>? class(record<u8, sequence<class>> v);
 };
 
+callback interface true {
+    void false();
+};
+
 dictionary return {
     u8 class;
     u8? object;

fixtures/keywords/kotlin/src/lib.rs

@@ -33,8 +33,17 @@ trait r#continue {
     }
 }
 
+#[allow(non_camel_case_types)]
+trait r#true {
+    fn r#false(&self);
+}
+
 #[uniffi::export]
-impl r#continue for r#break {}
+impl r#true for r#break {
+    fn r#false(&self) {
+        unimplemented!()
+    }
+}
 
 #[allow(non_camel_case_types)]
 pub struct r#return {

uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs

@@ -303,6 +303,55 @@ impl<'a> KotlinWrapper<'a> {
     }
 }
 
+/// Get the name of the interface and class name for a trait.
+///
+/// For a regular `struct Foo` or `trait Foo`, there's `FooInterface` with `Foo` as
+/// the name of the (Rust implemented) object. But if it's a foreign trait:
+/// * The name `Foo` is the name of the interface used by a the Kotlin implementation of the trait.
+/// * The Rust implemented object is `FooImpl`.
+///
+/// This all impacts what types `FfiConverter.lower()` inputs.  If it's a "foreign trait"
+/// `lower` must lower anything that implements the interface (ie, a kotlin implementation).
+/// If not, then lower only lowers the concrete class (ie, our simple instance with the pointer).
+fn object_interface_name(ci: &ComponentInterface, obj: &Object) -> String {
+    let class_name = KotlinCodeOracle.class_name(ci, obj.name());
+    if obj.has_callback_interface() {
+        class_name
+    } else {
+        format!("{class_name}Interface")
+    }
+}
+
+// *sigh* - same thing for a trait, which might be either Object or CallbackInterface.
+// (we should either fold it into object or kill it!)
+fn trait_interface_name(ci: &ComponentInterface, name: &str) -> String {
+    let (obj_name, has_callback_interface) = match ci.get_object_definition(name) {
+        Some(obj) => (obj.name(), obj.has_callback_interface()),
+        None => (
+            ci.get_callback_interface_definition(name)
+                .unwrap_or_else(|| panic!("no interface {}", name))
+                .name(),
+            true,
+        ),
+    };
+    let class_name = KotlinCodeOracle.class_name(ci, obj_name);
+    if has_callback_interface {
+        class_name
+    } else {
+        format!("{class_name}Interface")
+    }
+}
+
+// The name of the object exposing a Rust implementation.
+fn object_impl_name(ci: &ComponentInterface, obj: &Object) -> String {
+    let class_name = KotlinCodeOracle.class_name(ci, obj.name());
+    if obj.has_callback_interface() {
+        format!("{class_name}Impl")
+    } else {
+        class_name
+    }
+}
+
 #[derive(Clone)]
 pub struct KotlinCodeOracle;
 
@@ -443,24 +492,6 @@ impl KotlinCodeOracle {
             FfiType::VoidPointer => "Pointer".to_string(),
         }
     }
-
-    /// Get the name of the interface and class name for an object.
-    ///
-    /// If we support callback interfaces, the interface name is the object name, and the class name is derived from that.
-    /// Otherwise, the class name is the object name and the interface name is derived from that.
-    ///
-    /// This split determines what types `FfiConverter.lower()` inputs.  If we support callback
-    /// interfaces, `lower` must lower anything that implements the interface.  If not, then lower
-    /// only lowers the concrete class.
-    fn object_names(&self, ci: &ComponentInterface, obj: &Object) -> (String, String) {
-        let class_name = self.class_name(ci, obj.name());
-        if obj.has_callback_interface() {
-            let impl_name = format!("{class_name}Impl");
-            (class_name, impl_name)
-        } else {
-            (format!("{class_name}Interface"), class_name)
-        }
-    }
 }
 
 trait AsCodeType {
@@ -659,13 +690,6 @@ mod filters {
         Ok(KotlinCodeOracle.ffi_struct_name(nm.as_ref()))
     }
 
-    pub fn object_names(
-        obj: &Object,
-        ci: &ComponentInterface,
-    ) -> Result<(String, String), rinja::Error> {
-        Ok(KotlinCodeOracle.object_names(ci, obj))
-    }
-
     pub fn async_poll(
         callable: impl Callable,
         ci: &ComponentInterface,

uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt

@@ -101,7 +101,8 @@
 {%- endif %}
 
 {%- let obj = ci|get_object_definition(name) %}
-{%- let (interface_name, impl_class_name) = obj|object_names(ci) %}
+{%- let interface_name = self::object_interface_name(ci, obj) %}
+{%- let impl_class_name = self::object_impl_name(ci, obj) %}
 {%- let methods = obj.methods() %}
 {%- let interface_docstring = obj.docstring() %}
 {%- let is_error = ci.is_name_used_as_error(name) %}
@@ -113,7 +114,11 @@
 {% if (is_error) %}
 open class {{ impl_class_name }} : kotlin.Exception, Disposable, AutoCloseable, {{ interface_name }} {
 {% else -%}
-open class {{ impl_class_name }}: Disposable, AutoCloseable, {{ interface_name }} {
+open class {{ impl_class_name }}: Disposable, AutoCloseable, {{ interface_name }}
+{%- for t in obj.trait_impls() %}
+, {{ self::trait_interface_name(ci, t.trait_name) }}
+{% endfor %}
+{
 {%- endif %}
 
     constructor(pointer: Pointer) {

uniffi_bindgen/src/interface/mod.rs

@@ -921,6 +921,7 @@ impl ComponentInterface {
 
     /// Called by `APIBuilder` impls to add a newly-parsed object definition to the `ComponentInterface`.
     fn add_object_definition(&mut self, defn: Object) -> Result<()> {
+        self.types.add_known_type(&defn.as_type())?;
         self.types.add_known_types(defn.iter_types())?;
         self.objects.push(defn);
         Ok(())
@@ -931,8 +932,14 @@ impl ComponentInterface {
     }
 
     /// Called by `APIBuilder` impls to add a newly-parsed callback interface definition to the `ComponentInterface`.
-    pub(super) fn add_callback_interface_definition(&mut self, defn: CallbackInterface) {
+    pub(super) fn add_callback_interface_definition(
+        &mut self,
+        defn: CallbackInterface,
+    ) -> Result<()> {
+        self.types.add_known_type(&defn.as_type())?;
+        self.types.add_known_types(defn.iter_types())?;
         self.callback_interfaces.push(defn);
+        Ok(())
     }
 
     pub(super) fn add_trait_method_meta(&mut self, meta: TraitMethodMetadata) -> Result<()> {
@@ -1245,9 +1252,7 @@ new definition: Enum {
 
     #[test]
     fn test_contains_optional_types() {
-        let mut ci = ComponentInterface {
-            ..Default::default()
-        };
+        let mut ci = ComponentInterface::default();
 
         // check that `contains_optional_types` returns false when there is no Optional type in the interface
         assert!(!ci.contains_optional_types());
@@ -1362,4 +1367,50 @@ new definition: Enum {
         let ci = ComponentInterface::from_webidl(UDL, "crate_name").unwrap();
         assert_eq!(ci.namespace_docstring().unwrap(), "informative\ndocstring");
     }
+
+    #[test]
+    fn test_names() {
+        let mut ci = ComponentInterface::default();
+
+        let ob = Object {
+            name: "ob".to_string(),
+            module_path: "mp".to_string(),
+            imp: ObjectImpl::Struct,
+            constructors: Default::default(),
+            methods: Default::default(),
+            uniffi_traits: Default::default(),
+            ffi_func_clone: Default::default(),
+            trait_impls: Default::default(),
+            ffi_func_free: Default::default(),
+            ffi_init_callback: Default::default(),
+            docstring: Default::default(),
+        };
+        ci.add_object_definition(ob).unwrap();
+        assert!(ci.get_object_definition("ob").is_some());
+        assert_eq!(
+            ci.types.get_type_definition("ob"),
+            Some(Type::Object {
+                module_path: "mp".to_string(),
+                name: "ob".to_string(),
+                imp: ObjectImpl::Struct
+            })
+        );
+
+        let cb = CallbackInterface {
+            name: "cb".to_string(),
+            module_path: "mp".to_string(),
+            methods: vec![],
+            ffi_init_callback: FfiFunction::default(),
+            docstring: None,
+        };
+        ci.add_callback_interface_definition(cb).unwrap();
+        assert_eq!(
+            ci.types.get_type_definition("cb"),
+            Some(Type::CallbackInterface {
+                module_path: "mp".to_string(),
+                name: "cb".to_string()
+            })
+        );
+        assert!(ci.get_callback_interface_definition("cb").is_some());
+    }
 }

uniffi_bindgen/src/macro_metadata/ci.rs

@@ -107,11 +107,7 @@ fn add_item_to_ci(iface: &mut ComponentInterface, item: Metadata) -> anyhow::Res
             iface.add_object_trait_impl(meta)?;
         }
         Metadata::CallbackInterface(meta) => {
-            iface.types.add_known_type(&Type::CallbackInterface {
-                module_path: meta.module_path.clone(),
-                name: meta.name.clone(),
-            })?;
-            iface.add_callback_interface_definition(CallbackInterface::try_from(meta)?);
+            iface.add_callback_interface_definition(CallbackInterface::try_from(meta)?)?;
         }
         Metadata::TraitMethod(meta) => {
             iface.add_trait_method_meta(meta)?;

uniffi_meta/src/types.rs

@@ -151,7 +151,6 @@ impl Type {
             Type::Enum { name, .. } => Some(name),
             Type::External { name, .. } => Some(name),
             Type::Custom { name, .. } => Some(name),
-            Type::Optional { inner_type } | Type::Sequence { inner_type } => inner_type.name(),
             _ => None,
         }
     }
@@ -163,9 +162,6 @@ impl Type {
             Type::Enum { module_path, .. } => Some(module_path),
             Type::External { module_path, .. } => Some(module_path),
             Type::Custom { module_path, .. } => Some(module_path),
-            Type::Optional { inner_type } | Type::Sequence { inner_type } => {
-                inner_type.module_path()
-            }
             _ => None,
         }
     }