Merge pull request #1375 from mulkieran/master-clippy-run

Make clippy a mandatory test once more

.travis.yml

@@ -13,9 +13,6 @@ addons:
 language: rust
 
 matrix:
-    allow_failures:
-        # Temporarily allow failure until after next release.
-        - env: TASK=clippy TARGET=x86_64-unknown-linux-gnu
     include:
         # Verify formatting of Rust code
         - rust: stable

Makefile

@@ -59,7 +59,7 @@ stratisd.8.gz: stratisd.8
 	gzip --stdout docs/stratisd.8 > docs/stratisd.8.gz
 
 clippy:
-	RUSTFLAGS='-D warnings' cargo clippy
+	cargo clippy --all-targets --all-features -- -D warnings
 
 uml-graphs: ${HOME}/.cargo/bin/cargo-script
 	PATH=${HOME}/.cargo/bin:${PATH} cargo script scripts/uml_graphs.rs

src/bin/stratisd.rs

@@ -2,8 +2,7 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-#![cfg_attr(not(feature = "clippy"), allow(unknown_lints))]
-#![allow(doc_markdown)]
+#![allow(clippy::doc_markdown)]
 
 extern crate devicemapper;
 extern crate libstratis;
@@ -62,7 +61,7 @@ const DEFAULT_STATE_DUMP_MINUTES: i64 = 10;
 const DEFAULT_LOG_HOLD_MINUTES: i64 = 30;
 
 /// If writing a program error to stderr fails, panic.
