Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make clippy a mandatory test once more #1375

Merged
merged 24 commits into from
Jan 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
cc0b065
Run clippy on tests and non-default features
mulkieran Dec 18, 2018
c347d48
Use clippy scope for allowed lints
mulkieran Dec 21, 2018
4b4217d
Remove explicit unit return types
mulkieran Dec 21, 2018
f286103
Use _'s to make hex value more readable
mulkieran Dec 21, 2018
dcfb5d7
Use cloned() instead of map()
mulkieran Dec 21, 2018
ec4dcac
Allow a non-Self return value from new
mulkieran Dec 21, 2018
ff2fe25
Simplify a boolean expression
mulkieran Dec 21, 2018
1a98cb6
Use empty check instead of length check
mulkieran Dec 21, 2018
e171a68
Use [] operator to get item in map when the result is unwrapped
mulkieran Dec 21, 2018
b4ad126
Do not use & in pattern matching
mulkieran Dec 21, 2018
22df954
Use u64::from for a safe cast
mulkieran Dec 21, 2018
9710b89
Use & on expression instead of "ref" on identifier
mulkieran Dec 21, 2018
d719f3d
Do not clone a copy value
mulkieran Dec 21, 2018
dea302b
Remove an identify conversion
mulkieran Dec 21, 2018
32994d3
Avoid using function expression as argument to unwrap_or
mulkieran Dec 21, 2018
093c197
Remove a multiplication by the multiplicative identity
mulkieran Dec 21, 2018
eaace9b
Pass DeviceLimits as reference
mulkieran Dec 21, 2018
037d0dc
Omit redundant closure
mulkieran Dec 21, 2018
d9fcfbb
Allow type complexity for a test function
mulkieran Dec 21, 2018
3e49ac2
Allow a test function to contain a lot of assertions
mulkieran Dec 21, 2018
df81431
Get rid of a complicated assertion
mulkieran Dec 21, 2018
ceda928
Get rid of a complicated closure in an assert
mulkieran Dec 21, 2018
878e5cb
Use read_exact or write_all instead of read or write
mulkieran Dec 21, 2018
aad4451
Do not allow clippy failure in Travis tests
mulkieran Dec 21, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions src/bin/stratisd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion src/engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/engine/sim_engine/blockdev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
2 changes: 1 addition & 1 deletion src/engine/sim_engine/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
1 change: 1 addition & 0 deletions src/engine/sim_engine/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ pub struct SimPool {
}

impl SimPool {
#[allow(clippy::new_ret_no_self)]
pub fn new(
rdm: &Rc<RefCell<Randomizer>>,
paths: &[&Path],
Expand Down
4 changes: 2 additions & 2 deletions src/engine/sim_engine/randomization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
}
32 changes: 16 additions & 16 deletions src/engine/strat_engine/backstore/backstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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);
Expand Down Expand Up @@ -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,
);
}
Expand All @@ -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();
Expand Down Expand Up @@ -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.
Expand All @@ -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);
Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);
}
}
1 change: 1 addition & 0 deletions src/engine/strat_engine/backstore/blockdev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading