Skip to content

Space a rewritten argument list the way it was written - #154

Merged
virzak merged 2 commits into
masterfrom
fix/argument-list-whitespace
Jul 28, 2026
Merged

Space a rewritten argument list the way it was written#154
virzak merged 2 commits into
masterfrom
fix/argument-list-whitespace

Conversation

@virzak

@virzak virzak commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Addresses most of #152, found in the code generated for SharpCompress now that it has adopted the generator.

Before

global::Helpers.StreamExtensions.WriteTo(stream,                         destination,
        4096,
        progress: progress)                    ;

After

global::Helpers.StreamExtensions.WriteTo(stream, destination,
        4096,
        progress: progress);

Three seams, three causes

  1. Unwrapping rebuilt the argument list with fresh parentheses, losing the break after the opening one and the indentation before the closing one. The original parentheses are kept now. What trails the closing one is still dropped, because it described how the call sat in whatever contained it - which is exactly what unwrapping changes. Keeping it was what broke ConditionalExtensionTests.LongChained on my first attempt, stranding the ; on its own line.

  2. The argument the receiver displaces is no longer first on its line, so indentation written to put it at the start of one only leaves a gap after the comma. Dropped when it is bare indentation; left alone when it still follows a break.

  3. Dropping a trailing ConfigureAwait left whatever separated the call from the dot in front of the next token, usually the semicolon. When that is only whitespace it goes with the call. Anything else - a comment, a directive - was written about the expression which remains, so all of it is kept, rather than silently deleting something a person wrote.

Not fixed

The break which belongs before the first argument is still missing, so the receiver and the first argument share a line. That break is already gone from the tree by the time unwrapping runs - it is lost somewhere upstream - and finding where is a separate piece of work. #152 stays open for it, with the scope narrowed to that.

Verification

First commit records the current output; the second commit's diff is exactly what changed. Two snapshots move, both to the shape above. Everything else is untouched: 339 and 297 pass, all three Roslyn variants build.

🤖 Generated with Claude Code

virzak added 2 commits July 28, 2026 06:36
Recorded as the generator handles it today, so that the fix shows what
changes. An extension call whose arguments were written across several lines,
with a cancellation token last and a trailing ConfigureAwait, comes out as:

  global::Helpers.StreamExtensions.WriteTo(stream,                         destination,
          4096,
          progress: progress)                    ;

The receiver displaces the first argument without taking over the indentation
which placed it at the start of a line, and the whitespace which separated the
call from the dropped ConfigureAwait is left in front of the semicolon.

This is the shape SharpCompress is formatted in throughout, so every method
generated for it reads this way.

Generated with Claude Code
Three seams left whitespace behind which described a shape the rewrite had
already changed:

Unwrapping an extension rebuilt the argument list with fresh parentheses,
losing the break which followed the opening one and the indentation which
preceded the closing one. The original parentheses are kept instead. What
trailed the closing one is still dropped, since it described how the call sat
in whatever contained it, which unwrapping changes.

The argument the receiver displaces is no longer the first thing on its line,
so indentation written to place it at the start of one only leaves a gap after
the comma, and is dropped. Indentation which still follows a break is left.

Dropping a trailing ConfigureAwait left whatever separated the call from the
dot in front of whichever token came next, typically the semicolon. When that
is only whitespace it goes with the call. Anything else - a comment, a
directive - was written about the expression which remains, so it is kept.

  global::Helpers.StreamExtensions.WriteTo(stream, destination,
          4096,
          progress: progress);

The break which belongs before the first argument is still missing: it is lost
somewhere upstream of the unwrapping, which is left for #152.

Generated with Claude Code
@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.75000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 93.90%. Comparing base (c949773) to head (773b680).

Files with missing lines Patch % Lines
...rc/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs 93.75% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #154      +/-   ##
==========================================
- Coverage   93.92%   93.90%   -0.02%     
==========================================
  Files           9        9              
  Lines        1563     1576      +13     
  Branches      360      363       +3     
==========================================
+ Hits         1468     1480      +12     
  Misses         25       25              
- Partials       70       71       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@virzak virzak added the bug Something isn't working label Jul 28, 2026
@virzak
virzak merged commit 6a13efd into master Jul 28, 2026
10 checks passed
@virzak
virzak deleted the fix/argument-list-whitespace branch July 28, 2026 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant