Avoid using std::fs directly (#73)

casey · Oct 12, 2024 · 232fad2 · 232fad2
1 parent 01aaf4f
commit 232fad2
Showing 12 changed files with 73 additions and 117 deletions.
diff --git a/src/filesystem.rs b/src/filesystem.rs
@@ -1,17 +1,30 @@
 use super::*;
 
-pub(crate) fn read_to_string(path: &Utf8Path) -> Result<String> {
-  std::fs::read_to_string(path).context(error::Io { path })
+pub(crate) fn create_dir_all(path: &Utf8Path) -> Result<()> {
+  std::fs::create_dir_all(path).context(error::Io { path })
+}
+
+pub(crate) fn exists(path: &Utf8Path) -> Result<bool> {
+  path.try_exists().context(error::Io { path })
 }
 
 pub(crate) fn metadata(path: &Utf8Path) -> Result<std::fs::Metadata> {
   std::fs::metadata(path).context(error::Io { path })
 }
 
-pub(crate) fn write(path: &Utf8Path, contents: &[u8]) -> Result {
-  std::fs::write(path, contents).context(error::Io { path })
+pub(crate) fn read_to_string(path: impl AsRef<Utf8Path>) -> Result<String> {
+  std::fs::read_to_string(path.as_ref()).context(error::Io {
+    path: path.as_ref(),
+  })
 }
 
-pub(crate) fn exists(path: &Utf8Path) -> Result<bool> {
-  path.try_exists().context(error::Io { path })
+pub(crate) fn read_to_string_opt(path: &Utf8Path) -> Result<Option<String>> {
+  match std::fs::read_to_string(path) {
+    Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(None),
+    result => result.map(Some).context(error::Io { path }),
+  }
+}
+
+pub(crate) fn write(path: &Utf8Path, contents: impl AsRef<[u8]>) -> Result {
+  std::fs::write(path, contents).context(error::Io { path })
 }
diff --git a/src/io_result_ext.rs b/src/io_result_ext.rs
diff --git a/src/main.rs b/src/main.rs
@@ -1,11 +1,11 @@
 use {
   self::{
     arguments::Arguments, bytes::Bytes, display_path::DisplayPath, display_secret::DisplaySecret,
-    entry::Entry, error::Error, hash::Hash, io_result_ext::IoResultExt, lint::Lint,
-    lint_group::LintGroup, list::List, manifest::Manifest, metadata::Metadata, options::Options,
-    owo_colorize_ext::OwoColorizeExt, page::Page, private_key::PrivateKey, public_key::PublicKey,
-    relative_path::RelativePath, signature::Signature, signature_error::SignatureError,
-    style::Style, subcommand::Subcommand, template::Template, utf8_path_ext::Utf8PathExt,
+    entry::Entry, error::Error, hash::Hash, lint::Lint, lint_group::LintGroup, list::List,
+    manifest::Manifest, metadata::Metadata, options::Options, owo_colorize_ext::OwoColorizeExt,
+    page::Page, private_key::PrivateKey, public_key::PublicKey, relative_path::RelativePath,
+    signature::Signature, signature_error::SignatureError, style::Style, subcommand::Subcommand,
+    template::Template, utf8_path_ext::Utf8PathExt,
   },
   blake3::Hasher,
   boilerplate::Boilerplate,
@@ -25,7 +25,7 @@ use {
     collections::{BTreeMap, HashMap, HashSet},
     env,
     fmt::{self, Display, Formatter},
-    fs::{self, File},
+    fs::File,
     io::{self, IsTerminal},
     path::{Path, PathBuf},
     process,
@@ -45,7 +45,6 @@ mod entry;
 mod error;
 mod filesystem;
 mod hash;
-mod io_result_ext;
 mod lint;
 mod lint_group;
 mod list;

diff --git a/src/manifest.rs b/src/manifest.rs
@@ -30,8 +30,8 @@ impl Manifest {
       .context(error::DeserializeManifest { path })
   }
 
-  pub(crate) fn to_json(&self) -> String {
-    serde_json::to_string(self).unwrap()
+  pub(crate) fn store(&self, path: &Utf8Path) -> Result<()> {
+    filesystem::write(path, format!("{}\n", serde_json::to_string(self).unwrap()))
   }
 
   pub(crate) fn total_size(&self) -> u64 {
@@ -45,7 +45,7 @@ mod tests {
 
   #[test]
   fn manifests_in_readme_are_valid() {
-    let readme = fs::read_to_string("README.md").unwrap();
+    let readme = filesystem::read_to_string("README.md").unwrap();
 
     let re = Regex::new(r"(?s)```json(.*?)```").unwrap();
 

diff --git a/src/metadata.rs b/src/metadata.rs
@@ -14,8 +14,8 @@ impl Metadata {
       .context(error::DeserializeMetadata { path })
   }
 
-  pub(crate) fn to_json(&self) -> String {
-    serde_json::to_string(self).unwrap()
+  pub(crate) fn store(&self, path: &Utf8Path) -> Result<()> {
+    filesystem::write(path, format!("{}\n", serde_json::to_string(self).unwrap()))
   }
 }
 

diff --git a/src/private_key.rs b/src/private_key.rs
@@ -37,12 +37,8 @@ impl PrivateKey {
   }
 
   pub(crate) fn load(path: &Utf8Path) -> Result<Self> {
-    let private_key = match fs::read_to_string(path) {
-      Err(err) if err.kind() == io::ErrorKind::NotFound => {
-        return Err(error::PrivateKeyNotFound { path }.build())
-      }
-      result => result.context(error::Io { path })?,
-    };
+    let private_key = filesystem::read_to_string_opt(path)?
+      .ok_or_else(|| error::PrivateKeyNotFound { path }.build())?;
 
     let private_key = private_key
       .trim()
@@ -129,7 +125,7 @@ mod tests {
       .parse::<PrivateKey>()
       .unwrap();
 
-    fs::write(&path, format!(" \t{}\n", key.display_secret())).unwrap();
+    filesystem::write(&path, format!(" \t{}\n", key.display_secret())).unwrap();
 
     assert_eq!(PrivateKey::load(&path).unwrap(), key);
   }

diff --git a/src/public_key.rs b/src/public_key.rs
@@ -27,12 +27,8 @@ impl PublicKey {
   const LEN: usize = ed25519_dalek::PUBLIC_KEY_LENGTH;
 
   pub(crate) fn load(path: &Utf8Path) -> Result<Self> {
-    let public_key = match fs::read_to_string(path) {
-      Err(err) if err.kind() == io::ErrorKind::NotFound => {
-        return Err(error::PublicKeyNotFound { path }.build())
-      }
-      result => result.context(error::Io { path })?,
-    };
+    let public_key = filesystem::read_to_string_opt(path)?
+      .ok_or_else(|| error::PublicKeyNotFound { path }.build())?;
 
     let public_key = public_key
       .trim()
@@ -132,7 +128,7 @@ mod tests {
       .parse::<PublicKey>()
       .unwrap();
 
-    fs::write(&path, format!(" \t{key}\n")).unwrap();
+    filesystem::write(&path, format!(" \t{key}\n")).unwrap();
 
     assert_eq!(PublicKey::load(&path).unwrap(), key);
   }

diff --git a/src/subcommand/create.rs b/src/subcommand/create.rs
@@ -32,17 +32,15 @@ impl Create {
     };
 
     if let Some(path) = &self.metadata {
-      let yaml = fs::read_to_string(path).context(error::Io { path })?;
+      let yaml = filesystem::read_to_string(path)?;
       let template = serde_yaml::from_str::<Template>(&yaml)
         .context(error::DeserializeMetadataTemplate { path })?;
       let path = root.join(Metadata::FILENAME);
       ensure! {
-        self.force || !path.try_exists().context(error::Io { path: &path })?,
+        self.force || !filesystem::exists(&path)?,
         error::MetadataAlreadyExists { path: &path },
       }
-      let metadata = Metadata::from(template);
-      let json = metadata.to_json();
-      fs::write(&path, json).context(error::Io { path: &path })?;
+      Metadata::from(template).store(&path)?;
     }
 
     let cleaned_manifest = current_dir.join(&manifest_path).lexiclean();
@@ -117,7 +115,7 @@ impl Create {
         }
       }
 
-      let metadata = path.metadata().context(error::Io { path })?;
+      let metadata = filesystem::metadata(path)?;
 
       paths.insert(relative, metadata.len());
     }
@@ -187,9 +185,7 @@ impl Create {
       manifest.signatures.insert(public_key, signature);
     }
 
-    fs::write(&manifest_path, manifest.to_json()).context(error::Io {
-      path: manifest_path,
-    })?;
+    manifest.store(&manifest_path)?;
 
     Ok(())
   }

diff --git a/src/subcommand/keygen.rs b/src/subcommand/keygen.rs
@@ -3,30 +3,29 @@ use super::*;
 pub(crate) fn run(option: Options) -> Result {
   let key_dir = option.key_dir()?;
 
-  fs::create_dir_all(&key_dir).context(error::Io { path: &key_dir })?;
+  filesystem::create_dir_all(&key_dir)?;
 
   let private_path = key_dir.join(MASTER_PRIVATE_KEY);
 
   ensure! {
-    !private_path.try_exists().context(error::Io { path: &private_path})?,
+    !filesystem::exists(&private_path)?,
     error::PrivateKeyAlreadyExists { path: private_path },
   }
 
   let public_path = key_dir.join(MASTER_PUBLIC_KEY);
 
   ensure! {
-    !public_path.try_exists().context(error::Io { path: &public_path})?,
+    !filesystem::exists(&public_path)?,
     error::PublicKeyAlreadyExists { path: public_path },
   }
 
   let private_key = PrivateKey::generate();
 
-  fs::write(&private_path, format!("{}\n", private_key.display_secret()))
-    .context(error::Io { path: private_path })?;
+  filesystem::write(&private_path, format!("{}\n", private_key.display_secret()))?;
 
   let public_key = private_key.public_key();
 
-  fs::write(&public_path, format!("{public_key}\n")).context(error::Io { path: public_path })?;
+  filesystem::write(&public_path, format!("{public_key}\n"))?;
 
   Ok(())
 }
diff --git a/src/subcommand/sign.rs b/src/subcommand/sign.rs
@@ -47,7 +47,7 @@ impl Sign {
 
     manifest.signatures.insert(public_key, signature);
 
-    filesystem::write(&path, manifest.to_json().as_bytes())?;
+    manifest.store(&path)?;
 
     Ok(())
   }

diff --git a/src/subcommand/verify.rs b/src/subcommand/verify.rs
@@ -31,20 +31,15 @@ impl Verify {
       root.join(Manifest::FILENAME)
     };
 
-    let json = match fs::read_to_string(&source) {
-      Err(err) if err.kind() == io::ErrorKind::NotFound => {
-        return Err(
-          error::ManifestNotFound {
-            path: self
-              .manifest
-              .as_deref()
-              .unwrap_or(Utf8Path::new(Manifest::FILENAME)),
-          }
-          .build(),
-        );
+    let json = filesystem::read_to_string_opt(&source)?.ok_or_else(|| {
+      error::ManifestNotFound {
+        path: self
+          .manifest
+          .as_deref()
+          .unwrap_or(Utf8Path::new(Manifest::FILENAME)),
       }
-      result => result.context(error::Io { path: &source })?,
-    };
+      .build()
+    })?;
 
     let manifest = serde_json::from_str::<Manifest>(&json).context(error::DeserializeManifest {
       path: Manifest::FILENAME,
@@ -167,10 +162,7 @@ mismatched file: `{path}`
     {
       let path = root.join(Metadata::FILENAME);
 
-      if let Some(json) = fs::read_to_string(&path)
-        .into_option()
-        .context(error::Io { path: &path })?
-      {
+      if let Some(json) = filesystem::read_to_string_opt(&path)? {
         serde_json::from_str::<Metadata>(&json)
           .context(error::DeserializeMetadata { path: &path })?;
       }

diff --git a/tests/create.rs b/tests/create.rs
@@ -11,7 +11,7 @@ fn no_files() {
     .assert()
     .success();
 
-  dir.child("filepack.json").assert("{}");
+  dir.child("filepack.json").assert("{}\n");
 
   Command::cargo_bin("filepack")
     .unwrap()
@@ -35,7 +35,7 @@ fn single_file_omit_root() {
     .success();
 
   dir.child("filepack.json").assert(
-    r#"{"files":{"foo":{"hash":"af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262","size":0}}}"#,
+    r#"{"files":{"foo":{"hash":"af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262","size":0}}}"#.to_owned() + "\n",
   );
 
   Command::cargo_bin("filepack")
@@ -60,7 +60,7 @@ fn single_file() {
     .success();
 
   dir.child("filepack.json").assert(
-    r#"{"files":{"foo":{"hash":"af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262","size":0}}}"#,
+    r#"{"files":{"foo":{"hash":"af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262","size":0}}}"#.to_owned() + "\n",
   );
 
   Command::cargo_bin("filepack")
@@ -85,7 +85,7 @@ fn single_non_empty_file() {
     .success();
 
   dir.child("filepack.json").assert(
-    r#"{"files":{"foo":{"hash":"f2e897eed7d206cd855d441598fa521abc75aa96953e97c030c9612c30c1293d","size":3}}}"#,
+    r#"{"files":{"foo":{"hash":"f2e897eed7d206cd855d441598fa521abc75aa96953e97c030c9612c30c1293d","size":3}}}"#.to_owned() + "\n",
   );
 
   Command::cargo_bin("filepack")
@@ -110,7 +110,7 @@ fn single_file_mmap() {
     .success();
 
   dir.child("filepack.json").assert(
-    r#"{"files":{"foo":{"hash":"af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262","size":0}}}"#,
+    r#"{"files":{"foo":{"hash":"af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262","size":0}}}"#.to_owned() + "\n",
   );
 
   Command::cargo_bin("filepack")
@@ -135,7 +135,7 @@ fn single_file_parallel() {
     .success();
 
   dir.child("filepack.json").assert(
-    r#"{"files":{"foo":{"hash":"af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262","size":0}}}"#,
+    r#"{"files":{"foo":{"hash":"af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262","size":0}}}"#.to_owned() + "\n",
   );
 
   Command::cargo_bin("filepack")
@@ -160,7 +160,7 @@ fn file_in_subdirectory() {
     .success();
 
   dir.child("filepack.json").assert(
-    r#"{"files":{"foo/bar":{"hash":"af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262","size":0}}}"#,
+    r#"{"files":{"foo/bar":{"hash":"af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262","size":0}}}"#.to_owned() + "\n",
   );
 
   Command::cargo_bin("filepack")
@@ -384,7 +384,7 @@ fn force_overwrites_manifest() {
     .success();
 
   dir.child("filepack.json").assert(
-    r#"{"files":{"foo":{"hash":"af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262","size":0}}}"#,
+    r#"{"files":{"foo":{"hash":"af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262","size":0}}}"#.to_string() + "\n",
   );
 
   Command::cargo_bin("filepack")
@@ -410,7 +410,7 @@ fn force_overwrites_manifest_with_destination() {
     .success();
 
   dir.child("foo.json").assert(
-    r#"{"files":{"foo":{"hash":"af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262","size":0}}}"#,
+    r#"{"files":{"foo":{"hash":"af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262","size":0}}}"#.to_owned() + "\n",
   );
 
   Command::cargo_bin("filepack")
@@ -435,7 +435,7 @@ fn with_manifest_path() {
     .success();
 
   dir.child("hello.json").assert(
-    r#"{"files":{"foo":{"hash":"af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262","size":0}}}"#,
+    r#"{"files":{"foo":{"hash":"af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262","size":0}}}"#.to_owned() + "\n",
   );
 
   Command::cargo_bin("filepack")
@@ -462,10 +462,12 @@ fn with_metadata() {
     .success();
 
   dir.child("foo/filepack.json").assert(
-    r#"{"files":{"bar":{"hash":"af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262","size":0},"metadata.json":{"hash":"bf15e6d4fc37be38eb02255ad98c52ac3b54acd1ff7b8de56c343f022eb770de","size":15}}}"#,
+    r#"{"files":{"bar":{"hash":"af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262","size":0},"metadata.json":{"hash":"395190e326d9f4b03fff68cacda59e9c31b9b2a702d46a12f89bfb1ec568c0f1","size":16}}}"#.to_owned() + "\n",
   );
 
-  dir.child("foo/metadata.json").assert(r#"{"title":"Foo"}"#);
+  dir
+    .child("foo/metadata.json")
+    .assert(r#"{"title":"Foo"}"#.to_owned() + "\n");
 
   Command::cargo_bin("filepack")
     .unwrap()
@@ -627,5 +629,7 @@ fn metadata_already_exists() {
     .assert()
     .success();
 
-  dir.child("foo/metadata.json").assert(r#"{"title":"Foo"}"#);
+  dir
+    .child("foo/metadata.json")
+    .assert(r#"{"title":"Foo"}"#.to_owned() + "\n");
 }