Skip to content

Commit

Permalink
fix(idl-gen): Types with const generics produce invalid IDL (#515)
Browse files Browse the repository at this point in the history
* fix type names conflict with generic const params

* small improvements
  • Loading branch information
vobradovich authored Sep 5, 2024
1 parent e88e9ed commit aab0873
Show file tree
Hide file tree
Showing 7 changed files with 125 additions and 28 deletions.
68 changes: 62 additions & 6 deletions rs/idl-gen/src/type_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<const N: size>`
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:?}")));
}

Expand All @@ -236,7 +248,7 @@ impl ByPathTypeName {
})
}

fn possible_names(type_info: &Type<PortableForm>) -> impl Iterator<Item = String> + '_ {
fn possible_names_by_path(type_info: &Type<PortableForm>) -> impl Iterator<Item = String> + '_ {
let mut name = String::default();
type_info.path.segments.iter().rev().map(move |segment| {
name = segment.to_case(Case::Pascal) + &name;
Expand Down Expand Up @@ -709,6 +721,13 @@ mod tests {
field: T,
}

#[allow(dead_code)]
#[derive(TypeInfo)]
struct GenericConstStruct<const N: usize, const M: usize, T> {
field: [T; N],
field2: [T; M],
}

#[allow(dead_code)]
#[derive(TypeInfo)]
enum GenericEnum<T1, T2> {
Expand Down Expand Up @@ -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::<GenericConstStruct<8, 8, u8>>())
.id;
let n8_id_2 = registry
.register_type(&MetaType::new::<GenericConstStruct<8, 8, u8>>())
.id;
let n32_id = registry
.register_type(&MetaType::new::<GenericConstStruct<32, 8, u8>>())
.id;
let n256_id = registry
.register_type(&MetaType::new::<GenericConstStruct<256, 832, u8>>())
.id;
let n32u256_id = registry
.register_type(&MetaType::new::<GenericConstStruct<32, 8, U256>>())
.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"
);
}
}
17 changes: 12 additions & 5 deletions rs/idl-gen/tests/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ mod types {
pub p1: T,
}

#[derive(TypeInfo)]
pub struct GenericConstStruct<const N: usize> {
field: [u8; N],
}

#[derive(TypeInfo)]
pub enum GenericEnum<T1, T2> {
Variant1(T1),
Expand Down Expand Up @@ -62,6 +67,8 @@ mod meta_params {
p4: TupleStruct,
p5: GenericStruct<H256>,
p6: GenericStruct<String>,
p7: GenericConstStruct<8>,
p8: GenericConstStruct<32>,
}

#[derive(TypeInfo)]
Expand Down Expand Up @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
source: idlgen/tests/generator.rs
source: rs/idl-gen/tests/generator.rs
expression: generated_idl
---
type TupleStruct = struct {
Expand All @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -56,5 +64,3 @@ service {
ThatDone: struct { p1: str };
}
};


Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
source: idlgen/tests/generator.rs
source: rs/idl-gen/tests/generator.rs
expression: generated_idl
---
type TupleStruct = struct {
Expand All @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -68,5 +76,3 @@ service SomeService {
ThatDone: struct { p1: str };
}
};


Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
source: idl-gen/tests/generator.rs
source: rs/idl-gen/tests/generator.rs
expression: generated_idl
---
type TupleStruct = struct {
Expand All @@ -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,
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
source: idl-gen/tests/generator.rs
source: rs/idl-gen/tests/generator.rs
expression: generated_idl
---
type TupleStruct = struct {
Expand All @@ -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,
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
source: idlgen/tests/generator.rs
source: rs/idl-gen/tests/generator.rs
expression: generated_idl
---
type TupleStruct = struct {
Expand All @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -56,5 +64,3 @@ service {
ThatDone: struct { p1: str };
}
};


0 comments on commit aab0873

Please sign in to comment.