some simple clippy things V2

Author: Eh2406

src/cargo/core/features.rs

@@ -99,7 +99,7 @@ macro_rules! features {
                     }
                     static FEAT: Feature = Feature {
                         name: stringify!($feature),
-                        get: get,
+                        get,
                     };
                     &FEAT
                 }

src/cargo/core/registry.rs

@@ -187,9 +187,9 @@ impl<'cfg> PackageRegistry<'cfg> {
             trace!("\t-> {}", dep);
         }
         let sub_map = self.locked.entry(id.source_id().clone())
-                                 .or_insert(HashMap::new());
+                                 .or_insert_with(HashMap::new);
         let sub_vec = sub_map.entry(id.name().to_string())
-                             .or_insert(Vec::new());
+                             .or_insert_with(Vec::new);
         sub_vec.push((id, deps));
     }
 

src/cargo/core/resolver/encode.rs

@@ -206,7 +206,7 @@ fn build_path_deps(ws: &Workspace) -> HashMap<String, SourceId> {
     for member in members.iter() {
         build_pkg(member, ws.config(), &mut ret, &mut visited);
     }
-    for (_, deps) in ws.root_patch() {
+    for deps in ws.root_patch().values()  {
         for dep in deps {
             build_dep(dep, ws.config(), &mut ret, &mut visited);
         }

src/cargo/core/resolver/mod.rs

@@ -540,9 +540,9 @@ enum ConflictReason {
 
 impl ConflictReason {
     fn is_links(&self) -> bool {
-        match self {
-            &ConflictReason::Semver => false,
-            &ConflictReason::Links(_) => true,
+        match *self {
+            ConflictReason::Semver => false,
+            ConflictReason::Links(_) => true,
         }
     }
 }
@@ -739,7 +739,7 @@ fn activate_deps_loop<'a>(
                     registry,
                     &parent,
                     &dep,
-                    conflicting,
+                    &conflicting,
                     &candidates,
                     config,
                 )
@@ -780,8 +780,8 @@ fn activate_deps_loop<'a>(
 /// remaining candidates. For each one, also checks if rolling back
 /// could change the outcome of the failed resolution that caused backtracking
 /// in the first place. Namely, if we've backtracked past the parent of the
-/// failed dep, or any of the packages flagged as giving us trouble in conflicting_activations.
-/// Read https://github.com/rust-lang/cargo/pull/4834
+/// failed dep, or any of the packages flagged as giving us trouble in `conflicting_activations`.
+/// Read <https://github.com/rust-lang/cargo/pull/4834>
 /// For several more detailed explanations of the logic here.
 ///
 /// If the outcome could differ, resets `cx` and `remaining_deps` to that
@@ -826,7 +826,7 @@ fn activation_error(cx: &Context,
                     registry: &mut Registry,
                     parent: &Summary,
                     dep: &Dependency,
-                    conflicting_activations: HashMap<PackageId, ConflictReason>,
+                    conflicting_activations: &HashMap<PackageId, ConflictReason>,
                     candidates: &[Candidate],
                     config: Option<&Config>) -> CargoError {
     let graph = cx.graph();
@@ -860,17 +860,14 @@ fn activation_error(cx: &Context,
         let (links_errors, other_errors): (Vec<_>, Vec<_>) = conflicting_activations.drain(..).rev().partition(|&(_, r)| r.is_links());
 
         for &(p, r) in &links_errors {
-            match r {
-                &ConflictReason::Links(ref link) => {
-                     msg.push_str("\n\nthe package `");
-                     msg.push_str(dep.name());
-                     msg.push_str("` links to the native library `");
-                     msg.push_str(&link);
-                     msg.push_str("`, but it conflicts with a previous package which links to `");
-                     msg.push_str(&link);
-                     msg.push_str("` as well:\n");
-                },
-                _ => (),
+            if let ConflictReason::Links(ref link) = *r {
+                msg.push_str("\n\nthe package `");
+                msg.push_str(dep.name());
+                msg.push_str("` links to the native library `");
+                msg.push_str(link);
+                msg.push_str("`, but it conflicts with a previous package which links to `");
+                msg.push_str(link);
+                msg.push_str("` as well:\n");
             }
             msg.push_str(&describe_path(p));
         }
@@ -1031,7 +1028,7 @@ impl<'r> Requirements<'r> {
             return Ok(());
         }
         for f in self.summary.features().get(feat).expect("must be a valid feature") {
-            if f == &feat {
+            if f == feat {
                 bail!("Cyclic feature dependency: feature `{}` depends on itself", feat);
             }
             self.add_feature(f)?;
@@ -1095,7 +1092,7 @@ fn build_requirements<'a, 'b: 'a>(s: &'a Summary, method: &'b Method)
         }
         Method::Required { uses_default_features: false, .. } => {}
     }
-    return Ok(reqs);
+    Ok(reqs)
 }
 
 impl<'a> Context<'a> {
@@ -1110,7 +1107,7 @@ impl<'a> Context<'a> {
                        .entry(id.name().to_string())
                        .or_insert_with(HashMap::new)
                        .entry(id.source_id().clone())
-                       .or_insert(Vec::new());
+                       .or_insert_with(Vec::new);
         if !prev.iter().any(|c| c == summary) {
             self.resolve_graph.push(GraphNode::Add(id.clone()));
             if let Some(link) = summary.links() {
@@ -1294,7 +1291,7 @@ impl<'a> Context<'a> {
             let mut base = base.1;
             base.extend(dep.features().iter().cloned());
             for feature in base.iter() {
-                if feature.contains("/") {
+                if feature.contains('/') {
                     bail!("feature names may not contain slashes: `{}`", feature);
                 }
             }

src/cargo/core/shell.rs

@@ -27,13 +27,13 @@ pub struct Shell {
 
 impl fmt::Debug for Shell {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match &self.err {
-            &ShellOut::Write(_) => {
+        match self.err {
+            ShellOut::Write(_) => {
                 f.debug_struct("Shell")
                     .field("verbosity", &self.verbosity)
                     .finish()
             }
-            &ShellOut::Stream { color_choice, .. } => {
+            ShellOut::Stream { color_choice, .. } => {
                 f.debug_struct("Shell")
                     .field("verbosity", &self.verbosity)
                     .field("color_choice", &color_choice)
@@ -221,6 +221,12 @@ impl Shell {
     }
 }
 
+impl Default for Shell {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl ShellOut {
     /// Print out a message with a status. The status comes first and is bold + the given color.
     /// The status can be justified, in which case the max width that will right align is 12 chars.

src/cargo/core/source/source_id.rs

@@ -444,7 +444,7 @@ impl Hash for SourceId {
     }
 }
 
-/// A `Display`able view into a SourceId that will write it as a url
+/// A `Display`able view into a `SourceId` that will write it as a url
 pub struct SourceIdToUrl<'a> {
     inner: &'a SourceIdInner,
 }

src/cargo/core/workspace.rs

@@ -340,7 +340,7 @@ impl<'cfg> Workspace<'cfg> {
                 match *self.packages.load(&ances_manifest_path)?.workspace_config() {
                     WorkspaceConfig::Root(ref ances_root_config) => {
                         debug!("find_root - found a root checking exclusion");
-                        if !ances_root_config.is_excluded(&manifest_path) {
+                        if !ances_root_config.is_excluded(manifest_path) {
                             debug!("find_root - found!");
                             return Ok(Some(ances_manifest_path))
                         }
@@ -443,13 +443,10 @@ impl<'cfg> Workspace<'cfg> {
             return Ok(())
         }
 
-        match *self.packages.load(root_manifest)?.workspace_config() {
-            WorkspaceConfig::Root(ref root_config) => {
-                if root_config.is_excluded(&manifest_path) {
-                    return Ok(())
-                }
+        if let WorkspaceConfig::Root(ref root_config) = *self.packages.load(root_manifest)?.workspace_config() {
+            if root_config.is_excluded(&manifest_path) {
+                return Ok(())
             }
-            _ => {}
         }
 
         debug!("find_members - {}", manifest_path.display());

src/cargo/lib.rs

@@ -2,6 +2,11 @@
 #![cfg_attr(test, deny(warnings))]
 #![recursion_limit="128"]
 
+// Currently, Cargo does not use clippy for its source code.
+// But if someone runs it they should know that
+// @alexcrichton disagree with clippy on some style things
+#![cfg_attr(feature = "cargo-clippy", allow(explicit_iter_loop))]
+
 #[macro_use] extern crate failure;
 #[macro_use] extern crate log;
 #[macro_use] extern crate scoped_tls;
@@ -51,7 +56,7 @@ use core::shell::Verbosity::Verbose;
 pub use util::{CargoError, CargoResult, CliError, CliResult, Config};
 pub use util::errors::Internal;
 
-pub const CARGO_ENV: &'static str = "CARGO";
+pub const CARGO_ENV: &str = "CARGO";
 
 pub mod core;
 pub mod ops;

src/cargo/ops/cargo_compile.rs

@@ -321,7 +321,7 @@ pub fn compile_ws<'a>(ws: &Workspace<'a>,
                              config,
                              build_config,
                              profiles,
-                             exec)?
+                             &exec)?
     };
 
     ret.to_doc_test = to_builds.into_iter().cloned().collect();

src/cargo/ops/cargo_doc.rs

@@ -44,13 +44,11 @@ pub fn doc(ws: &Workspace, options: &DocOptions) -> CargoResult<()> {
                           or marking one of the targets as `doc = false`.",
                           target.crate_name(), prev, package);
                 }
-            } else {
-                if let Some(prev) = bin_names.insert(target.crate_name(), package) {
-                    bail!("The binary `{}` is specified by packages `{}` and \
-                          `{}` but can be documented only once. Consider renaming \
-                          or marking one of the targets as `doc = false`.",
-                          target.crate_name(), prev, package);
-                }
+            } else if let Some(prev) = bin_names.insert(target.crate_name(), package) {
+                bail!("The binary `{}` is specified by packages `{}` and \
+                      `{}` but can be documented only once. Consider renaming \
+                      or marking one of the targets as `doc = false`.",
+                      target.crate_name(), prev, package);
             }
         }
     }

src/cargo/ops/cargo_generate_lockfile.rs

@@ -174,10 +174,10 @@ pub fn update_lockfile(ws: &Workspace, opts: &UpdateOptions)
         let mut changes = BTreeMap::new();
         let empty = (Vec::new(), Vec::new());
         for dep in previous_resolve.iter() {
-            changes.entry(key(dep)).or_insert(empty.clone()).0.push(dep);
+            changes.entry(key(dep)).or_insert_with(||empty.clone()).0.push(dep);
         }
         for dep in resolve.iter() {
-            changes.entry(key(dep)).or_insert(empty.clone()).1.push(dep);
+            changes.entry(key(dep)).or_insert_with(||empty.clone()).1.push(dep);
         }
 
         for v in changes.values_mut() {

src/cargo/ops/cargo_install.rs

@@ -286,7 +286,7 @@ fn install_one(root: &Filesystem,
             }
         }
         list.v1.entry(pkg.package_id().clone())
-               .or_insert_with(|| BTreeSet::new())
+               .or_insert_with(BTreeSet::new)
                .insert(bin.to_string());
     }
 
@@ -301,7 +301,7 @@ fn install_one(root: &Filesystem,
     // If installation was successful record newly installed binaries.
     if result.is_ok() {
         list.v1.entry(pkg.package_id().clone())
-               .or_insert_with(|| BTreeSet::new())
+               .or_insert_with(BTreeSet::new)
                .extend(to_install.iter().map(|s| s.to_string()));
     }
 
@@ -347,7 +347,7 @@ fn select_pkg<'a, T>(mut source: T,
                     // version range, otherwise parse it as a specific version
                     let first = v.chars()
                         .nth(0)
-                        .ok_or(format_err!("no version provided for the `--vers` flag"))?;
+                        .ok_or_else(||format_err!("no version provided for the `--vers` flag"))?;
 
                     match first {
                         '<' | '>' | '=' | '^' | '~' => match v.parse::<VersionReq>() {
@@ -570,20 +570,20 @@ pub fn uninstall(root: Option<&str>,
                  specs: Vec<&str>,
                  bins: &[String],
                  config: &Config) -> CargoResult<()> {
-    if specs.len() > 1 && bins.len() > 0 {
+    if specs.len() > 1 && !bins.is_empty() {
         bail!("A binary can only be associated with a single installed package, specifying multiple specs with --bin is redundant.");
     }
 
     let root = resolve_root(root, config)?;
     let scheduled_error = if specs.len() == 1 {
-        uninstall_one(root, specs[0], bins, config)?;
+        uninstall_one(&root, specs[0], bins, config)?;
         false
     } else {
         let mut succeeded = vec![];
         let mut failed = vec![];
         for spec in specs {
             let root = root.clone();
-            match uninstall_one(root, spec, bins, config) {
+            match uninstall_one(&root, spec, bins, config) {
                 Ok(()) => succeeded.push(spec),
                 Err(e) => {
                     ::handle_error(e, &mut config.shell());
@@ -614,11 +614,11 @@ pub fn uninstall(root: Option<&str>,
     Ok(())
 }
 
-pub fn uninstall_one(root: Filesystem,
+pub fn uninstall_one(root: &Filesystem,
                      spec: &str,
                      bins: &[String],
                      config: &Config) -> CargoResult<()> {
-    let crate_metadata = metadata(config, &root)?;
+    let crate_metadata = metadata(config, root)?;
     let mut metadata = read_crate_list(&crate_metadata)?;
     let mut to_remove = Vec::new();
     {

src/cargo/ops/cargo_new.rs

@@ -90,10 +90,9 @@ impl<'a> NewOptions<'a> {
 
         let kind = match (bin, lib) {
             (true, true) => bail!("can't specify both lib and binary outputs"),
-            (true, false) => NewProjectKind::Bin,
             (false, true) => NewProjectKind::Lib,
             // default to bin
-            (false, false) => NewProjectKind::Bin,
+            (_, false) => NewProjectKind::Bin,
         };
 
         let opts = NewOptions { version_control, kind, path, name };

src/cargo/ops/cargo_run.rs

@@ -34,7 +34,7 @@ pub fn run(ws: &Workspace,
     .map(|bin| bin.name())
     .collect();
 
-    if bins.len() == 0 {
+    if bins.is_empty() {
         if !options.filter.is_specific() {
             bail!("a bin target must be available for `cargo run`")
         } else {

src/cargo/ops/cargo_rustc/context.rs

@@ -471,7 +471,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
 
     /// Return the target triple which this context is targeting.
     pub fn target_triple(&self) -> &str {
-        self.requested_target().unwrap_or(self.host_triple())
+        self.requested_target().unwrap_or_else(|| self.host_triple())
     }
 
     /// Requested (not actual) target for the build
@@ -694,8 +694,8 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
                     match *crate_type_info {
                         Some((ref prefix, ref suffix)) => {
                             let suffixes = add_target_specific_suffixes(
-                                &self.target_triple(),
-                                &crate_type,
+                                self.target_triple(),
+                                crate_type,
                                 unit.target.kind(),
                                 suffix,
                                 file_type,
@@ -1055,10 +1055,9 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
     }
 
     pub fn lib_or_check_profile(&self, unit: &Unit, target: &Target) -> &'a Profile {
-        if !target.is_custom_build() && !target.for_host() {
-            if unit.profile.check || (unit.profile.doc && !unit.profile.test) {
+        if !target.is_custom_build() && !target.for_host()
+            && (unit.profile.check || (unit.profile.doc && !unit.profile.test)) {
                 return &self.profiles.check
-            }
         }
         self.lib_profile()
     }