Skip to content

Commit

Permalink
Getting element access to break more consistent in long invocation ch…
Browse files Browse the repository at this point in the history 
…ains. (#967)

* notes

* Getting element access to break more consistently with invocations.

closes #956

* Getting properties breaking consistent with invocations

* Fixing more edge cases

* some self review

---------

Co-authored-by: Lasath Fernando <devel@lasath.org>
  • Loading branch information
@belav @shocklateboy92
belav and shocklateboy92 committed Oct 10, 2023
1 parent dbc74ca commit 4e4bb4d
Show file tree
Hide file tree
Showing 8 changed files with 119 additions and 76 deletions.
1 change: 0 additions & 1 deletion Src/CSharpier.Cli/EditorConfig/EditorConfigParser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ IFileSystem fileSystem
{
var configFile = ConfigFileParser.Parse(potentialPath, fileSystem);

DebugLogger.Log(potentialPath);
yield return configFile;
if (configFile.IsRoot)
{
Expand Down
1 change: 0 additions & 1 deletion Src/CSharpier.Cli/FormattingCache.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ public bool CanSkipFormatting(FileToFormatInfo fileToFormatInfo)
var currentHash = Hash(fileToFormatInfo.FileContents) + this.optionsHash;
if (this.cacheDictionary.TryGetValue(fileToFormatInfo.Path, out var cachedHash))
{
DebugLogger.Log(fileToFormatInfo.Path + " " + currentHash + " " + cachedHash);
if (currentHash == cachedHash)
{
return true;
Expand Down
2 changes: 0 additions & 2 deletions Src/CSharpier.Cli/IgnoreFile.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ public static async Task<IgnoreFile> Create(
CancellationToken cancellationToken
)
{
DebugLogger.Log("Creating ignore file for " + baseDirectoryPath);

var ignore = new Ignore.Ignore();

foreach (var name in alwaysIgnored)
Expand Down
15 changes: 15 additions & 0 deletions Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChain_ArraysConsistent.test
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
var x = someLongNameField
.CallMethod____________________________________()
.AccessArray[1]
.Property_______________;

var x = someLongNameField
.CallMethod____________________________________()
.CallMethod(1)
.Property_______________;

new Action(AssertConfigurationIsValid)
.ShouldThrow<AutoMapperConfigurationException>()
.Errors[0]
.UnmappedPropertyNames[0]
.ShouldBe(nameof(Destination.Count));
33 changes: 33 additions & 0 deletions Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChain_PropertiesConsistent.test
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
var someVariable = someObject
.Property
.CallMethod(someValue => someValue.SomeProperty == someOtherValue___________________________);

var someVariable = someObject
.Property()
.CallMethod(someValue => someValue.SomeProperty == someOtherValue___________________________);

var someVariable = someObject
.Property
.CallMethod(someValue => someValue.SomeProperty == someOtherValue___________________________)
.CallMethod();

var someVariable = someObject
.Property()
.CallMethod(someValue => someValue.SomeProperty == someOtherValue___________________________)
.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
.CallMethod(someValue => someValue.SomeProperty == someOtherValue___________________________)
.CallMethod();

var someVariable = this.Property()
.CallMethod(someValue => someValue.SomeProperty == someOtherValue___________________________)
.CallMethod();
29 changes: 21 additions & 8 deletions ...s/TestFiles/cs/InvocationExpressions.test → ...ttingTests/TestFiles/cs/MemberChains.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,13 @@ class ClassName
.Where(o => someLongCondition__________________________)
.Where(o => someLongCondition__________________________);

var someValue = someOtherValue!.Thing!
var someValue = someOtherValue!
.Thing!
.Where(o => someLongCondition__________________________)
.Where(o => someLongCondition__________________________);

var someValue = someOtherValue.Thing
var someValue = someOtherValue
.Thing
.Where(o => someLongCondition__________________________)
.Where(o => someLongCondition__________________________);

Expand All @@ -81,7 +83,8 @@ class ClassName
this.SomeProperty.Setup(o => longThing_______________________________________)
);

roleNames.Value
roleNames
.Value
.Where(o => o.SomeProperty____________________________________)
.Select(o => o.SomethingElse);

Expand Down Expand Up @@ -110,9 +113,12 @@ class ClassName
someParameter____________________________________
)!;

var someVariable = someObject.Property.CallMethod(
someValue => someValue.SomeProperty == someOtherValue___________________________________
);
var someVariable = someObject
.Property
.CallMethod(
someValue =>
someValue.SomeProperty == someOtherValue___________________________________
);

CallMethod(
firstParameter____________________________,
Expand Down Expand Up @@ -231,11 +237,18 @@ class ClassName
)
.CallMethod__________________();

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

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

someThing_______________________?.Property
someThing_______________________
.Property!
.CallMethod__________________()
.CallMethod__________________();
}
Expand Down
5 changes: 5 additions & 0 deletions Src/CSharpier/DocTypes/Doc.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@ namespace CSharpier.DocTypes;

internal abstract class Doc
{
public string Print()
{
return DocPrinter.DocPrinter.Print(this, new PrinterOptions(), Environment.NewLine);
}

public override string ToString()
{
return DocSerializer.Serialize(this);
Expand Down
109 changes: 45 additions & 64 deletions Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/InvocationExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,16 @@ And we want to work with them from Left to Right
)
);
}
else if (expression is ElementAccessExpressionSyntax elementAccessExpression)
{
FlattenAndPrintNodes(elementAccessExpression.Expression, printedNodes, context);
printedNodes.Add(
new PrintedNode(
elementAccessExpression,
Node.Print(elementAccessExpression.ArgumentList, context)
)
);
}
else if (expression is MemberAccessExpressionSyntax memberAccessExpressionSyntax)
{
FlattenAndPrintNodes(memberAccessExpressionSyntax.Expression, printedNodes, context);
Expand Down Expand Up @@ -169,8 +179,6 @@ expression is PostfixUnaryExpressionSyntax
}
}

// TODO maybe this should work more like prettier, where it makes groups in a way that they try to fill lines
// TODO also prettier doesn't seem to use fluid
private static List<List<PrintedNode>> GroupPrintedNodesOnLines(List<PrintedNode> printedNodes)
{
// We want to group the printed nodes in the following manner
Expand Down Expand Up @@ -223,25 +231,22 @@ 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

// The first group is the first node followed by
// - as many InvocationExpression as possible
// < fn()()() >.something()
// - as many array accessors as possible
// < fn()[0][1][2] >.something()
// - then, as many MemberAccessExpression as possible but the last one
// < this.items >.something()

// TODO #451 this whole thing could possibly just turn into a big loop
// based on the current node, and the next/previous node, decide when to create new groups.
// certain nodes need to stay in the current group, other nodes indicate that a new group needs to be created.
var groups = new List<List<PrintedNode>>();
var currentGroup = new List<PrintedNode> { printedNodes[0] };
var index = 1;
Expand All @@ -257,63 +262,47 @@ List<PrintedNode> printedNodes
}
}

