Repository: reubeno/brush
Pull request #323 (2025-01-13 09:32:26)

fix(patterns): fix incorrect parse of char ranges
* Don't allow an unescaped `']'` to be treated as the end range of a character range. This fixes patterns using, for example, `[+-]`.
* Allow either `^` or `!` to invert a character set.
* Make it easier to debug internal errors showing up during completion attempts.

github-actions[bot]: ## Test Results
    2 files      9 suites   1m 18s ⏱️
  587 tests   587 ✅ 0 💤 0 ❌
1 160 runs  1 160 ✅ 0 💤 0 ❌

Results for commit d48d9493.

[test-results]:data:application/gzip;base64,H4sIAIDghGcC/13MSw6DIBSF4a0Yxh2AInK7mYbySG6q0vAYme69SEqrHX7/Sc5GHM42kmvXXzoSM6YKKDA5qIR+LZxkcVnSvo1yarrFrPV/euCzJPoNTuF8CjYEHz4l5HX/ZEw0tc9T+n1WHy6rj4/aLwumAmK4NMBhoDBwQxm9C87ACakAbC8VlVQpOxpLXm+nZhm7BgEAAA==

github-actions[bot]: # Performance Benchmark Report
| Benchmark name                         | Baseline (μs) |  Test/PR (μs) |    Delta (μs) | Delta %         |
| -------------------------------------- | ------------- | ------------- | ------------- | --------------- |
| `clone_shell_object                  ` | `   16.11 μs` | `   16.36 μs` | `    0.25 μs` | `🟠 +1.54%    ` |
| `eval_arithmetic                     ` | `    0.18 μs` | `    0.19 μs` | `    0.01 μs` | `⚪  Unchanged` |
| `expand_one_string                   ` | `    1.59 μs` | `    1.55 μs` | `   -0.04 μs` | `⚪  Unchanged` |
| `for_loop                            ` | `   32.70 μs` | `   32.88 μs` | `    0.18 μs` | `⚪  Unchanged` |
| `function_call                       ` | `    3.30 μs` | `    3.22 μs` | `   -0.07 μs` | `⚪  Unchanged` |
| `instantiate_shell                   ` | `   47.63 μs` | `   48.03 μs` | `    0.40 μs` | `⚪  Unchanged` |
| `instantiate_shell_with_init_scripts ` | `21756.52 μs` | `22129.59 μs` | `  373.07 μs` | `⚪  Unchanged` |
| `parse_bash_completion               ` | ` 1666.41 μs` | ` 1663.01 μs` | `   -3.41 μs` | `⚪  Unchanged` |
| `parse_sample_script                 ` | `    1.75 μs` | `    1.76 μs` | `    0.01 μs` | `⚪  Unchanged` |
| `run_echo_builtin_command            ` | `   16.61 μs` | `   16.24 μs` | `   -0.37 μs` | `⚪  Unchanged` |
| `run_one_external_command            ` | ` 2318.40 μs` | ` 2357.49 μs` | `   39.09 μs` | `🟠 +1.69%    ` |
| `tokenize_sample_script              ` | `    2.81 μs` | `    2.85 μs` | `    0.03 μs` | `⚪  Unchanged` |

# Code Coverage Report: Only Changed Files listed

| Package | Base Coverage | New Coverage | Difference |
| ------- | ------------- | ------------ | ---------- |
| brush-core/src/completion.rs | 🔴 29.55% | 🔴 29.59% | 🟢 0.04% |
| **Overall Coverage** | **🟢 74.69%** | **🟢 74.7%** | **🟢 0.01%** |

_Minimum allowed coverage is_ `70%`_, this run produced_ `74.7%`
<!-- Sticky Pull Request Comment -->

Patch:
diff --git a/brush-core/src/completion.rs b/brush-core/src/completion.rs
index dbcfbd4..bc861ca 100644
--- a/brush-core/src/completion.rs
+++ b/brush-core/src/completion.rs
@@ -642,10 +642,8 @@ impl Spec {
             ("COMP_CWORD", context.token_index.to_string().into()),
         ];
 
