Fix clippy for latest stable toolchain (#3935)

* clippy::redundant-guards * clippy::incorrect_partial_ord_impl_on_ord_type * clippy::redundant-guards * clippy::unwrap-or-default * rustfmt * noop_method_call * nym-wallet Cargo.lock * Unpin rust toolchain for nym-wallet * cargo clippy --fix --workspace * clippy::redundant_locals * Reorder Makefile targets for more logical ordering
nymtech · Oct 6, 2023 · 369330f · 369330f
1 parent ada30f5
commit 369330f
Show file tree

Hide file tree

Showing 19 changed files with 45 additions and 60 deletions.
diff --git a/.github/workflows/ci-nym-wallet-rust.yml b/.github/workflows/ci-nym-wallet-rust.yml
@@ -33,7 +33,7 @@ jobs:
         uses: actions-rs/toolchain@v1
         with:
           profile: minimal
-          toolchain: 1.71.0
+          toolchain: stable
           override: true
           components: rustfmt, clippy
 

diff --git a/.github/workflows/nightly-nym-wallet-build.yml b/.github/workflows/nightly-nym-wallet-build.yml
@@ -31,8 +31,7 @@ jobs:
         uses: actions-rs/toolchain@v1
         with:
           profile: minimal
-          # There is an issue with 1.72.0 where clippy crashes on nym-wallet-types. Pin to 1.71.0 for now
-          toolchain: 1.71.0
+          toolchain: stable
           override: true
           components: rustfmt, clippy
 

diff --git a/Makefile b/Makefile
@@ -46,21 +46,9 @@ clippy:
 # -----------------------------------------------------------------------------
 define add_cargo_workspace
 
-clippy-$(1):
-	cargo $$($(1)_CLIPPY_TOOLCHAIN) clippy --manifest-path $(2)/Cargo.toml --workspace $(3) -- -D warnings
-
-clippy-extra-$(1):
-	cargo $$($(1)_CLIPPY_TOOLCHAIN) clippy --manifest-path $(2)/Cargo.toml --workspace --examples --tests -- -D warnings
-
 check-$(1):
 	cargo check --manifest-path $(2)/Cargo.toml --workspace $(3)
 
-test-$(1):
-	cargo test --manifest-path $(2)/Cargo.toml --workspace
-
-test-expensive-$(1):
-	cargo test --manifest-path $(2)/Cargo.toml --workspace -- --ignored
-
 build-$(1):
 	cargo build --manifest-path $(2)/Cargo.toml --workspace $(3)
 
@@ -70,15 +58,27 @@ build-extra-$(1):
 build-release-$(1):
 	$(4) cargo $$($(1)_BUILD_RELEASE_TOOLCHAIN) build --manifest-path $(2)/Cargo.toml --workspace --release $(3)
 
+test-$(1):
+	cargo test --manifest-path $(2)/Cargo.toml --workspace
+
+test-expensive-$(1):
+	cargo test --manifest-path $(2)/Cargo.toml --workspace -- --ignored
+
+clippy-$(1):
+	cargo $$($(1)_CLIPPY_TOOLCHAIN) clippy --manifest-path $(2)/Cargo.toml --workspace $(3) -- -D warnings
+
+clippy-extra-$(1):
+	cargo $$($(1)_CLIPPY_TOOLCHAIN) clippy --manifest-path $(2)/Cargo.toml --workspace --examples --tests -- -D warnings
+
 fmt-$(1):
 	cargo fmt --manifest-path $(2)/Cargo.toml --all
 
-clippy: clippy-$(1) clippy-extra-$(1)
 check: check-$(1)
-cargo-test: test-$(1)
-cargo-test-expensive: test-expensive-$(1)
 build: build-$(1) build-extra-$(1)
 build-release-all: build-release-$(1)
+cargo-test: test-$(1)
+cargo-test-expensive: test-expensive-$(1)
+clippy: clippy-$(1) clippy-extra-$(1)
 fmt: fmt-$(1)
 endef
 
@@ -93,10 +93,6 @@ $(eval $(call add_cargo_workspace,contracts,contracts,--lib --target wasm32-unkn
 $(eval $(call add_cargo_workspace,wallet,nym-wallet))
 $(eval $(call add_cargo_workspace,connect,nym-connect/desktop))
 
-# OVERRIDE: there is an issue where clippy crashes on nym-wallet-types with the latest
-# stable toolchain. So pin to 1.71.0 until that is resolved.
-wallet_CLIPPY_TOOLCHAIN := +1.71.0
-
 # OVERRIDE: wasm-opt fails if the binary has been built with the latest rustc.
 # Pin to the last working version.
 contracts_BUILD_RELEASE_TOOLCHAIN := +1.69.0

diff --git a/common/client-core/src/client/base_client/storage/gateway_details.rs b/common/client-core/src/client/base_client/storage/gateway_details.rs
@@ -47,11 +47,7 @@ impl<T> PersistedGatewayDetails<T> {
     pub fn validate(&self, shared_key: Option<&SharedKeys>) -> Result<(), ClientCoreError> {
         match self {
             PersistedGatewayDetails::Default(details) => {
-                if !details.verify(
-                    shared_key
-                        .ok_or(ClientCoreError::UnavailableSharedKey)?
-                        .deref(),
-                ) {
+                if !details.verify(shared_key.ok_or(ClientCoreError::UnavailableSharedKey)?) {
                     Err(ClientCoreError::MismatchedGatewayDetails {
                         gateway_id: details.details.gateway_id.clone(),
                     })

diff --git a/common/client-core/src/client/topology_control/geo_aware_provider.rs b/common/client-core/src/client/topology_control/geo_aware_provider.rs
@@ -199,7 +199,7 @@ fn group_mixnodes_by_country_code(
             if let Some(ref location) = m.location {
                 let country_code = location.two_letter_iso_country_code.clone();
                 let group_code = CountryGroup::new(country_code.as_str());
-                let mixnodes = acc.entry(group_code).or_insert_with(Vec::new);
+                let mixnodes = acc.entry(group_code).or_default();
                 mixnodes.push(m.mix_id);
             }
             acc

diff --git a/common/cosmwasm-smart-contracts/mixnet-contract/src/interval.rs b/common/cosmwasm-smart-contracts/mixnet-contract/src/interval.rs
@@ -572,7 +572,7 @@ mod tests {
         let env = mock_env();
 
         // epoch just begun
-        let interval = Interval {
+        let mut interval = Interval {
             id: 0,
             epochs_in_interval: 100,
             current_epoch_start: OffsetDateTime::from_unix_timestamp(
@@ -586,19 +586,16 @@ mod tests {
         assert!(!interval.is_current_epoch_over(&env));
 
         // current time == current epoch start
-        let mut interval = interval;
         interval.current_epoch_start =
             OffsetDateTime::from_unix_timestamp(env.block.time.seconds() as i64).unwrap();
         assert!(!interval.is_current_epoch_over(&env));
 
         // epoch HASN'T yet begun (weird edge case, but can happen if we decide to manually adjust things)
-        let mut interval = interval;
         interval.current_epoch_start =
             OffsetDateTime::from_unix_timestamp(env.block.time.seconds() as i64 + 100).unwrap();
         assert!(!interval.is_current_epoch_over(&env));
 
         // current_time = EXACTLY end of the epoch
-        let mut interval = interval;
         interval.current_epoch_start =
             OffsetDateTime::from_unix_timestamp(env.block.time.seconds() as i64).unwrap()
                 - interval.epoch_length;

diff --git a/common/nymsphinx/chunking/src/lib.rs b/common/nymsphinx/chunking/src/lib.rs
@@ -80,7 +80,7 @@ pub fn number_of_required_fragments(
     let max_linked = linked_fragment_payload_max_len(plaintext_per_fragment);
 
     match set::total_number_of_sets(message_len, plaintext_per_fragment) {
-        n if n == 1 => {
+        1 => {
             // is if it's a single fragment message
             if message_len < max_unlinked {
                 return (1, max_unlinked - message_len);

diff --git a/common/nymsphinx/params/src/packet_sizes.rs b/common/nymsphinx/params/src/packet_sizes.rs
@@ -89,7 +89,7 @@ pub enum PacketSize {
 impl PartialOrd for PacketSize {
     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
         // order them by actual packet size
-        self.size().partial_cmp(&other.size())
+        Some(self.cmp(other))
     }
 }
 

diff --git a/common/socks5/proxy-helpers/src/connection_controller.rs b/common/socks5/proxy-helpers/src/connection_controller.rs
@@ -193,10 +193,7 @@ impl Controller {
             }
         } else if !self.recently_closed.contains(&hdr.connection_id) {
             debug!("Received a 'Send' before 'Connect' - going to buffer the data");
-            let pending = self
-                .pending_messages
-                .entry(hdr.connection_id)
-                .or_insert_with(Vec::new);
+            let pending = self.pending_messages.entry(hdr.connection_id).or_default();
             pending.push(message);
         } else if !hdr.local_socket_closed {
             error!(

diff --git a/common/wasm/storage/src/lib.rs b/common/wasm/storage/src/lib.rs
@@ -175,8 +175,8 @@ impl WasmStorage {
         K: wasm_bindgen::JsCast,
     {
         match self.key_count(store, key).await? {
-            n if n == 0 => Ok(false),
-            n if n == 1 => Ok(true),
+            0 => Ok(false),
+            1 => Ok(true),
             n => Err(StorageError::DuplicateKey { count: n }),
         }
     }

diff --git a/ephemera/src/api/http/submit.rs b/ephemera/src/api/http/submit.rs
@@ -20,7 +20,7 @@ pub(crate) async fn submit_message(
     api: web::Data<CommandExecutor>,
 ) -> HttpResponse {
     match api.send_ephemera_message(message.into_inner()).await {
-        Ok(_) => HttpResponse::Ok().json("Message submitted"),
+        Ok(()) => HttpResponse::Ok().json("Message submitted"),
         Err(err) => {
             if let ApiError::DuplicateMessage = err {
                 debug!("Message already submitted {err:?}");
@@ -53,7 +53,7 @@ pub(crate) async fn store_in_dht(
     let value = request.value();
 
     match api.store_in_dht(key, value).await {
-        Ok(_) => HttpResponse::Ok().json("Store request submitted"),
+        Ok(()) => HttpResponse::Ok().json("Store request submitted"),
         Err(err) => {
             error!("Error storing in dht: {}", err);
             HttpResponse::InternalServerError().json("Server failed to process request")

diff --git a/ephemera/src/block/manager.rs b/ephemera/src/block/manager.rs
@@ -278,7 +278,7 @@ impl BlockManager {
         }
 
         match self.message_pool.remove_messages(&block.messages) {
-            Ok(_) => {
+            Ok(()) => {
                 self.block_chain_state
                     .mark_last_produced_block_as_committed();
             }

diff --git a/ephemera/src/core/api_cmd.rs b/ephemera/src/core/api_cmd.rs
@@ -132,7 +132,7 @@ impl ApiCmdProcessor {
             .send_ephemera_event(EphemeraEvent::StoreInDht { key, value })
             .await
         {
-            Ok(_) => Ok(()),
+            Ok(()) => Ok(()),
             Err(err) => {
                 error!("Error sending StoreInDht to network: {:?}", err);
                 Err(ApiError::Internal("Failed to store in DHT".to_string()))
@@ -153,7 +153,7 @@ impl ApiCmdProcessor {
             .send_ephemera_event(EphemeraEvent::QueryDht { key: key.clone() })
             .await
         {
-            Ok(_) => {
+            Ok(()) => {
                 //Save the reply channel in a map and send the reply when we get the response from the network
                 ephemera
                     .api_cmd_processor
@@ -278,7 +278,7 @@ impl ApiCmdProcessor {
                 // Send to BlockManager to verify it and put into memory pool
                 let ephemera_msg: message::EphemeraMessage = (*api_msg).into();
                 match ephemera.block_manager.on_new_message(ephemera_msg.clone()) {
-                    Ok(_) => {
+                    Ok(()) => {
                         //Gossip to network for other nodes to receive
                         match ephemera
                             .to_network
@@ -287,7 +287,7 @@ impl ApiCmdProcessor {
                             ))
                             .await
                         {
-                            Ok(_) => Ok(()),
+                            Ok(()) => Ok(()),
                             Err(err) => {
                                 error!("Error sending EphemeraMessage to network: {:?}", err);
                                 Err(ApiError::Internal("Failed to submit message".to_string()))

diff --git a/ephemera/src/core/builder.rs b/ephemera/src/core/builder.rs
@@ -267,7 +267,7 @@ impl<A: Application> EphemeraStarterWithApplication<A> {
                 }
                 ws_stopped = websocket.run() => {
                     match ws_stopped {
-                        Ok(_) => info!("Websocket stopped unexpectedly"),
+                        Ok(()) => info!("Websocket stopped unexpectedly"),
                         Err(e) => error!("Websocket stopped with error: {}", e),
                     }
                 }
@@ -293,7 +293,7 @@ impl<A: Application> EphemeraStarterWithApplication<A> {
                 }
                 http_stopped = http => {
                     match http_stopped {
-                        Ok(_) => info!("Http server stopped unexpectedly"),
+                        Ok(()) => info!("Http server stopped unexpectedly"),
                         Err(e) => error!("Http server stopped with error: {}", e),
                     }
                 }
@@ -330,7 +330,7 @@ impl<A: Application> EphemeraStarterWithApplication<A> {
                 }
                 nw_stopped = network.start() => {
                     match nw_stopped {
-                        Ok(_) => info!("Network stopped unexpectedly"),
+                        Ok(()) => info!("Network stopped unexpectedly"),
                         Err(e) => error!("Network stopped with error: {e}",),
                     }
                 }

diff --git a/ephemera/src/core/ephemera.rs b/ephemera/src/core/ephemera.rs
@@ -120,7 +120,7 @@ impl<A: Application> Ephemera<A> {
         while !shutdown.is_shutdown() {
             tokio::select! {
                 biased;
-                _ = shutdown.recv() => {
+                () = shutdown.recv() => {
                     trace!("UpdateHandler: Received shutdown");
                     self.shutdown_manager.stop().await;
                     break;

diff --git a/ephemera/src/core/shutdown.rs b/ephemera/src/core/shutdown.rs
@@ -57,7 +57,7 @@ impl ShutdownManager {
             .map(|(i, h)| (i + 1, h))
         {
             match handle.await.unwrap() {
-                Ok(_) => info!("Task {i} finished successfully"),
+                Ok(()) => info!("Task {i} finished successfully"),
                 Err(e) => info!("Task {i} finished with error: {e}",),
             }
         }

diff --git a/ephemera/src/network/libp2p/behaviours/membership/handler.rs b/ephemera/src/network/libp2p/behaviours/membership/handler.rs
@@ -309,7 +309,7 @@ impl ConnectionHandler for Handler {
         match event {
             ConnectionEvent::FullyNegotiatedInbound(FullyNegotiatedInbound {
                 protocol: stream,
-                info: _,
+                info: (),
             }) => {
                 if self.inbound_substream_attempts > MAX_SUBSTREAM_ATTEMPTS {
                     log::warn!("Too many inbound substream attempts, refusing stream");
@@ -320,7 +320,7 @@ impl ConnectionHandler for Handler {
             }
             ConnectionEvent::FullyNegotiatedOutbound(FullyNegotiatedOutbound {
                 protocol,
-                info: _,
+                info: (),
             }) => {
                 if self.outbound_substream_attempts > MAX_SUBSTREAM_ATTEMPTS {
                     log::warn!("Too many outbound substream attempts, refusing stream");

diff --git a/gateway/src/node/client_handling/websocket/connection_handler/mod.rs b/gateway/src/node/client_handling/websocket/connection_handler/mod.rs
@@ -19,11 +19,11 @@ mod authenticated;
 pub(crate) mod coconut;
 mod fresh;
 
-//// TODO: note for my future self to consider the following idea:
-//// split the socket connection into sink and stream
-//// stream will be for reading explicit requests
-//// and sink for pumping responses AND mix traffic
-//// but as byproduct this might (or might not) break the clean "SocketStream" enum here
+// TODO: note for my future self to consider the following idea:
+// split the socket connection into sink and stream
+// stream will be for reading explicit requests
+// and sink for pumping responses AND mix traffic
+// but as byproduct this might (or might not) break the clean "SocketStream" enum here
 
 pub(crate) enum SocketStream<S> {
     RawTcp(S),

diff --git a/nym-wallet/Cargo.lock b/nym-wallet/Cargo.lock