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

Metadata V16 (unstable): Enrich metadata with associated types of config traits #5274

Merged
merged 54 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
7e8baee
metadata-ir: Introduce PalletAssociatedTypeMetadata
lexnv Aug 7, 2024
1c12f15
frame/config: Add associated types to parsed config
lexnv Aug 7, 2024
20b22d8
frame/expand: Implement associated types expansion
lexnv Aug 7, 2024
350efce
frame/expand: Use provided cfgs for the associated types
lexnv Aug 7, 2024
3e97971
frame/construct_runtime: Extract associated types from pallet config
lexnv Aug 7, 2024
5d8021e
frame/pallet: Introduce `config(without_metadata)`
lexnv Aug 7, 2024
5679244
frame/pallet: Introduce `#[pallet::include_metadata]` for associated
lexnv Aug 7, 2024
958611e
frame/pallet: Include associated type iff bounds contain TypeInfo
lexnv Aug 7, 2024
7f26b67
frame/pallet: Proper flag for metdata collection
lexnv Aug 7, 2024
cc001f3
frame/tests/ui: Fix type in test
lexnv Aug 8, 2024
b2803e2
frame/tests/ui: Check config without metadata
lexnv Aug 8, 2024
94007d8
frame/tests/ui: Check config with multiple attributes
lexnv Aug 8, 2024
75d2697
frame/tests/ui: Add negative test for duplicate attributes in config
lexnv Aug 8, 2024
ebcb4a0
frame/tests/ui: Add negative test for collecting metadata from constants
lexnv Aug 8, 2024
5115021
frame/tests/ui: Add negative test for metadata collection on events
lexnv Aug 8, 2024
aba029a
frame/tests: Check PalletAssociatedTypeMetadataIR collection
lexnv Aug 8, 2024
6502c83
frame/support: Add documentation
lexnv Aug 8, 2024
e9571cb
Add PRdoc
lexnv Aug 8, 2024
31bf284
Merge remote-tracking branch 'origin/master' into lexnv/metadata-v16-…
lexnv Aug 8, 2024
7429eb7
Update prdoc
lexnv Aug 8, 2024
8a0c138
prdoc: Remove unneeded crate
lexnv Aug 8, 2024
c6c1800
prdoc: Include frame-support
lexnv Aug 8, 2024
90c100c
Prupdate
lexnv Aug 8, 2024
2982c59
Merge remote-tracking branch 'origin/master' into lexnv/metadata-v16-…
lexnv Sep 16, 2024
f703e4d
Update substrate/frame/support/test/tests/pallet_ui/config_metadata_o…
lexnv Oct 1, 2024
fbd36bc
Update substrate/frame/support/procedural/src/pallet/expand/config.rs
lexnv Oct 1, 2024
6be9ef3
Update substrate/frame/support/src/lib.rs
lexnv Oct 1, 2024
c9b0e2a
frame/parse: Include duplicates in error message
lexnv Oct 1, 2024
0382d32
frame/parse: Change parsing with lookahead
lexnv Oct 1, 2024
e52ba1d
frame/lib: Remove links to include_metadata
lexnv Oct 1, 2024
45f86eb
pallet/parse: Collect associated metadata while taking constants into
lexnv Oct 1, 2024
fb8b2d4
pallet/parse: Reverse enable metadata flag to disable metadata
lexnv Oct 1, 2024
4846726
Merge remote-tracking branch 'origin/master' into lexnv/metadata-v16-…
lexnv Oct 4, 2024
d4e597d
tests/pallet_ui: Adjust for new space in errors
lexnv Oct 4, 2024
a453595
construct-ui/tests: Adjust testing to new format
lexnv Oct 4, 2024
8b9549d
Merge branch 'master' into lexnv/metadata-v16-associated-types
lexnv Oct 4, 2024
cf6749b
Update substrate/frame/support/procedural/src/pallet/parse/config.rs
lexnv Oct 10, 2024
54e6d2a
Update substrate/frame/support/procedural/src/pallet/parse/config.rs
lexnv Oct 10, 2024
610bdc2
frame/pallet: Adjust error messages
lexnv Oct 10, 2024
b18a2f9
pallet/expand: Use associated type span for better errors
lexnv Oct 10, 2024
678944a
pallet/parse: Use lookahead properly
lexnv Oct 10, 2024
d43743c
tests/pallet_ui: Add include metadata for nonscale type info
lexnv Oct 10, 2024
0928d78
frame/support: Rename #[without_metadata] ->
lexnv Oct 10, 2024
8fd0180
frame/support: Rename WithoutAutomaticMetadata variant
lexnv Oct 10, 2024
ac08308
frame/support: Better error msg for invalid #[include_metadata]
lexnv Oct 11, 2024
1296dc2
pallet_ui/tests: Update error message
lexnv Oct 11, 2024
c1f204f
Merge branch 'master' into lexnv/metadata-v16-associated-types
lexnv Oct 11, 2024
bc0f528
Merge remote-tracking branch 'origin/master' into lexnv/metadata-v16-…
lexnv Oct 15, 2024
b125a1e
frame/support: Detect #[include_metadata] duplicates
lexnv Oct 15, 2024
39a9d4b
frame/support: Improve TypeInfo detection
lexnv Oct 15, 2024
ac92bc6
frame/support: Simplify duplicate detection logic
lexnv Oct 15, 2024
e96b36d
frame/support: Rename error for explicit types
lexnv Oct 15, 2024
e91da23
Merge branch 'master' into lexnv/metadata-v16-associated-types
shawntabrizi Oct 15, 2024
f3e3e1e
Merge branch 'master' into lexnv/metadata-v16-associated-types
lexnv Oct 16, 2024
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
24 changes: 24 additions & 0 deletions prdoc/pr_5274.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
title: Enrich metadata IR with associated types of config traits

