Unverified Commit 4d9e0b90 authored by Mark Hammond's avatar Mark Hammond Committed by GitHub
Browse files

Don't add builtins and composite types as named types (#2378) 

For apparently historical reasons, we added, eg, `Type::U8` as a named type with name
`u8`. This doesn't make sense for various reasons (these names are never actually
treated as names, and if they were, they'd never be correct for all bindings).

This is just a cleanup to help simplify our ComponentInterface. It did however
uncover two bugs:

* Type::iter_types neglected to include the CustomType builtin
* Type::get_name() returned None for a CallbackInterface even though they are named.
parent 6011f63c

uniffi_bindgen/src/interface/universe.rs

+7 −40
Original line number Diff line number Diff line
@@ -71,46 +71,13 @@ impl TypeUniverse { 
        if !self.all_known_types.contains(type_) {

            self.all_known_types.insert(type_.to_owned());

        }

        match type_ {

            Type::UInt8 => self.add_type_definition("u8", type_)?,

            Type::Int8 => self.add_type_definition("i8", type_)?,

            Type::UInt16 => self.add_type_definition("u16", type_)?,

            Type::Int16 => self.add_type_definition("i16", type_)?,

            Type::UInt32 => self.add_type_definition("i32", type_)?,

            Type::Int32 => self.add_type_definition("u32", type_)?,

            Type::UInt64 => self.add_type_definition("u64", type_)?,

            Type::Int64 => self.add_type_definition("i64", type_)?,

            Type::Float32 => self.add_type_definition("f32", type_)?,

            Type::Float64 => self.add_type_definition("f64", type_)?,

            Type::Boolean => self.add_type_definition("bool", type_)?,

            Type::String => self.add_type_definition("string", type_)?,

            Type::Bytes => self.add_type_definition("bytes", type_)?,

            Type::Timestamp => self.add_type_definition("timestamp", type_)?,

            Type::Duration => self.add_type_definition("duration", type_)?,

            Type::Object { name, .. }

            | Type::Record { name, .. }

            | Type::Enum { name, .. }

            | Type::CallbackInterface { name, .. }

            | Type::External { name, .. } => self.add_type_definition(name, type_)?,

            Type::Custom { name, builtin, .. } => {

                self.add_type_definition(name, type_)?;

                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)

                    .with_context(|| format!("adding optional type {inner_type:?}"))?;

            }

            Type::Map {

                key_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:?}"))?;

        // all sub-types, but we want to skip this type as we just added it above.

        for sub in type_.iter_nested_types() {

            self.add_known_type(sub)?;

        }

        if let Some(name) = type_.name() {

            self.add_type_definition(name, type_)

                .with_context(|| format!("adding named type {name}"))?;

        }

        Ok(())

    }

uniffi_meta/src/types.rs

+11 −3
Original line number Diff line number Diff line
@@ -127,8 +127,14 @@ pub enum Type { 
}



impl Type {

    // iterate over all types contained in the type, *including self*.

    pub fn iter_types(&self) -> TypeIterator<'_> {

        let nested_types = match self {

        Box::new(std::iter::once(self).chain(self.iter_nested_types()))

    }



    // iterate over all types contained in the type but *not including self*.

    pub fn iter_nested_types(&self) -> TypeIterator<'_> {

        match self {

            Type::Optional { inner_type } | Type::Sequence { inner_type } => {

                inner_type.iter_types()

            }
@@ -136,9 +142,9 @@ impl Type { 
                key_type,

                value_type,

            } => Box::new(key_type.iter_types().chain(value_type.iter_types())),

            Type::Custom { builtin, .. } => builtin.iter_types(),

            _ => Box::new(std::iter::empty()),

        };

        Box::new(std::iter::once(self).chain(nested_types))

        }

    }



    pub fn name(&self) -> Option<&str> {
@@ -148,6 +154,7 @@ impl Type { 
            Type::Enum { name, .. } => Some(name),

            Type::External { name, .. } => Some(name),

            Type::Custom { name, .. } => Some(name),

            Type::CallbackInterface { name, .. } => Some(name),

            _ => None,

        }

    }
@@ -159,6 +166,7 @@ impl Type { 
            Type::Enum { module_path, .. } => Some(module_path),

            Type::External { module_path, .. } => Some(module_path),

            Type::Custom { module_path, .. } => Some(module_path),

            Type::CallbackInterface { module_path, .. } => Some(module_path),

            _ => None,

        }

    }