Skip to content

Avoid chopping single trailing lambda #1876

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
mattwzawislak wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Avoid chopping single trailing lambda #1876

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9510e66
#1865 - Add in feature idea for not chopping the last and only lambda
mattwzawislak Jun 13, 2026
2edb528
#1865 - "no pointless comments" scared me so I removed all of them
mattwzawislak Jun 13, 2026
184ada0
#1865 - Fix two broken tests and add a few
mattwzawislak Jun 13, 2026
8fc28da
Merge branch 'main' into feature/1865-dont-chop-lambda
belav Jul 4, 2026
fadd080
Merge branch 'main' into feature/1865-dont-chop-lambda
belav Jul 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions Src/CSharpier.Core/CSharp/SyntaxPrinter/ArgumentListLikeSyntax.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using CSharpier.Core.DocTypes;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Text;

namespace CSharpier.Core.CSharp.SyntaxPrinter;

Expand Down Expand Up @@ -78,6 +79,21 @@ lambda.Block is not null
{
args = SeparatedSyntaxList.Print(arguments, Argument.Print, Doc.Line, context);
}
else if (
arguments.Count > 1
&& arguments[^1].Expression is LambdaExpressionSyntax lastLambda
&& !arguments
.Take(arguments.Count - 1)
.Any(o => o.Expression is AnonymousFunctionExpressionSyntax)
&& !openParenToken
.Parent!.DescendantTrivia(
TextSpan.FromBounds(openParenToken.SpanStart, lastLambda.ArrowToken.SpanStart)
)
.Any(o => o.IsComment() || o.IsDirective)
)
{
args = ArgumentListWithTrailingLambda.Print(arguments, lastLambda, context);
}
else if (arguments.Count > 0)
{
args = Doc.Concat(
Expand Down
75 changes: 75 additions & 0 deletions Src/CSharpier.Core/CSharp/SyntaxPrinter/ArgumentListWithTrailingLambda.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
using CSharpier.Core.CSharp.SyntaxPrinter.SyntaxNodePrinters;
using CSharpier.Core.DocTypes;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;

namespace CSharpier.Core.CSharp.SyntaxPrinter;

internal static class ArgumentListWithTrailingLambda
{
public static Doc Print(
SeparatedSyntaxList<ArgumentSyntax> arguments,
LambdaExpressionSyntax lastLambda,
PrintingContext context
)
{
var chop = Doc.Concat(
Doc.Indent(
Doc.SoftLine,
SeparatedSyntaxList.Print(arguments, Argument.Print, Doc.Line, context)
),
Doc.SoftLine
);

Doc lambdaHead;
Doc lambdaBody;
bool bodyIsBraced;
if (lastLambda is SimpleLambdaExpressionSyntax simpleLambda)
{
bodyIsBraced =
simpleLambda.Body
is BlockSyntax
or ObjectCreationExpressionSyntax
or AnonymousObjectCreationExpressionSyntax;

lambdaHead = SimpleLambdaExpression.PrintHead(simpleLambda, context);
lambdaBody = SimpleLambdaExpression.PrintBody(simpleLambda, context);
}
else if (lastLambda is ParenthesizedLambdaExpressionSyntax parenthesizedLambda)
{
bodyIsBraced = parenthesizedLambda.Block is not null;
lambdaHead = ParenthesizedLambdaExpression.PrintHead(parenthesizedLambda, context);
lambdaBody = bodyIsBraced
? ParenthesizedLambdaExpression.PrintBody(parenthesizedLambda, context)
: Doc.Indent(Doc.Line, Node.Print(parenthesizedLambda.Body, context));
}
else
{
return chop;
}

var flatParts = new List<Doc>((arguments.Count - 1) * 3 + 2);
for (var x = 0; x < arguments.Count - 1; x++)
{
flatParts.Add(Argument.Print(arguments[x], context));
flatParts.Add(Token.Print(arguments.GetSeparator(x), context));
flatParts.Add(" ");
}

flatParts.Add(Argument.PrintModifiers(arguments[^1], context));
flatParts.Add(lambdaHead);

if (flatParts.Any(DocUtilities.ContainsBreak))
{
return chop;
}

var wrap = Doc.Concat(
Doc.ForceFlat(flatParts),
lambdaBody,
bodyIsBraced ? Doc.Null : Doc.SoftLine
);

return Doc.ConditionalGroup(wrap, wrap, chop);

@belav belav Jul 4, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should just be return Doc.ConditionalGroup(wrap, chop);

Passing the same doc twice just ends up doing more work in the case where chop is what is used.

@mattwzawislak mattwzawislak Jul 5, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because the first wrap is PrintMode.Flat and the second is PrintMode.Break where it breaks the lambda. DocPrinter.ProcessGroup skips the first option for a ConditionalGroup

@belav belav Jul 17, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, basically all of that conditional group code was ported from prettier and I had completely forgotten how it actually gets processed.

}
}
5 changes: 2 additions & 3 deletions Src/CSharpier.Tests/FormattingTests/TestFiles/cs/ObnoxiousEdgeCases.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@ class ClassName
internal StackObjectPool<
Dictionary<object, KeyedItemInfo>
> KeyedItemInfoDictionaryPool { get; } =
new StackObjectPool<Dictionary<object, KeyedItemInfo>>(
maxPreservedItems: 10,
() => new Dictionary<object, KeyedItemInfo>()
new StackObjectPool<Dictionary<object, KeyedItemInfo>>(maxPreservedItems: 10, () =>
new Dictionary<object, KeyedItemInfo>()
);

RenderFragment<AuthenticationState> customNotAuthorized = state =>
Expand Down
18 changes: 18 additions & 0 deletions Src/CSharpier.Tests/FormattingTests/TestFiles/cs/ParenthesizedLambdaExpressions.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,26 @@ public class ClassName
CallAnotherMethod______________________________________________________123()
);

CallMethod(CallAnotherMethod_________________(), () =>
CallAnotherMethod_________________()
);
Comment on lines +107 to +109

@belav belav Jul 4, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't agree with this breaking this way, and prettier also avoids breaking this.

// input & expected output
        var antiforgeryMiddleware = new AntiforgeryMiddleware(
            antiforgeryService.Object,
            hc => Task.CompletedTask
        );

// this pr
        var antiforgeryMiddleware = new AntiforgeryMiddleware(antiforgeryService.Object, hc =>
            Task.CompletedTask
        );

This is what I think should be breaking, and prettier does it this way. The pr works this way currently but I don't see a test for it.

// input & expected output
        Assert.Collection(receiveAuthStateDiff.Edits, edit =>
        {
            Assert.Equal(RenderTreeEditType.PrependFrame, edit.Type);
            AssertFrame.Text(
                batch.ReferenceFrames[edit.ReferenceFrameIndex],
                "Authenticated: True; Name: Bert; Pending: False; Renders: 1"
            );
        });

I just did a quick scan of https://github.com/belav/csharpier-repos/pull/172/changes and ran into that first case right away.

@mattwzawislak mattwzawislak Jul 5, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an important part of this PR since it is where the lambdas will usually start to blow up:

Before:

public void ConfigureOptions()
{
    services.Configure<MyOptions>(
        "Primary",
        options =>
            options.SetName("primary").SetTimeout(TimeSpan.FromSeconds(30)).EnableRetries()
    );
}

With this change:

public void ConfigureOptions()
{
  services.Configure<MyOptions>("Primary", options =>
      options.SetName("primary").SetTimeout(TimeSpan.FromSeconds(30)).EnableRetries()
  );
}

In either way, CSharpier will start to add line breaks if the chaining goes on too long but the start of that method call is what changes and the method calls are on 1 lower indent level.

Is there a point where you think the body of the lambda is large enough to justify wrapping instead of chopping?

@belav belav Jul 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Poking around prettier some more it looks like they are actually inconsistent with how they deal with lambdas depending on if the lambda ends in a method or a property.

new AntiforgeryMiddleware(
  antiforgeryService.Object,
  (hc) => Task.CompletedTask____________________________________________,
);

new AntiforgeryMiddleware(antiforgeryService.Object, (hc) =>
  Task.CompletedTask____________________________________________(),
);

And with your example this pr definitely improves things.

I think ideally csharpier would do this - but I don't know that it is possible. It would need to understand that if the lambda is long enough that it is going to break after the => to not break before options

services.Configure<MyOptions>(
    "Primary",
    options => options.SetName("primary").SetTimeout(TimeSpan.FromSeconds(30)).EnableRetries()
);

services.Configure<MyOptions>("Primary", options => 
    options.SetName("primary").SetTimeout(TimeSpan.FromSeconds(30)).EnableRetries________()
);

I think I've been convinced. Let me just review more of the csharpier-repos code to see if there are any other edge cases to look at.


CallMethod(
CallAnotherMethod_________________(),
// Comment
() => CallAnotherMethod_________________()
);

CallMethod(
#if DEBUG
CallAnotherMethod_________________(),
#endif
() => CallAnotherMethod_________________()
);

CallMethod(
CallAnotherMethod_________________(),
() => CallAnotherMethod_________________(),
() => CallAnotherMethod_________________()
);
}
Expand Down