Repository: FuelLabs/fuelup (rust)

Issue:
uninstalling a custom toolchain does not re-set a default toolchain
### Problem

As described above in title.

### Steps

Assuming starting with another toolchain already installed,

1. fuelup toolchain new my-toolchain
2. fuelup toolchain uninstall my-toolchain
3. fuelup default

`fuelup default` still shows the uninstalled `my-toolchain` as default.

### Possible Solution(s)

We should default back to the `latest` or `nightly` toolchain if possible in that order, and if they don't exist exist we should switch to the first custom toolchain in alphabetical order.

### Notes

_No response_

### Fuelup version

```console
10.2
```


### Installed components

```console
-
```


Pull request:
bug: uninstall+default bug
closes #258 

When uninstalling a toolchain, if the toolchain is the currently active toolchain, `fuelup` should re-set a default.

This is done by doing `list_toolchains()` and selecting, in order of priority, 

an official toolchain (latest, nightly, in order of priority), or
a custom toolchain, arranged alphabetically

This PR also includes a refactor on the `default` command to split it into commands/ops.

Patch:
diff --git a/src/commands/default.rs b/src/commands/default.rs
index 78589d09..d7bfeb55 100644
--- a/src/commands/default.rs
+++ b/src/commands/default.rs
@@ -1,11 +1,7 @@
-use std::str::FromStr;
-
-use crate::toolchain::OfficialToolchainDescription;
-use anyhow::{bail, Result};
+use anyhow::Result;
 use clap::Parser;
-use tracing::info;
 
