add clippy to CI (#9694)

* Focus on correctness lints. This relies on a compiler patch that landed 8th July 2021.
If people are using an earlier version of the compiler everything will still work unless they try and run clippy.

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: Denis Pisarev <denis.pisarev@parity.io>

substrate/.cargo/config.toml

@@ -0,0 +1,14 @@
+#
+# An auto defined `clippy` feature was introduced,
+# but it was found to clash with user defined features,
+# so was renamed to `cargo-clippy`.
+#
+# If you want standard clippy run:
+# RUSTFLAGS= cargo clippy
+[target.'cfg(feature = "cargo-clippy")']
+rustflags = [
+  "-Aclippy::all",
+  "-Dclippy::correctness",
+  "-Aclippy::if-same-then-else",
+  "-Aclippy::clone-double-ref"
+]

substrate/.gitignore

@@ -21,7 +21,6 @@ rls*.log
 .local
 **/hfuzz_target/
 **/hfuzz_workspace/
-.cargo/
 .cargo-remote.toml
 *.bin
 *.iml

substrate/.gitlab-ci.yml

@@ -311,7 +311,7 @@ cargo-deny:
     when:                          always
     paths:
       - deny.log
-  # FIXME: Temorarily allow to fail.
+  # FIXME: Temporarily allow to fail.
   allow_failure:                   true
 
 cargo-fmt:
@@ -321,6 +321,13 @@ cargo-fmt:
   script:
     - cargo +nightly fmt --all -- --check
 
+cargo-clippy:
+  stage:                           test
+  <<:                              *docker-env
+  <<:                              *test-refs
+  script:
+    - SKIP_WASM_BUILD=1 env -u RUSTFLAGS cargo +nightly clippy
+
 cargo-check-benches:
   stage:                           test
   <<:                              *docker-env

substrate/client/executor/src/wasm_runtime.rs

@@ -304,7 +304,7 @@ pub fn create_wasm_runtime_with_code(
 			//
 			// We drop the cache_path here to silence warnings that cache_path is not used if
 			// compiling without the `wasmtime` flag.
-			drop(cache_path);
+			let _ = cache_path;
 
 			sc_executor_wasmi::create_runtime(
 				blob,

substrate/client/network/src/block_request_handler.rs

@@ -85,13 +85,14 @@ struct SeenRequestsKey<B: BlockT> {
 	support_multiple_justifications: bool,
 }
 
+#[allow(clippy::derive_hash_xor_eq)]
 impl<B: BlockT> Hash for SeenRequestsKey<B> {
 	fn hash<H: Hasher>(&self, state: &mut H) {
 		self.peer.hash(state);
 		self.max_blocks.hash(state);
 		self.direction.hash(state);
 		self.attributes.hash(state);
-
+		self.support_multiple_justifications.hash(state);
 		match self.from {
 			BlockId::Hash(h) => h.hash(state),
 			BlockId::Number(n) => n.hash(state),

substrate/client/network/src/state_request_handler.rs

@@ -78,6 +78,7 @@ struct SeenRequestsKey<B: BlockT> {
 	start: Vec<u8>,
 }
 
+#[allow(clippy::derive_hash_xor_eq)]
 impl<B: BlockT> Hash for SeenRequestsKey<B> {
 	fn hash<H: Hasher>(&self, state: &mut H) {
 		self.peer.hash(state);

substrate/frame/support/src/traits/metadata.rs

@@ -78,7 +78,7 @@ pub trait GetCallMetadata {
 }
 
 /// The version of a crate.
-#[derive(RuntimeDebug, Eq, PartialEq, Encode, Decode, Ord, Clone, Copy, Default)]
+#[derive(RuntimeDebug, Eq, PartialEq, Encode, Decode, Clone, Copy, Default)]
 pub struct CrateVersion {
 	/// The major version of the crate.
 	pub major: u16,
@@ -94,14 +94,17 @@ impl CrateVersion {
 	}
 }
 
+impl sp_std::cmp::Ord for CrateVersion {
+	fn cmp(&self, other: &Self) -> sp_std::cmp::Ordering {
+		self.major
+			.cmp(&other.major)
+			.then_with(|| self.minor.cmp(&other.minor).then_with(|| self.patch.cmp(&other.patch)))
+	}
+}
+
 impl sp_std::cmp::PartialOrd for CrateVersion {
 	fn partial_cmp(&self, other: &Self) -> Option<sp_std::cmp::Ordering> {
-		let res = self
-			.major
-			.cmp(&other.major)
-			.then_with(|| self.minor.cmp(&other.minor).then_with(|| self.patch.cmp(&other.patch)));
-
-		Some(res)
+		Some(<Self as Ord>::cmp(&self, other))
 	}
 }