Skip to content

Fix: chain reformatting leaves multi-line arguments orphaned#106

Merged
sorafujitani merged 1 commit intomainfrom
fix/chain-multiline-args
Apr 24, 2026
Merged

Fix: chain reformatting leaves multi-line arguments orphaned#106
sorafujitani merged 1 commit intomainfrom
fix/chain-multiline-args

Conversation

@sorafujitani
Copy link
Copy Markdown
Owner

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 .method lines get moved. Anything inside the call's argument list — strings split across lines, the trailing ), etc. — stays at its original column:

# before
@users = User.left_joins(articles: :article_likes)
             .group('users.id')
             .select(
               'users.*, ' \
               'COUNT(CASE WHEN articles.status = 1 THEN article_likes.id END) AS received_likes_count'
             )
             .having('received_likes_count > 0')

# after (bug)
@users = User.left_joins(articles: :article_likes)
  .group('users.id')
  .select(
               'users.*, ' \
               'COUNT(CASE WHEN articles.status = 1 THEN article_likes.id END) AS received_likes_count'
             )
  .having('received_likes_count > 0')

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_lines rewrites every .method line 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 .method line) and compute arg_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 by arg_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.

# after (this PR)
@users = User.left_joins(articles: :article_likes)
  .group('users.id')
  .select(
    'users.*, ' \
    'COUNT(CASE WHEN articles.status = 1 THEN article_likes.id END) AS received_likes_count'
  )
  .having('received_likes_count > 0')

Regression tests

spec/chain_multiline_args_spec.rb (3 cases):

  • multi-line .select(...) args shift with the chain
  • no-op when there is no chain reformat (paren-only call)
  • heredoc body indent is preserved when the chain reformat applies to a tail .squish/.strip etc.

Test plan

  • cargo test --manifest-path ext/rfmt/Cargo.toml --lib — 127 passed
  • bundle exec rspec — 134 passed (131 existing + 3 new)
  • cargo clippy -- -D warnings — clean
  • cargo fmt --check — clean
  • bundle exec rubocop — clean
  • All 50 Rails-idiom fixtures from earlier sweeps remain idempotent

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.
@sorafujitani sorafujitani merged commit bb44372 into main Apr 24, 2026
7 checks passed
@sorafujitani sorafujitani deleted the fix/chain-multiline-args branch April 24, 2026 04:57
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