doc:
- audience: Runtime Dev
description: |
This feature is part of the upcoming metadata V16. The associated types of the `Config` trait that require the `TypeInfo`
or `Parameter` bounds are included in the metadata of the pallet. The metadata is not yet exposed to the end-user, however
the metadata intermediate representation (IR) contains these types.

Developers can opt out of metadata collection of the associated types by specifying `without_metadata` optional attribute
to the `#[pallet::config]`.

Furthermore, the `without_metadata` argument can be used in combination with the newly added `#[pallet::include_metadata]`
attribute to selectively include only certain associated types in the metadata collection.

crates:
- name: frame-support
bump: patch
- name: frame-support-procedural
bump: patch
- name: frame-support-procedural-tools
bump: patch
- name: sp-metadata-ir
bump: major
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ pub fn expand_runtime_metadata(
let event = expand_pallet_metadata_events(&filtered_names, runtime, scrate, decl);
let constants = expand_pallet_metadata_constants(runtime, decl);
let errors = expand_pallet_metadata_errors(runtime, decl);
let associated_types = expand_pallet_metadata_associated_types(runtime, decl);
let docs = expand_pallet_metadata_docs(runtime, decl);
let attr = decl.cfg_pattern.iter().fold(TokenStream::new(), |acc, pattern| {
let attr = TokenStream::from_str(&format!("#[cfg({})]", pattern.original()))
Expand All @@ -70,6 +71,7 @@ pub fn expand_runtime_metadata(
constants: #constants,
error: #errors,
docs: #docs,
associated_types: #associated_types,
}
}
})
Expand Down Expand Up @@ -256,3 +258,12 @@ fn expand_pallet_metadata_docs(runtime: &Ident, decl: &Pallet) -> TokenStream {
#path::Pallet::<#runtime #(, #path::#instance)*>::pallet_documentation_metadata()
}
}

