fix(redirection): make sure redirection fd + operator are contiguous (#…

…359)

brush-parser/src/parser.rs

@@ -771,10 +771,16 @@ peg::parser! {
             linebreak() [Token::Word(e, _)] linebreak() { e }
 
         // N.B. An I/O number must be a string of only digits, and it must be
-        // followed by a '<' or '>' character (but not consume them).
+        // followed by a '<' or '>' character (but not consume them). We also
+        // need to make sure that there was no space between the number and the
+        // redirection operator; unfortunately we don't have the space anymore
+        // but we can infer it by looking at the tokens' locations.
         rule io_number() -> u32 =
-            [Token::Word(w, _) if w.chars().all(|c: char| c.is_ascii_digit())]
-            &([Token::Operator(o, _) if o.starts_with('<') || o.starts_with('>')]) {
+            [Token::Word(w, num_loc) if w.chars().all(|c: char| c.is_ascii_digit())]
+            &([Token::Operator(o, redir_loc) if
+                    o.starts_with(['<', '>']) &&
+                    locations_are_contiguous(num_loc, redir_loc)]) {
+
                 w.parse().unwrap()
             }
 
@@ -881,6 +887,13 @@ fn add_pipe_extension_redirection(c: &mut ast::Command) -> Result<(), &'static s
     Ok(())
 }
 
+fn locations_are_contiguous(
+    loc_left: &crate::TokenLocation,
+    loc_right: &crate::TokenLocation,
+) -> bool {
+    loc_left.end.index == loc_right.start.index
+}
+
 fn parse_array_assignment(
     word: &str,
     elements: &[&String],

brush-shell/tests/cases/redirection.yaml

@@ -29,6 +29,20 @@ cases:
     stdin: |
       echo hi >&2
 
+  - name: "Redirecting an fd"
+    stdin: |
+      echo hi-out 1>stdout.txt
+      echo hi-error 2>stderr.txt >&2
+
+  - name: "Almost redirecting but not actually redirecting"
+    stdin: |
+      # Checks to make sure the '1' isn't treated as a file descriptor
+      echo 1 >&2
+
+  - name: "Redirection with arbitrary fd number"
+    stdin: |
+      echo hi 4>file.txt >&4
+
   - name: "Process substitution: basic"
     stdin: |
       shopt -u -o posix
@@ -76,3 +90,24 @@ cases:
     stdin: |
       shopt -u -o posix
       cp <(echo hi) >(cat)
+
+  - name: "Redirection in command substitution"
+    stdin: |
+      echo $(echo hi >&2) 2>stderr.txt
+
+  - name: "Redirection in command substitution to non-standard fd"
+    ignore_stderr: true
+    stdin: |
+      echo $(echo hi >&3) 3>output.txt
+
+  - name: "Redirection in command substitution in braces to non-standard fd"
+    known_failure: true # https://github.com/reubeno/brush/issues/358
+    ignore_stderr: true
+    stdin: |
+      { : $(echo hi >&3); } 3>output.txt
+
+  - name: "Redirection in command substitution in subshell to non-standard fd"
+    known_failure: true # https://github.com/reubeno/brush/issues/358
+    ignore_stderr: true
+    stdin: |
+      ( : $(echo hi >&3); ) 3>output.txt

brush-shell/tests/compat_tests.rs

@@ -715,12 +715,38 @@ impl TestCaseResult {
                 for entry in entries {
                     const INDENT: &str = "        ";
                     match entry {
-                        DirComparisonEntry::Different(left, right) => {
+                        DirComparisonEntry::Different(
+                            left_path,
+                            left_contents,
+                            right_path,
+                            right_contents,
+                        ) => {
                             writeln!(
                                 writer,
                                 "{INDENT}oracle file {} differs from test file {}",
-                                left.to_string_lossy(),
-                                right.to_string_lossy()
+                                left_path.to_string_lossy(),
+                                right_path.to_string_lossy()
+                            )?;
+
+                            writeln!(
+                                writer,
+                                "{INDENT}{}",
+                                "------ Oracle <> Test: file ---------------------------------"
+                                    .cyan()
+                            )?;
+
+                            write_diff(
+                                &mut writer,
+                                8,
+                                left_contents.as_str(),
+                                right_contents.as_str(),
+                            )?;
+
+                            writeln!(
+                                writer,
+                                "        {}",
+                                "---------------------------------------------------------------"
+                                    .cyan()
                             )?;
                         }
                         DirComparisonEntry::LeftOnly(p) => {
@@ -1227,7 +1253,7 @@ impl StringComparison {
 enum DirComparisonEntry {
     LeftOnly(PathBuf),
     RightOnly(PathBuf),
-    Different(PathBuf, PathBuf),
+    Different(PathBuf, String, PathBuf, String),
 }
 
 enum DirComparison {
@@ -1265,7 +1291,9 @@ fn diff_dirs(oracle_path: &Path, test_path: &Path) -> Result<DirComparison> {
             }
             dir_cmp::full::DirCmpEntry::Both(l, r, _) => DirComparisonEntry::Different(
                 pathdiff::diff_paths(l, oracle_path).unwrap(),
+                std::fs::read_to_string(l).unwrap(),
                 pathdiff::diff_paths(r, test_path).unwrap(),
+                std::fs::read_to_string(r).unwrap(),
             ),
         })
         .collect();