Merge pull request #1847 from bendk/impl-lift-for-flat-errors

Implement `Lift` for flat errors

Author: bendk

fixtures/coverall/src/coverall.udl

@@ -22,6 +22,11 @@ namespace coverall {
     void test_getters(Getters g);
 
     sequence<string> ancestor_names(NodeTrait node);
+
+    ReturnOnlyDict output_return_only_dict();
+    ReturnOnlyEnum output_return_only_enum();
+
+    void try_input_return_only_dict(ReturnOnlyDict d);
 };
 
 dictionary SimpleDict {
@@ -49,6 +54,21 @@ dictionary SimpleDict {
     NodeTrait? test_trait;
 };
 
+// Create a type that stores `CoverallFlatError` and therefore can only be lowered but not lifted.
+// UniFFI should define a `Lower` implemenation but not try to define `Lift`.
+dictionary ReturnOnlyDict {
+    CoverallFlatError e;
+};
+
+// More complicated version of the above, each variant is return-only for different reasons
+[Enum]
+interface ReturnOnlyEnum {
+    One(CoverallFlatError e);
+    Two(ReturnOnlyDict d);
+    Three(sequence<CoverallFlatError> l);
+    Four(record<string, CoverallFlatError> m);
+};
+
 dictionary DictWithDefaults {
     string name = "default-value";
     string? category = null;

fixtures/coverall/src/lib.rs

@@ -191,6 +191,44 @@ pub struct SimpleDict {
     test_trait: Option<Arc<dyn NodeTrait>>,
 }
 
+pub struct ReturnOnlyDict {
+    e: CoverallFlatError,
+}
+
+pub enum ReturnOnlyEnum {
+    One {
+        e: CoverallFlatError,
+    },
+    Two {
+        d: ReturnOnlyDict,
+    },
+    Three {
+        l: Vec<CoverallFlatError>,
+    },
+    Four {
+        m: HashMap<String, CoverallFlatError>,
+    },
+}
+
+fn output_return_only_dict() -> ReturnOnlyDict {
+    ReturnOnlyDict {
+        e: CoverallFlatError::TooManyVariants { num: 1 },
+    }
+}
+
+fn output_return_only_enum() -> ReturnOnlyEnum {
+    ReturnOnlyEnum::One {
+        e: CoverallFlatError::TooManyVariants { num: 2 },
+    }
+}
+
+fn try_input_return_only_dict(_d: ReturnOnlyDict) {
+    // This function can't work because ReturnOnlyDict contains a flat error and therefore
+    // can't be lifted by Rust.  There's a Python test that the UniFFI code panics before we get here.
+    //
+    // FIXME: should be a compile-time error rather than a runtime error (#1850)
+}
+
 #[derive(Debug, Clone)]
 pub struct DictWithDefaults {
     name: String,

fixtures/coverall/tests/bindings/test_coverall.py

@@ -4,6 +4,7 @@
 
 import unittest
 from datetime import datetime, timezone
+
 from coverall import *
 
 class TestCoverall(unittest.TestCase):
@@ -289,6 +290,13 @@ def test_bytes(self):
         coveralls = Coveralls("test_bytes")
         self.assertEqual(coveralls.reverse(b"123"), b"321")
 
+    def test_return_only_dict(self):
+        # try_input_return_only_dict can never work, since ReturnOnlyDict should only be returned
+        # from Rust not inputted.  Test that an attempt raises an internal error rather than tries
+        # to use an invalid value.
+        with self.assertRaises(InternalError):
+            try_input_return_only_dict(ReturnOnlyDict(e=CoverallFlatError.TooManyVariants))
+
 class PyGetters:
     def get_bool(self, v, arg2):
         return v ^ arg2

uniffi_macros/src/error.rs

@@ -87,6 +87,7 @@ fn flat_error_ffi_converter_impl(
 ) -> TokenStream {
     let name = ident_to_string(ident);
     let lower_impl_spec = tagged_impl_header("Lower", ident, udl_mode);
+    let lift_impl_spec = tagged_impl_header("Lift", ident, udl_mode);
     let derive_ffi_traits = derive_ffi_traits(ident, udl_mode, &["ConvertError"]);
     let mod_path = match mod_path() {
         Ok(p) => p,
@@ -127,8 +128,7 @@ fn flat_error_ffi_converter_impl(
         }
     };
 
-    let lift_impl = implement_lift.then(|| {
-        let lift_impl_spec = tagged_impl_header("Lift", ident, udl_mode);
+    let lift_impl = if implement_lift {
         let match_arms = enum_.variants.iter().enumerate().map(|(i, v)| {
             let v_ident = &v.ident;
             let idx = Index::from(i + 1);
@@ -157,7 +157,31 @@ fn flat_error_ffi_converter_impl(
             }
 
         }
-    });
+    } else {
+        quote! {
+            // Lifting flat errors is not currently supported, but we still define the trait so
+            // that dicts containing flat errors don't cause compile errors (see ReturnOnlyDict in
+            // coverall.rs).
+            //
+            // Note: it would be better to not derive `Lift` for dictionaries containing flat
+            // errors, but getting the trait bounds and derived impls there would be much harder.
+            // For now, we just fail at runtime.
+            #[automatically_derived]
+            unsafe #lift_impl_spec {
+                type FfiType = ::uniffi::RustBuffer;
+
+                fn try_read(buf: &mut &[::std::primitive::u8]) -> ::uniffi::deps::anyhow::Result<Self> {
+                    panic!("Can't lift flat errors")
+                }
+
+                fn try_lift(v: ::uniffi::RustBuffer) -> ::uniffi::deps::anyhow::Result<Self> {
+                    panic!("Can't lift flat errors")
+                }
+
+                const TYPE_ID_META: ::uniffi::MetadataBuffer = <Self as ::uniffi::Lower<crate::UniFfiTag>>::TYPE_ID_META;
+            }
+        }
+    };
 
     quote! {
         #lower_impl