Refactoring the trait interface code

This is prep work for allowing the foreign bindings to implement traits.

Moved trait interface code in `uniffi_macros` to its own module.

Updated the test trait in the coverall fixture.  I think this one will
work better to test foreign trait impls.  The `ancestor_names()` method
is a good way to test trait implementations that bounce between the Rust
and foreign side of the FFI.

Added Getters test like we have with callback interfaces. We want to
replace callback interfaces with trait interfaces, so we should have
good test coverage.  This actually revealed that the trait code didn't
support exceptions yet.

fixtures/coverall/src/coverall.udl

@@ -4,7 +4,7 @@ namespace coverall {
 
     u64 get_num_alive();
 
-    sequence<TestTrait> get_traits();
+    sequence<NodeTrait> get_traits();
 
     MaybeSimpleDict get_maybe_simple_dict(i8 index);
 
@@ -17,6 +17,11 @@ namespace coverall {
 
     [Throws=CoverallRichErrorNoVariantData]
     void throw_rich_error_no_variant_data();
+
+    Getters make_rust_getters();
+    void test_getters(Getters g);
+
+    sequence<string> ancestor_names(NodeTrait node);
 };
 
 dictionary SimpleDict {
@@ -41,7 +46,7 @@ dictionary SimpleDict {
     double float64;
     double? maybe_float64;
     Coveralls? coveralls;
-    TestTrait? test_trait;
+    NodeTrait? test_trait;
 };
 
 dictionary DictWithDefaults {
@@ -190,24 +195,37 @@ interface ThreadsafeCounter {
   i32 increment_if_busy();
 };
 
-// This is a trait implemented on the Rust side.
+// Test trait #1
+//
+// The goal here is to test all possible arg, return, and error types.
 [Trait]
-interface TestTrait {
-    string name(); // The name of the implementation
+interface Getters {
+    boolean get_bool(boolean v, boolean arg2);
+    [Throws=CoverallError]
+    string get_string(string v, boolean arg2);
+    [Throws=ComplexError]
+    string? get_option(string v, boolean arg2);
+    sequence<i32> get_list(sequence<i32> v, boolean arg2);
+    void get_nothing(string v);
+};
 
-    [Self=ByArc]
-    u64 number();
+// Test trait #2
+//
+// The goal here is test passing objects back and forth between Rust and the foreign side
+[Trait]
+interface NodeTrait {
+    string name(); // The name of the this node
+
+    /// Takes an `Arc<Self>` and stores it our parent node, dropping any existing / reference. Note
+    //you can create circular references with this.
+    void set_parent(NodeTrait? parent);
+
+    /// Returns what was previously set via `set_parent()`, or null.
+    NodeTrait? get_parent();
 
     /// Calls `Arc::strong_count()` on the `Arc` containing `self`.
     [Self=ByArc]
     u64 strong_count();
-
-    /// Takes an `Arc<Self>` and stores it in `self`, dropping the existing
-    /// reference. Note you can create circular references by passing `self`.
-    void take_other(TestTrait? other);
-
-    /// Returns what was previously set via `take_other()`, or null.
-    TestTrait? get_other();
 };
 
 // Forward/backward declarations are fine in UDL.

fixtures/coverall/src/lib.rs

@@ -10,11 +10,11 @@ use std::time::SystemTime;
 use once_cell::sync::Lazy;
 
 mod traits;
-pub use traits::{get_traits, TestTrait};
+pub use traits::{ancestor_names, get_traits, make_rust_getters, test_getters, Getters, NodeTrait};
 
 static NUM_ALIVE: Lazy<RwLock<u64>> = Lazy::new(|| RwLock::new(0));
 
-#[derive(Debug, thiserror::Error)]
+#[derive(Debug, thiserror::Error, PartialEq, Eq)]
 pub enum CoverallError {
     #[error("The coverall has too many holes")]
     TooManyHoles,
@@ -80,7 +80,7 @@ impl From<InternalCoverallError> for CoverallError {
     }
 }
 
-#[derive(Debug, thiserror::Error)]
+#[derive(Debug, thiserror::Error, PartialEq, Eq)]
 pub enum ComplexError {
     #[error("OsError: {code} ({extended_code})")]
     OsError { code: i16, extended_code: i16 },
@@ -131,7 +131,7 @@ pub struct SimpleDict {
     float64: f64,
     maybe_float64: Option<f64>,
     coveralls: Option<Arc<Coveralls>>,
-    test_trait: Option<Arc<dyn TestTrait>>,
+    test_trait: Option<Arc<dyn NodeTrait>>,
 }
 
 #[derive(Debug, Clone)]
@@ -204,7 +204,7 @@ fn create_some_dict() -> SimpleDict {
         float64: 0.0,
         maybe_float64: Some(1.0),
         coveralls: Some(Arc::new(Coveralls::new("some_dict".to_string()))),
-        test_trait: Some(Arc::new(traits::Trait2 {})),
+        test_trait: Some(Arc::new(traits::Trait2::default())),
     }
 }
 

fixtures/coverall/src/traits.rs

@@ -2,73 +2,187 @@
  * 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 super::{ComplexError, CoverallError};
 use std::sync::{Arc, Mutex};
 
 // namespace functions.
-pub fn get_traits() -> Vec<Arc<dyn TestTrait>> {
-    vec![
-        Arc::new(Trait1 {
-            ..Default::default()
-        }),
-        Arc::new(Trait2 {}),
-    ]
+pub fn get_traits() -> Vec<Arc<dyn NodeTrait>> {
+    vec![Arc::new(Trait1::default()), Arc::new(Trait2::default())]
 }
 
-pub trait TestTrait: Send + Sync + std::fmt::Debug {
+pub trait NodeTrait: Send + Sync + std::fmt::Debug {
     fn name(&self) -> String;
 
-    fn number(self: Arc<Self>) -> u64;
+    fn set_parent(&self, parent: Option<Arc<dyn NodeTrait>>);
+
+    fn get_parent(&self) -> Option<Arc<dyn NodeTrait>>;
 
     fn strong_count(self: Arc<Self>) -> u64 {
         Arc::strong_count(&self) as u64
     }
+}
 
-    fn take_other(&self, other: Option<Arc<dyn TestTrait>>);
+pub fn ancestor_names(node: Arc<dyn NodeTrait>) -> Vec<String> {
+    let mut names = vec![];
+    let mut parent = node.get_parent();
+    while let Some(node) = parent {
+        names.push(node.name());
+        parent = node.get_parent();
+    }
+    names
+}
 
-    fn get_other(&self) -> Option<Arc<dyn TestTrait>>;
+/// Test trait
+///
+/// The goal here is to test all possible arg, return, and error types.
+pub trait Getters: Send + Sync {
+    fn get_bool(&self, v: bool, arg2: bool) -> bool;
+    fn get_string(&self, v: String, arg2: bool) -> Result<String, CoverallError>;
+    fn get_option(&self, v: String, arg2: bool) -> Result<Option<String>, ComplexError>;
+    fn get_list(&self, v: Vec<i32>, arg2: bool) -> Vec<i32>;
+    fn get_nothing(&self, v: String);
+}
+
+struct RustGetters;
+
+impl Getters for RustGetters {
+    fn get_bool(&self, v: bool, arg2: bool) -> bool {
+        v ^ arg2
+    }
+
+    fn get_string(&self, v: String, arg2: bool) -> Result<String, CoverallError> {
+        if v == "too-many-holes" {
+            Err(CoverallError::TooManyHoles)
+        } else if v == "unexpected-error" {
+            panic!("unexpected error")
+        } else if arg2 {
+            Ok(v.to_uppercase())
+        } else {
+            Ok(v)
+        }
+    }
+
+    fn get_option(&self, v: String, arg2: bool) -> Result<Option<String>, ComplexError> {
+        if v == "os-error" {
+            Err(ComplexError::OsError {
+                code: 100,
+                extended_code: 200,
+            })
+        } else if v == "unknown-error" {
+            Err(ComplexError::UnknownError)
+        } else if arg2 {
+            if !v.is_empty() {
+                Ok(Some(v.to_uppercase()))
+            } else {
+                Ok(None)
+            }
+        } else {
+            Ok(Some(v))
+        }
+    }
+
+    fn get_list(&self, v: Vec<i32>, arg2: bool) -> Vec<i32> {
+        if arg2 {
+            v
+        } else {
+            vec![]
+        }
+    }
+
+    fn get_nothing(&self, _v: String) {}
+}
+
+pub fn make_rust_getters() -> Arc<dyn Getters> {
+    Arc::new(RustGetters)
+}
+
+pub fn test_getters(getters: Arc<dyn Getters>) {
+    assert!(!getters.get_bool(true, true));
+    assert!(getters.get_bool(true, false));
+    assert!(getters.get_bool(false, true));
+    assert!(!getters.get_bool(false, false));
+
+    assert_eq!(
+        getters.get_string("hello".to_owned(), false).unwrap(),
+        "hello"
+    );
+    assert_eq!(
+        getters.get_string("hello".to_owned(), true).unwrap(),
+        "HELLO"
+    );
+
+    assert_eq!(
+        getters.get_option("hello".to_owned(), true).unwrap(),
+        Some("HELLO".to_owned())
+    );
+    assert_eq!(
+        getters.get_option("hello".to_owned(), false).unwrap(),
+        Some("hello".to_owned())
+    );
+    assert_eq!(getters.get_option("".to_owned(), true).unwrap(), None);
+
+    assert_eq!(getters.get_list(vec![1, 2, 3], true), vec![1, 2, 3]);
+    assert_eq!(getters.get_list(vec![1, 2, 3], false), Vec::<i32>::new());
+
+    // Call get_nothing to make sure it doesn't panic.  There's no point in checking the output
+    // though
+    getters.get_nothing("hello".to_owned());
+
+    assert_eq!(
+        getters.get_string("too-many-holes".to_owned(), true),
+        Err(CoverallError::TooManyHoles)
+    );
+    assert_eq!(
+        getters.get_option("os-error".to_owned(), true),
+        Err(ComplexError::OsError {
+            code: 100,
+            extended_code: 200
+        })
+    );
+    assert_eq!(
+        getters.get_option("unknown-error".to_owned(), true),
+        Err(ComplexError::UnknownError)
+    );
+    let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+        getters.get_string("unexpected-error".to_owned(), true)
+    }));
+    assert!(result.is_err());
 }
 
 #[derive(Debug, Default)]
 pub(crate) struct Trait1 {
     // A reference to another trait.
-    other: Mutex<Option<Arc<dyn TestTrait>>>,
+    parent: Mutex<Option<Arc<dyn NodeTrait>>>,
 }
 
-impl TestTrait for Trait1 {
+impl NodeTrait for Trait1 {
     fn name(&self) -> String {
-        "trait 1".to_string()
+        "node-1".to_string()
     }
 
-    fn number(self: Arc<Self>) -> u64 {
-        1_u64
+    fn set_parent(&self, parent: Option<Arc<dyn NodeTrait>>) {
+        *self.parent.lock().unwrap() = parent.map(|arc| Arc::clone(&arc))
     }
 
-    fn take_other(&self, other: Option<Arc<dyn TestTrait>>) {
-        *self.other.lock().unwrap() = other.map(|arc| Arc::clone(&arc))
-    }
-
-    fn get_other(&self) -> Option<Arc<dyn TestTrait>> {
-        (*self.other.lock().unwrap()).as_ref().map(Arc::clone)
+    fn get_parent(&self) -> Option<Arc<dyn NodeTrait>> {
+        (*self.parent.lock().unwrap()).as_ref().map(Arc::clone)
     }
 }
 
-#[derive(Debug)]
-pub(crate) struct Trait2 {}
-impl TestTrait for Trait2 {
+#[derive(Debug, Default)]
+pub(crate) struct Trait2 {
+    parent: Mutex<Option<Arc<dyn NodeTrait>>>,
+}
+impl NodeTrait for Trait2 {
     fn name(&self) -> String {
-        "trait 2".to_string()
-    }
-
-    fn number(self: Arc<Self>) -> u64 {
-        2_u64
+        "node-2".to_string()
     }
 
-    // Don't bother implementing these here - the test on the struct above is ok.
-    fn take_other(&self, _other: Option<Arc<dyn TestTrait>>) {
-        unimplemented!();
+    fn set_parent(&self, parent: Option<Arc<dyn NodeTrait>>) {
+        *self.parent.lock().unwrap() = parent.map(|arc| Arc::clone(&arc))
     }
 
-    fn get_other(&self) -> Option<Arc<dyn TestTrait>> {
-        unimplemented!()
+    fn get_parent(&self) -> Option<Arc<dyn NodeTrait>> {
+        (*self.parent.lock().unwrap()).as_ref().map(Arc::clone)
     }
 }