-        if tracing::enabled!(target: trace_categories::COMPLETION, tracing::Level::DEBUG) {
-            tracing::debug!(target: trace_categories::COMPLETION, "[calling completion func '{function_name}']: {}",
-                vars_and_values.iter().map(|(k, v)| std::format!("{k}={v}")).collect::<Vec<String>>().join(" "));
-        }
+        tracing::debug!(target: trace_categories::COMPLETION, "[calling completion func '{function_name}']: {}",
+            vars_and_values.iter().map(|(k, v)| std::format!("{k}={v}")).collect::<Vec<String>>().join(" "));
 
         let mut vars_to_remove = vec![];
         for (var, value) in vars_and_values {
@@ -673,6 +671,7 @@ impl Spec {
         shell.traps.handler_depth += 1;
 
         let invoke_result = shell.invoke_function(function_name, &args).await;
+        tracing::debug!(target: trace_categories::COMPLETION, "[completion function '{function_name}' returned: {invoke_result:?}]");
 
         shell.traps.handler_depth -= 1;
 
diff --git a/brush-parser/src/pattern.rs b/brush-parser/src/pattern.rs
index 5ec4287..7b3212d 100644
--- a/brush-parser/src/pattern.rs
+++ b/brush-parser/src/pattern.rs
@@ -55,7 +55,7 @@ peg::parser! {
             ['\\'] [c] { c.to_string() }
 
         rule bracket_expression() -> String =
-            "[" invert:(("!")?) members:bracket_member()+ "]" {
+            "[" invert:(invert_char()?) members:bracket_member()+ "]" {
                 let mut members = members;
                 if invert.is_some() {
                     members.insert(0, String::from("^"));
@@ -64,6 +64,9 @@ peg::parser! {
                 std::format!("[{}]", members.join(""))
             }
 
+        rule invert_char() -> bool =
+            ['!' | '^'] { true }
+
         rule bracket_member() -> String =
             char_class_expression() /
             char_range() /
@@ -76,7 +79,7 @@ peg::parser! {
             "alnum" / "alpha" / "blank" / "cntrl" / "digit" / "graph" / "lower" / "print" / "punct" / "space" / "upper"/ "xdigit"
 
         rule char_range() -> String =
-            range:$([_] "-" [_]) { range.to_owned() }
+            range:$([_] "-" [c if c != ']']) { range.to_owned() }
 
         rule char_list() -> String =
             chars:$([c if c != ']']+) { escape_char_class_char_list(chars) }


Test patch:
diff --git a/brush-shell/tests/cases/patterns.yaml b/brush-shell/tests/cases/patterns.yaml
index afe7a88..8a75a44 100644
--- a/brush-shell/tests/cases/patterns.yaml
+++ b/brush-shell/tests/cases/patterns.yaml
@@ -291,12 +291,25 @@ cases:
       myfunc abc
       myfunc "*"
 
+  - name: "Pattern matching: character ranges"
+    stdin: |
+      [[ "x" == [a-z] ]] && echo "1. Matched"
+      [[ "x" == [0-9] ]] && echo "2. Matched"
+      [[ "-" == [---] ]] && echo "3. Matched"
+
   - name: "Pattern matching: character sets"
     stdin: |
-      [[ "x" == [a-z] ]]       && echo "1. Matched"
-      [[ "x" == [xyz] ]]       && echo "2. Matched"
-      [[ "1" == [[:digit:]] ]] && echo "3. Matched"
-      [[ "(" == [\(]  ]]       && echo "4. Matched"
+      [[ "x" == [abc]  ]] && echo "1. Matched"
+      [[ "x" == [xyz]  ]] && echo "2. Matched"
+      [[ "x" == [^xyz] ]] && echo "3. Matched"
+      [[ "x" == [!xyz] ]] && echo "4. Matched"
+      [[ "(" == [\(]   ]] && echo "5. Matched"
+      [[ "+" == [+-]   ]] && echo "6. Matched"
+
+  - name: "Pattern matching: character classes"
+    stdin: |
+      [[ "1" == [[:digit:]] ]] && echo "1. Matched"
+      [[ "1" == [[:alpha:]] ]] && echo "2. Matched"
 
   - name: "Pattern matching: case sensitivity"
     stdin: |