From adca2788aaf7ac426a823119c5214be4eb8a8058 Mon Sep 17 00:00:00 2001 From: Victor Irzak Date: Thu, 27 Aug 2026 07:19:31 -0400 Subject: [PATCH 1/2] 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 --- tests/Generator.Tests/ArgumentTests.cs | 17 +++++++++++++++++ ...ropped#CallProgressMethodAsync.g.verified.cs | 7 +++++++ 2 files changed, 24 insertions(+) create mode 100644 tests/Generator.Tests/Snapshots/ArgumentTests.KeepLineBreaksWhenTheLastArgumentIsDropped#CallProgressMethodAsync.g.verified.cs diff --git a/tests/Generator.Tests/ArgumentTests.cs b/tests/Generator.Tests/ArgumentTests.cs index 2a58052..198fa9c 100644 --- a/tests/Generator.Tests/ArgumentTests.cs +++ b/tests/Generator.Tests/ArgumentTests.cs @@ -27,5 +27,22 @@ public async Task CallProgressMethodAsync() { await ProgressMethodAsync(progress: null); } +""".Verify(); + + [Fact] + public Task KeepLineBreaksWhenTheLastArgumentIsDropped() => """ +public void ProgressMethod(int p1, int p2) { } + +public async Task ProgressMethodAsync(int p1, int p2, CancellationToken cancellationToken) => await Task.CompletedTask; + +[Zomp.SyncMethodGenerator.CreateSyncVersion] +public async Task CallProgressMethodAsync(CancellationToken cancellationToken) +{ + await ProgressMethodAsync( + 1, + 2, + cancellationToken + ); +} """.Verify(); } diff --git a/tests/Generator.Tests/Snapshots/ArgumentTests.KeepLineBreaksWhenTheLastArgumentIsDropped#CallProgressMethodAsync.g.verified.cs b/tests/Generator.Tests/Snapshots/ArgumentTests.KeepLineBreaksWhenTheLastArgumentIsDropped#CallProgressMethodAsync.g.verified.cs new file mode 100644 index 0000000..a2e9486 --- /dev/null +++ b/tests/Generator.Tests/Snapshots/ArgumentTests.KeepLineBreaksWhenTheLastArgumentIsDropped#CallProgressMethodAsync.g.verified.cs @@ -0,0 +1,7 @@ +//HintName: Test.Class.CallProgressMethodAsync.g.cs +public void CallProgressMethod() +{ + ProgressMethod( + 1, + 2); +} From f0ffc4f74943442d7f9758a1b0a4e8b0f59442b9 Mon Sep 17 00:00:00 2001 From: Victor Irzak Date: Thu, 27 Aug 2026 07:23:45 -0400 Subject: [PATCH 2/2] 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 --- .../AsyncToSyncRewriter.cs | 38 +++++++++++++++---- tests/Generator.Tests/ExtensionMethodTests.cs | 2 +- ...Callers.ext.DrainTwiceAsync.g.verified.cs} | 4 +- ....StreamCallers.ext.CopyAsync.g.verified.cs | 4 +- 4 files changed, 37 insertions(+), 11 deletions(-) rename tests/Generator.Tests/Snapshots/{ExtensionMethodTests.CSharp_14_ExtensionUnwrapsOntoOneLine#Callers.StreamCallers.ext.DrainTwiceAsync.g.verified.cs => ExtensionMethodTests.CSharp_14_ExtensionDropsTheChainBreakAfterTheReceiver#Callers.StreamCallers.ext.DrainTwiceAsync.g.verified.cs} (72%) diff --git a/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs b/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs index 33ab7f7..f975add 100644 --- a/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs +++ b/src/Zomp.SyncMethodGenerator/AsyncToSyncRewriter.cs @@ -1159,9 +1159,14 @@ bool ShouldRemoveArgumentLocal(ArgumentSyntax arg, int index) if (invalid.Contains(node.Arguments.Count - 1)) { - retval = retval - .WithCloseParenToken(@base.CloseParenToken.WithLeadingTrivia()) - .WithOpenParenToken(@base.OpenParenToken.WithTrailingTrivia()); + retval = retval.WithCloseParenToken(@base.CloseParenToken.WithLeadingTrivia()); + + // The newline which followed the opening parenthesis belongs to whichever argument + // now comes first, so it only goes when nothing is left to put on that line. + if (newParams.Count == 0) + { + retval = retval.WithOpenParenToken(@base.OpenParenToken.WithTrailingTrivia()); + } } return retval; @@ -2021,23 +2026,40 @@ private InvocationExpressionSyntax UnwrapExtension(InvocationExpressionSyntax ie var arguments = ies.ArgumentList.Arguments; var separators = arguments.GetSeparators(); + // A list broken after its opening parenthesis gives each argument a line of its own. The + // receiver is about to become the first of them, so it wants the line the argument it + // displaces was starting, rather than the one the parenthesis is on. + var lineBreak = ies.ArgumentList.OpenParenToken.TrailingTrivia + .LastOrDefault(static t => t.IsKind(SyntaxKind.EndOfLineTrivia)); + var brokenAfterOpenParen = lineBreak != default; + SyntaxToken[] newSeparators = arguments.Count < 1 ? [] - : [Token(SyntaxKind.CommaToken).AppendSpace(), .. separators]; + : [brokenAfterOpenParen + ? Token(SyntaxKind.CommaToken).WithTrailingTrivia(lineBreak) + : Token(SyntaxKind.CommaToken).AppendSpace(), .. separators]; // The receiver becomes the first argument, so whatever separated it from the dot - a // line break in a chained call - would otherwise land between it and the comma which // now follows. var @as = Argument(expression.WithoutTrivia()); - // The argument the receiver displaces is no longer the first thing on its line, so - // indentation which was written to place it at the start of one only leaves a gap after - // the comma. Indentation which still follows a line break is left alone. + // The argument the receiver displaces either keeps the line it was starting, in which + // case the receiver takes its indentation and the comma between them takes the break, or + // it no longer starts one, in which case indentation written to place it at the start of + // a line only leaves a gap after the comma. List newArguments = [.. arguments]; if (newArguments is [var displaced, ..] && displaced.GetLeadingTrivia() is { Count: > 0 } leading && leading.All(static t => t.IsKind(SyntaxKind.WhitespaceTrivia))) { - newArguments[0] = displaced.WithoutLeadingTrivia(); + if (brokenAfterOpenParen) + { + @as = @as.WithLeadingTrivia(leading); + } + else + { + newArguments[0] = displaced.WithoutLeadingTrivia(); + } } var newList = SeparatedList([@as, .. newArguments], newSeparators); diff --git a/tests/Generator.Tests/ExtensionMethodTests.cs b/tests/Generator.Tests/ExtensionMethodTests.cs index 913011b..1ff1d3c 100644 --- a/tests/Generator.Tests/ExtensionMethodTests.cs +++ b/tests/Generator.Tests/ExtensionMethodTests.cs @@ -215,7 +215,7 @@ public async Task CopyAsync( #if NET8_0_OR_GREATER [Fact] - public Task CSharp_14_ExtensionUnwrapsOntoOneLine() => """ + public Task CSharp_14_ExtensionDropsTheChainBreakAfterTheReceiver() => """ namespace Helpers { internal static partial class StreamExtensions diff --git a/tests/Generator.Tests/Snapshots/ExtensionMethodTests.CSharp_14_ExtensionUnwrapsOntoOneLine#Callers.StreamCallers.ext.DrainTwiceAsync.g.verified.cs b/tests/Generator.Tests/Snapshots/ExtensionMethodTests.CSharp_14_ExtensionDropsTheChainBreakAfterTheReceiver#Callers.StreamCallers.ext.DrainTwiceAsync.g.verified.cs similarity index 72% rename from tests/Generator.Tests/Snapshots/ExtensionMethodTests.CSharp_14_ExtensionUnwrapsOntoOneLine#Callers.StreamCallers.ext.DrainTwiceAsync.g.verified.cs rename to tests/Generator.Tests/Snapshots/ExtensionMethodTests.CSharp_14_ExtensionDropsTheChainBreakAfterTheReceiver#Callers.StreamCallers.ext.DrainTwiceAsync.g.verified.cs index 9d45d3f..ec81e9a 100644 --- a/tests/Generator.Tests/Snapshots/ExtensionMethodTests.CSharp_14_ExtensionUnwrapsOntoOneLine#Callers.StreamCallers.ext.DrainTwiceAsync.g.verified.cs +++ b/tests/Generator.Tests/Snapshots/ExtensionMethodTests.CSharp_14_ExtensionDropsTheChainBreakAfterTheReceiver#Callers.StreamCallers.ext.DrainTwiceAsync.g.verified.cs @@ -10,7 +10,9 @@ public static partial class StreamCallers extension(global::System.IO.Stream stream) { public void DrainTwice() => - global::Helpers.StreamExtensions.Drain(stream, 1024); + global::Helpers.StreamExtensions.Drain( + stream, + 1024); } } } diff --git a/tests/Generator.Tests/Snapshots/ExtensionMethodTests.CSharp_14_ExtensionKeepsRemainingArgumentsOnTheirLines#Callers.StreamCallers.ext.CopyAsync.g.verified.cs b/tests/Generator.Tests/Snapshots/ExtensionMethodTests.CSharp_14_ExtensionKeepsRemainingArgumentsOnTheirLines#Callers.StreamCallers.ext.CopyAsync.g.verified.cs index a006646..7011e96 100644 --- a/tests/Generator.Tests/Snapshots/ExtensionMethodTests.CSharp_14_ExtensionKeepsRemainingArgumentsOnTheirLines#Callers.StreamCallers.ext.CopyAsync.g.verified.cs +++ b/tests/Generator.Tests/Snapshots/ExtensionMethodTests.CSharp_14_ExtensionKeepsRemainingArgumentsOnTheirLines#Callers.StreamCallers.ext.CopyAsync.g.verified.cs @@ -13,7 +13,9 @@ public void Copy( global::System.IO.Stream destination, global::System.IProgress? progress = null ) => - global::Helpers.StreamExtensions.WriteTo(stream, destination, + global::Helpers.StreamExtensions.WriteTo( + stream, + destination, 4096, progress: progress); }