Fix latest clippy assertion failures (#910)

* Use idiomatic assertions

Signed-off-by: Thane Thomson <[email protected]>

* Fix self conventions

Signed-off-by: Thane Thomson <[email protected]>

* Rename Time::to_rfc3339 to Time::as_rfc3339 to fix self convention

Signed-off-by: Thane Thomson <[email protected]>

* Remove unnecessary clones for Copy-able structs

Signed-off-by: Thane Thomson <[email protected]>

* Use idiomatic assertions

Signed-off-by: Thane Thomson <[email protected]>

* Use std::mem::take, which already provides this functionality

Signed-off-by: Thane Thomson <[email protected]>

* Use idiomatic assertions

Signed-off-by: Thane Thomson <[email protected]>

* Reduce to more idiomatic form

Signed-off-by: Thane Thomson <[email protected]>

* Pin tokio versions to 1.6.2 until they fix broken docsrs config

Signed-off-by: Thane Thomson <[email protected]>

* Rename to be consistent with Rust self conventions

Signed-off-by: Thane Thomson <[email protected]>

* Add .changelog entry for API change

Signed-off-by: Thane Thomson <[email protected]>

.changelog/unreleased/breaking-changes/909-self-conventions.md

@@ -0,0 +1,4 @@
+* `[tendermint]` Rename `time::Time::to_rfc3339` to `as_rfc3339` to be
+  consistent with Rust's [self reference
+  conventions](https://rust-lang.github.io/rust-clippy/master/index.html#wrong_self_convention)
+  ([#910](https://github.com/informalsystems/tendermint-rs/pull/910))

light-client/Cargo.toml

@@ -51,7 +51,8 @@ serde_derive = "1.0.106"
 sled = { version = "0.34.3", optional = true }
 static_assertions = "1.1.0"
 thiserror = "1.0.15"
-tokio = { version = "1.0", features = ["rt"], optional = true }
+# TODO(thane): Unpin this version once Tokio fixes the documentation compiling problem in v1.7+.
+tokio = { version = "~1.6.2", features = ["rt"], optional = true }
 
 [dev-dependencies]
 tendermint-testgen = { path = "../testgen" }

light-client/src/supervisor.rs

@@ -751,11 +751,10 @@ mod tests {
         assert_eq!(expected_state, &new_state);
 
         // Check that we successfully disconnected from the "faulty" primary node
-        assert!(latest_status
+        assert!(!latest_status
             .connected_nodes
             .iter()
-            .find(|&&peer| peer == primary[0].provider)
-            .is_none());
+            .any(|&peer| peer == primary[0].provider));
     }
 
     #[test]
@@ -779,11 +778,10 @@ mod tests {
         // and continues verification
         // Check if the node was removed from the list
 
-        assert!(latest_status
+        assert!(!latest_status
             .connected_nodes
             .iter()
-            .find(|&&peer| peer == primary[0].provider)
-            .is_none());
+            .any(|&peer| peer == primary[0].provider));
     }
 
     #[test]
@@ -809,10 +807,9 @@ mod tests {
         // and continues verification
         // Check if the node was removed from the list
 
-        assert!(latest_status
+        assert!(!latest_status
             .connected_nodes
             .iter()
-            .find(|&&peer| peer == primary[0].provider)
-            .is_none());
+            .any(|&peer| peer == primary[0].provider));
     }
 }

proto/src/serializers/timestamp.rs

@@ -47,7 +47,7 @@ where
     }
     match Utc.timestamp_opt(value.seconds, value.nanos as u32) {
         LocalResult::None => Err(S::Error::custom("invalid time")),
-        LocalResult::Single(t) => Ok(to_rfc3339_nanos(&t)),
+        LocalResult::Single(t) => Ok(as_rfc3339_nanos(&t)),
         LocalResult::Ambiguous(_, _) => Err(S::Error::custom("ambiguous time")),
     }?
     .serialize(serializer)
@@ -58,7 +58,7 @@ where
 /// This reproduces the behavior of Go's `time.RFC3339Nano` format,
 /// ie. a RFC3339 date-time with left-padded subsecond digits without
 ///     trailing zeros and no trailing dot.
-pub fn to_rfc3339_nanos(t: &DateTime<Utc>) -> String {
+pub fn as_rfc3339_nanos(t: &DateTime<Utc>) -> String {
     use chrono::format::{Fixed, Item, Numeric::*, Pad::Zero};
 
     const PREFIX: &[Item<'_>] = &[

proto/tests/unit.rs

@@ -63,7 +63,7 @@ pub fn protobuf_struct_example() {
     );
     let new_domain_type = BlockId::decode(wire.as_ref()).unwrap();
     assert_eq!(new_domain_type.hash, "Hello world!".to_string());
-    assert_eq!(new_domain_type.part_set_header_exists, false);
+    assert!(!new_domain_type.part_set_header_exists);
     assert_eq!(my_domain_type.encoded_len(), 14);
 }
 
@@ -83,7 +83,7 @@ pub fn protobuf_struct_length_delimited_example() {
 
     let new_domain_type = BlockId::decode_length_delimited(wire.as_ref()).unwrap();
     assert_eq!(new_domain_type.hash, "Hello world!".to_string());
-    assert_eq!(new_domain_type.part_set_header_exists, false);
+    assert!(!new_domain_type.part_set_header_exists);
 }
 
 #[test]

rpc/Cargo.toml

@@ -86,7 +86,8 @@ hyper = { version = "0.14", optional = true, features = ["client", "http1", "htt
 hyper-proxy = { version = "0.9", optional = true }
 hyper-rustls = { version = "0.22.1", optional = true }
 structopt = { version = "0.3", optional = true }
-tokio = { version = "1.0", optional = true }
+# TODO(thane): Unpin this version once Tokio fixes the documentation compiling problem in v1.7+.
+tokio = { version = "~1.6.2", optional = true }
 tracing = { version = "0.1", optional = true }
 tracing-subscriber = { version = "0.2", optional = true }
 

rpc/src/endpoint/consensus_state.rs

@@ -298,7 +298,7 @@ impl fmt::Display for VoteSummary {
             self.vote_type,
             self.block_id_hash_fingerprint,
             self.signature_fingerprint,
-            self.timestamp.to_rfc3339(),
+            self.timestamp.as_rfc3339(),
         )
     }
 }

rpc/tests/parse_response.rs

@@ -117,7 +117,7 @@ fn block_results() {
     let log_json_value = serde_json::Value::from_str(log_json.as_str()).unwrap();
 
     assert_eq!(log_json_value[0]["msg_index"].as_str().unwrap(), "0");
-    assert_eq!(log_json_value[0]["success"].as_bool().unwrap(), true);
+    assert!(log_json_value[0]["success"].as_bool().unwrap());
 
     assert_eq!(deliver_tx[0].gas_wanted.value(), 200_000);
     assert_eq!(deliver_tx[0].gas_used.value(), 105_662);
@@ -469,7 +469,7 @@ fn consensus_state() {
                 vec![123, 185, 116, 225, 186, 64]
             );
             assert_eq!(
-                summary.timestamp.to_rfc3339(),
+                summary.timestamp.as_rfc3339(),
                 "2019-08-01T11:52:35.513572509Z"
             );
         }
@@ -496,7 +496,7 @@ fn consensus_state() {
                 vec![139, 94, 255, 254, 171, 205]
             );
             assert_eq!(
-                summary.timestamp.to_rfc3339(),
+                summary.timestamp.as_rfc3339(),
                 "2019-08-01T11:52:36.25600005Z"
             );
         }

tendermint/src/public_key.rs

@@ -156,7 +156,7 @@ impl PublicKey {
     }
 
     /// Serialize this key as a byte vector.
-    pub fn to_bytes(&self) -> Vec<u8> {
+    pub fn to_bytes(self) -> Vec<u8> {
         match self {
             PublicKey::Ed25519(pk) => pk.as_bytes().to_vec(),
             #[cfg(feature = "secp256k1")]

tendermint/src/serializers/time.rs

@@ -10,7 +10,7 @@ pub fn serialize<S>(value: &Time, serializer: S) -> Result<S::Ok, S::Error>
 where
     S: Serializer,
 {
-    value.to_rfc3339().serialize(serializer)
+    value.as_rfc3339().serialize(serializer)
 }
 
 /// Deserialize `String` into `Time`

tendermint/src/signature.rs

@@ -75,7 +75,7 @@ impl Signature {
     }
 
     /// Get a vector containing the byte serialization of this key
-    pub fn to_bytes(&self) -> Vec<u8> {
+    pub fn to_bytes(self) -> Vec<u8> {
         self.as_bytes().to_vec()
     }
 }

tendermint/src/time.rs

@@ -75,14 +75,14 @@ impl Time {
     }
 
     /// Return an RFC 3339 and ISO 8601 date and time string with 6 subseconds digits and Z.
-    pub fn to_rfc3339(&self) -> String {
-        timestamp::to_rfc3339_nanos(&self.0)
+    pub fn as_rfc3339(&self) -> String {
+        timestamp::as_rfc3339_nanos(&self.0)
     }
 }
 
 impl fmt::Display for Time {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
-        write!(f, "{}", self.to_rfc3339())
+        write!(f, "{}", self.as_rfc3339())
     }
 }
 

tendermint/src/vote.rs

@@ -119,7 +119,7 @@ impl Vote {
 
     /// Returns block_id.hash
     pub fn header_hash(&self) -> Option<hash::Hash> {
-        self.block_id.clone().map(|b| b.hash)
+        self.block_id.map(|b| b.hash)
     }
 
     /// Create signable bytes from Vote.

tendermint/src/vote/power.rs

@@ -92,7 +92,7 @@ impl<'de> Deserialize<'de> for Power {
 
 impl Serialize for Power {
     fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
-        let proto_int: i64 = self.clone().into();
+        let proto_int: i64 = (*self).into();
         proto_int.to_string().serialize(serializer)
     }
 }

test/src/pipe.rs

@@ -22,7 +22,6 @@
 
 use std::cmp::min;
 use std::io::{self, BufRead, Read, Write};
-use std::mem::replace;
 
 use flume::{self, Receiver, SendError, Sender, TrySendError};
 
@@ -141,7 +140,7 @@ impl Write for BufWriter {
             self.flush()?;
         } else {
             // reserve capacity later to avoid needless allocations
-            let data = replace(&mut self.buffer, Vec::new());
+            let data = std::mem::take(&mut self.buffer);
 
             // buffer still has space but try to send it in case the other side already awaits
             match self.sender().try_send(data) {
@@ -162,7 +161,7 @@ impl Write for BufWriter {
         if self.buffer.is_empty() {
             Ok(())
         } else {
-            let data = replace(&mut self.buffer, Vec::new());
+            let data = std::mem::take(&mut self.buffer);
             match self.sender().send(data) {
                 Ok(_) => {
                     self.buffer.reserve(self.size);
@@ -184,7 +183,7 @@ impl Write for BufWriter {
 impl Drop for BufWriter {
     fn drop(&mut self) {
         if !self.buffer.is_empty() {
-            let data = replace(&mut self.buffer, Vec::new());
+            let data = std::mem::take(&mut self.buffer);
             self.sender().send(data).ok();
         }
     }

test/src/test/unit/p2p/secret_connection.rs

@@ -22,14 +22,14 @@ fn test_handshake() {
         let mut csprng = OsRng {};
         let privkey1: ed25519::Keypair = ed25519::Keypair::generate(&mut csprng);
         let conn1 = SecretConnection::new(pipe2, privkey1, Version::V0_34);
-        assert_eq!(conn1.is_ok(), true);
+        assert!(conn1.is_ok());
     });
 
     let peer2 = thread::spawn(|| {
         let mut csprng = OsRng {};
         let privkey2: ed25519::Keypair = ed25519::Keypair::generate(&mut csprng);
         let conn2 = SecretConnection::new(pipe1, privkey2, Version::V0_34);
-        assert_eq!(conn2.is_ok(), true);
+        assert!(conn2.is_ok());
     });
 
     peer1.join().expect("peer1 thread has panicked");
@@ -77,7 +77,7 @@ fn test_evil_peer_shares_invalid_eph_key() {
     let (mut h, _) = Handshake::new(local_privkey, Version::V0_34);
     let bytes: [u8; 32] = [0; 32];
     let res = h.got_key(EphemeralPublic::from(bytes));
-    assert_eq!(res.is_err(), true);
+    assert!(res.is_err());
 }
 
 #[test]
@@ -86,14 +86,14 @@ fn test_evil_peer_shares_invalid_auth_sig() {
     let local_privkey: ed25519::Keypair = ed25519::Keypair::generate(&mut csprng);
     let (mut h, _) = Handshake::new(local_privkey, Version::V0_34);
     let res = h.got_key(EphemeralPublic::from(x25519_dalek::X25519_BASEPOINT_BYTES));
-    assert_eq!(res.is_err(), false);
+    assert!(res.is_ok());
 
     let mut h = res.unwrap();
     let res = h.got_signature(proto::p2p::AuthSigMessage {
         pub_key: None,
         sig: vec![],
     });
-    assert_eq!(res.is_err(), true);
+    assert!(res.is_err());
 }
 
 #[test]