Renamed key_holder.

Add UI test for double storage_item.
Applied suggestion from the review.

crates/lang/ir/src/ir/config.rs

@@ -12,11 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use crate::{
-    ast,
-    ast::MetaNameValue,
-    error::ExtError as _,
-};
+use crate::{ast, ast::MetaNameValue, error::ExtError as _};
 use std::collections::HashMap;
 use syn::spanned::Spanned;
 
@@ -73,7 +69,7 @@ impl WhitelistedAttributes {
                 arg,
                 "expected a string with attributes separated by `,`",
             ))
-        }
+        };
     }
 
     /// Returns the filtered input vector of whitelisted attributes.
@@ -93,18 +89,18 @@ impl WhitelistedAttributes {
 }
 
 /// Return an error to notify about duplicate ink! configuration arguments.
-fn duplicate_config_err<F, S>(fst: F, snd: S, name: &str) -> syn::Error
+fn duplicate_config_err<F, S>(first: F, second: S, name: &str) -> syn::Error
 where
     F: Spanned,
     S: Spanned,
 {
     format_err!(
-        snd.span(),
+        second.span(),
         "encountered duplicate ink! `{}` configuration argument",
         name,
     )
     .into_combine(format_err!(
-        fst.span(),
+        first.span(),
         "first `{}` configuration argument here",
         name
     ))
@@ -120,23 +116,23 @@ impl TryFrom<ast::AttributeArgs> for Config {
         for arg in args.into_iter() {
             if arg.name.is_ident("env") {
                 if let Some((_, ast)) = env {
-                    return Err(duplicate_config_err(ast, arg, "env"))
+                    return Err(duplicate_config_err(ast, arg, "env"));
                 }
                 if let ast::PathOrLit::Path(path) = &arg.value {
                     env = Some((Environment { path: path.clone() }, arg))
                 } else {
                     return Err(format_err_spanned!(
                         arg,
                         "expected a path for `env` ink! configuration argument",
-                    ))
+                    ));
                 }
             } else if arg.name.is_ident("keep_attr") {
                 whitelisted_attributes.parse_arg_value(&arg)?;
             } else {
                 return Err(format_err_spanned!(
                     arg,
                     "encountered unknown or unsupported ink! configuration argument",
-                ))
+                ));
             }
         }
         Ok(Config {

crates/lang/ir/src/ir/storage_item/config.rs

@@ -12,10 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use crate::{
-    ast,
-    error::ExtError as _,
-};
+use crate::{ast, error::ExtError as _};
 use syn::spanned::Spanned;
 
 /// The ink! configuration.
@@ -29,18 +26,18 @@ pub struct StorageItemConfig {
 }
 
 /// Return an error to notify about duplicate ink! ink storage configuration arguments.
-fn duplicate_config_err<F, S>(fst: F, snd: S, name: &str) -> syn::Error
+fn duplicate_config_err<F, S>(first: F, second: S, name: &str) -> syn::Error
 where
     F: Spanned,
     S: Spanned,
 {
     format_err!(
-        snd.span(),
+        second.span(),
         "encountered duplicate ink! storage item `{}` configuration argument",
         name,
     )
     .into_combine(format_err!(
-        fst.span(),
+        first.span(),
         "first `{}` configuration argument here",
         name
     ))
@@ -54,21 +51,21 @@ impl TryFrom<ast::AttributeArgs> for StorageItemConfig {
         for arg in args.into_iter() {
             if arg.name.is_ident("derive") {
                 if let Some(lit_bool) = derive {
-                    return Err(duplicate_config_err(lit_bool, arg, "derive"))
+                    return Err(duplicate_config_err(lit_bool, arg, "derive"));
                 }
                 if let ast::PathOrLit::Lit(syn::Lit::Bool(lit_bool)) = &arg.value {
                     derive = Some(lit_bool.clone())
                 } else {
                     return Err(format_err_spanned!(
                         arg,
                         "expected a bool literal for `derive` ink! storage item configuration argument",
-                    ))
+                    ));
                 }
             } else {
                 return Err(format_err_spanned!(
                     arg,
                     "encountered unknown or unsupported ink! storage item configuration argument",
-                ))
+                ));
             }
         }
         Ok(StorageItemConfig {

crates/lang/ir/src/ir/storage_item/mod.rs

@@ -35,6 +35,20 @@ impl StorageItem {
         let parsed_config = syn::parse2::<crate::ast::AttributeArgs>(config)?;
         let config = StorageItemConfig::try_from(parsed_config)?;
 
+        for attr in &ast.attrs {
+            if attr
+                .path
+                .to_token_stream()
+                .to_string()
+                .contains("storage_item")
+            {
+                return Err(format_err_spanned!(
+                    attr,
+                    "only one `ink::storage_item` is allowed",
+                ))
+            }
+        }
+
         Ok(Self { ast, config })
     }
 

crates/lang/ir/src/ir/trait_def/config.rs

@@ -12,11 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use crate::{
-    ast,
-    error::ExtError as _,
-    ir::config::WhitelistedAttributes,
-};
+use crate::{ast, error::ExtError as _, ir::config::WhitelistedAttributes};
 use syn::spanned::Spanned;
 
 /// The ink! configuration.
@@ -49,18 +45,18 @@ impl TraitDefinitionConfig {
 }
 
 /// Return an error to notify about duplicate ink! trait definition configuration arguments.
-fn duplicate_config_err<F, S>(fst: F, snd: S, name: &str) -> syn::Error
+fn duplicate_config_err<F, S>(first: F, second: S, name: &str) -> syn::Error
 where
     F: Spanned,
     S: Spanned,
 {
     format_err!(
-        snd.span(),
+        second.span(),
         "encountered duplicate ink! trait definition `{}` configuration argument",
         name,
     )
     .into_combine(format_err!(
-        fst.span(),
+        first.span(),
         "first `{}` configuration argument here",
         name
     ))
@@ -75,29 +71,29 @@ impl TryFrom<ast::AttributeArgs> for TraitDefinitionConfig {
         for arg in args.into_iter() {
             if arg.name.is_ident("namespace") {
                 if let Some((_, meta_name_value)) = namespace {
-                    return Err(duplicate_config_err(meta_name_value, arg, "namespace"))
+                    return Err(duplicate_config_err(meta_name_value, arg, "namespace"));
                 }
                 if let ast::PathOrLit::Lit(syn::Lit::Str(lit_str)) = &arg.value {
                     if syn::parse_str::<syn::Ident>(&lit_str.value()).is_err() {
                         return Err(format_err_spanned!(
                             lit_str,
                             "encountered invalid Rust identifier for the ink! namespace configuration parameter"
-                        ))
+                        ));
                     }
                     namespace = Some((lit_str.clone(), arg))
                 } else {
                     return Err(format_err_spanned!(
                         arg,
                         "expected a string literal for `namespace` ink! trait definition configuration argument",
-                    ))
+                    ));
                 }
             } else if arg.name.is_ident("keep_attr") {
                 whitelisted_attributes.parse_arg_value(&arg)?;
             } else {
                 return Err(format_err_spanned!(
                     arg,
                     "encountered unknown or unsupported ink! trait definition configuration argument",
-                ))
+                ));
             }
         }
         Ok(TraitDefinitionConfig {

crates/lang/macro/src/lib.rs

@@ -668,16 +668,20 @@ pub fn trait_definition(attr: TokenStream, item: TokenStream) -> TokenStream {
 
 /// Prepares the type to be fully compatible and usable with the storage.
 /// It implements all necessary traits and calculates the storage key for types.
-/// Packed types don't have a storage key, but non-packed types
+/// [`Packed`](ink_storage::traits::Packed) types don't have a storage key, but non-packed types
 /// (like `Mapping`, `Lazy` etc.) require calculating the storage key during compilation.
 ///
-/// Consider annotating structs and enums that are intented to be a part of
+/// Consider annotating structs and enums that are intended to be a part of
 /// the storage with this macro. If the type is packed then the usage of the
 /// macro is optional.
 ///
 /// If the type is non-packed it is best to rely on automatic storage key
-/// calculation. The storage key can also be specified manually with the
-/// generic `KEY` parameter though.
+/// calculation via `ink::storage_item`.
+///
+/// The usage of `KEY: StorageKey` generic allows to propagate the parent's storage key to the type
+/// and offset the storage key of the type. It is helpful for non-packed types that can be used
+/// several times in the contract. Each field should have a unique storage key, so propagation of
+/// the parent's storage key allows one to achieve it.
 ///
 /// The macro should be called before `derive` macros because it can change the type.
 ///
@@ -702,6 +706,8 @@ pub fn trait_definition(attr: TokenStream, item: TokenStream) -> TokenStream {
 /// };
 /// use ink_primitives::traits::Storable;
 ///
+/// // Deriving `scale::Decode` and `scale::Encode` also derives blanket implementation of all
+/// // required traits to be storable.
 /// #[derive(scale::Decode, scale::Encode)]
 /// struct Packed {
 ///     s1: u128,
@@ -710,18 +716,7 @@ pub fn trait_definition(attr: TokenStream, item: TokenStream) -> TokenStream {
 ///     // s3: Vec<NonPacked>,
 /// }
 ///
-/// #[derive(scale::Decode, scale::Encode)]
-/// #[cfg_attr(
-///     feature = "std",
-///     derive(scale_info::TypeInfo, ink_storage::traits::StorageLayout)
-/// )]
-/// struct PackedManual {
-///     s1: u32,
-///     s2: Vec<(u128, String)>,
-///     // Fails because `StorableHint` is only implemented for `Vec` where `T: Packed`.
-///     // s3: Vec<NonPacked>,
-/// }
-///
+/// // Example of how to define the packed type with generic.
 /// #[derive(scale::Decode, scale::Encode)]
 /// #[cfg_attr(
 ///     feature = "std",
@@ -733,6 +728,14 @@ pub fn trait_definition(attr: TokenStream, item: TokenStream) -> TokenStream {
 ///     s3: String,
 /// }
 ///
+/// // Example of how to define the non-packed type.
+/// #[ink_lang::storage_item]
+/// struct NonPacked {
+///     s1: Mapping<u32, u128>,
+///     s2: Lazy<u128>,
+/// }
+///
+/// // Example of how to define the non-packed generic type.
 /// #[ink_lang::storage_item(derive = false)]
 /// #[derive(Storable, StorableHint, StorageKey)]
 /// #[cfg_attr(
@@ -745,6 +748,7 @@ pub fn trait_definition(attr: TokenStream, item: TokenStream) -> TokenStream {
 ///     s3: Mapping<u128, T>,
 /// }
 ///
+/// // Example of how to define a complex packed type.
 /// #[derive(scale::Decode, scale::Encode)]
 /// #[cfg_attr(
 ///     feature = "std",
@@ -753,15 +757,10 @@ pub fn trait_definition(attr: TokenStream, item: TokenStream) -> TokenStream {
 /// struct PackedComplex {
 ///     s1: u128,
 ///     s2: Vec<u128>,
-///     s3: Vec<PackedManual>,
-/// }
-///
-/// #[ink_lang::storage_item]
-/// struct NonPacked {
-///     s1: Mapping<u32, u128>,
-///     s2: Lazy<u128>,
+///     s3: Vec<Packed>,
 /// }
 ///
+/// // Example of how to define a complex non-packed type.
 /// #[ink_lang::storage_item]
 /// struct NonPackedComplex<KEY: StorageKey> {
 ///     s1: (String, u128, Packed),
@@ -778,8 +777,8 @@ pub fn trait_definition(attr: TokenStream, item: TokenStream) -> TokenStream {
 ///
 /// ## Header Arguments
 ///
-/// The `#[ink::storage_item]` macro can be provided with some additional comma-separated
-/// header arguments:
+/// The `#[ink::storage_item]` macro can be provided with an additional comma-separated
+/// header argument:
 ///
 /// - `derive: bool`
 ///

crates/lang/tests/ui/storage_item/fail/double_storage_item.rs

@@ -0,0 +1,10 @@
+#[ink_lang::storage_item(derive = false)]
+#[ink_lang::storage_item(derive = true)]
+#[derive(Default)]
+struct Contract<KEY: StorageKey = ManualKey<123>> {
+    a: u16,
+    b: u64,
+    c: u128,
+}
+
+fn main() {}

crates/lang/tests/ui/storage_item/fail/double_storage_item.stderr

@@ -0,0 +1,5 @@
+error: only one `ink::storage_item` is allowed
+ --> tests/ui/storage_item/fail/double_storage_item.rs:2:1
+  |
+2 | #[ink_lang::storage_item(derive = true)]
+  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

crates/primitives/derive/src/storable.rs

@@ -60,6 +60,15 @@ fn storable_enum_derive(s: &synstructure::Structure) -> TokenStream2 {
         !s.variants().is_empty(),
         "encountered invalid empty enum type deriving Storable trait"
     );
+
+    if s.variants().len() > 256 {
+        return syn::Error::new(
+            s.ast().span(),
+            "Currently only enums with at most 256 variants are supported.",
+        )
+        .to_compile_error()
+    }
+
     let decode_body = s
         .variants()
         .iter()
@@ -132,17 +141,7 @@ pub fn storable_derive(mut s: synstructure::Structure) -> TokenStream2 {
         .underscore_const(true);
     match &s.ast().data {
         syn::Data::Struct(_) => storable_struct_derive(&s),
-        syn::Data::Enum(data) => {
-            if s.variants().len() > 256 {
-                return syn::Error::new(
-                    data.variants.span(),
-                    "Currently only enums with at most 256 variants are supported.",
-                )
-                .to_compile_error()
-            }
-
-            storable_enum_derive(&s)
-        }
+        syn::Data::Enum(_) => storable_enum_derive(&s),
         _ => {
             panic!("cannot derive `Storable` for Rust `union` items")
         }