Unverified Commit 05778b15 authored by Mark Hammond's avatar Mark Hammond Committed by GitHub
Browse files

Remove a panic in add_type_definition and add error contexts so resulting... 

Remove a panic in add_type_definition and add error contexts so resulting errors make sense. (#2369)
parent 2baab843

fixtures/uitests/tests/ui/trait_methods_unknown_trait.stderr

+1 −1
Original line number Diff line number Diff line 
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 −1
Original line number Diff line number Diff line 
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

+31 −10
Original line number Diff line number Diff line
@@ -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

+17 −9
Original line number Diff line number Diff line
@@ -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

+5 −3
Original line number Diff line number Diff line
@@ -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(())

}