Fix codegen for codec::Compact as type parameters (#651)

* Add failing test for compact generic parameter

* WIP refactor type generation

* Fmt

* Remove deprecated rustfmt optionns

* Remove license template path

* Update parent type parameter visitor

* Introduce different methods for generating a type path for a field

* Add comment

* Fix weights refs

* Add extra compact test cases

* Fmt

.rustfmt.toml

@@ -9,7 +9,6 @@ format_code_in_doc_comments = false
 comment_width = 80
 normalize_comments = true # changed
 normalize_doc_attributes = false
-license_template_path = "FILE_TEMPLATE" # changed
 format_strings = false
 format_macro_matchers = false
 format_macro_bodies = true
@@ -57,8 +56,6 @@ skip_children = false
 hide_parse_errors = false
 error_on_line_overflow = false
 error_on_unformatted = false
-report_todo = "Always"
-report_fixme = "Always"
 ignore = []
 
 # Below are `rustfmt` internal settings

codegen/src/api/constants.rs

@@ -58,7 +58,7 @@ pub fn generate_constants(
         let constant_hash = subxt_metadata::get_constant_hash(metadata, pallet_name, constant_name)
             .unwrap_or_else(|_| abort_call_site!("Metadata information for the constant {}_{} could not be found", pallet_name, constant_name));
 
-        let return_ty = type_gen.resolve_type_path(constant.ty.id(), &[]);
+        let return_ty = type_gen.resolve_type_path(constant.ty.id());
         let docs = &constant.docs;
 
         quote! {

codegen/src/api/events.rs

@@ -69,7 +69,7 @@ pub fn generate_events(
             }
         }
     });
-    let event_type = type_gen.resolve_type_path(event.ty.id(), &[]);
+    let event_type = type_gen.resolve_type_path(event.ty.id());
     let event_ty = type_gen.resolve_type(event.ty.id());
     let docs = event_ty.docs();
 

codegen/src/api/storage.rs

