Fix clippy lints behind feature gates and introduce new CI step targeting all features

.gitlab/pipeline/check.yml

@@ -8,6 +8,7 @@ cargo-clippy:
     RUSTFLAGS: "-D warnings"
   script:
     - SKIP_WASM_BUILD=1 cargo clippy --all-targets --locked --workspace
+    - SKIP_WASM_BUILD=1 cargo clippy --all-targets --all-features --locked --workspace
 
 check-try-runtime:
   stage: check

Cargo.toml

@@ -482,20 +482,21 @@ suspicious_double_ref_op = { level = "allow", priority = 2 }
 
 [workspace.lints.clippy]
 all = { level = "allow", priority = 0 }
-correctness = { level = "deny", priority = 1 }
+correctness = { level = "warn", priority = 1 }
+complexity = { level = "warn", priority = 1 }
 if-same-then-else = { level = "allow", priority = 2 }
-complexity = { level = "deny", priority = 1 }
 zero-prefixed-literal = { level = "allow", priority = 2 }            # 00_1000_000
 type_complexity = { level = "allow", priority = 2 }                  # raison d'etre
 nonminimal-bool = { level = "allow", priority = 2 }                  # maybe
 borrowed-box = { level = "allow", priority = 2 }                     # Reasonable to fix this one
 too-many-arguments = { level = "allow", priority = 2 }               # (Turning this on would lead to)
+needless-lifetimes = { level = "allow", priority = 2 }               # generated code
 unnecessary_cast = { level = "allow", priority = 2 }                 # Types may change
 identity-op = { level = "allow", priority = 2 }                      # One case where we do 0 +
 useless_conversion = { level = "allow", priority = 2 }               # Types may change
-unit_arg = { level = "allow", priority = 2 }                         # styalistic.
-option-map-unit-fn = { level = "allow", priority = 2 }               # styalistic
-bind_instead_of_map = { level = "allow", priority = 2 }              # styalistic
+unit_arg = { level = "allow", priority = 2 }                         # stylistic
+option-map-unit-fn = { level = "allow", priority = 2 }               # stylistic
+bind_instead_of_map = { level = "allow", priority = 2 }              # stylistic
 erasing_op = { level = "allow", priority = 2 }                       # E.g. 0 * DOLLARS
 eq_op = { level = "allow", priority = 2 }                            # In tests we test equality.
 while_immutable_condition = { level = "allow", priority = 2 }        # false positives

bridges/modules/relayers/src/benchmarking.rs

@@ -104,7 +104,7 @@ benchmarks! {
 		// create slash destination account
 		let lane = LaneId([0, 0, 0, 0]);
 		let slash_destination = RewardsAccountParams::new(lane, *b"test", RewardsAccountOwner::ThisChain);
-		T::prepare_rewards_account(slash_destination.clone(), Zero::zero());
+		T::prepare_rewards_account(slash_destination, Zero::zero());
 	}: {
 		crate::Pallet::<T>::slash_and_deregister(&relayer, slash_destination)
 	}
@@ -121,7 +121,7 @@ benchmarks! {
 		let account_params =
 			RewardsAccountParams::new(lane, *b"test", RewardsAccountOwner::ThisChain);
 	}: {
-		crate::Pallet::<T>::register_relayer_reward(account_params.clone(), &relayer, One::one());
+		crate::Pallet::<T>::register_relayer_reward(account_params, &relayer, One::one());
 	}
 	verify {
 		assert_eq!(RelayerRewards::<T>::get(relayer, &account_params), Some(One::one()));

cumulus/pallets/xcmp-queue/src/benchmarking.rs

@@ -85,7 +85,7 @@ mod benchmarks {
 		}
 
 		assert!(
-			OutboundXcmpStatus::<T>::get().iter().find(|p| p.recipient == para).is_none(),
+			!OutboundXcmpStatus::<T>::get().iter().any(|p| p.recipient == para),
 			"No messages in the channel; therefore removed."
 		);
 	}

cumulus/parachains/pallets/collective-content/src/benchmarking.rs

