Allow ls works inside dir with [] brackets (#12625)

# Description
Fixes: #12429

To fix the issue, we need to pass the `input pattern` itself to
`glob_from` function, but currently on latest main, nushell pass
`expanded path of input pattern` to `glob_from` function.
It causes globbing failed if expanded path includes `[]` brackets.

It's a pity that I have to duplicate `nu_engine::glob_from` function
into `ls`, because `ls` might convert from `NuGlob::NotExpand` to
`NuGlob::Expand`, in that case, `nu_engine::glob_from` won't work if
user want to ls for a directory which includes tilde:
```
mkdir "~abc"
ls "~abc"
```
So I need to duplicate `glob_from` function and pass original
`expand_tilde` information.

# User-Facing Changes
Nan

# Tests + Formatting
Done

# After Submitting
Nan

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

@@ -1,9 +1,10 @@
 use super::util::get_rest_for_glob_pattern;
 use crate::{DirBuilder, DirInfo};
 use chrono::{DateTime, Local, LocalResult, TimeZone, Utc};
+use nu_engine::glob_from;
 #[allow(deprecated)]
 use nu_engine::{command_prelude::*, env::current_dir};
-use nu_glob::{MatchOptions, Pattern};
+use nu_glob::MatchOptions;
 use nu_path::expand_to_real_path;
 use nu_protocol::{DataSource, NuGlob, PipelineMetadata};
 use pathdiff::diff_paths;
@@ -31,6 +32,8 @@ struct Args {
     call_span: Span,
 }
 
+const GLOB_CHARS: &[char] = &['*', '?', '['];
+
 impl Command for Ls {
     fn name(&self) -> &str {
         "ls"
@@ -235,24 +238,20 @@ fn ls_for_one_pattern(
         }
     };
 
-    // it indicates we need to append an extra '*' after pattern for listing given directory
+    let mut just_read_dir = false;
-    // Example: 'ls directory' -> 'ls directory/*'
+    let p_tag: Span = pattern_arg.as_ref().map(|p| p.span).unwrap_or(call_span);
-    let mut extra_star_under_given_directory = false;
+    let (pattern_arg, absolute_path) = match pattern_arg {
-    let (path, p_tag, absolute_path, quoted) = match pattern_arg {
         Some(pat) => {
-            let p_tag = pat.span;
+            // expand with cwd here is only used for checking
-            let expanded = nu_path::expand_path_with(
+            let tmp_expanded =
-                pat.item.as_ref(),
+                nu_path::expand_path_with(pat.item.as_ref(), &cwd, pat.item.is_expand());
-                &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 !directory && tmp_expanded.is_dir() {
-                if permission_denied(&expanded) {
+                if permission_denied(&tmp_expanded) {
                     #[cfg(unix)]
                     let error_msg = format!(
                         "The permissions of {:o} do not allow access for this user",
-                        expanded
+                        tmp_expanded
                             .metadata()
                             .expect("this shouldn't be called since we already know there is a dir")
                             .permissions()
@@ -269,10 +268,10 @@ fn ls_for_one_pattern(
                         inner: vec![],
                     });
                 }
-                if is_empty_dir(&expanded) {
+                if is_empty_dir(&tmp_expanded) {
                     return Ok(Box::new(vec![].into_iter()));
                 }
-                extra_star_under_given_directory = true;
+                just_read_dir = !(pat.item.is_expand() && pat.item.as_ref().contains(GLOB_CHARS));
             }
 
             // it's absolute path if:
@@ -282,52 +281,28 @@ fn ls_for_one_pattern(
             //    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());
-            (
+            (pat.item, absolute_path)
-                expanded,
-                p_tag,
-                absolute_path,
-                matches!(pat.item, NuGlob::DoNotExpand(_)),
-            )
         }
         None => {
             // Avoid pushing "*" to the default path when directory (do not show contents) flag is true
             if directory {
-                (PathBuf::from("."), call_span, false, false)
+                (NuGlob::Expand(".".to_string()), false)
             } else if is_empty_dir(&cwd) {
                 return Ok(Box::new(vec![].into_iter()));
             } else {
-                (PathBuf::from("*"), call_span, false, false)
+                (NuGlob::Expand("*".to_string()), false)
             }
         }
     };
 
-    let hidden_dir_specified = is_hidden_dir(&path);
+    let hidden_dir_specified = is_hidden_dir(pattern_arg.as_ref());
-    // when it's quoted, we need to escape our glob pattern(but without the last extra
+    let path = pattern_arg.into_spanned(p_tag);
-    // start which may be added under given directory)
+    let (prefix, paths) = if just_read_dir {
-    // so we can do ls for a file or directory like `a[123]b`
+        let expanded = nu_path::expand_path_with(path.item.as_ref(), &cwd, path.item.is_expand());
-    let path = if quoted {
+        let paths = read_dir(&expanded)?;
-        let p = path.display().to_string();
+        // just need to read the directory, so prefix is path itself.
-        let mut glob_escaped = Pattern::escape(&p);
+        (Some(expanded), paths)
-        if extra_star_under_given_directory {
-            glob_escaped.push(std::path::MAIN_SEPARATOR);
-            glob_escaped.push('*');
-        }
-        glob_escaped
     } else {
-        let mut p = path.display().to_string();
-        if extra_star_under_given_directory {
-            p.push(std::path::MAIN_SEPARATOR);
-            p.push('*');
-        }
-        p
-    };
-
-    let glob_path = Spanned {
-        // use NeedExpand, the relative escaping logic is handled previously
-        item: NuGlob::Expand(path.clone()),
-        span: p_tag,
-    };
-
         let glob_options = if all {
             None
         } else {
@@ -337,12 +312,13 @@ fn ls_for_one_pattern(
             };
             Some(glob_options)
         };
-    let (prefix, paths) = nu_engine::glob_from(&glob_path, &cwd, call_span, glob_options)?;
+        glob_from(&path, &cwd, call_span, glob_options)?
+    };
 
     let mut paths_peek = paths.peekable();
     if paths_peek.peek().is_none() {
         return Err(ShellError::GenericError {
-            error: format!("No matches found for {}", &path),
+            error: format!("No matches found for {:?}", path.item),
             msg: "Pattern, file or folder not found".into(),
             span: Some(p_tag),
             help: Some("no matches found".into()),
@@ -904,3 +880,14 @@ mod windows_helper {
         false
     }
 }
+
+#[allow(clippy::type_complexity)]
+fn read_dir(
+    f: &Path,
+) -> Result<Box<dyn Iterator<Item = Result<PathBuf, ShellError>> + Send>, ShellError> {
+    let iter = f.read_dir()?.map(|d| {
+        d.map(|r| r.path())
+            .map_err(|e| ShellError::IOError { msg: e.to_string() })
+    });
+    Ok(Box::new(iter))
+}

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

@@ -85,13 +85,13 @@ fn lists_regular_files_in_special_folder() {
         assert_eq!(actual.out, "2");
         let actual = nu!(
             cwd: dirs.test().join("abcd/*"), format!(r#"ls ../* | length"#));
-        assert_eq!(actual.out, "3");
+        assert_eq!(actual.out, "2");
         let actual = nu!(
             cwd: dirs.test().join("abcd/?"), format!(r#"ls -D ../* | length"#));
         assert_eq!(actual.out, "2");
         let actual = nu!(
             cwd: dirs.test().join("abcd/?"), format!(r#"ls ../* | length"#));
-        assert_eq!(actual.out, "3");
+        assert_eq!(actual.out, "2");
     })
 }
 
@@ -237,6 +237,12 @@ fn list_files_from_two_parents_up_using_multiple_dots() {
         );
 
         assert_eq!(actual.out, "5");
+
+        let actual = nu!(
+            cwd: dirs.test().join("foo/bar"),
+            r#"ls ... | sort-by name | get name.0 | str replace -a '\' '/'"#
+        );
+        assert_eq!(actual.out, "../../andres.xml");
     })
 }
 
@@ -759,3 +765,46 @@ fn list_with_multiple_path() {
         assert_eq!(actual.out, "0");
     })
 }
+
+#[test]
+fn list_inside_glob_metachars_dir() {
+    Playground::setup("list_files_inside_glob_metachars_dir", |dirs, sandbox| {
+        let sub_dir = "test[]";
+        sandbox
+            .within(sub_dir)
+            .with_files(&[EmptyFile("test_file.txt")]);
+
+        let actual = nu!(
+            cwd: dirs.test().join(sub_dir),
+            "ls test_file.txt | get name.0 | path basename",
+        );
+        assert!(actual.out.contains("test_file.txt"));
+    });
+}
+
+#[test]
+fn list_inside_tilde_glob_metachars_dir() {
+    Playground::setup(
+        "list_files_inside_tilde_glob_metachars_dir",
+        |dirs, sandbox| {
+            let sub_dir = "~test[]";
+            sandbox
+                .within(sub_dir)
+                .with_files(&[EmptyFile("test_file.txt")]);
+
+            // need getname.0 | path basename because the output path
+            // might be too long to output as a single line.
+            let actual = nu!(
+                cwd: dirs.test().join(sub_dir),
+                "ls test_file.txt | get name.0 | path basename",
+            );
+            assert!(actual.out.contains("test_file.txt"));
+
+            let actual = nu!(
+                cwd: dirs.test(),
+                "ls '~test[]' | get name.0 | path basename"
+            );
+            assert!(actual.out.contains("test_file.txt"));
+        },
+    );
+}