From 5a937a71ec5550f4cfd5ca184f566643e81e2725 Mon Sep 17 00:00:00 2001 From: Dmitry Zhavoronkov Date: Wed, 13 Dec 2023 05:27:56 +0700 Subject: [PATCH] feat: Improve formatting of lambda expressions (#1066) --- .../TestFiles/cs/ArgumentLists.test | 41 +++++ .../TestFiles/cs/BinaryExpressions.test | 11 +- .../cs/MemberChain_PropertiesConsistent.test | 4 +- .../TestFiles/cs/MemberChains.test | 10 +- .../TestFiles/cs/SimpleLambdaExpressions.test | 142 +++++++++++++++++- .../TestFiles/cs/VariableDeclarations.test | 4 +- .../SyntaxPrinter/ArgumentListLikeSyntax.cs | 80 +++++++--- .../SyntaxNodePrinters/Argument.cs | 11 +- .../ParenthesizedLambdaExpression.cs | 42 +++--- .../SimpleLambdaExpression.cs | 24 ++- 10 files changed, 299 insertions(+), 70 deletions(-) diff --git a/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/ArgumentLists.test b/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/ArgumentLists.test index 9ac5f6f5d..63bbbbfc8 100644 --- a/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/ArgumentLists.test +++ b/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/ArgumentLists.test @@ -5,5 +5,46 @@ public class ClassName this.Method(true); this.NamedArguments(b: true, c: "false"); + + this.ToDictionary(o => o.Key, o => new { o.Prop }); + + this.ToDictionary(o => o.Key, o => new Entity { o.Prop }); + + this.ToDictionary_____________( + o => o.VeryLongKey_________________________, + o => new { o.Prop } + ); + + this.ToDictionary_____________( + o => o.VeryLongKey_________________________, + o => new Entity { o.Prop } + ); + + this.ToDictionary_____________( + keySelector: static o => o.Key, + elementSelector: static o => new Entity { o.Prop } + ); + + this.ToDictionary_____________( + keySelector: static o => o.Key, + elementSelector: static o => new Entity + { + o.LongName_______________________, + o.LongName_______________________ + } + ); + + this.GroupBy_____________( + keySelector: static o => new Entity + { + o.LongName_______________________, + o.LongName_______________________ + }, + resultSelector: static o => new Entity + { + o.LongName_______________________, + o.LongName_______________________ + } + ); } } diff --git a/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/BinaryExpressions.test b/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/BinaryExpressions.test index f398085ba..4ee21ee72 100644 --- a/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/BinaryExpressions.test +++ b/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/BinaryExpressions.test @@ -95,12 +95,11 @@ class TestClass "SecondParameter" ); - var y = someList.Where( - o => - someLongValue_______________________ - && theseShouldNotIndent_________________ - && theseShouldNotIndent_________________ - > butThisOneShould_________________________________________ + var y = someList.Where(o => + someLongValue_______________________ + && theseShouldNotIndent_________________ + && theseShouldNotIndent_________________ + > butThisOneShould_________________________________________ ); var someVariable = diff --git a/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChain_PropertiesConsistent.test b/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChain_PropertiesConsistent.test index 2dda4e8e7..8cf778ee9 100644 --- a/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChain_PropertiesConsistent.test +++ b/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChain_PropertiesConsistent.test @@ -17,8 +17,8 @@ var someVariable = someObject .CallMethod(); // TODO too hard to change this for now, will do it in https://github.com/belav/csharpier/issues/451 -var someVariable = this.Property.CallMethod( - someValue => someValue.SomeProperty == someOtherValue___________________________ +var someVariable = this.Property.CallMethod(someValue => + someValue.SomeProperty == someOtherValue___________________________ ); var someVariable = this.Property() diff --git a/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChains.test b/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChains.test index aa0d4309a..f43984917 100644 --- a/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChains.test +++ b/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChains.test @@ -78,9 +78,8 @@ class ClassName roleNames .ToList() - .ForEach( - role => - this.SomeProperty.Setup(o => longThing_______________________________________) + .ForEach(role => + this.SomeProperty.Setup(o => longThing_______________________________________) ); roleNames @@ -115,9 +114,8 @@ class ClassName var someVariable = someObject .Property - .CallMethod( - someValue => - someValue.SomeProperty == someOtherValue___________________________________ + .CallMethod(someValue => + someValue.SomeProperty == someOtherValue___________________________________ ); CallMethod( diff --git a/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/SimpleLambdaExpressions.test b/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/SimpleLambdaExpressions.test index 445751f47..8abb07f9c 100644 --- a/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/SimpleLambdaExpressions.test +++ b/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/SimpleLambdaExpressions.test @@ -8,18 +8,146 @@ public class ClassName this.WhereAsync(static o => true); - this.Where___________________( - longName__________________ => someOtherLongName__________________ + this.Select(o => new { o.Prop }); + + this.Select(o => new Entity { o.Prop }); + + this.Select(static o => new { o.Prop }); + + this.Select(static o => new Entity { o.Prop }); + + this.Select(selector: o => new { o.Prop }); + + this.Select(selector: o => new Entity { o.Prop }); + + this.Select(selector: static o => new { o.Prop }); + + this.Select(selector: static o => new Entity { o.Prop }); + + this.Where(x => x.IsSomething).Select(static o => new Entity { o.Prop }); + + this.Where(x => x.IsLongSomething______________________) + .Select(static o => new { o.Prop, }); + + this.Where(x => x.IsLongSomething______________________) + .Select(selector: static o => new { o.Prop, }); + + this.Where(x => x.IsLongSomething________________) + .Select(static o => new Entity { o.Prop, }); + + this.Where(x => x.IsSomeThing) + .Select(selector: static o => new + { + o.LongName_______________________, + o.LongName_______________________ + }); + + this.Where(x => x.IsSomeThing) + .Select(selector: static o => new Entity + { + o.LongName_______________________, + o.LongName_______________________ + }); + + this.Select_________________________________( + superLongName________________________________ => new + { + Prop = superLongName________________________________, + } ); - this.Where___________________( - superLongName________________________________ => - someOtherLongName_______________________________ + this.Select_________________________________( + superLongName_________________________________ => new Entity + { + Prop = superLongName________________________________, + } + ); + + this.Select_________________________________( + selector: static longName__________________ => new + { + Prop__________ = longName__________________ + } + ); + + this.Select_________________________________( + selector: static longName__________________ => new Entity( + LongArgument_______________________, + LongArgument_______________________ + ) + { + Prop__________ = longName__________________ + } + ); + + this.Select_________________________________(o => new Entity + { + o.LongName________________________________, + }); + + this.Select_________________________________(static o => new + { + o.LongName________________________________, + }); + + this.Select_________________________________(static o => new Entity + { + o.LongName________________________________, + }); + + this.Where(x => x.IsSomeThing) + .Select(selector: static o => new Entity( + LongArgument_______________________, + LongArgument_______________________ + ) + { + o.LongName_______________________, + o.LongName_______________________ + }); + + this.Where(t => + t.SomeLongColumn________________________ || t.OtherLongColumn_______________________ + ); + + this.Where(t => + t.SomeVeryLongColumn________________________________ + || t.OtherLongColumn________________________________ ); - this.Where___________________(superLongName________________________________ => + this.Select_________________________________________( + superLongName________________________________ => true + ); + this.Select_________________________________________(selector: static longName________ => + true + ); + + this.Select_________________________________________( + selector: static longName______________ => + someOtherLongName________________________________ + ); + + this.Where___________________(x => + { + return x; + }); + + this.Where___________________(selector: static x => { - return someOtherLongName__________________; + return x; }); + + this.Where______________________________________( + superLongName________________________________ => + { + return someOtherLongName__________________; + } + ); + + this.Where______________________________________( + selector: static longName_________________ => + { + return someOtherLongName__________________; + } + ); } } diff --git a/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/VariableDeclarations.test b/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/VariableDeclarations.test index 60fa1ac67..218e43743 100644 --- a/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/VariableDeclarations.test +++ b/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/VariableDeclarations.test @@ -120,8 +120,8 @@ class ClassName : null ) { } - var variableDeclarator = conditionalAccessExpression?.invocationExpression( - o => someLongValue___________________________________ + var variableDeclarator = conditionalAccessExpression?.invocationExpression(o => + someLongValue___________________________________ ); var variableDeclarator = (CastExpression) diff --git a/Src/CSharpier/SyntaxPrinter/ArgumentListLikeSyntax.cs b/Src/CSharpier/SyntaxPrinter/ArgumentListLikeSyntax.cs index 63dfeeae9..c6f0bac79 100644 --- a/Src/CSharpier/SyntaxPrinter/ArgumentListLikeSyntax.cs +++ b/Src/CSharpier/SyntaxPrinter/ArgumentListLikeSyntax.cs @@ -11,28 +11,70 @@ FormattingContext context { var docs = new List { Token.Print(openParenToken, context) }; - if ( - arguments.Count == 1 - && arguments[0].Expression - is ParenthesizedLambdaExpressionSyntax + switch (arguments) + { + case [{ Expression: SimpleLambdaExpressionSyntax lambda } arg]: + { + docs.Add( + Doc.GroupWithId( + "LambdaArguments", + Doc.Indent( + Doc.SoftLine, + Argument.PrintModifiers(arg, context), + SimpleLambdaExpression.PrintHead(lambda, context) + ) + ), + Doc.IndentIfBreak( + SimpleLambdaExpression.PrintBody(lambda, context), + "LambdaArguments" + ), + lambda.Body + is BlockSyntax + or ObjectCreationExpressionSyntax + or AnonymousObjectCreationExpressionSyntax + ? Doc.IfBreak(Doc.SoftLine, Doc.Null, "LambdaArguments") + : Doc.SoftLine + ); + break; + } + case [ + { + Expression: ParenthesizedLambdaExpressionSyntax { ParameterList.Parameters.Count: 0, Block: { } - } - or SimpleLambdaExpressionSyntax { Block: { } } - ) - { - docs.Add(Argument.Print(arguments[0], context)); - } - else if (arguments.Any()) - { - docs.Add( - Doc.Indent( - Doc.SoftLine, - SeparatedSyntaxList.Print(arguments, Argument.Print, Doc.Line, context) - ), - Doc.SoftLine - ); + } lambda + } arg + ]: + { + docs.Add( + Doc.GroupWithId( + "LambdaArguments", + Doc.Indent( + Doc.SoftLine, + Argument.PrintModifiers(arg, context), + ParenthesizedLambdaExpression.PrintHead(lambda, context) + ) + ), + Doc.IndentIfBreak( + ParenthesizedLambdaExpression.PrintBody(lambda, context), + "LambdaArguments" + ), + Doc.IfBreak(Doc.SoftLine, Doc.Null, "LambdaArguments") + ); + break; + } + default: + { + docs.Add( + Doc.Indent( + Doc.SoftLine, + SeparatedSyntaxList.Print(arguments, Argument.Print, Doc.Line, context) + ), + Doc.SoftLine + ); + break; + } } docs.Add(Token.Print(closeParenToken, context)); diff --git a/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/Argument.cs b/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/Argument.cs index 2978bbc7b..fad77a87d 100644 --- a/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/Argument.cs +++ b/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/Argument.cs @@ -4,7 +4,15 @@ internal static class Argument { public static Doc Print(ArgumentSyntax node, FormattingContext context) { - var docs = new List(); + return Doc.Concat( + Argument.PrintModifiers(node, context), + Node.Print(node.Expression, context) + ); + } + + public static Doc PrintModifiers(ArgumentSyntax node, FormattingContext context) + { + var docs = new List(2); if (node.NameColon != null) { docs.Add(BaseExpressionColon.Print(node.NameColon, context)); @@ -15,7 +23,6 @@ public static Doc Print(ArgumentSyntax node, FormattingContext context) docs.Add(Token.PrintWithSuffix(node.RefKindKeyword, " ", context)); } - docs.Add(Node.Print(node.Expression, context)); return Doc.Concat(docs); } } diff --git a/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/ParenthesizedLambdaExpression.cs b/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/ParenthesizedLambdaExpression.cs index 78b36c4bf..b3d6fd577 100644 --- a/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/ParenthesizedLambdaExpression.cs +++ b/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/ParenthesizedLambdaExpression.cs @@ -1,11 +1,17 @@ +using System.Diagnostics; + namespace CSharpier.SyntaxPrinter.SyntaxNodePrinters; internal static class ParenthesizedLambdaExpression { public static Doc Print(ParenthesizedLambdaExpressionSyntax node, FormattingContext context) { - var docs = new List - { + return Doc.Concat(PrintHead(node, context), PrintBody(node, context)); + } + + public static Doc PrintHead(ParenthesizedLambdaExpressionSyntax node, FormattingContext context) + { + return Doc.Concat( AttributeLists.Print(node, node.AttributeLists, context), Modifiers.Print(node.Modifiers, context), node.ReturnType != null @@ -14,25 +20,19 @@ public static Doc Print(ParenthesizedLambdaExpressionSyntax node, FormattingCont ParameterList.Print(node.ParameterList, context), " ", Token.Print(node.ArrowToken, context) - }; - if (node.ExpressionBody != null) - { - docs.Add(Doc.Group(Doc.Indent(Doc.Line, Node.Print(node.ExpressionBody, context)))); - } - else if (node.Block != null) - { - if (node.Block.Statements.Count > 0) - { - docs.Add(Doc.HardLine); - } - else - { - docs.Add(" "); - } - - docs.Add(Block.Print(node.Block, context)); - } + ); + } - return Doc.Concat(docs); + public static Doc PrintBody(ParenthesizedLambdaExpressionSyntax node, FormattingContext context) + { + return node.Body switch + { + BlockSyntax block + => Doc.Concat( + block.Statements.Count > 0 ? Doc.HardLine : " ", + Block.Print(block, context) + ), + _ => Doc.Group(Doc.Indent(Doc.Line, Node.Print(node.Body, context))), + }; } } diff --git a/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/SimpleLambdaExpression.cs b/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/SimpleLambdaExpression.cs index 61b7a5902..414c0aa95 100644 --- a/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/SimpleLambdaExpression.cs +++ b/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/SimpleLambdaExpression.cs @@ -4,14 +4,28 @@ internal static class SimpleLambdaExpression { public static Doc Print(SimpleLambdaExpressionSyntax node, FormattingContext context) { - return Doc.Group( + return Doc.Concat(PrintHead(node, context), PrintBody(node, context)); + } + + public static Doc PrintHead(SimpleLambdaExpressionSyntax node, FormattingContext context) + { + return Doc.Concat( Modifiers.Print(node.Modifiers, context), Node.Print(node.Parameter, context), " ", - Token.Print(node.ArrowToken, context), - node.Body is BlockSyntax blockSyntax - ? Block.Print(blockSyntax, context) - : Doc.Indent(Doc.Line, Node.Print(node.Body, context)) + Token.Print(node.ArrowToken, context) ); } + + public static Doc PrintBody(SimpleLambdaExpressionSyntax node, FormattingContext context) + { + return node.Body switch + { + BlockSyntax blockSyntax => Block.Print(blockSyntax, context), + ObjectCreationExpressionSyntax + or AnonymousObjectCreationExpressionSyntax + => Doc.Group(" ", Node.Print(node.Body, context)), + _ => Doc.Indent(Doc.Group(Doc.Line, Node.Print(node.Body, context))) + }; + } }