@@ -56,7 +56,7 @@ mod benchmarks {
 			.map_err(|_| BenchmarkError::Weightless)?;
 
 		#[extrinsic_call]
-		_(origin as T::RuntimeOrigin, cid.clone(), Some(expire_at.clone()));
+		_(origin as T::RuntimeOrigin, cid.clone(), Some(expire_at));
 
 		assert_eq!(<Announcements<T, I>>::count(), 1);
 		assert_last_event::<T, I>(

polkadot/runtime/parachains/src/paras_inherent/benchmarking.rs

@@ -132,7 +132,7 @@ benchmarks! {
 		// Ensure that the votes are for the correct session
 		assert_eq!(vote.session, scenario._session);
 		// Ensure that there are an expected number of candidates
-		let header = BenchBuilder::<T>::header(scenario._block_number.clone());
+		let header = BenchBuilder::<T>::header(scenario._block_number);
 		// Traverse candidates and assert descriptors are as expected
 		for (para_id, backing_validators) in vote.backing_validators_per_candidate.iter().enumerate() {
 			let descriptor = backing_validators.0.descriptor();
@@ -189,7 +189,7 @@ benchmarks! {
 		// Ensure that the votes are for the correct session
 		assert_eq!(vote.session, scenario._session);
 		// Ensure that there are an expected number of candidates
-		let header = BenchBuilder::<T>::header(scenario._block_number.clone());
+		let header = BenchBuilder::<T>::header(scenario._block_number);
 		// Traverse candidates and assert descriptors are as expected
 		for (para_id, backing_validators)
 			in vote.backing_validators_per_candidate.iter().enumerate() {

polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs

@@ -175,7 +175,7 @@ benchmarks! {
 	descend_origin {
 		let mut executor = new_executor::<T>(Default::default());
 		let who = X2(OnlyChild, OnlyChild);
-		let instruction = Instruction::DescendOrigin(who.clone());
+		let instruction = Instruction::DescendOrigin(who);
 		let xcm = Xcm(vec![instruction]);
 	} : {
 		executor.bench_process(xcm)?;
@@ -242,7 +242,7 @@ benchmarks! {
 			&origin,
 			assets.clone().into(),
 			&XcmContext {
-				origin: Some(origin.clone()),
+				origin: Some(origin),
 				message_id: [0; 32],
 				topic: None,
 			},
@@ -279,7 +279,7 @@ benchmarks! {
 		let origin = T::subscribe_origin()?;
 		let query_id = Default::default();
 		let max_response_weight = Default::default();
-		let mut executor = new_executor::<T>(origin.clone());
+		let mut executor = new_executor::<T>(origin);
 		let instruction = Instruction::SubscribeVersion { query_id, max_response_weight };
 		let xcm = Xcm(vec![instruction]);
 	} : {
@@ -299,14 +299,14 @@ benchmarks! {
 			query_id,
 			max_response_weight,
 			&XcmContext {
-				origin: Some(origin.clone()),
+				origin: Some(origin),
 				message_id: [0; 32],
 				topic: None,
 			},
 		).map_err(|_| "Could not start subscription")?;
 		assert!(<T::XcmConfig as xcm_executor::Config>::SubscriptionService::is_subscribed(&origin));
 
-		let mut executor = new_executor::<T>(origin.clone());
+		let mut executor = new_executor::<T>(origin);
 		let instruction = Instruction::UnsubscribeVersion;
 		let xcm = Xcm(vec![instruction]);
 	} : {
@@ -538,7 +538,7 @@ benchmarks! {
 
 		let mut executor = new_executor::<T>(origin);
 
-		let instruction = Instruction::UniversalOrigin(alias.clone());
+		let instruction = Instruction::UniversalOrigin(alias);
 		let xcm = Xcm(vec![instruction]);
 	}: {
 		executor.bench_process(xcm)?;
@@ -632,13 +632,13 @@ benchmarks! {
 
 		let (unlocker, owner, asset) = T::unlockable_asset()?;
 
-		let mut executor = new_executor::<T>(unlocker.clone());
+		let mut executor = new_executor::<T>(unlocker);
 
 		// We first place the asset in lock first...
 		<T::XcmConfig as xcm_executor::Config>::AssetLocker::prepare_lock(
 			unlocker,
 			asset.clone(),
-			owner.clone(),
+			owner,
 		)
 		.map_err(|_| BenchmarkError::Skip)?
 		.enact()
@@ -658,13 +658,13 @@ benchmarks! {
 
 		let (unlocker, owner, asset) = T::unlockable_asset()?;
 
-		let mut executor = new_executor::<T>(unlocker.clone());
+		let mut executor = new_executor::<T>(unlocker);
 
 		// We first place the asset in lock first...
 		<T::XcmConfig as xcm_executor::Config>::AssetLocker::prepare_lock(
 			unlocker,
 			asset.clone(),
-			owner.clone(),
+			owner,
 		)
 		.map_err(|_| BenchmarkError::Skip)?
 		.enact()
@@ -686,9 +686,9 @@ benchmarks! {
 
 		// We first place the asset in lock first...
 		<T::XcmConfig as xcm_executor::Config>::AssetLocker::prepare_lock(
-			locker.clone(),
+			locker,
 			asset.clone(),
-			owner.clone(),
+			owner,
 		)
 		.map_err(|_| BenchmarkError::Skip)?
 		.enact()
@@ -739,7 +739,7 @@ benchmarks! {
 
 		let mut executor = new_executor::<T>(origin);
 
-		let instruction = Instruction::AliasOrigin(target.clone());
+		let instruction = Instruction::AliasOrigin(target);
 		let xcm = Xcm(vec![instruction]);
 	}: {
 		executor.bench_process(xcm)?;

polkadot/xcm/xcm-simulator/example/src/parachain.rs

@@ -165,7 +165,7 @@ impl EnsureOriginWithArg<RuntimeOrigin, MultiLocation> for ForeignCreators {
 
 	#[cfg(feature = "runtime-benchmarks")]
 	fn try_successful_origin(a: &MultiLocation) -> Result<RuntimeOrigin, ()> {
-		Ok(pallet_xcm::Origin::Xcm(a.clone()).into())
+		Ok(pallet_xcm::Origin::Xcm(*a).into())
 	}
 }
 

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())