From 0ac449e6c7610803fdc160394954d34cf9a6b01e Mon Sep 17 00:00:00 2001 From: Bela VanderVoort Date: Mon, 22 Jan 2024 02:42:49 -0600 Subject: [PATCH] Fixing regression issue with weird breaking of linq statement. (#1133) * Adding test case and tracking the code that caused the problem closes #1130 * Fixing regression with invocations * Fixing some edge cases * One last edge case * format file --- .../cs/MemberChain_PropertiesConsistent.test | 18 ++++-- .../TestFiles/cs/MemberChains.test | 23 +++++++- .../TestFiles/cs/VariableDeclarations.test | 5 +- .../InvocationExpression.cs | 55 +++++++++++++------ 4 files changed, 74 insertions(+), 27 deletions(-) diff --git a/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChain_PropertiesConsistent.test b/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChain_PropertiesConsistent.test index 22452adda..a11de94bc 100644 --- a/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChain_PropertiesConsistent.test +++ b/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChain_PropertiesConsistent.test @@ -25,14 +25,22 @@ var someVariable = this.Property.CallMethod(someValue => someValue.SomeProperty == someOtherValue___________________________ ); -var someVariable = this.Property() +var someVariable = this.CallMethod() .CallMethod(someValue => someValue.SomeProperty == someOtherValue___________________________); -var someVariable = this.Property.CallMethod(someValue => - someValue.SomeProperty == someOtherValue___________________________ -) +var someVariable = this.Array[1] + .CallMethod(someValue => someValue.SomeProperty == someOtherValue___________________________); + +var someVariable = this + .Property.CallMethod(someValue => + someValue.SomeProperty == someOtherValue___________________________ + ) + .CallMethod(); + +var someVariable = this.CallMethod() + .CallMethod(someValue => someValue.SomeProperty == someOtherValue___________________________) .CallMethod(); -var someVariable = this.Property() +var someVariable = this.Array[1] .CallMethod(someValue => someValue.SomeProperty == someOtherValue___________________________) .CallMethod(); diff --git a/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChains.test b/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChains.test index 86d8d5412..0bde1c590 100644 --- a/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChains.test +++ b/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChains.test @@ -235,8 +235,7 @@ class ClassName .CallMethod__________________(); someThing_______________________ - ?.Property - .CallMethod__________________() + ?.Property.CallMethod__________________() .CallMethod__________________(); someThing_______________________ @@ -245,5 +244,25 @@ class ClassName IEnumerable valueProviderFactories = new ModelBinderAttribute_______().GetValueProviderFactories(config); + + var something________________________________________ = x + .SomeProperty.CallMethod(longParameter_____________, longParameter_____________) + .CallMethod(); + + CallMethod(o => + o.Property.CallMethod_____________________________________________________() + .CallMethod_____________________________________________________() + ); + + CallMethod( + o, + o.Property.CallMethod_____________________________________________________() + .CallMethod_____________________________________________________() + ); + + var someValue = + someValue + && o.Property.CallMethod_____________________________________________________() + .CallMethod_____________________________________________________(); } } diff --git a/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/VariableDeclarations.test b/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/VariableDeclarations.test index 86b0fa116..a9396cf94 100644 --- a/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/VariableDeclarations.test +++ b/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/VariableDeclarations.test @@ -80,9 +80,8 @@ class ClassName elementAccessExpression ].theMember______________________________; - var someLongValue_________________ = memberAccessExpression[ - elementAccessExpression - ].theMember______________________________(); + var someLongValue_________________ = memberAccessExpression[elementAccessExpression] + .theMember______________________________(); var someLongValue_________________ = memberAccessExpression( elementAccessExpression diff --git a/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/InvocationExpression.cs b/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/InvocationExpression.cs index 0cc74f751..c2305d59d 100644 --- a/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/InvocationExpression.cs +++ b/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/InvocationExpression.cs @@ -18,6 +18,7 @@ public static Doc Print(InvocationExpressionSyntax node, FormattingContext conte public static Doc PrintMemberChain(ExpressionSyntax node, FormattingContext context) { + var parent = node.Parent; var printedNodes = new List(); FlattenAndPrintNodes(node, printedNodes, context); @@ -28,7 +29,7 @@ public static Doc PrintMemberChain(ExpressionSyntax node, FormattingContext cont var oneLine = groups.SelectMany(o => o).Select(o => o.Doc).ToArray(); - var shouldMergeFirstTwoGroups = ShouldMergeFirstTwoGroups(groups); + var shouldMergeFirstTwoGroups = ShouldMergeFirstTwoGroups(groups, parent); var cutoff = shouldMergeFirstTwoGroups ? 3 : 2; @@ -41,6 +42,7 @@ public static Doc PrintMemberChain(ExpressionSyntax node, FormattingContext cont o.Last().Node is not ( InvocationExpressionSyntax + or ElementAccessExpressionSyntax or PostfixUnaryExpressionSyntax { Operand: InvocationExpressionSyntax @@ -198,7 +200,7 @@ private static List> GroupPrintedNodesOnLines(List(); + currentGroup = []; groups.Add(currentGroup); } else if ( @@ -209,7 +211,7 @@ or IdentifierNameSyntax && printedNodes[index + -1].Node is not ConditionalAccessExpressionSyntax ) { - currentGroup = new List(); + currentGroup = []; groups.Add(currentGroup); } @@ -229,16 +231,14 @@ List printedNodes // will be grouped as // [ // [Identifier, InvocationExpression], - // [MemberAccessExpression] - // [MemberAccessExpression, InvocationExpression], + // [MemberAccessExpression], [MemberAccessExpression, InvocationExpression], // [MemberAccessExpression, InvocationExpression], // [MemberAccessExpression], // ] // so that we can print it as // a() - // .b - // .c() + // .b.c() // .d() // .e @@ -273,7 +273,7 @@ or PostfixUnaryExpressionSyntax } groups.Add(currentGroup); - currentGroup = new List(); + currentGroup = []; var hasSeenNodeThatRequiresBreak = false; for (; index < printedNodes.Count; index++) @@ -286,17 +286,13 @@ or ConditionalAccessExpressionSyntax ) { groups.Add(currentGroup); - currentGroup = new List(); + currentGroup = []; hasSeenNodeThatRequiresBreak = false; } if ( printedNodes[index].Node - is ( - InvocationExpressionSyntax - or ElementAccessExpressionSyntax - or MemberBindingExpressionSyntax - ) + is (InvocationExpressionSyntax or ElementAccessExpressionSyntax) ) { hasSeenNodeThatRequiresBreak = true; @@ -342,7 +338,10 @@ private static Doc PrintIndentedGroup(ExpressionSyntax node, IList> groups) + private static bool ShouldMergeFirstTwoGroups( + List> groups, + SyntaxNode? parent + ) { if (groups.Count < 2 || groups[0].Count != 1) { @@ -351,11 +350,33 @@ private static bool ShouldMergeFirstTwoGroups(List> groups) var firstNode = groups[0][0].Node; - if (firstNode is IdentifierNameSyntax { Identifier.Text.Length: <= 4 }) + if ( + firstNode + is not ( + IdentifierNameSyntax { Identifier.Text.Length: <= 4 } + or ThisExpressionSyntax + or PredefinedTypeSyntax + or BaseExpressionSyntax + ) + ) + { + return false; + } + + // TODO maybe some things to fix in here + // https://github.com/belav/csharpier-repos/pull/100/files + if ( + groups[1].Count == 1 + || parent is SimpleLambdaExpressionSyntax or ArgumentSyntax or BinaryExpressionSyntax + || groups[1].Skip(1).First().Node + is InvocationExpressionSyntax + or ElementAccessExpressionSyntax + or PostfixUnaryExpressionSyntax + ) { return true; } - return firstNode is ThisExpressionSyntax or PredefinedTypeSyntax or BaseExpressionSyntax; + return false; } }