Use Nushell's PATH in which (#4690)

* Make which use our path instead of std::env

* Unignore which test

* Fix wrong fn signature without which feature

crates/nu-command/src/system/which_.rs

@@ -1,5 +1,6 @@
 use itertools::Itertools;
 use log::trace;
+use nu_engine::env;
 use nu_engine::CallExt;
 use nu_protocol::{
     ast::Call,
@@ -8,6 +9,9 @@ use nu_protocol::{
     Spanned, SyntaxShape, Value,
 };
 
+use std::ffi::OsStr;
+use std::path::Path;
+
 #[derive(Clone)]
 pub struct Which;
 
@@ -144,20 +148,35 @@ macro_rules! entry_path {
 }
 
 #[cfg(feature = "which")]
-fn get_first_entry_in_path(item: &str, span: Span) -> Option<Value> {
+fn get_first_entry_in_path(
-    which::which(item)
+    item: &str,
+    span: Span,
+    cwd: impl AsRef<Path>,
+    paths: impl AsRef<OsStr>,
+) -> Option<Value> {
+    which::which_in(item, Some(paths), cwd)
         .map(|path| entry_path!(item, path.to_string_lossy().to_string(), span))
         .ok()
 }
 
 #[cfg(not(feature = "which"))]
-fn get_first_entry_in_path(_: &str, _: Span) -> Option<Value> {
+fn get_first_entry_in_path(
+    _item: &str,
+    _span: Span,
+    _cwd: impl AsRef<Path>,
+    _paths: impl AsRef<OsStr>,
+) -> Option<Value> {
     None
 }
 
 #[cfg(feature = "which")]
-fn get_all_entries_in_path(item: &str, span: Span) -> Vec<Value> {
+fn get_all_entries_in_path(
-    which::which_all(&item)
+    item: &str,
+    span: Span,
+    cwd: impl AsRef<Path>,
+    paths: impl AsRef<OsStr>,
+) -> Vec<Value> {
+    which::which_in_all(&item, Some(paths), cwd)
         .map(|iter| {
             iter.map(|path| entry_path!(item, path.to_string_lossy().to_string(), span))
                 .collect()
@@ -165,7 +184,12 @@ fn get_all_entries_in_path(item: &str, span: Span) -> Vec<Value> {
         .unwrap_or_default()
 }
 #[cfg(not(feature = "which"))]
-fn get_all_entries_in_path(_: &str, _: Span) -> Vec<Value> {
+fn get_all_entries_in_path(
+    _item: &str,
+    _span: Span,
+    _cwd: impl AsRef<Path>,
+    _paths: impl AsRef<OsStr>,
+) -> Vec<Value> {
     vec![]
 }
 
@@ -175,7 +199,13 @@ struct WhichArgs {
     all: bool,
 }
 
-fn which_single(application: Spanned<String>, all: bool, engine_state: &EngineState) -> Vec<Value> {
+fn which_single(
+    application: Spanned<String>,
+    all: bool,
+    engine_state: &EngineState,
+    cwd: impl AsRef<Path>,
+    paths: impl AsRef<OsStr>,
+) -> Vec<Value> {
     let (external, prog_name) = if application.item.starts_with('^') {
         (true, application.item[1..].to_string())
     } else {
@@ -187,7 +217,7 @@ fn which_single(application: Spanned<String>, all: bool, engine_state: &EngineSt
     //program
     //This match handles all different cases
     match (all, external) {
-        (true, true) => get_all_entries_in_path(&prog_name, application.span),
+        (true, true) => get_all_entries_in_path(&prog_name, application.span, cwd, paths),
         (true, false) => {
             let mut output: Vec<Value> = vec![];
             output.extend(get_entries_in_nu(
@@ -196,11 +226,16 @@ fn which_single(application: Spanned<String>, all: bool, engine_state: &EngineSt
                 application.span,
                 false,
             ));
-            output.extend(get_all_entries_in_path(&prog_name, application.span));
+            output.extend(get_all_entries_in_path(
+                &prog_name,
+                application.span,
+                cwd,
+                paths,
+            ));
             output
         }
         (false, true) => {
-            if let Some(entry) = get_first_entry_in_path(&prog_name, application.span) {
+            if let Some(entry) = get_first_entry_in_path(&prog_name, application.span, cwd, paths) {
                 return vec![entry];
             }
             vec![]
@@ -209,7 +244,9 @@ fn which_single(application: Spanned<String>, all: bool, engine_state: &EngineSt
             let nu_entries = get_entries_in_nu(engine_state, &prog_name, application.span, true);
             if !nu_entries.is_empty() {
                 return vec![nu_entries[0].clone()];
-            } else if let Some(entry) = get_first_entry_in_path(&prog_name, application.span) {
+            } else if let Some(entry) =
+                get_first_entry_in_path(&prog_name, application.span, cwd, paths)
+            {
                 return vec![entry];
             }
             vec![]
@@ -237,8 +274,11 @@ fn which(
 
     let mut output = vec![];
 
+    let cwd = env::current_dir_str(engine_state, stack)?;
+    let paths = env::path_str(engine_state, stack, call.head)?;
+
     for app in which_args.applications {
-        let values = which_single(app, which_args.all, engine_state);
+        let values = which_single(app, which_args.all, engine_state, &cwd, &paths);
         output.extend(values);
     }
 

crates/nu-command/tests/commands/which.rs

@@ -68,7 +68,6 @@ fn multiple_reports_of_multiple_alias() {
     assert_eq!(length, 2);
 }
 
-#[ignore]
 #[test]
 fn multiple_reports_of_multiple_defs() {
     let actual = nu!(

crates/nu-engine/src/env.rs

@@ -3,7 +3,7 @@ use std::path::{Path, PathBuf};
 
 use nu_protocol::ast::PathMember;
 use nu_protocol::engine::{EngineState, Stack};
-use nu_protocol::{Config, PipelineData, ShellError, Value};
+use nu_protocol::{Config, PipelineData, ShellError, Span, Value};
 
 use crate::eval_block;
 
@@ -12,6 +12,13 @@ const ENV_SEP: &str = ";";
 #[cfg(not(windows))]
 const ENV_SEP: &str = ":";
 
+#[cfg(windows)]
+const ENV_PATH_NAME: &str = "Path";
+#[cfg(windows)]
+const ENV_PATH_NAME_SECONDARY: &str = "PATH";
+#[cfg(not(windows))]
+const ENV_PATH_NAME: &str = "PATH";
+
 const ENV_CONVERSIONS: &str = "ENV_CONVERSIONS";
 
 enum ConversionResult {
@@ -93,7 +100,6 @@ pub fn current_dir_str(engine_state: &EngineState, stack: &Stack) -> Result<Stri
                 if Path::new(&cwd).is_absolute() {
                     Ok(cwd)
                 } else {
-                    println!("cwd is: {}", cwd);
                     Err(ShellError::LabeledError(
                             "Invalid current directory".to_string(),
                             format!("The 'PWD' environment variable must be set to an absolute path. Found: '{}'", cwd)
@@ -115,6 +121,51 @@ pub fn current_dir(engine_state: &EngineState, stack: &Stack) -> Result<PathBuf,
     current_dir_str(engine_state, stack).map(PathBuf::from)
 }
 
+/// Get the contents of path environment variable as a list of strings
+///
+/// On non-Windows: It will fetch PATH
+/// On Windows: It will try to fetch Path first but if not present, try PATH
+pub fn path_str(
+    engine_state: &EngineState,
+    stack: &Stack,
+    span: Span,
+) -> Result<String, ShellError> {
+    let (pathname, pathval) = match stack.get_env_var(engine_state, ENV_PATH_NAME) {
+        Some(v) => Ok((ENV_PATH_NAME, v)),
+        None => {
+            #[cfg(windows)]
+            match stack.get_env_var(engine_state, ENV_PATH_NAME_SECONDARY) {
+                Some(v) => Ok((ENV_PATH_NAME_SECONDARY, v)),
+                None => Err(ShellError::EnvVarNotFoundAtRuntime(
+                    ENV_PATH_NAME_SECONDARY.to_string(),
+                    span,
+                )),
+            }
+            #[cfg(not(windows))]
+            Err(ShellError::EnvVarNotFoundAtRuntime(
+                ENV_PATH_NAME.to_string(),
+                span,
+            ))
+        }
+    }?;
+
+    if let Value::String { val, .. } = pathval {
+        Ok(val)
+    } else {
+        match get_converted_value(engine_state, stack, pathname, &pathval, "to_string") {
+        ConversionResult::Ok(v) => Ok(v.as_string()?),
+        ConversionResult::ConversionError(e) => Err(e),
+        ConversionResult::GeneralError(e) => Err(e),
+        ConversionResult::CellPathError => Err(ShellError::SpannedLabeledErrorHelp(
+            format!("Missing environment conversion of {} to string", pathname),
+            "Could not convert to string".to_string(),
+            pathval.span()?,
+            "The 'to_string' field of ENV_CONVERSIONS environment variable must be set up correctly".to_string()
+        ))
+        }
+    }
+}
+
 fn get_converted_value(
     engine_state: &EngineState,
     stack: &Stack,