Error cleanup (#46)

* `ParseErrorInner` should **not** be part of the public API

* `LibaryFolders` -> `LibraryFolders`

* Make `Error` non-exhaustive

* Add path to io error

* Add path to parse error

* Remove erroneous print

* Fixup error for missing app installation

src/error.rs

@@ -1,52 +1,82 @@
-use std::fmt;
+use std::{
+    fmt,
+    path::{Path, PathBuf},
+};
 
 pub type Result<T> = std::result::Result<T, Error>;
 
 #[derive(Debug)]
+#[non_exhaustive]
 pub enum Error {
     FailedLocatingSteamDir,
-    // TODO: make more specific and associate with a path?
-    Io(std::io::Error),
-    // TODO: associate with a path
+    Io {
+        inner: std::io::Error,
+        path: PathBuf,
+    },
     Parse {
         kind: ParseErrorKind,
         error: ParseError,
+        path: PathBuf,
     },
     MissingExpectedApp {
         app_id: u32,
     },
+    MissingAppInstall {
+        app_id: u32,
+        path: PathBuf,
+    },
 }
 
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
             Self::FailedLocatingSteamDir => f.write_str("Failed locating the steam dir"),
-            Self::Io(err) => write!(f, "Encountered an I/O error: {}", err),
-            Self::Parse { kind, error } => write!(
+            Self::Io { inner: err, path } => {
+                write!(f, "Encountered an I/O error: {} at {}", err, path.display())
+            }
+            Self::Parse { kind, error, path } => write!(
                 f,
-                "Failed parsing VDF file. File kind: {:?}, Error: {}",
-                kind, error
+                "Failed parsing VDF file. File kind: {:?}, Error: {} at {}",
+                kind,
+                error,
+                path.display(),
             ),
             Self::MissingExpectedApp { app_id } => {
                 write!(f, "Missing expected app with id: {}", app_id)
             }
+            Self::MissingAppInstall { app_id, path } => write!(
+                f,
+                "Missing expected app installation with id: {} at {}",
+                app_id,
+                path.display(),
+            ),
         }
     }
 }
 
 impl std::error::Error for Error {}
 
 impl Error {
-    pub(crate) fn parse(kind: ParseErrorKind, error: ParseError) -> Self {
-        Self::Parse { kind, error }
+    pub(crate) fn io(io: std::io::Error, path: &Path) -> Self {
+        Self::Io {
+            inner: io,
+            path: path.to_owned(),
+        }
+    }
+
+    pub(crate) fn parse(kind: ParseErrorKind, error: ParseError, path: &Path) -> Self {
+        Self::Parse {
+            kind,
+            error,
+            path: path.to_owned(),
+        }
     }
 }
 
 #[derive(Copy, Clone, Debug)]
 #[non_exhaustive]
 pub enum ParseErrorKind {
-    // FIXME(cosmic): this is misspelled ;-;
-    LibaryFolders,
+    LibraryFolders,
     SteamApp,
     Shortcut,
 }
@@ -66,7 +96,7 @@ impl fmt::Display for ParseError {
 }
 
 #[derive(Debug)]
-pub enum ParseErrorInner {
+pub(crate) enum ParseErrorInner {
     Parse(keyvalues_parser::error::Error),
     Serde(keyvalues_serde::error::Error),
     UnexpectedStructure,

src/lib.rs

@@ -115,6 +115,11 @@
 //! }
 //! ```
 
+#![warn(
+	// We're a library after all
+	clippy::print_stderr, clippy::print_stdout
+)]
+
 #[cfg(not(any(target_os = "windows", target_os = "macos", target_os = "linux")))]
 compile_error!("Unsupported operating system!");
 

src/libraryfolders.rs

@@ -40,13 +40,13 @@ use keyvalues_parser::Vdf;
 /// }
 /// ```
 pub fn parse_library_folders(path: &Path) -> Result<LibraryIter> {
-    let parse_error = |err| Error::parse(ParseErrorKind::LibaryFolders, err);
+    let parse_error = |err| Error::parse(ParseErrorKind::LibraryFolders, err, path);
 
     if !path.is_file() {
         return Err(parse_error(ParseError::missing()));
     }
 
-    let contents = fs::read_to_string(path).map_err(Error::Io)?;
+    let contents = fs::read_to_string(path).map_err(|io| Error::io(io, path))?;
     let value = Vdf::parse(&contents)
         .map_err(|err| parse_error(ParseError::from_parser(err)))?
         .value;
@@ -63,12 +63,7 @@ pub fn parse_library_folders(path: &Path) -> Result<LibraryIter> {
                 .and_then(|obj| obj.get("path"))
                 .and_then(|values| values.first())
                 .and_then(|value| value.get_str())
-                .ok_or_else(|| {
-                    Error::parse(
-                        ParseErrorKind::LibaryFolders,
-                        ParseError::unexpected_structure(),
-                    )
-                })
+                .ok_or_else(|| parse_error(ParseError::unexpected_structure()))
                 .map(PathBuf::from)
         })
         .collect::<Result<_>>()?;
@@ -107,8 +102,9 @@ impl Library {
         // Read the manifest files at the library to get an up-to-date list of apps since the
         // values in `libraryfolders.vdf` may be stale
         let mut apps = Vec::new();
-        for entry in fs::read_dir(path.join("steamapps")).map_err(Error::Io)? {
-            let entry = entry.map_err(Error::Io)?;
+        let steamapps = path.join("steamapps");
+        for entry in fs::read_dir(&steamapps).map_err(|io| Error::io(io, &steamapps))? {
+            let entry = entry.map_err(|io| Error::io(io, &steamapps))?;
             if let Some(id) = entry
                 .file_name()
                 .to_str()

src/shortcut.rs

@@ -1,6 +1,11 @@
 //! **WARN:** This is all hacky and should be replaced with proper binary VDF parsing
 
-use std::{fs, io, iter::Peekable, path::Path, slice::Iter};
+use std::{
+    fs, io,
+    iter::Peekable,
+    path::{Path, PathBuf},
+    slice::Iter,
+};
 
 use crate::{error::ParseErrorKind, Error, ParseError, Result};
 
@@ -38,6 +43,7 @@ impl Shortcut {
 }
 
 pub struct ShortcutIter {
+    dir: PathBuf,
     read_dir: fs::ReadDir,
     pending: std::vec::IntoIter<Shortcut>,
 }
@@ -49,11 +55,13 @@ impl ShortcutIter {
             return Err(Error::parse(
                 ParseErrorKind::Shortcut,
                 ParseError::missing(),
+                &user_data,
             ));
         }
 
-        let read_dir = fs::read_dir(user_data).map_err(Error::Io)?;
+        let read_dir = fs::read_dir(&user_data).map_err(|io| Error::io(io, &user_data))?;
         Ok(Self {
+            dir: user_data,
             read_dir,
             pending: Vec::new().into_iter(),
         })
@@ -83,6 +91,7 @@ impl Iterator for ShortcutIter {
                                 break Err(Error::parse(
                                     ParseErrorKind::Shortcut,
                                     ParseError::unexpected_structure(),
+                                    &shortcuts_path,
                                 ));
                             }
                         }
@@ -91,12 +100,12 @@ impl Iterator for ShortcutIter {
                             if err.kind() == io::ErrorKind::NotFound {
                                 continue;
                             } else {
-                                break Err(Error::Io(err));
+                                break Err(Error::io(err, &shortcuts_path));
                             }
                         }
                     }
                 }
-                Err(err) => break Err(Error::Io(err)),
+                Err(err) => break Err(Error::io(err, &self.dir)),
             }
         };
 

