Remove a panic in add_type_definition and add error contexts so resul…

…ting errors make sense. (#2369)

fixtures/uitests/tests/ui/trait_methods_unknown_trait.stderr

@@ -1,4 +1,4 @@
-error: Failed to generate scaffolding from UDL file at ../../../../fixtures/uitests/src/trait_methods_unknown_trait.udl: Invalid trait name: PartialEq
+error: Failed to generate scaffolding from UDL file at ../../../../fixtures/uitests/src/trait_methods_unknown_trait.udl: parsing udl file ../../../../fixtures/uitests/src/trait_methods_unknown_trait.udl: Invalid trait name: PartialEq
  --> tests/ui/trait_methods_unknown_trait.rs:2:1
   |
 2 | uniffi_macros::generate_and_include_scaffolding!("../../../../fixtures/uitests/src/trait_methods_unknown_trait.udl");

fixtures/uitests/tests/ui/typedef_extern.stderr

@@ -1,4 +1,4 @@
-error: Failed to generate scaffolding from UDL file at ../../../../fixtures/uitests/src/typedef_extern.udl: `typedef extern` is no longer supported.
+error: Failed to generate scaffolding from UDL file at ../../../../fixtures/uitests/src/typedef_extern.udl: parsing udl file ../../../../fixtures/uitests/src/typedef_extern.udl: `typedef extern` is no longer supported.
        You must replace `extern` with the type of the object:
        * "enum" for Enums.
        * "record", "dictionary" or "struct" for Records.

uniffi_bindgen/src/interface/mod.rs

@@ -49,7 +49,7 @@ use std::{
     iter,
 };
 
-use anyhow::{anyhow, bail, ensure, Result};
+use anyhow::{anyhow, bail, ensure, Context, Result};
 
 pub mod universe;
 pub use uniffi_meta::{AsType, EnumShape, ExternalKind, ObjectImpl, Type};
@@ -822,7 +822,9 @@ impl ComponentInterface {
                 if matches!(defn.shape, EnumShape::Error { .. }) {
                     self.errors.insert(defn.name.clone());
                 }
-                self.types.add_known_types(defn.iter_types())?;
+                self.types
+                    .add_known_types(defn.iter_types())
+                    .with_context(|| format!("adding enum {defn:?}"))?;
                 v.insert(defn);
             }
             Entry::Occupied(o) => {
@@ -845,7 +847,9 @@ impl ComponentInterface {
     pub(super) fn add_record_definition(&mut self, defn: Record) -> Result<()> {
         match self.records.entry(defn.name().to_owned()) {
             Entry::Vacant(v) => {
-                self.types.add_known_types(defn.iter_types())?;
+                self.types
+                    .add_known_types(defn.iter_types())
+                    .with_context(|| format!("adding record {defn:?}"))?;
                 v.insert(defn);
             }
             Entry::Occupied(o) => {
@@ -874,7 +878,9 @@ impl ComponentInterface {
         if self.types.get_type_definition(defn.name()).is_some() {
             bail!("Conflicting type definition for \"{}\"", defn.name());
         }
-        self.types.add_known_types(defn.iter_types())?;
+        self.types
+            .add_known_types(defn.iter_types())
+            .with_context(|| format!("adding function {defn:?}"))?;
         defn.throws_name()
             .map(|n| self.errors.insert(n.to_string()));
         self.functions.push(defn);
@@ -887,7 +893,9 @@ impl ComponentInterface {
             .ok_or_else(|| anyhow!("add_constructor_meta: object {} not found", &meta.self_name))?;
         let defn: Constructor = meta.into();
 
-        self.types.add_known_types(defn.iter_types())?;
+        self.types
+            .add_known_types(defn.iter_types())
+            .with_context(|| format!("adding constructor {defn:?}"))?;
         defn.throws_name()
             .map(|n| self.errors.insert(n.to_string()));
         object.constructors.push(defn);
@@ -900,7 +908,9 @@ impl ComponentInterface {
         let object = get_object(&mut self.objects, &method.object_name)
             .ok_or_else(|| anyhow!("add_method_meta: object {} not found", &method.object_name))?;
 
-        self.types.add_known_types(method.iter_types())?;
+        self.types
+            .add_known_types(method.iter_types())
+            .with_context(|| format!("adding method {method:?}"))?;
         method
             .throws_name()
             .map(|n| self.errors.insert(n.to_string()));
@@ -913,7 +923,10 @@ impl ComponentInterface {
         let object = get_object(&mut self.objects, meta.self_name())
             .ok_or_else(|| anyhow!("add_uniffitrait_meta: object not found"))?;
         let ut: UniffiTrait = meta.into();
-        self.types.add_known_types(ut.iter_types())?;
+        self.types
+            .add_known_types(ut.iter_types())
+            .with_context(|| format!("adding builtin trait {ut:?}"))?;
+
         object.uniffi_traits.push(ut);
         Ok(())
     }
@@ -925,7 +938,10 @@ 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.types
+            .add_known_types(defn.iter_types())
+            .with_context(|| format!("adding object {defn:?}'"))?;
+
         self.objects.push(defn);
         Ok(())
     }
@@ -940,7 +956,10 @@ impl ComponentInterface {
         defn: CallbackInterface,
     ) -> Result<()> {
         self.types.add_known_type(&defn.as_type())?;
-        self.types.add_known_types(defn.iter_types())?;
+        self.types
+            .add_known_types(defn.iter_types())
+            .with_context(|| format!("adding callback {defn:?}'"))?;
+
         self.callback_interfaces.push(defn);
         Ok(())
     }
@@ -962,7 +981,9 @@ impl ComponentInterface {
             if let Some(error) = method.throws_type() {
                 self.callback_interface_throws_types.insert(error.clone());
             }
-            self.types.add_known_types(method.iter_types())?;
+            self.types
+                .add_known_types(method.iter_types())
+                .with_context(|| format!("adding trait method {method:?}"))?;
             method
                 .throws_name()
                 .map(|n| self.errors.insert(n.to_string()));

uniffi_bindgen/src/interface/universe.rs

@@ -6,7 +6,7 @@
 //! which can be used by the bindings generator code to determine what type-related helper
 //! functions to emit for a given component.
 //!
-use anyhow::Result;
+use anyhow::{Context, Result};
 use std::{collections::hash_map::Entry, collections::BTreeSet, collections::HashMap};
 
 pub use uniffi_meta::{AsType, ExternalKind, NamespaceMetadata, ObjectImpl, Type, TypeIterator};
@@ -45,9 +45,12 @@ impl TypeUniverse {
     fn add_type_definition(&mut self, name: &str, type_: &Type) -> Result<()> {
         match self.type_definitions.entry(name.to_string()) {
             Entry::Occupied(o) => {
-                // all conflicts have been resolved by now in udl.
-                // I doubt procmacros could cause this?
-                assert_eq!(type_, o.get());
+                // mismatched types are bad.
+                let cur = o.get();
+                anyhow::ensure!(
+                    type_ == cur,
+                    "conflicting types:\ncur: {cur:?}\nnew: {type_:?}",
+                );
                 Ok(())
             }
             Entry::Vacant(e) => {
@@ -91,18 +94,22 @@ impl TypeUniverse {
             | Type::External { name, .. } => self.add_type_definition(name, type_)?,
             Type::Custom { name, builtin, .. } => {
                 self.add_type_definition(name, type_)?;
-                self.add_known_type(builtin)?;
+                self.add_known_type(builtin)
+                    .with_context(|| format!("adding custom builtin type {builtin:?}"))?;
             }
             // Structurally recursive types.
             Type::Optional { inner_type, .. } | Type::Sequence { inner_type, .. } => {
-                self.add_known_type(inner_type)?;
+                self.add_known_type(inner_type)
+                    .with_context(|| format!("adding optional type {inner_type:?}"))?;
             }
             Type::Map {
                 key_type,
                 value_type,
             } => {
-                self.add_known_type(key_type)?;
-                self.add_known_type(value_type)?;
+                self.add_known_type(key_type)
+                    .with_context(|| format!("adding map key type {key_type:?}"))?;
+                self.add_known_type(value_type)
+                    .with_context(|| format!("adding map value type {value_type:?}"))?;
             }
         }
         Ok(())
@@ -111,7 +118,8 @@ impl TypeUniverse {
     /// Add many [`Type`]s...
     pub fn add_known_types(&mut self, types: TypeIterator<'_>) -> Result<()> {
         for t in types {
-            self.add_known_type(t)?
+            self.add_known_type(t)
+                .with_context(|| format!("adding type {t:?}"))?
         }
         Ok(())
     }

uniffi_bindgen/src/lib.rs

@@ -335,7 +335,8 @@ pub fn generate_component_scaffolding(
     out_dir_override: Option<&Utf8Path>,
     format_code: bool,
 ) -> Result<()> {
-    let component = parse_udl(udl_file, &crate_name_from_cargo_toml(udl_file)?)?;
+    let component = parse_udl(udl_file, &crate_name_from_cargo_toml(udl_file)?)
+        .with_context(|| format!("parsing udl file {udl_file}"))?;
     generate_component_scaffolding_inner(component, udl_file, out_dir_override, format_code)
 }
 
@@ -348,7 +349,8 @@ pub fn generate_component_scaffolding_for_crate(
     out_dir_override: Option<&Utf8Path>,
     format_code: bool,
 ) -> Result<()> {
-    let component = parse_udl(udl_file, crate_name)?;
+    let component =
+        parse_udl(udl_file, crate_name).with_context(|| format!("parsing udl file {udl_file}"))?;
     generate_component_scaffolding_inner(component, udl_file, out_dir_override, format_code)
 }
 
@@ -365,7 +367,7 @@ fn generate_component_scaffolding_inner(
     write!(f, "{}", RustScaffolding::new(&component, file_stem))
         .context("Failed to write output file")?;
     if format_code {
-        format_code_with_rustfmt(&out_path)?;
+        format_code_with_rustfmt(&out_path).context("formatting generated Rust code")?;
     }
     Ok(())
 }

uniffi_bindgen/src/library_mode.rs

@@ -19,7 +19,7 @@ use crate::{
     macro_metadata, overridden_config_value, BindgenCrateConfigSupplier, BindingGenerator,
     Component, ComponentInterface, GenerationSettings, Result,
 };
-use anyhow::bail;
+use anyhow::{bail, Context};
 use camino::Utf8Path;
 use std::{collections::HashMap, fs};
 use toml::value::Table as TomlTable;
@@ -43,11 +43,14 @@ pub fn generate_bindings<T: BindingGenerator + ?Sized>(
     out_dir: &Utf8Path,
     try_format_code: bool,
 ) -> Result<Vec<Component<T::Config>>> {
-    let mut components = find_components(library_path, config_supplier)?
+    let mut components = find_components(library_path, config_supplier)
+        .with_context(|| format!("finding components in '{library_path}'"))?
         .into_iter()
         .map(|Component { ci, config }| {
             let toml_value = overridden_config_value(config, config_file_override)?;
-            let config = binding_generator.new_config(&toml_value)?;
+            let config = binding_generator
+                .new_config(&toml_value)
+                .context("loading toml")?;
             Ok(Component { ci, config })
         })
         .collect::<Result<Vec<_>>>()?;