Skip to content

Keep the receiver on the same line as the comma after it - #150

Merged
virzak merged 1 commit into
masterfrom
fix/unwrapped-extension-argument-formatting
Jul 28, 2026
Merged

Keep the receiver on the same line as the comma after it#150
virzak merged 1 commit into
masterfrom
fix/unwrapped-extension-argument-formatting

Conversation

@virzak

@virzak virzak commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Problem

Unwrapping an extension call turns the receiver into the first argument. When the call was written as a chain across several lines, the line break which had preceded the dot ended up between that argument and its comma:

global::Helpers.StreamExtensions.Drain(stream
,                         1024);

Seen in the code generated for SharpCompress, which chains calls that way throughout.

Cause

The receiver was inserted with expression.WithoutLeadingTrivia(). Its trailing trivia - the break and indent which separated it from the dot - came along, and the comma is emitted after it.

Fix

Drop the trailing trivia too. Whatever separated the receiver from the dot describes a shape which no longer exists once the receiver is an argument.

Not addressed here

The generated line still carries the indentation the original arguments had, so it reads as

global::Helpers.StreamExtensions.Drain(stream,                         1024);

That comes from the argument trivia rather than from unwrapping, applies equally to calls which were never extensions, and is left for a separate change. The comma placement above was specific to this path.

Verification

First commit is the failing test. Afterwards the full suite passes on both target frameworks with every existing snapshot unchanged, and all three Roslyn variants build.

🤖 Generated with Claude Code

Unwrapping an extension call turns the receiver into the first argument, so a
call written as a chain across several lines put the line break which had
preceded the dot between that argument and its comma:

  global::Helpers.StreamExtensions.Drain(stream
,                         1024);

Only the leading trivia was being dropped. Drop the trailing trivia as well,
since whatever separated the receiver from the dot describes a shape which no
longer exists once it is an argument.

Generated with Claude Code
@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.82%. Comparing base (5f7ace0) to head (02bca8b).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #150   +/-   ##
=======================================
  Coverage   93.82%   93.82%           
=======================================
  Files           9        9           
  Lines        1522     1522           
  Branches      352      352           
=======================================
  Hits         1428     1428           
  Misses         25       25           
  Partials       69       69           

☔ 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 9205073 into master Jul 28, 2026
10 checks passed
@virzak
virzak deleted the fix/unwrapped-extension-argument-formatting branch July 28, 2026 02:55
virzak added a commit that referenced this pull request Aug 27, 2026
* Show that dropping the last argument flattens the list

An argument list written across lines, whose last argument is a
CancellationToken, comes out with the first argument stranded after the
opening parenthesis and its original indentation still in front of it:

    ProgressMethod(            1,
        2);

The snapshot committed here is what it should be instead. The test fails
until the next commit.

Generated with Claude Code

* Keep the break which follows an opening parenthesis

Removing the last argument from a list stripped the trailing trivia of the
opening parenthesis, which is where Roslyn puts the newline that follows
it. Correct when the list empties - `(` and `)` should meet - but it fired
whenever the last argument went, so every call ending in a
CancellationToken lost the break before its first argument and kept the
indentation which had placed that argument at the start of a line.

Only strip it when nothing remains to put on that line. That leaves the
argument list broken where it was written to break, which in turn lets
unwrapping do the thing it could not do before: give the receiver the line
the argument it displaces was starting, and the comma between them the
break, rather than crowding the two onto the parenthesis's line.

    -WriteTo(stream, destination,
    +WriteTo(
    +        stream,
    +        destination,
             4096,
             progress: progress);

This closes the last item in #152.

One test changes meaning rather than merely output.
CSharp_14_ExtensionUnwrapsOntoOneLine asserted that a two argument call
collapsed onto a single line, which was never designed - it was this bug
removing the break the author had written. The generator preserves the
layout it is given everywhere else, so the test is renamed to
CSharp_14_ExtensionDropsTheChainBreakAfterTheReceiver, which is what it
was actually built to cover in #150.

Generated with Claude Code
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