Cleanup lingering todos (#66)

* Remove outdated todos

* Remove some problematic `impl Default`s

* Rename our hidden internal test module

examples/overview.rs

@@ -4,7 +4,6 @@ fn main() {
     let steamdir = SteamDir::locate().unwrap();
     println!("Steam Dir - {:?}", steamdir.path());
 
-    // TODO: use `anyhow` to make error handling here simpler
     for maybe_library in steamdir.libraries().unwrap() {
         match maybe_library {
             Err(err) => eprintln!("Failed reading library: {err}"),

src/tests/helpers.rs → src/__private_tests/helpers.rs

@@ -1,18 +1,14 @@
 //! Some test helpers for setting up isolated dummy steam installations.
 
-// TODO: add a test with an env var flag that runs against your real local steam installation?
-
 use std::{
     collections::BTreeMap,
     convert::{TryFrom, TryInto},
     fs, iter,
     path::{Path, PathBuf},
 };
 
-use crate::{
-    tests::{temp::TempDir, TestError},
-    SteamDir,
-};
+use super::{temp::TempDir, TestError};
+use crate::SteamDir;
 
 use serde::Serialize;
 
@@ -305,7 +301,7 @@ impl SampleApp {
 #[cfg(test)]
 mod test {
     use super::*;
-    use crate::tests::TestResult;
+    use crate::__private_tests::TestResult;
 
     #[test]
     fn sanity() -> TestResult {

src/tests/legacy.rs → src/__private_tests/legacy.rs

@@ -1,10 +1,8 @@
-use crate::{
-    tests::{
-        helpers::{expect_test_env, SampleApp},
-        TestResult,
-    },
-    Error,
+use super::{
+    helpers::{expect_test_env, SampleApp},
+    TestResult,
 };
+use crate::Error;
 
 static GMOD_ID: u32 = SampleApp::GarrysMod.id();
 

src/tests/temp.rs → src/__private_tests/temp.rs

@@ -5,7 +5,7 @@
 
 use std::{collections, env, fs, hash, path};
 
-use crate::tests::TestError;
+use super::TestError;
 
 #[derive(Debug)]
 pub struct TempDir(Option<path::PathBuf>);
@@ -15,7 +15,6 @@ impl TempDir {
         let mut dir = env::temp_dir();
         let random_name = format!("steamlocate-test-{:x}", random_seed());
         dir.push(random_name);
-        // TODO: could retry on failure
         fs::create_dir_all(&dir)?;
         Ok(Self(Some(dir)))
     }

src/tests/tests.rs → src/__private_tests/tests.rs

@@ -1,4 +1,4 @@
-use crate::tests::{
+use super::{
     helpers::{SampleApp, TempSteamDir},
     TestResult,
 };

src/app.rs

@@ -65,7 +65,7 @@ pub struct App {
     pub universe: Option<Universe>,
     pub launcher_path: Option<PathBuf>,
     pub state_flags: Option<StateFlags>,
-    // TODO: Need to handle this for serializing too before `App` can `impl Serialize`
+    // NOTE: Need to handle this for serializing too before `App` can `impl Serialize`
     #[serde(
         alias = "lastupdated",
         default,
@@ -163,15 +163,9 @@ impl StateFlags {
     }
 }
 
-#[derive(Clone, Debug, Default)]
+#[derive(Clone, Debug)]
 pub struct StateFlagIter(Option<StateFlagIterInner>);
 
-impl StateFlagIter {
-    fn from_valid(valid: ValidIter) -> Self {
-        Self(Some(StateFlagIterInner::Valid(valid)))
-    }
-}
-
 impl From<StateFlags> for StateFlagIter {
     fn from(state: StateFlags) -> Self {
         Self(Some(state.into()))
@@ -186,22 +180,21 @@ impl Iterator for StateFlagIter {
         // - None indicates the iterator is done (trap state)
         // - Invalid will emit invalid once and finish
         // - Valid will pull on the inner iterator till it's finished
-        let current = std::mem::take(self);
-        let (next, ret) = match current.0? {
-            StateFlagIterInner::Invalid => (Self::default(), StateFlag::Invalid),
+        let current = std::mem::take(&mut self.0);
+        let (next, ret) = match current? {
+            StateFlagIterInner::Invalid => (None, StateFlag::Invalid),
             StateFlagIterInner::Valid(mut valid) => {
                 let ret = valid.next()?;
-                (Self::from_valid(valid), ret)
+                (Some(StateFlagIterInner::Valid(valid)), ret)
             }
         };
-        *self = next;
+        self.0 = next;
         Some(ret)
     }
 }
 
-#[derive(Clone, Debug, Default)]
+#[derive(Clone, Debug)]
 enum StateFlagIterInner {
-    #[default]
     Invalid,
     Valid(ValidIter),
 }
@@ -337,12 +330,6 @@ impl From<u64> for AllowOtherDownloadsWhileRunning {
     }
 }
 
-impl Default for AllowOtherDownloadsWhileRunning {
-    fn default() -> Self {
-        Self::UseGlobalSetting
-    }
-}
-
 impl_deserialize_from_u64!(AllowOtherDownloadsWhileRunning);
 
 #[derive(Debug, Clone, PartialEq)]
@@ -365,13 +352,6 @@ impl From<u64> for AutoUpdateBehavior {
     }
 }
 
-// TODO: Maybe don't have these defaults?
-impl Default for AutoUpdateBehavior {
-    fn default() -> Self {
-        Self::KeepUpToDate
-    }
-}
-
 impl_deserialize_from_u64!(AutoUpdateBehavior);
 
 #[derive(Debug, Clone, PartialEq)]

src/error.rs

@@ -139,7 +139,6 @@ impl fmt::Display for BackendError {
 
 // TODO: move all this conditional junk into different modules, so that I don't have to keep
 // repeating it everywhere
-// TODO: ^^
 #[derive(Clone, Debug)]
 #[cfg(all(feature = "locate", target_os = "windows"))]
 struct BackendErrorInner(std::sync::Arc<io::Error>);

src/lib.rs

@@ -28,7 +28,7 @@
 //! # /*
 //! let steam_dir = steamlocate::SteamDir::locate()?;
 //! # */
-//! # let temp_steam_dir = steamlocate::tests::helpers::expect_test_env();
+//! # let temp_steam_dir = steamlocate::__private_tests::helpers::expect_test_env();
 //! # let steam_dir = temp_steam_dir.steam_dir();
 //! println!("Steam installation - {}", steam_dir.path().display());
 //! // ^^ prints something like `Steam installation - C:\Program Files (x86)\Steam`
@@ -40,7 +40,7 @@
 //! assert_eq!(garrys_mod.name.as_ref().unwrap(), "Garry's Mod");
 //! println!("{garrys_mod:#?}");
 //! // ^^ prints something like vv
-//! # Ok::<_, steamlocate::tests::TestError>(())
+//! # Ok::<_, steamlocate::__private_tests::TestError>(())
 //! ```
 //! ```ignore
 //! App {
@@ -61,7 +61,7 @@
 //! # /*
 //! let steam_dir = steamlocate::SteamDir::locate()?;
 //! # */
-//! # let temp_steam_dir = steamlocate::tests::helpers::expect_test_env();
+//! # let temp_steam_dir = steamlocate::__private_tests::helpers::expect_test_env();
 //! # let steam_dir = temp_steam_dir.steam_dir();
 //!
 //! for library in steam_dir.libraries()? {
@@ -73,7 +73,7 @@
 //!         println!("    App {} - {:?}", app.app_id, app.name);
 //!     }
 //! }
-//! # Ok::<_, steamlocate::tests::TestError>(())
+//! # Ok::<_, steamlocate::__private_tests::TestError>(())
 //! ```
 //!
 //! On my laptop this prints
@@ -106,7 +106,7 @@ pub mod shortcut;
 // NOTE: exposed publicly, so that we can use them in doctests
 /// Not part of the public API >:V
 #[doc(hidden)]
-pub mod tests; // TODO: rename this since it may leak out in compiler error messages
+pub mod __private_tests;
 
 use std::collections::HashMap;
 use std::fs;
@@ -149,7 +149,7 @@ pub struct ReadmeDoctests;
 /// # /*
 /// let steam_dir = SteamDir::locate()?;
 /// # */
-/// # let temp_steam_dir = steamlocate::tests::helpers::expect_test_env();
+/// # let temp_steam_dir = steamlocate::__private_tests::helpers::expect_test_env();
 /// # let steam_dir = temp_steam_dir.steam_dir();
 /// assert!(steam_dir.path().ends_with("Steam"));
 /// ```
@@ -180,7 +180,7 @@ impl SteamDir {
     ///
     /// # Example
     /// ```
-    /// # let temp_steam_dir = steamlocate::tests::helpers::expect_test_env();
+    /// # let temp_steam_dir = steamlocate::__private_tests::helpers::expect_test_env();
     /// # let steam_dir = temp_steam_dir.steam_dir();
     /// # /*
     /// let steam_dir = SteamDir::locate()?;
@@ -189,7 +189,7 @@ impl SteamDir {
     /// let (warframe, library) = steam_dir.find_app(WARFRAME)?.unwrap();
     /// assert_eq!(warframe.app_id, WARFRAME);
     /// assert!(library.app_ids().contains(&warframe.app_id));
-    /// # Ok::<_, steamlocate::tests::TestError>(())
+    /// # Ok::<_, steamlocate::__private_tests::TestError>(())
     /// ```
     pub fn find_app(&self, app_id: u32) -> Result<Option<(App, Library)>> {
         // Search for the `app_id` in each library

src/library.rs

@@ -135,7 +135,6 @@ impl Library {
         &self.path
     }
 
-    // TODO: if this was sorted then we could locate single apps faster
     pub fn app_ids(&self) -> &[u32] {
         &self.apps
     }
@@ -163,14 +162,14 @@ impl Library {
     ///
     /// ```
     /// # use std::path::Path;
-    /// # let temp_steam_dir = steamlocate::tests::helpers::expect_test_env();
+    /// # let temp_steam_dir = steamlocate::__private_tests::helpers::expect_test_env();
     /// # let steam_dir = temp_steam_dir.steam_dir();
     /// const GRAVEYARD_KEEPER: u32 = 599_140;
     /// let (graveyard_keeper, library) = steam_dir.find_app(GRAVEYARD_KEEPER)?.unwrap();
     /// let app_dir = library.resolve_app_dir(&graveyard_keeper);
     /// let expected_rel_path = Path::new("steamapps").join("common").join("Graveyard Keeper");
     /// assert!(app_dir.ends_with(expected_rel_path));
-    /// # Ok::<_, steamlocate::tests::TestError>(())
+    /// # Ok::<_, steamlocate::__private_tests::TestError>(())
     /// ```
     pub fn resolve_app_dir(&self, app: &App) -> PathBuf {
         self.path