From ec21e12d8adccb277d155c30bc65aea776f312df Mon Sep 17 00:00:00 2001
From: Eric Huss <eric@huss.org>
Date: Mon, 20 May 2019 12:36:21 -0700
Subject: [PATCH 1/2] Some clippy fixes.
---
src/cargo/core/compiler/context/mod.rs | 2 +-
src/cargo/core/compiler/job_queue.rs | 2 +-
src/cargo/core/compiler/unit.rs | 2 +-
src/cargo/core/resolver/mod.rs | 2 +-
src/cargo/core/resolver/resolve.rs | 4 ++--
src/cargo/lib.rs | 11 +++++++++--
src/cargo/ops/cargo_package.rs | 4 ++--
src/cargo/ops/common_for_install_and_uninstall.rs | 5 +++--
src/cargo/sources/path.rs | 9 +++------
src/cargo/sources/registry/index.rs | 6 +++---
src/cargo/util/config.rs | 4 ++--
tests/testsuite/freshness.rs | 6 ++++--
tests/testsuite/support/mod.rs | 14 ++++++++++----
13 files changed, 42 insertions(+), 29 deletions(-)
diff --git a/src/cargo/core/compiler/context/mod.rs b/src/cargo/core/compiler/context/mod.rs
index 61b05a0d782..76265f0d8fd 100644
--- a/src/cargo/core/compiler/context/mod.rs
+++ b/src/cargo/core/compiler/context/mod.rs
@@ -219,7 +219,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
self.compilation
.rustdocflags
.entry(unit.pkg.package_id())
- .or_insert(rustdocflags.to_vec());
+ .or_insert_with(|| rustdocflags.to_vec());
}
super::output_depinfo(&mut self, unit)?;
diff --git a/src/cargo/core/compiler/job_queue.rs b/src/cargo/core/compiler/job_queue.rs
index 693dc01b603..bc43f312f7a 100644
--- a/src/cargo/core/compiler/job_queue.rs
+++ b/src/cargo/core/compiler/job_queue.rs
@@ -216,7 +216,7 @@ impl<'a, 'cfg> JobQueue<'a, 'cfg> {
self.queue.queue(*unit, job, queue_deps);
*self.counts.entry(unit.pkg.package_id()).or_insert(0) += 1;
- return Ok(());
+ Ok(())
}
/// Executes all jobs necessary to build the dependency graph.
diff --git a/src/cargo/core/compiler/unit.rs b/src/cargo/core/compiler/unit.rs
index bf7d6f0871c..0451fc42a9e 100644
--- a/src/cargo/core/compiler/unit.rs
+++ b/src/cargo/core/compiler/unit.rs
@@ -166,6 +166,6 @@ impl<'a> UnitInterner<'a> {
}
me.cache.insert(Box::new(item.clone()));
let item = me.cache.get(item).unwrap();
- return unsafe { &*(&**item as *const UnitInner<'a>) };
+ unsafe { &*(&**item as *const UnitInner<'a>) }
}
}
diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs
index c410b7accca..4a21f42a58a 100644
--- a/src/cargo/core/resolver/mod.rs
+++ b/src/cargo/core/resolver/mod.rs
@@ -657,7 +657,7 @@ fn activate(
let candidate = match registry.replacement_summary(candidate_pid) {
Some(replace) => {
- if cx.flag_activated(&replace, &method)? && activated {
+ if cx.flag_activated(replace, &method)? && activated {
return Ok(None);
}
trace!(
diff --git a/src/cargo/core/resolver/resolve.rs b/src/cargo/core/resolver/resolve.rs
index 6a953d00ad8..1684dc7fdff 100644
--- a/src/cargo/core/resolver/resolve.rs
+++ b/src/cargo/core/resolver/resolve.rs
@@ -55,7 +55,7 @@ impl Resolve {
.find(|d| d.kind() == Kind::Normal)
.and_then(|d| {
if d.is_public() {
- Some(dep_package.clone())
+ Some(*dep_package)
} else {
None
}
@@ -64,7 +64,7 @@ impl Resolve {
})
.collect::<HashSet<PackageId>>();
- (p.clone(), public_deps)
+ (*p, public_deps)
})
.collect();
diff --git a/src/cargo/lib.rs b/src/cargo/lib.rs
index 90f0e30750c..16e67a45836 100644
--- a/src/cargo/lib.rs
+++ b/src/cargo/lib.rs
@@ -1,8 +1,8 @@
#![cfg_attr(test, deny(warnings))]
-#![warn(rust_2018_idioms)]
// While we're getting used to 2018:
+#![warn(rust_2018_idioms)]
// Clippy isn't enforced by CI (@alexcrichton isn't a fan).
-#![allow(clippy::boxed_local)] // bug rust-lang-nursery/rust-clippy#1123
+#![allow(clippy::blacklisted_name)] // frequently used in tests
#![allow(clippy::cyclomatic_complexity)] // large project
#![allow(clippy::derive_hash_xor_eq)] // there's an intentional incoherence
#![allow(clippy::explicit_into_iter_loop)] // explicit loops are clearer
@@ -10,6 +10,7 @@
#![allow(clippy::identity_op)] // used for vertical alignment
#![allow(clippy::implicit_hasher)] // large project
#![allow(clippy::large_enum_variant)] // large project
+#![allow(clippy::new_without_default)] // explicit is maybe clearer
#![allow(clippy::redundant_closure)] // closures can be less verbose
#![allow(clippy::redundant_closure_call)] // closures over try catch blocks
#![allow(clippy::too_many_arguments)] // large project
@@ -17,6 +18,12 @@
#![allow(clippy::wrong_self_convention)] // perhaps `Rc` should be special-cased in Clippy?
#![warn(clippy::needless_borrow)]
#![warn(clippy::redundant_clone)]
+// Unit is now interned, and would probably be better as pass-by-copy, but
+// doing so causes a lot of & and * shenanigans that makes the code arguably
+// less clear and harder to read.
+#![allow(clippy::trivially_copy_pass_by_ref)]
+// exhaustively destructuring ensures future fields are handled
+#![allow(clippy::unneeded_field_pattern)]
use std::fmt;
diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs
index fc39b51796a..609f9bd69de 100644
--- a/src/cargo/ops/cargo_package.rs
+++ b/src/cargo/ops/cargo_package.rs
@@ -440,7 +440,7 @@ fn tar(
}
if pkg.include_lockfile() {
- let new_lock = build_lock(&ws)?;
+ let new_lock = build_lock(ws)?;
config
.shell()
@@ -609,7 +609,7 @@ fn run_verify(ws: &Workspace<'_>, tar: &FileLock, opts: &PackageOpts<'_>) -> Car
{
// FIXME: Turn this on at some point in the future
//Some(vec!["-D exported_private_dependencies".to_string()])
- None
+ Some(vec![])
} else {
None
};
diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs
index 8502d190649..12e435864d0 100644
--- a/src/cargo/ops/common_for_install_and_uninstall.rs
+++ b/src/cargo/ops/common_for_install_and_uninstall.rs
@@ -211,7 +211,7 @@ impl InstallTracker {
// `cargo install --path ...` is always rebuilt.
return Ok((Freshness::Dirty, duplicates));
}
- if matching_duplicates.iter().all(|dupe_pkg_id| {
+ let is_up_to_date = |dupe_pkg_id| {
let info = self
.v2
.installs
@@ -229,7 +229,8 @@ impl InstallTracker {
&& dupe_pkg_id.source_id() == source_id
&& precise_equal
&& info.is_up_to_date(opts, target, &exes)
- }) {
+ };
+ if matching_duplicates.iter().all(is_up_to_date) {
Ok((Freshness::Fresh, duplicates))
} else {
Ok((Freshness::Dirty, duplicates))
diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs
index 548363e0da5..571a0a2ce18 100644
--- a/src/cargo/sources/path.rs
+++ b/src/cargo/sources/path.rs
@@ -145,7 +145,7 @@ impl<'cfg> PathSource<'cfg> {
.exclude()
.iter()
.chain(pkg.manifest().include().iter())
- .any(|p| p.starts_with("!"));
+ .any(|p| p.starts_with('!'));
// Don't warn about glob mismatch if it doesn't parse.
let glob_is_valid = glob_exclude.is_ok() && glob_include.is_ok() && !has_negate;
let glob_exclude = glob_exclude.unwrap_or_else(|_| Vec::new());
@@ -479,12 +479,9 @@ impl<'cfg> PathSource<'cfg> {
if name.map(|s| s.starts_with('.')) == Some(true) {
continue;
}
- if is_root {
+ if is_root && name == Some("target") {
// Skip Cargo artifacts.
- match name {
- Some("target") => continue,
- _ => {}
- }
+ continue;
}
PathSource::walk(&path, ret, false, filter)?;
}
diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs
index a4614b17d76..67116074cb7 100644
--- a/src/cargo/sources/registry/index.rs
+++ b/src/cargo/sources/registry/index.rs
@@ -268,7 +268,7 @@ impl<'cfg> RegistryIndex<'cfg> {
where
'a: 'b,
{
- let source_id = self.source_id.clone();
+ let source_id = self.source_id;
// First up actually parse what summaries we have available. If Cargo
// has run previously this will parse a Cargo-specific cache file rather
@@ -337,7 +337,7 @@ impl<'cfg> RegistryIndex<'cfg> {
for path in UncanonicalizedIter::new(&raw_path).take(1024) {
let summaries = Summaries::parse(
index_version.as_ref().map(|s| &**s),
- &root,
+ root,
&cache_root,
path.as_ref(),
self.source_id,
@@ -671,7 +671,7 @@ impl<'a> SummariesCache<'a> {
contents.extend_from_slice(data);
contents.push(0);
}
- return contents;
+ contents
}
}
diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs
index 2bea77525bc..54ee81704e8 100644
--- a/src/cargo/util/config.rs
+++ b/src/cargo/util/config.rs
@@ -704,7 +704,7 @@ impl Config {
fn resolve_registry_index(&self, index: Value<String>) -> CargoResult<Url> {
let base = index
.definition
- .root(&self)
+ .root(self)
.join("truncated-by-url_with_base");
// Parse val to check it is a URL, not a relative path without a protocol.
let _parsed = index.val.to_url()?;
@@ -857,7 +857,7 @@ impl Config {
`acquire_package_cache_lock` before we got to this stack frame",
);
assert!(ret.starts_with(self.home_path.as_path_unlocked()));
- return ret;
+ ret
}
/// Acquires an exclusive lock on the global "package cache"
diff --git a/tests/testsuite/freshness.rs b/tests/testsuite/freshness.rs
index 8ef12c7b9c5..da7686befff 100644
--- a/tests/testsuite/freshness.rs
+++ b/tests/testsuite/freshness.rs
@@ -1,5 +1,6 @@
use filetime::FileTime;
use std::fs::{self, File, OpenOptions};
+use std::io;
use std::io::prelude::*;
use std::net::TcpListener;
use std::path::{Path, PathBuf};
@@ -1269,10 +1270,11 @@ fn fingerprint_cleaner(mut dir: PathBuf, timestamp: filetime::FileTime) {
for fing in fs::read_dir(&dir).unwrap() {
let fing = fing.unwrap();
- if fs::read_dir(fing.path()).unwrap().all(|f| {
+ let outdated = |f: io::Result<fs::DirEntry>| {
filetime::FileTime::from_last_modification_time(&f.unwrap().metadata().unwrap())
<= timestamp
- }) {
+ };
+ if fs::read_dir(fing.path()).unwrap().all(outdated) {
fs::remove_dir_all(fing.path()).unwrap();
println!("remove: {:?}", fing.path());
// a real cleaner would remove the big files in deps and build as well
diff --git a/tests/testsuite/support/mod.rs b/tests/testsuite/support/mod.rs
index 74b9defc87e..553bd48a806 100644
--- a/tests/testsuite/support/mod.rs
+++ b/tests/testsuite/support/mod.rs
@@ -884,8 +884,14 @@ impl Execs {
panic!("`.stream()` is for local debugging")
}
process.exec_with_streaming(
- &mut |out| Ok(println!("{}", out)),
- &mut |err| Ok(eprintln!("{}", err)),
+ &mut |out| {
+ println!("{}", out);
+ Ok(())
+ },
+ &mut |err| {
+ eprintln!("{}", err);
+ Ok(())
+ },
true,
)
} else {
@@ -1166,7 +1172,7 @@ impl Execs {
let e = out.lines();
let mut diffs = self.diff_lines(a.clone(), e.clone(), true);
- while let Some(..) = a.next() {
+ while a.next().is_some() {
let a = self.diff_lines(a.clone(), e.clone(), true);
if a.len() < diffs.len() {
diffs = a;
@@ -1214,7 +1220,7 @@ impl Execs {
let e = out.lines();
let mut diffs = self.diff_lines(a.clone(), e.clone(), true);
- while let Some(..) = a.next() {
+ while a.next().is_some() {
let a = self.diff_lines(a.clone(), e.clone(), true);
if a.len() < diffs.len() {
diffs = a;
From 6b07c8da63c0896cc31fa6cb8b47a71fe29c88e2 Mon Sep 17 00:00:00 2001
From: Eric Huss <eric@huss.org>
Date: Mon, 20 May 2019 12:36:59 -0700
Subject: [PATCH 2/2] cargo fmt
---
src/cargo/core/compiler/custom_build.rs | 3 ++-
src/cargo/ops/cargo_generate_lockfile.rs | 11 +++++++++--
tests/testsuite/build.rs | 12 ++++++++----
tests/testsuite/registry.rs | 1 -
tests/testsuite/rustc.rs | 3 ++-
tests/testsuite/update.rs | 1 -
6 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs
index 15f824ce1ea..327dc9358ab 100644
--- a/src/cargo/core/compiler/custom_build.rs
+++ b/src/cargo/core/compiler/custom_build.rs
@@ -110,7 +110,8 @@ fn emit_build_output(state: &JobState<'_>, output: &BuildOutput, package_id: Pac
linked_paths: &library_paths,
cfgs: &output.cfgs,
env: &output.env,
- }.to_json_string();
+ }
+ .to_json_string();
state.stdout(&msg);
}
diff --git a/src/cargo/ops/cargo_generate_lockfile.rs b/src/cargo/ops/cargo_generate_lockfile.rs
index 7034ea099e2..16495a41cda 100644
--- a/src/cargo/ops/cargo_generate_lockfile.rs
+++ b/src/cargo/ops/cargo_generate_lockfile.rs
@@ -54,8 +54,15 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes
// by precise package update later.
Some(_) => {
let mut registry = PackageRegistry::new(opts.config)?;
- ops::resolve_with_previous(&mut registry, ws, Method::Everything,
- None, None, &[], true)?
+ ops::resolve_with_previous(
+ &mut registry,
+ ws,
+ Method::Everything,
+ None,
+ None,
+ &[],
+ true,
+ )?
}
}
}
diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs
index c87e5ac4683..d8d3c06303b 100644
--- a/tests/testsuite/build.rs
+++ b/tests/testsuite/build.rs
@@ -4573,11 +4573,13 @@ fn pipelining_works() {
foo.cargo("build")
.env("CARGO_BUILD_PIPELINING", "true")
.with_stdout("")
- .with_stderr("\
+ .with_stderr(
+ "\
[COMPILING] [..]
[COMPILING] [..]
[FINISHED] [..]
-")
+",
+ )
.run();
}
@@ -4628,13 +4630,15 @@ fn forward_rustc_output() {
foo.cargo("build")
.with_stdout("a\nb\n{}")
- .with_stderr("\
+ .with_stderr(
+ "\
[COMPILING] [..]
[COMPILING] [..]
c
d
{a
[FINISHED] [..]
-")
+",
+ )
.run();
}
diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs
index a3aae80b606..9b5cd728529 100644
--- a/tests/testsuite/registry.rs
+++ b/tests/testsuite/registry.rs
@@ -2008,7 +2008,6 @@ fn readonly_registry_still_works() {
// make sure we un-readonly the files afterwards so "cargo clean" can remove them (#6934)
chmod_readonly(&paths::home(), false);
-
fn chmod_readonly(path: &Path, readonly: bool) {
for entry in t!(path.read_dir()) {
let entry = t!(entry);
diff --git a/tests/testsuite/rustc.rs b/tests/testsuite/rustc.rs
index 7674afdb62d..ff909908cca 100644
--- a/tests/testsuite/rustc.rs
+++ b/tests/testsuite/rustc.rs
@@ -113,7 +113,8 @@ fn build_with_args_to_one_of_multiple_binaries() {
-C debuginfo=2 -C debug-assertions [..]`
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
",
- ).run();
+ )
+ .run();
}
#[test]
diff --git a/tests/testsuite/update.rs b/tests/testsuite/update.rs
index 28246f3e982..3c7487dab20 100644
--- a/tests/testsuite/update.rs
+++ b/tests/testsuite/update.rs
@@ -556,7 +556,6 @@ fn update_precise_first_run() {
",
)
.run();
-
}
#[test]