if (printedNodes[0].Node is not InvocationExpressionSyntax)
if (
printedNodes[0].Node is not (InvocationExpressionSyntax or PostfixUnaryExpressionSyntax)
&& index < printedNodes.Count
&& printedNodes[index].Node
is ElementAccessExpressionSyntax
or PostfixUnaryExpressionSyntax
)
{
for (; index + 1 < printedNodes.Count; ++index)
{
/* this handles the special case where we want ?.Property on the same line
someThing_______________________?.Property
.CallMethod__________________()
.CallMethod__________________();
*/
if (
printedNodes[index].Node is ConditionalAccessExpressionSyntax
&& printedNodes[index + 1].Node
is MemberBindingExpressionSyntax { Parent: MemberAccessExpressionSyntax }
)
{
currentGroup.Add(printedNodes[index]);
currentGroup.Add(printedNodes[index + 1]);
index++;
continue;
}

if (
(
IsMemberish(printedNodes[index].Node)
&& (
IsMemberish(printedNodes[index + 1].Node)
|| printedNodes[index + 1].Node is PostfixUnaryExpressionSyntax
)
)
|| printedNodes[index].Node is PostfixUnaryExpressionSyntax
)
{
currentGroup.Add(printedNodes[index]);
}
else
{
break;
}
}
currentGroup.Add(printedNodes[index]);
index++;
}

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

var hasSeenInvocationExpression = false;
var hasSeenNodeThatRequiresBreak = false;
for (; index < printedNodes.Count; index++)
{
if (hasSeenInvocationExpression && IsMemberish(printedNodes[index].Node))
if (
hasSeenNodeThatRequiresBreak
&& printedNodes[index].Node
is MemberAccessExpressionSyntax
or ConditionalAccessExpressionSyntax
)
{
groups.Add(currentGroup);
currentGroup = new List<PrintedNode>();
hasSeenInvocationExpression = false;
hasSeenNodeThatRequiresBreak = false;
}

if (printedNodes[index].Node is InvocationExpressionSyntax)
if (
printedNodes[index].Node
is (
InvocationExpressionSyntax
or MemberAccessExpressionSyntax
or ElementAccessExpressionSyntax
or MemberBindingExpressionSyntax
)
)
{
hasSeenInvocationExpression = true;
hasSeenNodeThatRequiresBreak = true;
}
currentGroup.Add(printedNodes[index]);
}
Expand All @@ -326,11 +315,6 @@ printedNodes[index].Node is ConditionalAccessExpressionSyntax
return groups;
}

private static bool IsMemberish(CSharpSyntaxNode node)
{
return node is MemberAccessExpressionSyntax or ConditionalAccessExpressionSyntax;
}

private static Doc PrintIndentedGroup(ExpressionSyntax node, IList<List<PrintedNode>> groups)
{
if (groups.Count == 0)
Expand Down Expand Up @@ -370,10 +354,7 @@ private static bool ShouldMergeFirstTwoGroups(List<List<PrintedNode>> groups)

var firstNode = groups[0][0].Node;

if (
firstNode is IdentifierNameSyntax identifierNameSyntax
&& identifierNameSyntax.Identifier.Text.Length <= 4
)
if (firstNode is IdentifierNameSyntax { Identifier.Text.Length: <= 4 })
{
return true;
}
Expand Down

0 comments on commit 4e4bb4d

Please sign in to comment.