src/steamapp.rs

@@ -77,21 +77,33 @@ pub struct SteamApp {
 
 impl SteamApp {
     pub(crate) fn new(library_path: &Path, manifest: &Path) -> Result<Self> {
-        let contents = fs::read_to_string(manifest).map_err(Error::Io)?;
-        let app = Self::from_manifest_str(library_path, &contents)?;
+        let contents = fs::read_to_string(manifest).map_err(|io| Error::io(io, manifest))?;
+        let internal = keyvalues_serde::from_str(&contents).map_err(|err| {
+            Error::parse(
+                ParseErrorKind::SteamApp,
+                ParseError::from_serde(err),
+                manifest,
+            )
+        })?;
+        let app = Self::from_internal_steam_app(internal, library_path);
 
         // Check if the installation path exists and is a valid directory
-        // TODO: lint against printing
-        println!("{:?}", app.path);
+        // TODO: this one check really shapes a lot of the API (in terms of how the data for the
+        // `SteamApp` is resolved. Maybe move this to something like
+        // ```rust
+        // library.resolve_install_dir(&app)?;
+        // ```
         if app.path.is_dir() {
             Ok(app)
         } else {
-            // TODO: app id here
-            Err(Error::MissingExpectedApp { app_id: 1 })
+            Err(Error::MissingAppInstall {
+                app_id: app.app_id,
+                path: app.path,
+            })
         }
     }
 
-    pub(crate) fn from_manifest_str(library_path: &Path, manifest: &str) -> Result<Self> {
+    pub(crate) fn from_internal_steam_app(internal: InternalSteamApp, library_path: &Path) -> Self {
         let InternalSteamApp {
             app_id,
             universe,
@@ -121,10 +133,7 @@ impl SteamApp {
             mounted_config,
             install_scripts,
             shared_depots,
-        } = keyvalues_serde::from_str(manifest).map_err(|err| Error::Parse {
-            kind: ParseErrorKind::SteamApp,
-            error: ParseError::from_serde(err),
-        })?;
+        } = internal;
 
         let path = library_path
             .join("steamapps")
@@ -148,7 +157,7 @@ impl SteamApp {
         let full_validate_before_next_update = full_validate_before_next_update.unwrap_or_default();
         let full_validate_after_next_update = full_validate_after_next_update.unwrap_or_default();
 
-        Ok(Self {
+        Self {
             app_id,
             universe,
             launcher_path,
@@ -177,7 +186,7 @@ impl SteamApp {
             mounted_config,
             install_scripts,
             shared_depots,
-        })
+        }
     }
 }
 
@@ -348,7 +357,7 @@ pub struct Depot {
 }
 
 #[derive(Debug, Deserialize)]
-struct InternalSteamApp {
+pub(crate) struct InternalSteamApp {
     #[serde(rename = "appid")]
     app_id: u32,
     #[serde(rename = "installdir")]
@@ -410,18 +419,23 @@ struct InternalSteamApp {
 mod tests {
     use super::*;
 
+    fn app_from_manifest_str(s: &str, library_path: &Path) -> SteamApp {
+        let internal: InternalSteamApp = keyvalues_serde::from_str(s).unwrap();
+        SteamApp::from_internal_steam_app(internal, library_path)
+    }
+
     #[test]
     fn sanity() {
         let manifest = include_str!("../tests/assets/appmanifest_230410.acf");
-        let app = SteamApp::from_manifest_str(Path::new("C:\\redact\\me"), manifest).unwrap();
+        let app = app_from_manifest_str(manifest, Path::new("C:\\redact\\me"));
         // Redact the path because the path separator used is not cross-platform
         insta::assert_ron_snapshot!(app, { ".path" => "[path]" });
     }
 
     #[test]
     fn more_sanity() {
         let manifest = include_str!("../tests/assets/appmanifest_599140.acf");
-        let app = SteamApp::from_manifest_str(Path::new("/redact/me"), manifest).unwrap();
+        let app = app_from_manifest_str(manifest, Path::new("/redact/me"));
         insta::assert_ron_snapshot!(app, { ".path" => "[path]" });
     }
 }