Skip to content

Commit

Permalink
Auto merge of #7574 - igor-makarov:clippy-fixes, r=Eh2406
Browse files Browse the repository at this point in the history
Fix all Clippy suggestions (but not add it to CI πŸ™ƒ)

This PR adds a `clippy` job to the Azure Pipelines CI.

In addition to adding the job, I made sure to fix all the lints in all the packages.

I'm new to Rust, so please check my code modifications extra carefully πŸ˜‚
  • Loading branch information
bors committed Nov 15, 2019
2 parents d0a41be + 1432651 commit 3738e1d
Show file tree
Hide file tree
Showing 13 changed files with 34 additions and 20 deletions.
2 changes: 1 addition & 1 deletion crates/cargo-test-macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub fn cargo_test(attr: TokenStream, item: TokenStream) -> TokenStream {
))));
}

return ret;
ret
}

fn contains_ident(t: &TokenStream, ident: &str) -> bool {
Expand Down
7 changes: 5 additions & 2 deletions crates/cargo-test-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ dependency.
*/

#![allow(clippy::needless_doctest_main)] // according to @ehuss this lint is fussy
#![allow(clippy::inefficient_to_string)] // this causes suggestions that result in `(*s).to_string()`

use std::env;
use std::ffi::OsStr;
use std::fmt;
Expand Down Expand Up @@ -1141,9 +1144,9 @@ impl Execs {

// Do the template replacements on the expected string.
let matcher = match &self.process_builder {
None => matcher.to_string(),
None => matcher,
Some(p) => match p.get_cwd() {
None => matcher.to_string(),
None => matcher,
Some(cwd) => replace_path(&matcher, cwd, "[CWD]"),
},
};
Expand Down
3 changes: 3 additions & 0 deletions crates/resolver-tests/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
#![allow(clippy::many_single_char_names)]
#![allow(clippy::needless_range_loop)] // false positives

use std::cell::RefCell;
use std::cmp::PartialEq;
use std::cmp::{max, min};
Expand Down
2 changes: 1 addition & 1 deletion crates/resolver-tests/tests/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ fn pub_fail() {
pkg!(("e", "0.0.6") => [dep_req_kind("a", "<= 0.0.4", Kind::Normal, true),]),
pkg!(("kB", "0.0.3") => [dep_req("a", ">= 0.0.5"),dep("e"),]),
];
let reg = registry(input.clone());
let reg = registry(input);
assert!(resolve_and_validated(vec![dep("kB")], &reg, None).is_err());
}

Expand Down
1 change: 0 additions & 1 deletion src/bin/cargo/main.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#![warn(rust_2018_idioms)] // while we're getting used to 2018
#![allow(clippy::too_many_arguments)] // large project
#![allow(clippy::redundant_closure)] // there's a false positive
#![warn(clippy::needless_borrow)]
#![warn(clippy::redundant_clone)]
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/core/profiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ impl ProfileMaker {
Some(ref toml) => toml,
None => return Ok(()),
};
let overrides = match toml.package.as_ref().or(toml.overrides.as_ref()) {
let overrides = match toml.package.as_ref().or_else(|| toml.overrides.as_ref()) {
Some(overrides) => overrides,
None => return Ok(()),
};
Expand Down Expand Up @@ -612,7 +612,7 @@ fn merge_toml_overrides(
merge_profile(profile, build_override);
}
}
if let Some(overrides) = toml.package.as_ref().or(toml.overrides.as_ref()) {
if let Some(overrides) = toml.package.as_ref().or_else(|| toml.overrides.as_ref()) {
if !is_member {
if let Some(all) = overrides.get(&ProfilePackageSpec::All) {
merge_profile(profile, all);
Expand Down
11 changes: 6 additions & 5 deletions src/cargo/core/resolver/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -564,19 +564,20 @@ pub struct EncodeState<'a> {

impl<'a> EncodeState<'a> {
pub fn new(resolve: &'a Resolve) -> EncodeState<'a> {
let mut counts = None;
if *resolve.version() == ResolveVersion::V2 {
let counts = if *resolve.version() == ResolveVersion::V2 {
let mut map = HashMap::new();
for id in resolve.iter() {
let slot = map
.entry(id.name())
.or_insert(HashMap::new())
.or_insert_with(HashMap::new)
.entry(id.version())
.or_insert(0);
*slot += 1;
}
counts = Some(map);
}
Some(map)
} else {
None
};
EncodeState { counts }
}
}
Expand Down
1 change: 1 addition & 0 deletions src/cargo/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#![allow(clippy::type_complexity)] // there's an exceptionally complex type
#![allow(clippy::wrong_self_convention)] // perhaps `Rc` should be special-cased in Clippy?
#![allow(clippy::write_with_newline)] // too pedantic
#![allow(clippy::inefficient_to_string)] // this causes suggestions that result in `(*s).to_string()`
#![warn(clippy::needless_borrow)]
#![warn(clippy::redundant_clone)]
// Unit is now interned, and would probably be better as pass-by-copy, but
Expand Down
11 changes: 8 additions & 3 deletions src/cargo/ops/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,13 +371,18 @@ fn registry(
let mut src = RegistrySource::remote(sid, &HashSet::new(), config);
// Only update the index if the config is not available or `force` is set.
let cfg = src.config();
let cfg = if force_update || cfg.is_err() {
let mut updated_cfg = || {
src.update()
.chain_err(|| format!("failed to update {}", sid))?;
cfg.or_else(|_| src.config())?
src.config()
};

let cfg = if force_update {
updated_cfg()?
} else {
cfg.unwrap()
cfg.or_else(|_| updated_cfg())?
};

cfg.and_then(|cfg| cfg.api)
.ok_or_else(|| format_err!("{} does not support API commands", sid))?
};
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/util/config/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ impl<'de, 'config> de::MapAccess<'de> for ConfigMapAccess<'config> {
};
let result = seed.deserialize(name.into_deserializer()).map(Some);
self.next = Some(key);
return result;
result
}
None => Ok(None),
}
Expand All @@ -273,7 +273,7 @@ impl<'de, 'config> de::MapAccess<'de> for ConfigMapAccess<'config> {
key: self.de.key.clone(),
});
self.de.key.pop();
return result;
result
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/cargo/util/config/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ impl ConfigKey {
for part in key.split('.') {
cfg.push(part);
}
return cfg;
cfg
}

/// Pushes a new sub-key on this `ConfigKey`. This sub-key should be
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/util/network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ fn with_retry_repeats_the_call_then_works() {
let config = Config::default().unwrap();
*config.shell() = Shell::from_write(Box::new(Vec::new()));
let result = with_retry(&config, || results.pop().unwrap());
assert_eq!(result.unwrap(), ())
assert!(result.is_ok())
}

#[test]
Expand All @@ -135,7 +135,7 @@ fn with_retry_finds_nested_spurious_errors() {
let config = Config::default().unwrap();
*config.shell() = Shell::from_write(Box::new(Vec::new()));
let result = with_retry(&config, || results.pop().unwrap());
assert_eq!(result.unwrap(), ())
assert!(result.is_ok())
}

#[test]
Expand Down
2 changes: 2 additions & 0 deletions tests/testsuite/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#![allow(clippy::blacklisted_name)]
#![allow(clippy::explicit_iter_loop)]
#![allow(clippy::redundant_closure)]
#![allow(clippy::block_in_if_condition_stmt)] // clippy doesn't agree with rustfmt πŸ˜‚
#![allow(clippy::inefficient_to_string)] // this causes suggestions that result in `(*s).to_string()`
#![warn(clippy::needless_borrow)]
#![warn(clippy::redundant_clone)]

Expand Down

0 comments on commit 3738e1d

Please sign in to comment.