amber-lang / amber-lang/bash2amber

[Detail Bug] Bash parser mis-parses spaced closing parens in arithmetic and nested subshells in command substitutions

Open
#3 0 comments 0 reactions 0 assignees View on GitHub
bug detail
Dominant language
Rust
Stars
2
Forks
0
PR merge metrics
No merged PRs in 30d

Description

# Summary
- **Context**: The `src/bash/parser.rs` file implements a recursive descent parser for Bash, converting Bash source code into an AST.
- **Bug**: The parser fails to correctly handle closing parentheses for arithmetic commands `(( ... ))` and command substitutions `$( ... )` when they are separated by whitespace or split across multiple tokens.
- **Actual vs. expected**: For arithmetic commands, it fails with an "Unterminated arithmetic command" error if the closing `))` are not in the same token. For command substitutions, it prematurely decrements the substitution depth when encountering a closing parenthesis of a nested subshell, causing the parser to incorrectly break at the next command separator (like `;`).
- **Impact**: Valid Bash scripts using common formatting (like spaces between parentheses) or nested subshells inside command substitutions will fail to parse or be parsed into an incorrect and broken AST.

# Code with bug
In `consume_arithmetic_chunk`:
```rust
if ch == ')' {
if i + 1 < chars.len() && chars[i + 1] == ')' && *nested_parens == 0 { // <-- BUG 🔴 [Only looks for '))' within a single token chunk]
append_arithmetic_segment(expression, current.trim());
*closed = true;
// ...
```

In `update_command_substitution_depth`:
```rust
if !in_single && !in_double && ch == ')' && depth > 0 {
depth -= 1; // <-- BUG 🔴 [Decrements depth on any ')', without balancing against '(' that aren't '$(']
}
```

# Evidence
### 1. Arithmetic Command Failure
Running a test with `(( 1 + (2) ) )` (with spaces between the closing parens) results in a parse error, even though this is valid Bash.

**Reproduction Test:**
```rust
#[test]
fn parses_arithmetic_with_split_close_parens() {
let program = parse("(( 1 + (2) ) )\n", None).expect("script should parse");
assert_eq!(program.statements.len(), 1);
}
```
**Actual Result:**
`script should parse: "Unterminated arithmetic command at 1:15"`

### 2. Command Substitution Premature Termination
Parsing `echo $( (echo hi) ; echo lo )` incorrectly results in two statements because the `)` after `hi` makes the parser think the `$(` has ended.

**Reproduction Test:**
```rust
#[test]
fn parses_command_substitution_with_internal_subshell() {
let program = parse("echo $( (echo hi) ; echo lo )\n", None).expect("script should parse");
assert_eq!(program.statements.len(), 1);
}
```
**Actual Result:**
`assertion left == right failed: left: 2, right: 1`

# Why has this bug gone undetected?
This bug has likely gone undetected because most automated tests and simple scripts use the more compact `))` and `$(...)` syntax without internal spaces or nested subshells. The lexer often combines `))` into a single token if there is no space, which happens to work for the current implementation of `consume_arithmetic_chunk`. However, Bash is highly flexible with whitespace, and standard-compliant scripts frequently use spaces for readability.

# Recommended fix
1. **For Arithmetic Commands**: Modify `parse_arithmetic` to look for the closing `))` across tokens. Instead of looking for the character pair `))` in a single chunk, it should track nested parentheses and terminate only when it sees a `)` token that brings the depth to -1 (relative to the starting `((`).
2. **For Command Substitutions**: Update `update_command_substitution_depth` to increment a counter for every `(` and decrement it for every `)`. Only when the counter returns to the level before the `$(` started should the command substitution be considered closed. Alternatively, ensure that `parse_simple` correctly balances all types of parentheses.

# History
This bug was introduced in commit e1373fe (@Ph0enixKM, 2026-02-07). This commit overhauled the Bash parser and renderer to support complex features like arithmetic expressions and nested subshells, but it implemented a naive parenthesis-matching logic that fails to account for whitespace between parentheses and nested subshell balancing.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.