Skip to content

Commit

Permalink
[Bugfix] Invalid generation of types with >1 generic parameters (#1023)
Browse files Browse the repository at this point in the history
* add original name match

* add unit test for generic params
  • Loading branch information
tadeohepperle authored Jun 23, 2023
1 parent 95c9bce commit 8413c4d
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 10 deletions.
6 changes: 5 additions & 1 deletion codegen/src/types/composite_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,11 @@ impl CompositeDefFields {
let mut unnamed_fields = Vec::new();

for field in fields {
let type_path = type_gen.resolve_field_type_path(field.ty.id, parent_type_params);
let type_path = type_gen.resolve_field_type_path(
field.ty.id,
parent_type_params,
field.type_name.as_deref(),
);
let field_type =
CompositeDefFieldType::new(field.ty.id, type_path, field.type_name.clone());

Expand Down
25 changes: 17 additions & 8 deletions codegen/src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,9 @@ impl<'a> TypeGenerator<'a> {
&self,
id: u32,
parent_type_params: &[TypeParameter],
original_name: Option<&str>,
) -> TypePath {
self.resolve_type_path_recurse(id, true, parent_type_params)
self.resolve_type_path_recurse(id, true, parent_type_params, original_name)
}

/// Get the type path for the given type identifier.
Expand All @@ -147,22 +148,25 @@ impl<'a> TypeGenerator<'a> {
///
/// If no type with the given id found in the type registry.
pub fn resolve_type_path(&self, id: u32) -> TypePath {
self.resolve_type_path_recurse(id, false, &[])
self.resolve_type_path_recurse(id, false, &[], None)
}

/// Visit each node in a possibly nested type definition to produce a type path.
///
/// e.g `Result<GenericStruct<NestedGenericStruct<T>>, String>`
///
/// if `original_name` is `Some(original_name)`, the resolved type needs to have the same `original_name`.
fn resolve_type_path_recurse(
&self,
id: u32,
is_field: bool,
parent_type_params: &[TypeParameter],
original_name: Option<&str>,
) -> TypePath {
if let Some(parent_type_param) = parent_type_params
.iter()
.find(|tp| tp.concrete_type_id == id)
{
if let Some(parent_type_param) = parent_type_params.iter().find(|tp| {
tp.concrete_type_id == id
&& original_name.map_or(true, |original_name| tp.original_name == original_name)
}) {
return TypePath::from_parameter(parent_type_param.clone());
}

Expand All @@ -181,7 +185,7 @@ impl<'a> TypeGenerator<'a> {
.type_params
.iter()
.filter_map(|f| {
f.ty.map(|f| self.resolve_type_path_recurse(f.id, false, parent_type_params))
f.ty.map(|f| self.resolve_type_path_recurse(f.id, false, parent_type_params, None))
})
.collect();

Expand All @@ -205,27 +209,30 @@ impl<'a> TypeGenerator<'a> {
arr.type_param.id,
false,
parent_type_params,
None,
)),
},
TypeDef::Sequence(seq) => TypePathType::Vec {
of: Box::new(self.resolve_type_path_recurse(
seq.type_param.id,
false,
parent_type_params,
None,
)),
},
TypeDef::Tuple(tuple) => TypePathType::Tuple {
elements: tuple
.fields
.iter()
.map(|f| self.resolve_type_path_recurse(f.id, false, parent_type_params))
.map(|f| self.resolve_type_path_recurse(f.id, false, parent_type_params, None))
.collect(),
},
TypeDef::Compact(compact) => TypePathType::Compact {
inner: Box::new(self.resolve_type_path_recurse(
compact.type_param.id,
false,
parent_type_params,
None,
)),
is_field,
crate_path: self.crate_path.clone(),
Expand All @@ -235,11 +242,13 @@ impl<'a> TypeGenerator<'a> {
bitseq.bit_order_type.id,
false,
parent_type_params,
None,
)),
bit_store_type: Box::new(self.resolve_type_path_recurse(
bitseq.bit_store_type.id,
false,
parent_type_params,
None,
)),
crate_path: self.crate_path.clone(),
},
Expand Down
75 changes: 74 additions & 1 deletion testing/integration-tests/src/codegen/codegen_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ use frame_metadata::{
v15::{ExtrinsicMetadata, RuntimeMetadataV15},
RuntimeMetadataPrefixed,
};
use scale_info::{meta_type, IntoPortable, TypeInfo};
use scale_info::{meta_type, IntoPortable, PortableRegistry, Registry, TypeInfo};
use subxt_codegen::{CratePath, DerivesRegistry, RuntimeGenerator, TypeSubstitutes};
use syn::__private::quote;

fn generate_runtime_interface_from_metadata(metadata: RuntimeMetadataPrefixed) -> String {
// Generate a runtime interface from the provided metadata.
Expand Down Expand Up @@ -144,3 +145,75 @@ fn generic_types_overwrite_each_other() {
// We do _not_ expect this to exist, since a generic is present on the type:
assert!(!interface.contains("DuplicateType2"));
}

#[test]
fn more_than_1_generic_parameters_work() {
#[allow(unused)]
#[derive(TypeInfo)]
struct Foo<T, U, V, W> {
a: T,
b: U,
c: V,
d: W,
}

#[allow(unused)]
#[derive(TypeInfo)]
struct Bar {
p: Foo<u32, u32, u64, u128>,
q: Foo<u8, u8, u8, u8>,
}

let mut registry = Registry::new();
registry.register_type(&meta_type::<Bar>());
let portable_types: PortableRegistry = registry.into();

let type_gen = subxt_codegen::TypeGenerator::new(
&portable_types,
"root",
Default::default(),
Default::default(),
CratePath::default(),
false,
);

let types = type_gen.generate_types_mod().unwrap();
let generated_mod = quote::quote!( #types);

let expected_mod = quote::quote! {
pub mod root {
use super::root;
pub mod integration_tests {
use super::root;
pub mod codegen {
use super::root;
pub mod codegen_tests {
use super::root;
pub struct Bar {
pub p: root::integration_tests::codegen::codegen_tests::Foo<
::core::primitive::u32,
::core::primitive::u32,
::core::primitive::u64,
::core::primitive::u128
>,
pub q: root::integration_tests::codegen::codegen_tests::Foo<
::core::primitive::u8,
::core::primitive::u8,
::core::primitive::u8,
::core::primitive::u8
>,
}
pub struct Foo<_0, _1, _2, _3> {
pub a: _0,
pub b: _1,
pub c: _2,
pub d: _3,
}
}
}
}
}
};

assert_eq!(generated_mod.to_string(), expected_mod.to_string());
}

0 comments on commit 8413c4d

Please sign in to comment.