Skip to content
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

Fixing regression issue with weird breaking of linq statement. #1133

Merged
merged 5 commits into from
Jan 22, 2024
Merged

Fixing regression issue with weird breaking of linq statement. #1133

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
50d8502
Adding test case and tracking the code that caused the problem
belav Jan 17, 2024
6260db7
Fixing regression with invocations
belav Jan 21, 2024
2c62f4f
Fixing some edge cases
belav Jan 21, 2024
0492259
One last edge case
belav Jan 21, 2024
e9a2176
format file
belav Jan 21, 2024
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
18 changes: 13 additions & 5 deletions Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChain_PropertiesConsistent.test
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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;
}
}
Loading