fn expand_pallet_metadata_associated_types(runtime: &Ident, decl: &Pallet) -> TokenStream {
let path = &decl.path;
let instance = decl.instance.as_ref().into_iter();

quote! {
#path::Pallet::<#runtime #(, #path::#instance)*>::pallet_associated_types_metadata()
}
}
9 changes: 9 additions & 0 deletions substrate/frame/support/procedural/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -955,6 +955,15 @@ pub fn event(_: TokenStream, _: TokenStream) -> TokenStream {
pallet_macro_stub()
}

///
/// ---
///
/// Documentation for this macro can be found at `frame_support::pallet_macros::include_metadata`.
gui1117 marked this conversation as resolved.
Show resolved Hide resolved
#[proc_macro_attribute]
pub fn include_metadata(_: TokenStream, _: TokenStream) -> TokenStream {
pallet_macro_stub()
}

///
/// ---
///
Expand Down
47 changes: 47 additions & 0 deletions substrate/frame/support/procedural/src/pallet/expand/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,50 @@ Consequently, a runtime that wants to include this pallet must implement this tr
_ => Default::default(),
}
}

/// Generate the metadata for the associated types of the config trait.
///
/// Implements the `pallet_associated_types_metadata` function for the pallet.
pub fn expand_config_metadata(def: &Def) -> proc_macro2::TokenStream {
let frame_support = &def.frame_support;
let type_impl_gen = &def.type_impl_generics(proc_macro2::Span::call_site());
let type_use_gen = &def.type_use_generics(proc_macro2::Span::call_site());
let pallet_ident = &def.pallet_struct.pallet;
let trait_use_gen = &def.trait_use_generics(proc_macro2::Span::call_site());

let mut where_clauses = vec![&def.config.where_clause];
where_clauses.extend(def.extra_constants.iter().map(|d| &d.where_clause));
let completed_where_clause = super::merge_where_clauses(&where_clauses);

let types = def.config.associated_types_metadata.iter().map(|metadata| {
let ident = &metadata.ident;
let ident_str = format!("{}", ident);
lexnv marked this conversation as resolved.
Show resolved Hide resolved
let cfgs = &metadata.cfg;

let no_docs = vec![];
let doc = if cfg!(feature = "no-metadata-docs") { &no_docs } else { &metadata.doc };

quote::quote!({
gui1117 marked this conversation as resolved.
Show resolved Hide resolved
#( #cfgs ) *
#frame_support::__private::metadata_ir::PalletAssociatedTypeMetadataIR {
name: #ident_str,
ty: #frame_support::__private::scale_info::meta_type::<
<T as Config #trait_use_gen>::#ident
>(),
docs: #frame_support::__private::sp_std::vec![ #( #doc ),* ],
}
})
});

