Merge pull request #1188 from spacejam/tyler_clippy

Run clippy on tests and other new parts of the codebase

Author: spacejam

Cargo.toml

@@ -46,7 +46,7 @@ crossbeam-utils = "0.8.0"
 fxhash = "0.2.1"
 libc = "0.2.79"
 zstd = { version = "0.5.3", optional = true }
-crc32fast = "1.2.0"
+crc32fast = "1.2.1"
 log = "0.4.11"
 parking_lot = "0.11.0"
 color-backtrace = { version = "0.4.2", optional = true }

examples/playground.rs

@@ -13,10 +13,10 @@ fn basic() -> Result<()> {
 
     // set and get
     db.insert(k.clone(), v1.clone())?;
-    assert_eq!(db.get(&k).unwrap().unwrap(), (v1.clone()));
+    assert_eq!(db.get(&k).unwrap().unwrap(), (v1));
 
     // compare and swap
-    match db.compare_and_swap(k.clone(), Some(&v1.clone()), Some(v2.clone()))? {
+    match db.compare_and_swap(k.clone(), Some(&v1), Some(v2.clone()))? {
         Ok(()) => println!("it worked!"),
         Err(sled::CompareAndSwapError { current: cur, proposed: _ }) => {
             println!("the actual current value is {:?}", cur)
@@ -26,8 +26,8 @@ fn basic() -> Result<()> {
     // scan forward
     let mut iter = db.range(k.as_slice()..);
     let (k1, v1) = iter.next().unwrap().unwrap();
-    assert_eq!(v1, v2.clone());
-    assert_eq!(k1, k.clone());
+    assert_eq!(v1, v2);
+    assert_eq!(k1, k);
     assert_eq!(iter.next(), None);
 
     // deletion
@@ -43,7 +43,7 @@ fn merge_operator() -> Result<()> {
         merged_bytes: &[u8],      // the new bytes being merged in
     ) -> Option<Vec<u8>> {
         // set the new value, return None to delete
-        let mut ret = old_value.map_or_else(|| vec![], |ov| ov.to_vec());
+        let mut ret = old_value.map_or_else(Vec::new, |ov| ov.to_vec());
 
         ret.extend_from_slice(merged_bytes);
 

src/binary_search.rs

@@ -2,18 +2,15 @@ use std::cmp::Ordering::{Equal, Greater, Less};
 
 use crate::{fastcmp, IVec};
 
-pub(crate) fn binary_search_lub<'a>(
-    key: &[u8],
-    s: &'a [IVec],
-) -> Option<usize> {
+pub(crate) fn binary_search_lub(key: &[u8], s: &[IVec]) -> Option<usize> {
     match binary_search(key, s) {
         Ok(i) => Some(i),
         Err(i) if i == 0 => None,
         Err(i) => Some(i - 1),
     }
 }
 
-pub fn binary_search<'a>(key: &[u8], s: &'a [IVec]) -> Result<usize, usize> {
+pub fn binary_search(key: &[u8], s: &[IVec]) -> Result<usize, usize> {
     let mut size = s.len();
     if size == 0 || *key < *s[0] {
         return Err(0);
@@ -35,7 +32,11 @@ pub fn binary_search<'a>(key: &[u8], s: &'a [IVec]) -> Result<usize, usize> {
     #[allow(unsafe_code)]
     let l = unsafe { s.get_unchecked(base).as_ref() };
     let cmp = fastcmp(l, key);
-    if cmp == Equal { Ok(base) } else { Err(base + (cmp == Less) as usize) }
+    if cmp == Equal {
+        Ok(base)
+    } else {
+        Err(base + (cmp == Less) as usize)
+    }
 }
 
 #[test]

src/ivec.rs

@@ -376,20 +376,20 @@ fn ivec_as_mut_identity() {
 mod qc {
     use super::IVec;
 
-    fn prop_identity(ivec: IVec) -> bool {
+    fn prop_identity(ivec: &IVec) -> bool {
         let mut iv2 = ivec.clone();
 
         if iv2 != ivec {
             println!("expected clone to equal original");
             return false;
         }
 
-        if &*ivec != &mut *iv2 {
+        if *ivec != *iv2 {
             println!("expected AsMut to equal original");
             return false;
         }
 
-        if &*ivec != iv2.as_mut() {
+        if *ivec != iv2.as_mut() {
             println!("expected AsMut to equal original");
             return false;
         }
@@ -400,7 +400,7 @@ mod qc {
     quickcheck::quickcheck! {
         #[cfg_attr(miri, ignore)]
         fn bool(item: IVec) -> bool {
-            prop_identity(item)
+            prop_identity(&item)
         }
     }
 }

src/lib.rs

@@ -129,7 +129,6 @@
     // clippy::missing_const_for_fn,
     // clippy::missing_docs_in_private_items,
     clippy::module_name_repetitions,
-    clippy::multiple_crate_versions,
     clippy::multiple_inherent_impl,
     clippy::mut_mut,
     clippy::needless_borrow,
@@ -155,6 +154,7 @@
     // clippy::wildcard_enum_match_arm,
     clippy::wrong_pub_self_convention,
 )]
+#![warn(clippy::multiple_crate_versions)]
 #![allow(clippy::mem_replace_with_default)] // Not using std::mem::take() due to MSRV of 1.37 (intro'd in 1.40)
 #![allow(clippy::match_like_matches_macro)] // Not using std::matches! due to MSRV of 1.37 (intro'd in 1.42)
 

src/pagecache/iterator.rs

@@ -334,9 +334,7 @@ fn scan_segment_headers_and_tail(
     );
 
     // scatter
-    let header_promises_res: Result<
-        Vec<OneShot<Option<(LogOffset, SegmentHeader)>>>,
-    > = (0..segments)
+    let header_promises_res: Result<Vec<_>> = (0..segments)
         .map({
             // let config = config.clone();
             move |idx| {

src/serialization.rs

@@ -1020,91 +1020,91 @@ mod qc {
         }
     }
 
-    fn prop_serialize<T>(item: T) -> bool
+    fn prop_serialize<T>(item: &T) -> bool
     where
         T: Serialize + PartialEq + Clone + std::fmt::Debug,
     {
-        let mut buf = vec![0; item.serialized_size() as usize];
+        let mut buf = vec![0; usize::try_from(item.serialized_size()).unwrap()];
         let buf_ref = &mut buf.as_mut_slice();
         item.serialize_into(buf_ref);
         assert_eq!(
             buf_ref.len(),
             0,
             "round-trip failed to consume produced bytes"
         );
-        assert_eq!(buf.len(), item.serialized_size() as usize,);
+        assert_eq!(buf.len() as u64, item.serialized_size());
         let deserialized = T::deserialize(&mut buf.as_slice()).unwrap();
-        if item != deserialized {
+        if *item == deserialized {
+            true
+        } else {
             eprintln!(
                 "round-trip serialization failed. original:\n\n{:?}\n\n \
                  deserialized(serialized(original)):\n\n{:?}",
                 item, deserialized
             );
             false
-        } else {
-            true
         }
     }
 
     quickcheck::quickcheck! {
         #[cfg_attr(miri, ignore)]
         fn bool(item: bool) -> bool {
-            prop_serialize(item)
+            prop_serialize(&item)
         }
 
         #[cfg_attr(miri, ignore)]
         fn u8(item: u8) -> bool {
-            prop_serialize(item)
+            prop_serialize(&item)
         }
 
         #[cfg_attr(miri, ignore)]
         fn i64(item: SpreadI64) -> bool {
-            prop_serialize(item.0)
+            prop_serialize(&item.0)
         }
 
         #[cfg_attr(miri, ignore)]
         fn u64(item: SpreadU64) -> bool {
-            prop_serialize(item.0)
+            prop_serialize(&item.0)
         }
 
         #[cfg_attr(miri, ignore)]
         fn disk_ptr(item: DiskPtr) -> bool {
-            prop_serialize(item)
+            prop_serialize(&item)
         }
 
         #[cfg_attr(miri, ignore)]
         fn page_state(item: PageState) -> bool {
-            prop_serialize(item)
+            prop_serialize(&item)
         }
 
         #[cfg_attr(miri, ignore)]
         fn meta(item: Meta) -> bool {
-            prop_serialize(item)
+            prop_serialize(&item)
         }
 
         #[cfg_attr(miri, ignore)]
         fn snapshot(item: Snapshot) -> bool {
-            prop_serialize(item)
+            prop_serialize(&item)
         }
 
         #[cfg_attr(miri, ignore)]
         fn node(item: Node) -> bool {
-            prop_serialize(item)
+            prop_serialize(&item)
         }
 
         #[cfg_attr(miri, ignore)]
         fn data(item: Data) -> bool {
-            prop_serialize(item)
+            prop_serialize(&item)
         }
 
         #[cfg_attr(miri, ignore)]
         fn link(item: Link) -> bool {
-            prop_serialize(item)
+            prop_serialize(&item)
         }
 
         #[cfg_attr(miri, ignore)]
         fn msg_header(item: MessageHeader) -> bool {
-            prop_serialize(item)
+            prop_serialize(&item)
         }
     }
 
@@ -1122,6 +1122,6 @@ mod qc {
             data: Data::Index(Index::default()),
         };
 
-        prop_serialize(node);
+        prop_serialize(&node);
     }
 }

src/subscriber.rs

@@ -305,28 +305,19 @@ fn basic_subscriber() {
 
     let k2: IVec = vec![].into();
     let r2 = subs.reserve(&k2).unwrap();
-    r2.complete(&Event::Insert {
-        key: k2.clone(),
-        value: IVec::from(k2.clone()),
-    });
+    r2.complete(&Event::Insert { key: k2.clone(), value: k2.clone() });
 
     let k3: IVec = vec![0].into();
     let r3 = subs.reserve(&k3).unwrap();
-    r3.complete(&Event::Insert {
-        key: k3.clone(),
-        value: IVec::from(k3.clone()),
-    });
+    r3.complete(&Event::Insert { key: k3.clone(), value: k3.clone() });
 
     let k4: IVec = vec![0, 1].into();
     let r4 = subs.reserve(&k4).unwrap();
     r4.complete(&Event::Remove { key: k4.clone() });
 
     let k5: IVec = vec![0, 1, 2].into();
     let r5 = subs.reserve(&k5).unwrap();
-    r5.complete(&Event::Insert {
-        key: k5.clone(),
-        value: IVec::from(k5.clone()),
-    });
+    r5.complete(&Event::Insert { key: k5.clone(), value: k5.clone() });
 
     let k6: IVec = vec![1, 1, 2].into();
     let r6 = subs.reserve(&k6).unwrap();
@@ -338,10 +329,7 @@ fn basic_subscriber() {
 
     let k8: IVec = vec![1, 2, 2].into();
     let r8 = subs.reserve(&k8).unwrap();
-    r8.complete(&Event::Insert {
-        key: k8.clone(),
-        value: IVec::from(k8.clone()),
-    });
+    r8.complete(&Event::Insert { key: k8.clone(), value: k8.clone() });
 
     assert_eq!(s1.next().unwrap().key(), &*k2);
     assert_eq!(s1.next().unwrap().key(), &*k3);

src/tree.rs

@@ -1517,12 +1517,12 @@ impl Tree {
         Ok(())
     }
 
-    fn root_hoist<'g>(
+    fn root_hoist(
         &self,
         from: PageId,
         to: PageId,
         at: IVec,
-        guard: &'g Guard,
+        guard: &Guard,
     ) -> Result<bool> {
         M.tree_root_split_attempt();
         // hoist new root, pointing to lhs & rhs

tests/test_crash_recovery.rs

@@ -75,7 +75,7 @@ fn verify(tree: &sled::Tree) -> (u32, u32) {
     let mut lowest = 0;
     for res in iter {
         let (_k, v) = res.unwrap();
-        if &v[..4] == &highest_vec[..4] {
+        if v[..4] == highest_vec[..4] {
             contiguous += 1;
         } else {
             let expected = if highest == 0 {
@@ -238,12 +238,9 @@ fn run() {
         .path(RECOVERY_DIR.to_string())
         .segment_size(SEGMENT_SIZE);
 
-    match thread::spawn(|| run_inner(config)).join() {
-        Err(e) => {
-            println!("worker thread failed: {:?}", e);
-            std::process::exit(15);
-        }
-        _ => {}
+    if let Err(e) = thread::spawn(|| run_inner(config)).join() {
+        println!("worker thread failed: {:?}", e);
+        std::process::exit(15);
     }
 }
 
@@ -270,12 +267,9 @@ fn run_batches() {
         spawn_killah();
     }
 
-    match t1.join().and_then(|_| t2.join()) {
-        Err(e) => {
-            println!("worker thread failed: {:?}", e);
-            std::process::exit(15);
-        }
-        _ => {}
+    if let Err(e) = t1.join().and_then(|_| t2.join()) {
+        println!("worker thread failed: {:?}", e);
+        std::process::exit(15);
     }
 }
 
@@ -288,10 +282,9 @@ fn run_child_process(test_name: &str) -> Child {
         .env(TEST_ENV_VAR, test_name)
         .env("SLED_CRASH_CHANCE", CRASH_CHANCE.to_string())
         .spawn()
-        .expect(&format!(
-            "could not spawn child process for {} test",
-            test_name
-        ))
+        .unwrap_or_else(|_| {
+            panic!("could not spawn child process for {} test", test_name)
+        })
 }
 
 fn handle_child_exit_status(dir: &str, status: ExitStatus) {
@@ -516,8 +509,6 @@ fn run_iter() {
     let deleter = thread::Builder::new()
         .name("deleter".into())
         .spawn({
-            let t = t.clone();
-            let barrier = barrier.clone();
             move || {
                 barrier.wait();