Auto merge of #5280 - klausi:clippy-191-trvials, r=matklad

bors · bors · commit 75ec2d3a8d04 · 2018-04-05T09:22:15.000Z
chore(clippy): Simplify minor stuff found by clippy Executed clippy 0.0.191, fixed the trivial stuff. Filed rust-lang/rust-clippy#2615 for all the format_err!() false positives.
diff --git a/src/cargo/core/dependency.rs b/src/cargo/core/dependency.rs
@@ -186,7 +186,7 @@ impl Dependency {
     }
 
     pub fn new_override(name: &str, source_id: &SourceId) -> Dependency {
-        assert!(name.len() > 0);
+        assert!(!name.is_empty());
         Dependency {
             inner: Rc::new(Inner {
                 name: InternedString::new(name),
diff --git a/src/cargo/core/resolver/context.rs b/src/cargo/core/resolver/context.rs
@@ -340,7 +340,7 @@ struct Requirements<'a> {
 }
 
 impl<'r> Requirements<'r> {
-    fn new<'a>(summary: &'a Summary) -> Requirements<'a> {
+    fn new(summary: &Summary) -> Requirements {
         Requirements {
             summary,
             deps: HashMap::new(),
@@ -390,7 +390,7 @@ impl<'r> Requirements<'r> {
                 ),
                 _ => {}
             }
-            self.require_value(&fv)?;
+            self.require_value(fv)?;
         }
         Ok(())
     }
diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs
@@ -432,7 +432,7 @@ fn activate_deps_loop(
                             .remaining_siblings
                             .clone()
                             .filter_map(|(_, (ref new_dep, _, _))| {
-                                past_conflicting_activations.conflicting(&cx, &new_dep)
+                                past_conflicting_activations.conflicting(&cx, new_dep)
                             })
                             .next()
                         {
@@ -818,7 +818,7 @@ fn compatible(a: &semver::Version, b: &semver::Version) -> bool {
 ///
 /// Read <https://github.com/rust-lang/cargo/pull/4834>
 /// For several more detailed explanations of the logic here.
-fn find_candidate<'a>(
+fn find_candidate(
     backtrack_stack: &mut Vec<BacktrackFrame>,
     parent: &Summary,
     conflicting_activations: &HashMap<PackageId, ConflictReason>,
diff --git a/src/cargo/core/resolver/types.rs b/src/cargo/core/resolver/types.rs
@@ -134,8 +134,8 @@ impl<'a> RegistryQueryer<'a> {
             let previous_cmp = a_in_previous.cmp(&b_in_previous).reverse();
             match previous_cmp {
                 Ordering::Equal => {
-                    let cmp = a.summary.version().cmp(&b.summary.version());
-                    if self.minimal_versions == true {
+                    let cmp = a.summary.version().cmp(b.summary.version());
+                    if self.minimal_versions {
                         // Lower version ordered first.
                         cmp
                     } else {
diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs
@@ -130,7 +130,7 @@ impl PartialEq for Summary {
 // and creates FeatureValues for each feature.
 fn build_feature_map(
     features: BTreeMap<String, Vec<String>>,
-    dependencies: &Vec<Dependency>,
+    dependencies: &[Dependency],
 ) -> CargoResult<FeatureMap> {
     use self::FeatureValue::*;
     let mut map = BTreeMap::new();
@@ -145,9 +145,9 @@ fn build_feature_map(
 
             // Find data for the referenced dependency...
             let dep_data = {
-                let dep_name = match &val {
-                    &Feature(_) => "",
-                    &Crate(ref dep_name) | &CrateFeature(ref dep_name, _) => dep_name,
+                let dep_name = match val {
+                    Feature(_) => "",
+                    Crate(ref dep_name) | CrateFeature(ref dep_name, _) => dep_name,
                 };
                 dependencies.iter().find(|d| *d.name() == *dep_name)
             };
diff --git a/src/cargo/ops/cargo_new.rs b/src/cargo/ops/cargo_new.rs
@@ -318,7 +318,7 @@ pub fn new(opts: &NewOptions, config: &Config) -> CargoResult<()> {
 
     let mkopts = MkOptions {
         version_control: opts.version_control,
-        path: &path,
+        path,
         name,
         source_files: vec![plan_new_source_file(opts.kind.is_bin(), name.to_string())],
         bin: opts.kind.is_bin(),
@@ -341,12 +341,12 @@ pub fn init(opts: &NewOptions, config: &Config) -> CargoResult<()> {
         bail!("`cargo init` cannot be run on existing Cargo projects")
     }
 
-    let name = get_name(&path, opts)?;
+    let name = get_name(path, opts)?;
     check_name(name, opts)?;
 
     let mut src_paths_types = vec![];
 
-    detect_source_paths_and_types(&path, name, &mut src_paths_types)?;
+    detect_source_paths_and_types(path, name, &mut src_paths_types)?;
 
     if src_paths_types.is_empty() {
         src_paths_types.push(plan_new_source_file(opts.kind.is_bin(), name.to_string()));
@@ -444,37 +444,40 @@ fn mk(config: &Config, opts: &MkOptions) -> CargoResult<()> {
 
     match vcs {
         VersionControl::Git => {
-            if !fs::metadata(&path.join(".git")).is_ok() {
+            if !path.join(".git").exists() {
                 GitRepo::init(path, config.cwd())?;
             }
-            let ignore = match fs::metadata(&path.join(".gitignore")) {
-                Ok(_) => format!("\n{}", ignore),
-                _ => ignore,
+            let ignore = if path.join(".gitignore").exists() {
+                format!("\n{}", ignore)
+            } else {
+                ignore
             };
             paths::append(&path.join(".gitignore"), ignore.as_bytes())?;
         }
         VersionControl::Hg => {
-            if !fs::metadata(&path.join(".hg")).is_ok() {
+            if !path.join(".hg").exists() {
                 HgRepo::init(path, config.cwd())?;
             }
-            let hgignore = match fs::metadata(&path.join(".hgignore")) {
-                Ok(_) => format!("\n{}", hgignore),
-                _ => hgignore,
+            let hgignore = if path.join(".hgignore").exists() {
+                format!("\n{}", hgignore)
+            } else {
+                hgignore
             };
             paths::append(&path.join(".hgignore"), hgignore.as_bytes())?;
         }
         VersionControl::Pijul => {
-            if !fs::metadata(&path.join(".pijul")).is_ok() {
+            if !path.join(".pijul").exists() {
                 PijulRepo::init(path, config.cwd())?;
             }
-            let ignore = match fs::metadata(&path.join(".ignore")) {
-                Ok(_) => format!("\n{}", ignore),
-                _ => ignore,
+            let ignore = if path.join(".ignore").exists() {
+                format!("\n{}", ignore)
+            } else {
+                ignore
             };
             paths::append(&path.join(".ignore"), ignore.as_bytes())?;
         }
         VersionControl::Fossil => {
-            if !fs::metadata(&path.join(".fossil")).is_ok() {
+            if path.join(".fossil").exists() {
                 FossilRepo::init(path, config.cwd())?;
             }
         }
diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs
@@ -47,7 +47,7 @@ pub fn package(ws: &Workspace, opts: &PackageOpts) -> CargoResult<Option<FileLoc
             .iter()
             .map(|file| util::without_prefix(file, root).unwrap().to_path_buf())
             .collect();
-        if include_lockfile(&pkg) {
+        if include_lockfile(pkg) {
             list.push("Cargo.lock".into());
         }
         list.sort();
diff --git a/src/cargo/ops/cargo_run.rs b/src/cargo/ops/cargo_run.rs
@@ -18,7 +18,7 @@ pub fn run(
         Packages::Packages(ref xs) => match xs.len() {
             0 => ws.current()?,
             1 => ws.members()
-                .find(|pkg| &*pkg.name() == xs[0])
+                .find(|pkg| *pkg.name() == xs[0])
                 .ok_or_else(|| {
                     format_err!("package `{}` is not a member of the workspace", xs[0])
                 })?,
diff --git a/src/cargo/ops/cargo_rustc/context/compilation_files.rs b/src/cargo/ops/cargo_rustc/context/compilation_files.rs
@@ -384,7 +384,7 @@ fn compute_metadata<'a, 'cfg>(
         && (unit.target.is_dylib() || unit.target.is_cdylib()
             || (unit.target.is_bin() && cx.target_triple().starts_with("wasm32-")))
         && unit.pkg.package_id().source_id().is_path()
-        && !__cargo_default_lib_metadata.is_ok()
+        && __cargo_default_lib_metadata.is_err()
     {
         return None;
     }
diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs
@@ -603,13 +603,13 @@ fn link_targets<'a, 'cfg>(
                 }
             };
             destinations.push(dst.display().to_string());
-            hardlink_or_copy(&src, &dst)?;
+            hardlink_or_copy(src, dst)?;
             if let Some(ref path) = export_dir {
                 if !path.exists() {
                     fs::create_dir_all(path)?;
                 }
 
-                hardlink_or_copy(&src, &path.join(dst.file_name().unwrap()))?;
+                hardlink_or_copy(src, &path.join(dst.file_name().unwrap()))?;
             }
         }
 
diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs
@@ -604,15 +604,15 @@ pub fn search(
     }
 
     let search_max_limit = 100;
-    if total_crates > u32::from(limit) && limit < search_max_limit {
+    if total_crates > limit && limit < search_max_limit {
         println!(
             "... and {} crates more (use --limit N to see more)",
-            total_crates - u32::from(limit)
+            total_crates - limit
         );
-    } else if total_crates > u32::from(limit) && limit >= search_max_limit {
+    } else if total_crates > limit && limit >= search_max_limit {
         println!(
             "... and {} crates more (go to http://crates.io/search?q={} to see more)",
-            total_crates - u32::from(limit),
+            total_crates - limit,
             percent_encode(query.as_bytes(), QUERY_ENCODE_SET)
         );
     }
diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs
@@ -212,13 +212,11 @@ impl GitReference {
             })()
                 .chain_err(|| format!("failed to find tag `{}`", s))?,
             GitReference::Branch(ref s) => {
-                (|| {
-                    let b = repo.find_branch(s, git2::BranchType::Local)?;
-                    b.get()
-                        .target()
-                        .ok_or_else(|| format_err!("branch `{}` did not have a target", s))
-                })()
-                    .chain_err(|| format!("failed to find branch `{}`", s))?
+                let b = repo.find_branch(s, git2::BranchType::Local)
+                    .chain_err(|| format!("failed to find branch `{}`", s))?;
+                b.get()
+                    .target()
+                    .ok_or_else(|| format_err!("branch `{}` did not have a target", s))?
             }
             GitReference::Rev(ref s) => {
                 let obj = repo.revparse_single(s)?;
diff --git a/src/cargo/util/paths.rs b/src/cargo/util/paths.rs
@@ -237,12 +237,10 @@ fn _remove_file(p: &Path) -> CargoResult<()> {
         Err(e) => e,
     };
 
-    if err.kind() == io::ErrorKind::PermissionDenied {
-        if set_not_readonly(p).unwrap_or(false) {
-            match fs::remove_file(p) {
-                Ok(()) => return Ok(()),
-                Err(e) => err = e,
-            }
+    if err.kind() == io::ErrorKind::PermissionDenied && set_not_readonly(p).unwrap_or(false) {
+        match fs::remove_file(p) {
+            Ok(()) => return Ok(()),
+            Err(e) => err = e,
         }
     }
 
diff --git a/src/crates-io/lib.rs b/src/crates-io/lib.rs
@@ -167,18 +167,16 @@ impl Registry {
                     (json.len() >> 8) as u8,
                     (json.len() >> 16) as u8,
                     (json.len() >> 24) as u8,
-                ].iter()
-                    .map(|x| *x),
+                ].iter().cloned(),
             );
-            w.extend(json.as_bytes().iter().map(|x| *x));
+            w.extend(json.as_bytes().iter().cloned());
             w.extend(
                 [
                     (stat.len() >> 0) as u8,
                     (stat.len() >> 8) as u8,
                     (stat.len() >> 16) as u8,
                     (stat.len() >> 24) as u8,
-                ].iter()
-                    .map(|x| *x),
+                ].iter().cloned(),
             );
             w
         };
@@ -201,10 +199,10 @@ impl Registry {
 
         let body = handle(&mut self.handle, &mut |buf| body.read(buf).unwrap_or(0))?;
 
-        let response = if body.len() > 0 {
-            body.parse::<serde_json::Value>()?
-        } else {
+        let response = if body.is_empty() {
             "{}".parse()?
+        } else {
+            body.parse::<serde_json::Value>()?
         };
 
         let invalid_categories: Vec<String> = response
@@ -329,12 +327,9 @@ fn handle(handle: &mut Easy, read: &mut FnMut(&mut [u8]) -> usize) -> Result<Str
         Ok(body) => body,
         Err(..) => bail!("response body was not valid utf-8"),
     };
-    match serde_json::from_str::<ApiErrorList>(&body) {
-        Ok(errors) => {
-            let errors = errors.errors.into_iter().map(|s| s.detail);
-            bail!("api errors: {}", errors.collect::<Vec<_>>().join(", "))
-        }
-        Err(..) => {}
+    if let Ok(errors) = serde_json::from_str::<ApiErrorList>(&body) {
+        let errors = errors.errors.into_iter().map(|s| s.detail);
+        bail!("api errors: {}", errors.collect::<Vec<_>>().join(", "));
     }
     Ok(body)
 }
diff --git a/tests/testsuite/resolve.rs b/tests/testsuite/resolve.rs
@@ -186,7 +186,7 @@ fn loc_names(names: &[(&'static str, &'static str)]) -> Vec<PackageId> {
 }
 
 #[test]
-#[should_panic(expected = "assertion failed: name.len() > 0")]
+#[should_panic(expected = "assertion failed: !name.is_empty()")]
 fn test_dependency_with_empty_name() {
     // Bug 5229, dependency-names must not be empty
     "".to_dep();

Original file line number	Diff line number	Diff line change
`@@ -186,7 +186,7 @@ impl Dependency {`
`186`	`186`	`}`
`187`	`187`
`188`	`188`	`pub fn new_override(name: &str, source_id: &SourceId) -> Dependency {`
`189`		`- assert!(name.len() > 0);`
	`189`	`+ assert!(!name.is_empty());`
`190`	`190`	`Dependency {`
`191`	`191`	`inner: Rc::new(Inner {`
`192`	`192`	`name: InternedString::new(name),`
Original file line number	Diff line number	Diff line change
`@@ -340,7 +340,7 @@ struct Requirements<'a> {`
`340`	`340`	`}`
`341`	`341`
`342`	`342`	`impl<'r> Requirements<'r> {`
`343`		`- fn new<'a>(summary: &'a Summary) -> Requirements<'a> {`
	`343`	`+ fn new(summary: &Summary) -> Requirements {`
`344`	`344`	`Requirements {`
`345`	`345`	`summary,`
`346`	`346`	`deps: HashMap::new(),`
`@@ -390,7 +390,7 @@ impl<'r> Requirements<'r> {`
`390`	`390`	`),`
`391`	`391`	`_ => {}`
`392`	`392`	`}`
`393`		`- self.require_value(&fv)?;`
	`393`	`+ self.require_value(fv)?;`
`394`	`394`	`}`
`395`	`395`	`Ok(())`
`396`	`396`	`}`
Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,7 @@ pub fn package(ws: &Workspace, opts: &PackageOpts) -> CargoResult<Option<FileLoc`
`47`	`47`	`.iter()`
`48`	`48`	`.map(\|file\| util::without_prefix(file, root).unwrap().to_path_buf())`
`49`	`49`	`.collect();`
`50`		`- if include_lockfile(&pkg) {`
	`50`	`+ if include_lockfile(pkg) {`
`51`	`51`	`list.push("Cargo.lock".into());`
`52`	`52`	`}`
`53`	`53`	`list.sort();`
Original file line number	Diff line number	Diff line change
`@@ -384,7 +384,7 @@ fn compute_metadata<'a, 'cfg>(`
`384`	`384`	`&& (unit.target.is_dylib() \|\| unit.target.is_cdylib()`
`385`	`385`	`\|\| (unit.target.is_bin() && cx.target_triple().starts_with("wasm32-")))`
`386`	`386`	`&& unit.pkg.package_id().source_id().is_path()`
`387`		`- && !__cargo_default_lib_metadata.is_ok()`
	`387`	`+ && __cargo_default_lib_metadata.is_err()`
`388`	`388`	`{`
`389`	`389`	`return None;`
`390`	`390`	`}`
Original file line number	Diff line number	Diff line change
`@@ -603,13 +603,13 @@ fn link_targets<'a, 'cfg>(`
`603`	`603`	`}`
`604`	`604`	`};`
`605`	`605`	`destinations.push(dst.display().to_string());`
`606`		`- hardlink_or_copy(&src, &dst)?;`
	`606`	`+ hardlink_or_copy(src, dst)?;`
`607`	`607`	`if let Some(ref path) = export_dir {`
`608`	`608`	`if !path.exists() {`
`609`	`609`	`fs::create_dir_all(path)?;`
`610`	`610`	`}`
`611`	`611`
`612`		`- hardlink_or_copy(&src, &path.join(dst.file_name().unwrap()))?;`
	`612`	`+ hardlink_or_copy(src, &path.join(dst.file_name().unwrap()))?;`
`613`	`613`	`}`
`614`	`614`	`}`
`615`	`615`
Original file line number	Diff line number	Diff line change
`@@ -186,7 +186,7 @@ fn loc_names(names: &[(&'static str, &'static str)]) -> Vec<PackageId> {`
`186`	`186`	`}`
`187`	`187`
`188`	`188`	`#[test]`
`189`		`-#[should_panic(expected = "assertion failed: name.len() > 0")]`
	`189`	`+#[should_panic(expected = "assertion failed: !name.is_empty()")]`
`190`	`190`	`fn test_dependency_with_empty_name() {`
`191`	`191`	`// Bug 5229, dependency-names must not be empty`
`192`	`192`	`"".to_dep();`