refactor: Rewrite the shell detection code

crates/omnix-health/src/check/shell.rs

@@ -1,5 +1,8 @@
 use serde::{Deserialize, Serialize};
-use std::{collections::HashMap, path::PathBuf};
+use std::{
+    collections::HashMap,
+    path::{Path, PathBuf},
+};
 
 use crate::traits::{Check, CheckResult, Checkable};
 
@@ -28,55 +31,40 @@ impl Checkable for ShellCheck {
         if !self.enable {
             return vec![];
         }
-        let shell = match Shell::current_shell() {
-            Some(shell) => shell,
-            None => {
-                let msg = "Unsupported shell. Please file an issue at <https://github.com/juspay/omnix/issues>";
+        let user_shell_env = match CurrentUserShellEnv::new() {
+            Ok(shell) => shell,
+            Err(err) => {
+                tracing::error!("Skipping shell dotfile check! {:?}", err);
                 if self.required {
-                    panic!("{}", msg);
+                    panic!("Unable to determine user's shell environment (see above)");
                 } else {
-                    tracing::warn!("Skipping shell dotfile check! {}", msg);
+                    tracing::warn!("Skipping shell dotfile check! (see above)");
                     return vec![];
                 }
             }
         };
 
-        // Iterate over each dotfile and check if it is managed by nix
-        let mut managed: HashMap<PathBuf, PathBuf> = HashMap::new();
+        // Iterate over each dotfile and check if it is managed by Nix
+        let mut managed: HashMap<&'static str, PathBuf> = HashMap::new();
         let mut unmanaged: Vec<PathBuf> = Vec::new();
-        for path in &shell.get_dotfiles() {
-            match resolve_symlink(path) {
-                Ok(Some(target)) => {
-                    if super::direnv::is_path_in_nix_store(&target) {
-                        managed.insert(path.clone(), target);
-                    } else {
-                        // A symlink to a non-nix path
-                        unmanaged.push(path.clone());
-                    }
-                }
-                Ok(None) => {
-                    // Not a symlink; cannot be managed by nix
-                    unmanaged.push(path.clone());
-                }
-                Err(err) => {
-                    panic!("Dotfile {:?} symlink error: {:?}.", path, err);
-                }
+        for (name, path) in user_shell_env.dotfiles {
+            if super::direnv::is_path_in_nix_store(&path) {
+                managed.insert(name, path.clone());
+            } else {
+                unmanaged.push(path.clone());
             }
         }
 
         let title = "Shell dotfiles".to_string();
         let info = format!(
             "Shell={:?}; HOME={:?}; Managed: {:?}; Unmanaged: {:?}",
-            shell,
-            get_home_dir(),
-            managed,
-            unmanaged
+            user_shell_env.shell, user_shell_env.home, managed, unmanaged
         );
         let result = if !managed.is_empty() {
             CheckResult::Green
         } else {
             CheckResult::Red {
-                msg: format!("Default Shell: {:?} is not managed by Nix", shell),
+                msg: format!("Default Shell: {:?} is not managed by Nix", user_shell_env.shell),
                     suggestion: "You can use `home-manager` to manage shell configuration. See <https://github.com/juspay/nixos-unified-template>".to_string(),
             }
         };
@@ -91,37 +79,73 @@ impl Checkable for ShellCheck {
     }
 }
 
+/// The shell environment of the current user
+struct CurrentUserShellEnv {
+    /// The user's home directory
+    home: PathBuf,
+    /// Current shell
+    shell: Shell,
+    /// *Absolute* paths to the dotfiles
+    dotfiles: HashMap<&'static str, PathBuf>,
+}
+
+impl CurrentUserShellEnv {
+    /// Get the current user's shell environment
+    fn new() -> Result<Self, ShellError> {
+        let home = PathBuf::from(std::env::var("HOME")?);
+        let shell = Shell::current_shell()?;
+        let dotfiles = shell.get_dotfiles(&home)?;
+        let v = CurrentUserShellEnv {
+            home,
+            shell,
+            dotfiles,
+        };
+        Ok(v)
+    }
+}
+
+#[derive(thiserror::Error, Debug)]
+enum ShellError {
+    #[error("IO error: {0}")]
+    Io(#[from] std::io::Error),
+
+    #[error("Environment variable error: {0}")]
+    Var(#[from] std::env::VarError),
+
+    #[error("Bad $SHELL value")]
+    BadShellPath,
+
+    #[error("Unsupported shell. Please file an issue at <https://github.com/juspay/omnix/issues>")]
+    UnsupportedShell,
+}
+
 /// An Unix shell
 #[derive(Debug, Serialize, Deserialize, Clone, Hash, Eq, PartialEq)]
 #[serde(rename_all = "lowercase")]
-pub enum Shell {
+enum Shell {
     Zsh,
     Bash,
 }
 
 impl Shell {
     /// Returns the user's current [Shell]
-    fn current_shell() -> Option<Self> {
-        let shell_path =
-            PathBuf::from(std::env::var("SHELL").expect("Environment variable `SHELL` not set"));
+    fn current_shell() -> Result<Self, ShellError> {
+        let shell_path = PathBuf::from(std::env::var("SHELL")?);
         Self::from_path(shell_path)
     }
 
     /// Lookup [Shell] from the given executable path
     /// For example if path is `/bin/zsh`, it would return `Zsh`
-    fn from_path(exe_path: PathBuf) -> Option<Self> {
+    fn from_path(exe_path: PathBuf) -> Result<Self, ShellError> {
         let shell_name = exe_path
             .file_name()
-            .expect("Path does not have a file name component")
+            .ok_or(ShellError::BadShellPath)?
             .to_string_lossy();
 
         match shell_name.as_ref() {
-            "zsh" => Some(Shell::Zsh),
-            "bash" => Some(Shell::Bash),
-            _ => {
-                tracing::warn!("Unrecognized shell: {:?}. Please file an issue at <https://github.com/juspay/omnix/issues>", exe_path);
-                None
-            }
+            "zsh" => Ok(Shell::Zsh),
+            "bash" => Ok(Shell::Bash),
+            _ => Err(ShellError::UnsupportedShell),
         }
     }
 
@@ -134,28 +158,21 @@ impl Shell {
     }
 
     /// Get the currently existing dotfiles under $HOME
-    fn get_dotfiles(&self) -> Vec<PathBuf> {
-        self.dotfile_names()
-            .iter()
-            .map(|dotfile| get_home_dir().join(dotfile))
-            .filter(|path| path.exists())
-            .collect()
-    }
-}
-
-fn get_home_dir() -> PathBuf {
-    PathBuf::from(std::env::var("HOME").expect("Environment variable `HOME` not set"))
-}
-
-/// Resolve symlink to its target path.
-///
-/// Returns `None` if the path is not a symlink. This function can thus be used to both resolve and check if a path is a symlink.
-fn resolve_symlink(path: &PathBuf) -> std::io::Result<Option<PathBuf>> {
-    let meta = std::fs::symlink_metadata(path)?;
-    if meta.file_type().is_symlink() {
-        let target = std::fs::read_link(path)?;
-        Ok(Some(target))
-    } else {
-        Ok(None)
+    ///
+    /// Returned paths will be absolute (i.e., symlinks are resolved).
+    fn get_dotfiles(&self, home_dir: &Path) -> std::io::Result<HashMap<&'static str, PathBuf>> {
+        let mut paths = HashMap::new();
+        for dotfile in self.dotfile_names() {
+            match std::fs::canonicalize(home_dir.join(dotfile)) {
+                Ok(path) => {
+                    paths.insert(dotfile, path);
+                }
+                Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
+                    // If file not found, skip
+                }
+                Err(err) => return Err(err),
+            }
+        }
+        Ok(paths)
     }
 }