diff --git a/frame/support/procedural/src/pallet/expand/storage.rs b/frame/support/procedural/src/pallet/expand/storage.rs index c956425379c53..0000051dd9b94 100644 --- a/frame/support/procedural/src/pallet/expand/storage.rs +++ b/frame/support/procedural/src/pallet/expand/storage.rs @@ -15,22 +15,48 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::pallet::Def; +use crate::pallet::{Def, parse::storage::StorageDef}; use crate::pallet::parse::storage::{Metadata, QueryKind, StorageGenerics}; use frame_support_procedural_tools::clean_type_string; +use std::collections::HashSet; /// Generate the prefix_ident related the the storage. /// prefix_ident is used for the prefix struct to be given to storage as first generic param. -fn prefix_ident(storage_ident: &syn::Ident) -> syn::Ident { +fn prefix_ident(storage: &StorageDef) -> syn::Ident { + let storage_ident = &storage.ident; syn::Ident::new(&format!("_GeneratedPrefixForStorage{}", storage_ident), storage_ident.span()) } +/// Check for duplicated storage prefixes. This step is necessary since users can specify an +/// alternative storage prefix using the #[pallet::storage_prefix] syntax, and we need to ensure +/// that the prefix specified by the user is not a duplicate of an existing one. +fn check_prefix_duplicates( + storage_def: &StorageDef, + set: &mut HashSet, +) -> syn::Result<()> { + let prefix = storage_def.prefix(); + + if !set.insert(prefix.clone()) { + let err = syn::Error::new( + storage_def.prefix_span(), + format!("Duplicate storage prefixes found for `{}`", prefix), + ); + return Err(err); + } + + Ok(()) +} + /// * if generics are unnamed: replace the first generic `_` by the generated prefix structure /// * if generics are named: reorder the generic, remove their name, and add the missing ones. /// * Add `#[allow(type_alias_bounds)]` -pub fn process_generics(def: &mut Def) { +pub fn process_generics(def: &mut Def) -> syn::Result<()> { let frame_support = &def.frame_support; + let mut prefix_set = HashSet::new(); + for storage_def in def.storages.iter_mut() { + check_prefix_duplicates(storage_def, &mut prefix_set)?; + let item = &mut def.item.content.as_mut().expect("Checked by def").1[storage_def.index]; let typ_item = match item { @@ -50,7 +76,7 @@ pub fn process_generics(def: &mut Def) { _ => unreachable!("Checked by def"), }; - let prefix_ident = prefix_ident(&storage_def.ident); + let prefix_ident = prefix_ident(&storage_def); let type_use_gen = if def.config.has_instance { quote::quote_spanned!(storage_def.attr_span => T, I) } else { @@ -116,6 +142,8 @@ pub fn process_generics(def: &mut Def) { args.args[0] = syn::parse_quote!( #prefix_ident<#type_use_gen> ); } } + + Ok(()) } /// * generate StoragePrefix structs (e.g. for a storage `MyStorage` a struct with the name @@ -125,7 +153,9 @@ pub fn process_generics(def: &mut Def) { /// * Add `#[allow(type_alias_bounds)]` on storages type alias /// * generate metadatas pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { - process_generics(def); + if let Err(e) = process_generics(def) { + return e.into_compile_error().into(); + } let frame_support = &def.frame_support; let frame_system = &def.frame_system; @@ -344,9 +374,9 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { let prefix_structs = def.storages.iter().map(|storage_def| { let type_impl_gen = &def.type_impl_generics(storage_def.attr_span); let type_use_gen = &def.type_use_generics(storage_def.attr_span); - let prefix_struct_ident = prefix_ident(&storage_def.ident); + let prefix_struct_ident = prefix_ident(&storage_def); let prefix_struct_vis = &storage_def.vis; - let prefix_struct_const = storage_def.ident.to_string(); + let prefix_struct_const = storage_def.prefix(); let config_where_clause = &def.config.where_clause; let cfg_attrs = &storage_def.cfg_attrs; diff --git a/frame/support/procedural/src/pallet/parse/storage.rs b/frame/support/procedural/src/pallet/parse/storage.rs index 6b842ab7fa401..9ec890e66e57a 100644 --- a/frame/support/procedural/src/pallet/parse/storage.rs +++ b/frame/support/procedural/src/pallet/parse/storage.rs @@ -25,28 +25,60 @@ mod keyword { syn::custom_keyword!(Error); syn::custom_keyword!(pallet); syn::custom_keyword!(getter); + syn::custom_keyword!(storage_prefix); syn::custom_keyword!(OptionQuery); syn::custom_keyword!(ValueQuery); } -/// Parse for `#[pallet::getter(fn dummy)]` -pub struct PalletStorageAttr { - getter: syn::Ident, +/// Parse for one of the following: +/// * `#[pallet::getter(fn dummy)]` +/// * `#[pallet::storage_prefix = "CustomName"]` +pub enum PalletStorageAttr { + Getter(syn::Ident, proc_macro2::Span), + StorageName(syn::LitStr, proc_macro2::Span), +} + +impl PalletStorageAttr { + fn attr_span(&self) -> proc_macro2::Span { + match self { + Self::Getter(_, span) | Self::StorageName(_, span) => *span, + } + } } impl syn::parse::Parse for PalletStorageAttr { fn parse(input: syn::parse::ParseStream) -> syn::Result { input.parse::()?; + let attr_span = input.span(); let content; syn::bracketed!(content in input); content.parse::()?; content.parse::()?; - content.parse::()?; - let generate_content; - syn::parenthesized!(generate_content in content); - generate_content.parse::()?; - Ok(Self { getter: generate_content.parse::()? }) + let lookahead = content.lookahead1(); + if lookahead.peek(keyword::getter) { + content.parse::()?; + + let generate_content; + syn::parenthesized!(generate_content in content); + generate_content.parse::()?; + Ok(Self::Getter(generate_content.parse::()?, attr_span)) + } else if lookahead.peek(keyword::storage_prefix) { + content.parse::()?; + content.parse::()?; + + let renamed_prefix = content.parse::()?; + // Ensure the renamed prefix is a proper Rust identifier + syn::parse_str::(&renamed_prefix.value()) + .map_err(|_| { + let msg = format!("`{}` is not a valid identifier", renamed_prefix.value()); + syn::Error::new(renamed_prefix.span(), msg) + })?; + + Ok(Self::StorageName(renamed_prefix, attr_span)) + } else { + Err(lookahead.error()) + } } } @@ -89,6 +121,8 @@ pub struct StorageDef { pub instances: Vec, /// Optional getter to generate. If some then query_kind is ensured to be some as well. pub getter: Option, + /// Optional expression that evaluates to a type that can be used as StoragePrefix instead of ident. + pub rename_as: Option, /// Whereas the querytype of the storage is OptionQuery or ValueQuery. /// Note that this is best effort as it can't be determined when QueryKind is generic, and /// result can be false if user do some unexpected type alias. @@ -105,7 +139,6 @@ pub struct StorageDef { pub named_generics: Option, } - /// The parsed generic from the #[derive(Clone)] pub enum StorageGenerics { @@ -541,6 +574,25 @@ fn extract_key(ty: &syn::Type) -> syn::Result { } impl StorageDef { + /// Return the storage prefix for this storage item + pub fn prefix(&self) -> String { + self + .rename_as + .as_ref() + .map(syn::LitStr::value) + .unwrap_or(self.ident.to_string()) + } + + /// Return either the span of the ident or the span of the literal in the + /// #[storage_prefix] attribute + pub fn prefix_span(&self) -> proc_macro2::Span { + self + .rename_as + .as_ref() + .map(syn::LitStr::span) + .unwrap_or(self.ident.span()) + } + pub fn try_from( attr_span: proc_macro2::Span, index: usize, @@ -552,12 +604,30 @@ impl StorageDef { return Err(syn::Error::new(item.span(), "Invalid pallet::storage, expect item type.")); }; - let mut attrs: Vec = helper::take_item_pallet_attrs(&mut item.attrs)?; - if attrs.len() > 1 { + let attrs: Vec = helper::take_item_pallet_attrs(&mut item.attrs)?; + let (mut getters, mut names) = attrs + .into_iter() + .partition::, _>(|attr| matches!(attr, PalletStorageAttr::Getter(..))); + if getters.len() > 1 { let msg = "Invalid pallet::storage, multiple argument pallet::getter found"; - return Err(syn::Error::new(attrs[1].getter.span(), msg)); + return Err(syn::Error::new(getters[1].attr_span(), msg)); } - let getter = attrs.pop().map(|attr| attr.getter); + if names.len() > 1 { + let msg = "Invalid pallet::storage, multiple argument pallet::storage_prefix found"; + return Err(syn::Error::new(names[1].attr_span(), msg)); + } + let getter = getters.pop().map(|attr| { + match attr { + PalletStorageAttr::Getter(ident, _) => ident, + _ => unreachable!(), + } + }); + let rename_as = names.pop().map(|attr| { + match attr { + PalletStorageAttr::StorageName(lit, _) => lit, + _ => unreachable!(), + } + }); let cfg_attrs = helper::get_item_cfg_attrs(&item.attrs); @@ -609,6 +679,7 @@ impl StorageDef { metadata, docs, getter, + rename_as, query_kind, where_clause, cfg_attrs, diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs index a79c25ae8f3e3..412622b3b194d 100644 --- a/frame/support/test/tests/pallet.rs +++ b/frame/support/test/tests/pallet.rs @@ -198,6 +198,10 @@ pub mod pallet { #[pallet::storage] pub type Value = StorageValue; + #[pallet::storage] + #[pallet::storage_prefix = "Value2"] + pub type RenamedValue = StorageValue; + #[pallet::type_value] pub fn MyDefault() -> u16 where T::AccountId: From + From + SomeAssociation1 @@ -577,6 +581,10 @@ fn storage_expand() { let k = [twox_128(b"Example"), twox_128(b"Value")].concat(); assert_eq!(unhashed::get::(&k), Some(1u32)); + pallet::RenamedValue::::put(2); + let k = [twox_128(b"Example"), twox_128(b"Value2")].concat(); + assert_eq!(unhashed::get::(&k), Some(2)); + pallet::Map::::insert(1, 2); let mut k = [twox_128(b"Example"), twox_128(b"Map")].concat(); k.extend(1u8.using_encoded(blake2_128_concat)); @@ -697,6 +705,13 @@ fn metadata() { default: DecodeDifferent::Decoded(vec![0]), documentation: DecodeDifferent::Decoded(vec![]), }, + StorageEntryMetadata { + name: DecodeDifferent::Decoded("Value2".to_string()), + modifier: StorageEntryModifier::Optional, + ty: StorageEntryType::Plain(DecodeDifferent::Decoded("u64".to_string())), + default: DecodeDifferent::Decoded(vec![0]), + documentation: DecodeDifferent::Decoded(vec![]), + }, StorageEntryMetadata { name: DecodeDifferent::Decoded("Map".to_string()), modifier: StorageEntryModifier::Default, @@ -993,6 +1008,11 @@ fn test_storage_info() { max_values: Some(1), max_size: Some(4), }, + StorageInfo { + prefix: prefix(b"Example", b"Value2"), + max_values: Some(1), + max_size: Some(8), + }, StorageInfo { prefix: prefix(b"Example", b"Map"), max_values: None, diff --git a/frame/support/test/tests/pallet_ui/duplicate_storage_prefix.rs b/frame/support/test/tests/pallet_ui/duplicate_storage_prefix.rs new file mode 100644 index 0000000000000..d103fa09d991b --- /dev/null +++ b/frame/support/test/tests/pallet_ui/duplicate_storage_prefix.rs @@ -0,0 +1,21 @@ +#[frame_support::pallet] +mod pallet { + use frame_support::pallet_prelude::StorageValue; + + #[pallet::config] + pub trait Config: frame_system::Config {} + + #[pallet::pallet] + #[pallet::generate_store(trait Store)] + pub struct Pallet(core::marker::PhantomData); + + #[pallet::storage] + type Foo = StorageValue<_, u8>; + + #[pallet::storage] + #[pallet::storage_prefix = "Foo"] + type NotFoo = StorageValue<_, u16>; +} + +fn main() { +} diff --git a/frame/support/test/tests/pallet_ui/duplicate_storage_prefix.stderr b/frame/support/test/tests/pallet_ui/duplicate_storage_prefix.stderr new file mode 100644 index 0000000000000..63a6e71e44045 --- /dev/null +++ b/frame/support/test/tests/pallet_ui/duplicate_storage_prefix.stderr @@ -0,0 +1,17 @@ +error: Duplicate storage prefixes found for `Foo` + --> $DIR/duplicate_storage_prefix.rs:16:32 + | +16 | #[pallet::storage_prefix = "Foo"] + | ^^^^^ + +error[E0412]: cannot find type `_GeneratedPrefixForStorageFoo` in this scope + --> $DIR/duplicate_storage_prefix.rs:13:7 + | +13 | type Foo = StorageValue<_, u8>; + | ^^^ not found in this scope + +error[E0121]: the type placeholder `_` is not allowed within types on item signatures + --> $DIR/duplicate_storage_prefix.rs:17:35 + | +17 | type NotFoo = StorageValue<_, u16>; + | ^ not allowed in type signatures diff --git a/frame/support/test/tests/pallet_ui/storage_invalid_attribute.rs b/frame/support/test/tests/pallet_ui/storage_invalid_attribute.rs new file mode 100644 index 0000000000000..c6a88c083135d --- /dev/null +++ b/frame/support/test/tests/pallet_ui/storage_invalid_attribute.rs @@ -0,0 +1,21 @@ +#[frame_support::pallet] +mod pallet { + use frame_support::pallet_prelude::Hooks; + use frame_system::pallet_prelude::BlockNumberFor; + + #[pallet::config] + pub trait Config: frame_system::Config {} + + #[pallet::pallet] + pub struct Pallet(_); + + #[pallet::call] + impl Pallet {} + + #[pallet::storage] + #[pallet::generate_store(pub trait Store)] + type Foo = StorageValue; +} + +fn main() { +} diff --git a/frame/support/test/tests/pallet_ui/storage_invalid_attribute.stderr b/frame/support/test/tests/pallet_ui/storage_invalid_attribute.stderr new file mode 100644 index 0000000000000..bf93d99cf56bd --- /dev/null +++ b/frame/support/test/tests/pallet_ui/storage_invalid_attribute.stderr @@ -0,0 +1,5 @@ +error: expected `getter` or `storage_prefix` + --> $DIR/storage_invalid_attribute.rs:16:12 + | +16 | #[pallet::generate_store(pub trait Store)] + | ^^^^^^^^^^^^^^ diff --git a/frame/support/test/tests/pallet_ui/storage_invalid_rename_value.rs b/frame/support/test/tests/pallet_ui/storage_invalid_rename_value.rs new file mode 100644 index 0000000000000..c3a08e05e2ac7 --- /dev/null +++ b/frame/support/test/tests/pallet_ui/storage_invalid_rename_value.rs @@ -0,0 +1,18 @@ +#[frame_support::pallet] +mod pallet { + use frame_support::pallet_prelude::Hooks; + use frame_system::pallet_prelude::BlockNumberFor; + + #[pallet::config] + pub trait Config: frame_system::Config {} + + #[pallet::pallet] + pub struct Pallet(core::marker::PhantomData); + + #[pallet::storage] + #[pallet::storage_prefix = "pub"] + type Foo = StorageValue<_, u8>; +} + +fn main() { +} diff --git a/frame/support/test/tests/pallet_ui/storage_invalid_rename_value.stderr b/frame/support/test/tests/pallet_ui/storage_invalid_rename_value.stderr new file mode 100644 index 0000000000000..513970f98a4f7 --- /dev/null +++ b/frame/support/test/tests/pallet_ui/storage_invalid_rename_value.stderr @@ -0,0 +1,5 @@ +error: `pub` is not a valid identifier + --> $DIR/storage_invalid_rename_value.rs:13:29 + | +13 | #[pallet::storage_prefix = "pub"] + | ^^^^^ diff --git a/frame/support/test/tests/pallet_ui/storage_multiple_getters.rs b/frame/support/test/tests/pallet_ui/storage_multiple_getters.rs new file mode 100644 index 0000000000000..309b9b24136fa --- /dev/null +++ b/frame/support/test/tests/pallet_ui/storage_multiple_getters.rs @@ -0,0 +1,25 @@ +#[frame_support::pallet] +mod pallet { + use frame_support::pallet_prelude::Hooks; + use frame_system::pallet_prelude::BlockNumberFor; + + #[pallet::config] + pub trait Config: frame_system::Config {} + + #[pallet::pallet] + pub struct Pallet(core::marker::PhantomData); + + #[pallet::hooks] + impl Hooks> for Pallet {} + + #[pallet::call] + impl Pallet {} + + #[pallet::storage] + #[pallet::getter(fn get_foo)] + #[pallet::getter(fn foo_error)] + type Foo = StorageValue<_, u8>; +} + +fn main() { +} diff --git a/frame/support/test/tests/pallet_ui/storage_multiple_getters.stderr b/frame/support/test/tests/pallet_ui/storage_multiple_getters.stderr new file mode 100644 index 0000000000000..188eed3cb0d17 --- /dev/null +++ b/frame/support/test/tests/pallet_ui/storage_multiple_getters.stderr @@ -0,0 +1,5 @@ +error: Invalid pallet::storage, multiple argument pallet::getter found + --> $DIR/storage_multiple_getters.rs:20:3 + | +20 | #[pallet::getter(fn foo_error)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/frame/support/test/tests/pallet_ui/storage_multiple_renames.rs b/frame/support/test/tests/pallet_ui/storage_multiple_renames.rs new file mode 100644 index 0000000000000..f3caef80a7ee2 --- /dev/null +++ b/frame/support/test/tests/pallet_ui/storage_multiple_renames.rs @@ -0,0 +1,25 @@ +#[frame_support::pallet] +mod pallet { + use frame_support::pallet_prelude::Hooks; + use frame_system::pallet_prelude::BlockNumberFor; + + #[pallet::config] + pub trait Config: frame_system::Config {} + + #[pallet::pallet] + pub struct Pallet(core::marker::PhantomData); + + #[pallet::hooks] + impl Hooks> for Pallet {} + + #[pallet::call] + impl Pallet {} + + #[pallet::storage] + #[pallet::storage_prefix = "Bar"] + #[pallet::storage_prefix = "Baz"] + type Foo = StorageValue<_, u8>; +} + +fn main() { +} diff --git a/frame/support/test/tests/pallet_ui/storage_multiple_renames.stderr b/frame/support/test/tests/pallet_ui/storage_multiple_renames.stderr new file mode 100644 index 0000000000000..9288d131d95af --- /dev/null +++ b/frame/support/test/tests/pallet_ui/storage_multiple_renames.stderr @@ -0,0 +1,5 @@ +error: Invalid pallet::storage, multiple argument pallet::storage_prefix found + --> $DIR/storage_multiple_renames.rs:20:3 + | +20 | #[pallet::storage_prefix = "Baz"] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^