@@ -101,7 +101,7 @@ fn generate_storage_entry_fns(
                         .enumerate()
                         .map(|(i, f)| {
                             let field_name = format_ident!("_{}", syn::Index::from(i));
-                            let field_type = type_gen.resolve_type_path(f.id(), &[]);
+                            let field_type = type_gen.resolve_type_path(f.id());
                             (field_name, field_type)
                         })
                         .collect::<Vec<_>>();
@@ -142,7 +142,7 @@ fn generate_storage_entry_fns(
                     (fields, key_impl)
                 }
                 _ => {
-                    let ty_path = type_gen.resolve_type_path(key.id(), &[]);
+                    let ty_path = type_gen.resolve_type_path(key.id());
                     let fields = vec![(format_ident!("_0"), ty_path)];
                     let hasher = hashers.get(0).unwrap_or_else(|| {
                         abort_call_site!("No hasher found for single key")
@@ -173,7 +173,7 @@ fn generate_storage_entry_fns(
         StorageEntryType::Plain(ref ty) => ty,
         StorageEntryType::Map { ref value, .. } => value,
     };
-    let storage_entry_value_ty = type_gen.resolve_type_path(storage_entry_ty.id(), &[]);
+    let storage_entry_value_ty = type_gen.resolve_type_path(storage_entry_ty.id());
 
     let docs = &storage_entry.docs;
     let docs_token = quote! { #( #[doc = #docs ] )* };

codegen/src/types/composite_def.rs

@@ -190,7 +190,7 @@ impl CompositeDefFields {
 
         for field in fields {
             let type_path =
-                type_gen.resolve_type_path(field.ty().id(), parent_type_params);
+                type_gen.resolve_field_type_path(field.ty().id(), parent_type_params);
             let field_type = CompositeDefFieldType::new(
                 field.ty().id(),
                 type_path,

codegen/src/types/mod.rs

@@ -46,7 +46,6 @@ pub use self::{
     type_path::{
         TypeParameter,
         TypePath,
-        TypePathSubstitute,
         TypePathType,
     },
 };
@@ -145,13 +144,47 @@ impl<'a> TypeGenerator<'a> {
             .clone()
     }
 
+    /// Get the type path for a field of a struct or an enum variant, providing any generic
+    /// type parameters from the containing type. This is for identifying where a generic type
+    /// parameter is used in a field type e.g.
+    ///
+    /// ```rust
+    /// struct S<T> {
+    ///     a: T, // `T` is the "parent" type param from the containing type.
+    ///     b: Vec<Option<T>>, // nested use of generic type param `T`.
+    /// }
+    /// ```
+    ///
+    /// This allows generating the correct generic field type paths.
+    ///
     /// # Panics
     ///
     /// If no type with the given id found in the type registry.
-    pub fn resolve_type_path(
+    pub fn resolve_field_type_path(
         &self,
         id: u32,
         parent_type_params: &[TypeParameter],
+    ) -> TypePath {
+        self.resolve_type_path_recurse(id, true, parent_type_params)
+    }
+
+    /// Get the type path for the given type identifier.
+    ///
+    /// # Panics
+    ///
+    /// 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, &[])
+    }
+
+    /// Visit each node in a possibly nested type definition to produce a type path.
+    ///
+    /// e.g `Result<GenericStruct<NestedGenericStruct<T>>, String>`
+    fn resolve_type_path_recurse(
+        &self,
+        id: u32,
+        is_field: bool,
+        parent_type_params: &[TypeParameter],
     ) -> TypePath {
         if let Some(parent_type_param) = parent_type_params
             .iter()
@@ -171,40 +204,100 @@ impl<'a> TypeGenerator<'a> {
             )
         }
 
-        let params_type_ids = match ty.type_def() {
+        let params = ty
-            TypeDef::Array(arr) => vec![arr.type_param().id()],
+            .type_params()
-            TypeDef::Sequence(seq) => vec![seq.type_param().id()],
+            .iter()
-            TypeDef::Tuple(tuple) => tuple.fields().iter().map(|f| f.id()).collect(),
+            .filter_map(|f| {
-            TypeDef::Compact(compact) => vec![compact.type_param().id()],
+                f.ty().map(|f| {
-            TypeDef::BitSequence(seq) => {
+                    self.resolve_type_path_recurse(f.id(), false, parent_type_params)
-                vec![seq.bit_order_type().id(), seq.bit_store_type().id()]
+                })
+            })
+            .collect();
+
+        let ty = match ty.type_def() {
+            TypeDef::Composite(_) | TypeDef::Variant(_) => {
+                let joined_path = ty.path().segments().join("::");
+                if let Some(substitute_type_path) =
+                    self.type_substitutes.get(&joined_path)
+                {
+                    TypePathType::Path {
+                        path: substitute_type_path.clone(),
+                        params,
+                    }
+                } else {
+                    TypePathType::from_type_def_path(
+                        ty.path(),
+                        self.types_mod_ident.clone(),
+                        params,
+                    )
+                }
             }
-            _ => {
+            TypeDef::Primitive(primitive) => {
-                ty.type_params()
+                TypePathType::Primitive {
-                    .iter()
+                    def: primitive.clone(),
-                    .filter_map(|f| f.ty().map(|f| f.id()))
+                }
-                    .collect()
+            }
+            TypeDef::Array(arr) => {
+                TypePathType::Array {
+                    len: arr.len() as usize,
+                    of: Box::new(self.resolve_type_path_recurse(
+                        arr.type_param().id(),
+                        false,
+                        parent_type_params,
+                    )),
+                }
+            }
+            TypeDef::Sequence(seq) => {
+                TypePathType::Vec {
+                    of: Box::new(self.resolve_type_path_recurse(
+                        seq.type_param().id(),
+                        false,
+                        parent_type_params,
+                    )),
+                }
+            }
+            TypeDef::Tuple(tuple) => {
+                TypePathType::Tuple {
+                    elements: tuple
+                        .fields()
+                        .iter()
+                        .map(|f| {
+                            self.resolve_type_path_recurse(
+                                f.id(),
+                                false,
+                                parent_type_params,
+                            )
+                        })
+                        .collect(),
+                }
+            }
+            TypeDef::Compact(compact) => {
+                TypePathType::Compact {
+                    inner: Box::new(self.resolve_type_path_recurse(
+                        compact.type_param().id(),
+                        false,
+                        parent_type_params,
+                    )),
+                    is_field,
+                }
+            }
+            TypeDef::BitSequence(bitseq) => {
+                TypePathType::BitVec {
+                    bit_order_type: Box::new(self.resolve_type_path_recurse(
+                        bitseq.bit_order_type().id(),
+                        false,
+                        parent_type_params,
+                    )),
+                    bit_store_type: Box::new(self.resolve_type_path_recurse(
+                        bitseq.bit_store_type().id(),
+                        false,
+                        parent_type_params,
+                    )),
+                }
             }
         };
 
-        let params = params_type_ids
+        TypePath::Type(ty)
-            .iter()
-            .map(|tp| self.resolve_type_path(*tp, parent_type_params))
-            .collect::<Vec<_>>();
-
-        let joined_path = ty.path().segments().join("::");
-        if let Some(substitute_type_path) = self.type_substitutes.get(&joined_path) {
-            TypePath::Substitute(TypePathSubstitute {
-                path: substitute_type_path.clone(),
-                params,
-            })
-        } else {
-            TypePath::Type(TypePathType {
-                ty,
-                params,
-                root_mod_ident: self.types_mod_ident.clone(),
-            })
-        }
     }
 
     /// Returns the derives to be applied to all generated types.
@@ -228,7 +321,7 @@ pub struct Module {
     name: Ident,
     root_mod: Ident,
     children: BTreeMap<Ident, Module>,
-    types: BTreeMap<scale_info::Path<scale_info::form::PortableForm>, TypeDefGen>,
+    types: BTreeMap<scale_info::Path<PortableForm>, TypeDefGen>,
 }
 
 impl ToTokens for Module {

codegen/src/types/tests.rs

@@ -6,6 +6,7 @@ use super::*;
 use pretty_assertions::assert_eq;
 use scale_info::{
     meta_type,
+    scale,
     Registry,
     TypeInfo,
 };
@@ -371,6 +372,53 @@ fn compact_fields() {
     )
 }
 
+#[test]
+fn compact_generic_parameter() {
+    use scale::Compact;
+
+    #[allow(unused)]
+    #[derive(TypeInfo)]
+    struct S {
+        a: Option<<u128 as codec::HasCompact>::Type>,
+        nested: Option<Result<Compact<u128>, u8>>,
+        vector: Vec<Compact<u16>>,
+        array: [Compact<u8>; 32],
+        tuple: (Compact<u8>, Compact<u16>),
+    }
+
+    let mut registry = Registry::new();
+    registry.register_type(&meta_type::<S>());
+    let portable_types: PortableRegistry = registry.into();
+
+    let type_gen = TypeGenerator::new(
+        &portable_types,
+        "root",
+        Default::default(),
+        Default::default(),
+    );
+    let types = type_gen.generate_types_mod();
+    let tests_mod = get_mod(&types, MOD_PATH).unwrap();
+
+    assert_eq!(
+        tests_mod.into_token_stream().to_string(),
+        quote! {
+            pub mod tests {
+                use super::root;
+
+                #[derive(::subxt::ext::codec::Decode, ::subxt::ext::codec::Encode, Debug)]
+                pub struct S {
+                    pub a: ::core::option::Option<::subxt::ext::codec::Compact<::core::primitive::u128> >,
+                    pub nested: ::core::option::Option<::core::result::Result<::subxt::ext::codec::Compact<::core::primitive::u128>, ::core::primitive::u8 > >,
+                    pub vector: ::std::vec::Vec<::subxt::ext::codec::Compact<::core::primitive::u16> >,
+                    pub array: [::subxt::ext::codec::Compact<::core::primitive::u8>; 32usize],
+                    pub tuple: (::subxt::ext::codec::Compact<::core::primitive::u8>, ::subxt::ext::codec::Compact<::core::primitive::u16>,),
+                }
+            }
+        }
+            .to_string()
+    )
+}
+
 #[test]
 fn generate_array_field() {
     #[allow(unused)]

codegen/src/types/type_path.rs

@@ -6,14 +6,10 @@ use proc_macro2::{
     Ident,
     TokenStream,
 };
-use quote::{
+use quote::format_ident;
-    format_ident,
-    quote,
-};
 use scale_info::{
     form::PortableForm,
-    Type,
+    Path,
-    TypeDef,
     TypeDefPrimitive,
 };
 use std::collections::BTreeSet;
@@ -23,7 +19,6 @@ use syn::parse_quote;
 pub enum TypePath {
     Parameter(TypeParameter),
     Type(TypePathType),
-    Substitute(TypePathSubstitute),
 }
 
 impl quote::ToTokens for TypePath {
@@ -38,7 +33,6 @@ impl TypePath {
         match self {
             TypePath::Parameter(ty_param) => syn::Type::Path(parse_quote! { #ty_param }),
             TypePath::Type(ty) => ty.to_syn_type(),
-            TypePath::Substitute(sub) => sub.to_syn_type(),
         }
     }
 
@@ -61,7 +55,6 @@ impl TypePath {
                 acc.insert(type_parameter.clone());
             }
             Self::Type(type_path) => type_path.parent_type_params(acc),
-            Self::Substitute(sub) => sub.parent_type_params(acc),
         }
     }
 
@@ -74,88 +67,144 @@ impl TypePath {
             _ => return None,
         };
 
-        match ty.ty.type_def() {
+        match ty {
-            TypeDef::Sequence(_) => Some(&ty.params[0]),
+            TypePathType::Vec { ref of } => Some(of),
             _ => None,
         }
     }
 }
 
 #[derive(Clone, Debug)]
-pub struct TypePathType {
+pub enum TypePathType {
-    pub(super) ty: Type<PortableForm>,
+    Path {
-    pub(super) params: Vec<TypePath>,
+        path: syn::TypePath,
-    pub(super) root_mod_ident: Ident,
+        params: Vec<TypePath>,
+    },
+    Vec {
+        of: Box<TypePath>,
+    },
+    Array {
+        len: usize,
+        of: Box<TypePath>,
+    },
+    Tuple {
+        elements: Vec<TypePath>,
+    },
+    Primitive {
+        def: TypeDefPrimitive,
+    },
+    Compact {
+        inner: Box<TypePath>,
+        is_field: bool,
+    },
+    BitVec {
+        bit_order_type: Box<TypePath>,
+        bit_store_type: Box<TypePath>,
+    },
 }
 
 impl TypePathType {
+    pub fn from_type_def_path(
+        path: &Path<PortableForm>,
+        root_mod_ident: Ident,
+        params: Vec<TypePath>,
+    ) -> Self {
+        let path_segments = path.segments();
+
+        let path: syn::TypePath = match path_segments {
+            [] => panic!("Type has no ident"),
+            [ident] => {
+                // paths to prelude types
+                match ident.as_str() {
+                    "Option" => parse_quote!(::core::option::Option),
+                    "Result" => parse_quote!(::core::result::Result),
+                    "Cow" => parse_quote!(::std::borrow::Cow),
+                    "BTreeMap" => parse_quote!(::std::collections::BTreeMap),
+                    "BTreeSet" => parse_quote!(::std::collections::BTreeSet),
+                    "Range" => parse_quote!(::core::ops::Range),
+                    "RangeInclusive" => parse_quote!(::core::ops::RangeInclusive),
+                    ident => panic!("Unknown prelude type '{}'", ident),
+                }
+            }
+            _ => {
+                // paths to generated types in the root types module
+                let mut ty_path = path_segments
+                    .iter()
+                    .map(|s| syn::PathSegment::from(format_ident!("{}", s)))
+                    .collect::<syn::punctuated::Punctuated<
+                        syn::PathSegment,
+                        syn::Token![::],
+                    >>();
+                ty_path.insert(0, syn::PathSegment::from(root_mod_ident));
+                parse_quote!( #ty_path )
+            }
+        };
+        Self::Path { path, params }
+    }
+
+    /// Visits a type path, collecting all the generic type parameters from the containing type.
+    ///
+    /// # Example
+    ///
+    /// ```rust
+    /// struct S<T> {
+    ///     a: Vec<Option<T>>, // the parent type param here is `T`
+    /// }
+    /// ```
+    fn parent_type_params(&self, acc: &mut BTreeSet<TypeParameter>) {
+        match self {
+            TypePathType::Path { params, .. } => {
+                for p in params {
+                    p.parent_type_params(acc)
+                }
+            }
+            TypePathType::Vec { of } => of.parent_type_params(acc),
+            TypePathType::Array { of, .. } => of.parent_type_params(acc),
+            TypePathType::Tuple { elements } => {
+                for e in elements {
+                    e.parent_type_params(acc)
+                }
+            }
+            TypePathType::Primitive { .. } => (),
+            TypePathType::Compact { inner, .. } => inner.parent_type_params(acc),
+            TypePathType::BitVec {
+                bit_order_type,
+                bit_store_type,
+            } => {
+                bit_order_type.parent_type_params(acc);
+                bit_store_type.parent_type_params(acc);
+            }
+        }
+    }
+
     pub(crate) fn is_compact(&self) -> bool {
-        matches!(self.ty.type_def(), TypeDef::Compact(_))
+        matches!(self, TypePathType::Compact { .. })
     }
 
     fn to_syn_type(&self) -> syn::Type {
-        let params = &self.params;
+        match &self {
-        match self.ty.type_def() {
+            TypePathType::Path { path, params } => {
-            TypeDef::Composite(_) | TypeDef::Variant(_) => {
-                let path_segments = self.ty.path().segments();
-
-                let ty_path: syn::TypePath = match path_segments {
-                    [] => panic!("Type has no ident"),
-                    [ident] => {
-                        // paths to prelude types
-                        match ident.as_str() {
-                            "Option" => parse_quote!(::core::option::Option),
-                            "Result" => parse_quote!(::core::result::Result),
-                            "Cow" => parse_quote!(::std::borrow::Cow),
-                            "BTreeMap" => parse_quote!(::std::collections::BTreeMap),
-                            "BTreeSet" => parse_quote!(::std::collections::BTreeSet),
-                            "Range" => parse_quote!(::core::ops::Range),
-                            "RangeInclusive" => parse_quote!(::core::ops::RangeInclusive),
-                            ident => panic!("Unknown prelude type '{}'", ident),
-                        }
-                    }
-                    _ => {
-                        // paths to generated types in the root types module
-                        let mut ty_path = path_segments
-                            .iter()
-                            .map(|s| syn::PathSegment::from(format_ident!("{}", s)))
-                            .collect::<syn::punctuated::Punctuated<
-                                syn::PathSegment,
-                                syn::Token![::],
-                            >>();
-                        ty_path.insert(
-                            0,
-                            syn::PathSegment::from(self.root_mod_ident.clone()),
-                        );
-                        parse_quote!( #ty_path )
-                    }
-                };
-
-                let params = &self.params;
                 let path = if params.is_empty() {
-                    parse_quote! { #ty_path }
+                    parse_quote! { #path }
                 } else {
-                    parse_quote! { #ty_path< #( #params ),* > }
+                    parse_quote! { #path< #( #params ),* > }
                 };
                 syn::Type::Path(path)
             }
-            TypeDef::Sequence(_) => {
+            TypePathType::Vec { of } => {
-                let type_param = &self.params[0];
+                let type_path = parse_quote! { ::std::vec::Vec<#of> };
-                let type_path = parse_quote! { ::std::vec::Vec<#type_param> };
                 syn::Type::Path(type_path)
             }
-            TypeDef::Array(array) => {
+            TypePathType::Array { len, of } => {
-                let array_type = &self.params[0];
+                let array = parse_quote! { [#of; #len] };
-                let array_len = array.len() as usize;
-                let array = parse_quote! { [#array_type; #array_len] };
                 syn::Type::Array(array)
             }
-            TypeDef::Tuple(_) => {
+            TypePathType::Tuple { elements } => {
-                let tuple = parse_quote! { (#( # params, )* ) };
+                let tuple = parse_quote! { (#( # elements, )* ) };
                 syn::Type::Tuple(tuple)
             }
-            TypeDef::Primitive(primitive) => {
+            TypePathType::Primitive { def } => {
-                let path = match primitive {
+                syn::Type::Path(match def {
                     TypeDefPrimitive::Bool => parse_quote!(::core::primitive::bool),
                     TypeDefPrimitive::Char => parse_quote!(::core::primitive::char),
                     TypeDefPrimitive::Str => parse_quote!(::std::string::String),
@@ -171,45 +220,34 @@ impl TypePathType {
                     TypeDefPrimitive::I64 => parse_quote!(::core::primitive::i64),
                     TypeDefPrimitive::I128 => parse_quote!(::core::primitive::i128),
                     TypeDefPrimitive::I256 => unimplemented!("not a rust primitive"),
+                })
+            }
+            TypePathType::Compact { inner, is_field } => {
+                let path = if *is_field {
+                    // compact fields can use the inner compact type directly and be annotated with
+                    // the `compact` attribute e.g. `#[codec(compact)] my_compact_field: u128`
+                    parse_quote! ( #inner )
+                } else {
+                    parse_quote! ( ::subxt::ext::codec::Compact<#inner> )
                 };
                 syn::Type::Path(path)
             }
-            TypeDef::Compact(_) => {
+            TypePathType::BitVec {
-                let compact_type = &self.params[0];
+                bit_order_type,
-                parse_quote! ( #compact_type )
+                bit_store_type,
-            }
+            } => {
-            TypeDef::BitSequence(_) => {
-                let bit_order_type = &self.params[0];
-                let bit_store_type = &self.params[1];
-
                 let type_path = parse_quote! { ::subxt::ext::bitvec::vec::BitVec<#bit_store_type, #bit_order_type> };
-
                 syn::Type::Path(type_path)
             }
         }
     }
-
-    /// Returns the type parameters in a path which are inherited from the containing type.
-    ///
-    /// # Example
-    ///
-    /// ```rust
-    /// struct S<T> {
-    ///     a: Vec<Option<T>>, // the parent type param here is `T`
-    /// }
-    /// ```
-    fn parent_type_params(&self, acc: &mut BTreeSet<TypeParameter>) {
-        for p in &self.params {
-            p.parent_type_params(acc);
-        }
-    }
 }
 
 #[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)]
 pub struct TypeParameter {
     pub(super) concrete_type_id: u32,
     pub(super) original_name: String,
-    pub(super) name: proc_macro2::Ident,
+    pub(super) name: Ident,
 }
 
 impl quote::ToTokens for TypeParameter {
@@ -217,41 +255,3 @@ impl quote::ToTokens for TypeParameter {
         self.name.to_tokens(tokens)
     }
 }
-
-#[derive(Clone, Debug)]
-pub struct TypePathSubstitute {
-    pub(super) path: syn::TypePath,
-    pub(super) params: Vec<TypePath>,
-}
-
-impl quote::ToTokens for TypePathSubstitute {
-    fn to_tokens(&self, tokens: &mut TokenStream) {
-        if self.params.is_empty() {
-            self.path.to_tokens(tokens)
-        } else {
-            let substitute_path = &self.path;
-            let params = &self.params;
-            tokens.extend(quote! {
-                #substitute_path< #( #params ),* >
-            })
-        }
-    }
-}
-
-impl TypePathSubstitute {
-    fn parent_type_params(&self, acc: &mut BTreeSet<TypeParameter>) {
-        for p in &self.params {
-            p.parent_type_params(acc);
-        }
-    }
-
-    fn to_syn_type(&self) -> syn::Type {
-        if self.params.is_empty() {
-            syn::Type::Path(self.path.clone())
-        } else {
-            let substitute_path = &self.path;
-            let params = &self.params;
-            parse_quote! ( #substitute_path< #( #params ),* > )
-        }
-    }
-}