Make CI check format of UI tests (#534)

* [ci] Check format of UI tests as well

* [lang] Apply our formatting guidelines to UI tests

* [ci] Make Gitlab happy

* [ci] Remove superfluous space

* [ci] Output rustfmt version

* [ci] Tune rustfmt params

* Remove todo's

* Migrate erc721 ui test contract to recent example contract

* Remove --version command used to debug

* Apply cargo fmt

* .gitlab-ci.yml formatting

* Add main fn

* Remove todo from stderr expect

* Adapt fixtures

* Revert "Adapt fixtures"

This reverts commit 174c012e.

* Adapt fixtures

.gitlab-ci.yml

@@ -187,6 +187,9 @@ fmt:
   script:
     - cargo fmt --verbose --all -- --check
 
+    # For the UI tests we need to disable the license check
+    - cargo fmt --verbose --all -- --check --config=license_template_path="" crates/lang/macro/tests/ui/{pass,fail}/*.rs
+
 
 #### stage:                        examples
 

crates/lang/macro/tests/ui/fail/M-05-message-invalid-selector.rs

@@ -12,7 +12,7 @@ mod message_invalid_selector {
         }
 
         #[ink(message, selector = "0x00")]
-        pub fn invalid_selector(&self) { }
+        pub fn invalid_selector(&self) {}
     }
 }
 

crates/lang/macro/tests/ui/fail/S-04-non-storage-ink-impls.rs

@@ -11,7 +11,9 @@ mod non_storage_ink_impls {
     // This ink! impl block is okay.
     impl StorageStruct {
         #[ink(constructor)]
-        pub fn constructor1() -> Self { todo!() }
+        pub fn constructor1() -> Self {
+            Self {}
+        }
 
         #[ink(message)]
         pub fn message1(&self) {}
@@ -25,7 +27,9 @@ mod non_storage_ink_impls {
     // storage struct. We expect a failure here.
     impl NonStorageStruct {
         #[ink(constructor)]
-        pub fn constructor2() -> Self { todo!() }
+        pub fn constructor2() -> Self {
+            Self {}
+        }
 
         #[ink(message)]
         pub fn message2(&self) {}

crates/lang/macro/tests/ui/fail/S-04-non-storage-ink-impls.stderr

 error[E0271]: type mismatch resolving `<NonStorageStruct as non_storage_ink_impls::_::_::{closure#0}::TypeEq>::This == StorageStruct`
-  --> $DIR/S-04-non-storage-ink-impls.rs:26:10
+  --> $DIR/S-04-non-storage-ink-impls.rs:28:10
    |
-26 |     impl NonStorageStruct {
+28 |     impl NonStorageStruct {
    |          ^^^^^^^^^^^^^^^^
    |          |
    |          expected struct `StorageStruct`, found struct `NonStorageStruct`
@@ -11,24 +11,24 @@ error[E0271]: type mismatch resolving `<NonStorageStruct as non_storage_ink_impl
    = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error[E0599]: no function or associated item named `constructor2` found for struct `StorageStruct` in the current scope
-  --> $DIR/S-04-non-storage-ink-impls.rs:28:16
+  --> $DIR/S-04-non-storage-ink-impls.rs:30:16
    |
 9  |     pub struct StorageStruct {}
    |     ------------------------ function or associated item `constructor2` not found for this
 ...
-28 |         pub fn constructor2() -> Self { todo!() }
+30 |         pub fn constructor2() -> Self {
    |                ^^^^^^^^^^^^
    |                |
    |                function or associated item not found in `StorageStruct`
    |                help: there is an associated function with a similar name: `constructor1`
 
 error[E0599]: no function or associated item named `message2` found for struct `StorageStruct` in the current scope
-  --> $DIR/S-04-non-storage-ink-impls.rs:31:16
+  --> $DIR/S-04-non-storage-ink-impls.rs:35:16
    |
 9  |     pub struct StorageStruct {}
    |     ------------------------ function or associated item `message2` not found for this
 ...
-31 |         pub fn message2(&self) {}
+35 |         pub fn message2(&self) {}
    |                ^^^^^^^^
    |                |
    |                function or associated item not found in `StorageStruct`

crates/lang/macro/tests/ui/pass/02-flipper-contract.rs

@@ -10,9 +10,7 @@ mod flipper {
     impl Flipper {
         #[ink(constructor)]
         pub fn new(init_value: bool) -> Self {
-            Self {
-                value: init_value,
-            }
+            Self { value: init_value }
         }
 
         #[ink(constructor)]

crates/lang/macro/tests/ui/pass/05-erc721-contract.rs

@@ -2,262 +2,685 @@ use ink_lang as ink;
 
 #[ink::contract]
 mod erc721 {
-    use ink_storage::collections::HashMap as StorageHashMap;
+    #[cfg(not(feature = "ink-as-dependency"))]
+    use ink_storage::collections::{
+        hashmap::Entry,
+        HashMap as StorageHashMap,
+    };
+    use scale::{
+        Decode,
+        Encode,
+    };
 
     /// A token ID.
     pub type TokenId = u32;
 
-    #[derive(scale::Encode, scale::Decode)]
-    #[cfg_attr(feature = "std", derive(scale_info::TypeInfo))]
-    pub enum Error {
-        SpecifiedTokenHasNoOwner,
-        ApprovalToCurrentOwner,
-        ApproveCallerNotLegitimate,
-        ApprovedQueryForNonexistentToken,
-        ApproveToCaller,
-        TransferCallerIsNotOwnerOrApproved,
-        OperatorQueryForNonexistentToken,
-        TransferOfTokenThatIsNotOwned,
-        TokenAlreadyMinted,
-        CannotBurnNonexistentToken,
-    }
-
-    pub type Result<T> = core::result::Result<T, Error>;
-
-    /// The storage items for a typical ERC721 token implementation.
     #[ink(storage)]
     #[derive(Default)]
     pub struct Erc721 {
-        /// Stores one owner for every token.
+        /// Mapping from token to owner.
         token_owner: StorageHashMap<TokenId, AccountId>,
-        /// Mapping from token ID to approved address.
+        /// Mapping from token to approvals users.
         token_approvals: StorageHashMap<TokenId, AccountId>,
-        /// Mapping from owner to number of owned tokens.
+        /// Mapping from owner to number of owned token.
         owned_tokens_count: StorageHashMap<AccountId, u32>,
-        /// Mapping from owner to operator approval.
+        /// Mapping from owner to operator approvals.
         operator_approvals: StorageHashMap<(AccountId, AccountId), bool>,
     }
 
-    /// Notifies about token approvals.
+    #[derive(Encode, Decode, Debug, PartialEq, Eq, Copy, Clone)]
+    #[cfg_attr(feature = "std", derive(scale_info::TypeInfo))]
+    pub enum Error {
+        NotOwner,
+        NotApproved,
+        TokenExists,
+        TokenNotFound,
+        CannotInsert,
+        CannotRemove,
+        CannotFetchValue,
+        NotAllowed,
+    }
+
+    /// Event emitted when a token transfer occurs.
     #[ink(event)]
-    pub struct Approval {
-        /// The owner of the token.
-        owner: AccountId,
-        /// The approved account.
-        to: AccountId,
-        /// The approved token.
-        token: TokenId,
+    pub struct Transfer {
+        #[ink(topic)]
+        from: Option<AccountId>,
+        #[ink(topic)]
+        to: Option<AccountId>,
+        #[ink(topic)]
+        id: TokenId,
     }
 
-    /// Notifies about approval for all tokens.
+    /// Event emitted when a token approve occurs.
     #[ink(event)]
-    pub struct ApprovalForAll {
-        /// The source.
+    pub struct Approval {
+        #[ink(topic)]
         from: AccountId,
-        /// The destination.
+        #[ink(topic)]
         to: AccountId,
-        /// If it was approved.
-        approved: bool,
+        #[ink(topic)]
+        id: TokenId,
     }
 
-    /// Notifies about token transfers.
+    /// Event emitted when an operator is enabled or disabled for an owner.
+    /// The operator can manage all NFTs of the owner.
     #[ink(event)]
-    pub struct Transfer {
-        /// The source of the transfered token.
-        from: Option<AccountId>,
-        /// The destination of the transfered token.
-        to: Option<AccountId>,
-        /// The transfered token.
-        token: TokenId,
+    pub struct ApprovalForAll {
+        #[ink(topic)]
+        owner: AccountId,
+        #[ink(topic)]
+        operator: AccountId,
+        approved: bool,
     }
 
     impl Erc721 {
-        /// Nothing to do for initialization.
+        /// Creates a new ERC721 token contract.
         #[ink(constructor)]
         pub fn new() -> Self {
-            Default::default()
+            Self {
+                token_owner: Default::default(),
+                token_approvals: Default::default(),
+                owned_tokens_count: Default::default(),
+                operator_approvals: Default::default(),
+            }
         }
 
-        /// Returns the balance of the specified address.
+        /// Returns the balance of the owner.
         ///
-        /// # Note
-        ///
-        /// The returned amount represents the number of owned tokens by the address.
+        /// This represents the amount of unique tokens the owner has.
         #[ink(message)]
         pub fn balance_of(&self, owner: AccountId) -> u32 {
-            *self.owned_tokens_count.get(&owner).unwrap_or(&0)
+            self.balance_of_or_zero(&owner)
         }
 
-        /// Returns the owner of the specified token ID if any.
+        /// Returns the owner of the token.
         #[ink(message)]
-        pub fn owner_of(&self, token: TokenId) -> Option<AccountId> {
-            self.token_owner.get(&token).cloned()
+        pub fn owner_of(&self, id: TokenId) -> Option<AccountId> {
+            self.token_owner.get(&id).cloned()
         }
 
-        /// Approves another address to transfer the given token ID.
-        ///
-        /// There can only be one approved address per token at a given time.
-        /// Can only be called by the token owner or an approved operator.
+        /// Returns the approved account ID for this token if any.
         #[ink(message)]
-        pub fn approve(&mut self, to: AccountId, token: TokenId) -> Result<()> {
-            let owner = self
-                .owner_of(token)
-                .ok_or(Error::SpecifiedTokenHasNoOwner)?;
-            if to == owner {
-                return Err(Error::ApprovalToCurrentOwner)
-            }
-            let caller = self.env().caller();
-            if caller == owner || self.is_approved_for_all(owner, caller) {
-                return Err(Error::ApproveCallerNotLegitimate)
-            }
-            self.token_approvals.insert(token, to);
-            self.env().emit_event(Approval { owner, to, token });
-            Ok(())
+        pub fn get_approved(&self, id: TokenId) -> Option<AccountId> {
+            self.token_approvals.get(&id).cloned()
         }
 
-        /// Returns the approved address for the token ID if any.
-        ///
-        /// Reverts if the token ID does not exist.
+        /// Returns `true` if the operator is approved by the owner.
         #[ink(message)]
-        pub fn get_approved(&self, token: TokenId) -> Result<AccountId> {
-            self.token_owner
-                .get(&token)
-                .ok_or(Error::ApprovedQueryForNonexistentToken)
-                .map(Clone::clone)
+        pub fn is_approved_for_all(&self, owner: AccountId, operator: AccountId) -> bool {
+            self.approved_for_all(owner, operator)
         }
 
-        /// Sets of unsets the approval of a given operator.
-        ///
-        /// An operator is allowed to transfer all tokens of the sender on their behalf.
+        /// Approves or disapproves the operator for all tokens of the caller.
         #[ink(message)]
         pub fn set_approval_for_all(
             &mut self,
             to: AccountId,
             approved: bool,
-        ) -> Result<()> {
-            let caller = self.env().caller();
-            if to == caller {
-                return Err(Error::ApproveToCaller)
+        ) -> Result<(), Error> {
+            self.approve_for_all(to, approved)?;
+            Ok(())
         }
-            self.operator_approvals.insert((caller, to), approved);
-            self.env().emit_event(ApprovalForAll {
-                from: caller,
-                to,
-                approved,
-            });
+
+        /// Approves the account to transfer the specified token on behalf of the caller.
+        #[ink(message)]
+        pub fn approve(&mut self, to: AccountId, id: TokenId) -> Result<(), Error> {
+            self.approve_for(&to, id)?;
             Ok(())
         }
 
-        /// Returns `true` if an operator is approved by a given owner.
+        /// Transfers the token from the caller to the given destination.
         #[ink(message)]
-        pub fn is_approved_for_all(&self, owner: AccountId, operator: AccountId) -> bool {
-            *self
-                .operator_approvals
-                .get(&(owner, operator))
-                .unwrap_or(&false)
+        pub fn transfer(
+            &mut self,
+            destination: AccountId,
+            id: TokenId,
+        ) -> Result<(), Error> {
+            let caller = self.env().caller();
+            self.transfer_token_from(&caller, &destination, id)?;
+            Ok(())
         }
 
-        /// Transfers the ownership of a given token ID to another address.
-        ///
-        /// # Note
-        ///
-        /// Usage of this method is discouraged, use `safe_transfer_from` whenever possible.
-        ///
-        /// # Errors
-        ///
-        /// If the caller is not the owner, approved or operator.
+        /// Transfer approved or owned token.
         #[ink(message)]
         pub fn transfer_from(
             &mut self,
             from: AccountId,
             to: AccountId,
-            token: TokenId,
-        ) -> Result<()> {
-            let caller = self.env().caller();
-            if !self.is_approved_or_owner(&caller, token) {
-                return Err(Error::TransferCallerIsNotOwnerOrApproved)
+            id: TokenId,
+        ) -> Result<(), Error> {
+            self.transfer_token_from(&from, &to, id)?;
+            Ok(())
         }
-            self.transfer_from_impl(from, to, token)?;
+
+        /// Creates a new token.
+        #[ink(message)]
+        pub fn mint(&mut self, id: TokenId) -> Result<(), Error> {
+            let caller = self.env().caller();
+            self.add_token_to(&caller, id)?;
+            self.env().emit_event(Transfer {
+                from: Some(AccountId::from([0x0; 32])),
+                to: Some(caller),
+                id,
+            });
             Ok(())
         }
 
-        /// Returns `true` if the given spender can transfer the given token.
-        fn is_approved_or_owner(&self, spender: &AccountId, token: TokenId) -> bool {
-            self.token_owner
-                .get(&token)
-                .ok_or(Error::OperatorQueryForNonexistentToken)
-                .map(|&owner| {
-                    let approved = self.get_approved(token).unwrap_or(owner);
-                    *spender == owner
-                        || approved == *spender
-                        || self.is_approved_for_all(owner, *spender)
-                })
-                .unwrap_or(false)
+        /// Deletes an existing token. Only the owner can burn the token.
+        #[ink(message)]
+        pub fn burn(&mut self, id: TokenId) -> Result<(), Error> {
+            let caller = self.env().caller();
+            let Self {
+                token_owner,
+                owned_tokens_count,
+                ..
+            } = self;
+            let occupied = match token_owner.entry(id) {
+                Entry::Vacant(_) => return Err(Error::TokenNotFound),
+                Entry::Occupied(occupied) => occupied,
+            };
+            if occupied.get() != &caller {
+                return Err(Error::NotOwner)
+            };
+            decrease_counter_of(owned_tokens_count, &caller)?;
+            occupied.remove_entry();
+            self.env().emit_event(Transfer {
+                from: Some(caller),
+                to: Some(AccountId::from([0x0; 32])),
+                id,
+            });
+            Ok(())
         }
 
-        /// Transfers ownership of the token to another address.
-        ///
-        /// # Safety
-        ///
-        /// As opposed to `transfer_from` this imposes no restructions on the `caller`.
-        fn transfer_from_impl(
+        /// Transfers token `id` `from` the sender to the `to` AccountId.
+        fn transfer_token_from(
             &mut self,
-            from: AccountId,
-            to: AccountId,
-            token: TokenId,
-        ) -> Result<()> {
-            if self.owner_of(token).unwrap_or(from) != from {
-                return Err(Error::TransferOfTokenThatIsNotOwned)
-            }
-            self.clear_approval(token);
-            self.owned_tokens_count[&from] -= 1; // TODO: are these calls safe here?
-            self.owned_tokens_count[&to] += 1;
-            self.token_owner[&token] = to;
+            from: &AccountId,
+            to: &AccountId,
+            id: TokenId,
+        ) -> Result<(), Error> {
+            let caller = self.env().caller();
+            if !self.exists(id) {
+                return Err(Error::TokenNotFound)
+            };
+            if !self.approved_or_owner(Some(caller), id) {
+                return Err(Error::NotApproved)
+            };
+            self.clear_approval(id)?;
+            self.remove_token_from(from, id)?;
+            self.add_token_to(to, id)?;
             self.env().emit_event(Transfer {
-                from: Some(from),
-                to: Some(to),
-                token,
+                from: Some(*from),
+                to: Some(*to),
+                id,
             });
             Ok(())
         }
 
-        /// Clears the current approval of a given token.
-        fn clear_approval(&mut self, token: TokenId) {
-            self.token_approvals.take(&token);
+        /// Removes token `id` from the owner.
+        fn remove_token_from(
+            &mut self,
+            from: &AccountId,
+            id: TokenId,
+        ) -> Result<(), Error> {
+            let Self {
+                token_owner,
+                owned_tokens_count,
+                ..
+            } = self;
+            let occupied = match token_owner.entry(id) {
+                Entry::Vacant(_) => return Err(Error::TokenNotFound),
+                Entry::Occupied(occupied) => occupied,
+            };
+            decrease_counter_of(owned_tokens_count, from)?;
+            occupied.remove_entry();
+            Ok(())
         }
 
-        /// Mints a new token.
-        fn mint(&mut self, to: AccountId, token: TokenId) -> Result<()> {
-            let _ = self
-                .token_owner
-                .get(&token)
-                .ok_or(Error::TokenAlreadyMinted)?;
-            self.token_owner[&token] = to;
-            self.owned_tokens_count[&to] += 1;
-            self.env().emit_event(Transfer {
-                from: None,
-                to: Some(to),
-                token,
+        /// Adds the token `id` to the `to` AccountID.
+        fn add_token_to(&mut self, to: &AccountId, id: TokenId) -> Result<(), Error> {
+            let Self {
+                token_owner,
+                owned_tokens_count,
+                ..
+            } = self;
+            let vacant_token_owner = match token_owner.entry(id) {
+                Entry::Vacant(vacant) => vacant,
+                Entry::Occupied(_) => return Err(Error::TokenExists),
+            };
+            if *to == AccountId::from([0x0; 32]) {
+                return Err(Error::NotAllowed)
+            };
+            let entry = owned_tokens_count.entry(*to);
+            increase_counter_of(entry)?;
+            vacant_token_owner.insert(*to);
+            Ok(())
+        }
+
+        /// Approves or disapproves the operator to transfer all tokens of the caller.
+        fn approve_for_all(
+            &mut self,
+            to: AccountId,
+            approved: bool,
+        ) -> Result<(), Error> {
+            let caller = self.env().caller();
+            if to == caller {
+                return Err(Error::NotAllowed)
+            }
+            self.env().emit_event(ApprovalForAll {
+                owner: caller,
+                operator: to,
+                approved,
             });
+            if self.approved_for_all(caller, to) {
+                let status = self
+                    .operator_approvals
+                    .get_mut(&(caller, to))
+                    .ok_or(Error::CannotFetchValue)?;
+                *status = approved;
                 Ok(())
+            } else {
+                match self.operator_approvals.insert((caller, to), approved) {
+                    Some(_) => Err(Error::CannotInsert),
+                    None => Ok(()),
+                }
+            }
         }
 
-        // Burns the token.
-        fn burn(&mut self, token: TokenId) -> Result<()> {
-            let owner = *self
-                .token_owner
-                .get(&token)
-                .ok_or(Error::CannotBurnNonexistentToken)?;
-            self.clear_approval(token);
-            self.owned_tokens_count[&owner] -= 1;
-            self.token_owner.take(&token);
-            self.env().emit_event(Transfer {
-                from: Some(owner),
-                to: None,
-                token,
+        /// Approve the passed AccountId to transfer the specified token on behalf of the message's sender.
+        fn approve_for(&mut self, to: &AccountId, id: TokenId) -> Result<(), Error> {
+            let caller = self.env().caller();
+            let owner = self.owner_of(id);
+            if !(owner == Some(caller)
+                || self.approved_for_all(owner.expect("Error with AccountId"), caller))
+            {
+                return Err(Error::NotAllowed)
+            };
+            if *to == AccountId::from([0x0; 32]) {
+                return Err(Error::NotAllowed)
+            };
+
+            if self.token_approvals.insert(id, *to).is_some() {
+                return Err(Error::CannotInsert)
+            };
+            self.env().emit_event(Approval {
+                from: caller,
+                to: *to,
+                id,
             });
             Ok(())
         }
+
+        /// Removes existing approval from token `id`.
+        fn clear_approval(&mut self, id: TokenId) -> Result<(), Error> {
+            if !self.token_approvals.contains_key(&id) {
+                return Ok(())
+            };
+            match self.token_approvals.take(&id) {
+                Some(_res) => Ok(()),