Skip to content

Fix data section loss, chain indent idempotency, and else comments - #124

Merged
sorafujitani merged 6 commits into
mainfrom
fix/format-bugs-data-chain-else
Jul 19, 2026
Merged

sorafujitani merged 6 commits into
mainfrom
fix/format-bugs-data-chain-else

Conversation

@sorafujitani

Copy link
Copy Markdown
Owner

Three formatter bugs found by ad-hoc property testing over Rails-style code (crash / idempotency / AST equivalence / comment preservation, plus a DATA-section property the existing corpus check does not cover).

What was broken

The `END` data section was silently dropped. `Rfmt.format("puts DATA.read\n\n__END__\nhello\n")` returned only `"puts DATA.read\n"`, destroying any script that reads `DATA`. The section is not part of Prism's AST, and the formatter rebuilds output from the AST, so nothing ever emitted it. Fix: `NativeAdapter::parse` records `data_loc()`'s start offset in the root metadata, and `Formatter::format` re-appends the verbatim source slice after printing. `data_loc` comes from Prism, so a fake `END` inside a heredoc cannot false-positive.

Chain continuation indentation was not idempotent. The chain reformatter baked the statement's input column into a `Doc::Text`, but the doc engine places the statement at its output position. On misindented input (conflict resolutions, generated code) the first pass under- or over-indented continuations and the second pass moved them again. Fix: `reformat_chain_lines` became `reformat_chain_doc`, emitting continuation lines as `hardline` docs inside `indent(...)` so the printer anchors them at render time. Multi-line arguments keep their offset relative to the chain; heredoc bodies are emitted through `literalline` verbatim, because for plain/dash heredocs the leading whitespace is string content (a lexical heredoc scanner marks those lines; ambiguity fails toward verbatim, the semantics-safe direction).

A trailing comment on `else` moved below the line, where it reads as annotating the branch body. The `ElseNode` arm never consumed the else-line comment as a trailing comment, so the body's leading-comment path claimed it. Fix mirrors the existing if-header and end-line handling.

Verification

  • Failing repro specs land first in history (`spec/end_data_section_spec.rb`, `spec/chain_indent_idempotency_spec.rb`, `spec/else_trailing_comment_spec.rb`), fixes on top.
  • `bundle exec rspec`: 177 examples, 0 failures. `cargo test`: 139 + 5 + 3 passed. `scripts/corpus_check.rb`: 49/49.
  • A 77-case Rails-idiom corpus (migrations, models, controllers, routes, jobs, RSpec, Rake DSL, mangled-indentation variants, lexical traps) passes all properties including the new DATA-section check.

Known follow-ups (pre-existing, not regressions)

  • A variable-assigned chain ending in a `do…end` block under-indents the block body on misindented input (output is stable and semantically equivalent).
  • `case/when` and `begin/rescue` `else` lines still relocate their trailing comment; the fix here covers `if`/`unless`/`elsif` only.
  • Two lexical heredoc detectors now coexist (`heredoc_body_lines` and `statement_contains_heredoc_tail` in if_unless.rs); the older, cruder one could be reimplemented on the new scanner.

@sorafujitani
sorafujitani merged commit ec35755 into main Jul 19, 2026
10 checks passed
@sorafujitani
sorafujitani deleted the fix/format-bugs-data-chain-else branch July 19, 2026 16:38
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.

1 participant