Make clippy _a little_ more annoying (#10570)

* Clippy: +complexity

* Update client/cli/src/arg_enums.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Update bin/node/inspect/src/lib.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Update primitives/keystore/src/testing.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Update frame/elections/src/lib.rs

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

* Update primitives/npos-elections/fuzzer/src/reduce.rs

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

* Incorporating feedback

* No need for Ok

* Additional

* Needed slice

* Wigy's suggestions on less derefs

* fix count

* reverting changes brought in by option_map_unit_fn

* add --all-targets

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

substrate/client/chain-spec/src/chain_spec.rs

@@ -223,7 +223,7 @@ impl<G, E> ChainSpec<G, E> {
 
 	/// Network protocol id.
 	pub fn protocol_id(&self) -> Option<&str> {
-		self.client_spec.protocol_id.as_ref().map(String::as_str)
+		self.client_spec.protocol_id.as_deref()
 	}
 
 	/// Additional loosly-typed properties of the chain.

substrate/client/cli/src/arg_enums.rs

@@ -51,7 +51,7 @@ impl std::str::FromStr for WasmExecutionMethod {
 			}
 			#[cfg(not(feature = "wasmtime"))]
 			{
-				Err(format!("`Compiled` variant requires the `wasmtime` feature to be enabled"))
+				Err("`Compiled` variant requires the `wasmtime` feature to be enabled".into())
 			}
 		} else {
 			Err(format!("Unknown variant `{}`, known variants: {:?}", s, Self::variants()))

substrate/client/cli/src/commands/sign.rs

@@ -70,7 +70,7 @@ fn sign<P: sp_core::Pair>(
 	message: Vec<u8>,
 ) -> error::Result<String> {
 	let pair = utils::pair_from_suri::<P>(suri, password)?;
-	Ok(format!("{}", hex::encode(pair.sign(&message))))
+	Ok(hex::encode(pair.sign(&message)))
 }
 
 #[cfg(test)]

substrate/client/cli/src/commands/verify.rs

@@ -57,7 +57,7 @@ impl VerifyCmd {
 		let message = utils::read_message(self.message.as_ref(), self.hex)?;
 		let sig_data = utils::decode_hex(&self.sig)?;
 		let uri = utils::read_uri(self.uri.as_ref())?;
-		let uri = if uri.starts_with("0x") { &uri[2..] } else { &uri };
+		let uri = if let Some(uri) = uri.strip_prefix("0x") { uri } else { &uri };
 
 		with_crypto_scheme!(self.crypto_scheme.scheme, verify(sig_data, message, uri))
 	}

substrate/client/cli/src/params/mod.rs

@@ -79,8 +79,8 @@ impl FromStr for BlockNumberOrHash {
 	type Err = String;
 
 	fn from_str(block_number: &str) -> Result<Self, Self::Err> {
-		if block_number.starts_with("0x") {
-			if let Some(pos) = &block_number[2..].chars().position(|c| !c.is_ascii_hexdigit()) {
+		if let Some(rest) = block_number.strip_prefix("0x") {
+			if let Some(pos) = rest.chars().position(|c| !c.is_ascii_hexdigit()) {
 				Err(format!(
 					"Expected block hash, found illegal hex character at position: {}",
 					2 + pos,

substrate/client/consensus/common/src/block_import.rs

@@ -287,9 +287,9 @@ impl<Block: BlockT, Transaction> BlockImportParams<Block, Transaction> {
 	pub fn take_intermediate<T: 'static>(&mut self, key: &[u8]) -> Result<Box<T>, Error> {
 		let (k, v) = self.intermediates.remove_entry(key).ok_or(Error::NoIntermediate)?;
 
-		v.downcast::<T>().or_else(|v| {
+		v.downcast::<T>().map_err(|v| {
 			self.intermediates.insert(k, v);
-			Err(Error::InvalidIntermediate)
+			Error::InvalidIntermediate
 		})
 	}
 

substrate/client/consensus/manual-seal/src/consensus/babe.rs

@@ -279,7 +279,7 @@ where
 		// a quick check to see if we're in the authorities
 		let epoch = self.epoch(parent, slot)?;
 		let (authority, _) = self.authorities.first().expect("authorities is non-emptyp; qed");
-		let has_authority = epoch.authorities.iter().find(|(id, _)| *id == *authority).is_some();
+		let has_authority = epoch.authorities.iter().any(|(id, _)| *id == *authority);
 
 		if !has_authority {
 			log::info!(target: "manual-seal", "authority not found");

substrate/client/consensus/manual-seal/src/lib.rs

@@ -173,7 +173,7 @@ pub async fn run_manual_seal<B, BI, CB, E, C, TP, SC, CS, CIDP>(
 					env: &mut env,
 					select_chain: &select_chain,
 					block_import: &mut block_import,
-					consensus_data_provider: consensus_data_provider.as_ref().map(|p| &**p),
+					consensus_data_provider: consensus_data_provider.as_deref(),
 					pool: pool.clone(),
 					client: client.clone(),
 					create_inherent_data_providers: &create_inherent_data_providers,
@@ -408,8 +408,8 @@ mod tests {
 		})
 		.await
 		.unwrap();
-		// assert that the background task returns ok
-		assert_eq!(rx.await.unwrap().unwrap(), ());
+		// check that the background task returns ok:
+		rx.await.unwrap().unwrap();
 	}
 
 	#[tokio::test]

substrate/client/db/src/bench.rs

@@ -672,7 +672,6 @@ mod test {
 			assert_eq!(rw_tracker.1, 0);
 			assert_eq!(rw_tracker.2, 2);
 			assert_eq!(rw_tracker.3, 0);
-			drop(rw_tracker);
 			bench_state.wipe().unwrap();
 		}
 	}

substrate/client/db/src/lib.rs

@@ -1993,7 +1993,7 @@ impl<Block: BlockT> sc_client_api::backend::Backend<Block> for Backend<Block> {
 		});
 		let database_cache = MemorySize::from_bytes(0);
 		let state_cache =
-			MemorySize::from_bytes((*&self.shared_cache).read().used_storage_cache_size());
+			MemorySize::from_bytes(self.shared_cache.read().used_storage_cache_size());
 		let state_db = self.storage.state_db.memory_info();
 
 		Some(UsageInfo {
@@ -2452,7 +2452,7 @@ pub(crate) mod tests {
 			let storage = vec![(vec![1, 3, 5], None), (vec![5, 5, 5], Some(vec![4, 5, 6]))];
 
 			let (root, overlay) = op.old_state.storage_root(
-				storage.iter().map(|(k, v)| (&k[..], v.as_ref().map(|v| &v[..]))),
+				storage.iter().map(|(k, v)| (k.as_slice(), v.as_ref().map(|v| &v[..]))),
 				state_version,
 			);
 			op.update_db_storage(overlay).unwrap();
@@ -3000,7 +3000,7 @@ pub(crate) mod tests {
 			let storage = vec![(b"test".to_vec(), Some(b"test2".to_vec()))];
 
 			let (root, overlay) = op.old_state.storage_root(
-				storage.iter().map(|(k, v)| (&k[..], v.as_ref().map(|v| &v[..]))),
+				storage.iter().map(|(k, v)| (k.as_slice(), v.as_ref().map(|v| &v[..]))),
 				state_version,
 			);
 			op.update_db_storage(overlay).unwrap();

substrate/client/db/src/offchain.rs

@@ -92,7 +92,7 @@ impl sp_core::offchain::OffchainStorage for LocalStorage {
 		{
 			let _key_guard = key_lock.lock();
 			let val = self.db.get(columns::OFFCHAIN, &key);
-			is_set = val.as_ref().map(|x| &**x) == old_value;
+			is_set = val.as_deref() == old_value;
 
 			if is_set {
 				self.set(prefix, item_key, new_value)

substrate/client/executor/src/integration_tests/mod.rs

@@ -126,7 +126,7 @@ fn call_in_wasm<E: Externalities>(
 	let executor =
 		crate::WasmExecutor::<HostFunctions>::new(execution_method, Some(1024), 8, None, 2);
 	executor.uncached_call(
-		RuntimeBlob::uncompress_if_needed(&wasm_binary_unwrap()[..]).unwrap(),
+		RuntimeBlob::uncompress_if_needed(wasm_binary_unwrap()).unwrap(),
 		ext,
 		true,
 		function,
@@ -479,7 +479,7 @@ fn should_trap_when_heap_exhausted(wasm_method: WasmExecutionMethod) {
 
 	let err = executor
 		.uncached_call(
-			RuntimeBlob::uncompress_if_needed(&wasm_binary_unwrap()[..]).unwrap(),
+			RuntimeBlob::uncompress_if_needed(wasm_binary_unwrap()).unwrap(),
 			&mut ext.ext(),
 			true,
 			"test_exhaust_heap",
@@ -491,7 +491,7 @@ fn should_trap_when_heap_exhausted(wasm_method: WasmExecutionMethod) {
 }
 
 fn mk_test_runtime(wasm_method: WasmExecutionMethod, pages: u64) -> Arc<dyn WasmModule> {
-	let blob = RuntimeBlob::uncompress_if_needed(&wasm_binary_unwrap()[..])
+	let blob = RuntimeBlob::uncompress_if_needed(wasm_binary_unwrap())
 		.expect("failed to create a runtime blob out of test runtime");
 
 	crate::wasm_runtime::create_wasm_runtime_with_code::<HostFunctions>(
@@ -597,7 +597,7 @@ fn parallel_execution(wasm_method: WasmExecutionMethod) {
 				assert_eq!(
 					executor
 						.uncached_call(
-							RuntimeBlob::uncompress_if_needed(&wasm_binary_unwrap()[..]).unwrap(),
+							RuntimeBlob::uncompress_if_needed(wasm_binary_unwrap()).unwrap(),
 							&mut ext,
 							true,
 							"test_twox_128",
@@ -691,7 +691,7 @@ fn panic_in_spawned_instance_panics_on_joining_its_result(wasm_method: WasmExecu
 	let error_result =
 		call_in_wasm("test_panic_in_spawned", &[], wasm_method, &mut ext).unwrap_err();
 
-	assert!(format!("{}", error_result).contains("Spawned task"));
+	assert!(error_result.contains("Spawned task"));
 }
 
 test_wasm_execution!(memory_is_cleared_between_invocations);

substrate/client/executor/src/lib.rs

@@ -82,7 +82,7 @@ mod tests {
 		);
 		let res = executor
 			.uncached_call(
-				RuntimeBlob::uncompress_if_needed(&wasm_binary_unwrap()[..]).unwrap(),
+				RuntimeBlob::uncompress_if_needed(wasm_binary_unwrap()).unwrap(),
 				&mut ext,
 				true,
 				"test_empty_return",

substrate/client/executor/src/wasm_runtime.rs

@@ -77,7 +77,7 @@ struct VersionedRuntime {
 
 impl VersionedRuntime {
 	/// Run the given closure `f` with an instance of this runtime.
-	fn with_instance<'c, R, F>(&self, ext: &mut dyn Externalities, f: F) -> Result<R, Error>
+	fn with_instance<R, F>(&self, ext: &mut dyn Externalities, f: F) -> Result<R, Error>
 	where
 		F: FnOnce(
 			&Arc<dyn WasmModule>,

substrate/client/executor/wasmtime/src/host.rs

@@ -195,7 +195,7 @@ impl<'a> Sandbox for HostContext<'a> {
 		&mut self,
 		instance_id: u32,
 		export_name: &str,
-		args: &[u8],
+		mut args: &[u8],
 		return_val: Pointer<u8>,
 		return_val_len: u32,
 		state: u32,
@@ -203,7 +203,7 @@ impl<'a> Sandbox for HostContext<'a> {
 		trace!(target: "sp-sandbox", "invoke, instance_idx={}", instance_id);
 
 		// Deserialize arguments and convert them into wasmi types.
-		let args = Vec::<sp_wasm_interface::Value>::decode(&mut &args[..])
+		let args = Vec::<sp_wasm_interface::Value>::decode(&mut args)
 			.map_err(|_| "Can't decode serialized arguments for the invocation")?
 			.into_iter()
 			.map(Into::into)

substrate/client/executor/wasmtime/src/tests.rs

@@ -313,7 +313,7 @@ fn test_max_memory_pages() {
 #[test]
 fn test_instances_without_reuse_are_not_leaked() {
 	let runtime = crate::create_runtime::<HostFunctions>(
-		RuntimeBlob::uncompress_if_needed(&wasm_binary_unwrap()[..]).unwrap(),
+		RuntimeBlob::uncompress_if_needed(wasm_binary_unwrap()).unwrap(),
 		crate::Config {
 			heap_pages: 2048,
 			max_memory_size: None,

substrate/client/finality-grandpa/src/communication/gossip.rs

@@ -615,8 +615,7 @@ impl<N: Ord> Peers<N> {
 		// `LUCKY_PEERS / 2` is filled we start allocating to the second stage set.
 		let half_lucky = LUCKY_PEERS / 2;
 		let one_and_a_half_lucky = LUCKY_PEERS + half_lucky;
-		let mut n_authorities_added = 0;
-		for peer_id in shuffled_authorities {
+		for (n_authorities_added, peer_id) in shuffled_authorities.enumerate() {
 			if n_authorities_added < half_lucky {
 				first_stage_peers.insert(*peer_id);
 			} else if n_authorities_added < one_and_a_half_lucky {
@@ -624,8 +623,6 @@ impl<N: Ord> Peers<N> {
 			} else {
 				break
 			}
-
-			n_authorities_added += 1;
 		}
 
 		// fill up first and second sets with remaining peers (either full or authorities)

substrate/client/finality-grandpa/src/lib.rs

@@ -267,7 +267,7 @@ pub struct Config {
 
 impl Config {
 	fn name(&self) -> &str {
-		self.name.as_ref().map(|s| s.as_str()).unwrap_or("<unknown>")
+		self.name.as_deref().unwrap_or("<unknown>")
 	}
 }
 

substrate/client/rpc/src/state/state_full.rs

@@ -441,10 +441,7 @@ where
 		keys: Option<Vec<StorageKey>>,
 	) {
 		let keys = Into::<Option<Vec<_>>>::into(keys);
-		let stream = match self
-			.client
-			.storage_changes_notification_stream(keys.as_ref().map(|x| &**x), None)
-		{
+		let stream = match self.client.storage_changes_notification_stream(keys.as_deref(), None) {
 			Ok(stream) => stream,
 			Err(err) => {
 				let _ = subscriber.reject(client_err(err).into());

substrate/client/rpc/src/state/tests.rs

@@ -32,7 +32,7 @@ use substrate_test_runtime_client::{prelude::*, runtime};
 const STORAGE_KEY: &[u8] = b"child";
 
 fn prefixed_storage_key() -> PrefixedStorageKey {
-	let child_info = ChildInfo::new_default(&STORAGE_KEY[..]);
+	let child_info = ChildInfo::new_default(STORAGE_KEY);
 	child_info.prefixed_storage_key()
 }
 

substrate/client/rpc/src/system/tests.rs

@@ -354,26 +354,26 @@ fn test_add_reset_log_filter() {
 	};
 
 	// Initiate logs loop in child process
-	child_in.write(b"\n").unwrap();
+	child_in.write_all(b"\n").unwrap();
 	assert!(read_line().contains(EXPECTED_BEFORE_ADD));
 
 	// Initiate add directive & reload in child process
-	child_in.write(b"add_reload\n").unwrap();
+	child_in.write_all(b"add_reload\n").unwrap();
 	assert!(read_line().contains(EXPECTED_BEFORE_ADD));
 	assert!(read_line().contains(EXPECTED_AFTER_ADD));
 
 	// Check that increasing the max log level works
-	child_in.write(b"add_trace\n").unwrap();
+	child_in.write_all(b"add_trace\n").unwrap();
 	assert!(read_line().contains(EXPECTED_WITH_TRACE));
 	assert!(read_line().contains(EXPECTED_BEFORE_ADD));
 	assert!(read_line().contains(EXPECTED_AFTER_ADD));
 
 	// Initiate logs filter reset in child process
-	child_in.write(b"reset\n").unwrap();
+	child_in.write_all(b"reset\n").unwrap();
 	assert!(read_line().contains(EXPECTED_BEFORE_ADD));
 
 	// Return from child process
-	child_in.write(b"exit\n").unwrap();
+	child_in.write_all(b"exit\n").unwrap();
 	assert!(child_process.wait().expect("Error waiting for child process").success());
 
 	// Check for EOF

substrate/client/service/src/builder.rs

@@ -363,7 +363,7 @@ where
 		spawn_handle,
 		config.clone(),
 	)?;
-	Ok(crate::client::Client::new(
+	crate::client::Client::new(
 		backend,
 		executor,
 		genesis_storage,
@@ -373,7 +373,7 @@ where
 		prometheus_registry,
 		telemetry,
 		config,
-	)?)
+	)
 }
 
 /// Parameters to pass into `build`.

substrate/client/service/src/client/call_executor.rs

@@ -266,9 +266,7 @@ where
 					&runtime_code,
 					self.spawn_handle.clone(),
 				)
-				.with_storage_transaction_cache(
-					storage_transaction_cache.as_mut().map(|c| &mut **c),
-				)
+				.with_storage_transaction_cache(storage_transaction_cache.as_deref_mut())
 				.set_parent_hash(at_hash);
 				state_machine.execute_using_consensus_failure_handler(
 					execution_manager,

substrate/client/service/src/client/client.rs

@@ -1409,10 +1409,9 @@ where
 		id: &BlockId<Block>,
 		key: &StorageKey,
 	) -> sp_blockchain::Result<Option<Block::Hash>> {
-		Ok(self
-			.state_at(id)?
+		self.state_at(id)?
 			.storage_hash(&key.0)
-			.map_err(|e| sp_blockchain::Error::from_state(Box::new(e)))?)
+			.map_err(|e| sp_blockchain::Error::from_state(Box::new(e)))
 	}
 
 	fn child_storage_keys(
@@ -1449,10 +1448,9 @@ where
 		child_info: &ChildInfo,
 		key: &StorageKey,
 	) -> sp_blockchain::Result<Option<Block::Hash>> {
-		Ok(self
-			.state_at(id)?
+		self.state_at(id)?
 			.child_storage_hash(child_info, &key.0)
-			.map_err(|e| sp_blockchain::Error::from_state(Box::new(e)))?)
+			.map_err(|e| sp_blockchain::Error::from_state(Box::new(e)))
 	}
 }
 

substrate/client/state-db/src/noncanonical.rs

@@ -134,12 +134,12 @@ fn discard_descendants<BlockHash: Hash, Key: Hash>(
 	hash: &BlockHash,
 ) -> u32 {
 	let (first, mut remainder) = if let Some((first, rest)) = levels.0.split_first_mut() {
-		(Some(first), (rest, &mut levels.1[..]))
+		(Some(first), (rest, &mut *levels.1))
 	} else {
 		if let Some((first, rest)) = levels.1.split_first_mut() {
-			(Some(first), (&mut levels.0[..], rest))
+			(Some(first), (&mut *levels.0, rest))
 		} else {
-			(None, (&mut levels.0[..], &mut levels.1[..]))
+			(None, (&mut *levels.0, &mut *levels.1))
 		}
 	};
 	let mut pinned_children = 0;
@@ -261,8 +261,7 @@ impl<BlockHash: Hash, Key: Hash> NonCanonicalOverlay<BlockHash, Key> {
 				.push((to_meta_key(LAST_CANONICAL, &()), last_canonicalized.encode()));
 			self.last_canonicalized = Some(last_canonicalized);
 		} else if self.last_canonicalized.is_some() {
-			if number < front_block_number ||
-				number >= front_block_number + self.levels.len() as u64 + 1
+			if number < front_block_number || number > front_block_number + self.levels.len() as u64
 			{
 				trace!(target: "state-db", "Failed to insert block {}, current is {} .. {})",
 					number,

substrate/client/tracing/proc-macro/src/lib.rs

@@ -119,7 +119,7 @@ pub fn prefix_logs_with(arg: TokenStream, item: TokenStream) -> TokenStream {
 	let name = syn::parse_macro_input!(arg as Expr);
 
 	let crate_name = match crate_name("sc-tracing") {
-		Ok(FoundCrate::Itself) => Ident::from(Ident::new("sc_tracing", Span::call_site())),
+		Ok(FoundCrate::Itself) => Ident::new("sc_tracing", Span::call_site()),
 		Ok(FoundCrate::Name(crate_name)) => Ident::new(&crate_name, Span::call_site()),
 		Err(e) => return Error::new(Span::call_site(), e).to_compile_error().into(),
 	};

substrate/client/tracing/src/block/mod.rs

@@ -267,7 +267,7 @@ where
 			.lock()
 			.drain()
 			// Patch wasm identifiers
-			.filter_map(|(_, s)| patch_and_filter(SpanDatum::from(s), targets))
+			.filter_map(|(_, s)| patch_and_filter(s, targets))
 			.collect();
 		let events: Vec<_> = block_subscriber
 			.events
@@ -315,7 +315,7 @@ fn event_values_filter(event: &TraceEvent, filter_kind: &str, values: &str) -> b
 		.values
 		.string_values
 		.get(filter_kind)
-		.and_then(|value| Some(check_target(values, value, &event.level)))
+		.map(|value| check_target(values, value, &event.level))
 		.unwrap_or(false)
 }
 

substrate/client/tracing/src/lib.rs

@@ -464,7 +464,10 @@ impl From<SpanDatum> for sp_rpc::tracing::Span {
 mod tests {
 	use super::*;
 	use parking_lot::Mutex;
-	use std::sync::Arc;
+	use std::sync::{
+		mpsc::{Receiver, Sender},
+		Arc,
+	};
 	use tracing_subscriber::layer::SubscriberExt;
 
 	struct TestTraceHandler {
@@ -617,14 +620,14 @@ mod tests {
 			let span1 = tracing::info_span!(target: "test_target", "test_span1");
 			let _guard1 = span1.enter();
 
-			let (tx, rx) = mpsc::channel();
+			let (tx, rx): (Sender<bool>, Receiver<bool>) = mpsc::channel();
 			let handle = thread::spawn(move || {
 				let span2 = tracing::info_span!(target: "test_target", "test_span2");
 				let _guard2 = span2.enter();
 				// emit event
 				tracing::event!(target: "test_target", tracing::Level::INFO, "test_event1");
 				for msg in rx.recv() {
-					if msg == false {
+					if !msg {
 						break
 					}
 				}

substrate/client/transaction-pool/src/graph/pool.rs

@@ -224,12 +224,8 @@ impl<B: ChainApi> Pool<B> {
 		hashes: &[ExtrinsicHash<B>],
 	) -> Result<(), B::Error> {
 		// Get details of all extrinsics that are already in the pool
-		let in_pool_tags = self
-			.validated_pool
-			.extrinsics_tags(hashes)
-			.into_iter()
-			.filter_map(|x| x)
-			.flatten();
+		let in_pool_tags =
+			self.validated_pool.extrinsics_tags(hashes).into_iter().flatten().flatten();
 
 		// Prune all transactions that provide given tags
 		let prune_status = self.validated_pool.prune_tags(in_pool_tags)?;