Fix clippy (#83)

Builds on #57, however due to the high number of conflicts I fixed the errors in this PR. ### Next Steps - [ ] Sync node with this PR --------- Signed-off-by: Alexandru Vasile <[email protected]>
paritytech · Apr 24, 2024 · 19515ea · 19515ea
1 parent 08112ca
commit 19515ea
Show file tree

Hide file tree

Showing 57 changed files with 263 additions and 279 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -117,10 +117,7 @@ jobs:
           cache-on-failure: true
           cache-all-crates: true
 
-      # TODO: Allow clippy to fail and do not cancel other tasks.
-      # Clippy is fixed by: https://github.com/paritytech/litep2p/pull/57.
       - name: Run clippy
-        continue-on-error: true
         run: cargo clippy
 
   test:

diff --git a/examples/custom_protocol.rs b/examples/custom_protocol.rs
@@ -289,7 +289,7 @@ async fn main() {
         }
     });
 
-    for message in vec![
+    for message in [
         b"hello, world".to_vec(),
         b"testing 123".to_vec(),
         b"goodbye, world".to_vec(),

diff --git a/src/codec/unsigned_varint.rs b/src/codec/unsigned_varint.rs
@@ -59,14 +59,14 @@ impl UnsignedVarint {
 
         let mut bytes = BytesMut::with_capacity(payload.len() + 4);
         let mut codec = Self::new(None);
-        codec.encode(payload.into(), &mut bytes)?;
+        codec.encode(payload, &mut bytes)?;
 
         Ok(bytes.into())
     }
 
     /// Decode `payload` into `BytesMut`.
     pub fn decode(payload: &mut BytesMut) -> crate::Result<BytesMut> {
-        Ok(UviBytes::<Bytes>::default().decode(payload)?.ok_or(Error::InvalidData)?)
+        UviBytes::<Bytes>::default().decode(payload)?.ok_or(Error::InvalidData)
     }
 }
 

diff --git a/src/config.rs b/src/config.rs
@@ -111,6 +111,12 @@ pub struct ConfigBuilder {
     max_parallel_dials: usize,
 }
 
+impl Default for ConfigBuilder {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl ConfigBuilder {
     /// Create empty [`ConfigBuilder`].
     pub fn new() -> Self {

diff --git a/src/crypto/noise/mod.rs b/src/crypto/noise/mod.rs
@@ -73,6 +73,7 @@ pub const MAX_FRAME_LEN: usize = MAX_NOISE_MSG_LEN - NOISE_EXTRA_ENCRYPT_SPACE;
 const LOG_TARGET: &str = "litep2p::crypto::noise";
 
 #[derive(Debug)]
+#[allow(clippy::large_enum_variant)]
 enum NoiseState {
     Handshake(HandshakeState),
     Transport(TransportState),
@@ -171,7 +172,7 @@ impl NoiseContext {
 
     /// Get remote public key from the received Noise payload.
     // TODO: refactor
-    pub fn get_remote_public_key(&mut self, reply: &Vec<u8>) -> crate::Result<PublicKey> {
+    pub fn get_remote_public_key(&mut self, reply: &[u8]) -> crate::Result<PublicKey> {
         if reply.len() <= 2 {
             return Err(error::Error::InvalidData);
         }
@@ -192,10 +193,8 @@ impl NoiseContext {
 
         let payload = handshake_schema::NoiseHandshakePayload::decode(inner.as_slice())?;
 
-        Ok(PublicKey::from_protobuf_encoding(
-            &payload.identity_key.ok_or(error::Error::NegotiationError(
-                error::NegotiationError::PeerIdMissing,
-            ))?,
+        PublicKey::from_protobuf_encoding(&payload.identity_key.ok_or(
+            error::Error::NegotiationError(error::NegotiationError::PeerIdMissing),
         )?)
     }
 
@@ -600,7 +599,7 @@ impl<S: AsyncRead + AsyncWrite + Unpin> AsyncWrite for NoiseSocket<S> {
                             return Poll::Ready(Err(io::ErrorKind::InvalidData.into()));
                         }
                         Ok(nwritten) => {
-                            this.encrypt_buffer[offset + 0] = (nwritten >> 8) as u8;
+                            this.encrypt_buffer[offset] = (nwritten >> 8) as u8;
                             this.encrypt_buffer[offset + 1] = (nwritten & 0xff) as u8;
 
                             if let Some(next_chunk) = chunks.peek() {
@@ -691,15 +690,15 @@ pub async fn handshake<S: AsyncRead + AsyncWrite + Unpin>(
             // write initial message
             let first_message = noise.first_message(Role::Dialer);
             let _ = io.write(&first_message).await?;
-            let _ = io.flush().await?;
+            io.flush().await?;
 
             // read back response which contains the remote peer id
             let message = noise.read_handshake_message(&mut io).await?;
 
             // send the final message which contains local peer id
             let second_message = noise.second_message();
             let _ = io.write(&second_message).await?;
-            let _ = io.flush().await?;
+            io.flush().await?;
 
             parse_peer_id(&message)?
         }
@@ -710,7 +709,7 @@ pub async fn handshake<S: AsyncRead + AsyncWrite + Unpin>(
             // send local peer id.
             let second_message = noise.second_message();
             let _ = io.write(&second_message).await?;
-            let _ = io.flush().await?;
+            io.flush().await?;
 
             // read remote's second message which contains their peer id
             let message = noise.read_handshake_message(&mut io).await?;

diff --git a/src/error.rs b/src/error.rs
@@ -37,6 +37,7 @@ use multihash::{Multihash, MultihashGeneric};
 
 use std::io::{self, ErrorKind};
 
+#[allow(clippy::large_enum_variant)]
 #[derive(Debug, thiserror::Error)]
 pub enum Error {
     #[error("Peer `{0}` does not exist")]

diff --git a/src/executor.rs b/src/executor.rs
@@ -36,11 +36,11 @@ pub(crate) struct DefaultExecutor;
 
 impl Executor for DefaultExecutor {
     fn run(&self, future: Pin<Box<dyn Future<Output = ()> + Send>>) {
-        let _ = tokio::spawn(future);
+        tokio::spawn(future);
     }
 
     fn run_with_name(&self, _: &'static str, future: Pin<Box<dyn Future<Output = ()> + Send>>) {
-        let _ = tokio::spawn(future);
+        tokio::spawn(future);
     }
 }
 

diff --git a/src/lib.rs b/src/lib.rs
@@ -18,6 +18,16 @@
 // FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 // DEALINGS IN THE SOFTWARE.
 
+#![allow(clippy::single_match)]
+#![allow(clippy::result_large_err)]
+#![allow(clippy::redundant_pattern_matching)]
+#![allow(clippy::type_complexity)]
+#![allow(clippy::result_unit_err)]
+#![allow(clippy::should_implement_trait)]
+#![allow(clippy::too_many_arguments)]
+#![allow(clippy::assign_op_pattern)]
+#![allow(clippy::match_like_matches_macro)]
+
 use crate::{
     config::Litep2pConfig,
     protocol::{
@@ -48,8 +58,6 @@ pub use error::Error;
 pub use peer_id::PeerId;
 pub use types::protocol::ProtocolName;
 
-// pub use yamux;
-
 pub(crate) mod peer_id;
 
 pub mod codec;
@@ -219,7 +227,7 @@ impl Litep2p {
             );
 
             let main_protocol =
-                kademlia_config.protocol_names.get(0).expect("protocol name to exist");
+                kademlia_config.protocol_names.first().expect("protocol name to exist");
             let fallback_names = kademlia_config.protocol_names.iter().skip(1).cloned().collect();
 
             let service = transport_manager.register_protocol(
@@ -245,7 +253,7 @@ impl Litep2p {
                 let service = transport_manager.register_protocol(
                     identify_config.protocol.clone(),
                     Vec::new(),
-                    identify_config.codec.clone(),
+                    identify_config.codec,
                 );
                 identify_config.public = Some(litep2p_config.keypair.public().into());
 

diff --git a/src/multistream_select/dialer_select.rs b/src/multistream_select/dialer_select.rs
@@ -274,7 +274,7 @@ impl DialerState {
             std::iter::once(protocol.clone())
                 .chain(fallback_names.clone())
                 .filter_map(|protocol| Protocol::try_from(protocol.as_ref()).ok())
-                .map(|protocol| Message::Protocol(protocol)),
+                .map(Message::Protocol),
         )?
         .freeze()
         .to_vec();

diff --git a/src/multistream_select/protocol.rs b/src/multistream_select/protocol.rs
@@ -234,16 +234,15 @@ pub fn encode_multistream_message(
     // encode `/multistream-select/1.0.0` header
     let mut bytes = BytesMut::with_capacity(32);
     let message = Message::Header(HeaderLine::V1);
-    let _ = message.encode(&mut bytes).map_err(|_| Litep2pError::InvalidData)?;
+    message.encode(&mut bytes).map_err(|_| Litep2pError::InvalidData)?;
     let mut header = UnsignedVarint::encode(bytes)?;
 
     // encode each message
     for message in messages {
         let mut proto_bytes = BytesMut::with_capacity(256);
-        let _ = message.encode(&mut proto_bytes).map_err(|_| Litep2pError::InvalidData)?;
-        let proto_bytes = UnsignedVarint::encode(proto_bytes)?;
-
-        header.append(&mut proto_bytes.into());
+        message.encode(&mut proto_bytes).map_err(|_| Litep2pError::InvalidData)?;
+        let mut proto_bytes = UnsignedVarint::encode(proto_bytes)?;
+        header.append(&mut proto_bytes);
     }
 
     Ok(BytesMut::from(&header[..]))

diff --git a/src/protocol/connection.rs b/src/protocol/connection.rs
@@ -69,7 +69,7 @@ impl ConnectionHandle {
     /// This function is only called once when the connection is established to remote peer and that
     /// one time the connection type must be `Active`, unless there is a logic bug in `litep2p`.
     pub fn downgrade(&mut self) -> Self {
-        let connection = match &self.connection {
+        match &self.connection {
             ConnectionType::Active(connection) => {
                 let handle = Self::new(self.connection_id, connection.clone());
                 self.connection = ConnectionType::Inactive(connection.downgrade());
@@ -79,9 +79,7 @@ impl ConnectionHandle {
             ConnectionType::Inactive(_) => {
                 panic!("state mismatch: tried to downgrade an inactive connection")
             }
-        };
-
-        connection
+        }
     }
 
     /// Get reference to connection ID.

diff --git a/src/protocol/libp2p/identify.rs b/src/protocol/libp2p/identify.rs
@@ -209,11 +209,7 @@ impl Identify {
             service,
             tx: config.tx_event,
             peers: HashMap::new(),
-            listen_addresses: config
-                .public_addresses
-                .into_iter()
-                .chain(listen_addresses.into_iter())
-                .collect(),
+            listen_addresses: config.public_addresses.into_iter().chain(listen_addresses).collect(),
             public: config.public.expect("public key to be supplied"),
             protocol_version: config.protocol_version,
             user_agent: config.user_agent.unwrap_or(DEFAULT_AGENT.to_string()),
@@ -355,7 +351,7 @@ impl Identify {
                 .filter_map(|address| Multiaddr::try_from(address.clone()).ok())
                 .collect();
             let observed_address =
-                info.observed_addr.map(|address| Multiaddr::try_from(address).ok()).flatten();
+                info.observed_addr.and_then(|address| Multiaddr::try_from(address).ok());
             let protocol_version = info.protocol_version;
             let user_agent = info.agent_version;
 

diff --git a/src/protocol/libp2p/kademlia/bucket.rs b/src/protocol/libp2p/kademlia/bucket.rs
@@ -70,9 +70,9 @@ impl KBucket {
 
     /// Get entry into the bucket.
     // TODO: this is horrible code
-    pub fn entry<'a, K: Clone>(&'a mut self, key: Key<K>) -> KBucketEntry<'a> {
+    pub fn entry<K: Clone>(&mut self, key: Key<K>) -> KBucketEntry<'_> {
         for i in 0..self.nodes.len() {
-            if &self.nodes[i].key == &key {
+            if self.nodes[i].key == key {
                 return KBucketEntry::Occupied(&mut self.nodes[i]);
             }
         }

diff --git a/src/protocol/libp2p/kademlia/config.rs b/src/protocol/libp2p/kademlia/config.rs
@@ -122,6 +122,12 @@ pub struct ConfigBuilder {
     pub(super) protocol_names: Vec<ProtocolName>,
 }
 
+impl Default for ConfigBuilder {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl ConfigBuilder {
     /// Create new [`ConfigBuilder`].
     pub fn new() -> Self {

diff --git a/src/protocol/libp2p/kademlia/executor.rs b/src/protocol/libp2p/kademlia/executor.rs
@@ -88,18 +88,16 @@ impl QueryExecutor {
     pub fn send_message(&mut self, peer: PeerId, message: Bytes, mut substream: Substream) {
         self.futures.push(Box::pin(async move {
             match substream.send_framed(message).await {
-                Ok(_) =>
-                    return QueryContext {
-                        peer,
-                        query_id: None,
-                        result: QueryResult::SendSuccess { substream },
-                    },
-                Err(_) =>
-                    return QueryContext {
-                        peer,
-                        query_id: None,
-                        result: QueryResult::SubstreamClosed,
-                    },
+                Ok(_) => QueryContext {
+                    peer,
+                    query_id: None,
+                    result: QueryResult::SendSuccess { substream },
+                },
+                Err(_) => QueryContext {
+                    peer,
+                    query_id: None,
+                    result: QueryResult::SubstreamClosed,
+                },
             }
         }));
     }
@@ -113,24 +111,21 @@ impl QueryExecutor {
     ) {
         self.futures.push(Box::pin(async move {
             match tokio::time::timeout(READ_TIMEOUT, substream.next()).await {
-                Err(_) =>
-                    return QueryContext {
-                        peer,
-                        query_id,
-                        result: QueryResult::Timeout,
-                    },
-                Ok(Some(Ok(message))) =>
-                    return QueryContext {
-                        peer,
-                        query_id,
-                        result: QueryResult::ReadSuccess { substream, message },
-                    },
-                Ok(None) | Ok(Some(Err(_))) =>
-                    return QueryContext {
-                        peer,
-                        query_id,
-                        result: QueryResult::SubstreamClosed,
-                    },
+                Err(_) => QueryContext {
+                    peer,
+                    query_id,
+                    result: QueryResult::Timeout,
+                },
+                Ok(Some(Ok(message))) => QueryContext {
+                    peer,
+                    query_id,
+                    result: QueryResult::ReadSuccess { substream, message },
+                },
+                Ok(None) | Ok(Some(Err(_))) => QueryContext {
+                    peer,
+                    query_id,
+                    result: QueryResult::SubstreamClosed,
+                },
             }
         }));
     }
@@ -154,24 +149,21 @@ impl QueryExecutor {
             }
 
             match tokio::time::timeout(READ_TIMEOUT, substream.next()).await {
-                Err(_) =>
-                    return QueryContext {
-                        peer,
-                        query_id,
-                        result: QueryResult::Timeout,
-                    },
-                Ok(Some(Ok(message))) =>
-                    return QueryContext {
-                        peer,
-                        query_id,
-                        result: QueryResult::ReadSuccess { substream, message },
-                    },
-                Ok(None) | Ok(Some(Err(_))) =>
-                    return QueryContext {
-                        peer,
-                        query_id,
-                        result: QueryResult::SubstreamClosed,
-                    },
+                Err(_) => QueryContext {
+                    peer,
+                    query_id,
+                    result: QueryResult::Timeout,
+                },
+                Ok(Some(Ok(message))) => QueryContext {
+                    peer,
+                    query_id,
+                    result: QueryResult::ReadSuccess { substream, message },
+                },
+                Ok(None) | Ok(Some(Err(_))) => QueryContext {
+                    peer,
+                    query_id,
+                    result: QueryResult::SubstreamClosed,
+                },
             }
         }));
     }