Make foreign trait implementations opt-in (#1827)

This changes the default behavior for exported trait interfaces to not
generate a foreign callback interface implementation for them.  Users
can opt-in to this using the `WithForeign` attribute for UDL and the
`with_foreign` for proc-macros.

The main reason for this change is that callback interfaces and trait
interfaces aren't completely compatible.  For example, `ByRef` is not
supported for callback interfaces, but it is supported for trait
interfaces that only have Rust implementations.

CHANGELOG.md

@@ -21,6 +21,8 @@
 
 - Rust traits `Display`, `Hash` and `Eq` exposed to Kotlin and Swift [#1817](https://github.com/mozilla/uniffi-rs/pull/1817)
 - Foreign types can now implement trait interfaces [#1791](https://github.com/mozilla/uniffi-rs/pull/1791)
+ - UDL: use the `[WithForeign]` attribute
+ - proc-macros: use the `#[uniffi::export(with_foreign)]` attribute
 - Generated Python code is able to specify a package name for the module [#1784](https://github.com/mozilla/uniffi-rs/pull/1784)
 - UDL can describe async function [#1834](https://github.com/mozilla/uniffi-rs/pull/1834)
 - UDL files can reference types defined in procmacros in this crate - see

docs/manual/src/proc_macro/index.md

@@ -89,6 +89,13 @@ trait MyTrait {
  // ...
 }
 
+// Corresponding UDL:
+// [Trait, WithForeign]
+// interface MyTrait {};
+#[uniffi::export(with_foreign)]
+trait MyTrait {
+ // ...
+}
 ```
 
 

docs/manual/src/udl/ext_types.md

@@ -37,6 +37,6 @@ namespace app {
 ```
 
 Supported values:
-* "enum", "trait", "callback"
+* "enum", "trait", "callback", "trait_with_foreign"
 * For records, either "record" or "dictionary"
 * For objects, either "object" or "interface"

docs/manual/src/udl/interfaces.md

@@ -124,7 +124,14 @@ fn press(button: Arc<dyn Button>) -> Arc<dyn Button> { ... }
 
 ### Foreign implementations
 
-Traits can also be implemented on the foreign side passed into Rust, for example:
+Use the `WithForeign` attribute to allow traits to also be implemented on the foreign side passed into Rust, for example:
+
+```idl
+[Trait, WithForeign]
+interface Button {
+ string name();
+};
+```
 
 ```python
 class PyButton(uniffi_module.Button):
@@ -134,7 +141,7 @@ class PyButton(uniffi_module.Button):
 uniffi_module.press(PyButton())
 ```
 
-Note: This is currently supported on Python, Kotlin, and Swift.
+Note: This is currently only supported on Python, Kotlin, and Swift.
 
 ### Traits construction
 

examples/callbacks/src/callbacks.udl

@@ -3,7 +3,7 @@ namespace callbacks {
 };
 
 // This trait is implemented in Rust and in foreign bindings.
-[Trait]
+[Trait, WithForeign]
 interface SimCard {
  string name(); // The name of the carrier/provider.
 };

examples/traits/src/traits.udl

@@ -6,7 +6,7 @@ namespace traits {
 };
 
 // This is a trait in Rust.
-[Trait]
+[Trait, WithForeign]
 interface Button {
  string name();
 };

fixtures/coverall/src/coverall.udl

@@ -23,6 +23,8 @@ namespace coverall {
 
  sequence<string> ancestor_names(NodeTrait node);
 
+ sequence<StringUtil> get_string_util_traits();
+
  ReturnOnlyDict output_return_only_dict();
  ReturnOnlyEnum output_return_only_enum();
 
@@ -220,10 +222,10 @@ interface ThreadsafeCounter {
  i32 increment_if_busy();
 };
 
-// Test trait #1
+// Test trait interface #1
 //
 // The goal here is to test all possible arg, return, and error types.
-[Trait]
+[Trait, WithForeign]
 interface Getters {
  boolean get_bool(boolean v, boolean arg2);
  [Throws=CoverallError]
@@ -235,10 +237,10 @@ interface Getters {
  Coveralls round_trip_object(Coveralls coveralls);
 };
 
-// Test trait #2
+// Test trait interface #2
 //
 // The goal here is test passing objects back and forth between Rust and the foreign side
-[Trait]
+[Trait, WithForeign]
 interface NodeTrait {
  string name(); // The name of the this node
 
@@ -254,6 +256,14 @@ interface NodeTrait {
  u64 strong_count();
 };
 
+// Test trait interface #3
+//
+// The goal here is test Rust-only trait interfaces
+[Trait]
+interface StringUtil {
+ string concat([ByRef]string a, [ByRef]string b);
+};
+
 // Forward/backward declarations are fine in UDL.
 // Running the Python tests & type checks will ensure this works,
 // no function calls needed

fixtures/coverall/src/lib.rs

@@ -11,8 +11,8 @@ use once_cell::sync::Lazy;
 
 mod traits;
 pub use traits::{
- ancestor_names, get_traits, make_rust_getters, test_getters, test_round_trip_through_foreign,
- test_round_trip_through_rust, Getters, NodeTrait,
+ ancestor_names, get_string_util_traits, get_traits, make_rust_getters, test_getters,
+ test_round_trip_through_foreign, test_round_trip_through_rust, Getters, NodeTrait, StringUtil,
 };
 
 static NUM_ALIVE: Lazy<RwLock<u64>> = Lazy::new(|| RwLock::new(0));

fixtures/coverall/src/traits.rs

@@ -200,3 +200,26 @@ impl NodeTrait for Trait2 {
  (*self.parent.lock().unwrap()).as_ref().map(Arc::clone)
  }
 }
+
+pub trait StringUtil: Send + Sync {
+ fn concat(&self, a: &str, b: &str) -> String;
+}
+
+pub struct StringUtilImpl1;
+pub struct StringUtilImpl2;
+
+pub fn get_string_util_traits() -> Vec<Arc<dyn StringUtil>> {
+ vec![Arc::new(StringUtilImpl1), Arc::new(StringUtilImpl2)]
+}
+
+impl StringUtil for StringUtilImpl1 {
+ fn concat(&self, a: &str, b: &str) -> String {
+ format!("{a}{b}")
+ }
+}
+
+impl StringUtil for StringUtilImpl2 {
+ fn concat(&self, a: &str, b: &str) -> String {
+ format!("{a}{b}")
+ }
+}

fixtures/coverall/tests/bindings/test_coverall.kts

@@ -406,6 +406,12 @@ makeRustGetters().let { rustGetters ->
  testRoundTripThroughForeign(KotlinGetters())
 }
 
+// Test StringUtil
+getStringUtilTraits().let { traits ->
+ assert(traits[0].concat("cow", "boy") == "cowboy")
+ assert(traits[1].concat("cow", "boy") == "cowboy")
+}
+
 // This tests that the UniFFI-generated scaffolding doesn't introduce any unexpected locking.
 // We have one thread busy-wait for a some period of time, while a second thread repeatedly
 // increments the counter and then checks if the object is still busy. The second thread should

fixtures/coverall/tests/bindings/test_coverall.py

@@ -443,5 +443,10 @@ def test_round_tripping(self):
 
  test_round_trip_through_foreign(PyGetters())
 
+ def test_rust_only_traits(self):
+ traits = get_string_util_traits()
+ self.assertEqual(traits[0].concat("cow", "boy"), "cowboy")
+ self.assertEqual(traits[1].concat("cow", "boy"), "cowboy")
+
 if __name__=='__main__':
  unittest.main()

fixtures/coverall/tests/bindings/test_coverall.swift

@@ -457,3 +457,10 @@ do {
 
  testRoundTripThroughForeign(getters: SwiftGetters())
 }
+
+// Test rust-only traits
+do {
+ let stringUtils = getStringUtilTraits()
+ assert(stringUtils[0].concat(a: "cow", b: "boy") == "cowboy")
+ assert(stringUtils[1].concat(a: "cow", b: "boy") == "cowboy")
+}

fixtures/ext-types/uniffi-one/src/lib.rs

@@ -39,7 +39,7 @@ async fn get_uniffi_one_async() -> UniffiOneEnum {
  UniffiOneEnum::One
 }
 
-#[uniffi::export]
+#[uniffi::export(with_foreign)]
 pub trait UniffiOneTrait: Send + Sync {
  fn hello(&self) -> String;
 }

fixtures/ext-types/uniffi-one/src/uniffi-one.udl

@@ -15,7 +15,7 @@ interface UniffiOneInterface {
  i32 increment();
 };
 
-[Trait]
+[Trait, WithForeign]
 interface UniffiOneUDLTrait {
  string hello();
 };

fixtures/metadata/src/tests.rs

@@ -471,6 +471,15 @@ mod test_function_metadata {
  }
  }
 
+ #[uniffi::export(with_foreign)]
+ pub trait TraitWithForeign: Send + Sync {
+ fn test_method(&self, a: String, b: u32) -> String;
+ }
+
+ #[allow(unused)]
+ #[uniffi::export]
+ fn input_trait_with_foreign(val: Arc<dyn TraitWithForeign>) {}
+
  #[test]
  fn test_function() {
  check_metadata(
@@ -680,20 +689,48 @@ mod test_function_metadata {
  }
 
  #[test]
- fn test_trait_result() {
+ fn test_trait_metadata() {
+ check_metadata(
+ &UNIFFI_META_UNIFFI_FIXTURE_METADATA_INTERFACE_CALCULATORDISPLAY,
+ ObjectMetadata {
+ module_path: "uniffi_fixture_metadata".into(),
+ name: "CalculatorDisplay".into(),
+ imp: ObjectImpl::Trait,
+ docstring: None,
+ },
+ );
+ }
+
+ #[test]
+ fn test_trait_with_foreign_metadata() {
+ check_metadata(
+ &UNIFFI_META_UNIFFI_FIXTURE_METADATA_INTERFACE_TRAITWITHFOREIGN,
+ ObjectMetadata {
+ module_path: "uniffi_fixture_metadata".into(),
+ name: "TraitWithForeign".into(),
+ imp: ObjectImpl::CallbackTrait,
+ docstring: None,
+ },
+ );
+ }
+
+ #[test]
+ fn test_trait_type_data() {
  check_metadata(
  &UNIFFI_META_UNIFFI_FIXTURE_METADATA_METHOD_CALCULATOR_GET_DISPLAY,
  MethodMetadata {
- module_path: "uniffi_fixture_metadata".into(),
- self_name: "Calculator".into(),
- name: "get_display".into(),
- is_async: false,
- inputs: vec![],
+ // The main point of this test is to check the `Type` value for a trait interface
  return_type: Some(Type::Object {
  module_path: "uniffi_fixture_metadata".into(),
  name: "CalculatorDisplay".into(),
  imp: ObjectImpl::Trait,
  }),
+ // We might as well test other fields too though
+ module_path: "uniffi_fixture_metadata".into(),
+ self_name: "Calculator".into(),
+ name: "get_display".into(),
+ is_async: false,
+ inputs: vec![],
  throws: None,
  takes_self_by_arc: false,
  checksum: Some(
@@ -728,6 +765,37 @@ mod test_function_metadata {
  );
  }
 
+ #[test]
+ fn test_trait_with_foreign_type_data() {
+ check_metadata(
+ &UNIFFI_META_UNIFFI_FIXTURE_METADATA_FUNC_INPUT_TRAIT_WITH_FOREIGN,
+ FnMetadata {
+ inputs: vec![
+ // The main point of this test is to check the `Type` value for a trait interface
+ FnParamMetadata::simple(
+ "val",
+ Type::Object {
+ module_path: "uniffi_fixture_metadata".into(),
+ name: "TraitWithForeign".into(),
+ imp: ObjectImpl::CallbackTrait,
+ },
+ ),
+ ],
+ // We might as well test other fields too though
+ return_type: None,
+ module_path: "uniffi_fixture_metadata".into(),
+ name: "input_trait_with_foreign".into(),
+ is_async: false,
+ throws: None,
+ checksum: Some(
+ UNIFFI_META_CONST_UNIFFI_FIXTURE_METADATA_FUNC_INPUT_TRAIT_WITH_FOREIGN
+ .checksum(),
+ ),
+ docstring: None,
+ },
+ );
+ }
+
  #[test]
  fn test_callback_interface() {
  check_metadata(

fixtures/proc-macro/src/lib.rs

@@ -56,14 +56,30 @@ impl Unused {
 
 #[uniffi::export]
 pub trait Trait: Send + Sync {
- fn name(&self) -> String;
+ // Test the absence of `with_foreign` by inputting reference arguments, which is
+ // incompatible with callback interfaces
+ #[allow(clippy::ptr_arg)]
+ fn concat_strings(&self, a: &str, b: &str) -> String;
 }
 
 struct TraitImpl {}
 
 impl Trait for TraitImpl {
+ fn concat_strings(&self, a: &str, b: &str) -> String {
+ format!("{a}{b}")
+ }
+}
+
+#[uniffi::export(with_foreign)]
+pub trait TraitWithForeign: Send + Sync {
+ fn name(&self) -> String;
+}
+
+struct RustTraitImpl {}
+
+impl TraitWithForeign for RustTraitImpl {
  fn name(&self) -> String {
- "TraitImpl".to_string()
+ "RustTraitImpl".to_string()
  }
 }
 
@@ -97,15 +113,22 @@ impl Object {
  inc.unwrap_or_else(|| Arc::new(TraitImpl {}))
  }
 
+ fn get_trait_with_foreign(
+ &self,
+ inc: Option<Arc<dyn TraitWithForeign>>,
+ ) -> Arc<dyn TraitWithForeign> {
+ inc.unwrap_or_else(|| Arc::new(RustTraitImpl {}))
+ }
+
  fn take_error(&self, e: BasicError) -> u32 {
  assert!(matches!(e, BasicError::InvalidInput));
  42
  }
 }
 
 #[uniffi::export]
-fn get_trait_name_by_ref(t: &dyn Trait) -> String {
- t.name()
+fn concat_strings_by_ref(t: &dyn Trait, a: &str, b: &str) -> String {
+ t.concat_strings(a, b)
 }
 
 #[uniffi::export]
@@ -260,6 +283,12 @@ fn get_trait(o: Option<Arc<dyn Trait>>) -> Arc<dyn Trait> {
  o.unwrap_or_else(|| Arc::new(TraitImpl {}))
 }
 
+fn get_trait_with_foreign(
+ o: Option<Arc<dyn TraitWithForeign>>,
+) -> Arc<dyn TraitWithForeign> {
+ o.unwrap_or_else(|| Arc::new(RustTraitImpl {}))
+}
+
 #[derive(Default)]
 struct Externals {
  one: Option<One>,