Auto merge of #8124 - ehuss:fs-cleanup, r=Eh2406

Use some fs shorthand functions.

This is just some cleanup, using some fs convenience functions that I think make the code more readable.

Note: While going through these, I found the `out_dir_is_preserved` test was broken and not doing what it was supposed to.

crates/cargo-test-support/src/git.rs

@@ -39,8 +39,7 @@ use some of the helper functions in this file to interact with the repository.
 */
 
 use crate::{path2url, project, Project, ProjectBuilder};
-use std::fs::{self, File};
-use std::io::prelude::*;
+use std::fs;
 use std::path::{Path, PathBuf};
 use url::Url;
 
@@ -81,7 +80,7 @@ impl RepoBuilder {
     pub fn nocommit_file(self, path: &str, contents: &str) -> RepoBuilder {
         let dst = self.repo.workdir().unwrap().join(path);
         t!(fs::create_dir_all(dst.parent().unwrap()));
-        t!(t!(File::create(&dst)).write_all(contents.as_bytes()));
+        t!(fs::write(&dst, contents));
         self
     }
 

crates/cargo-test-support/src/lib.rs

@@ -112,7 +112,6 @@ use std::env;
 use std::ffi::OsStr;
 use std::fmt;
 use std::fs;
-use std::io::prelude::*;
 use std::os;
 use std::path::{Path, PathBuf};
 use std::process::{Command, Output};
@@ -166,11 +165,8 @@ impl FileBuilder {
 
     fn mk(&self) {
         self.dirname().mkdir_p();
-
-        let mut file = fs::File::create(&self.path)
+        fs::write(&self.path, &self.body)
             .unwrap_or_else(|e| panic!("could not create file {}: {}", self.path.display(), e));
-
-        t!(file.write_all(self.body.as_bytes()));
     }
 
     fn dirname(&self) -> &Path {
@@ -458,25 +454,15 @@ impl Project {
 
     /// Returns the contents of a path in the project root
     pub fn read_file(&self, path: &str) -> String {
-        let mut buffer = String::new();
-        fs::File::open(self.root().join(path))
-            .unwrap()
-            .read_to_string(&mut buffer)
-            .unwrap();
-        buffer
+        let full = self.root().join(path);
+        fs::read_to_string(&full)
+            .unwrap_or_else(|e| panic!("could not read file {}: {}", full.display(), e))
     }
 
     /// Modifies `Cargo.toml` to remove all commented lines.
     pub fn uncomment_root_manifest(&self) {
-        let mut contents = String::new();
-        fs::File::open(self.root().join("Cargo.toml"))
-            .unwrap()
-            .read_to_string(&mut contents)
-            .unwrap();
-        fs::File::create(self.root().join("Cargo.toml"))
-            .unwrap()
-            .write_all(contents.replace("#", "").as_bytes())
-            .unwrap();
+        let contents = self.read_file("Cargo.toml").replace("#", "");
+        fs::write(self.root().join("Cargo.toml"), contents).unwrap();
     }
 
     pub fn symlink(&self, src: impl AsRef<Path>, dst: impl AsRef<Path>) {

crates/cargo-test-support/src/registry.rs

@@ -162,36 +162,37 @@ pub struct Dependency {
 pub fn init() {
     let config = paths::home().join(".cargo/config");
     t!(fs::create_dir_all(config.parent().unwrap()));
-    if fs::metadata(&config).is_ok() {
+    if config.exists() {
         return;
     }
-    t!(t!(File::create(&config)).write_all(
+    t!(fs::write(
+        &config,
         format!(
             r#"
-        [source.crates-io]
-        registry = 'https://wut'
-        replace-with = 'dummy-registry'
+                [source.crates-io]
+                registry = 'https://wut'
+                replace-with = 'dummy-registry'
 
-        [source.dummy-registry]
-        registry = '{reg}'
+                [source.dummy-registry]
+                registry = '{reg}'
 
-        [registries.alternative]
-        index = '{alt}'
-    "#,
+                [registries.alternative]
+                index = '{alt}'
+            "#,
             reg = registry_url(),
             alt = alt_registry_url()
         )
-        .as_bytes()
     ));
     let credentials = paths::home().join(".cargo/credentials");
-    t!(t!(File::create(&credentials)).write_all(
-        br#"
-        [registry]
-        token = "api-token"
-
-        [registries.alternative]
-        token = "api-token"
-    "#
+    t!(fs::write(
+        &credentials,
+        r#"
+            [registry]
+            token = "api-token"
+
+            [registries.alternative]
+            token = "api-token"
+        "#
     ));
 
     // Initialize a new registry.
@@ -404,8 +405,7 @@ impl Package {
             })
             .collect::<Vec<_>>();
         let cksum = {
-            let mut c = Vec::new();
-            t!(t!(File::open(&self.archive_dst())).read_to_end(&mut c));
+            let c = t!(fs::read(&self.archive_dst()));
             cksum(&c)
         };
         let name = if self.invalid_json {
@@ -442,10 +442,9 @@ impl Package {
         } else {
             registry_path.join(&file)
         };
-        let mut prev = String::new();
-        let _ = File::open(&dst).and_then(|mut f| f.read_to_string(&mut prev));
+        let prev = fs::read_to_string(&dst).unwrap_or(String::new());
         t!(fs::create_dir_all(dst.parent().unwrap()));
-        t!(t!(File::create(&dst)).write_all((prev + &line[..] + "\n").as_bytes()));
+        t!(fs::write(&dst, prev + &line[..] + "\n"));
 
         // Add the new file to the index.
         if !self.local {

src/bin/cargo/main.rs

@@ -165,9 +165,7 @@ fn is_executable<P: AsRef<Path>>(path: P) -> bool {
 }
 #[cfg(windows)]
 fn is_executable<P: AsRef<Path>>(path: P) -> bool {
-    fs::metadata(path)
-        .map(|metadata| metadata.is_file())
-        .unwrap_or(false)
+    path.as_ref().is_file()
 }
 
 fn search_directories(config: &Config) -> Vec<PathBuf> {

src/cargo/ops/cargo_new.rs

@@ -273,10 +273,7 @@ fn detect_source_paths_and_types(
         let pp = i.proposed_path;
 
         // path/pp does not exist or is not a file
-        if !fs::metadata(&path.join(&pp))
-            .map(|x| x.is_file())
-            .unwrap_or(false)
-        {
+        if !path.join(&pp).is_file() {
             continue;
         }
 
@@ -358,7 +355,7 @@ fn plan_new_source_file(bin: bool, package_name: String) -> SourceFileInformatio
 
 pub fn new(opts: &NewOptions, config: &Config) -> CargoResult<()> {
     let path = &opts.path;
-    if fs::metadata(path).is_ok() {
+    if path.exists() {
         anyhow::bail!(
             "destination `{}` already exists\n\n\
              Use `cargo init` to initialize the directory",
@@ -397,7 +394,7 @@ pub fn init(opts: &NewOptions, config: &Config) -> CargoResult<()> {
 
     let path = &opts.path;
 
-    if fs::metadata(&path.join("Cargo.toml")).is_ok() {
+    if path.join("Cargo.toml").exists() {
         anyhow::bail!("`cargo init` cannot be run on existing Cargo packages")
     }
 
@@ -428,22 +425,22 @@ pub fn init(opts: &NewOptions, config: &Config) -> CargoResult<()> {
     if version_control == None {
         let mut num_detected_vsces = 0;
 
-        if fs::metadata(&path.join(".git")).is_ok() {
+        if path.join(".git").exists() {
             version_control = Some(VersionControl::Git);
             num_detected_vsces += 1;
         }
 
-        if fs::metadata(&path.join(".hg")).is_ok() {
+        if path.join(".hg").exists() {
             version_control = Some(VersionControl::Hg);
             num_detected_vsces += 1;
         }
 
-        if fs::metadata(&path.join(".pijul")).is_ok() {
+        if path.join(".pijul").exists() {
             version_control = Some(VersionControl::Pijul);
             num_detected_vsces += 1;
         }
 
-        if fs::metadata(&path.join(".fossil")).is_ok() {
+        if path.join(".fossil").exists() {
             version_control = Some(VersionControl::Fossil);
             num_detected_vsces += 1;
         }
@@ -743,10 +740,7 @@ mod tests {
 "
         };
 
-        if !fs::metadata(&path_of_source_file)
-            .map(|x| x.is_file())
-            .unwrap_or(false)
-        {
+        if !path_of_source_file.is_file() {
             paths::write(&path_of_source_file, default_file_content)?;
 
             // Format the newly created source file

src/cargo/ops/cargo_read_manifest.rs

@@ -60,7 +60,7 @@ pub fn read_packages(
             }
 
             // Don't automatically discover packages across git submodules
-            if fs::metadata(&dir.join(".git")).is_ok() {
+            if dir.join(".git").exists() {
                 return Ok(false);
             }
         }

src/cargo/ops/registry.rs

@@ -1,5 +1,5 @@
 use std::collections::{BTreeMap, HashSet};
-use std::fs::{self, File};
+use std::fs::File;
 use std::io::{self, BufRead};
 use std::iter::repeat;
 use std::str;
@@ -233,7 +233,7 @@ fn transmit(
         None => None,
     };
     if let Some(ref file) = *license_file {
-        if fs::metadata(&pkg.root().join(file)).is_err() {
+        if !pkg.root().join(file).exists() {
             bail!("the license file `{}` does not exist", file)
         }
     }

src/cargo/ops/vendor.rs

@@ -8,8 +8,7 @@ use anyhow::bail;
 use serde::Serialize;
 use std::collections::HashSet;
 use std::collections::{BTreeMap, BTreeSet, HashMap};
-use std::fs::{self, File};
-use std::io::Write;
+use std::fs;
 use std::path::{Path, PathBuf};
 
 pub struct VendorOptions<'a> {
@@ -223,7 +222,7 @@ fn sync(
             "files": map,
         });
 
-        File::create(&cksum)?.write_all(json.to_string().as_bytes())?;
+        paths::write(&cksum, json.to_string())?;
     }
 
     for path in to_remove {

src/cargo/sources/path.rs

@@ -373,14 +373,14 @@ impl<'cfg> PathSource<'cfg> {
         is_root: bool,
         filter: &mut dyn FnMut(&Path) -> CargoResult<bool>,
     ) -> CargoResult<()> {
-        if !fs::metadata(&path).map(|m| m.is_dir()).unwrap_or(false) {
+        if !path.is_dir() {
             if (*filter)(path)? {
                 ret.push(path.to_path_buf());
             }
             return Ok(());
         }
         // Don't recurse into any sub-packages that we have.
-        if !is_root && fs::metadata(&path.join("Cargo.toml")).is_ok() {
+        if !is_root && path.join("Cargo.toml").exists() {
             return Ok(());
         }
 

src/cargo/sources/registry/remote.rs

@@ -9,7 +9,7 @@ use lazycell::LazyCell;
 use log::{debug, trace};
 use std::cell::{Cell, Ref, RefCell};
 use std::fmt::Write as FmtWrite;
-use std::fs::{File, OpenOptions};
+use std::fs::{self, File, OpenOptions};
 use std::io::prelude::*;
 use std::io::SeekFrom;
 use std::mem;
@@ -300,10 +300,8 @@ impl<'cfg> RegistryData for RemoteRegistry<'cfg> {
 
         let path = self.cache_path.join(path);
         let path = self.config.assert_package_cache_locked(&path);
-        if let Ok(dst) = File::open(path) {
-            if let Ok(meta) = dst.metadata() {
-                return meta.len() > 0;
-            }
+        if let Ok(meta) = fs::metadata(path) {
+            return meta.len() > 0;
         }
         false
     }

src/cargo/util/command_prelude.rs

@@ -13,7 +13,6 @@ use crate::CargoResult;
 use anyhow::bail;
 use clap::{self, SubCommand};
 use std::ffi::{OsStr, OsString};
-use std::fs;
 use std::path::PathBuf;
 
 pub use crate::core::compiler::CompileMode;
@@ -285,7 +284,7 @@ pub trait ArgMatchesExt {
             if !path.ends_with("Cargo.toml") {
                 anyhow::bail!("the manifest-path must be a path to a Cargo.toml file")
             }
-            if fs::metadata(&path).is_err() {
+            if !path.exists() {
                 anyhow::bail!(
                     "manifest path `{}` does not exist",
                     self._value_of("manifest-path").unwrap()

src/cargo/util/config/mod.rs

@@ -949,8 +949,8 @@ impl Config {
         let possible = dir.join(filename_without_extension);
         let possible_with_extension = dir.join(format!("{}.toml", filename_without_extension));
 
-        if fs::metadata(&possible).is_ok() {
-            if warn && fs::metadata(&possible_with_extension).is_ok() {
+        if possible.exists() {
+            if warn && possible_with_extension.exists() {
                 // We don't want to print a warning if the version
                 // without the extension is just a symlink to the version
                 // WITH an extension, which people may want to do to
@@ -973,7 +973,7 @@ impl Config {
             }
 
             Ok(Some(possible))
-        } else if fs::metadata(&possible_with_extension).is_ok() {
+        } else if possible_with_extension.exists() {
             Ok(Some(possible_with_extension))
         } else {
             Ok(None)

src/cargo/util/cpu.rs

@@ -26,8 +26,7 @@ impl State {
 
 #[cfg(target_os = "linux")]
 mod imp {
-    use std::fs::File;
-    use std::io::{self, Read};
+    use std::{fs, io};
 
     pub struct State {
         user: u64,
@@ -43,8 +42,7 @@ mod imp {
     }
 
     pub fn current() -> io::Result<State> {
-        let mut state = String::new();
-        File::open("/proc/stat")?.read_to_string(&mut state)?;
+        let state = fs::read_to_string("/proc/stat")?;
 
         (|| {
             let mut parts = state.lines().next()?.split_whitespace();

src/cargo/util/important_paths.rs

@@ -1,14 +1,13 @@
 use crate::util::errors::CargoResult;
 use crate::util::paths;
-use std::fs;
 use std::path::{Path, PathBuf};
 
 /// Finds the root `Cargo.toml`.
 pub fn find_root_manifest_for_wd(cwd: &Path) -> CargoResult<PathBuf> {
     let file = "Cargo.toml";
     for current in paths::ancestors(cwd) {
         let manifest = current.join(file);
-        if fs::metadata(&manifest).is_ok() {
+        if manifest.exists() {
             return Ok(manifest);
         }
     }