integrate clippy into CI (#108)

.travis.yml

@@ -15,8 +15,12 @@ before_script:
   - rustup toolchain install nightly
   - rustup component add rustfmt-preview --toolchain nightly
   - command -v rustfmt || cargo install --force rustfmt-nightly
+  # install clippy via rustup, use the github source as a fallback
+  - rustup component add clippy || cargo install --git https://github.com/rust-lang/rust-clippy/ --force clippy
 script:
   - cargo +nightly fmt --all -- --check
+  # check all the targets (source code, tests, non-default crate features), and stop the build upon any warning
+  - cargo clippy --all-targets --all-features -- -D warnings
   - cargo build --verbose
   - cargo test --verbose
   - ./run-all-examples.sh

examples/iterator.rs

@@ -42,7 +42,7 @@ fn main() {
         println!("{}, {:?}", str::from_utf8(country).unwrap(), city);
     }
 
-    println!("");
+    println!();
     println!("Iterating from the given key...");
     // Reader::iter_from() iterates from the first key equal to or greater
     // than the given key.
@@ -51,7 +51,7 @@ fn main() {
         println!("{}, {:?}", str::from_utf8(country).unwrap(), city);
     }
 
-    println!("");
+    println!();
     println!("Iterating from the given prefix...");
     let mut iter = store.iter_from(&reader, "Un").unwrap();
     while let Some(Ok((country, city))) = iter.next() {

examples/simple-store.rs

@@ -34,12 +34,12 @@ fn getput<'env, 's>(store: MultiStore, writer: &'env mut RwTransaction, ids: &'s
             }
         }
     }
-    for i in 0..ids.len() {
-        let _r = store.put(writer, &ids[i], &Value::Blob(b"weeeeeee")).unwrap();
+    for id in ids {
+        store.put(writer, &id, &Value::Blob(b"weeeeeee")).unwrap();
     }
 }
 
