*: Apply clippy suggestions and enable clippy on CI (#1850)

* *: Apply clippy suggestions

* .github: Add clippy check

* protocols/kad/record: Implement custom PartialEq for ProviderRecord

.github/workflows/ci.yml

@@ -109,6 +109,42 @@ jobs:
     - name: Check rustdoc links
       run: RUSTDOCFLAGS="--deny broken_intra_doc_links" cargo doc --verbose --workspace --no-deps --document-private-items
 
+  check-clippy:
+    runs-on: ubuntu-latest
+    steps:
+
+    - name: Cancel Previous Runs
+      uses: styfle/cancel-workflow-action@0.6.0
+      with:
+        access_token: ${{ github.token }}
+
+    - uses: actions/checkout@v2
+
+    - uses: actions-rs/toolchain@v1
+      with:
+        profile: minimal
+        toolchain: stable
+        override: true
+        components: clippy
+
+    - name: Cache CARGO_HOME
+      uses: actions/cache@v2
+      with:
+        path: ~/.cargo
+        key: cargo-home-${{ hashFiles('Cargo.toml') }}
+
+    - name: Cache cargo build
+      uses: actions/cache@v2
+      with:
+        path: target
+        key: cargo-build-target-${{ hashFiles('Cargo.toml') }}
+
+    - name: Run cargo clippy
+      uses: actions-rs/cargo@v1
+      with:
+        command: clippy
+        args: -- -A clippy::mutable_key_type -A clippy::type_complexity
+
   integration-test:
     name: Integration tests
     runs-on: ubuntu-latest

core/src/connection.rs

@@ -63,20 +63,12 @@ impl std::ops::Not for Endpoint {
 impl Endpoint {
     /// Is this endpoint a dialer?
     pub fn is_dialer(self) -> bool {
-        if let Endpoint::Dialer = self {
+        matches!(self, Endpoint::Dialer)
-            true
-        } else {
-            false
-        }
     }
 
     /// Is this endpoint a listener?
     pub fn is_listener(self) -> bool {
-        if let Endpoint::Listener = self {
+        matches!(self, Endpoint::Listener)
-            true
-        } else {
-            false
-        }
     }
 }
 

core/src/connection/listeners.rs

@@ -41,7 +41,6 @@ use std::{collections::VecDeque, fmt, pin::Pin};
 /// # Example
 ///
 /// ```no_run
-/// # fn main() {
 /// use futures::prelude::*;
 /// use libp2p_core::connection::{ListenersEvent, ListenersStream};
 ///
@@ -75,7 +74,6 @@ use std::{collections::VecDeque, fmt, pin::Pin};
 ///         }
 ///     }
 /// })
-/// # }
 /// ```
 pub struct ListenersStream<TTrans>
 where

core/src/connection/manager.rs

@@ -336,10 +336,7 @@ impl<I, O, H, TE, HE> Manager<I, O, H, TE, HE> {
 
     /// Checks whether an established connection with the given ID is currently managed.
     pub fn is_established(&self, id: &ConnectionId) -> bool {
-        match self.tasks.get(&id.0) {
+        matches!(self.tasks.get(&id.0), Some(TaskInfo { state: TaskState::Established(..), .. }))
-            Some(TaskInfo { state: TaskState::Established(..), .. }) => true,
-            _ => false
-        }
     }
 
     /// Polls the manager for events relating to the managed connections.
@@ -528,4 +525,3 @@ impl<'a, I> PendingEntry<'a, I> {
         self.task.remove();
     }
 }
-

core/src/connection/pool.rs

@@ -457,15 +457,12 @@ impl<TInEvent, TOutEvent, THandler, TTransErr, THandlerErr>
             // Count upwards because we push to / pop from the end. See also `Pool::poll`.
             let mut num_established = 0;
             for (&id, endpoint) in conns.iter() {
-                match self.manager.entry(id) {
+                if let Some(manager::Entry::Established(e)) = self.manager.entry(id) {
-                    Some(manager::Entry::Established(e)) => {
                     let connected = e.remove();
                     self.disconnected.push(Disconnected {
                         id, connected, num_established
                     });
                     num_established += 1;
-                    },
-                    _ => {}
                 }
                 self.counters.dec_established(endpoint);
             }
@@ -475,12 +472,9 @@ impl<TInEvent, TOutEvent, THandler, TTransErr, THandlerErr>
         let mut aborted = Vec::new();
         for (&id, (_endpoint, peer2)) in &self.pending {
             if Some(peer) == peer2.as_ref() {
-                match self.manager.entry(id) {
+                if let Some(manager::Entry::Pending(e)) = self.manager.entry(id) {
-                    Some(manager::Entry::Pending(e)) => {
                     e.abort();
                     aborted.push(id);
-                    },
-                    _ => {}
                 }
             }
         }
@@ -578,7 +572,7 @@ impl<TInEvent, TOutEvent, THandler, TTransErr, THandlerErr>
         // connections. Thus we `pop()` them off from the end to emit the
         // events in an order that properly counts down `num_established`.
         // See also `Pool::disconnect`.
-        while let Some(Disconnected {
+        if let Some(Disconnected {
             id, connected, num_established
         }) = self.disconnected.pop() {
             return Poll::Ready(PoolEvent::ConnectionClosed {

core/src/network.rs

@@ -384,14 +384,11 @@ where
         let event = match self.pool.poll(cx) {
             Poll::Pending => return Poll::Pending,
             Poll::Ready(PoolEvent::ConnectionEstablished { connection, num_established }) => {
-                match self.dialing.entry(connection.peer_id().clone()) {
+                if let hash_map::Entry::Occupied(mut e) = self.dialing.entry(connection.peer_id().clone()) {
-                    hash_map::Entry::Occupied(mut e) => {
                     e.get_mut().retain(|s| s.current.0 != connection.id());
                     if e.get().is_empty() {
                         e.remove();
                     }
-                    },
-                    _ => {}
                 }
 
                 NetworkEvent::ConnectionEstablished {

core/src/network/peer.rs

@@ -197,10 +197,7 @@ where
     ///
     /// Returns `true` iff [`Peer::into_disconnected`] returns `Some`.
     pub fn is_disconnected(&self) -> bool {
-        match self {
+        matches!(self, Peer::Disconnected(..))
-            Peer::Disconnected(..) => true,
-            _ => false
-        }
     }
 
     /// Initiates a new dialing attempt to this peer using the given addresses.
@@ -303,8 +300,8 @@ where
     }
 
     /// Obtains an established connection to the peer by ID.
-    pub fn connection<'b>(&'b mut self, id: ConnectionId)
+    pub fn connection(&mut self, id: ConnectionId)
-        -> Option<EstablishedConnection<'b, TInEvent>>
+        -> Option<EstablishedConnection<TInEvent>>
     {
         self.network.pool.get_established(id)
     }
@@ -334,8 +331,8 @@ where
     }
 
     /// Gets an iterator over all established connections to the peer.
-    pub fn connections<'b>(&'b mut self) ->
+    pub fn connections(&mut self) ->
-        EstablishedConnectionIter<'b,
+        EstablishedConnectionIter<
             impl Iterator<Item = ConnectionId>,
             TInEvent,
             TOutEvent,
@@ -347,8 +344,8 @@ where
     }
 
     /// Obtains some established connection to the peer.
-    pub fn some_connection<'b>(&'b mut self)
+    pub fn some_connection(&mut self)
-        -> EstablishedConnection<'b, TInEvent>
+        -> EstablishedConnection<TInEvent>
     {
         self.connections()
             .into_first()

core/src/peer_id.rs

@@ -20,7 +20,6 @@
 
 use crate::PublicKey;
 use bytes::Bytes;
-use bs58;
 use thiserror::Error;
 use multihash::{Code, Multihash, MultihashDigest};
 use rand::Rng;

core/src/transport.rs

@@ -258,11 +258,7 @@ impl<TUpgr, TErr> ListenerEvent<TUpgr, TErr> {
 
     /// Returns `true` if this is an `Upgrade` listener event.
     pub fn is_upgrade(&self) -> bool {
-        if let ListenerEvent::Upgrade {..} = self {
+        matches!(self, ListenerEvent::Upgrade {..})
-            true
-        } else {
-            false
-        }
     }
 
     /// Try to turn this listener event into upgrade parts.
@@ -279,11 +275,7 @@ impl<TUpgr, TErr> ListenerEvent<TUpgr, TErr> {
 
     /// Returns `true` if this is a `NewAddress` listener event.
     pub fn is_new_address(&self) -> bool {
-        if let ListenerEvent::NewAddress(_) = self {
+        matches!(self, ListenerEvent::NewAddress(_))
-            true
-        } else {
-            false
-        }
     }
 
     /// Try to turn this listener event into the `NewAddress` part.
@@ -300,11 +292,7 @@ impl<TUpgr, TErr> ListenerEvent<TUpgr, TErr> {
 
     /// Returns `true` if this is an `AddressExpired` listener event.
     pub fn is_address_expired(&self) -> bool {
-        if let ListenerEvent::AddressExpired(_) = self {
+        matches!(self, ListenerEvent::AddressExpired(_))
-            true
-        } else {
-            false
-        }
     }
 
     /// Try to turn this listener event into the `AddressExpired` part.
@@ -321,11 +309,7 @@ impl<TUpgr, TErr> ListenerEvent<TUpgr, TErr> {
 
     /// Returns `true` if this is an `Error` listener event.
     pub fn is_error(&self) -> bool {
-        if let ListenerEvent::Error(_) = self {
+        matches!(self, ListenerEvent::Error(_))
-            true
-        } else {
-            false
-        }
     }
 
     /// Try to turn this listener event into the `Error` part.

core/src/transport/memory.rs

@@ -101,7 +101,7 @@ pub struct DialFuture {
 
 impl DialFuture {
     fn new(port: NonZeroU64) -> Option<Self> {
-        let sender = HUB.get(&port)?.clone();
+        let sender = HUB.get(&port)?;
 
         let (_dial_port_channel, dial_port) = HUB.register_port(0)
             .expect("there to be some random unoccupied port.");

core/src/transport/upgrade.rs

@@ -367,7 +367,7 @@ where
     type Dial = DialUpgradeFuture<T::Dial, U, C>;
 
     fn dial(self, addr: Multiaddr) -> Result<Self::Dial, TransportError<Self::Error>> {
-        let future = self.inner.dial(addr.clone())
+        let future = self.inner.dial(addr)
             .map_err(|err| err.map(TransportUpgradeError::Transport))?;
         Ok(DialUpgradeFuture {
             future: Box::pin(future),

misc/multiaddr/src/errors.rs

@@ -1,12 +1,11 @@
 use std::{net, fmt, error, io, num, str, string};
-use bs58;
-use multihash;
 use unsigned_varint::decode;
 
 pub type Result<T> = ::std::result::Result<T, Error>;
 
 /// Error types
 #[derive(Debug)]
+#[non_exhaustive]
 pub enum Error {
     DataLessThanLen,
     InvalidMultiaddr,
@@ -15,8 +14,6 @@ pub enum Error {
     ParsingError(Box<dyn error::Error + Send + Sync>),
     UnknownProtocolId(u32),
     UnknownProtocolString(String),
-    #[doc(hidden)]
-    __Nonexhaustive
 }
 
 impl fmt::Display for Error {
@@ -29,7 +26,6 @@ impl fmt::Display for Error {
             Error::ParsingError(e) => write!(f, "failed to parse: {}", e),
             Error::UnknownProtocolId(id) => write!(f, "unknown protocol id: {}", id),
             Error::UnknownProtocolString(string) => write!(f, "unknown protocol string: {}", string),
-            Error::__Nonexhaustive => f.write_str("__Nonexhaustive")
         }
     }
 }
@@ -92,4 +88,3 @@ impl From<decode::Error> for Error {
         Error::InvalidUvar(e)
     }
 }
-

misc/multiaddr/src/lib.rs

@@ -40,6 +40,7 @@ static_assertions::const_assert! {
 }
 
 /// Representation of a Multiaddr.
+#[allow(clippy::rc_buffer)]
 #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Hash)]
 pub struct Multiaddr { bytes: Arc<Vec<u8>> }
 
@@ -59,6 +60,11 @@ impl Multiaddr {
         self.bytes.len()
     }
 
+    /// Returns true if the length of this multiaddress is 0.
+    pub fn is_empty(&self) -> bool {
+        self.bytes.len() == 0
+    }
+
     /// Return a copy of this [`Multiaddr`]'s byte representation.
     pub fn to_vec(&self) -> Vec<u8> {
         Vec::from(&self.bytes[..])

misc/multiaddr/src/protocol.rs

@@ -1,6 +1,5 @@
 
 use arrayref::array_ref;
-use bs58;
 use byteorder::{BigEndian, ByteOrder, ReadBytesExt, WriteBytesExt};
 use crate::{Result, Error};
 use data_encoding::BASE32;

misc/multistream-select/src/lib.rs

@@ -76,7 +76,6 @@
 //! For a dialer:
 //!
 //! ```no_run
-//! # fn main() {
 //! use async_std::net::TcpStream;
 //! use multistream_select::{dialer_select_proto, Version};
 //! use futures::prelude::*;
@@ -90,7 +89,6 @@
 //!     println!("Negotiated protocol: {:?}", protocol);
 //!     // You can now use `_io` to communicate with the remote.
 //! });
-//! # }
 //! ```
 //!
 
@@ -105,4 +103,3 @@ pub use self::negotiated::{Negotiated, NegotiatedComplete, NegotiationError};
 pub use self::protocol::{ProtocolError, Version};
 pub use self::dialer_select::{dialer_select_proto, DialerSelectFuture};
 pub use self::listener_select::{listener_select_proto, ListenerSelectFuture};
-

misc/multistream-select/src/negotiated.rs

@@ -61,12 +61,12 @@ where
         match Negotiated::poll(Pin::new(&mut io), cx) {
             Poll::Pending => {
                 self.inner = Some(io);
-                return Poll::Pending
+                Poll::Pending
             },
             Poll::Ready(Ok(())) => Poll::Ready(Ok(io)),
             Poll::Ready(Err(err)) => {
                 self.inner = Some(io);
-                return Poll::Ready(Err(err));
+                Poll::Ready(Err(err))
             }
         }
     }
@@ -104,9 +104,8 @@ impl<TInner> Negotiated<TInner> {
 
         let mut this = self.project();
 
-        match this.state.as_mut().project() {
+        if let StateProj::Completed { .. } = this.state.as_mut().project() {
-            StateProj::Completed { .. } => return Poll::Ready(Ok(())),
+             return Poll::Ready(Ok(()));
-            _ => {}
         }
 
         // Read outstanding protocol negotiation messages.
@@ -189,12 +188,9 @@ where
         -> Poll<Result<usize, io::Error>>
     {
         loop {
-            match self.as_mut().project().state.project() {
+            if let StateProj::Completed { io } = self.as_mut().project().state.project() {
-                StateProj::Completed { io } => {
                 // If protocol negotiation is complete, commence with reading.
-                    return io.poll_read(cx, buf)
+                return io.poll_read(cx, buf);
-                },
-                _ => {}
             }
 
             // Poll the `Negotiated`, driving protocol negotiation to completion,
@@ -220,12 +216,9 @@ where
         -> Poll<Result<usize, io::Error>>
     {
         loop {
-            match self.as_mut().project().state.project() {
+            if let StateProj::Completed { io } = self.as_mut().project().state.project() {
-                StateProj::Completed { io } => {
                 // If protocol negotiation is complete, commence with reading.
                 return io.poll_read_vectored(cx, bufs)
-                },
-                _ => {}
             }
 
             // Poll the `Negotiated`, driving protocol negotiation to completion,

misc/multistream-select/src/protocol.rs

@@ -240,7 +240,7 @@ impl Message {
         let mut remaining: &[u8] = &msg;
         loop {
             // A well-formed message must be terminated with a newline.
-            if remaining == &[b'\n'] {
+            if remaining == [b'\n'] {
                 break
             } else if protocols.len() == MAX_PROTOCOLS {
                 return Err(ProtocolError::TooManyProtocols)
@@ -261,7 +261,7 @@ impl Message {
             remaining = &tail[len ..];
         }
 
-        return Ok(Message::Protocols(protocols));
+        Ok(Message::Protocols(protocols))
     }
 }
 
@@ -342,7 +342,7 @@ where
             Poll::Pending => Poll::Pending,
             Poll::Ready(None) => Poll::Ready(None),
             Poll::Ready(Some(Ok(m))) => Poll::Ready(Some(Ok(m))),
-            Poll::Ready(Some(Err(err))) => Poll::Ready(Some(Err(From::from(err)))),
+            Poll::Ready(Some(Err(err))) => Poll::Ready(Some(Err(err))),
         }
     }
 }
@@ -450,7 +450,7 @@ impl Into<io::Error> for ProtocolError {
         if let ProtocolError::IoError(e) = self {
             return e
         }
-        return io::ErrorKind::InvalidData.into()
+        io::ErrorKind::InvalidData.into()
     }
 }
 
@@ -529,4 +529,3 @@ mod tests {
         quickcheck(prop as fn(_))
     }
 }
-

muxers/mplex/src/codec.rs

@@ -61,6 +61,7 @@ impl fmt::Display for LocalStreamId {
 }
 
 impl Hash for LocalStreamId {
+    #![allow(clippy::derive_hash_xor_eq)]
     fn hash<H: Hasher>(&self, state: &mut H) {
         state.write_u32(self.num);
     }

protocols/kad/CHANGELOG.md

@@ -2,6 +2,9 @@
 
 - Update `libp2p-core` and `libp2p-swarm`.
 
+- Have two `ProviderRecord`s be equal iff their `key` and `provider` fields are
+  equal. [PR 1850](https://github.com/libp2p/rust-libp2p/pull/1850/).
+
 # 0.25.0 [2020-11-09]
 
 - Upon newly established connections, delay routing table

protocols/kad/src/record.rs

@@ -103,7 +103,11 @@ impl Record {
 
 /// A record stored in the DHT whose value is the ID of a peer
 /// who can provide the value on-demand.
-#[derive(Clone, Debug, PartialEq, Eq)]
+///
+/// Note: Two [`ProviderRecord`]s as well as their corresponding hashes are
+/// equal iff their `key` and `provider` fields are equal. See the [`Hash`] and
+/// [`PartialEq`] implementations.
+#[derive(Clone, Debug)]
 pub struct ProviderRecord {
     /// The key whose value is provided by the provider.
     pub key: Key,
@@ -122,6 +126,14 @@ impl Hash for ProviderRecord {
     }
 }
 
+impl PartialEq for ProviderRecord {
+    fn eq(&self, other: &Self) -> bool {
+        self.key == other.key && self.provider == other.provider
+    }
+}
+
+impl Eq for ProviderRecord {}
+
 impl ProviderRecord {
     /// Creates a new provider record for insertion into a `RecordStore`.
     pub fn new<K>(key: K, provider: PeerId, addresses: Vec<Multiaddr>) -> Self
@@ -187,4 +199,3 @@ mod tests {
         }
     }
 }
-

protocols/mdns/src/service.rs

@@ -551,7 +551,7 @@ impl MdnsPeer {
 
         MdnsPeer {
             addrs,
-            peer_id: my_peer_id.clone(),
+            peer_id: my_peer_id,
             ttl,
         }
     }

protocols/pnet/src/lib.rs

@@ -74,7 +74,7 @@ impl PreSharedKey {
         cipher.apply_keystream(&mut enc);
         let mut hasher = Shake128::default();
         hasher.write_all(&enc).expect("shake128 failed");
-        hasher.finalize_xof().read(&mut out).expect("shake128 failed");
+        hasher.finalize_xof().read_exact(&mut out).expect("shake128 failed");
         Fingerprint(out)
     }
 }
@@ -109,7 +109,7 @@ impl FromStr for PreSharedKey {
     type Err = KeyParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        if let &[keytype, encoding, key] = s.lines().take(3).collect::<Vec<_>>().as_slice() {
+        if let [keytype, encoding, key] = *s.lines().take(3).collect::<Vec<_>>().as_slice() {
             if keytype != "/key/swarm/psk/1.0.0/" {
                 return Err(KeyParseError::InvalidKeyType);
             }