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.

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

[Bugfix] Invalid generation of types with >1 generic parameters #1023

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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;
Copy link
Member

Choose a reason for hiding this comment

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

hrm, does that compile? :)

It would be better to bring in quote in dependency tree as this might break between releases...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could do that, good idea.


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() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice!

#[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());
}