ls, rm, cp, open, touch, mkdir: Don't expand tilde if input path is q…

…uoted string or a variable. (#12232)

# Description
Fixes:  #11887
Fixes: #11626

This pr unify the tilde expand behavior over several filesystem relative
commands. It follows the same rule with glob expansion:
|  command  |  result |
| ----------- |  ------ |
| ls ~/aaa  | expand tilde
| ls "~/aaa"  | don't expand tilde
| let f = "~/aaa"; ls $f | don't expand tilde, if you want to: use `ls
($f \| path expand)`
| let f: glob = "~/aaa"; ls $f | expand tilde, they don't expand on
`mkdir`, `touch` comamnd.

Actually I'm not sure for 4th item, currently it's expanding is just
because it followes the same rule with glob expansion.

### About the change
It changes `expand_path_with` to accept a new argument called
`expand_tilde`, if it's true, expand it, if not, just keep it as `~`
itself.

# User-Facing Changes
After this change, `ls "~/aaa"` won't expand tilde.

# Tests + Formatting
Done

crates/nu-cli/src/repl.rs

@@ -734,7 +734,7 @@ fn parse_operation(
  orig = trim_quotes_str(&orig).to_string()
  }
 
- let path = nu_path::expand_path_with(&orig, &cwd);
+ let path = nu_path::expand_path_with(&orig, &cwd, true);
  if looks_like_path(&orig) && path.is_dir() && tokens.0.len() == 1 {
  Ok(ReplOperation::AutoCd {
  cwd,

crates/nu-command/src/env/load_env.rs

@@ -70,7 +70,7 @@ impl Command for LoadEnv {
  if env_var == "PWD" {
  let cwd = current_dir(engine_state, stack)?;
  let rhs = rhs.coerce_into_string()?;
- let rhs = nu_path::expand_path_with(rhs, cwd);
+ let rhs = nu_path::expand_path_with(rhs, cwd, true);
  stack.add_env_var(
  env_var,
  Value::string(rhs.to_string_lossy(), call.head),

crates/nu-command/src/filesystem/ls.rs

@@ -118,12 +118,14 @@ impl Command for Ls {
  let (path, p_tag, absolute_path, quoted) = match pattern_arg {
  Some(pat) => {
  let p_tag = pat.span;
- let p = expand_to_real_path(pat.item.as_ref());
-
- let expanded = nu_path::expand_path_with(&p, &cwd);
+ let expanded = nu_path::expand_path_with(
+ pat.item.as_ref(),
+ &cwd,
+ matches!(pat.item, NuGlob::Expand(..)),
+ );
  // Avoid checking and pushing "*" to the path when directory (do not show contents) flag is true
  if !directory && expanded.is_dir() {
- if permission_denied(&p) {
+ if permission_denied(&expanded) {
  #[cfg(unix)]
  let error_msg = format!(
  "The permissions of {:o} do not allow access for this user",
@@ -151,9 +153,17 @@ impl Command for Ls {
  }
  extra_star_under_given_directory = true;
  }
- let absolute_path = p.is_absolute();
+
+ // it's absolute path if:
+ // 1. pattern is absolute.
+ // 2. pattern can be expanded, and after expands to real_path, it's absolute.
+ // here `expand_to_real_path` call is required, because `~/aaa` should be absolute
+ // path.
+ let absolute_path = Path::new(pat.item.as_ref()).is_absolute()
+ || (pat.item.is_expand()
+ && expand_to_real_path(pat.item.as_ref()).is_absolute());
  (
- p,
+ expanded,
  p_tag,
  absolute_path,
  matches!(pat.item, NuGlob::DoNotExpand(_)),

crates/nu-command/src/filesystem/open.rs

@@ -1,6 +1,5 @@
 use super::util::get_rest_for_glob_pattern;
 use nu_engine::{current_dir, get_eval_block, CallExt};
-use nu_path::expand_to_real_path;
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{Command, EngineState, Stack};
 use nu_protocol::util::BufferedReader;
@@ -153,7 +152,6 @@ impl Command for Open {
  };
 
  let buf_reader = BufReader::new(file);
- let real_path = expand_to_real_path(path);
 
  let file_contents = PipelineData::ExternalStream {
  stdout: Some(RawStream::new(
@@ -166,7 +164,7 @@ impl Command for Open {
  exit_code: None,
  span: call_span,
  metadata: Some(PipelineMetadata {
- data_source: DataSource::FilePath(real_path),
+ data_source: DataSource::FilePath(path.to_path_buf()),
  }),
  trim_end_newline: false,
  };

crates/nu-command/src/filesystem/rm.rs

@@ -157,7 +157,7 @@ fn rm(
 
  for (idx, path) in paths.clone().into_iter().enumerate() {
  if let Some(ref home) = home {
- if expand_path_with(path.item.as_ref(), &currentdir_path)
+ if expand_path_with(path.item.as_ref(), &currentdir_path, path.item.is_expand())
  .to_string_lossy()
  .as_ref()
  == home.as_str()
@@ -242,7 +242,11 @@ fn rm(
  let mut all_targets: HashMap<PathBuf, Span> = HashMap::new();
 
  for target in paths {
- let path = expand_path_with(target.item.as_ref(), &currentdir_path);
+ let path = expand_path_with(
+ target.item.as_ref(),
+ &currentdir_path,
+ target.item.is_expand(),
+ );
  if currentdir_path.to_string_lossy() == path.to_string_lossy()
  || currentdir_path.starts_with(format!("{}{}", target.item, std::path::MAIN_SEPARATOR))
  {
@@ -281,7 +285,11 @@ fn rm(
  }
 
  all_targets
- .entry(nu_path::expand_path_with(f, &currentdir_path))
+ .entry(nu_path::expand_path_with(
+ f,
+ &currentdir_path,
+ target.item.is_expand(),
+ ))
  .or_insert_with(|| target.span);
  }
  Err(e) => {

crates/nu-command/src/filesystem/save.rs

@@ -92,14 +92,14 @@ impl Command for Save {
 
  let path_arg = call.req::<Spanned<PathBuf>>(engine_state, stack, 0)?;
  let path = Spanned {
- item: expand_path_with(path_arg.item, &cwd),
+ item: expand_path_with(path_arg.item, &cwd, true),
  span: path_arg.span,
  };
 
  let stderr_path = call
  .get_flag::<Spanned<PathBuf>>(engine_state, stack, "stderr")?
  .map(|arg| Spanned {
- item: expand_path_with(arg.item, cwd),
+ item: expand_path_with(arg.item, cwd, true),
  span: arg.span,
  });
 

crates/nu-command/src/filesystem/ucp.rs

@@ -183,7 +183,7 @@ impl Command for UCp {
  target.item.to_string(),
  ));
  let cwd = current_dir(engine_state, stack)?;
- let target_path = nu_path::expand_path_with(target_path, &cwd);
+ let target_path = nu_path::expand_path_with(target_path, &cwd, target.item.is_expand());
  if target.item.as_ref().ends_with(PATH_SEPARATOR) && !target_path.is_dir() {
  return Err(ShellError::GenericError {
  error: "is not a directory".into(),
@@ -196,7 +196,7 @@ impl Command for UCp {
 
  // paths now contains the sources
 
- let mut sources: Vec<PathBuf> = Vec::new();
+ let mut sources: Vec<(Vec<PathBuf>, bool)> = Vec::new();
 
  for mut p in paths {
  p.item = p.item.strip_ansi_string_unlikely();
@@ -230,16 +230,19 @@ impl Command for UCp {
  Err(e) => return Err(e),
  }
  }
- sources.append(&mut app_vals);
+ sources.push((app_vals, p.item.is_expand()));
  }
 
  // Make sure to send absolute paths to avoid uu_cp looking for cwd in std::env which is not
  // supported in Nushell
- for src in sources.iter_mut() {
- if !src.is_absolute() {
- *src = nu_path::expand_path_with(&src, &cwd);
+ for (sources, need_expand_tilde) in sources.iter_mut() {
+ for src in sources.iter_mut() {
+ if !src.is_absolute() {
+ *src = nu_path::expand_path_with(&src, &cwd, *need_expand_tilde);
+ }
  }
  }
+ let sources: Vec<PathBuf> = sources.into_iter().flat_map(|x| x.0).collect();
 
  let attributes = make_attributes(preserve)?;
 

crates/nu-command/src/filesystem/umkdir.rs

@@ -1,8 +1,8 @@
-use nu_engine::env::current_dir;
 use nu_engine::CallExt;
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{Command, EngineState, Stack};
 use nu_protocol::{Category, Example, PipelineData, ShellError, Signature, SyntaxShape, Type};
+use std::path::PathBuf;
 
 use uu_mkdir::mkdir;
 #[cfg(not(windows))]
@@ -60,11 +60,10 @@ impl Command for UMkdir {
  call: &Call,
  _input: PipelineData,
  ) -> Result<PipelineData, ShellError> {
- let cwd = current_dir(engine_state, stack)?;
  let mut directories = call
  .rest::<String>(engine_state, stack, 0)?
  .into_iter()
- .map(|dir| nu_path::expand_path_with(dir, &cwd))
+ .map(PathBuf::from)
  .peekable();
 
  let is_verbose = call.has_flag(engine_state, stack, "verbose")?;

crates/nu-command/src/filesystem/umv.rs

@@ -1,9 +1,10 @@
 use super::util::get_rest_for_glob_pattern;
 use nu_engine::current_dir;
 use nu_engine::CallExt;
-use nu_path::{expand_path_with, expand_to_real_path};
+use nu_path::expand_path_with;
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{Command, EngineState, Stack};
+use nu_protocol::NuGlob;
 use nu_protocol::{Category, Example, PipelineData, ShellError, Signature, SyntaxShape, Type};
 use std::ffi::OsString;
 use std::path::PathBuf;
@@ -98,7 +99,8 @@ impl Command for UMv {
  error: "Missing destination path".into(),
  msg: format!(
  "Missing destination path operand after {}",
- expand_path_with(paths[0].item.as_ref(), cwd).to_string_lossy()
+ expand_path_with(paths[0].item.as_ref(), cwd, paths[0].item.is_expand())
+ .to_string_lossy()
  ),
  span: Some(paths[0].span),
  help: None,
@@ -112,7 +114,7 @@ impl Command for UMv {
  label: "Missing file operand".into(),
  span: call.head,
  })?;
- let mut files: Vec<PathBuf> = Vec::new();
+ let mut files: Vec<(Vec<PathBuf>, bool)> = Vec::new();
  for mut p in paths {
  p.item = p.item.strip_ansi_string_unlikely();
  let exp_files: Vec<Result<PathBuf, ShellError>> =
@@ -134,22 +136,26 @@ impl Command for UMv {
  Err(e) => return Err(e),
  }
  }
- files.append(&mut app_vals);
+ files.push((app_vals, p.item.is_expand()));
  }
 
  // Make sure to send absolute paths to avoid uu_cp looking for cwd in std::env which is not
  // supported in Nushell
- for src in files.iter_mut() {
- if !src.is_absolute() {
- *src = nu_path::expand_path_with(&src, &cwd);
+ for (files, need_expand_tilde) in files.iter_mut() {
+ for src in files.iter_mut() {
+ if !src.is_absolute() {
+ *src = nu_path::expand_path_with(&src, &cwd, *need_expand_tilde);
+ }
  }
  }
+ let mut files: Vec<PathBuf> = files.into_iter().flat_map(|x| x.0).collect();
 
  // Add back the target after globbing
- let expanded_target = expand_to_real_path(nu_utils::strip_ansi_string_unlikely(
- spanned_target.item.to_string(),
- ));
- let abs_target_path = expand_path_with(expanded_target, &cwd);
+ let abs_target_path = expand_path_with(
+ nu_utils::strip_ansi_string_unlikely(spanned_target.item.to_string()),
+ &cwd,
+ matches!(spanned_target.item, NuGlob::Expand(..)),
+ );
  files.push(abs_target_path.clone());
  let files = files
  .into_iter()

crates/nu-command/src/path/exists.rs

@@ -137,7 +137,7 @@ fn exists(path: &Path, span: Span, args: &Arguments) -> Value {
  if path.as_os_str().is_empty() {
  return Value::bool(false, span);
  }
- let path = expand_path_with(path, &args.pwd);
+ let path = expand_path_with(path, &args.pwd, true);
  let exists = if args.not_follow_symlink {
  // symlink_metadata returns true if the file/folder exists
  // whether it is a symbolic link or not. Sorry, but returns Err

crates/nu-command/src/path/expand.rs

@@ -152,7 +152,10 @@ fn expand(path: &Path, span: Span, args: &Arguments) -> Value {
  match canonicalize_with(path, &args.cwd) {
  Ok(p) => {
  if args.not_follow_symlink {
- Value::string(expand_path_with(path, &args.cwd).to_string_lossy(), span)
+ Value::string(
+ expand_path_with(path, &args.cwd, true).to_string_lossy(),
+ span,
+ )
  } else {
  Value::string(p.to_string_lossy(), span)
  }
@@ -171,12 +174,18 @@ fn expand(path: &Path, span: Span, args: &Arguments) -> Value {
  ),
  }
  } else if args.not_follow_symlink {
- Value::string(expand_path_with(path, &args.cwd).to_string_lossy(), span)
+ Value::string(
+ expand_path_with(path, &args.cwd, true).to_string_lossy(),
+ span,
+ )
  } else {
  canonicalize_with(path, &args.cwd)
  .map(|p| Value::string(p.to_string_lossy(), span))
  .unwrap_or_else(|_| {
- Value::string(expand_path_with(path, &args.cwd).to_string_lossy(), span)
+ Value::string(
+ expand_path_with(path, &args.cwd, true).to_string_lossy(),
+ span,
+ )
  })
  }
 }

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

@@ -711,3 +711,25 @@ fn list_empty_string() {
  assert!(actual.err.contains("does not exist"));
  })
 }
+
+#[test]
+fn list_with_tilde() {
+ Playground::setup("ls_tilde", |dirs, sandbox| {
+ sandbox
+ .within("~tilde")
+ .with_files(vec![EmptyFile("f1.txt"), EmptyFile("f2.txt")]);
+
+ let actual = nu!(cwd: dirs.test(), "ls '~tilde'");
+ assert!(actual.out.contains("f1.txt"));
+ assert!(actual.out.contains("f2.txt"));
+ assert!(actual.out.contains("~tilde"));
+ let actual = nu!(cwd: dirs.test(), "ls ~tilde");
+ assert!(actual.err.contains("does not exist"));
+
+ // pass variable
+ let actual = nu!(cwd: dirs.test(), "let f = '~tilde'; ls $f");
+ assert!(actual.out.contains("f1.txt"));
+ assert!(actual.out.contains("f2.txt"));
+ assert!(actual.out.contains("~tilde"));
+ })
+}

crates/nu-command/tests/commands/move_/umv.rs

@@ -2,6 +2,7 @@ use nu_test_support::fs::{files_exist_at, Stub::EmptyFile, Stub::FileWithContent
 use nu_test_support::nu;
 use nu_test_support::playground::Playground;
 use rstest::rstest;
+use std::path::Path;
 
 #[test]
 fn moves_a_file() {
@@ -656,3 +657,33 @@ fn test_cp_inside_glob_metachars_dir() {
  assert!(files_exist_at(vec!["test_file.txt"], dirs.test()));
  });
 }
+
+#[test]
+fn mv_with_tilde() {
+ Playground::setup("mv_tilde", |dirs, sandbox| {
+ sandbox.within("~tilde").with_files(vec![
+ EmptyFile("f1.txt"),
+ EmptyFile("f2.txt"),
+ EmptyFile("f3.txt"),
+ ]);
+ sandbox.within("~tilde2");
+
+ // mv file
+ let actual = nu!(cwd: dirs.test(), "mv '~tilde/f1.txt' ./");
+ assert!(actual.err.is_empty());
+ assert!(!files_exist_at(
+ vec![Path::new("f1.txt")],
+ dirs.test().join("~tilde")
+ ));
+ assert!(files_exist_at(vec![Path::new("f1.txt")], dirs.test()));
+
+ // pass variable
+ let actual = nu!(cwd: dirs.test(), "let f = '~tilde/f2.txt'; mv $f ./");
+ assert!(actual.err.is_empty());
+ assert!(!files_exist_at(
+ vec![Path::new("f2.txt")],
+ dirs.test().join("~tilde")
+ ));
+ assert!(files_exist_at(vec![Path::new("f1.txt")], dirs.test()));
+ })
+}