Skip to content

Commit

Permalink
Fixing regression issue with weird breaking of linq statement. (#1133)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@belav
belav committed Jan 22, 2024
1 parent 56ff119 commit 0ac449e
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 27 deletions.
18 changes: 13 additions & 5 deletions Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChain_PropertiesConsistent.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
23 changes: 21 additions & 2 deletions Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChains.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,8 +235,7 @@ class ClassName
.CallMethod__________________();

someThing_______________________
?.Property
.CallMethod__________________()
?.Property.CallMethod__________________()
.CallMethod__________________();

someThing_______________________
Expand All @@ -245,5 +244,25 @@ class ClassName

IEnumerable<ValueProviderFactory> 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_____________________________________________________();
}
}
5 changes: 2 additions & 3 deletions Src/CSharpier.Tests/FormattingTests/TestFiles/cs/VariableDeclarations.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,8 @@ class ClassName
elementAccessExpression
].theMember______________________________;

var someLongValue_________________ = memberAccessExpression[
elementAccessExpression
].theMember______________________________();
var someLongValue_________________ = memberAccessExpression[elementAccessExpression]
.theMember______________________________();

var someLongValue_________________ = memberAccessExpression(
elementAccessExpression
Expand Down
55 changes: 38 additions & 17 deletions Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/InvocationExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<PrintedNode>();

FlattenAndPrintNodes(node, printedNodes, context);
Expand All @@ -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;

Expand All @@ -41,6 +42,7 @@ public static Doc PrintMemberChain(ExpressionSyntax node, FormattingContext cont
o.Last().Node
is not (
InvocationExpressionSyntax
or ElementAccessExpressionSyntax
or PostfixUnaryExpressionSyntax
{
Operand: InvocationExpressionSyntax
Expand Down Expand Up @@ -198,7 +200,7 @@ private static List<List<PrintedNode>> GroupPrintedNodesOnLines(List<PrintedNode
{
if (printedNodes[index].Node is ConditionalAccessExpressionSyntax)
{
currentGroup = new List<PrintedNode>();
currentGroup = [];
groups.Add(currentGroup);
}
else if (
Expand All @@ -209,7 +211,7 @@ or IdentifierNameSyntax
&& printedNodes[index + -1].Node is not ConditionalAccessExpressionSyntax
)
{
currentGroup = new List<PrintedNode>();
currentGroup = [];
groups.Add(currentGroup);
}

Expand All @@ -229,16 +231,14 @@ List<PrintedNode> 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

Expand Down Expand Up @@ -273,7 +273,7 @@ or PostfixUnaryExpressionSyntax
}

groups.Add(currentGroup);
currentGroup = new List<PrintedNode>();
currentGroup = [];

var hasSeenNodeThatRequiresBreak = false;
for (; index < printedNodes.Count; index++)
Expand All @@ -286,17 +286,13 @@ or ConditionalAccessExpressionSyntax
)
{
groups.Add(currentGroup);
currentGroup = new List<PrintedNode>();
currentGroup = [];
hasSeenNodeThatRequiresBreak = false;
}

if (
printedNodes[index].Node
is (
InvocationExpressionSyntax
or ElementAccessExpressionSyntax
or MemberBindingExpressionSyntax
)
is (InvocationExpressionSyntax or ElementAccessExpressionSyntax)
)
{
hasSeenNodeThatRequiresBreak = true;
Expand Down Expand Up @@ -342,7 +338,10 @@ private static Doc PrintIndentedGroup(ExpressionSyntax node, IList<List<PrintedN
this.CallMethod()
.CallMethod();
*/
private static bool ShouldMergeFirstTwoGroups(List<List<PrintedNode>> groups)
private static bool ShouldMergeFirstTwoGroups(
List<List<PrintedNode>> groups,
SyntaxNode? parent
)
{
if (groups.Count < 2 || groups[0].Count != 1)
{
Expand All @@ -351,11 +350,33 @@ private static bool ShouldMergeFirstTwoGroups(List<List<PrintedNode>> 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;
}
}

0 comments on commit 0ac449e

Please sign in to comment.