-fn delete<'env, 's>(store: MultiStore, writer: &'env mut RwTransaction) {
+fn delete(store: MultiStore, writer: &mut RwTransaction) {
     let keys = vec!["str1", "str2", "str3"];
     let vals = vec!["string uno", "string quatro", "string siete"];
     // we convert the writer into a cursor so that we can safely read
@@ -69,7 +69,7 @@ fn main() {
         store.put(&mut writer, "int", &Value::I64(1234)).unwrap();
         store.put(&mut writer, "uint", &Value::U64(1234_u64)).unwrap();
         store.put(&mut writer, "float", &Value::F64(1234.0.into())).unwrap();
-        store.put(&mut writer, "instant", &Value::Instant(1528318073700)).unwrap();
+        store.put(&mut writer, "instant", &Value::Instant(1_528_318_073_700)).unwrap();
         store.put(&mut writer, "boolean", &Value::Bool(true)).unwrap();
         store.put(&mut writer, "string", &Value::Str("héllo, yöu")).unwrap();
         store.put(&mut writer, "json", &Value::Json(r#"{"foo":"bar", "number": 1}"#)).unwrap();

src/env.rs

@@ -55,6 +55,7 @@ impl Rkv {
     }
 
     /// Return a new Rkv environment that supports up to `DEFAULT_MAX_DBS` open databases.
+    #[allow(clippy::new_ret_no_self)]
     pub fn new(path: &Path) -> Result<Rkv, StoreError> {
         Rkv::with_capacity(path, DEFAULT_MAX_DBS)
     }
@@ -195,6 +196,7 @@ impl Rkv {
     }
 }
 
+#[allow(clippy::cyclomatic_complexity)]
 #[cfg(test)]
 mod tests {
     use byteorder::{
@@ -459,9 +461,9 @@ mod tests {
         multistore.put(&mut writer, "str3", &Value::Str("str3 foo")).unwrap();
         multistore.put(&mut writer, "str3", &Value::Str("str3 bar")).unwrap();
         writer.commit().unwrap();
-        let mut writer = k.write().unwrap();
+        let writer = k.write().unwrap();
         {
-            let mut iter = multistore.get(&mut writer, "str1").unwrap();
+            let mut iter = multistore.get(&writer, "str1").unwrap();
             let (id, val) = iter.next().unwrap().unwrap();
             assert_eq!((id, val), (&b"str1"[..], Some(Value::Str("str1 bar"))));
             let (id, val) = iter.next().unwrap().unwrap();
@@ -532,7 +534,7 @@ mod tests {
         // as the Value::I64 borrows an immutable reference to the Writer.
         // So we extract and copy its primitive value.
 
-        fn get_existing_foo(txn: &RwTransaction, store: &SingleStore) -> Option<i64> {
+        fn get_existing_foo(txn: &RwTransaction, store: SingleStore) -> Option<i64> {
             match store.get(txn, "foo").expect("read") {
                 Some(Value::I64(val)) => Some(val),
                 _ => None,
@@ -540,11 +542,11 @@ mod tests {
         }
 
         let mut writer = k.write().expect("writer");
-        let mut existing = get_existing_foo(&writer, &sk).unwrap_or(99);
+        let mut existing = get_existing_foo(&writer, sk).unwrap_or(99);
         existing += 1;
         sk.put(&mut writer, "foo", &Value::I64(existing)).expect("success");
 
-        let updated = get_existing_foo(&writer, &sk).unwrap_or(99);
+        let updated = get_existing_foo(&writer, sk).unwrap_or(99);
         assert_eq!(updated, 100);
         writer.commit().expect("commit");
     }
@@ -704,7 +706,7 @@ mod tests {
                 k.open_integer(&format!("sk{}", i)[..], StoreOptions::create()).expect("opened");
             {
                 let mut writer = k.write().expect("writer");
-                sk.put(&mut writer, i, &Value::I64(i as i64)).expect("wrote");
+                sk.put(&mut writer, i, &Value::I64(i64::from(i))).expect("wrote");
                 writer.commit().expect("committed");
             }
         }
@@ -822,9 +824,9 @@ mod tests {
         s2.put(&mut writer, "foo", &Value::I64(123)).expect("wrote");
         s3.put(&mut writer, "foo", &Value::Bool(true)).expect("wrote");
 
-        assert_eq!(s1.get(&mut writer, "foo").expect("read"), Some(Value::Str("bar")));
-        assert_eq!(s2.get(&mut writer, "foo").expect("read"), Some(Value::I64(123)));
-        assert_eq!(s3.get(&mut writer, "foo").expect("read"), Some(Value::Bool(true)));
+        assert_eq!(s1.get(&writer, "foo").expect("read"), Some(Value::Str("bar")));
+        assert_eq!(s2.get(&writer, "foo").expect("read"), Some(Value::I64(123)));
+        assert_eq!(s3.get(&writer, "foo").expect("read"), Some(Value::Bool(true)));
 
         writer.commit().expect("committed");
 

src/store/integer.rs

@@ -65,7 +65,8 @@ impl<K> Key<K>
 where
     K: EncodableKey,
 {
-    pub(crate) fn new(k: K) -> Result<Key<K>, DataError> {
+    #[allow(clippy::new_ret_no_self)]
+    pub(crate) fn new(k: &K) -> Result<Key<K>, DataError> {
         Ok(Key {
             bytes: k.to_bytes()?,
             phantom: PhantomData,
@@ -93,15 +94,15 @@ where
     }
 
     pub fn get<'env, T: Transaction>(&self, txn: &'env T, k: K) -> Result<Option<Value<'env>>, StoreError> {
-        self.inner.get(txn, Key::new(k)?)
+        self.inner.get(txn, Key::new(&k)?)
     }
 
     pub fn put(&self, txn: &mut RwTransaction, k: K, v: &Value) -> Result<(), StoreError> {
-        self.inner.put(txn, Key::new(k)?, v)
+        self.inner.put(txn, Key::new(&k)?, v)
     }
 
     pub fn delete(&self, txn: &mut RwTransaction, k: K) -> Result<(), StoreError> {
-        self.inner.delete(txn, Key::new(k)?)
+        self.inner.delete(txn, Key::new(&k)?)
     }
 }
 
@@ -118,7 +119,7 @@ mod tests {
         let root = Builder::new().prefix("test_integer_keys").tempdir().expect("tempdir");
         fs::create_dir_all(root.path()).expect("dir created");
         let k = Rkv::new(root.path()).expect("new succeeded");
-        let mut s = k.open_integer("s", StoreOptions::create()).expect("open");
+        let s = k.open_integer("s", StoreOptions::create()).expect("open");
 
         macro_rules! test_integer_keys {
             ($type:ty, $key:expr) => {{

src/store/integermulti.rs

@@ -51,15 +51,15 @@ where
     }
 
     pub fn get<'env, T: Transaction>(&self, txn: &'env T, k: K) -> Result<Iter<'env>, StoreError> {
-        self.inner.get(txn, Key::new(k)?)
+        self.inner.get(txn, Key::new(&k)?)
     }
 
     pub fn get_first<'env, T: Transaction>(&self, txn: &'env T, k: K) -> Result<Option<Value<'env>>, StoreError> {
-        self.inner.get_first(txn, Key::new(k)?)
+        self.inner.get_first(txn, Key::new(&k)?)
     }
 
     pub fn put(&self, txn: &mut RwTransaction, k: K, v: &Value) -> Result<(), StoreError> {
-        self.inner.put(txn, Key::new(k)?, v)
+        self.inner.put(txn, Key::new(&k)?, v)
     }
 
     pub fn put_with_flags(
@@ -69,15 +69,15 @@ where
         v: &Value,
         flags: WriteFlags,
     ) -> Result<(), StoreError> {
-        self.inner.put_with_flags(txn, Key::new(k)?, v, flags)
+        self.inner.put_with_flags(txn, Key::new(&k)?, v, flags)
     }
 
     pub fn delete_all(&self, txn: &mut RwTransaction, k: K) -> Result<(), StoreError> {
-        self.inner.delete_all(txn, Key::new(k)?)
+        self.inner.delete_all(txn, Key::new(&k)?)
     }
 
     pub fn delete(&self, txn: &mut RwTransaction, k: K, v: &Value) -> Result<(), StoreError> {
-        self.inner.delete(txn, Key::new(k)?, v)
+        self.inner.delete(txn, Key::new(&k)?, v)
     }
 }
 
@@ -96,7 +96,7 @@ mod tests {
         let root = Builder::new().prefix("test_integer_keys").tempdir().expect("tempdir");
         fs::create_dir_all(root.path()).expect("dir created");
         let k = Rkv::new(root.path()).expect("new succeeded");
-        let mut s = k.open_multi_integer("s", StoreOptions::create()).expect("open");
+        let s = k.open_multi_integer("s", StoreOptions::create()).expect("open");
 
         macro_rules! test_integer_keys {
             ($type:ty, $key:expr) => {{

src/store/multi.rs

@@ -62,7 +62,7 @@ impl MultiStore {
     }
 
     /// Provides a cursor to all of the values for the duplicate entries that match this key
-    pub fn get<'env, T: Transaction, K: AsRef<[u8]>>(&self, txn: &'env T, k: K) -> Result<Iter<'env>, StoreError> {
+    pub fn get<T: Transaction, K: AsRef<[u8]>>(self, txn: &T, k: K) -> Result<Iter, StoreError> {
         let mut cursor = txn.open_ro_cursor(self.db).map_err(StoreError::LmdbError)?;
         let iter = cursor.iter_dup_of(k);
         Ok(Iter {
@@ -72,11 +72,7 @@ impl MultiStore {
     }
 
     /// Provides a cursor to all of the values for the duplicate entries that match this key
-    pub fn get_first<'env, T: Transaction, K: AsRef<[u8]>>(
-        &self,
-        txn: &'env T,
-        k: K,
-    ) -> Result<Option<Value<'env>>, StoreError> {
+    pub fn get_first<T: Transaction, K: AsRef<[u8]>>(self, txn: &T, k: K) -> Result<Option<Value>, StoreError> {
         let result = txn.get(self.db, &k);
         read_transform(result)
     }
@@ -84,13 +80,13 @@ impl MultiStore {
     /// Insert a value at the specified key.
     /// This put will allow duplicate entries.  If you wish to have duplicate entries
     /// rejected, use the `put_with_flags` function and specify NO_DUP_DATA
-    pub fn put<K: AsRef<[u8]>>(&self, txn: &mut RwTransaction, k: K, v: &Value) -> Result<(), StoreError> {
+    pub fn put<K: AsRef<[u8]>>(self, txn: &mut RwTransaction, k: K, v: &Value) -> Result<(), StoreError> {
         let bytes = v.to_bytes()?;
         txn.put(self.db, &k, &bytes, WriteFlags::empty()).map_err(StoreError::LmdbError)
     }
 
     pub fn put_with_flags<K: AsRef<[u8]>>(
-        &self,
+        self,
         txn: &mut RwTransaction,
         k: K,
         v: &Value,
@@ -100,11 +96,11 @@ impl MultiStore {
         txn.put(self.db, &k, &bytes, flags).map_err(StoreError::LmdbError)
     }
 
-    pub fn delete_all<K: AsRef<[u8]>>(&self, txn: &mut RwTransaction, k: K) -> Result<(), StoreError> {
+    pub fn delete_all<K: AsRef<[u8]>>(self, txn: &mut RwTransaction, k: K) -> Result<(), StoreError> {
         txn.del(self.db, &k, None).map_err(StoreError::LmdbError)
     }
 
-    pub fn delete<K: AsRef<[u8]>>(&self, txn: &mut RwTransaction, k: K, v: &Value) -> Result<(), StoreError> {
+    pub fn delete<K: AsRef<[u8]>>(self, txn: &mut RwTransaction, k: K, v: &Value) -> Result<(), StoreError> {
         txn.del(self.db, &k, Some(&v.to_bytes()?)).map_err(StoreError::LmdbError)
     }
 

src/store/single.rs

@@ -50,27 +50,23 @@ impl SingleStore {
         }
     }
 
-    pub fn get<'env, T: Transaction, K: AsRef<[u8]>>(
-        &self,
-        txn: &'env T,
-        k: K,
-    ) -> Result<Option<Value<'env>>, StoreError> {
+    pub fn get<T: Transaction, K: AsRef<[u8]>>(self, txn: &T, k: K) -> Result<Option<Value>, StoreError> {
         let bytes = txn.get(self.db, &k);
         read_transform(bytes)
     }
 
     // TODO: flags
-    pub fn put<K: AsRef<[u8]>>(&self, txn: &mut RwTransaction, k: K, v: &Value) -> Result<(), StoreError> {
+    pub fn put<K: AsRef<[u8]>>(self, txn: &mut RwTransaction, k: K, v: &Value) -> Result<(), StoreError> {
         // TODO: don't allocate twice.
         let bytes = v.to_bytes()?;
         txn.put(self.db, &k, &bytes, WriteFlags::empty()).map_err(StoreError::LmdbError)
     }
 
-    pub fn delete<K: AsRef<[u8]>>(&self, txn: &mut RwTransaction, k: K) -> Result<(), StoreError> {
+    pub fn delete<K: AsRef<[u8]>>(self, txn: &mut RwTransaction, k: K) -> Result<(), StoreError> {
         txn.del(self.db, &k, None).map_err(StoreError::LmdbError)
     }
 
-    pub fn iter_start<'env, T: Transaction>(&self, txn: &'env T) -> Result<Iter<'env>, StoreError> {
+    pub fn iter_start<T: Transaction>(self, txn: &T) -> Result<Iter, StoreError> {
         let mut cursor = txn.open_ro_cursor(self.db).map_err(StoreError::LmdbError)?;
 
         // We call Cursor.iter() instead of Cursor.iter_start() because
@@ -89,11 +85,7 @@ impl SingleStore {
         })
     }
 
-    pub fn iter_from<'env, T: Transaction, K: AsRef<[u8]>>(
-        &self,
-        txn: &'env T,
-        k: K,
-    ) -> Result<Iter<'env>, StoreError> {
+    pub fn iter_from<T: Transaction, K: AsRef<[u8]>>(self, txn: &T, k: K) -> Result<Iter, StoreError> {
         let mut cursor = txn.open_ro_cursor(self.db).map_err(StoreError::LmdbError)?;
         let iter = cursor.iter_from(k);
         Ok(Iter {

src/value.rs

@@ -46,6 +46,7 @@ impl Type {
         Type::from_primitive(tag).ok_or_else(|| DataError::UnknownType(tag))
     }
 
+    #[allow(clippy::wrong_self_convention)]
     pub fn to_tag(self) -> u8 {
         self as u8
     }