Allow UDL to avoid the [Rust=...] attribute by using a plain-old ty…

…pedef (#2199)

CHANGELOG.md

@@ -11,6 +11,10 @@
 - Lift errors will not cause an abort when `panic=abort` is set.
 - Added the `cargo_metadata` feature, which is on by default.  In some cases, this can be disabled
   for better compatibility with projects that don't use cargo.
+- A new bindgen command line option `--metadata-no-deps` is available to avoid processing
+  cargo_metadata for all dependencies.
+- In UDL it's now possible (and preferred) to remove the `[Rust=]` attribute and use a plain-old typedef.
+  See [the manual page for this](https://mozilla.github.io/uniffi-rs/next/udl/ext_types.html#types-from-procmacros-in-this-crate).
 
 ### What's changed?
 - Kotlin will use the more efficient Enum.entries property instead of Enum.values() when possible
@@ -20,10 +24,6 @@
 ## v0.28.0 (backend crates: v0.28.0) - (_2024-06-11_)
 
 ### What's new?
-
-- A new bindgen command line option `--metadata-no-deps` is available to avoid processing
-  cargo_metadata for all dependencies.
-
 - Objects error types can now be as `Result<>` error type without wrapping them in `Arc<>`.
 
 - Swift errors now provide `localizedDescription` ([#2116](https://github.com/mozilla/uniffi-rs/pull/2116))

docs/manual/src/udl/ext_types.md

@@ -14,8 +14,7 @@ giving more detail.
 ## Types from procmacros in this crate.
 
 If your crate has types defined via `#[uniffi::export]` etc you can make them available
-to the UDL file in your own crate via a `typedef` with a `[Rust=]` attribute. Eg, your Rust
-might have:
+to the UDL file in your own crate via a `typedef` describing the concrete type.
 
 ```rust
 #[derive(uniffi::Record)]
@@ -26,8 +25,7 @@ pub struct One {
 you can use it in your UDL:
 
 ```idl
-[Rust="record"]
-typedef extern One;
+typedef record One;
 
 namespace app {
     // use the procmacro type.
@@ -37,6 +35,16 @@ namespace app {
 ```
 
 Supported values:
-*  "enum", "trait", "callback", "trait_with_foreign"
-* For records, either "record" or "dictionary"
-* For objects, either "object" or "interface"
+* "enum", "trait", "callback", "trait_with_foreign"
+* For records, either "record", "dictionary" or "struct"
+* For objects, either "object", "impl" or "interface"
+
+eg:
+```
+typedef enum MyEnum;
+typedef interface MyObject;
+```
+etc.
+
+Note that in 0.28 and prior, we also supported this capability with a `[Rust=]` attribute.
+This attribute is deprecated and may be removed in a later version.

fixtures/proc-macro-no-implicit-prelude/src/proc-macro.udl

@@ -1,9 +1,10 @@
-// Use this to test that types defined in the UDL can be used in the proc-macros
+// Like our proc-macro fixture, but tests everything works without Rust `std::` type preludes.
 dictionary Zero {
     string inner;
 };
 
-// And all of these for the opposite - proc-macro types used in UDL.
+// NOTE: `[Rust=..]` is deprecated and this test hasn't migrated.
+// This helps testing the attribute, so don't remove them unless you are removing support entirely!
 [Rust="record"]
 typedef extern One;
 

fixtures/proc-macro/src/proc-macro.udl

@@ -4,20 +4,11 @@ dictionary Zero {
 };
 
 // And all of these for the opposite - proc-macro types used in UDL.
-[Rust="record"]
-typedef extern One;
-
-[Rust="enum"]
-typedef extern MaybeBool;
-
-[Rust="interface"]
-typedef extern Object;
-
-[Rust="trait"]
-typedef extern Trait;
-
-[Rust="trait_with_foreign"]
-typedef extern TraitWithForeign;
+typedef record One;
+typedef enum MaybeBool;
+typedef interface Object;
+typedef trait Trait;
+typedef trait_with_foreign TraitWithForeign;
 
 // Then stuff defined here but referencing the imported types.
 dictionary Externals {

uniffi_udl/src/finder.rs

@@ -129,43 +129,89 @@ impl TypeFinder for weedle::TypedefDefinition<'_> {
                 },
             )
         } else {
+            let typedef_type = match &self.type_.type_ {
+                weedle::types::Type::Single(weedle::types::SingleType::NonAny(
+                    weedle::types::NonAnyType::Identifier(weedle::types::MayBeNull {
+                        type_: i,
+                        ..
+                    }),
+                )) => i.0,
+                _ => bail!("Failed to get typedef type: {:?}", self),
+            };
+
             let module_path = types.module_path();
             let name = self.identifier.0.to_string();
-            let ty = match attrs.rust_kind() {
-                Some(RustKind::Object) => Type::Object {
-                    module_path,
-                    name,
-                    imp: ObjectImpl::Struct,
-                },
-                Some(RustKind::Trait) => Type::Object {
-                    module_path,
-                    name,
-                    imp: ObjectImpl::Trait,
-                },
-                Some(RustKind::CallbackTrait) => Type::Object {
-                    module_path,
-                    name,
-                    imp: ObjectImpl::CallbackTrait,
-                },
-                Some(RustKind::Record) => Type::Record { module_path, name },
-                Some(RustKind::Enum) => Type::Enum { module_path, name },
-                Some(RustKind::CallbackInterface) => Type::CallbackInterface { module_path, name },
-                // must be external
+
+            let ty = match attrs.external_tagged() {
                 None => {
-                    let kind = attrs.external_kind().expect("External missing kind");
-                    let tagged = attrs.external_tagged().expect("External missing tagged");
-                    Type::External {
-                        name,
-                        namespace: "".to_string(), // we don't know this yet
-                        module_path: attrs.get_crate_name(),
-                        kind,
-                        tagged,
+                    // Not external, not custom, not Rust - so we basically
+                    // pretend it is Rust, thus soft-deprecating it.
+                    // We use `type_`
+                    match typedef_type {
+                        "dictionary" | "record" | "struct" => Type::Record {
+                            module_path,
+                            name,
+                        },
+                        "enum" => Type::Enum {
+                            module_path,
+                            name,
+                        },
+                        "custom" => panic!("don't know builtin"),
+                        "interface" | "impl" => Type::Object {
+                            module_path,
+                            name,
+                            imp: ObjectImpl::Struct,
+                        },
+                        "trait" => Type::Object {
+                            module_path,
+                            name,
+                            imp: ObjectImpl::Trait,
+                        },
+                        "callback" | "trait_with_foreign" => Type::Object {
+                            module_path,
+                            name,
+                            imp: ObjectImpl::CallbackTrait,
+                        },
+                        _ => bail!("Can't work out the type - no attributes and unknown extern type '{typedef_type}'"),
+                    }
+                }
+                Some(tagged) => {
+                    // Must be either `[Rust..]` or `[Extern..]`
+                    match attrs.rust_kind() {
+                        Some(RustKind::Object) => Type::Object {
+                            module_path,
+                            name,
+                            imp: ObjectImpl::Struct,
+                        },
+                        Some(RustKind::Trait) => Type::Object {
+                            module_path,
+                            name,
+                            imp: ObjectImpl::Trait,
+                        },
+                        Some(RustKind::CallbackTrait) => Type::Object {
+                            module_path,
+                            name,
+                            imp: ObjectImpl::CallbackTrait,
+                        },
+                        Some(RustKind::Record) => Type::Record { module_path, name },
+                        Some(RustKind::Enum) => Type::Enum { module_path, name },
+                        Some(RustKind::CallbackInterface) => {
+                            Type::CallbackInterface { module_path, name }
+                        }
+                        // must be external
+                        None => {
+                            let kind = attrs.external_kind().expect("External missing kind");
+                            Type::External {
+                                name,
+                                namespace: "".to_string(), // we don't know this yet
+                                module_path: attrs.get_crate_name(),
+                                kind,
+                                tagged,
+                            }
+                        }
                     }
                 }
             };
-            // A crate which can supply an `FfiConverter`.
-            // We don't reference `self._type`, so ideally we could insist on it being
-            // the literal 'extern' but that's tricky
             types.add_type_definition(self.identifier.0, ty)
         }
     }
@@ -190,7 +236,7 @@ impl TypeFinder for weedle::CallbackInterfaceDefinition<'_> {
 #[cfg(test)]
 mod test {
     use super::*;
-    use uniffi_meta::ExternalKind;
+    use uniffi_meta::{ExternalKind, ObjectImpl};
 
     // A helper to take valid UDL and a closure to check what's in it.
     fn test_a_finding<F>(udl: &str, tester: F)
@@ -289,6 +335,74 @@ mod test {
         );
     }
 
+    #[test]
+    fn test_extern_local_types() {
+        // should test more, but these are already deprecated
+        test_a_finding(
+            r#"
+            typedef interface Interface;
+            typedef impl Interface2;
+            typedef trait Trait;
+            typedef callback Callback;
+
+            typedef dictionary R1;
+            typedef record R2;
+            typedef record R3;
+            typedef enum Enum;
+        "#,
+            |types| {
+                assert!(matches!(
+                    types.get_type_definition("Interface").unwrap(),
+                    Type::Object { name, module_path, imp: ObjectImpl::Struct } if name == "Interface" && module_path.is_empty()));
+                assert!(matches!(
+                    types.get_type_definition("Interface2").unwrap(),
+                    Type::Object { name, module_path, imp: ObjectImpl::Struct } if name == "Interface2" && module_path.is_empty()));
+                assert!(matches!(
+                    types.get_type_definition("Trait").unwrap(),
+                    Type::Object { name, module_path, imp: ObjectImpl::Trait } if name == "Trait" && module_path.is_empty()));
+                assert!(matches!(
+                    types.get_type_definition("Callback").unwrap(),
+                    Type::Object { name, module_path, imp: ObjectImpl::CallbackTrait } if name == "Callback" && module_path.is_empty()));
+                assert!(matches!(
+                    types.get_type_definition("R1").unwrap(),
+                    Type::Record { name, module_path } if name == "R1" && module_path.is_empty()));
+                assert!(matches!(
+                    types.get_type_definition("R2").unwrap(),
+                    Type::Record { name, module_path } if name == "R2" && module_path.is_empty()));
+                assert!(matches!(
+                    types.get_type_definition("R3").unwrap(),
+                    Type::Record { name, module_path } if name == "R3" && module_path.is_empty()));
+                assert!(matches!(
+                    types.get_type_definition("Enum").unwrap(),
+                    Type::Enum { name, module_path } if name == "Enum" && module_path.is_empty()));
+            },
+        );
+    }
+
+    #[test]
+    fn test_rust_attr_types() {
+        // should test more, but these are already deprecated
+        test_a_finding(
+            r#"
+            [Rust="interface"]
+            typedef extern LocalInterface;
+
+            [Rust="dictionary"]
+            typedef extern Dict;
+        "#,
+            |types| {
+                assert!(
+                    matches!(types.get_type_definition("LocalInterface").unwrap(), Type::Object { name, module_path, imp: ObjectImpl::Struct }
+                                                                                if name == "LocalInterface" && module_path.is_empty())
+                );
+                assert!(
+                    matches!(types.get_type_definition("Dict").unwrap(), Type::Record { name, module_path }
+                                                                                if name == "Dict" && module_path.is_empty())
+                );
+            },
+        );
+    }
+
     fn get_err(udl: &str) -> String {
         let parsed = weedle::parse(udl).unwrap();
         let mut types = TypeCollector::default();
@@ -299,9 +413,8 @@ mod test {
     }
 
     #[test]
-    #[should_panic]
-    fn test_typedef_error_on_no_attr() {
-        // Sorry, still working out what we want for non-imported typedefs..
-        get_err("typedef string Custom;");
+    fn test_local_type_unknown_typedef() {
+        let e = get_err("typedef xyz Foo;");
+        assert!(e.contains("unknown extern type 'xyz'"));
     }
 }