Fix clippy lints behind feature gates and add new CI step all features (

paritytech#2569)

Many clippy lints usually enforced by `-Dcomplexity` and `-Dcorrectness`
are not caught by CI as they are gated by `features`, like
`runtime-benchmarks`, while the clippy CI job runs with only the default
features for all targets.

This PR also adds a CI step to run clippy with `--all-features` to
ensure the code quality is maintained behind feature gates from now on.

To improve local development, clippy lints are downgraded to warnings,
but they still will result in an error at CI due to the `-Dwarnings`
rustflag.

---------

Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>

substrate/frame/alliance/src/benchmarking.rs

@@ -183,20 +183,15 @@ mod benchmarks {
 			let voter = &members[j as usize];
 			Alliance::<T, I>::vote(
 				SystemOrigin::Signed(voter.clone()).into(),
-				last_hash.clone(),
+				last_hash,
 				index,
 				true,
 			)?;
 		}
 
 		let voter = members[m as usize - 3].clone();
 		// Voter votes aye without resolving the vote.
-		Alliance::<T, I>::vote(
-			SystemOrigin::Signed(voter.clone()).into(),
-			last_hash.clone(),
-			index,
-			true,
-		)?;
+		Alliance::<T, I>::vote(SystemOrigin::Signed(voter.clone()).into(), last_hash, index, true)?;
 
 		// Voter switches vote to nay, but does not kill the vote, just updates + inserts
 		let approve = false;
@@ -206,7 +201,7 @@ mod benchmarks {
 		frame_benchmarking::benchmarking::add_to_whitelist(voter_key.into());
 
 		#[extrinsic_call]
-		_(SystemOrigin::Signed(voter), last_hash.clone(), index, approve);
+		_(SystemOrigin::Signed(voter), last_hash, index, approve);
 
 		//nothing to verify
 		Ok(())
@@ -255,24 +250,19 @@ mod benchmarks {
 			let voter = &members[j as usize];
 			Alliance::<T, I>::vote(
 				SystemOrigin::Signed(voter.clone()).into(),
-				last_hash.clone(),
+				last_hash,
 				index,
 				true,
 			)?;
 		}
 
 		// Voter votes aye without resolving the vote.
-		Alliance::<T, I>::vote(
-			SystemOrigin::Signed(voter.clone()).into(),
-			last_hash.clone(),
-			index,
-			true,
-		)?;
+		Alliance::<T, I>::vote(SystemOrigin::Signed(voter.clone()).into(), last_hash, index, true)?;
 
 		// Voter switches vote to nay, which kills the vote
 		Alliance::<T, I>::vote(
 			SystemOrigin::Signed(voter.clone()).into(),
-			last_hash.clone(),
+			last_hash,
 			index,
 			false,
 		)?;
@@ -282,7 +272,7 @@ mod benchmarks {
 		frame_benchmarking::benchmarking::add_to_whitelist(voter_key.into());
 
 		#[extrinsic_call]
-		close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage);
+		close(SystemOrigin::Signed(voter), last_hash, index, Weight::MAX, bytes_in_storage);
 
 		assert_eq!(T::ProposalProvider::proposal_of(last_hash), None);
 		Ok(())
@@ -330,7 +320,7 @@ mod benchmarks {
 		// approval vote
 		Alliance::<T, I>::vote(
 			SystemOrigin::Signed(proposer.clone()).into(),
-			last_hash.clone(),
+			last_hash,
 			index,
 			false,
 		)?;
@@ -340,7 +330,7 @@ mod benchmarks {
 			let voter = &members[j as usize];
 			Alliance::<T, I>::vote(
 				SystemOrigin::Signed(voter.clone()).into(),
-				last_hash.clone(),
+				last_hash,
 				index,
 				false,
 			)?;
@@ -349,22 +339,17 @@ mod benchmarks {
 		// Member zero is the first aye
 		Alliance::<T, I>::vote(
 			SystemOrigin::Signed(members[0].clone()).into(),
-			last_hash.clone(),
+			last_hash,
 			index,
 			true,
 		)?;
 
 		let voter = members[1].clone();
 		// Caller switches vote to aye, which passes the vote
-		Alliance::<T, I>::vote(
-			SystemOrigin::Signed(voter.clone()).into(),
-			last_hash.clone(),
-			index,
-			true,
-		)?;
+		Alliance::<T, I>::vote(SystemOrigin::Signed(voter.clone()).into(), last_hash, index, true)?;
 
 		#[extrinsic_call]
-		close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage);
+		close(SystemOrigin::Signed(voter), last_hash, index, Weight::MAX, bytes_in_storage);
 
 		assert_eq!(T::ProposalProvider::proposal_of(last_hash), None);
 		Ok(())
@@ -414,23 +399,23 @@ mod benchmarks {
 			let voter = &members[j as usize];
 			Alliance::<T, I>::vote(
 				SystemOrigin::Signed(voter.clone()).into(),
-				last_hash.clone(),
+				last_hash,
 				index,
 				true,
 			)?;
 		}
 
 		Alliance::<T, I>::vote(
 			SystemOrigin::Signed(voter.clone()).into(),
-			last_hash.clone(),
+			last_hash,
 			index,
 			false,
 		)?;
 
 		System::<T>::set_block_number(BlockNumberFor::<T>::max_value());
 
 		#[extrinsic_call]
-		close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage);
+		close(SystemOrigin::Signed(voter), last_hash, index, Weight::MAX, bytes_in_storage);
 
 		// The last proposal is removed.
 		assert_eq!(T::ProposalProvider::proposal_of(last_hash), None);
@@ -477,7 +462,7 @@ mod benchmarks {
 		// The prime member votes aye, so abstentions default to aye.
 		Alliance::<T, I>::vote(
 			SystemOrigin::Signed(proposer.clone()).into(),
-			last_hash.clone(),
+			last_hash,
 			p - 1,
 			true, // Vote aye.
 		)?;
@@ -489,7 +474,7 @@ mod benchmarks {
 			let voter = &members[j as usize];
 			Alliance::<T, I>::vote(
 				SystemOrigin::Signed(voter.clone()).into(),
-				last_hash.clone(),
+				last_hash,
 				index,
 				false,
 			)?;
@@ -499,13 +484,7 @@ mod benchmarks {
 		System::<T>::set_block_number(BlockNumberFor::<T>::max_value());
 
 		#[extrinsic_call]
-		close(
-			SystemOrigin::Signed(proposer),
-			last_hash.clone(),
-			index,
-			Weight::MAX,
-			bytes_in_storage,
-		);
+		close(SystemOrigin::Signed(proposer), last_hash, index, Weight::MAX, bytes_in_storage);
 
 		assert_eq!(T::ProposalProvider::proposal_of(last_hash), None);
 		Ok(())

substrate/frame/bounties/src/benchmarking.rs

@@ -222,7 +222,7 @@ benchmarks_instance_pallet! {
 		);
 	}
 	verify {
-		ensure!(missed_any == false, "Missed some");
+		ensure!(!missed_any, "Missed some");
 		if b > 0 {
 			ensure!(budget_remaining < BalanceOf::<T, I>::max_value(), "Budget not used");
 			assert_last_event::<T, I>(Event::BountyBecameActive { index: b - 1 }.into())

substrate/frame/contracts/src/benchmarking/mod.rs

@@ -1749,7 +1749,7 @@ benchmarks! {
 			.collect::<Vec<BalanceOf<T>>>();
 		let deposits_bytes: Vec<u8> = deposits.iter().flat_map(|i| i.encode()).collect();
 		let deposits_len = deposits_bytes.len() as u32;
-		let deposit_len = value_len.clone();
+		let deposit_len = value_len;
 		let callee_offset = value_len + deposits_len;
 		let code = WasmModule::<T>::from(ModuleDefinition {
 			memory: Some(ImportedMemory::max::<T>()),
@@ -2246,13 +2246,12 @@ benchmarks! {
 		let message_len = message.len() as i32;
 		let key_type = sp_core::crypto::KeyTypeId(*b"code");
 		let sig_params = (0..r)
-			.map(|i| {
+			.flat_map(|i| {
 				let pub_key = sp_io::crypto::sr25519_generate(key_type, None);
 				let sig = sp_io::crypto::sr25519_sign(key_type, &pub_key, &message).expect("Generates signature");
 				let data: [u8; 96] = [AsRef::<[u8]>::as_ref(&sig), AsRef::<[u8]>::as_ref(&pub_key)].concat().try_into().unwrap();
 				data
 			})
-			.flatten()
 			.collect::<Vec<_>>();
 		let sig_params_len = sig_params.len() as i32;
 

substrate/frame/contracts/src/migration/v12.rs

@@ -317,7 +317,7 @@ where
 		let (_, old_deposit, storage_module) = state;
 		// CodeInfoOf::max_encoded_len == OwnerInfoOf::max_encoded_len + 1
 		// I.e. code info adds up 1 byte per record.
-		let info_bytes_added = items.clone();
+		let info_bytes_added = items;
 		// We removed 1 PrefabWasmModule, and added 1 byte of determinism flag, per contract code.
 		let storage_removed = storage_module.saturating_sub(info_bytes_added);
 		// module+code+info - bytes

substrate/frame/democracy/src/benchmarking.rs

@@ -65,7 +65,7 @@ fn add_referendum<T: Config>(n: u32) -> (ReferendumIndex, T::Hash, T::Hash) {
 		0u32.into(),
 	);
 	let preimage_hash = note_preimage::<T>();
-	MetadataOf::<T>::insert(crate::MetadataOwner::Referendum(index), preimage_hash.clone());
+	MetadataOf::<T>::insert(crate::MetadataOwner::Referendum(index), preimage_hash);
 	(index, hash, preimage_hash)
 }
 

substrate/frame/democracy/src/migrations/v1.rs

@@ -172,7 +172,7 @@ mod test {
 			let hash = H256::repeat_byte(1);
 			let status = ReferendumStatus {
 				end: 1u32.into(),
-				proposal: hash.clone(),
+				proposal: hash,
 				threshold: VoteThreshold::SuperMajorityApprove,
 				delay: 1u32.into(),
 				tally: Tally { ayes: 1u32.into(), nays: 1u32.into(), turnout: 1u32.into() },
@@ -187,13 +187,10 @@ mod test {
 
 			// Case 3: Public proposals
 			let hash2 = H256::repeat_byte(2);
-			v0::PublicProps::<T>::put(vec![
-				(3u32, hash.clone(), 123u64),
-				(4u32, hash2.clone(), 123u64),
-			]);
+			v0::PublicProps::<T>::put(vec![(3u32, hash, 123u64), (4u32, hash2, 123u64)]);
 
 			// Case 4: Next external
-			v0::NextExternal::<T>::put((hash.clone(), VoteThreshold::SuperMajorityApprove));
+			v0::NextExternal::<T>::put((hash, VoteThreshold::SuperMajorityApprove));
 
 			// Migrate.
 			let state = v1::Migration::<T>::pre_upgrade().unwrap();

substrate/frame/fast-unstake/src/benchmarking.rs

@@ -162,7 +162,7 @@ benchmarks! {
 			fast_unstake_events::<T>().last(),
 			Some(Event::BatchChecked { .. })
 		));
-		assert!(stashes.iter().all(|(s, _)| request.stashes.iter().find(|(ss, _)| ss == s).is_some()));
+		assert!(stashes.iter().all(|(s, _)| request.stashes.iter().any(|(ss, _)| ss == s)));
 	}
 
 	register_fast_unstake {

substrate/frame/recovery/src/benchmarking.rs

@@ -190,7 +190,7 @@ benchmarks! {
 
 		let recovery_config = RecoveryConfig {
 			delay_period: DEFAULT_DELAY.into(),
-			deposit: total_deposit.clone(),
+			deposit: total_deposit,
 			friends: bounded_friends.clone(),
 			threshold: n as u16,
 		};
@@ -243,7 +243,7 @@ benchmarks! {
 
 		let recovery_config = RecoveryConfig {
 			delay_period: 0u32.into(),
-			deposit: total_deposit.clone(),
+			deposit: total_deposit,
 			friends: bounded_friends.clone(),
 			threshold: n as u16,
 		};
@@ -294,7 +294,7 @@ benchmarks! {
 
 		let recovery_config = RecoveryConfig {
 			delay_period: DEFAULT_DELAY.into(),
-			deposit: total_deposit.clone(),
+			deposit: total_deposit,
 			friends: bounded_friends.clone(),
 			threshold: n as u16,
 		};
@@ -342,7 +342,7 @@ benchmarks! {
 
 		let recovery_config = RecoveryConfig {
 			delay_period: DEFAULT_DELAY.into(),
-			deposit: total_deposit.clone(),
+			deposit: total_deposit,
 			friends: bounded_friends.clone(),
 			threshold: n as u16,
 		};

substrate/frame/sassafras/src/benchmarking.rs

@@ -260,7 +260,7 @@ mod benchmarks {
 		// Update metadata
 		let mut meta = TicketsMeta::<T>::get();
 		meta.unsorted_tickets_count = tickets_count;
-		TicketsMeta::<T>::set(meta.clone());
+		TicketsMeta::<T>::set(meta);
 
 		log::debug!(target: LOG_TARGET, "Before sort: {:?}", meta);
 		#[block]

substrate/frame/scheduler/src/migration.rs

@@ -105,7 +105,7 @@ pub mod v3 {
 			// Check that no agenda overflows `MaxScheduledPerBlock`.
 			let max_scheduled_per_block = T::MaxScheduledPerBlock::get() as usize;
 			for (block_number, agenda) in Agenda::<T>::iter() {
-				if agenda.iter().cloned().filter_map(|s| s).count() > max_scheduled_per_block {
+				if agenda.iter().cloned().flatten().count() > max_scheduled_per_block {
 					log::error!(
 						target: TARGET,
 						"Would truncate agenda of block {:?} from {} items to {} items.",
@@ -119,7 +119,7 @@ pub mod v3 {
 			// Check that bounding the calls will not overflow `MAX_LENGTH`.
 			let max_length = T::Preimages::MAX_LENGTH as usize;
 			for (block_number, agenda) in Agenda::<T>::iter() {
-				for schedule in agenda.iter().cloned().filter_map(|s| s) {
+				for schedule in agenda.iter().cloned().flatten() {
 					match schedule.call {
 						frame_support::traits::schedule::MaybeHashed::Value(call) => {
 							let l = call.using_encoded(|c| c.len());
@@ -362,7 +362,7 @@ mod test {
 					Some(ScheduledV3Of::<Test> {
 						maybe_id: Some(vec![i as u8; 320]),
 						priority: 123,
-						call: MaybeHashed::Hash(undecodable_hash.clone()),
+						call: MaybeHashed::Hash(undecodable_hash),
 						maybe_periodic: Some((4u64, 20)),
 						origin: root(),
 						_phantom: PhantomData::<u64>::default(),

substrate/frame/staking/src/pallet/impls.rs

@@ -1801,7 +1801,7 @@ impl<T: Config> StakingInterface for Pallet<T> {
 		) {
 			let others = exposures
 				.iter()
-				.map(|(who, value)| IndividualExposure { who: who.clone(), value: value.clone() })
+				.map(|(who, value)| IndividualExposure { who: who.clone(), value: *value })
 				.collect::<Vec<_>>();
 			let exposure = Exposure { total: Default::default(), own: Default::default(), others };
 			EraInfo::<T>::set_exposure(*current_era, stash, exposure);

substrate/frame/state-trie-migration/src/lib.rs

@@ -1637,7 +1637,7 @@ pub(crate) mod remote_tests {
 			weight_sum +=
 				StateTrieMigration::<Runtime>::on_initialize(System::<Runtime>::block_number());
 
-			root = System::<Runtime>::finalize().state_root().clone();
+			root = *System::<Runtime>::finalize().state_root();
 			System::<Runtime>::on_finalize(System::<Runtime>::block_number());
 		}
 		(root, weight_sum)
@@ -1687,7 +1687,7 @@ pub(crate) mod remote_tests {
 		);
 
 		loop {
-			let last_state_root = ext.backend.root().clone();
+			let last_state_root = *ext.backend.root();
 			let ((finished, weight), proof) = ext.execute_and_prove(|| {
 				let weight = run_to_block::<Runtime>(now + One::one()).1;
 				if StateTrieMigration::<Runtime>::migration_process().finished() {

substrate/frame/uniques/src/benchmarking.rs

@@ -431,9 +431,9 @@ benchmarks_instance_pallet! {
 		let buyer_lookup = T::Lookup::unlookup(buyer.clone());
 		let price = ItemPrice::<T, I>::from(0u32);
 		let origin = SystemOrigin::Signed(seller.clone()).into();
-		Uniques::<T, I>::set_price(origin, collection.clone(), item, Some(price.clone()), Some(buyer_lookup))?;
+		Uniques::<T, I>::set_price(origin, collection.clone(), item, Some(price), Some(buyer_lookup))?;
 		T::Currency::make_free_balance_be(&buyer, DepositBalanceOf::<T, I>::max_value());
-	}: _(SystemOrigin::Signed(buyer.clone()), collection.clone(), item, price.clone())
+	}: _(SystemOrigin::Signed(buyer.clone()), collection.clone(), item, price)
 	verify {
 		assert_last_event::<T, I>(Event::ItemBought {
 			collection: collection.clone(),

substrate/primitives/core/src/paired_crypto.rs

@@ -128,7 +128,7 @@ pub mod ecdsa_bls377 {
 			let Ok(right_sig) = sig.0[ecdsa::SIGNATURE_SERIALIZED_SIZE..].try_into() else {
 				return false
 			};
-			bls377::Pair::verify(&right_sig, message.as_ref(), &right_pub)
+			bls377::Pair::verify(&right_sig, message, &right_pub)
 		}
 	}
 }