Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feature: Add type_name override to Describe proc macro #1671

Merged
merged 2 commits into from
Jan 8, 2024
Merged

feature: Add type_name override to Describe proc macro #1671

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b182d89
feature: Add type_name override to Describe proc macro
dhedey Jan 6, 2024
7379878
fix: Fix Describe syntax comparison tests
dhedey Jan 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions radix-engine-derive/src/scrypto_describe.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ mod tests {
quote! {
impl ::sbor::Describe<radix_engine_common::data::scrypto::ScryptoCustomTypeKind > for MyStruct {
const TYPE_ID: ::sbor::RustTypeId = ::sbor::RustTypeId::novel_with_code(
stringify!(MyStruct),
"MyStruct",
&[],
&#code_hash
);
fn type_data() -> ::sbor::TypeData<radix_engine_common::data::scrypto::ScryptoCustomTypeKind, ::sbor::RustTypeId> {
::sbor::TypeData::struct_with_named_fields(
stringify!(MyStruct),
"MyStruct",
::sbor::rust::vec![],
)
}
Expand All @@ -63,13 +63,13 @@ mod tests {
>
{
const TYPE_ID: ::sbor::RustTypeId = ::sbor::RustTypeId::novel_with_code(
stringify!(Thing),
"Thing",
&[<T>::TYPE_ID,],
&#code_hash
);
fn type_data() -> ::sbor::TypeData<radix_engine_common::data::scrypto::ScryptoCustomTypeKind, ::sbor::RustTypeId> {
::sbor::TypeData::struct_with_named_fields(
stringify!(Thing),
"Thing",
::sbor::rust::vec![
("field", <T as ::sbor::Describe<radix_engine_common::data::scrypto::ScryptoCustomTypeKind >>::TYPE_ID),
],
Expand Down Expand Up @@ -99,14 +99,14 @@ mod tests {
T: ::sbor::Describe<radix_engine_common::data::scrypto::ScryptoCustomTypeKind >
{
const TYPE_ID: ::sbor::RustTypeId = ::sbor::RustTypeId::novel_with_code(
stringify!(MyEnum),
"MyEnum",
&[<T>::TYPE_ID,],
&#code_hash
);
fn type_data() -> ::sbor::TypeData<radix_engine_common::data::scrypto::ScryptoCustomTypeKind, ::sbor::RustTypeId> {
use ::sbor::rust::borrow::ToOwned;
::sbor::TypeData::enum_variants(
stringify!(MyEnum),
"MyEnum",
:: sbor :: rust :: prelude :: indexmap ! [
0u8 => :: sbor :: TypeData :: struct_with_named_fields ("A", :: sbor :: rust :: vec ! [("named", < T as :: sbor :: Describe < radix_engine_common::data::scrypto::ScryptoCustomTypeKind >> :: TYPE_ID) ,] ,) ,
1u8 => :: sbor :: TypeData :: struct_with_unnamed_fields ("B", :: sbor :: rust :: vec ! [< String as :: sbor :: Describe < radix_engine_common::data::scrypto::ScryptoCustomTypeKind >> :: TYPE_ID ,] ,) ,
Expand Down
101 changes: 44 additions & 57 deletions sbor-derive-common/src/describe.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,17 @@ fn handle_transparent_describe(
};

// Replace the type name, unless opted out using the "transparent_name" tag
if !get_sbor_bool_value(&attrs, "transparent_name")? {
if !get_sbor_attribute_bool_value(&attrs, "transparent_name")? {
let type_name = get_sbor_attribute_string_value(&attrs, "type_name")?
.unwrap_or(ident.to_string());
type_data_content = quote! {
use ::sbor::rust::prelude::*;
#type_data_content
.with_name(Some(Cow::Borrowed(stringify!(#ident))))
.with_name(Some(Cow::Borrowed(#type_name)))
};
type_id = quote! {
::sbor::RustTypeId::novel_with_code(
stringify!(#ident),
#type_name,
&[#type_id],
&#code_hash
)
Expand Down Expand Up @@ -126,6 +128,27 @@ fn handle_normal_describe(
let (impl_generics, ty_generics, where_clause, child_types, custom_type_kind_generic) =
build_describe_generics(&generics, &attrs, context_custom_type_kind)?;

let type_name =
get_sbor_attribute_string_value(&attrs, "type_name")?.unwrap_or(ident.to_string());

let type_id = quote! {
::sbor::RustTypeId::novel_with_code(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for the future - we can consider just using core::any::TypeId::of::<Self> here, if we add where Self: 'static to the Describe trait (which should be fine) rust-lang/rust#72488

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although this is scary!

Type ids have some edges that this stabilization exposes to more contexts. It's possible for type ids to collide (but this is a bug). Since they can change between compiler versions, it's never valid to cast a type id to its underlying value.

And looks like it was actually un-stabilized as const - so perhaps we're good to wait for it to be improved... see discussion on rust-lang/rust#75923

#type_name,
// Here we really want to cause distinct types to have distinct hashes, whilst still supporting (most) recursive types.
// The code hash itself is pretty good for this, but if you allow generic types, it's not enough, as the same code can create
// different types depending on the generic types providing. Adding in the generic types' TYPE_IDs solves that issue.
//
// It's still technically possible to get a collision (by abusing type namespacing to have two types with identical code
// reference other types) but it's good enough - you're only shooting yourself in the food at that point.
//
// Note that it might seem possible to still hit issues with infinite recursion, if you pass a type as its own generic type parameter.
// EG (via a type alias B = A<B>), but these types won't come up in practice because they require an infinite generic depth
// which the compiler will throw out for other reasons.
&[#(<#child_types>::TYPE_ID,)*],
&#code_hash
)
};

let output = match data {
Data::Struct(s) => match &s.fields {
syn::Fields::Named(FieldsNamed { .. }) => {
Expand All @@ -137,25 +160,11 @@ fn handle_normal_describe(
let unique_field_types: Vec<_> = get_unique_types(&unskipped_field_types);
quote! {
impl #impl_generics ::sbor::Describe <#custom_type_kind_generic> for #ident #ty_generics #where_clause {
const TYPE_ID: ::sbor::RustTypeId = ::sbor::RustTypeId::novel_with_code(
stringify!(#ident),
// Here we really want to cause distinct types to have distinct hashes, whilst still supporting (most) recursive types.
// The code hash itself is pretty good for this, but if you allow generic types, it's not enough, as the same code can create
// different types depending on the generic types providing. Adding in the generic types' TYPE_IDs solves that issue.
//
// It's still technically possible to get a collision (by abusing type namespacing to have two types with identical code
// reference other types) but it's good enough - you're only shooting yourself in the food at that point.
//
// Note that it might seem possible to still hit issues with infinite recursion, if you pass a type as its own generic type parameter.
// EG (via a type alias B = A<B>), but these types won't come up in practice because they require an infinite generic depth
// which the compiler will throw out for other reasons.
&[#(<#child_types>::TYPE_ID,)*],
&#code_hash
);
const TYPE_ID: ::sbor::RustTypeId = #type_id;

fn type_data() -> ::sbor::TypeData<#custom_type_kind_generic, ::sbor::RustTypeId> {
::sbor::TypeData::struct_with_named_fields(
stringify!(#ident),
#type_name,
::sbor::rust::vec![
#((#unskipped_field_name_strings, <#unskipped_field_types as ::sbor::Describe<#custom_type_kind_generic>>::TYPE_ID),)*
],
Expand All @@ -177,25 +186,11 @@ fn handle_normal_describe(

quote! {
impl #impl_generics ::sbor::Describe <#custom_type_kind_generic> for #ident #ty_generics #where_clause {
const TYPE_ID: ::sbor::RustTypeId = ::sbor::RustTypeId::novel_with_code(
stringify!(#ident),
// Here we really want to cause distinct types to have distinct hashes, whilst still supporting (most) recursive types.
// The code hash itself is pretty good for this, but if you allow generic types, it's not enough, as the same code can create
// different types depending on the generic types providing. Adding in the generic types' TYPE_IDs solves that issue.
//
// It's still technically possible to get a collision (by abusing type namespacing to have two types with identical code
// reference other types) but it's good enough - you're only shooting yourself in the food at that point.
//
// Note that it might seem possible to still hit issues with infinite recursion, if you pass a type as its own generic type parameter.
// EG (via a type alias B = A<B>), but these types won't come up in practice because they require an infinite generic depth
// which the compiler will throw out for other reasons.
&[#(<#child_types>::TYPE_ID,)*],
&#code_hash
);
const TYPE_ID: ::sbor::RustTypeId = #type_id;

fn type_data() -> ::sbor::TypeData<#custom_type_kind_generic, ::sbor::RustTypeId> {
::sbor::TypeData::struct_with_unnamed_fields(
stringify!(#ident),
#type_name,
::sbor::rust::vec![
#(<#unskipped_field_types as ::sbor::Describe<#custom_type_kind_generic>>::TYPE_ID,)*
],
Expand All @@ -211,14 +206,10 @@ fn handle_normal_describe(
syn::Fields::Unit => {
quote! {
impl #impl_generics ::sbor::Describe <#custom_type_kind_generic> for #ident #ty_generics #where_clause {
const TYPE_ID: ::sbor::RustTypeId = ::sbor::RustTypeId::novel_with_code(
stringify!(#ident),
&[#(<#child_types>::TYPE_ID,)*],
&#code_hash
);
const TYPE_ID: ::sbor::RustTypeId = #type_id;

fn type_data() -> ::sbor::TypeData<#custom_type_kind_generic, ::sbor::RustTypeId> {
::sbor::TypeData::struct_with_unit_fields(stringify!(#ident))
::sbor::TypeData::struct_with_unit_fields(#type_name)
}
}
}
Expand Down Expand Up @@ -278,16 +269,12 @@ fn handle_normal_describe(

quote! {
impl #impl_generics ::sbor::Describe <#custom_type_kind_generic> for #ident #ty_generics #where_clause {
const TYPE_ID: ::sbor::RustTypeId = ::sbor::RustTypeId::novel_with_code(
stringify!(#ident),
&[#(<#child_types>::TYPE_ID,)*],
&#code_hash
);
const TYPE_ID: ::sbor::RustTypeId = #type_id;

fn type_data() -> ::sbor::TypeData<#custom_type_kind_generic, ::sbor::RustTypeId> {
use ::sbor::rust::borrow::ToOwned;
::sbor::TypeData::enum_variants(
stringify!(#ident),
#type_name,
::sbor::rust::prelude::indexmap![
#(#variant_discriminators => #variant_type_data,)*
],
Expand Down Expand Up @@ -330,14 +317,14 @@ mod tests {
quote! {
impl <C: ::sbor::CustomTypeKind<::sbor::RustTypeId> > ::sbor::Describe<C> for Test {
const TYPE_ID: ::sbor::RustTypeId = ::sbor::RustTypeId::novel_with_code(
stringify!(Test),
"Test",
&[],
&#code_hash
);

fn type_data() -> ::sbor::TypeData <C, ::sbor::RustTypeId> {
::sbor::TypeData::struct_with_named_fields(
stringify!(Test),
"Test",
::sbor::rust::vec![
("a", <u32 as ::sbor::Describe<C>>::TYPE_ID),
("b", <Vec<u8> as ::sbor::Describe<C>>::TYPE_ID),
Expand Down Expand Up @@ -372,7 +359,7 @@ mod tests {
for Test
{
const TYPE_ID: ::sbor::RustTypeId = ::sbor::RustTypeId::novel_with_code(
stringify!(Test),
"Test",
&[],
&#code_hash
);
Expand All @@ -381,7 +368,7 @@ mod tests {
radix_engine_interface::data::ScryptoCustomTypeKind<::sbor::RustTypeId>,
::sbor::RustTypeId> {
::sbor::TypeData::struct_with_named_fields(
stringify!(Test),
"Test",
::sbor::rust::vec![
(
"a",
Expand Down Expand Up @@ -428,14 +415,14 @@ mod tests {
quote! {
impl <C: ::sbor::CustomTypeKind<::sbor::RustTypeId> > ::sbor::Describe<C> for Test {
const TYPE_ID: ::sbor::RustTypeId = ::sbor::RustTypeId::novel_with_code(
stringify!(Test),
"Test",
&[],
&#code_hash
);

fn type_data() -> ::sbor::TypeData <C, ::sbor::RustTypeId> {
::sbor::TypeData::struct_with_unnamed_fields(
stringify!(Test),
"Test",
::sbor::rust::vec![
<u32 as ::sbor::Describe<C>>::TYPE_ID,
<Vec<u8> as ::sbor::Describe<C>>::TYPE_ID,
Expand Down Expand Up @@ -464,13 +451,13 @@ mod tests {
quote! {
impl <C: ::sbor::CustomTypeKind<::sbor::RustTypeId> > ::sbor::Describe<C> for Test {
const TYPE_ID: ::sbor::RustTypeId = ::sbor::RustTypeId::novel_with_code(
stringify!(Test),
"Test",
&[],
&#code_hash
);

fn type_data() -> ::sbor::TypeData <C, ::sbor::RustTypeId> {
::sbor::TypeData::struct_with_unit_fields(stringify!(Test))
::sbor::TypeData::struct_with_unit_fields("Test")
}
}
},
Expand All @@ -493,15 +480,15 @@ mod tests {
T2: ::sbor::Describe<C>
{
const TYPE_ID: ::sbor::RustTypeId = ::sbor::RustTypeId::novel_with_code(
stringify!(Test),
"Test",
&[<T>::TYPE_ID, <T2>::TYPE_ID,],
&#code_hash
);

fn type_data() -> ::sbor::TypeData <C, ::sbor::RustTypeId> {
use ::sbor::rust::borrow::ToOwned;
::sbor::TypeData::enum_variants(
stringify!(Test),
"Test",
::sbor::rust::prelude::indexmap![
0u8 => ::sbor::TypeData::struct_with_unit_fields("A"),
1u8 => ::sbor::TypeData::struct_with_unnamed_fields(
Expand Down
19 changes: 9 additions & 10 deletions sbor-derive-common/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ pub fn get_variant_discriminator_mapping(
}

let use_repr_discriminators =
get_sbor_attribute_boolean_value(enum_attributes, "use_repr_discriminators")?;
get_sbor_attribute_bool_value(enum_attributes, "use_repr_discriminators")?;
let mut variant_ids: BTreeMap<usize, VariantValue> = BTreeMap::new();

for (i, variant) in variants.iter().enumerate() {
Expand Down Expand Up @@ -300,19 +300,18 @@ fn parse_u8_from_literal(literal: &Lit) -> Option<u8> {
}
}

fn get_sbor_attribute_string_value(
pub fn get_sbor_attribute_string_value(
attributes: &[Attribute],
field_name: &str,
attribute_name: &str,
) -> Result<Option<String>> {
extract_sbor_typed_attributes(attributes)?.get_string_value(&field_name)
extract_sbor_typed_attributes(attributes)?.get_string_value(attribute_name)
}

fn get_sbor_attribute_boolean_value(attributes: &[Attribute], field_name: &str) -> Result<bool> {
extract_sbor_typed_attributes(attributes)?.get_bool_value(&field_name)
}

pub fn get_sbor_bool_value(attributes: &[Attribute], attribute_name: &str) -> Result<bool> {
extract_sbor_typed_attributes(&attributes)?.get_bool_value(attribute_name)
pub fn get_sbor_attribute_bool_value(
attributes: &[Attribute],
attribute_name: &str,
) -> Result<bool> {
extract_sbor_typed_attributes(attributes)?.get_bool_value(attribute_name)
}

pub fn is_categorize_skipped(f: &Field) -> Result<bool> {
Expand Down
17 changes: 17 additions & 0 deletions sbor-tests/tests/schema.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ use sbor::*;
#[derive(Sbor)]
pub struct UnitStruct;

#[derive(Sbor)]
#[sbor(type_name = "UnitStructRenamed2")]
pub struct UnitStructRenamed;

#[derive(Sbor)]
pub struct BasicSample {
pub a: (),
Expand Down Expand Up @@ -229,3 +233,16 @@ fn create_recursive_schema_works_correctly() {
assert_eq!(metadata.get_name().unwrap(), "IndirectRecursive1");
assert!(schema.v1().validate().is_ok());
}

#[test]
fn test_type_name_works_correctly() {
// Most of this test is checking that such recursive schemas can: (A) happily compile and (B) don't panic when a schema is generated
let (type_id, schema) =
generate_full_schema_from_single_type::<UnitStructRenamed, NoCustomSchema>();

// The original type should be the first type in the schema
assert!(matches!(type_id, LocalTypeId::SchemaLocalIndex(0)));
let metadata = schema.v1().resolve_type_metadata(type_id).unwrap();
assert_eq!(metadata.get_name().unwrap(), "UnitStructRenamed2");
assert!(schema.v1().validate().is_ok());
}
7 changes: 7 additions & 0 deletions sbor-tests/tests/transparent.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ pub struct TestStructNamed {
pub state: u32,
}

#[derive(Sbor, PartialEq, Eq, Debug)]
#[sbor(transparent, type_name = "TestStructRenamed2")]
pub struct TestStructRenamed {
pub state: u32,
}

#[derive(Sbor, PartialEq, Eq, Debug)]
#[sbor(transparent)]
pub struct TestStructUnnamed(u32);
Expand Down Expand Up @@ -134,6 +140,7 @@ fn decode_is_correct() {
fn describe_is_correct() {
// With inner u32
check_identical_types::<TestStructNamed, u32>("TestStructNamed");
check_identical_types::<TestStructRenamed, u32>("TestStructRenamed2");
check_identical_types::<TestStructUnnamed, u32>("TestStructUnnamed");
check_identical_types::<TestStruct<u32>, u32>("TestStruct");

Expand Down
Loading