-fn print_err(err: &StratisError) -> () {
+fn print_err(err: &StratisError) {
     eprintln!("{}", err);
 }
 

src/engine/mod.rs

@@ -30,7 +30,7 @@ pub use self::types::RenameAction;
 mod macros;
 
 mod devlinks;
-#[allow(module_inception)]
+#[allow(clippy::module_inception)]
 mod engine;
 mod event;
 mod sim_engine;

src/engine/sim_engine/blockdev.rs

@@ -63,6 +63,7 @@ impl BlockDev for SimDev {
 
 impl SimDev {
     /// Generates a new device from any devnode.
+    #[allow(clippy::new_ret_no_self)]
     pub fn new(rdm: Rc<RefCell<Randomizer>>, devnode: &Path) -> (Uuid, SimDev) {
         (
             Uuid::new_v4(),

src/engine/sim_engine/engine.rs

@@ -44,7 +44,7 @@ impl Engine for SimEngine {
         }
 
         let device_set: HashSet<_, RandomState> = HashSet::from_iter(blockdev_paths);
-        let devices = device_set.into_iter().map(|x| *x).collect::<Vec<&Path>>();
+        let devices = device_set.into_iter().cloned().collect::<Vec<&Path>>();
 
         let (pool_uuid, pool) = SimPool::new(&Rc::clone(&self.rdm), &devices, redundancy);
 

src/engine/sim_engine/pool.rs

@@ -41,6 +41,7 @@ pub struct SimPool {
 }
 
 impl SimPool {
+    #[allow(clippy::new_ret_no_self)]
     pub fn new(
         rdm: &Rc<RefCell<Randomizer>>,
         paths: &[&Path],

src/engine/sim_engine/randomization.rs

@@ -61,8 +61,8 @@ mod tests {
                 .set_probability(denominator)
                 .throw_die();
             prop_assert!(denominator > 1
-                         || (denominator != 0 && result == false)
-                         || (denominator == 0 && result == true));
+                         || (denominator != 0 && !result)
+                         || (denominator == 0 && result));
         }
     }
 }

src/engine/strat_engine/backstore/backstore.rs

@@ -590,7 +590,7 @@ mod tests {
     /// * backstore's data tier allocated is equal to the size of the cap device
     /// * backstore's next index is always less than the size of the cap
     ///   device
-    fn invariant(backstore: &Backstore) -> () {
+    fn invariant(backstore: &Backstore) {
         assert!(
             (backstore.cache_tier.is_none() && backstore.cache.is_none())
                 || (backstore.cache_tier.is_some()
@@ -614,7 +614,7 @@ mod tests {
     /// Nonetheless, because nothing is written or read, cache usage ought
     /// to be 0. Adding some more cachedevs exercises different code path
     /// from adding initial cachedevs.
-    fn test_add_cache_devs(paths: &[&Path]) -> () {
+    fn test_add_cache_devs(paths: &[&Path]) {
         assert!(paths.len() > 3);
 
         let meta_size = Sectors(IEC::Mi);
@@ -687,23 +687,23 @@ mod tests {
     #[test]
     pub fn loop_test_add_cache_devs() {
         loopbacked::test_with_spec(
-            loopbacked::DeviceLimits::Range(4, 5, None),
+            &loopbacked::DeviceLimits::Range(4, 5, None),
             test_add_cache_devs,
         );
     }
 
     #[test]
     pub fn real_test_add_cache_devs() {
         real::test_with_spec(
-            real::DeviceLimits::AtLeast(4, None, None),
+            &real::DeviceLimits::AtLeast(4, None, None),
             test_add_cache_devs,
         );
     }
 
     #[test]
     pub fn travis_test_add_cache_devs() {
         loopbacked::test_with_spec(
-            loopbacked::DeviceLimits::Range(4, 5, None),
+            &loopbacked::DeviceLimits::Range(4, 5, None),
             test_add_cache_devs,
         );
     }
@@ -713,8 +713,8 @@ mod tests {
     /// bigger than the reqested amount.
     /// Request an impossibly large amount.
     /// Verify that the backstore is now all used up.
-    fn test_request(paths: &[&Path]) -> () {
-        assert!(paths.len() > 0);
+    fn test_request(paths: &[&Path]) {
+        assert!(!paths.is_empty());
 
         let pool_uuid = Uuid::new_v4();
         let mut backstore = Backstore::initialize(pool_uuid, paths, MIN_MDA_SECTORS).unwrap();
@@ -752,17 +752,17 @@ mod tests {
 
     #[test]
     pub fn loop_test_request() {
-        loopbacked::test_with_spec(loopbacked::DeviceLimits::Range(1, 3, None), test_request);
+        loopbacked::test_with_spec(&loopbacked::DeviceLimits::Range(1, 3, None), test_request);
     }
 
     #[test]
     pub fn real_test_request() {
-        real::test_with_spec(real::DeviceLimits::AtLeast(1, None, None), test_request);
+        real::test_with_spec(&real::DeviceLimits::AtLeast(1, None, None), test_request);
     }
 
     #[test]
     pub fn travis_test_request() {
-        loopbacked::test_with_spec(loopbacked::DeviceLimits::Range(1, 3, None), test_request);
+        loopbacked::test_with_spec(&loopbacked::DeviceLimits::Range(1, 3, None), test_request);
     }
 
     /// Create a backstore with a cache.
@@ -772,7 +772,7 @@ mod tests {
     /// Setup the same backstore again.
     /// Verify blockdev metadata again.
     /// Destroy all.
-    fn test_setup(paths: &[&Path]) -> () {
+    fn test_setup(paths: &[&Path]) {
         assert!(paths.len() > 1);
 
         let (paths1, paths2) = paths.split_at(paths.len() / 2);
@@ -798,7 +798,7 @@ mod tests {
 
         cmd::udev_settle().unwrap();
         let map = find_all().unwrap();
-        let map = map.get(&pool_uuid).unwrap();
+        let map = &map[&pool_uuid];
         let mut backstore = Backstore::setup(pool_uuid, &backstore_save, &map, None).unwrap();
         invariant(&backstore);
 
@@ -810,7 +810,7 @@ mod tests {
 
         cmd::udev_settle().unwrap();
         let map = find_all().unwrap();
-        let map = map.get(&pool_uuid).unwrap();
+        let map = &map[&pool_uuid];
         let mut backstore = Backstore::setup(pool_uuid, &backstore_save, &map, None).unwrap();
         invariant(&backstore);
 
@@ -823,16 +823,16 @@ mod tests {
 
     #[test]
     pub fn loop_test_setup() {
-        loopbacked::test_with_spec(loopbacked::DeviceLimits::Range(2, 3, None), test_setup);
+        loopbacked::test_with_spec(&loopbacked::DeviceLimits::Range(2, 3, None), test_setup);
     }
 
     #[test]
     pub fn real_test_setup() {
-        real::test_with_spec(real::DeviceLimits::AtLeast(2, None, None), test_setup);
+        real::test_with_spec(&real::DeviceLimits::AtLeast(2, None, None), test_setup);
     }
 
     #[test]
     pub fn travis_test_setup() {
-        loopbacked::test_with_spec(loopbacked::DeviceLimits::Range(2, 3, None), test_setup);
+        loopbacked::test_with_spec(&loopbacked::DeviceLimits::Range(2, 3, None), test_setup);
     }
 }

src/engine/strat_engine/backstore/blockdev.rs

@@ -49,6 +49,7 @@ impl StratBlockDev {
     /// on the device is simply invisible to the blockdev. Consequently, it
     /// is invisible to the engine, and is not part of the total size value
     /// reported on the D-Bus.
+    #[allow(clippy::new_ret_no_self)]
     pub fn new(
         dev: Device,
         devnode: PathBuf,