Add support for fish shell and continue adding support for non-posix compliant shells.

.gitignore

@@ -9,3 +9,4 @@
 /local-rustup
 /snapcraft
 flake.lock
+.vscode/

src/cli/self_update/env.fish

@@ -0,0 +1 @@
+contains {cargo_bin} $fish_user_paths; or set -Ua fish_user_paths {cargo_bin}

src/cli/self_update/shell.rs

@@ -1,3 +1,7 @@
+#![warn(clippy::all)]
+#![warn(clippy::pedantic)]
+#![allow(clippy::doc_markdown)]
+
 //! Paths and Unix shells
 //!
 //! MacOS, Linux, FreeBSD, and many other OS model their design on Unix,
@@ -26,7 +30,7 @@
 use std::borrow::Cow;
 use std::path::PathBuf;
 
-use anyhow::{bail, Result};
+use anyhow::{bail, Context, Result};
 
 use super::utils;
 use crate::process;
@@ -71,7 +75,12 @@ pub(crate) fn cargo_home_str() -> Result<Cow<'static, str>> {
 // TODO?: Make a decision on Ion Shell, Power Shell, Nushell
 // Cross-platform non-POSIX shells have not been assessed for integration yet
 fn enumerate_shells() -> Vec<Shell> {
-    vec![Box::new(Posix), Box::new(Bash), Box::new(Zsh)]
+    vec![
+        Box::new(Posix),
+        Box::new(Bash),
+        Box::new(Zsh),
+        Box::new(Fish),
+    ]
 }
 
 pub(crate) fn get_available_shells() -> impl Iterator<Item = Shell> {
@@ -90,6 +99,43 @@ pub(crate) trait UnixShell {
     // Gives rcs that should be written to.
     fn update_rcs(&self) -> Vec<PathBuf>;
 
+    // By default follows POSIX and sources <cargo home>/env
+    fn add_to_path(&self) -> Result<(), anyhow::Error> {
+        let source_cmd = self.source_string()?;
+        let source_cmd_with_newline = format!("\n{}", &source_cmd);
+        for rc in self.update_rcs() {
+            let cmd_to_write = match utils::read_file("rcfile", &rc) {
+                Ok(contents) if contents.contains(&source_cmd) => continue,
+                Ok(contents) if !contents.ends_with('\n') => &source_cmd_with_newline,
+                _ => &source_cmd,
+            };
+
+            utils::append_file("rcfile", &rc, cmd_to_write)
+                .with_context(|| format!("could not amend shell profile: '{}'", rc.display()))?;
+        }
+        Ok(())
+    }
+
+    fn remove_from_path(&self) -> Result<(), anyhow::Error> {
+        let source_bytes = format!("{}\n", self.source_string()?).into_bytes();
+        for rc in self.rcfiles().iter().filter(|rc| rc.is_file()) {
+            let file = utils::read_file("rcfile", rc)?;
+            let file_bytes = file.into_bytes();
+            // FIXME: This is whitespace sensitive where it should not be.
+            if let Some(idx) = file_bytes
+                .windows(source_bytes.len())
+                .position(|w| w == source_bytes.as_slice())
+            {
+                // Here we rewrite the file without the offending line.
+                let mut new_bytes = file_bytes[..idx].to_vec();
+                new_bytes.extend(&file_bytes[idx + source_bytes.len()..]);
+                let new_file = String::from_utf8(new_bytes).unwrap();
+                utils::write_file("rcfile", rc, &new_file)?;
+            }
+        }
+        Ok(())
+    }
+
     // Writes the relevant env file.
     fn env_script(&self) -> ShellScript {
         ShellScript {
@@ -201,14 +247,117 @@ impl UnixShell for Zsh {
     }
 }
 
+struct Fish;
+
+impl Fish {
+    #![allow(non_snake_case)]
+    /// Gets fish vendor config location from `XDG_DATA_DIRS`
+    /// Returns None if `XDG_DATA_DIRS` is not set or if there is no fis`fish/vendor_conf.d`.d directory found
+    pub fn get_vendor_config_from_XDG_DATA_DIRS() -> Option<PathBuf> {
+        // Skip the directory during testing as we don't want to write into the XDG_DATA_DIRS by accident
+        // TODO: Change the definition of XDG_DATA_DIRS in test so that doesn't happen
+
+        // TODO: Set up XDG DATA DIRS in a test to test the location being correct
+        return process()
+            .var("XDG_DATA_DIRS")
+            .map(|var| {
+                var.split(':')
+                    .map(PathBuf::from)
+                    .find(|path| path.ends_with("fish/vendor_conf.d"))
+            })
+            .ok()
+            .flatten();
+    }
+}
+
+impl UnixShell for Fish {
+    fn does_exist(&self) -> bool {
+        matches!(process().var("SHELL"), Ok(sh) if sh.contains("fish"))
+            || matches!(utils::find_cmd(&["fish"]), Some(_))
+    }
+
+    fn rcfiles(&self) -> Vec<PathBuf> {
+        // As per https://fishshell.com/docs/current/language.html#configuration
+        // Vendor config files should be written to `/usr/share/fish/vendor_config.d/`
+        // if that does not exist then it should be written to `/usr/share/fish/vendor_conf.d/`
+        // otherwise it should be written to `$HOME/.config/fish/conf.d/ as per discussions in github issue #478
+        [
+            // #[cfg(test)] // Write to test location so we don't pollute during testing.
+            // utils::home_dir().map(|home| home.join(".config/fish/conf.d/")),
+            Self::get_vendor_config_from_XDG_DATA_DIRS(),
+            Some(PathBuf::from("/usr/share/fish/vendor_conf.d/")),
+            Some(PathBuf::from("/usr/local/share/fish/vendor_conf.d/")),
+            utils::home_dir().map(|home| home.join(".config/fish/conf.d/")),
+        ]
+        .iter_mut()
+        .flatten()
+        .map(|x| x.join("cargo.fish"))
+        .collect()
+    }
+
+    fn update_rcs(&self) -> Vec<PathBuf> {
+        // TODO: Change rcfiles to just read parent dirs
+        self.rcfiles()
+            .into_iter()
+            .filter(|rc| {
+                // We only want to check if the directory exists as in fish we create a new file for every applications env
+                rc.parent().map_or(false, |parent| {
+                    parent
+                        .metadata() // Returns error if path doesn't exist so separate check is not needed
+                        .map_or(false, |metadata| !metadata.permissions().readonly())
+                })
+            })
+            .collect()
+    }
+
+    fn add_to_path(&self) -> Result<(), anyhow::Error> {
+        let source_cmd = self.source_string()?;
+        // Write to first path location if it exists.
+        if let Some(rc) = self.update_rcs().get(0) {
+            let cmd_to_write = match utils::read_file("rcfile", rc) {
+                Ok(contents) if contents.contains(&source_cmd) => return Ok(()),
+                _ => &source_cmd,
+            };
+
+            utils::write_file("fish conf.d", rc, cmd_to_write)
+                .with_context(|| format!("Could not add source to {}", rc.display()))?;
+        }
+        Ok(())
+    }
+
+    fn remove_from_path(&self) -> Result<(), anyhow::Error> {
+        for rc in self.update_rcs() {
+            utils::remove_file("Cargo.fish", &rc)
+                .with_context(|| format!("Could not remove {}", rc.display()))?;
+        }
+        Ok(())
+    }
+
+    fn env_script(&self) -> ShellScript {
+        ShellScript {
+            name: "env.fish",
+            content: include_str!("env.fish"),
+        }
+    }
+
+    fn source_string(&self) -> Result<String> {
+        Ok(format!(
+            "contains {}/bin $fish_user_paths; or set -Ua fish_user_paths {}/bin",
+            cargo_home_str()?,
+            cargo_home_str()?
+        ))
+    }
+}
+
 pub(crate) fn legacy_paths() -> impl Iterator<Item = PathBuf> {
-    let zprofiles = Zsh::zdotdir()
+    let z_profiles = Zsh::zdotdir()
         .into_iter()
         .chain(utils::home_dir())
         .map(|d| d.join(".zprofile"));
-    let profiles = [".bash_profile", ".profile"]
+
+    let bash_profiles = [".bash_profile", ".profile"]
         .iter()
         .filter_map(|rc| utils::home_dir().map(|d| d.join(rc)));
 
-    profiles.chain(zprofiles)
+    bash_profiles.chain(z_profiles)
 }

src/cli/self_update/unix.rs

@@ -54,24 +54,7 @@ pub(crate) fn delete_rustup_and_cargo_home() -> Result<()> {
 
 pub(crate) fn do_remove_from_path() -> Result<()> {
     for sh in shell::get_available_shells() {
-        let source_bytes = format!("{}\n", sh.source_string()?).into_bytes();
-
-        // Check more files for cleanup than normally are updated.
-        for rc in sh.rcfiles().iter().filter(|rc| rc.is_file()) {
-            let file = utils::read_file("rcfile", rc)?;
-            let file_bytes = file.into_bytes();
-            // FIXME: This is whitespace sensitive where it should not be.
-            if let Some(idx) = file_bytes
-                .windows(source_bytes.len())
-                .position(|w| w == source_bytes.as_slice())
-            {
-                // Here we rewrite the file without the offending line.
-                let mut new_bytes = file_bytes[..idx].to_vec();
-                new_bytes.extend(&file_bytes[idx + source_bytes.len()..]);
-                let new_file = String::from_utf8(new_bytes).unwrap();
-                utils::write_file("rcfile", rc, &new_file)?;
-            }
-        }
+        sh.remove_from_path()?;
     }
 
     remove_legacy_paths()?;
@@ -81,19 +64,7 @@ pub(crate) fn do_remove_from_path() -> Result<()> {
 
 pub(crate) fn do_add_to_path() -> Result<()> {
     for sh in shell::get_available_shells() {
-        let source_cmd = sh.source_string()?;
-        let source_cmd_with_newline = format!("\n{}", &source_cmd);
-
-        for rc in sh.update_rcs() {
-            let cmd_to_write = match utils::read_file("rcfile", &rc) {
-                Ok(contents) if contents.contains(&source_cmd) => continue,
-                Ok(contents) if !contents.ends_with('\n') => &source_cmd_with_newline,
-                _ => &source_cmd,
-            };
-
-            utils::append_file("rcfile", &rc, cmd_to_write)
-                .with_context(|| format!("could not amend shell profile: '{}'", rc.display()))?;
-        }
+        sh.add_to_path()?;
     }
 
     remove_legacy_paths()?;

tests/cli-paths.rs

@@ -71,6 +71,31 @@ export PATH="$HOME/apple/bin"
         });
     }
 
+    #[test]
+    fn install_creates_necessary_fish_scripts_new() {
+        clitools::setup(Scenario::Empty, &|config| {
+            let mut cmd = clitools::cmd(config, "rustup-init", &INIT_NONE[1..]);
+            let rcs: Vec<PathBuf> = [".cargo/env.fish", ".config/fish/config.fish"]
+                .iter()
+                .map(|file| config.homedir.join(file))
+                .collect();
+            for file in &rcs {
+                assert!(!file.exists());
+            }
+            cmd.env_remove("CARGO_HOME");
+            cmd.env("SHELL", "fish");
+            cmd.env("XDG_DATA_DIRS", ""); // Overwrite XDG as host shouldn't be affected by the test.
+            assert!(cmd.output().unwrap().status.success());
+            let expected = include_str!("../src/cli/self_update/env.fish")
+                .replace("{cargo_bin}", "$HOME/.cargo/bin");
+
+            assert!(rcs
+                .iter()
+                .filter_map(|rc| fs::read_to_string(rc).ok()) // Read contents of files that exist
+                .any(|rc_content| rc_content.contains(&expected)));
+        })
+    }
+
     #[test]
     fn install_updates_bash_rcs() {
         clitools::setup(Scenario::Empty, &|config| {

tests/mock/clitools.rs

@@ -99,7 +99,8 @@ pub fn setup(s: Scenario, f: &dyn Fn(&mut Config)) {
     env::remove_var("RUSTUP_TOOLCHAIN");
     env::remove_var("SHELL");
     env::remove_var("ZDOTDIR");
-    // clap does its own terminal colour probing, and that isn't
+    env::remove_var("XDG_DATA_DIRS");
+    // clap does it's own terminal colour probing, and that isn't
     // trait-controllable, but it does honour the terminal. To avoid testing
     // claps code, lie about whatever terminal this process was started under.
     env::set_var("TERM", "dumb");
@@ -637,8 +638,7 @@ where
     let mut vars: HashMap<String, String> = HashMap::default();
     self::env(config, &mut vars);
     vars.extend(env.iter().map(|(k, v)| (k.to_string(), v.to_string())));
-    let mut arg_strings: Vec<Box<str>> = Vec::new();
-    arg_strings.push(name.to_owned().into_boxed_str());
+    let mut arg_strings: Vec<Box<str>> = vec![name.to_owned().into_boxed_str()];
     for arg in args {
         arg_strings.push(
             arg.as_ref()