Don't make unquoted file/dir paths absolute (#15878)

# Description

Closes #15848. Currently, we expand unquoted strings to absolute paths
if they are of type `path` or `directory`. This PR makes this no longer
happen. `~`, `.`, and `..+` are still expanded, but a path like
`.../foo/bar/..` will only be turned into `../../foo`, rather than a
full absolute path.

This is mostly so that paths don't get modified before being sent to
known external commands (as in the linked issue). But also, it seems
unnecessary to make all unquoted paths absolute.

After feedback from @132ikl, this PR also makes it so that unquoted
paths are expanded at parse time, so that it matches the runtime
behavior. Previously, `path` expressions were turned into strings
verbatim, while `directory` expressions were treated as not being const.

API change: `nu_path::expansions::expand_path` is now exposed as
`nu_path::expand_path`.

# User-Facing Changes

This has the potential to silently break a lot of scripts. For example,
if someone has a command that expects an already-expanded absolute path,
changes the current working directory, and then passes the path
somewhere, they will now need to use `path expand` to expand the path
themselves before changing the current working directory.

# Tests + Formatting

Just added one test to make sure unquoted `path` arguments aren't made
absolute.

# After Submitting

This is a breaking change, so will need to be mentioned in the release
notes.

crates/nu-engine/src/eval.rs

@@ -1,7 +1,6 @@
 use crate::eval_ir_block;
 #[allow(deprecated)]
 use crate::get_full_help;