quote::quote!(
impl<#type_impl_gen> #pallet_ident<#type_use_gen> #completed_where_clause {

#[doc(hidden)]
pub fn pallet_associated_types_metadata()
-> #frame_support::__private::sp_std::vec::Vec<#frame_support::__private::metadata_ir::PalletAssociatedTypeMetadataIR>
{
#frame_support::__private::sp_std::vec![ #( #types ),* ]
}
}
)
}
2 changes: 2 additions & 0 deletions substrate/frame/support/procedural/src/pallet/expand/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ pub fn expand(mut def: Def) -> proc_macro2::TokenStream {
let constants = constants::expand_constants(&mut def);
let pallet_struct = pallet_struct::expand_pallet_struct(&mut def);
let config = config::expand_config(&mut def);
let associated_types = config::expand_config_metadata(&def);
let call = call::expand_call(&mut def);
let tasks = tasks::expand_tasks(&mut def);
let error = error::expand_error(&mut def);
Expand Down Expand Up @@ -101,6 +102,7 @@ storage item. Otherwise, all storage items are listed among [*Type Definitions*]
#constants
#pallet_struct
#config
#associated_types
#call
#tasks
#error
Expand Down
94 changes: 91 additions & 3 deletions substrate/frame/support/procedural/src/pallet/parse/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
// limitations under the License.

use super::helper;
use frame_support_procedural_tools::{get_doc_literals, is_using_frame_crate};
use frame_support_procedural_tools::{get_cfg_attributes, get_doc_literals, is_using_frame_crate};
use quote::ToTokens;
use syn::{spanned::Spanned, token, Token};
use syn::{spanned::Spanned, token, Token, TraitItemType};

/// List of additional token to be used for parsing.
mod keyword {
Expand All @@ -36,6 +36,7 @@ mod keyword {
syn::custom_keyword!(no_default);
syn::custom_keyword!(no_default_bounds);
syn::custom_keyword!(constant);
syn::custom_keyword!(include_metadata);
}

#[derive(Default)]
Expand All @@ -55,6 +56,8 @@ pub struct ConfigDef {
pub has_instance: bool,
/// Const associated type.
pub consts_metadata: Vec<ConstMetadataDef>,
/// Associated types metadata.
pub associated_types_metadata: Vec<AssociatedTypeMetadataDef>,
/// Whether the trait has the associated type `Event`, note that those bounds are
/// checked:
/// * `IsType<Self as frame_system::Config>::RuntimeEvent`
Expand All @@ -70,6 +73,26 @@ pub struct ConfigDef {
pub default_sub_trait: Option<DefaultTrait>,
}

/// Input definition for an associated type in pallet config.
pub struct AssociatedTypeMetadataDef {
/// Name of the associated type.
pub ident: syn::Ident,
/// The doc associated.
pub doc: Vec<syn::Expr>,
/// The cfg associated.
pub cfg: Vec<syn::Attribute>,
}

impl From<&syn::TraitItemType> for AssociatedTypeMetadataDef {
fn from(trait_ty: &syn::TraitItemType) -> Self {
let ident = trait_ty.ident.clone();
let doc = get_doc_literals(&trait_ty.attrs);
let cfg = get_cfg_attributes(&trait_ty.attrs);

Self { ident, doc, cfg }
}
}

/// Input definition for a constant in pallet config.
pub struct ConstMetadataDef {
/// Name of the associated type.
Expand Down Expand Up @@ -144,6 +167,8 @@ pub enum PalletAttrType {
NoBounds(keyword::no_default_bounds),
#[peek(keyword::constant, name = "constant")]
Constant(keyword::constant),
#[peek(keyword::include_metadata, name = "include_metadata")]
IncludeMetadata(keyword::include_metadata),
}

/// Parsing for `#[pallet::X]`
Expand Down Expand Up @@ -320,12 +345,30 @@ pub fn replace_self_by_t(input: proc_macro2::TokenStream) -> proc_macro2::TokenS
.collect()
}

/// Check that the trait item requires the `TypeInfo` bound (or similar).
fn contains_type_info_bound(ty: &TraitItemType) -> bool {
const KNOWN_TYPE_INFO_BOUNDS: &[&str] = &[
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Why not a Set?

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 think we'd need some feature similar to rust-lang/rust#93140 to make the hashSet const here

// Explicit TypeInfo trait.
"TypeInfo",
// Implicit known substrate traits that implement type info.
// Note: Aim to keep this list as small as possible.
"Parameter",
];

ty.bounds.iter().any(|bound| {
let syn::TypeParamBound::Trait(bound) = bound else { return false };

KNOWN_TYPE_INFO_BOUNDS.iter().any(|known| bound.path.is_ident(known))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that this will not match scale::TypeInfo but only TypeInfo, isn't it?

What I mean is that we do the match on the whole path, not the last segment, no?

If we want to support only this. Then we can say in the error message of include_metadata something like: "Note: only ident is supported, e.g. scale::TypeInfo is not detected, only TypeInfo.

Copy link
Member

Choose a reason for hiding this comment

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

We should just take the last element of the path to improve the detection?

})
}

impl ConfigDef {
pub fn try_from(
frame_system: &syn::Path,
index: usize,
item: &mut syn::Item,
enable_default: bool,
enable_associated_metadata: bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: maybe disable_associated_metadata as by default it should be enabled(?)

) -> syn::Result<Self> {
let syn::Item::Trait(item) = item else {
let msg = "Invalid pallet::config, expected trait definition";
Expand Down Expand Up @@ -366,6 +409,7 @@ impl ConfigDef {

let mut has_event_type = false;
let mut consts_metadata = vec![];
let mut associated_types_metadata = vec![];
let mut default_sub_trait = if enable_default {
Some(DefaultTrait {
items: Default::default(),
Expand All @@ -381,6 +425,7 @@ impl ConfigDef {
let mut already_no_default = false;
let mut already_constant = false;
let mut already_no_default_bounds = false;
let mut already_collected_associated_type = false;

while let Ok(Some(pallet_attr)) =
helper::take_first_item_pallet_attr::<PalletAttr>(trait_item)
Expand All @@ -401,11 +446,39 @@ impl ConfigDef {
trait_item.span(),
"Invalid #[pallet::constant] in #[pallet::config], expected type item",
)),
// Pallet developer has explicitly requested to include metadata for this associated type.
//
// They must provide a type item that implements `TypeInfo`.
(PalletAttrType::IncludeMetadata(_), syn::TraitItem::Type(ref typ)) => {
if is_event {
return Err(syn::Error::new(
pallet_attr._bracket.span.join(),
"Invalid #[pallet::include_metadata] in `type RuntimeEvent`, \
expected type item. The associated type `RuntimeEvent` is already collected.",
))
}

if already_constant {
Copy link
Member

Choose a reason for hiding this comment

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

So, if I put include_metadata before constant, this branch will not be taken.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching this 🙏

Have moved the checks after we iterate over all pallet attributes

return Err(syn::Error::new(
pallet_attr._bracket.span.join(),
"Invalid #[pallet::include_metadata] in #[pallet::constant], \
expected type item. Pallet constant's metadata is already collected.",
))
}

already_collected_associated_type = true;
associated_types_metadata.push(AssociatedTypeMetadataDef::from(typ));
}
(PalletAttrType::IncludeMetadata(_), _) =>
return Err(syn::Error::new(
pallet_attr._bracket.span.join(),
"Invalid #[pallet::include_metadata] in #[pallet::config], expected type item",
)),
(PalletAttrType::NoDefault(_), _) => {
if !enable_default {
return Err(syn::Error::new(
pallet_attr._bracket.span.join(),
"`#[pallet:no_default]` can only be used if `#[pallet::config(with_default)]` \
"`#[pallet::no_default]` can only be used if `#[pallet::config(with_default)]` \
has been specified"
))
}
Expand Down Expand Up @@ -437,6 +510,20 @@ impl ConfigDef {
}
}

// Metadata of associated types is collected by default, iff the associated type
bkchr marked this conversation as resolved.
Show resolved Hide resolved
// implements `TypeInfo`, or a similar trait that requires the `TypeInfo` bound.
if !already_collected_associated_type &&
enable_associated_metadata &&
!is_event && !already_constant
{
if let syn::TraitItem::Type(ref ty) = trait_item {
// Collect the metadata of the associated type if it implements `TypeInfo`.
if contains_type_info_bound(ty) {
associated_types_metadata.push(AssociatedTypeMetadataDef::from(ty));
}
}
}

if !already_no_default && enable_default {
default_sub_trait
.as_mut()
Expand Down Expand Up @@ -479,6 +566,7 @@ impl ConfigDef {
index,
has_instance,
consts_metadata,
associated_types_metadata,
has_event_type,
where_clause,
default_sub_trait,
Expand Down
Loading
Loading