Skip to content

fix(format): align cmdsub reformatter with bash canonical form#49

Merged
mpecan merged 1 commit into
mainfrom
feat/40-cmdsub-reformat
Apr 14, 2026
Merged

fix(format): align cmdsub reformatter with bash canonical form#49
mpecan merged 1 commit into
mainfrom
feat/40-cmdsub-reformat

Conversation

@mpecan
Copy link
Copy Markdown
Owner

@mpecan mpecan commented Apr 14, 2026

Summary

Fixes the six canonical-reformatter drifts in $(…) body formatting tracked by #40.

bug fix
{fd}> dropped new Redirect.varfd: Option<String> AST field + parse_redirect_with_varfd + formatter emits {name} prefix
select rendered as raw S-expr new format_select arm, shares layout helper with format_for
function body not wrapped in implicit braces format_function_body rewritten to always emit { … }
statements in function body single-lined new format_list_block helper — wraps on ;/& but keeps &&/`
<<\EOF quoted-delimiter lost honor HereDoc.quoted in both regular and heredoc-pipe paths

build_redirect hit the 60-line limit with the new field, so its heredoc branch was factored into build_heredoc_redirect. format_for and format_select delegate to a shared format_for_or_select helper to remove duplication.

Parable corpus correction

Three tests in tests/parable/12_command_substitution.tests expected the reformatter to strip heredoc delimiter quoting (<<'EOF'<<EOF). Parable was wrong about bash's canonical form — verified directly against bash-oracle --dump-ast:

$ printf "x=\$(cat <<'EOF'\nbody\nEOF\n)\n" | bash-oracle --dump-ast
(command (word "x=$(cat <<'EOF'\nbody\nEOF\n)"))

Updated the 3 goldens to match the correct quoted form.

Bonus fix

The heredoc-quoting fix also newly passes heredoc_in_cmdsub 1 (tracked under #39), removed from KNOWN_ORACLE_FAILURES with an attribution comment.

Test plan

  • cargo fmt
  • cargo clippy --all-targets -- -D warnings — no warnings
  • cargo test — 240 passed, 0 failed (185 lib + 54 integration + 1 tree-sitter compare)
  • All 6 cmdsub_reformat N cases now pass in oracle_bash_valid_divergences
  • heredoc_in_cmdsub 1 also newly passes
  • New parser unit tests pin Redirect.varfd populated vs. None
  • 3 new corpus regressions in tests/parable/12_command_substitution.tests: tab-stripped quoted heredoc, && in function body, varfd close-fd
  • Full test suite green

Closes #40.

Fix the six canonical-reformatter drifts tracked in issue #40:

1. **varfd `{fd}>` dropped** — add a `varfd: Option<String>` field to
   `NodeKind::Redirect`, thread it through `build_redirect` via a new
   `parse_redirect_with_varfd` entry, and emit the `{name}` prefix in
   `format_redirect`. The sexp layer destructures `Redirect` with `..`
   so top-level output is unchanged (matches existing Parable corpus
   in `tests/parable/26_variable_fd.tests`). `build_redirect` hit the
   60-line limit with the new field, so its heredoc branch is factored
   into `build_heredoc_redirect`.

2. **`select` rendered as raw S-expression** — add a `Select` arm to
   `format_node` and a shared `format_for_or_select` helper that both
   `format_for` and `format_select` delegate to.

3. **Function body not wrapped in implicit braces** — rewrite
   `format_function_body` to always emit `{ \n … \n }`, unwrapping an
   inner `BraceGroup` when present and wrapping a bare body (e.g. a
   `Subshell` from `function f ( cmd )`) otherwise.

4. **Statement separators inside function body** — add
   `format_list_block` for the canonical multi-line form. Only `;`
   and `&` wrap to a new indented line; `&&` and `||` stay inline
   because they form a single logical command.

5. **`<<\EOF` quoted-delimiter marker lost** — honor `HereDoc.quoted`
   in both `format_redirect` and `format_command_with_heredoc_pipe`
   via a shared `write_heredoc_delimiter` helper. Bash always emits
   `<<'EOF'` for any quoted form (`<<'EOF'`, `<<"EOF"`, `<<\EOF`) —
   verified against `bash-oracle --dump-ast`. Three tests in
   `tests/parable/12_command_substitution.tests` that expected the
   wrong strip-quoting form are updated to the correct form.

As a side effect, `heredoc_in_cmdsub 1` (tracked under #39) also
newly passes — removed from `KNOWN_ORACLE_FAILURES` too.

New coverage:
- parser unit tests pinning the `varfd` AST field (populated vs. `None`)
- 3 corpus regressions in `tests/parable/12_command_substitution.tests`:
  tab-stripped quoted heredoc in cmdsub, function body with `&&`
  separator, varfd close-fd `{fd}>&-`

Closes #40.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mpecan mpecan merged commit c7a4411 into main Apr 14, 2026
5 checks passed
@mpecan mpecan deleted the feat/40-cmdsub-reformat branch April 14, 2026 21:24
mpecan pushed a commit that referenced this pull request Apr 19, 2026
🤖 I have created a release *beep* *boop*
---


##
[0.2.0](rable-v0.1.15...rable-v0.2.0)
(2026-04-18)


### ⚠ BREAKING CHANGES

* tighten lexer API surface and relocate WordSpan to ast
([#70](#70))

### Bug Fixes

* **format:** align cmdsub reformatter with bash canonical form
([#49](#49))
([c7a4411](c7a4411))
* **lexer:** accept sloppy heredoc terminator in cmdsub mode
([#50](#50))
([40f394f](40f394f))
* **lexer:** backticks opaque when content is invalid
([#71](#71))
([e72166f](e72166f)),
closes [#38](#38)
* **lexer:** disable reserved-word recognition after assignment words
([#44](#44))
([42e1fc0](42e1fc0))
* **lexer:** stop treating ]] and unbalanced [...] as special outside
conditionals ([#45](#45))
([4bf5a5c](4bf5a5c))
* **parser:** fall back from (( … )) arith to nested subshells
([#48](#48))
([1437f00](1437f00))


### Code Refactoring

* **format:** introduce Formatter struct
([#65](#65))
([d965a8f](d965a8f))
* **lexer:** drop Result&lt;Token&gt; wrapper from operator readers
([#62](#62))
([d52a841](d52a841))
* **lexer:** split read_word_token into classify + advance + dispatch
helpers ([#63](#63))
([3ba09f5](3ba09f5))
* **parser:** extract fill_heredoc_contents visitor helpers
([#68](#68))
([40e6165](40e6165))
* **parser:** extract helpers from three oversize parsers
([#69](#69))
([25d0762](25d0762))
* **sexp:** dispatch NodeKind Display to per-category helpers
([#66](#66))
([44b0330](44b0330))
* **sexp:** table-drive ANSI-C escape dispatch
([#67](#67))
([91a5267](91a5267))
* tighten lexer API surface and relocate WordSpan to ast
([#70](#70))
([5171d01](5171d01))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: repository-butler[bot] <166800726+repository-butler[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

format: command-substitution canonical reformatter drift from bash

1 participant