Auto merge of #3733 - llogiq:clippy, r=alexcrichton

some clippy-suggested improvements

This fixes a number of [clippy](https://github.com/Manishearth/rust-clippy) warnings. It's mostly about readability, though a few changes could affect performance (though probably not measurably).

I've left out things to fix later; I thought I'd just push the first batch to see if you like it.

src/cargo/lib.rs

@@ -77,15 +77,12 @@ impl fmt::Display for VersionInfo {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         write!(f, "cargo {}.{}.{}",
                self.major, self.minor, self.patch)?;
-        match self.cfg_info.as_ref().map(|ci| &ci.release_channel) {
-            Some(channel) => {
-                if channel != "stable" {
-                    write!(f, "-{}", channel)?;
-                    let empty = String::from("");
-                    write!(f, "{}", self.pre_release.as_ref().unwrap_or(&empty))?;
-                }
-            },
-            None => (),
+        if let Some(channel) = self.cfg_info.as_ref().map(|ci| &ci.release_channel) {
+            if channel != "stable" {
+                write!(f, "-{}", channel)?;
+                let empty = String::from("");
+                write!(f, "{}", self.pre_release.as_ref().unwrap_or(&empty))?;
+            }
         };
 
         if let Some(ref cfg) = self.cfg_info {

src/cargo/ops/cargo_rustc/fingerprint.rs

@@ -583,16 +583,13 @@ pub fn parse_dep_info(dep_info: &Path) -> CargoResult<Option<Vec<PathBuf>>> {
 
     let mut paths = Vec::new();
     let mut deps = deps.split(' ').map(|s| s.trim()).filter(|s| !s.is_empty());
-    loop {
-        let mut file = match deps.next() {
-            Some(s) => s.to_string(),
-            None => break,
-        };
-        while file.ends_with("\\") {
+    while let Some(s) = deps.next() {
+        let mut file = s.to_string();
+        while file.ends_with('\\') {
             file.pop();
             file.push(' ');
             file.push_str(deps.next().chain_error(|| {
-                internal(format!("malformed dep-info format, trailing \\"))
+                internal("malformed dep-info format, trailing \\".to_string())
             })?);
         }
         paths.push(cwd.join(&file));
@@ -602,7 +599,7 @@ pub fn parse_dep_info(dep_info: &Path) -> CargoResult<Option<Vec<PathBuf>>> {
 
 fn dep_info_mtime_if_fresh(dep_info: &Path) -> CargoResult<Option<FileTime>> {
     if let Some(paths) = parse_dep_info(dep_info)? {
-        Ok(mtime_if_fresh(&dep_info, paths.iter()))
+        Ok(mtime_if_fresh(dep_info, paths.iter()))
     } else {
         Ok(None)
     }

src/cargo/ops/cargo_rustc/layout.rs

@@ -76,7 +76,7 @@ impl Layout {
         // the target triple as a Path and then just use the file stem as the
         // component for the directory name.
         if let Some(triple) = triple {
-            path.push(Path::new(triple).file_stem().ok_or(human(format!("target was empty")))?);
+            path.push(Path::new(triple).file_stem().ok_or(human("target was empty".to_string()))?);
         }
         path.push(dest);
         Layout::at(ws.config(), path)

src/cargo/ops/cargo_rustc/mod.rs

@@ -170,7 +170,7 @@ pub fn compile_targets<'a, 'cfg: 'a>(ws: &Workspace<'cfg>,
                 let pkgid = unit.pkg.package_id();
                 if !unit.target.is_lib() { continue }
                 if unit.profile.doc { continue }
-                if cx.compilation.libraries.contains_key(&pkgid) {
+                if cx.compilation.libraries.contains_key(pkgid) {
                     continue
                 }
 
@@ -182,9 +182,9 @@ pub fn compile_targets<'a, 'cfg: 'a>(ws: &Workspace<'cfg>,
             }
         }
 
-        if let Some(feats) = cx.resolve.features(&unit.pkg.package_id()) {
+        if let Some(feats) = cx.resolve.features(unit.pkg.package_id()) {
             cx.compilation.cfgs.entry(unit.pkg.package_id().clone())
-                .or_insert(HashSet::new())
+                .or_insert_with(HashSet::new)
                 .extend(feats.iter().map(|feat| format!("feature=\"{}\"", feat)));
         }
 
@@ -193,7 +193,7 @@ pub fn compile_targets<'a, 'cfg: 'a>(ws: &Workspace<'cfg>,
 
     for (&(ref pkg, _), output) in cx.build_state.outputs.lock().unwrap().iter() {
         cx.compilation.cfgs.entry(pkg.clone())
-            .or_insert(HashSet::new())
+            .or_insert_with(HashSet::new)
             .extend(output.cfgs.iter().cloned());
 
         for dir in output.library_paths.iter() {
@@ -344,7 +344,7 @@ fn rustc(cx: &mut Context, unit: &Unit, exec: Arc<Executor>) -> CargoResult<Work
                 },
                 &mut |line| {
                     // stderr from rustc can have a mix of JSON and non-JSON output
-                    if line.starts_with("{") {
+                    if line.starts_with('{') {
                         // Handle JSON lines
                         let compiler_message = json::Json::from_str(line).map_err(|_| {
                             internal(&format!("compiler produced invalid json: `{}`", line))

src/cargo/ops/cargo_rustc/output_depinfo.rs

@@ -22,7 +22,7 @@ fn render_filename<P: AsRef<Path>>(path: P, basedir: Option<&str>) -> CargoResul
 fn add_deps_for_unit<'a, 'b>(deps: &mut HashSet<PathBuf>, context: &mut Context<'a, 'b>,
     unit: &Unit<'a>, visited: &mut HashSet<Unit<'a>>) -> CargoResult<()>
 {
-    if !visited.insert(unit.clone()) {
+    if !visited.insert(*unit) {
         return Ok(());
     }
 
@@ -76,13 +76,10 @@ pub fn output_depinfo<'a, 'b>(context: &mut Context<'a, 'b>, unit: &Unit<'a>) ->
                 // dep-info generation failed, so delete output file. This will usually
                 // cause the build system to always rerun the build rule, which is correct
                 // if inefficient.
-                match fs::remove_file(output_path) {
-                    Err(err) => {
-                        if err.kind() != ErrorKind::NotFound {
-                            return Err(err.into());
-                        }
+                if let Err(err) = fs::remove_file(output_path) {
+                    if err.kind() != ErrorKind::NotFound {
+                        return Err(err.into());
                     }
-                    _ => ()
                 }
             }
         }

src/cargo/ops/cargo_test.rs

@@ -85,7 +85,7 @@ fn run_unit_tests(options: &TestOptions,
     let mut errors = Vec::new();
 
     for &(ref pkg, _, ref exe) in &compilation.tests {
-        let to_display = match util::without_prefix(exe, &cwd) {
+        let to_display = match util::without_prefix(exe, cwd) {
             Some(path) => path,
             None => &**exe,
         };
@@ -145,7 +145,7 @@ fn run_doc_tests(options: &TestOptions,
                 p.arg("--test-args").arg(arg);
             }
 
-            if let Some(cfgs) = compilation.cfgs.get(&package.package_id()) {
+            if let Some(cfgs) = compilation.cfgs.get(package.package_id()) {
                 for cfg in cfgs.iter() {
                     p.arg("--cfg").arg(cfg);
                 }

src/cargo/ops/lockfile.rs

@@ -67,20 +67,17 @@ pub fn write_pkg_lockfile(ws: &Workspace, resolve: &Resolve) -> CargoResult<()>
         emit_package(root.as_table().unwrap(), &mut out);
     }
 
-    let deps = e.toml.get(&"package".to_string()).unwrap().as_slice().unwrap();
+    let deps = e.toml[&"package".to_string()].as_slice().unwrap();
     for dep in deps.iter() {
         let dep = dep.as_table().unwrap();
 
         out.push_str("[[package]]\n");
         emit_package(dep, &mut out);
     }
 
-    match e.toml.get(&"metadata".to_string()) {
-        Some(metadata) => {
-            out.push_str("[metadata]\n");
-            out.push_str(&metadata.to_string());
-        }
-        None => {}
+    if let Some(metadata) = e.toml.get(&"metadata".to_string()) {
+        out.push_str("[metadata]\n");
+        out.push_str(&metadata.to_string());
     }
 
     // If the lockfile contents haven't changed so don't rewrite it. This is
@@ -128,8 +125,8 @@ fn emit_package(dep: &toml::Table, out: &mut String) {
         out.push_str(&format!("source = {}\n", lookup(dep, "source")));
     }
 
-    if let Some(ref s) = dep.get("dependencies") {
-        let slice = Value::as_slice(*s).unwrap();
+    if let Some(s) = dep.get("dependencies") {
+        let slice = Value::as_slice(s).unwrap();
 
         if !slice.is_empty() {
             out.push_str("dependencies = [\n");

src/cargo/ops/registry.rs

@@ -51,7 +51,7 @@ pub fn publish(ws: &Workspace, opts: &PublishOpts) -> CargoResult<()> {
     let (mut registry, reg_id) = registry(opts.config,
                                                opts.token.clone(),
                                                opts.index.clone())?;
-    verify_dependencies(&pkg, &reg_id)?;
+    verify_dependencies(pkg, &reg_id)?;
 
     // Prepare a tarball, with a non-surpressable warning if metadata
     // is missing since this is being put online.
@@ -66,7 +66,7 @@ pub fn publish(ws: &Workspace, opts: &PublishOpts) -> CargoResult<()> {
 
     // Upload said tarball to the specified destination
     opts.config.shell().status("Uploading", pkg.package_id().to_string())?;
-    transmit(opts.config, &pkg, tarball.file(), &mut registry, opts.dry_run)?;
+    transmit(opts.config, pkg, tarball.file(), &mut registry, opts.dry_run)?;
 
     Ok(())
 }
@@ -121,13 +121,10 @@ fn transmit(config: &Config,
         Some(ref readme) => Some(paths::read(&pkg.root().join(readme))?),
         None => None,
     };
-    match *license_file {
-        Some(ref file) => {
-            if fs::metadata(&pkg.root().join(file)).is_err() {
-                bail!("the license file `{}` does not exist", file)
-            }
+    if let Some(ref file) = *license_file {
+        if fs::metadata(&pkg.root().join(file)).is_err() {
+            bail!("the license file `{}` does not exist", file)
         }
-        None => {}
     }
 
     // Do not upload if performing a dry run
@@ -246,18 +243,13 @@ pub fn http_handle(config: &Config) -> CargoResult<Easy> {
 /// Favor cargo's `http.proxy`, then git's `http.proxy`. Proxies specified
 /// via environment variables are picked up by libcurl.
 fn http_proxy(config: &Config) -> CargoResult<Option<String>> {
-    match config.get_string("http.proxy")? {
-        Some(s) => return Ok(Some(s.val)),
-        None => {}
+    if let Some(s) = config.get_string("http.proxy")? {
+        return Ok(Some(s.val))
     }
-    match git2::Config::open_default() {
-        Ok(cfg) => {
-            match cfg.get_str("http.proxy") {
-                Ok(s) => return Ok(Some(s.to_string())),
-                Err(..) => {}
-            }
+    if let Ok(cfg) = git2::Config::open_default() {
+        if let Ok(s) = cfg.get_str("http.proxy") {
+            return Ok(Some(s.to_string()))
         }
-        Err(..) => {}
     }
     Ok(None)
 }
@@ -282,9 +274,8 @@ pub fn http_proxy_exists(config: &Config) -> CargoResult<bool> {
 }
 
 pub fn http_timeout(config: &Config) -> CargoResult<Option<i64>> {
-    match config.get_i64("http.timeout")? {
-        Some(s) => return Ok(Some(s.val)),
-        None => {}
+    if let Some(s) = config.get_i64("http.timeout")? {
+        return Ok(Some(s.val))
     }
     Ok(env::var("HTTP_TIMEOUT").ok().and_then(|s| s.parse().ok()))
 }
@@ -293,11 +284,8 @@ pub fn registry_login(config: &Config, token: String) -> CargoResult<()> {
     let RegistryConfig { index, token: _ } = registry_configuration(config)?;
     let mut map = HashMap::new();
     let p = config.cwd().to_path_buf();
-    match index {
-        Some(index) => {
-            map.insert("index".to_string(), ConfigValue::String(index, p.clone()));
-        }
-        None => {}
+    if let Some(index) = index {
+        map.insert("index".to_string(), ConfigValue::String(index, p.clone()));
     }
     map.insert("token".to_string(), ConfigValue::String(token, p));
 
@@ -327,28 +315,22 @@ pub fn modify_owners(config: &Config, opts: &OwnersOptions) -> CargoResult<()> {
     let (mut registry, _) = registry(config, opts.token.clone(),
                                           opts.index.clone())?;
 
-    match opts.to_add {
-        Some(ref v) => {
-            let v = v.iter().map(|s| &s[..]).collect::<Vec<_>>();
-            config.shell().status("Owner", format!("adding {:?} to crate {}",
-                                                        v, name))?;
-            registry.add_owners(&name, &v).map_err(|e| {
-                human(format!("failed to add owners to crate {}: {}", name, e))
-            })?;
-        }
-        None => {}
+    if let Some(ref v) = opts.to_add {
+        let v = v.iter().map(|s| &s[..]).collect::<Vec<_>>();
+        config.shell().status("Owner", format!("adding {:?} to crate {}",
+                                                    v, name))?;
+        registry.add_owners(&name, &v).map_err(|e| {
+            human(format!("failed to add owners to crate {}: {}", name, e))
+        })?;
     }
 
-    match opts.to_remove {
-        Some(ref v) => {
-            let v = v.iter().map(|s| &s[..]).collect::<Vec<_>>();
-            config.shell().status("Owner", format!("removing {:?} from crate {}",
-                                                        v, name))?;
-            registry.remove_owners(&name, &v).map_err(|e| {
-                human(format!("failed to remove owners from crate {}: {}", name, e))
-            })?;
-        }
-        None => {}
+    if let Some(ref v) = opts.to_remove {
+        let v = v.iter().map(|s| &s[..]).collect::<Vec<_>>();
+        config.shell().status("Owner", format!("removing {:?} from crate {}",
+                                                    v, name))?;
+        registry.remove_owners(&name, &v).map_err(|e| {
+            human(format!("failed to remove owners from crate {}: {}", name, e))
+        })?;
     }
 
     if opts.list {

src/cargo/ops/resolve.rs

@@ -61,7 +61,7 @@ pub fn resolve_ws_precisely<'a>(ws: &Workspace<'a>,
     let resolved_with_overrides =
     ops::resolve_with_previous(&mut registry, ws,
                                method, Some(&resolve), None,
-                               &specs)?;
+                               specs)?;
 
     for &(ref replace_spec, _) in ws.root_replace() {
         if !resolved_with_overrides.replacements().keys().any(|r| replace_spec.matches(r)) {

src/cargo/sources/directory.rs

@@ -76,7 +76,7 @@ impl<'cfg> Source for DirectorySource<'cfg> {
             // crates and otherwise may conflict with a VCS
             // (rust-lang/cargo#3414).
             if let Some(s) = path.file_name().and_then(|s| s.to_str()) {
-                if s.starts_with(".") {
+                if s.starts_with('.') {
                     continue
                 }
             }

src/cargo/sources/git/source.rs

@@ -146,7 +146,7 @@ impl<'cfg> Source for GitSource<'cfg> {
 
             trace!("updating git source `{:?}`", self.remote);
 
-            let repo = self.remote.checkout(&db_path, &self.config)?;
+            let repo = self.remote.checkout(&db_path, self.config)?;
             let rev = repo.rev_for(&self.reference)?;
             (repo, rev)
         } else {
@@ -166,7 +166,7 @@ impl<'cfg> Source for GitSource<'cfg> {
         // in scope so the destructors here won't tamper with too much.
         // Checkout is immutable, so we don't need to protect it with a lock once
         // it is created.
-        repo.copy_to(actual_rev.clone(), &checkout_path, &self.config)?;
+        repo.copy_to(actual_rev.clone(), &checkout_path, self.config)?;
 
         let source_id = self.source_id.with_precise(Some(actual_rev.to_string()));
         let path_source = PathSource::new_recursive(&checkout_path,