Accept filenames in other plugin management commands (#12639)

# Description

This allows the following commands to all accept a filename instead of a
plugin name:

- `plugin use`
- `plugin rm`
- `plugin stop`

Slightly complicated because of the need to also check against
`NU_PLUGIN_DIRS`, but I also fixed some issues with that at the same
time

Requested by @fdncred

# User-Facing Changes

The new commands are updated as described.

# Tests + Formatting

Tests for `NU_PLUGIN_DIRS` handling also made more robust.

- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`

# After Submitting

- [ ] Double check new docs to make sure they describe this capability

crates/nu-cmd-plugin/src/commands/plugin/add.rs

@@ -4,7 +4,7 @@ use nu_engine::{command_prelude::*, current_dir};
 use nu_plugin::{GetPlugin, PersistentPlugin};
 use nu_protocol::{PluginCacheItem, PluginGcConfig, PluginIdentity, RegisteredPlugin};
 
-use crate::util::modify_plugin_file;
+use crate::util::{get_plugin_dirs, modify_plugin_file};
 
 #[derive(Clone)]
 pub struct PluginAdd;
@@ -85,24 +85,10 @@ apparent the next time `nu` is next launched with that plugin cache file.
         let cwd = current_dir(engine_state, stack)?;
 
         // Check the current directory, or fall back to NU_PLUGIN_DIRS
-        let filename_expanded = match nu_path::canonicalize_with(&filename.item, &cwd) {
-            Ok(path) => path,
-            Err(err) => {
-                // Try to find it in NU_PLUGIN_DIRS first, before giving up
-                let mut found = None;
-                if let Some(nu_plugin_dirs) = stack.get_env_var(engine_state, "NU_PLUGIN_DIRS") {
-                    for dir in nu_plugin_dirs.into_list().unwrap_or(vec![]) {
-                        if let Ok(path) = nu_path::canonicalize_with(dir.as_str()?, &cwd)
-                            .and_then(|dir| nu_path::canonicalize_with(&filename.item, dir))
-                        {
-                            found = Some(path);
-                            break;
-                        }
-                    }
-                }
-                found.ok_or(err.into_spanned(filename.span))?
-            }
-        };
+        let filename_expanded = nu_path::locate_in_dirs(&filename.item, &cwd, || {
+            get_plugin_dirs(engine_state, stack)
+        })
+        .err_span(filename.span)?;
 
         let shell_expanded = shell
             .as_ref()

crates/nu-cmd-plugin/src/commands/plugin/rm.rs

@@ -1,6 +1,6 @@
 use nu_engine::command_prelude::*;
 
-use crate::util::modify_plugin_file;
+use crate::util::{canonicalize_possible_filename_arg, modify_plugin_file};
 
 #[derive(Clone)]
 pub struct PluginRm;
@@ -28,7 +28,7 @@ impl Command for PluginRm {
             .required(
                 "name",
                 SyntaxShape::String,
-                "The name of the plugin to remove (not the filename)",
+                "The name, or filename, of the plugin to remove",
             )
             .category(Category::Plugin)
     }
@@ -61,6 +61,11 @@ fixed with `plugin add`.
                 description: "Remove the installed signatures for the `inc` plugin.",
                 result: None,
             },
+            Example {
+                example: "plugin rm ~/.cargo/bin/nu_plugin_inc",
+                description: "Remove the installed signatures for the plugin with the filename `~/.cargo/bin/nu_plugin_inc`.",
+                result: None,
+            },
             Example {
                 example: "plugin rm --plugin-config polars.msgpackz polars",
                 description: "Remove the installed signatures for the `polars` plugin from the \"polars.msgpackz\" plugin cache file.",
@@ -80,8 +85,19 @@ fixed with `plugin add`.
         let custom_path = call.get_flag(engine_state, stack, "plugin-config")?;
         let force = call.has_flag(engine_state, stack, "force")?;
 
+        let filename = canonicalize_possible_filename_arg(engine_state, stack, &name.item);
+
         modify_plugin_file(engine_state, stack, call.head, custom_path, |contents| {
-            if !force && !contents.plugins.iter().any(|p| p.name == name.item) {
+            if let Some(index) = contents
+                .plugins
+                .iter()
+                .position(|p| p.name == name.item || p.filename == filename)
+            {
+                contents.plugins.remove(index);
+                Ok(())
+            } else if force {
+                Ok(())
+            } else {
                 Err(ShellError::GenericError {
                     error: format!("Failed to remove the `{}` plugin", name.item),
                     msg: "couldn't find a plugin with this name in the cache file".into(),
@@ -89,9 +105,6 @@ fixed with `plugin add`.
                     help: None,
                     inner: vec![],
                 })
-            } else {
-                contents.remove_plugin(&name.item);
-                Ok(())
             }
         })?;
 

crates/nu-cmd-plugin/src/commands/plugin/stop.rs

@@ -1,5 +1,7 @@
 use nu_engine::command_prelude::*;
 
+use crate::util::canonicalize_possible_filename_arg;
+
 #[derive(Clone)]
 pub struct PluginStop;
 
@@ -14,7 +16,7 @@ impl Command for PluginStop {
             .required(
                 "name",
                 SyntaxShape::String,
-                "The name of the plugin to stop.",
+                "The name, or filename, of the plugin to stop",
             )
             .category(Category::Plugin)
     }
@@ -30,6 +32,11 @@ impl Command for PluginStop {
                 description: "Stop the plugin named `inc`.",
                 result: None,
             },
+            Example {
+                example: "plugin stop ~/.cargo/bin/nu_plugin_inc",
+                description: "Stop the plugin with the filename `~/.cargo/bin/nu_plugin_inc`.",
+                result: None,
+            },
             Example {
                 example: "plugin list | each { |p| plugin stop $p.name }",
                 description: "Stop all plugins.",
@@ -47,9 +54,12 @@ impl Command for PluginStop {
     ) -> Result<PipelineData, ShellError> {
         let name: Spanned<String> = call.req(engine_state, stack, 0)?;
 
+        let filename = canonicalize_possible_filename_arg(engine_state, stack, &name.item);
+
         let mut found = false;
         for plugin in engine_state.plugins() {
-            if plugin.identity().name() == name.item {
+            let id = &plugin.identity();
+            if id.name() == name.item || id.filename() == filename {
                 plugin.stop()?;
                 found = true;
             }

crates/nu-cmd-plugin/src/commands/plugin/use_.rs

@@ -24,7 +24,7 @@ impl Command for PluginUse {
             .required(
                 "name",
                 SyntaxShape::String,
-                "The name of the plugin to load (not the filename)",
+                "The name, or filename, of the plugin to load",
             )
             .category(Category::Plugin)
     }
@@ -41,6 +41,9 @@ preparing a plugin cache file and passing `--plugin-config`, or using the
 
 If the plugin was already loaded, this will reload the latest definition from
 the cache file into scope.
+
+Note that even if the plugin filename is specified, it will only be loaded if
+it was already previously registered with `plugin add`.
 "#
         .trim()
     }
@@ -70,6 +73,11 @@ the cache file into scope.
                 example: r#"plugin use query"#,
                 result: None,
             },
+            Example {
+                description: "Load the commands for the plugin with the filename `~/.cargo/bin/nu_plugin_query` from $nu.plugin-path",
+                example: r#"plugin use ~/.cargo/bin/nu_plugin_query"#,
+                result: None,
+            },
             Example {
                 description:
                     "Load the commands for the `query` plugin from a custom plugin cache file",

crates/nu-cmd-plugin/src/util.rs

@@ -1,7 +1,10 @@
-use std::fs::{self, File};
+use std::{
+    fs::{self, File},
+    path::PathBuf,
+};
 
 use nu_engine::{command_prelude::*, current_dir};
-use nu_protocol::PluginCacheFile;
+use nu_protocol::{engine::StateWorkingSet, PluginCacheFile};
 
 pub(crate) fn modify_plugin_file(
     engine_state: &EngineState,
@@ -48,3 +51,39 @@ pub(crate) fn modify_plugin_file(
 
     Ok(())
 }
+
+pub(crate) fn canonicalize_possible_filename_arg(
+    engine_state: &EngineState,
+    stack: &Stack,
+    arg: &str,
+) -> PathBuf {
+    // This results in the best possible chance of a match with the plugin item
+    if let Ok(cwd) = nu_engine::current_dir(engine_state, stack) {
+        let path = nu_path::expand_path_with(arg, &cwd, true);
+        // Try to canonicalize
+        nu_path::locate_in_dirs(&path, &cwd, || get_plugin_dirs(engine_state, stack))
+            // If we couldn't locate it, return the expanded path alone
+            .unwrap_or(path)
+    } else {
+        arg.into()
+    }
+}
+
+pub(crate) fn get_plugin_dirs(
+    engine_state: &EngineState,
+    stack: &Stack,
+) -> impl Iterator<Item = String> {
+    // Get the NU_PLUGIN_DIRS constant or env var
+    let working_set = StateWorkingSet::new(engine_state);
+    let value = working_set
+        .find_variable(b"$NU_PLUGIN_DIRS")
+        .and_then(|var_id| working_set.get_constant(var_id).ok().cloned())
+        .or_else(|| stack.get_env_var(engine_state, "NU_PLUGIN_DIRS"));
+
+    // Get all of the strings in the list, if possible
+    value
+        .into_iter()
+        .flat_map(|value| value.into_list().ok())
+        .flatten()
+        .flat_map(|list_item| list_item.coerce_into_string().ok())
+}

crates/nu-parser/src/parse_keywords.rs

@@ -3803,6 +3803,17 @@ pub fn parse_plugin_use(working_set: &mut StateWorkingSet, call: Box<Call>) -> P
             })
             .transpose()?;
 
+        // The name could also be a filename, so try our best to expand it for that match.
+        let filename_query = {
+            let path = nu_path::expand_path_with(&name.item, &cwd, true);
+            path.to_str()
+                .and_then(|path_str| {
+                    find_in_dirs(path_str, working_set, &cwd, Some("NU_PLUGIN_DIRS"))
+                })
+                .map(|parser_path| parser_path.path_buf())
+                .unwrap_or(path)
+        };
+
         // Find the actual plugin config path location. We don't have a const/env variable for this,
         // it either lives in the current working directory or in the script's directory
         let plugin_config_path = if let Some(custom_path) = &plugin_config {
@@ -3842,7 +3853,7 @@ pub fn parse_plugin_use(working_set: &mut StateWorkingSet, call: Box<Call>) -> P
         let plugin_item = contents
             .plugins
             .iter()
-            .find(|plugin| plugin.name == name.item)
+            .find(|plugin| plugin.name == name.item || plugin.filename == filename_query)
             .ok_or_else(|| ParseError::PluginNotFound {
                 name: name.item.clone(),
                 name_span: name.span,

crates/nu-path/src/expansions.rs

@@ -92,3 +92,35 @@ where
     let path = expand_tilde(path);
     expand_ndots(path)
 }
+
+/// Attempts to canonicalize the path against the current directory. Failing that, if
+/// the path is relative, it attempts all of the dirs in `dirs`. If that fails, it returns
+/// the original error.
+pub fn locate_in_dirs<I, P>(
+    filename: impl AsRef<Path>,
+    cwd: impl AsRef<Path>,
+    dirs: impl FnOnce() -> I,
+) -> std::io::Result<PathBuf>
+where
+    I: IntoIterator<Item = P>,
+    P: AsRef<Path>,
+{
+    let filename = filename.as_ref();
+    let cwd = cwd.as_ref();
+    match canonicalize_with(filename, cwd) {
+        Ok(path) => Ok(path),
+        Err(err) => {
+            // Try to find it in `dirs` first, before giving up
+            let mut found = None;
+            for dir in dirs() {
+                if let Ok(path) =
+                    canonicalize_with(dir, cwd).and_then(|dir| canonicalize_with(filename, dir))
+                {
+                    found = Some(path);
+                    break;
+                }
+            }
+            found.ok_or(err)
+        }
+    }
+}

crates/nu-path/src/lib.rs

@@ -4,7 +4,7 @@ mod helpers;
 mod tilde;
 mod util;
 
-pub use expansions::{canonicalize_with, expand_path_with, expand_to_real_path};
+pub use expansions::{canonicalize_with, expand_path_with, expand_to_real_path, locate_in_dirs};
 pub use helpers::{config_dir, config_dir_old, home_dir};
 pub use tilde::expand_tilde;
 pub use util::trim_trailing_slash;

crates/nu-protocol/src/plugin/cache_file/mod.rs

@@ -96,11 +96,6 @@ impl PluginCacheFile {
                 .sort_by(|item1, item2| item1.name.cmp(&item2.name));
         }
     }
-
-    /// Remove a plugin from the plugin cache file by name.
-    pub fn remove_plugin(&mut self, name: &str) {
-        self.plugins.retain_mut(|item| item.name != name)
-    }
 }
 
 /// A single plugin definition from a [`PluginCacheFile`].

tests/plugin_persistence/mod.rs

@@ -72,6 +72,17 @@ fn plugin_process_exits_after_stop() {
     );
 }
 
+#[test]
+fn plugin_stop_can_find_by_filename() {
+    let result = nu_with_plugins!(
+        cwd: ".",
+        plugin: ("nu_plugin_inc"),
+        r#"plugin stop (plugin list | where name == inc).0.filename"#
+    );
+    assert!(result.status.success());
+    assert!(result.err.is_empty());
+}
+
 #[test]
 fn plugin_process_exits_when_nushell_exits() {
     let out = nu_with_plugins!(

tests/plugins/cache_file.rs

@@ -38,6 +38,75 @@ fn plugin_add_then_restart_nu() {
     assert_eq!(r#"["example"]"#, result.out);
 }
 
+#[test]
+fn plugin_add_in_nu_plugin_dirs_const() {
+    let example_plugin_path = example_plugin_path();
+
+    let dirname = example_plugin_path.parent().expect("no parent");
+    let filename = example_plugin_path
+        .file_name()
+        .expect("no file_name")
+        .to_str()
+        .expect("not utf-8");
+
+    let result = nu_with_plugins!(
+        cwd: ".",
+        plugins: [],
+        &format!(
+            r#"
+                $env.NU_PLUGIN_DIRS = null
+                const NU_PLUGIN_DIRS = ['{0}']
+                plugin add '{1}'
+                (
+                    ^$nu.current-exe
+                        --config $nu.config-path
+                        --env-config $nu.env-path
+                        --plugin-config $nu.plugin-path
+                        --commands 'plugin list | get name | to json --raw'
+                )
+            "#,
+            dirname.display(),
+            filename
+        )
+    );
+    assert!(result.status.success());
+    assert_eq!(r#"["example"]"#, result.out);
+}
+
+#[test]
+fn plugin_add_in_nu_plugin_dirs_env() {
+    let example_plugin_path = example_plugin_path();
+
+    let dirname = example_plugin_path.parent().expect("no parent");
+    let filename = example_plugin_path
+        .file_name()
+        .expect("no file_name")
+        .to_str()
+        .expect("not utf-8");
+
+    let result = nu_with_plugins!(
+        cwd: ".",
+        plugins: [],
+        &format!(
+            r#"
+                $env.NU_PLUGIN_DIRS = ['{0}']
+                plugin add '{1}'
+                (
+                    ^$nu.current-exe
+                        --config $nu.config-path
+                        --env-config $nu.env-path
+                        --plugin-config $nu.plugin-path
+                        --commands 'plugin list | get name | to json --raw'
+                )
+            "#,
+            dirname.display(),
+            filename
+        )
+    );
+    assert!(result.status.success());
+    assert_eq!(r#"["example"]"#, result.out);
+}
+
 #[test]
 fn plugin_add_to_custom_path() {
     let example_plugin_path = example_plugin_path();
@@ -192,6 +261,57 @@ fn plugin_rm_from_custom_path() {
     })
 }
 
+#[test]
+fn plugin_rm_using_filename() {
+    let example_plugin_path = example_plugin_path();
+    Playground::setup("plugin rm using filename", |dirs, _playground| {
+        let file = File::create(dirs.test().join("test-plugin-file.msgpackz"))
+            .expect("failed to create file");
+        let mut contents = PluginCacheFile::new();
+
+        contents.upsert_plugin(PluginCacheItem {
+            name: "example".into(),
+            filename: example_plugin_path.clone(),
+            shell: None,
+            data: PluginCacheItemData::Valid { commands: vec![] },
+        });
+
+        contents.upsert_plugin(PluginCacheItem {
+            name: "foo".into(),
+            // this doesn't exist, but it should be ok
+            filename: dirs.test().join("nu_plugin_foo"),
+            shell: None,
+            data: PluginCacheItemData::Valid { commands: vec![] },
+        });
+
+        contents
+            .write_to(file, None)
+            .expect("failed to write plugin file");
+
+        let result = nu!(
+            cwd: dirs.test(),
+            &format!(
+                "plugin rm --plugin-config test-plugin-file.msgpackz '{}'",
+                example_plugin_path.display()
+            )
+        );
+        assert!(result.status.success());
+        assert!(result.err.trim().is_empty());
+
+        // Check the contents after running
+        let contents = PluginCacheFile::read_from(
+            File::open(dirs.test().join("test-plugin-file.msgpackz")).expect("failed to open file"),
+            None,
+        )
+        .expect("failed to read file");
+
+        assert!(!contents.plugins.iter().any(|p| p.name == "example"));
+
+        // Shouldn't remove anything else
+        assert!(contents.plugins.iter().any(|p| p.name == "foo"));
+    })
+}
+
 /// Running nu with a test plugin file that fails to parse on one plugin should just cause a warning
 /// but the others should be loaded
 #[test]
@@ -306,6 +426,27 @@ fn plugin_add_and_then_use() {
     assert_eq!(r#"["example"]"#, result.out);
 }
 
+#[test]
+fn plugin_add_and_then_use_by_filename() {
+    let example_plugin_path = example_plugin_path();
+    let result = nu_with_plugins!(
+        cwd: ".",
+        plugins: [],
+        &format!(r#"
+            plugin add '{0}'
+            (
+                ^$nu.current-exe
+                    --config $nu.config-path
+                    --env-config $nu.env-path
+                    --plugin-config $nu.plugin-path
+                    --commands 'plugin use '{0}'; plugin list | get name | to json --raw'
+            )
+        "#, example_plugin_path.display())
+    );
+    assert!(result.status.success());
+    assert_eq!(r#"["example"]"#, result.out);
+}
+
 #[test]
 fn plugin_add_then_use_with_custom_path() {
     let example_plugin_path = example_plugin_path();