Add clippy support (#643)

* fp-*: make clippy happy

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* pallet-*: make clippy happy

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* fc-*: make clippy happy

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* template: make clippy happy

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Add clippy support for CI

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Fix rpc-core

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Fix

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Improve CI

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Add clippy component

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Make test more stable

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Make clippy happy

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Fix CI

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Some nits

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

.github/workflows/rust.yml

@@ -63,6 +63,8 @@ jobs:
         toolchain: nightly-2022-05-02
         target: wasm32-unknown-unknown
         override: true
-        components: rustfmt
+        components: rustfmt, clippy
     - name: Rustfmt
       run: cargo fmt --all -- --check
+    - name: Clippy
+      run: cargo clippy --all -- -D warnings

client/consensus/src/lib.rs

@@ -125,7 +125,7 @@ where
 		// We validate that there are only one frontier log. No other
 		// actions are needed and mapping syncing is delegated to a separate
 		// worker.
-		ensure_log(&block.header.digest()).map_err(|e| Error::from(e))?;
+		ensure_log(block.header.digest()).map_err(Error::from)?;
 
 		self.inner
 			.import_block(block, new_cache)

client/db/src/lib.rs

@@ -16,10 +16,9 @@
 // You should have received a copy of the GNU General Public License
 // along with this program. If not, see <https://www.gnu.org/licenses/>.
 
-mod utils;
-
 #[cfg(feature = "parity-db")]
 mod parity_db_adapter;
+mod utils;
 
 use std::{marker::PhantomData, sync::Arc};
 
@@ -94,7 +93,7 @@ impl<Block: BlockT> MetaDb<Block> {
 	pub fn current_syncing_tips(&self) -> Result<Vec<Block::Hash>, String> {
 		match self.db.get(
 			crate::columns::META,
-			&crate::static_keys::CURRENT_SYNCING_TIPS,
+			crate::static_keys::CURRENT_SYNCING_TIPS,
 		) {
 			Some(raw) => {
 				Ok(Vec::<Block::Hash>::decode(&mut &raw[..]).map_err(|e| format!("{:?}", e))?)

client/db/src/utils.rs

@@ -22,15 +22,15 @@ use crate::{Database, DatabaseSettings, DatabaseSource, DbHash};
 
 pub fn open_database(config: &DatabaseSettings) -> Result<Arc<dyn Database<DbHash>>, String> {
 	let db: Arc<dyn Database<DbHash>> = match &config.source {
-		DatabaseSource::ParityDb { path } => open_parity_db(&path)?,
-		DatabaseSource::RocksDb { path, .. } => open_kvdb_rocksdb(&path, true)?,
+		DatabaseSource::ParityDb { path } => open_parity_db(path)?,
+		DatabaseSource::RocksDb { path, .. } => open_kvdb_rocksdb(path, true)?,
 		DatabaseSource::Auto {
 			paritydb_path,
 			rocksdb_path,
 			..
-		} => match open_kvdb_rocksdb(&rocksdb_path, false) {
+		} => match open_kvdb_rocksdb(rocksdb_path, false) {
 			Ok(db) => db,
-			Err(_) => open_parity_db(&paritydb_path)?,
+			Err(_) => open_parity_db(paritydb_path)?,
 		},
 		_ => return Err("Missing feature flags `parity-db`".to_string()),
 	};
@@ -59,7 +59,7 @@ fn open_kvdb_rocksdb(_path: &Path, _create: bool) -> Result<Arc<dyn Database<DbH
 fn open_parity_db(path: &Path) -> Result<Arc<dyn Database<DbHash>>, String> {
 	let config = parity_db::Options::with_columns(path, crate::columns::NUM_COLUMNS as u8);
 	let db = parity_db::Db::open_or_create(&config).map_err(|err| format!("{}", err))?;
-	return Ok(Arc::new(crate::parity_db_adapter::DbAdapter(db)));
+	Ok(Arc::new(crate::parity_db_adapter::DbAdapter(db)))
 }
 
 #[cfg(not(feature = "parity-db"))]

client/mapping-sync/src/lib.rs

@@ -16,6 +16,8 @@
 // You should have received a copy of the GNU General Public License
 // along with this program. If not, see <https://www.gnu.org/licenses/>.
 
+#![allow(clippy::too_many_arguments)]
+
 mod worker;
 
 pub use worker::{MappingSyncWorker, SyncStrategy};
@@ -78,7 +80,7 @@ where
 			.current_block(&id)
 			.map_err(|e| format!("{:?}", e))?;
 		let block_hash = block
-			.ok_or("Ethereum genesis block not found".to_string())?
+			.ok_or_else(|| "Ethereum genesis block not found".to_string())?
 			.header
 			.hash();
 		let mapping_commitment = fc_db::MappingCommitment::<Block> {

client/mapping-sync/src/worker.rs

@@ -101,7 +101,7 @@ where
 			}
 		}
 
-		let timeout = self.timeout.clone();
+		let timeout = self.timeout;
 		let inner_delay = self.inner_delay.get_or_insert_with(|| Delay::new(timeout));
 
 		match Future::poll(Pin::new(inner_delay), cx) {

client/rpc-core/src/types/block_number.rs

@@ -115,8 +115,8 @@ impl<'a> Visitor<'a> for BlockNumberVisitor {
 				Some(key) => match key.as_str() {
 					"blockNumber" => {
 						let value: String = visitor.next_value()?;
-						if value.starts_with("0x") {
-							let number = u64::from_str_radix(&value[2..], 16).map_err(|e| {
+						if let Some(stripped) = value.strip_prefix("0x") {
+							let number = u64::from_str_radix(stripped, 16).map_err(|e| {
 								Error::custom(format!("Invalid block number: {}", e))
 							})?;
 
@@ -151,7 +151,7 @@ impl<'a> Visitor<'a> for BlockNumberVisitor {
 			});
 		}
 
-		return Err(Error::custom("Invalid input"));
+		Err(Error::custom("Invalid input"))
 	}
 
 	fn visit_str<E>(self, value: &str) -> Result<Self::Value, E>
@@ -165,13 +165,9 @@ impl<'a> Visitor<'a> for BlockNumberVisitor {
 			_ if value.starts_with("0x") => u64::from_str_radix(&value[2..], 16)
 				.map(BlockNumber::Num)
 				.map_err(|e| Error::custom(format!("Invalid block number: {}", e))),
-			_ => u64::from_str_radix(&value, 10)
-				.map(BlockNumber::Num)
-				.map_err(|_| {
-					Error::custom(
-						"Invalid block number: non-decimal or missing 0x prefix".to_string(),
-					)
-				}),
+			_ => value.parse::<u64>().map(BlockNumber::Num).map_err(|_| {
+				Error::custom("Invalid block number: non-decimal or missing 0x prefix".to_string())
+			}),
 		}
 	}
 
@@ -207,8 +203,8 @@ mod tests {
 		let bn_hex: BlockNumber = serde_json::from_str(r#""0x45""#).unwrap();
 		let bn_u64: BlockNumber = serde_json::from_str(r#"420"#).unwrap();
 
-		assert_eq!(match_block_number(bn_dec).unwrap(), 42 as u64);
-		assert_eq!(match_block_number(bn_hex).unwrap(), 69 as u64);
-		assert_eq!(match_block_number(bn_u64).unwrap(), 420 as u64);
+		assert_eq!(match_block_number(bn_dec).unwrap(), 42);
+		assert_eq!(match_block_number(bn_hex).unwrap(), 69);
+		assert_eq!(match_block_number(bn_u64).unwrap(), 420);
 	}
 }

client/rpc-core/src/types/bytes.rs

@@ -46,9 +46,9 @@ impl From<Vec<u8>> for Bytes {
 	}
 }
 
-impl Into<Vec<u8>> for Bytes {
-	fn into(self) -> Vec<u8> {
-		self.0
+impl From<Bytes> for Vec<u8> {
+	fn from(bytes: Bytes) -> Vec<u8> {
+		bytes.0
 	}
 }
 

client/rpc-core/src/types/filter.rs

@@ -83,10 +83,10 @@ impl From<&VariadicValue<H160>> for Vec<Option<Bloom>> {
 				blooms.push(Some(bloom))
 			}
 			VariadicValue::Multiple(addresses) => {
-				if addresses.len() == 0 {
+				if addresses.is_empty() {
 					blooms.push(None);
 				} else {
-					for address in addresses.into_iter() {
+					for address in addresses.iter() {
 						let bloom: Bloom = BloomInput::Raw(address.as_ref()).into();
 						blooms.push(Some(bloom));
 					}
@@ -111,10 +111,10 @@ impl From<&VariadicValue<Option<H256>>> for Vec<Option<Bloom>> {
 				}
 			}
 			VariadicValue::Multiple(topics) => {
-				if topics.len() == 0 {
+				if topics.is_empty() {
 					blooms.push(None);
 				} else {
-					for topic in topics.into_iter() {
+					for topic in topics.iter() {
 						if let Some(topic) = topic {
 							let bloom: Bloom = BloomInput::Raw(topic.as_ref()).into();
 							blooms.push(Some(bloom));
@@ -149,28 +149,19 @@ pub struct Filter {
 
 /// Helper for Filter matching.
 /// Supports conditional indexed parameters and wildcards.
-#[derive(Debug)]
+#[derive(Debug, Default)]
 pub struct FilteredParams {
 	pub filter: Option<Filter>,
 	pub flat_topics: Vec<FlatTopic>,
 }
 
-impl Default for FilteredParams {
-	fn default() -> Self {
-		FilteredParams {
-			filter: None,
-			flat_topics: Vec::new(),
-		}
-	}
-}
-
 impl FilteredParams {
 	pub fn new(f: Option<Filter>) -> Self {
 		if let Some(f) = f {
 			return FilteredParams {
 				filter: Some(f.clone()),
 				flat_topics: {
-					if let Some(t) = f.clone().topics {
+					if let Some(t) = f.topics {
 						Self::flatten(&t)
 					} else {
 						Vec::new()
@@ -182,15 +173,15 @@ impl FilteredParams {
 	}
 
 	/// Build an address-based BloomFilter.
-	pub fn adresses_bloom_filter<'a>(address: &'a Option<FilterAddress>) -> BloomFilter<'a> {
+	pub fn adresses_bloom_filter(address: &Option<FilterAddress>) -> BloomFilter<'_> {
 		if let Some(address) = address {
 			return address.into();
 		}
 		Vec::new()
 	}
 
 	/// Build a topic-based BloomFilter.
-	pub fn topics_bloom_filter<'a>(topics: &'a Option<Vec<FlatTopic>>) -> Vec<BloomFilter<'a>> {
+	pub fn topics_bloom_filter(topics: &Option<Vec<FlatTopic>>) -> Vec<BloomFilter<'_>> {
 		let mut output: Vec<BloomFilter> = Vec::new();
 		if let Some(topics) = topics {
 			for flat in topics {
@@ -201,13 +192,13 @@ impl FilteredParams {
 	}
 
 	/// Evaluates if a Bloom contains a provided sequence of topics.
-	pub fn topics_in_bloom(bloom: Bloom, topic_bloom_filters: &Vec<BloomFilter>) -> bool {
-		if topic_bloom_filters.len() == 0 {
+	pub fn topics_in_bloom(bloom: Bloom, topic_bloom_filters: &[BloomFilter]) -> bool {
+		if topic_bloom_filters.is_empty() {
 			// No filter provided, match.
 			return true;
 		}
 		// A logical OR evaluation over `topic_bloom_filters`.
-		for subset in topic_bloom_filters.into_iter() {
+		for subset in topic_bloom_filters.iter() {
 			let mut matches = false;
 			for el in subset {
 				matches = match el {
@@ -230,7 +221,7 @@ impl FilteredParams {
 
 	/// Evaluates if a Bloom contains the provided address(es).
 	pub fn address_in_bloom(bloom: Bloom, address_bloom_filter: &BloomFilter) -> bool {
-		if address_bloom_filter.len() == 0 {
+		if address_bloom_filter.is_empty() {
 			// No filter provided, match.
 			return true;
 		} else {
@@ -251,7 +242,7 @@ impl FilteredParams {
 	/// Executed once on struct instance.
 	/// i.e. `[A,[B,C]]` to `[[A,B],[A,C]]`.
 	fn flatten(topic: &Topic) -> Vec<FlatTopic> {
-		fn cartesian(lists: &Vec<Vec<Option<H256>>>) -> Vec<Vec<Option<H256>>> {
+		fn cartesian(lists: &[Vec<Option<H256>>]) -> Vec<Vec<Option<H256>>> {
 			let mut res = vec![];
 			let mut list_iter = lists.iter();
 			if let Some(first_list) = list_iter.next() {
@@ -275,13 +266,13 @@ impl FilteredParams {
 		let mut out: Vec<FlatTopic> = Vec::new();
 		match topic {
 			VariadicValue::Multiple(multi) => {
-				let mut foo: Vec<Vec<Option<H256>>> = Vec::new();
+				let mut values: Vec<Vec<Option<H256>>> = Vec::new();
 				for v in multi {
-					foo.push({
+					values.push({
 						if let Some(v) = v {
 							match v {
 								VariadicValue::Single(s) => {
-									vec![s.clone()]
+									vec![*s]
 								}
 								VariadicValue::Multiple(s) => s.clone(),
 								VariadicValue::Null => {
@@ -293,7 +284,7 @@ impl FilteredParams {
 						}
 					});
 				}
-				for permut in cartesian(&foo) {
+				for permut in cartesian(&values) {
 					out.push(FlatTopic::Multiple(permut));
 				}
 			}
@@ -313,23 +304,21 @@ impl FilteredParams {
 	pub fn replace(&self, log: &Log, topic: FlatTopic) -> Option<Vec<H256>> {
 		let mut out: Vec<H256> = Vec::new();
 		match topic {
-			VariadicValue::Single(value) => {
-				if let Some(value) = value {
-					out.push(value);
-				}
+			VariadicValue::Single(Some(value)) => {
+				out.push(value);
 			}
 			VariadicValue::Multiple(value) => {
 				for (k, v) in value.into_iter().enumerate() {
 					if let Some(v) = v {
 						out.push(v);
 					} else {
-						out.push(log.topics[k].clone());
+						out.push(log.topics[k]);
 					}
 				}
 			}
 			_ => {}
 		};
-		if out.len() == 0 {
+		if out.is_empty() {
 			return None;
 		}
 		Some(out)
@@ -338,20 +327,15 @@ impl FilteredParams {
 	pub fn filter_block_range(&self, block_number: u64) -> bool {
 		let mut out = true;
 		let filter = self.filter.clone().unwrap();
-		if let Some(from) = filter.from_block {
-			match from {
-				BlockNumber::Num(_) => {
-					if from.to_min_block_num().unwrap_or(0 as u64) > block_number {
-						out = false;
-					}
-				}
-				_ => {}
+		if let Some(BlockNumber::Num(from)) = filter.from_block {
+			if from > block_number {
+				out = false;
 			}
 		}
 		if let Some(to) = filter.to_block {
 			match to {
-				BlockNumber::Num(_) => {
-					if to.to_min_block_num().unwrap_or(0 as u64) < block_number {
+				BlockNumber::Num(to) => {
+					if to < block_number {
 						out = false;
 					}
 				}
@@ -400,7 +384,7 @@ impl FilteredParams {
 			match topic {
 				VariadicValue::Single(single) => {
 					if let Some(single) = single {
-						if !log.topics.starts_with(&vec![single]) {
+						if !log.topics.starts_with(&[single]) {
 							out = false;
 						}
 					}

client/rpc-core/src/types/pubsub.rs

@@ -108,7 +108,7 @@ impl<'a> Deserialize<'a> for Params {
 			return Ok(Params::None);
 		}
 
-		from_value(v.clone())
+		from_value(v)
 			.map(Params::Logs)
 			.map_err(|e| D::Error::custom(format!("Invalid Pub-Sub parameters: {}", e)))
 	}