-use crate::{path::settings_file, settings::SettingsFile, toolchain::Toolchain};
+use crate::ops::fuelup_default;
 
 #[derive(Debug, Parser)]
 pub struct DefaultCommand {
@@ -16,32 +12,5 @@ pub struct DefaultCommand {
 pub fn exec(command: DefaultCommand) -> Result<()> {
     let DefaultCommand { toolchain } = command;
 
-    let current_toolchain = Toolchain::from_settings()?;
-
-    let toolchain = match toolchain {
-        Some(toolchain) => toolchain,
-        None => {
-            info!("{} (default)", current_toolchain.name);
-            return Ok(());
-        }
-    };
-
-    let mut new_default = Toolchain::from_path(&toolchain)?;
-
-    if let Ok(description) = OfficialToolchainDescription::from_str(&toolchain) {
-        new_default = Toolchain::from_path(&description.to_string())?;
-    }
-
-    if !new_default.exists() {
-        bail!("Toolchain with name '{}' does not exist", &new_default.name);
-    };
-
-    let settings = SettingsFile::new(settings_file());
-    settings.with_mut(|s| {
-        s.default_toolchain = Some(new_default.name.clone());
-        Ok(())
-    })?;
-    info!("default toolchain set to '{}'", new_default.name);
-
-    Ok(())
+    fuelup_default::default(toolchain)
 }
diff --git a/src/config.rs b/src/config.rs
index 44da46be..49f7df49 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -5,8 +5,8 @@ use anyhow::{bail, Result};
 use std::io::{self, ErrorKind};
 
 use crate::file::write_file;
+use crate::fmt::format_toolchain_with_target;
 use crate::path::{ensure_dir_exists, hashes_dir, toolchains_dir};
-use crate::target_triple::TargetTriple;
 use crate::toolchain::{OfficialToolchainDescription, RESERVED_TOOLCHAIN_NAMES};
 
 pub struct Config {
@@ -54,13 +54,28 @@ impl Config {
 
     pub(crate) fn list_toolchains(&self) -> Result<Vec<String>> {
         if self.toolchains_dir.is_dir() {
-            let mut toolchains: Vec<String> = fs::read_dir(&self.toolchains_dir)?
+            let mut custom_toolchains: Vec<String> = vec![];
+            let mut toolchains: Vec<String> = vec![];
+
+            for dir_entry in fs::read_dir(&self.toolchains_dir)?
                 .filter_map(io::Result::ok)
                 .filter(|e| e.file_type().map(|f| f.is_dir()).unwrap_or(false))
-                .map(|e| e.file_name().into_string().ok().unwrap_or_default())
-                .collect();
+            {
+                let toolchain = dir_entry.file_name().to_string_lossy().to_string();
+                if RESERVED_TOOLCHAIN_NAMES
+                    .iter()
+                    .any(|t| toolchain == format_toolchain_with_target(t))
+                {
+                    toolchains.push(toolchain)
+                } else {
+                    custom_toolchains.push(toolchain)
+                }
+            }
+
             toolchains.sort();
+            custom_toolchains.sort();
 
+            toolchains.extend(custom_toolchains);
             Ok(toolchains)
         } else {
             Ok(Vec::new())
@@ -74,12 +89,7 @@ impl Config {
                 .filter(|e| {
                     e.file_type().map(|f| f.is_dir()).unwrap_or(false)
                         && RESERVED_TOOLCHAIN_NAMES.iter().any(|t| {
-                            e.file_name().to_string_lossy()
-                                == format!(
-                                    "{}-{}",
-                                    t,
-                                    &TargetTriple::from_host().unwrap_or_default().to_string()
-                                )
+                            e.file_name().to_string_lossy() == format_toolchain_with_target(t)
                         })
                 })
                 .map(|e| e.file_name().into_string().ok().unwrap_or_default())
diff --git a/src/fmt.rs b/src/fmt.rs
index 10c6eae2..03c3864f 100644
--- a/src/fmt.rs
+++ b/src/fmt.rs
@@ -1,6 +1,8 @@
 use std::io::Write;
 use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor};
 
+use crate::target_triple::TargetTriple;
+
 // In the below functions, we ignore the `Result`s of `set_color` and `reset` to allow `write`
 // to work even when those functions fail to set/reset colors, since `StandardStream::stdout` is
 // a wrapper over `std::io::stdout`.
@@ -31,3 +33,11 @@ pub fn print_header(header: &str) {
     writeln!(stdout, "{}", "-".repeat(header.len())).expect("Unexpected error writing to stdout");
     let _ = stdout.reset();
 }
+
+pub fn format_toolchain_with_target(toolchain: &str) -> String {
+    format!(
+        "{}-{}",
+        toolchain,
+        TargetTriple::from_host().unwrap_or_default()
+    )
+}
diff --git a/src/ops/fuelup_default.rs b/src/ops/fuelup_default.rs
new file mode 100644
index 00000000..bb1fda08
--- /dev/null
+++ b/src/ops/fuelup_default.rs
@@ -0,0 +1,39 @@
+use anyhow::{bail, Result};
+use std::str::FromStr;
+use tracing::info;
+
+use crate::{
+    path::settings_file,
+    settings::SettingsFile,
+    toolchain::{OfficialToolchainDescription, Toolchain},
+};
+
+pub fn default(toolchain: Option<String>) -> Result<()> {
+    let current_toolchain = Toolchain::from_settings()?;
+
+    let toolchain = match toolchain {
+        Some(toolchain) => toolchain,
+        None => {
+            info!("{} (default)", current_toolchain.name);
+            return Ok(());
+        }
+    };
+
+    let new_default = match OfficialToolchainDescription::from_str(&toolchain) {
+        Ok(desc) => Toolchain::from_path(&desc.to_string())?,
+        Err(_) => Toolchain::from_path(&toolchain)?,
+    };
+
+    if !new_default.exists() {
+        bail!("Toolchain with name '{}' does not exist", &new_default.name);
+    };
+
+    let settings = SettingsFile::new(settings_file());
+    settings.with_mut(|s| {
+        s.default_toolchain = Some(new_default.name.clone());
+        Ok(())
+    })?;
+    info!("default toolchain set to '{}'", new_default.name);
+
+    Ok(())
+}
diff --git a/src/ops/fuelup_toolchain/uninstall.rs b/src/ops/fuelup_toolchain/uninstall.rs
index e36e8d55..2057f3bd 100644
--- a/src/ops/fuelup_toolchain/uninstall.rs
+++ b/src/ops/fuelup_toolchain/uninstall.rs
@@ -1,11 +1,12 @@
-use anyhow::Result;
+use anyhow::{bail, Result};
 use std::fs;
 use std::str::FromStr;
-use tracing::info;
+use tracing::{error, info};
 
 use crate::{
     commands::toolchain::UninstallCommand,
     config::Config,
+    ops::fuelup_default,
     toolchain::{OfficialToolchainDescription, Toolchain},
 };
 
@@ -14,12 +15,11 @@ pub fn uninstall(command: UninstallCommand) -> Result<()> {
 
     let mut toolchain = Toolchain::from_path(&name)?;
 
+    let config = Config::from_env()?;
     if toolchain.is_official() {
         let description = OfficialToolchainDescription::from_str(&name)?;
         toolchain = Toolchain::from_path(&description.to_string())?;
 
-        let config = Config::from_env()?;
-
         if config.hash_exists(&description) {
             let hash_file = config.hashes_dir().join(description.to_string());
             fs::remove_file(hash_file)?;
@@ -31,8 +31,27 @@ pub fn uninstall(command: UninstallCommand) -> Result<()> {
         return Ok(());
     }
 
-    toolchain.uninstall_self()?;
-    info!("toolchain '{}' uninstalled", &toolchain.name);
+    match toolchain.uninstall_self() {
+        Ok(_) => {
+            info!("toolchain '{}' uninstalled", &toolchain.name);
+            let active_toolchain = Toolchain::from_settings()?;
+            if active_toolchain.name == toolchain.name {
+                for toolchain in config.list_toolchains()? {
+                    if fuelup_default::default(Some(toolchain)).is_ok() {
+                        return Ok(());
+                    }
+                }
+
+                error!(
+                "Could not set default toolchain after uninstallation of currently used toolchain. 
+                Please run `fuelup default <toolchain>` to manually switch your current toolchain."
+                )
+            }
+        }
+        Err(e) => {
+            bail!("Failed to uninstall toolchain '{}': {}", &toolchain.name, e)
+        }
+    };
 
     Ok(())
 }
diff --git a/src/ops/mod.rs b/src/ops/mod.rs
index fbf4ba93..b73f57f2 100644
--- a/src/ops/mod.rs
+++ b/src/ops/mod.rs
@@ -1,5 +1,6 @@
 pub mod fuelup_check;
 pub mod fuelup_component;
+pub mod fuelup_default;
 pub mod fuelup_self;
 pub mod fuelup_show;
 pub mod fuelup_toolchain;


Test patch:
diff --git a/tests/commands.rs b/tests/commands.rs
index 3336d481..90b23d49 100644
--- a/tests/commands.rs
+++ b/tests/commands.rs
@@ -1,5 +1,5 @@
 use anyhow::Result;
-use fuelup::{channel, target_triple::TargetTriple};
+use fuelup::{channel, fmt::format_toolchain_with_target, target_triple::TargetTriple};
 use std::{env, path::Path};
 
 mod testcfg;
@@ -129,10 +129,6 @@ fn fuelup_toolchain_install_date_target_disallowed() -> Result<()> {
 #[test]
 fn fuelup_update() -> Result<()> {
     testcfg::setup(FuelupState::LatestToolchainInstalled, &|cfg| {
-        for entry in cfg.toolchains_dir().read_dir().expect("Could not read dir") {
-            let toolchain_dir = entry.unwrap();
-            println!("{:?}", toolchain_dir);
-        }
         let output = cfg.fuelup(&["update"]);
         assert!(output.status.success());
 
@@ -156,8 +152,7 @@ fn fuelup_toolchain_uninstall() -> Result<()> {
     testcfg::setup(FuelupState::Empty, &|cfg| {
         let toolchains = ["latest", "nightly", &format!("nightly-{}", DATE)];
         for toolchain in toolchains {
-            let toolchain_with_target =
-                format!("{}-{}", toolchain, TargetTriple::from_host().unwrap());
+            let toolchain_with_target = format_toolchain_with_target(toolchain);
             let output = cfg.fuelup(&["toolchain", "uninstall", toolchain]);
             let expected_stdout = format!("toolchain '{}' does not exist\n", toolchain_with_target);
 
@@ -168,18 +163,29 @@ fn fuelup_toolchain_uninstall() -> Result<()> {
     testcfg::setup(FuelupState::AllInstalled, &|cfg| {
         let toolchains = ["latest", "nightly", &format!("nightly-{}", DATE)];
         for toolchain in toolchains {
-            let toolchain_with_target =
-                format!("{}-{}", toolchain, TargetTriple::from_host().unwrap());
+            let toolchain_with_target = format_toolchain_with_target(toolchain);
             let output = cfg.fuelup(&["toolchain", "uninstall", toolchain]);
             let expected_stdout = format!("toolchain '{}' uninstalled\n", toolchain_with_target);
 
-            assert_eq!(output.stdout, expected_stdout);
+            assert!(output.stdout.contains(&expected_stdout));
         }
     })?;
 
     Ok(())
 }
 
+#[test]
+fn fuelup_toolchain_uninstall_active_switches_default() -> Result<()> {
+    testcfg::setup(FuelupState::LatestAndCustomInstalled, &|cfg| {
+        cfg.fuelup(&["toolchain", "uninstall", "latest"]);
+        let stdout = cfg.fuelup(&["default"]).stdout;
+
+        assert_eq!(stdout, "my-toolchain (default)\n")
+    })?;
+
+    Ok(())
+}
+
 #[test]
 fn fuelup_check() -> Result<()> {
     testcfg::setup(FuelupState::Empty, &|cfg| {
@@ -209,9 +215,9 @@ fn fuelup_show() -> Result<()> {
 installed toolchains
 --------------------
 latest-{target}
+nightly-{target}
 my_toolchain (default)
 nightly-2022-08-30-{target}
-nightly-{target}
 
 active toolchain
 -----------------
@@ -260,7 +266,7 @@ fn fuelup_default_empty() -> Result<()> {
 
 #[test]
 fn fuelup_default() -> Result<()> {
-    let latest = format!("latest-{}", TargetTriple::from_host().unwrap());
+    let latest = format_toolchain_with_target("latest");
     testcfg::setup(FuelupState::LatestToolchainInstalled, &|cfg| {
         let output = cfg.fuelup(&["default"]);
         let expected_stdout = format!("{} (default)\n", latest);
@@ -414,8 +420,8 @@ fn fuelup_component_add_with_version() -> Result<()> {
 
 #[test]
 fn fuelup_component_add_disallowed() -> Result<()> {
-    let latest = format!("latest-{}", TargetTriple::from_host().unwrap());
-    let nightly = format!("nightly-{}", TargetTriple::from_host().unwrap());
+    let latest = format_toolchain_with_target("latest");
+    let nightly = format_toolchain_with_target("nightly");
     let nightly_date = format!("nightly-{}-{}", DATE, TargetTriple::from_host().unwrap());
 
     testcfg::setup(FuelupState::LatestToolchainInstalled, &|cfg| {
@@ -470,7 +476,7 @@ You may create a custom toolchain using 'fuelup toolchain new <toolchain>'.
 
 #[test]
 fn fuelup_component_remove_disallowed() -> Result<()> {
-    let latest = format!("latest-{}", TargetTriple::from_host().unwrap());
+    let latest = format_toolchain_with_target("latest");
     let nightly_date = format!("nightly-{}-{}", DATE, TargetTriple::from_host().unwrap());
 
     testcfg::setup(FuelupState::LatestToolchainInstalled, &|cfg| {
@@ -484,7 +490,7 @@ fn fuelup_component_remove_disallowed() -> Result<()> {
 You are currently using '{latest}'.
 
 You may create a custom toolchain using 'fuelup toolchain new <toolchain>'.
-"#
+"#,
         );
         assert_eq!(output.stdout, expected_stdout);
         expect_files_exist(&latest_toolchain_bin_dir, ALL_BINS);