Fix: chain reformatting leaves multi-line arguments orphaned#106
Merged
sorafujitani merged 1 commit intomainfrom Apr 24, 2026
Merged
Fix: chain reformatting leaves multi-line arguments orphaned#106sorafujitani merged 1 commit intomainfrom
sorafujitani merged 1 commit intomainfrom
Conversation
When `reformat_chain_lines` rewrote an aligned chain to indented style,
it only moved the `.method` lines. Any multi-line *arguments* that
lived inside a chain call (e.g. `.select( 'users.*, ' \ 'COUNT…' )`)
were kept at their original column, producing output like:
@users = User.left_joins(...)
.select(
'users.*, ' \
'COUNT(CASE ...'
)
.having(...)
The chain body sits at column 6 but the args still float at column 21
(where they used to align under `.select(` in the original aligned
style). The closing `)` lands midway — visually orphaned and trailing
off to the right.
Fix: before rewriting chain lines, capture the *original* chain indent
and compute the delta to the new chain indent. For every non-chain
continuation line whose leading indent is at or below the original
chain indent, shift it by the same delta so it stays aligned relative
to the reformatted chain. Lines shallower than the original chain
indent (e.g. a squiggly-heredoc body measured against its own
terminator) are left alone, so we don't accidentally strip through
their content.
Regression tests (spec/chain_multiline_args_spec.rb):
- multi-line `.select(...)` args shift with the chain
- no-op when there is no chain reformat to perform
- heredoc body indent is preserved when reformatting a tail chain
Full test suite: 134 rspec + 127 cargo tests pass; clippy, fmt, and
rubocop are clean; all 50 Rails-idiom fixtures remain idempotent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When
reformat_chain_lines(PR #100's aligned → indented rewrite) fires on a multi-line method chain that contains a call with multi-line arguments such as.select(...), only the.methodlines get moved. Anything inside the call's argument list — strings split across lines, the trailing), etc. — stays at its original column:The chain body sits at column 6 but the
.select(...)arguments still float at column 15, and the closing)lands at column 13 — visually orphaned and trailing off to the right.Root cause
reformat_chain_linesrewrites every.methodline to" " * (base_indent + indent_width)but preserves every non-chain continuation line as-is. When the original chain sat at a deeper indent than the new one (aligned → indented is always this direction), anything aligned relative to the original chain — function arguments, closing parens, etc. — is left stranded.Fix
Before rewriting, capture the original chain indent (the leading whitespace of the first
.methodline) and computearg_shift = original_chain_indent − new_chain_indent. For every non-chain continuation line whose leading indent is at or deeper than the original chain indent, shift its leading whitespace byarg_shift. Lines shallower than the original chain indent — a squiggly-heredoc body measured against its own terminator, for instance — are left alone so we don't eat through their content.Regression tests
spec/chain_multiline_args_spec.rb(3 cases):.select(...)args shift with the chain.squish/.stripetc.Test plan
cargo test --manifest-path ext/rfmt/Cargo.toml --lib— 127 passedbundle exec rspec— 134 passed (131 existing + 3 new)cargo clippy -- -D warnings— cleancargo fmt --check— cleanbundle exec rubocop— clean