Repository: reubeno/brush
Pull request #521 (2025-05-25 16:44:58)

fix(tokenizer): correctly treat $( (...) ) as a cmd substitution
While running the `bash` test suite, we identified that `brush`'s tokenization process was causing `$( (...) )` to be treated as an arithmetic expression (incorrect) instead of a command substitution with a nested subshell (correct).

This change adds a targeted fix that lets parsing of a command substitution preserve the whitespace in its body during tokenization.

github-actions[bot]: ## Test Results
    3 files     14 suites   3m 39s ⏱️
  762 tests   762 ✅ 0 💤 0 ❌
2 272 runs  2 272 ✅ 0 💤 0 ❌

Results for commit 1b0670d2.

[test-results]:data:application/gzip;base64,H4sIAOBLM2gC/13MQQ6DIBCF4asY1l3ASAbtZRoLTEKq0iCsmt69I9FWu/z+l7yXoDD6RVyb9tKIpYRcoTTLlTTkEGc2qJ4Db3ldDcKu21Ks/U+P8OQkv4GGMJ6CTymmraQyr58ABjbtn6f0+6w+XFYfH22cppAZQt0lGunAq04bBdQSUIcovUNAB9b6nlA7Jd4fcyHv1ggBAAA=


Patch:
diff --git a/brush-parser/src/tokenizer.rs b/brush-parser/src/tokenizer.rs
index 2df9d52..9af6a02 100644
--- a/brush-parser/src/tokenizer.rs
+++ b/brush-parser/src/tokenizer.rs
@@ -586,9 +586,19 @@ impl<'a, R: ?Sized + std::io::BufRead> Tokenizer<'a, R> {
     }
 
     pub fn next_token(&mut self) -> Result<TokenizeResult, TokenizerError> {
-        self.next_token_until(None)
-    }
-
+        self.next_token_until(None, false /* include space? */)
+    }
+
+    /// Returns the next token from the input stream, optionally stopping early when a specified
+    /// terminating character is encountered.
+    ///
+    /// # Arguments
+    ///
+    /// * `terminating_char` - An optional character that, if encountered, will stop the
+    ///   tokenization process and return the token up to that character.
+    /// * `include_space` - If true, include spaces in the tokenization process. This is not
+    ///   typically the case, but can be helpful when needing to preserve the original source text
+    ///   embedded within a command substitution or similar construct.
     #[allow(clippy::if_same_then_else)]
     #[allow(clippy::too_many_lines)]
     #[allow(clippy::unwrap_in_result)]
@@ -596,6 +606,7 @@ impl<'a, R: ?Sized + std::io::BufRead> Tokenizer<'a, R> {
     fn next_token_until(
         &mut self,
         terminating_char: Option<char>,
+        include_space: bool,
     ) -> Result<TokenizeResult, TokenizerError> {
         let mut state = TokenParseState::new(&self.cross_state.cursor);
         let mut result: Option<TokenizeResult> = None;
@@ -848,7 +859,10 @@ impl<'a, R: ?Sized + std::io::BufRead> Tokenizer<'a, R> {
 
                                     pending_here_doc_tokens.remove(0)
                                 } else {
-                                    let cur_token = self.next_token_until(Some(')'))?;
+                                    let cur_token = self.next_token_until(
+                                        Some(')'),
+                                        true, /* include space? */
+                                    )?;
 
                                     // See if this is a here-document-related token we need to hold
                                     // onto until after we've seen all the tokens that need to show
@@ -938,7 +952,10 @@ impl<'a, R: ?Sized + std::io::BufRead> Tokenizer<'a, R> {
 
                                     pending_here_doc_tokens.remove(0)
                                 } else {
-                                    let cur_token = self.next_token_until(Some('}'))?;
+                                    let cur_token = self.next_token_until(
+                                        Some('}'),
+                                        false, /* include space? */
+                                    )?;
 
                                     // See if this is a here-document-related token we need to hold
                                     // onto until after we've seen all the tokens that need to show
@@ -1088,6 +1105,8 @@ impl<'a, R: ?Sized + std::io::BufRead> Tokenizer<'a, R> {
                         TokenEndReason::NonNewLineBlank,
                         &mut self.cross_state,
                     )?;
+                } else if include_space {
+                    state.append_char(c);
                 } else {
                     // Make sure we don't include this char in the token range.
                     state.start_position.column += 1;
@@ -1526,7 +1545,7 @@ HERE2
             [t1 @ Token::Word(..),
              t2 @ Token::Word(..)] if
                 t1.to_str() == "echo" &&
-                t2.to_str() == "$(cat <<HERE1 <<HERE2 |wc -l\nTEXT\nHERE1\nOTHER\nHERE2\n)"
+                t2.to_str() == "$(cat <<HERE1 <<HERE2 | wc -l\nTEXT\nHERE1\nOTHER\nHERE2\n)"
         );
         Ok(())
     }
@@ -1580,6 +1599,15 @@ HERE2
         Ok(())
     }
 
+    #[test]
+    fn tokenize_command_substitution_with_subshell() -> Result<()> {
+        assert_matches!(
+            &tokenize_str("$( (:) )")?[..],
+            [t @ Token::Word(..)] if t.to_str() == "$( (:) )"
+        );
+        Ok(())
+    }
+
     #[test]
     fn tokenize_command_substitution_containing_extglob() -> Result<()> {
         assert_matches!(
@@ -1609,7 +1637,7 @@ HERE2
         assert_matches!(
             &tokenize_str("$(( 1 ))")?[..],
             [t1 @ Token::Word(..)] if
-                t1.to_str() == "$((1 ))"
+                t1.to_str() == "$(( 1 ))"
         );
         Ok(())
     }
@@ -1618,7 +1646,7 @@ HERE2
         assert_matches!(
             &tokenize_str("$(( (0) ))")?[..],
             [t1 @ Token::Word(..)] if
-                t1.to_str() == "$(((0)))"
+                t1.to_str() == "$(( (0) ))"
         );
         Ok(())
     }


Test patch:
diff --git a/brush-shell/tests/cases/word_expansion.yaml b/brush-shell/tests/cases/word_expansion.yaml
index 7edfb6d..10872f0 100644
--- a/brush-shell/tests/cases/word_expansion.yaml
+++ b/brush-shell/tests/cases/word_expansion.yaml
@@ -61,6 +61,11 @@ cases:
       x=$(echo foo | (wc -l; echo hi))
       echo "\$x: $x"
 
+  - name: "Command substitution with subshell"
+    stdin: |
+      x=$( (echo hi) )
+      echo "\$x: $x"
+
   - name: "Command substitution exit code"
     stdin: |
       x=$(false) && echo "1. Made it past false"