-use nu_path::{AbsolutePathBuf, expand_path_with};
 use nu_protocol::{
     BlockId, Config, DataSource, ENV_VARIABLE_ID, IntoPipelineData, PipelineData, PipelineMetadata,
     ShellError, Span, Value, VarId,
@@ -404,45 +403,6 @@ impl Eval for EvalRuntime {
         stack.get_config(engine_state)
     }
 
-    fn eval_filepath(
-        engine_state: &EngineState,
-        stack: &mut Stack,
-        path: String,
-        quoted: bool,
-        span: Span,
-    ) -> Result<Value, ShellError> {
-        if quoted {
-            Ok(Value::string(path, span))
-        } else {
-            let cwd = engine_state.cwd(Some(stack))?;
-            let path = expand_path_with(path, cwd, true);
-
-            Ok(Value::string(path.to_string_lossy(), span))
-        }
-    }
-
-    fn eval_directory(
-        engine_state: Self::State<'_>,
-        stack: &mut Self::MutState,
-        path: String,
-        quoted: bool,
-        span: Span,
-    ) -> Result<Value, ShellError> {
-        if path == "-" {
-            Ok(Value::string("-", span))
-        } else if quoted {
-            Ok(Value::string(path, span))
-        } else {
-            let cwd = engine_state
-                .cwd(Some(stack))
-                .map(AbsolutePathBuf::into_std_path_buf)
-                .unwrap_or_default();
-            let path = expand_path_with(path, cwd, true);
-
-            Ok(Value::string(path.to_string_lossy(), span))
-        }
-    }
-
     fn eval_var(
         engine_state: &EngineState,
         stack: &mut Stack,

crates/nu-engine/src/eval_ir.rs

@@ -1,6 +1,6 @@
 use std::{borrow::Cow, fs::File, sync::Arc};
 
-use nu_path::{AbsolutePathBuf, expand_path_with};
+use nu_path::{expand_path, expand_path_with};
 use nu_protocol::{
     DataSource, DeclId, ENV_VARIABLE_ID, Flag, IntoPipelineData, IntoSpanned, ListStream, OutDest,
     PipelineData, PipelineMetadata, PositionalArg, Range, Record, RegId, ShellError, Signals,
@@ -888,9 +888,7 @@ fn literal_value(
             if *no_expand {
                 Value::string(path, span)
             } else {
-                let cwd = ctx.engine_state.cwd(Some(ctx.stack))?;
-                let path = expand_path_with(path, cwd, true);
-
+                let path = expand_path(path, true);
                 Value::string(path.to_string_lossy(), span)
             }
         }
@@ -904,13 +902,7 @@ fn literal_value(
             } else if *no_expand {
                 Value::string(path, span)
             } else {
-                let cwd = ctx
-                    .engine_state
-                    .cwd(Some(ctx.stack))
-                    .map(AbsolutePathBuf::into_std_path_buf)
-                    .unwrap_or_default();
-                let path = expand_path_with(path, cwd, true);
-
+                let path = expand_path(path, true);
                 Value::string(path.to_string_lossy(), span)
             }
         }

crates/nu-path/src/expansions.rs

@@ -60,7 +60,10 @@ where
     canonicalize(path)
 }
 
-fn expand_path(path: impl AsRef<Path>, need_expand_tilde: bool) -> PathBuf {
+/// Resolve only path components (tilde, ., .., ...+), if possible.
+///
+/// Doesn't convert to absolute form or use syscalls. Output may begin with "../"
+pub fn expand_path(path: impl AsRef<Path>, need_expand_tilde: bool) -> PathBuf {
     let path = if need_expand_tilde {
         expand_tilde(path)
     } else {
@@ -77,7 +80,7 @@ fn expand_path(path: impl AsRef<Path>, need_expand_tilde: bool) -> PathBuf {
 ///
 /// Furthermore, unlike canonicalize(), it does not use sys calls (such as readlink).
 ///
-/// Does not convert to absolute form nor does it resolve symlinks.
+/// Converts to absolute form but does not resolve symlinks.
 /// The input path is specified relative to another path
 pub fn expand_path_with<P, Q>(path: P, relative_to: Q, expand_tilde: bool) -> PathBuf
 where

crates/nu-path/src/lib.rs

@@ -10,7 +10,9 @@ mod tilde;
 mod trailing_slash;
 
 pub use components::components;
-pub use expansions::{canonicalize_with, expand_path_with, expand_to_real_path, locate_in_dirs};
+pub use expansions::{
+    canonicalize_with, expand_path, expand_path_with, expand_to_real_path, locate_in_dirs,
+};
 pub use helpers::{cache_dir, data_dir, home_dir, nu_config_dir};
 pub use path::*;
 pub use tilde::expand_tilde;

crates/nu-protocol/src/eval_base.rs

@@ -1,4 +1,6 @@
 //! Foundational [`Eval`] trait allowing dispatch between const-eval and regular evaluation
+use nu_path::expand_path;
+
 use crate::{
     BlockId, Config, ENV_VARIABLE_ID, GetSpan, Range, Record, ShellError, Span, Value, VarId,
     ast::{
@@ -32,9 +34,23 @@ pub trait Eval {
             Expr::Int(i) => Ok(Value::int(*i, expr_span)),
             Expr::Float(f) => Ok(Value::float(*f, expr_span)),
             Expr::Binary(b) => Ok(Value::binary(b.clone(), expr_span)),
-            Expr::Filepath(path, quoted) => Self::eval_filepath(state, mut_state, path.clone(), *quoted, expr_span),
+            Expr::Filepath(path, quoted) => {
+                if *quoted {
+                    Ok(Value::string(path, expr_span))
+                } else {
+                    let path = expand_path(path, true);
+                    Ok(Value::string(path.to_string_lossy(), expr_span))
+                }
+            }
             Expr::Directory(path, quoted) => {
-                Self::eval_directory(state, mut_state, path.clone(), *quoted, expr_span)
+                if path == "-" {
+                    Ok(Value::string("-", expr_span))
+                } else if *quoted {
+                    Ok(Value::string(path, expr_span))
+                } else {
+                    let path = expand_path(path, true);
+                    Ok(Value::string(path.to_string_lossy(), expr_span))
+                }
             }
             Expr::Var(var_id) => Self::eval_var(state, mut_state, *var_id, expr_span),
             Expr::CellPath(cell_path) => Ok(Value::cell_path(cell_path.clone(), expr_span)),
@@ -329,22 +345,6 @@ pub trait Eval {
 
     fn get_config(state: Self::State<'_>, mut_state: &mut Self::MutState) -> Arc<Config>;
 
-    fn eval_filepath(
-        state: Self::State<'_>,
-        mut_state: &mut Self::MutState,
-        path: String,
-        quoted: bool,
-        span: Span,
-    ) -> Result<Value, ShellError>;
-
-    fn eval_directory(
-        state: Self::State<'_>,
-        mut_state: &mut Self::MutState,
-        path: String,
-        quoted: bool,
-        span: Span,
-    ) -> Result<Value, ShellError>;
-
     fn eval_var(
         state: Self::State<'_>,
         mut_state: &mut Self::MutState,

crates/nu-protocol/src/eval_const.rs

@@ -441,26 +441,6 @@ impl Eval for EvalConst {
         state.get_config().clone()
     }
 
-    fn eval_filepath(
-        _: &StateWorkingSet,
-        _: &mut (),
-        path: String,
-        _: bool,
-        span: Span,
-    ) -> Result<Value, ShellError> {
-        Ok(Value::string(path, span))
-    }
-
-    fn eval_directory(
-        _: &StateWorkingSet,
-        _: &mut (),
-        _: String,
-        _: bool,
-        span: Span,
-    ) -> Result<Value, ShellError> {
-        Err(ShellError::NotAConstant { span })
-    }
-
     fn eval_var(
         working_set: &StateWorkingSet,
         _: &mut (),

tests/repl/test_custom_commands.rs

@@ -279,6 +279,18 @@ fn path_argument_dont_auto_expand_if_double_quoted() -> TestResult {
     run_test(r#"def spam [foo: path] { echo $foo }; spam "~/aa""#, "~/aa")
 }
 
+#[test]
+fn path_argument_dont_make_absolute_if_unquoted() -> TestResult {
+    #[cfg(windows)]
+    let expected = "..\\bar";
+    #[cfg(not(windows))]
+    let expected = "../bar";
+    run_test(
+        r#"def spam [foo: path] { echo $foo }; spam foo/.../bar"#,
+        expected,
+    )
+}
+
 #[test]
 fn dont_allow_implicit_casting_between_glob_and_string() -> TestResult {
     let _ = fail_test(