refactor(powershell): cleanup and simplify code

src/steps/powershell.rs

@@ -1,14 +1,17 @@
 use std::path::PathBuf;
 use std::process::Command;
 
+#[cfg(windows)]
+use color_eyre::eyre::eyre;
 use color_eyre::eyre::Result;
 use rust_i18n::t;
+use tracing::debug;
 
 use crate::command::CommandExt;
 use crate::execution_context::ExecutionContext;
 use crate::step::Step;
 use crate::terminal::{is_dumb, print_separator};
-use crate::utils::{require_option, which};
+use crate::utils::{require_option, which, PathExt};
 
 pub struct Powershell {
     path: Option<PathBuf>,
@@ -19,7 +22,7 @@ impl Powershell {
     pub fn new() -> Self {
         let path = which("pwsh").or_else(|| which("powershell")).filter(|_| !is_dumb());
         let profile = path.as_ref().and_then(Self::get_profile);
-        Powershell { path, profile }
+        Self { path, profile }
     }
 
     #[cfg(windows)]
@@ -35,28 +38,28 @@ impl Powershell {
     }
 
     fn get_profile(path: &PathBuf) -> Option<PathBuf> {
-        Self::execute_with_command(path, &["-NoProfile", "-Command", "Split-Path $PROFILE"], |stdout| {
+        let profile = Self::build_command_internal(path, "Split-Path $PROFILE")
-            Ok(stdout)
+            .output_checked_utf8()
-        })
+            .map(|output| output.stdout.trim().to_string())
-        .ok() // Convert the Result<String> to Option<String>
+            .and_then(|s| PathBuf::from(s).require())
-        .and_then(|s| super::super::utils::PathExt::require(PathBuf::from(s)).ok())
+            .ok();
+        debug!("Found PowerShell profile: {:?}", profile);
+        profile
     }
 
-    fn execute_with_command<F>(path: &PathBuf, args: &[&str], f: F) -> Result<String>
+    /// Builds an "internal" powershell command
-    where
+    fn build_command_internal(path: &PathBuf, cmd: &str) -> Command {
-        F: FnOnce(String) -> Result<String>,
+        let mut command = Command::new(path);
-    {
+
-        let output = Command::new(path).args(args).output_checked_utf8()?;
+        command.args(["-NoProfile", "-Command"]);
-        let stdout = output.stdout.trim().to_string();
+        command.arg(cmd);
-        f(stdout)
+
+        command
     }
 
-    /// Builds a command with common arguments and optional sudo support.
+    /// Builds a "primary" powershell command (uses dry-run if required):
-    fn build_command_internal<'a>(
+    /// {powershell} -NoProfile -Command {cmd}
-        &self,
+    fn build_command<'a>(&self, ctx: &'a ExecutionContext, cmd: &str) -> Result<impl CommandExt + 'a> {
-        ctx: &'a ExecutionContext,
-        additional_args: &[&str],
-    ) -> Result<impl CommandExt + 'a> {
         let powershell = require_option(self.path.as_ref(), t!("Powershell is not installed").to_string())?;
         let executor = &mut ctx.run_type();
         let mut command = if let Some(sudo) = ctx.sudo() {
@@ -73,32 +76,33 @@ impl Powershell {
             self.execution_policy_args_if_needed()?;
         }
 
-        command.args(Self::common_args()).args(additional_args);
+        command.args(["-NoProfile", "-Command"]);
+        command.arg(cmd);
+
         Ok(command)
     }
 
     pub fn update_modules(&self, ctx: &ExecutionContext) -> Result<()> {
         print_separator(t!("Powershell Modules Update"));
-        let mut cmd_args = vec!["-Command", "Update-Module"];
+
+        let mut cmd = "Update-Module".to_string();
 
         if ctx.config().verbose() {
-            cmd_args.push("-Verbose");
+            cmd.push_str(" -Verbose");
         }
         if ctx.config().yes(Step::Powershell) {
-            cmd_args.push("-Force");
+            cmd.push_str(" -Force");
         }
-        println!("{}", t!("Updating modules..."));
-        self.build_command_internal(ctx, &cmd_args)?.status_checked()
-    }
 
-    fn common_args() -> &'static [&'static str] {
+        println!("{}", t!("Updating modules..."));
-        &["-NoProfile"]
+
+        self.build_command(ctx, &cmd)?.status_checked()
     }
 
     #[cfg(windows)]
     pub fn execution_policy_args_if_needed(&self) -> Result<()> {
         if !self.is_execution_policy_set("RemoteSigned") {
-            Err(color_eyre::eyre::eyre!(
+            Err(eyre!(
                 "PowerShell execution policy is too restrictive. \
                 Please run 'Set-ExecutionPolicy RemoteSigned -Scope CurrentUser' in PowerShell \
                 (or use Unrestricted/Bypass if you're sure about the security implications)"
@@ -117,13 +121,15 @@ impl Powershell {
             // Find the index of our target policy
             let target_idx = valid_policies.iter().position(|&p| p == policy);
 
-            let output = Command::new(powershell)
+            let mut command = Self::build_command_internal(powershell, "Get-ExecutionPolicy");
-                .args(["-NoProfile", "-Command", "Get-ExecutionPolicy"])
-                .output_checked_utf8();
 
-            if let Ok(output) = output {
+            let current_policy = command
-                let current_policy = output.stdout.trim();
+                .output_checked_utf8()
+                .map(|output| output.stdout.trim().to_string());
 
+            debug!("Found PowerShell ExecutionPolicy: {:?}", current_policy);
+
+            if let Ok(current_policy) = current_policy {
                 // Find the index of the current policy
                 let current_idx = valid_policies.iter().position(|&p| p == current_policy);
 
@@ -140,74 +146,49 @@ impl Powershell {
 
 #[cfg(windows)]
 impl Powershell {
+    fn has_module(&self, module_name: &str) -> bool {
+        if let Some(powershell) = &self.path {
+            let cmd = format!("Get-Module -ListAvailable {}", module_name);
+
+            return Self::build_command_internal(powershell, &cmd)
+                .output_checked()
+                .map(|output| !output.stdout.trim_ascii().is_empty())
+                .unwrap_or(false);
+        }
+        false
+    }
+
     pub fn supports_windows_update(&self) -> bool {
-        windows::supports_windows_update(self)
+        self.has_module("PSWindowsUpdate")
     }
 
     pub fn windows_update(&self, ctx: &ExecutionContext) -> Result<()> {
-        windows::windows_update(self, ctx)
+        use crate::config::UpdatesAutoReboot;
-    }
 
-    pub fn microsoft_store(&self, ctx: &ExecutionContext) -> Result<()> {
+        debug_assert!(self.supports_windows_update());
-        windows::microsoft_store(self, ctx)
-    }
-}
 
-#[cfg(windows)]
+        let mut cmd = "Import-Module PSWindowsUpdate; Install-WindowsUpdate -Verbose".to_string();
-mod windows {
-    use super::*;
-    use crate::config::UpdatesAutoReboot;
 
-    pub fn supports_windows_update(powershell: &Powershell) -> bool {
-        powershell
-            .path
-            .as_ref()
-            .map(|p| has_module(p, "PSWindowsUpdate"))
-            .unwrap_or(false)
-    }
-
-    #[cfg(windows)]
-    pub fn windows_update(powershell: &Powershell, ctx: &ExecutionContext) -> Result<()> {
-        debug_assert!(supports_windows_update(powershell));
-
-        // Build the full command string
-        let mut command_str = "Install-WindowsUpdate -Verbose".to_string();
         if ctx.config().accept_all_windows_updates() {
-            command_str.push_str(" -AcceptAll");
+            cmd.push_str(" -AcceptAll");
         }
+
         match ctx.config().windows_updates_auto_reboot() {
-            UpdatesAutoReboot::Yes => command_str.push_str(" -AutoReboot"),
+            UpdatesAutoReboot::Yes => cmd.push_str(" -AutoReboot"),
-            UpdatesAutoReboot::No => command_str.push_str(" -IgnoreReboot"),
+            UpdatesAutoReboot::No => cmd.push_str(" -IgnoreReboot"),
             UpdatesAutoReboot::Ask => (), // Prompting is the default for Install-WindowsUpdate
         }
 
-        // Pass the command string using the -Command flag
+        self.build_command(ctx, &cmd)?.status_checked()
-        powershell
-            .build_command_internal(ctx, &["-Command", &command_str])?
-            .status_checked()
     }
 
-    pub fn microsoft_store(powershell: &Powershell, ctx: &ExecutionContext) -> Result<()> {
+    pub fn microsoft_store(&self, ctx: &ExecutionContext) -> Result<()> {
         println!("{}", t!("Scanning for updates..."));
-        let update_command = "Start-Process powershell -Verb RunAs -ArgumentList '-Command', \
+        let cmd = "Start-Process powershell -Verb RunAs -ArgumentList '-Command', \
             '(Get-CimInstance -Namespace \"Root\\cimv2\\mdm\\dmmap\" \
             -ClassName \"MDM_EnterpriseModernAppManagement_AppManagement01\" | \
             Invoke-CimMethod -MethodName UpdateScanMethod).ReturnValue'";
 
-        powershell
+        self.build_command(ctx, cmd)?.status_checked()
-            .build_command_internal(ctx, &["-Command", update_command])?
-            .status_checked()
-    }
-
-    fn has_module(powershell: &PathBuf, command: &str) -> bool {
-        Command::new(powershell)
-            .args([
-                "-NoProfile",
-                "-Command",
-                &format!("Get-Module -ListAvailable {}", command),
-            ])
-            .output_checked_utf8()
-            .map(|result| !result.stdout.is_empty())
-            .unwrap_or(false)
     }
 }