From aab08733ee9ccdb809dc9b29c57dd411b2b917b1 Mon Sep 17 00:00:00 2001 From: Vadim Obradovich Date: Thu, 5 Sep 2024 16:40:20 +0200 Subject: [PATCH] fix(idl-gen): Types with const generics produce invalid IDL (#515) * fix type names conflict with generic const params * small improvements --- rs/idl-gen/src/type_names.rs | 68 +++++++++++++++++-- rs/idl-gen/tests/generator.rs | 17 +++-- ...r__program_idl_works_with_empty_ctors.snap | 14 ++-- ...gram_idl_works_with_multiple_services.snap | 16 +++-- ...rogram_idl_works_with_non_empty_ctors.snap | 12 +++- ..._service_idl_works_with_base_services.snap | 12 +++- ...erator__service_idl_works_with_basics.snap | 14 ++-- 7 files changed, 125 insertions(+), 28 deletions(-) diff --git a/rs/idl-gen/src/type_names.rs b/rs/idl-gen/src/type_names.rs index ada3107e..8dfb05db 100644 --- a/rs/idl-gen/src/type_names.rs +++ b/rs/idl-gen/src/type_names.rs @@ -217,16 +217,28 @@ impl ByPathTypeName { }, )?; - let possible_names = - Self::possible_names(type_info).fold(Vec::new(), |mut possible_names, name| { + let mut possible_names = Self::possible_names_by_path(type_info).fold( + Vec::with_capacity(type_info.path.segments.len() + 1), + |mut possible_names, name| { possible_names.push((name.clone(), type_params.0.clone())); let name_ref_count = by_path_type_names - .entry((name, type_params.0.clone())) + .entry((name.clone(), type_params.0.clone())) .or_default(); *name_ref_count += 1; possible_names - }); - if possible_names.is_empty() { + }, + ); + if let Some(first_name) = possible_names.first() { + // add numbered type name like `TypeName1`, `TypeName2` as last name + // to solve name conflict with const generic parameters `` + let name_ref_count = by_path_type_names.get(first_name).unwrap_or(&0); + let name = format!("{}{}", first_name.0, name_ref_count); + possible_names.push((name.clone(), first_name.1.clone())); + let name_ref_count = by_path_type_names + .entry((name.clone(), type_params.0.clone())) + .or_default(); + *name_ref_count += 1; + } else { return Err(Error::TypeIsUnsupported(format!("{type_info:?}"))); } @@ -236,7 +248,7 @@ impl ByPathTypeName { }) } - fn possible_names(type_info: &Type) -> impl Iterator + '_ { + fn possible_names_by_path(type_info: &Type) -> impl Iterator + '_ { let mut name = String::default(); type_info.path.segments.iter().rev().map(move |segment| { name = segment.to_case(Case::Pascal) + &name; @@ -709,6 +721,13 @@ mod tests { field: T, } + #[allow(dead_code)] + #[derive(TypeInfo)] + struct GenericConstStruct { + field: [T; N], + field2: [T; M], + } + #[allow(dead_code)] #[derive(TypeInfo)] enum GenericEnum { @@ -1017,4 +1036,41 @@ mod tests { fn nonzero_u256_type_name_resolution_works() { type_name_resolution_works!(NonZeroU256, nat256); } + + #[test] + fn generic_const_struct_type_name_resolution_works() { + let mut registry = Registry::new(); + let n8_id = registry + .register_type(&MetaType::new::>()) + .id; + let n8_id_2 = registry + .register_type(&MetaType::new::>()) + .id; + let n32_id = registry + .register_type(&MetaType::new::>()) + .id; + let n256_id = registry + .register_type(&MetaType::new::>()) + .id; + let n32u256_id = registry + .register_type(&MetaType::new::>()) + .id; + let portable_registry = PortableRegistry::from(registry); + + let type_names = resolve(portable_registry.types.iter()).unwrap(); + + assert_eq!(n8_id, n8_id_2); + assert_ne!(n8_id, n32_id); + assert_ne!(n8_id, n256_id); + assert_eq!(type_names.get(&n8_id).unwrap(), "GenericConstStruct1ForU8"); + assert_eq!(type_names.get(&n32_id).unwrap(), "GenericConstStruct2ForU8"); + assert_eq!( + type_names.get(&n256_id).unwrap(), + "GenericConstStruct3ForU8" + ); + assert_eq!( + type_names.get(&n32u256_id).unwrap(), + "GenericConstStructForU256" + ); + } } diff --git a/rs/idl-gen/tests/generator.rs b/rs/idl-gen/tests/generator.rs index 486902f6..446a4ed6 100644 --- a/rs/idl-gen/tests/generator.rs +++ b/rs/idl-gen/tests/generator.rs @@ -16,6 +16,11 @@ mod types { pub p1: T, } + #[derive(TypeInfo)] + pub struct GenericConstStruct { + field: [u8; N], + } + #[derive(TypeInfo)] pub enum GenericEnum { Variant1(T1), @@ -62,6 +67,8 @@ mod meta_params { p4: TupleStruct, p5: GenericStruct, p6: GenericStruct, + p7: GenericConstStruct<8>, + p8: GenericConstStruct<32>, } #[derive(TypeInfo)] @@ -259,7 +266,7 @@ fn program_idl_works_with_empty_ctors() { assert!(generated_idl_program.ctor().is_none()); assert_eq!(generated_idl_program.services().len(), 1); assert_eq!(generated_idl_program.services()[0].funcs().len(), 4); - assert_eq!(generated_idl_program.types().len(), 8); + assert_eq!(generated_idl_program.types().len(), 10); } #[test] @@ -274,7 +281,7 @@ fn program_idl_works_with_non_empty_ctors() { assert_eq!(generated_idl_program.ctor().unwrap().funcs().len(), 2); assert_eq!(generated_idl_program.services().len(), 1); assert_eq!(generated_idl_program.services()[0].funcs().len(), 4); - assert_eq!(generated_idl_program.types().len(), 8); + assert_eq!(generated_idl_program.types().len(), 10); } #[test] @@ -292,7 +299,7 @@ fn program_idl_works_with_multiple_services() { assert_eq!(generated_idl_program.services()[0].funcs().len(), 4); assert_eq!(generated_idl_program.services()[1].name(), "SomeService"); assert_eq!(generated_idl_program.services()[1].funcs().len(), 4); - assert_eq!(generated_idl_program.types().len(), 8); + assert_eq!(generated_idl_program.types().len(), 10); } #[test] @@ -307,7 +314,7 @@ fn service_idl_works_with_basics() { assert!(generated_idl_program.ctor().is_none()); assert_eq!(generated_idl_program.services().len(), 1); assert_eq!(generated_idl_program.services()[0].funcs().len(), 4); - assert_eq!(generated_idl_program.types().len(), 8); + assert_eq!(generated_idl_program.types().len(), 10); } #[test] @@ -330,7 +337,7 @@ fn service_idl_works_with_base_services() { assert!(generated_idl_program.ctor().is_none()); assert_eq!(generated_idl_program.services().len(), 1); assert_eq!(generated_idl_program.services()[0].funcs().len(), 6); - assert_eq!(generated_idl_program.types().len(), 8); + assert_eq!(generated_idl_program.types().len(), 10); } #[test] diff --git a/rs/idl-gen/tests/snapshots/generator__program_idl_works_with_empty_ctors.snap b/rs/idl-gen/tests/snapshots/generator__program_idl_works_with_empty_ctors.snap index 2eda5fce..6a24ad5e 100644 --- a/rs/idl-gen/tests/snapshots/generator__program_idl_works_with_empty_ctors.snap +++ b/rs/idl-gen/tests/snapshots/generator__program_idl_works_with_empty_ctors.snap @@ -1,5 +1,5 @@ --- -source: idlgen/tests/generator.rs +source: rs/idl-gen/tests/generator.rs expression: generated_idl --- type TupleStruct = struct { @@ -14,6 +14,14 @@ type GenericStructForStr = struct { p1: str, }; +type GenericConstStruct1 = struct { + field: [u8, 8], +}; + +type GenericConstStruct2 = struct { + field: [u8, 32], +}; + type DoThatParam = struct { p1: u32, p2: str, @@ -46,7 +54,7 @@ type ThatParam = struct { }; service { - DoThis : (p1: u32, p2: str, p3: struct { opt str, u8 }, p4: TupleStruct, p5: GenericStructForH256, p6: GenericStructForStr) -> str; + DoThis : (p1: u32, p2: str, p3: struct { opt str, u8 }, p4: TupleStruct, p5: GenericStructForH256, p6: GenericStructForStr, p7: GenericConstStruct1, p8: GenericConstStruct2) -> str; DoThat : (par1: DoThatParam) -> result (struct { str, u32 }, struct { str }); query This : (p1: u32, p2: str, p3: struct { opt str, u8 }, p4: TupleStruct, p5: GenericEnumForBoolAndU32) -> result (struct { str, u32 }, str); query That : (pr1: ThatParam) -> str; @@ -56,5 +64,3 @@ service { ThatDone: struct { p1: str }; } }; - - diff --git a/rs/idl-gen/tests/snapshots/generator__program_idl_works_with_multiple_services.snap b/rs/idl-gen/tests/snapshots/generator__program_idl_works_with_multiple_services.snap index d4ed876b..d11b1ef4 100644 --- a/rs/idl-gen/tests/snapshots/generator__program_idl_works_with_multiple_services.snap +++ b/rs/idl-gen/tests/snapshots/generator__program_idl_works_with_multiple_services.snap @@ -1,5 +1,5 @@ --- -source: idlgen/tests/generator.rs +source: rs/idl-gen/tests/generator.rs expression: generated_idl --- type TupleStruct = struct { @@ -14,6 +14,14 @@ type GenericStructForStr = struct { p1: str, }; +type GenericConstStruct1 = struct { + field: [u8, 8], +}; + +type GenericConstStruct2 = struct { + field: [u8, 32], +}; + type DoThatParam = struct { p1: u32, p2: str, @@ -46,7 +54,7 @@ type ThatParam = struct { }; service { - DoThis : (p1: u32, p2: str, p3: struct { opt str, u8 }, p4: TupleStruct, p5: GenericStructForH256, p6: GenericStructForStr) -> str; + DoThis : (p1: u32, p2: str, p3: struct { opt str, u8 }, p4: TupleStruct, p5: GenericStructForH256, p6: GenericStructForStr, p7: GenericConstStruct1, p8: GenericConstStruct2) -> str; DoThat : (par1: DoThatParam) -> result (struct { str, u32 }, struct { str }); query This : (p1: u32, p2: str, p3: struct { opt str, u8 }, p4: TupleStruct, p5: GenericEnumForBoolAndU32) -> result (struct { str, u32 }, str); query That : (pr1: ThatParam) -> str; @@ -58,7 +66,7 @@ service { }; service SomeService { - DoThis : (p1: u32, p2: str, p3: struct { opt str, u8 }, p4: TupleStruct, p5: GenericStructForH256, p6: GenericStructForStr) -> str; + DoThis : (p1: u32, p2: str, p3: struct { opt str, u8 }, p4: TupleStruct, p5: GenericStructForH256, p6: GenericStructForStr, p7: GenericConstStruct1, p8: GenericConstStruct2) -> str; DoThat : (par1: DoThatParam) -> result (struct { str, u32 }, struct { str }); query This : (p1: u32, p2: str, p3: struct { opt str, u8 }, p4: TupleStruct, p5: GenericEnumForBoolAndU32) -> result (struct { str, u32 }, str); query That : (pr1: ThatParam) -> str; @@ -68,5 +76,3 @@ service SomeService { ThatDone: struct { p1: str }; } }; - - diff --git a/rs/idl-gen/tests/snapshots/generator__program_idl_works_with_non_empty_ctors.snap b/rs/idl-gen/tests/snapshots/generator__program_idl_works_with_non_empty_ctors.snap index 76bba35d..7d09cb7a 100644 --- a/rs/idl-gen/tests/snapshots/generator__program_idl_works_with_non_empty_ctors.snap +++ b/rs/idl-gen/tests/snapshots/generator__program_idl_works_with_non_empty_ctors.snap @@ -1,5 +1,5 @@ --- -source: idl-gen/tests/generator.rs +source: rs/idl-gen/tests/generator.rs expression: generated_idl --- type TupleStruct = struct { @@ -14,6 +14,14 @@ type GenericStructForStr = struct { p1: str, }; +type GenericConstStruct1 = struct { + field: [u8, 8], +}; + +type GenericConstStruct2 = struct { + field: [u8, 32], +}; + type DoThatParam = struct { p1: u32, p2: str, @@ -51,7 +59,7 @@ constructor { }; service { - DoThis : (p1: u32, p2: str, p3: struct { opt str, u8 }, p4: TupleStruct, p5: GenericStructForH256, p6: GenericStructForStr) -> str; + DoThis : (p1: u32, p2: str, p3: struct { opt str, u8 }, p4: TupleStruct, p5: GenericStructForH256, p6: GenericStructForStr, p7: GenericConstStruct1, p8: GenericConstStruct2) -> str; DoThat : (par1: DoThatParam) -> result (struct { str, u32 }, struct { str }); query This : (p1: u32, p2: str, p3: struct { opt str, u8 }, p4: TupleStruct, p5: GenericEnumForBoolAndU32) -> result (struct { str, u32 }, str); query That : (pr1: ThatParam) -> str; diff --git a/rs/idl-gen/tests/snapshots/generator__service_idl_works_with_base_services.snap b/rs/idl-gen/tests/snapshots/generator__service_idl_works_with_base_services.snap index cfc8b76b..3afa1b96 100644 --- a/rs/idl-gen/tests/snapshots/generator__service_idl_works_with_base_services.snap +++ b/rs/idl-gen/tests/snapshots/generator__service_idl_works_with_base_services.snap @@ -1,5 +1,5 @@ --- -source: idl-gen/tests/generator.rs +source: rs/idl-gen/tests/generator.rs expression: generated_idl --- type TupleStruct = struct { @@ -14,6 +14,14 @@ type GenericStructForStr = struct { p1: str, }; +type GenericConstStruct1 = struct { + field: [u8, 8], +}; + +type GenericConstStruct2 = struct { + field: [u8, 32], +}; + type DoThatParam = struct { p1: u32, p2: str, @@ -46,7 +54,7 @@ type ThatParam = struct { }; service { - DoThis : (p1: u32, p2: str, p3: struct { opt str, u8 }, p4: TupleStruct, p5: GenericStructForH256, p6: GenericStructForStr) -> str; + DoThis : (p1: u32, p2: str, p3: struct { opt str, u8 }, p4: TupleStruct, p5: GenericStructForH256, p6: GenericStructForStr, p7: GenericConstStruct1, p8: GenericConstStruct2) -> str; DoThat : (par1: DoThatParam) -> result (struct { str, u32 }, struct { str }); DoThatBase : (p1: str) -> str; query This : (p1: u32, p2: str, p3: struct { opt str, u8 }, p4: TupleStruct, p5: GenericEnumForBoolAndU32) -> result (struct { str, u32 }, str); diff --git a/rs/idl-gen/tests/snapshots/generator__service_idl_works_with_basics.snap b/rs/idl-gen/tests/snapshots/generator__service_idl_works_with_basics.snap index 2eda5fce..6a24ad5e 100644 --- a/rs/idl-gen/tests/snapshots/generator__service_idl_works_with_basics.snap +++ b/rs/idl-gen/tests/snapshots/generator__service_idl_works_with_basics.snap @@ -1,5 +1,5 @@ --- -source: idlgen/tests/generator.rs +source: rs/idl-gen/tests/generator.rs expression: generated_idl --- type TupleStruct = struct { @@ -14,6 +14,14 @@ type GenericStructForStr = struct { p1: str, }; +type GenericConstStruct1 = struct { + field: [u8, 8], +}; + +type GenericConstStruct2 = struct { + field: [u8, 32], +}; + type DoThatParam = struct { p1: u32, p2: str, @@ -46,7 +54,7 @@ type ThatParam = struct { }; service { - DoThis : (p1: u32, p2: str, p3: struct { opt str, u8 }, p4: TupleStruct, p5: GenericStructForH256, p6: GenericStructForStr) -> str; + DoThis : (p1: u32, p2: str, p3: struct { opt str, u8 }, p4: TupleStruct, p5: GenericStructForH256, p6: GenericStructForStr, p7: GenericConstStruct1, p8: GenericConstStruct2) -> str; DoThat : (par1: DoThatParam) -> result (struct { str, u32 }, struct { str }); query This : (p1: u32, p2: str, p3: struct { opt str, u8 }, p4: TupleStruct, p5: GenericEnumForBoolAndU32) -> result (struct { str, u32 }, str); query That : (pr1: ThatParam) -> str; @@ -56,5 +64,3 @@ service { ThatDone: struct { p1: str }; } }; - -