Auto merge of #6787 - ehuss:code-cleanup, r=alexcrichton

Code cleanup

I think these changes should not be controversial, at least it looks cleaner to me. Split up into separate commits for different things.

src/bin/cargo/cli.rs

@@ -47,7 +47,7 @@ Run with 'cargo -Z [FLAG] [SUBCOMMAND]'"
         return Ok(());
     }
 
-    if let Some(ref code) = args.value_of("explain") {
+    if let Some(code) = args.value_of("explain") {
         let mut procss = config.rustc(None)?.process();
         procss.arg("--explain").arg(code).exec()?;
         return Ok(());

src/bin/cargo/main.rs

@@ -1,6 +1,8 @@
 #![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)]
 
 use std::collections::BTreeSet;
 use std::env;

src/cargo/core/compiler/build_plan.rs

@@ -113,7 +113,7 @@ impl BuildPlan {
         let id = self.plan.invocations.len();
         self.invocation_map.insert(unit.buildkey(), id);
         let deps = cx
-            .dep_targets(&unit)
+            .dep_targets(unit)
             .iter()
             .map(|dep| self.invocation_map[&dep.buildkey()])
             .collect();

src/cargo/core/compiler/compilation.rs

@@ -120,7 +120,7 @@ impl<'cfg> Compilation<'cfg> {
             rustc_process: rustc,
             host: bcx.host_triple().to_string(),
             target: bcx.target_triple().to_string(),
-            target_runner: target_runner(&bcx)?,
+            target_runner: target_runner(bcx)?,
         })
     }
 

src/cargo/core/compiler/context/compilation_files.rs

@@ -491,7 +491,7 @@ fn compute_metadata<'a, 'cfg>(
     // settings like debuginfo and whatnot.
     unit.profile.hash(&mut hasher);
     unit.mode.hash(&mut hasher);
-    if let Some(ref args) = bcx.extra_args_for(unit) {
+    if let Some(args) = bcx.extra_args_for(unit) {
         args.hash(&mut hasher);
     }
 

src/cargo/core/compiler/context/mod.rs

@@ -474,11 +474,11 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
         for unit in keys {
             for output in self.outputs(unit)?.iter() {
                 if let Some(other_unit) = output_collisions.insert(output.path.clone(), unit) {
-                    report_collision(unit, &other_unit, &output.path)?;
+                    report_collision(unit, other_unit, &output.path)?;
                 }
                 if let Some(hardlink) = output.hardlink.as_ref() {
                     if let Some(other_unit) = output_collisions.insert(hardlink.clone(), unit) {
-                        report_collision(unit, &other_unit, hardlink)?;
+                        report_collision(unit, other_unit, hardlink)?;
                     }
                 }
                 if let Some(ref export_path) = output.export_path {
@@ -488,7 +488,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
                              {}\
                              The exported filenames should be unique.\n\
                              {}",
-                            describe_collision(unit, &other_unit, &export_path),
+                            describe_collision(unit, other_unit, export_path),
                             suggestion
                         ))?;
                     }

src/cargo/core/compiler/custom_build.rs

@@ -162,7 +162,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes
         .env(
             "TARGET",
             &match unit.kind {
-                Kind::Host => &bcx.host_triple(),
+                Kind::Host => bcx.host_triple(),
                 Kind::Target => bcx.target_triple(),
             },
         )

src/cargo/core/compiler/fingerprint.rs

@@ -682,7 +682,7 @@ fn calculate<'a, 'cfg>(
         local.extend(local_fingerprint_run_custom_build_deps(cx, unit));
         local
     } else {
-        let fingerprint = pkg_fingerprint(&cx.bcx, unit.pkg)?;
+        let fingerprint = pkg_fingerprint(cx.bcx, unit.pkg)?;
         vec![LocalFingerprint::Precalculated(fingerprint)]
     };
 
@@ -701,7 +701,7 @@ fn calculate<'a, 'cfg>(
         profile: profile_hash,
         // Note that .0 is hashed here, not .1 which is the cwd. That doesn't
         // actually affect the output artifact so there's no need to hash it.
-        path: util::hash_u64(&super::path_args(&cx.bcx, unit).0),
+        path: util::hash_u64(&super::path_args(cx.bcx, unit).0),
         features: format!("{:?}", bcx.resolve.features_sorted(unit.pkg.package_id())),
         deps,
         local,
@@ -855,7 +855,7 @@ fn build_script_local_fingerprints<'a, 'cfg>(
     let output = deps.build_script_output.clone();
     if deps.rerun_if_changed.is_empty() && deps.rerun_if_env_changed.is_empty() {
         debug!("old local fingerprints deps");
-        let s = pkg_fingerprint(&cx.bcx, unit.pkg)?;
+        let s = pkg_fingerprint(cx.bcx, unit.pkg)?;
         return Ok((vec![LocalFingerprint::Precalculated(s)], Some(output)));
     }
 

src/cargo/core/compiler/mod.rs

@@ -640,7 +640,7 @@ fn rustdoc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult
 
     add_error_format(bcx, &mut rustdoc);
 
-    if let Some(ref args) = bcx.extra_args_for(unit) {
+    if let Some(args) = bcx.extra_args_for(unit) {
         rustdoc.args(args);
     }
 
@@ -831,7 +831,7 @@ fn build_base_args<'a, 'cfg>(
         cmd.arg("-C").arg(format!("debuginfo={}", debuginfo));
     }
 
-    if let Some(ref args) = bcx.extra_args_for(unit) {
+    if let Some(args) = bcx.extra_args_for(unit) {
         cmd.args(args);
     }
 

src/cargo/core/package.rs

@@ -105,7 +105,7 @@ impl ser::Serialize for Package {
 
         SerializedPackage {
             name: &*package_id.name(),
-            version: &package_id.version(),
+            version: package_id.version(),
             id: package_id,
             license,
             license_file,
@@ -740,7 +740,7 @@ impl<'a, 'cfg> Downloads<'a, 'cfg> {
             self.set.multi.messages(|msg| {
                 let token = msg.token().expect("failed to read token");
                 let handle = &pending[&token].1;
-                if let Some(result) = msg.result_for(&handle) {
+                if let Some(result) = msg.result_for(handle) {
                     results.push((token, result));
                 } else {
                     debug!("message without a result (?)");

src/cargo/core/source/source_id.rs

@@ -368,7 +368,7 @@ impl PartialOrd for SourceId {
 
 impl Ord for SourceId {
     fn cmp(&self, other: &SourceId) -> Ordering {
-        self.inner.cmp(&other.inner)
+        self.inner.cmp(other.inner)
     }
 }
 

src/cargo/core/summary.rs

@@ -58,7 +58,7 @@ impl Summary {
                 )
             }
         }
-        let feature_map = build_feature_map(&features, &dependencies, namespaced_features)?;
+        let feature_map = build_feature_map(features, &dependencies, namespaced_features)?;
         Ok(Summary {
             inner: Rc::new(Inner {
                 package_id: pkg_id,
@@ -170,7 +170,7 @@ where
         // iteration over the list if the dependency is found in the list.
         let mut dependency_found = if namespaced {
             match dep_map.get(feature.borrow()) {
-                Some(ref dep_data) => {
+                Some(dep_data) => {
                     if !dep_data.iter().any(|d| d.is_optional()) {
                         failure::bail!(
                             "Feature `{}` includes the dependency of the same name, but this is \

src/cargo/lib.rs

@@ -14,6 +14,8 @@
 #![allow(clippy::too_many_arguments)] // large project
 #![allow(clippy::type_complexity)] // there's an exceptionally complex type
 #![allow(clippy::wrong_self_convention)] // perhaps `Rc` should be special-cased in Clippy?
+#![warn(clippy::needless_borrow)]
+#![warn(clippy::redundant_clone)]
 
 use std::fmt;
 

src/cargo/ops/cargo_compile.rs

@@ -372,12 +372,12 @@ pub fn compile_ws<'a>(
             &resolve_with_overrides,
             &packages,
             config,
-            &build_config,
+            build_config,
             profiles,
             extra_compiler_args,
         )?;
         let cx = Context::new(config, &bcx)?;
-        cx.compile(&units, export_dir.clone(), &exec)?
+        cx.compile(&units, export_dir.clone(), exec)?
     };
 
     Ok(ret)

src/cargo/ops/cargo_output_metadata.rs

@@ -42,7 +42,7 @@ fn metadata_no_deps(ws: &Workspace<'_>, _opt: &OutputMetadataOptions) -> CargoRe
         packages: ws.members().cloned().collect(),
         workspace_members: ws.members().map(|pkg| pkg.package_id()).collect(),
         resolve: None,
-        target_directory: ws.target_dir().clone().into_path_unlocked(),
+        target_directory: ws.target_dir().into_path_unlocked(),
         version: VERSION,
         workspace_root: ws.root().to_path_buf(),
     })
@@ -70,7 +70,7 @@ fn metadata_full(ws: &Workspace<'_>, opt: &OutputMetadataOptions) -> CargoResult
             resolve: (packages, resolve),
             root: ws.current_opt().map(|pkg| pkg.package_id()),
         }),
-        target_directory: ws.target_dir().clone().into_path_unlocked(),
+        target_directory: ws.target_dir().into_path_unlocked(),
         version: VERSION,
         workspace_root: ws.root().to_path_buf(),
     })

src/cargo/ops/cargo_package.rs

@@ -59,7 +59,7 @@ pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult<Option
     // dirty. This will `bail!` if dirty, unless allow_dirty. Produce json
     // info for any sha1 (HEAD revision) returned.
     let vcs_info = if !opts.allow_dirty {
-        check_repo_state(pkg, &src_files, &config, opts.allow_dirty)?
+        check_repo_state(pkg, &src_files, config, opts.allow_dirty)?
             .map(|h| json!({"git":{"sha1": h}}))
     } else {
         None
@@ -364,7 +364,7 @@ fn tar(
         }
     }
 
-    if let Some(ref json) = vcs_info {
+    if let Some(json) = vcs_info {
         let filename: PathBuf = Path::new(VCS_INFO_FILE).into();
         debug_assert!(check_filename(&filename).is_ok());
         let fnd = filename.display();

src/cargo/ops/cargo_uninstall.rs

@@ -147,7 +147,7 @@ fn uninstall_pkgid(
             installed.remove();
         }
     }
-    write_crate_list(&crate_metadata, metadata)?;
+    write_crate_list(crate_metadata, metadata)?;
     for bin in to_remove {
         config.shell().status("Removing", bin.display())?;
         paths::remove_file(bin)?;

src/cargo/ops/registry.rs

@@ -238,7 +238,7 @@ fn transmit(
         .map(|(feat, values)| {
             (
                 feat.to_string(),
-                values.iter().map(|fv| fv.to_string(&summary)).collect(),
+                values.iter().map(|fv| fv.to_string(summary)).collect(),
             )
         })
         .collect::<BTreeMap<String, Vec<String>>>();

src/cargo/util/command_prelude.rs

@@ -434,19 +434,19 @@ about this warning.";
         compile_opts: &CompileOptions<'_>,
     ) -> CargoResult<()> {
         if self.is_present_with_zero_values("example") {
-            print_available_examples(&workspace, &compile_opts)?;
+            print_available_examples(workspace, compile_opts)?;
         }
 
         if self.is_present_with_zero_values("bin") {
-            print_available_binaries(&workspace, &compile_opts)?;
+            print_available_binaries(workspace, compile_opts)?;
         }
 
         if self.is_present_with_zero_values("bench") {
-            print_available_benches(&workspace, &compile_opts)?;
+            print_available_benches(workspace, compile_opts)?;
         }
 
         if self.is_present_with_zero_values("test") {
-            print_available_tests(&workspace, &compile_opts)?;
+            print_available_tests(workspace, compile_opts)?;
         }
 
         Ok(())

src/cargo/util/dependency_queue.rs

@@ -124,7 +124,7 @@ impl<K: Hash + Eq + Clone, V> DependencyQueue<K, V> {
             results.insert(key.clone(), IN_PROGRESS);
 
             let depth = 1 + map
-                .get(&key)
+                .get(key)
                 .into_iter()
                 .flat_map(|it| it)
                 .map(|dep| depth(dep, map, results))

src/cargo/util/toml/mod.rs

@@ -835,7 +835,7 @@ impl TomlManifest {
         // Parse features first so they will be available when parsing other parts of the TOML.
         let empty = Vec::new();
         let cargo_features = me.cargo_features.as_ref().unwrap_or(&empty);
-        let features = Features::new(&cargo_features, &mut warnings)?;
+        let features = Features::new(cargo_features, &mut warnings)?;
 
         let project = me.project.as_ref().or_else(|| me.package.as_ref());
         let project = project.ok_or_else(|| failure::format_err!("no `package` section found"))?;
@@ -1010,7 +1010,7 @@ impl TomlManifest {
 
         let workspace_config = match (me.workspace.as_ref(), project.workspace.as_ref()) {
             (Some(config), None) => WorkspaceConfig::Root(WorkspaceRootConfig::new(
-                &package_root,
+                package_root,
                 &config.members,
                 &config.default_members,
                 &config.exclude,
@@ -1138,7 +1138,7 @@ impl TomlManifest {
         let mut deps = Vec::new();
         let empty = Vec::new();
         let cargo_features = me.cargo_features.as_ref().unwrap_or(&empty);
-        let features = Features::new(&cargo_features, &mut warnings)?;
+        let features = Features::new(cargo_features, &mut warnings)?;
 
         let (replace, patch) = {
             let mut cx = Context {
@@ -1157,7 +1157,7 @@ impl TomlManifest {
         let profiles = Profiles::new(me.profile.as_ref(), config, &features, &mut warnings)?;
         let workspace_config = match me.workspace {
             Some(ref config) => WorkspaceConfig::Root(WorkspaceRootConfig::new(
-                &root,
+                root,
                 &config.members,
                 &config.default_members,
                 &config.exclude,

tests/testsuite/directory.rs

@@ -577,7 +577,7 @@ fn git_lock_file_doesnt_change() {
     let root = paths::root();
     t!(fs::create_dir(&root.join(".cargo")));
     t!(t!(File::create(root.join(".cargo/config"))).write_all(
-        &format!(
+        format!(
             r#"
         [source.my-git-repo]
         git = '{}'

tests/testsuite/git.rs

@@ -158,7 +158,7 @@ fn cargo_compile_offline_with_cached_git_dep() {
         File::create(&prj.root().join("Cargo.toml"))
             .unwrap()
             .write_all(
-                &format!(
+                format!(
                     r#"
             [project]
             name = "cache_git_dep"
@@ -220,7 +220,7 @@ fn cargo_compile_offline_with_cached_git_dep() {
     File::create(&p.root().join("Cargo.toml"))
         .unwrap()
         .write_all(
-            &format!(
+            format!(
                 r#"
         [project]
         name = "foo"
@@ -2861,7 +2861,7 @@ fn templatedir_doesnt_cause_problems() {
     File::create(paths::home().join(".gitconfig"))
         .unwrap()
         .write_all(
-            &format!(
+            format!(
                 r#"
                 [init]
                 templatedir = {}

tests/testsuite/init.rs

@@ -61,10 +61,11 @@ fn simple_bin() {
 fn simple_git_ignore_exists() {
     // write a .gitignore file with one entry
     fs::create_dir_all(paths::root().join("foo")).unwrap();
-    let mut ignore_file = File::create(paths::root().join("foo/.gitignore")).unwrap();
-    ignore_file
-        .write("/target\n**/some.file".as_bytes())
-        .unwrap();
+    fs::write(
+        paths::root().join("foo/.gitignore"),
+        "/target\n**/some.file",
+    )
+    .unwrap();
 
     cargo_process("init --lib foo --edition 2015")
         .env("USER", "foo")

tests/testsuite/install.rs

@@ -1343,7 +1343,7 @@ fn install_global_cargo_config() {
     pkg("bar", "0.0.1");
 
     let config = cargo_home().join("config");
-    let mut toml = fs::read_to_string(&config).unwrap_or(String::new());
+    let mut toml = fs::read_to_string(&config).unwrap_or_default();
 
     toml.push_str(
         r#"

tests/testsuite/main.rs

@@ -2,6 +2,8 @@
 #![cfg_attr(feature = "deny-warnings", deny(warnings))]
 #![allow(clippy::blacklisted_name)]
 #![allow(clippy::explicit_iter_loop)]
+#![warn(clippy::needless_borrow)]
+#![warn(clippy::redundant_clone)]
 
 #[macro_use]
 mod support;

tests/testsuite/metabuild.rs

@@ -434,8 +434,7 @@ fn metabuild_metadata() {
         .as_array()
         .unwrap()
         .iter()
-        .filter(|p| p["name"].as_str().unwrap() == "foo")
-        .next()
+        .find(|p| p["name"].as_str().unwrap() == "foo")
         .unwrap()["metabuild"]
         .as_array()
         .unwrap()