Skip to content

Commit fd3865b

Browse files
committed
Moving away from prettier style breaks for invocation chains
closes #1676
1 parent 03dd509 commit fd3865b

5 files changed

Lines changed: 61 additions & 139 deletions

File tree

Src/CSharpier.Core/CSharp/SyntaxPrinter/SyntaxNodePrinters/InvocationExpression.cs

Lines changed: 1 addition & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
using System.Diagnostics.CodeAnalysis;
21
using CSharpier.Core.DocTypes;
32
using CSharpier.Core.Utilities;
43
using Microsoft.CodeAnalysis;
@@ -9,13 +8,6 @@ namespace CSharpier.Core.CSharp.SyntaxPrinter.SyntaxNodePrinters;
98

109
internal record PrintedNode(CSharpSyntaxNode Node, Doc Doc);
1110

12-
// This is based on prettier/src/language-js/print/member-chain.js
13-
// various discussions/prs about how to potentially improve the formatting
14-
// https://github.com/prettier/prettier/issues/5737
15-
// https://github.com/prettier/prettier/issues/7884
16-
// https://github.com/prettier/prettier/issues/8003
17-
// https://github.com/prettier/prettier/issues/8902
18-
// https://github.com/prettier/prettier/pull/7889
1911
internal static class InvocationExpression
2012
{
2113
public static Doc Print(InvocationExpressionSyntax node, PrintingContext context)
@@ -30,9 +22,7 @@ public static Doc PrintMemberChain(ExpressionSyntax node, PrintingContext contex
3022

3123
FlattenAndPrintNodes(node, printedNodes, context);
3224

33-
var groups = printedNodes.Any(o => o.Node is InvocationExpressionSyntax)
34-
? GroupPrintedNodesPrettierStyle(printedNodes)
35-
: GroupPrintedNodesOnLines(printedNodes);
25+
var groups = GroupPrintedNodesOnLines(printedNodes);
3626

3727
var oneLine = SelectManyDocsToArray(groups);
3828

@@ -266,95 +256,6 @@ or IdentifierNameSyntax
266256
return groups;
267257
}
268258

269-
private static List<List<PrintedNode>> GroupPrintedNodesPrettierStyle(
270-
List<PrintedNode> printedNodes
271-
)
272-
{
273-
// We want to group the printed nodes in the following manner
274-
//
275-
// a().b.c().d().e
276-
// will be grouped as
277-
// [
278-
// [Identifier, InvocationExpression],
279-
// [MemberAccessExpression], [MemberAccessExpression, InvocationExpression],
280-
// [MemberAccessExpression, InvocationExpression],
281-
// [MemberAccessExpression],
282-
// ]
283-
284-
// so that we can print it as
285-
// a()
286-
// .b.c()
287-
// .d()
288-
// .e
289-
290-
// TODO #451 this whole thing could possibly just turn into a big loop
291-
// based on the current node, and the next/previous node, decide when to create new groups.
292-
// certain nodes need to stay in the current group, other nodes indicate that a new group needs to be created.
293-
var groups = new List<List<PrintedNode>>();
294-
var currentGroup = new List<PrintedNode> { printedNodes[0] };
295-
var index = 1;
296-
for (; index < printedNodes.Count; index++)
297-
{
298-
if (printedNodes[index].Node is InvocationExpressionSyntax)
299-
{
300-
currentGroup.Add(printedNodes[index]);
301-
}
302-
else
303-
{
304-
break;
305-
}
306-
}
307-
308-
if (
309-
printedNodes[0].Node is not (InvocationExpressionSyntax or PostfixUnaryExpressionSyntax)
310-
&& index < printedNodes.Count
311-
&& printedNodes[index].Node
312-
is ElementAccessExpressionSyntax
313-
or PostfixUnaryExpressionSyntax
314-
)
315-
{
316-
currentGroup.Add(printedNodes[index]);
317-
index++;
318-
}
319-
320-
groups.Add(currentGroup);
321-
currentGroup = [];
322-
323-
var hasSeenNodeThatRequiresBreak = false;
324-
for (; index < printedNodes.Count; index++)
325-
{
326-
if (
327-
hasSeenNodeThatRequiresBreak
328-
&& printedNodes[index].Node
329-
is MemberAccessExpressionSyntax
330-
or ConditionalAccessExpressionSyntax
331-
)
332-
{
333-
groups.Add(currentGroup);
334-
currentGroup = [];
335-
hasSeenNodeThatRequiresBreak = false;
336-
}
337-
338-
if (
339-
printedNodes[index].Node
340-
is InvocationExpressionSyntax
341-
or ElementAccessExpressionSyntax
342-
)
343-
{
344-
hasSeenNodeThatRequiresBreak = true;
345-
}
346-
currentGroup.Add(printedNodes[index]);
347-
}
348-
349-
if (currentGroup.Count != 0)
350-
{
351-
groups.Add(currentGroup);
352-
}
353-
354-
return groups;
355-
}
356-
357-
[SuppressMessage("ReSharper", "ForeachCanBePartlyConvertedToQueryUsingAnotherGetEnumerator")]
358259
private static Doc[] SelectManyDocsToArray(List<List<PrintedNode>> groups)
359260
{
360261
var arrayLength = 0;
@@ -469,8 +370,6 @@ or BaseExpressionSyntax
469370
return false;
470371
}
471372

472-
// TODO maybe some things to fix in here
473-
// https://github.com/belav/csharpier-repos/pull/100/files
474373
if (
475374
groups[1].Count == 1
476375
|| parent

Src/CSharpier.Tests/FormattingTests/TestFiles/cs/BinaryExpressions.test

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,8 @@ class TestClass
202202

203203
var x =
204204
someValue
205-
.Property.CallLongMethod_____________________________________()
205+
.Property
206+
.CallLongMethod_____________________________________()
206207
.CallMethod(
207208
someLongValue__________________________________________,
208209
someLongValue__________________________________________
@@ -273,23 +274,29 @@ class TestClass
273274
someLongValue__________________________________________,
274275
someLongValue__________________________________________
275276
)
276-
).SomeProperty.CallMethod() ?? someOtherValue;
277+
)
278+
.SomeProperty
279+
.CallMethod() ?? someOtherValue;
277280

278281
var x =
279282
(
280283
CallMethod(
281284
someLongValue__________________________________________,
282285
someLongValue__________________________________________
283286
)
284-
)?.SomeProperty.CallMethod() ?? someOtherValue;
287+
)
288+
?.SomeProperty
289+
.CallMethod() ?? someOtherValue;
285290

286291
var x =
287292
(
288293
CallMethod(
289294
someLongValue__________________________________________,
290295
someLongValue__________________________________________
291296
)
292-
)?.SomeProperty?.CallMethod() ?? someOtherValue;
297+
)
298+
?.SomeProperty
299+
?.CallMethod() ?? someOtherValue;
293300

294301
var x =
295302
(
@@ -298,7 +305,8 @@ class TestClass
298305
someLongValue__________________________________________
299306
)
300307
)
301-
.SomeProperty.CallMethod()
308+
.SomeProperty
309+
.CallMethod()
302310
.CallMethod()
303311
?? someOtherValue;
304312

@@ -309,7 +317,8 @@ class TestClass
309317
someLongValue__________________________________________
310318
)
311319
)
312-
?.SomeProperty.CallMethod()
320+
?.SomeProperty
321+
.CallMethod()
313322
.CallMethod()
314323
?? someOtherValue;
315324

@@ -347,19 +356,22 @@ class TestClass
347356

348357
var x =
349358
someValue
350-
.Property.CallLongMethod_____________________________________()
359+
.Property
360+
.CallLongMethod_____________________________________()
351361
.CallMethod__________()
352362
?? throw new Exception();
353363

354364
var x =
355365
someValue
356-
.Property.CallLongMethod_____________________________________()
366+
.Property
367+
.CallLongMethod_____________________________________()
357368
.CallMethod__________(someParameter)
358369
?? throw new Exception();
359370

360371
var x =
361372
someValue
362-
.Property.CallLongMethod_____________________________________()
373+
.Property
374+
.CallLongMethod_____________________________________()
363375
.CallLongMethod___________________________________________________()
364376
?? throw new Exception();
365377

Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChain_PropertiesConsistent.test

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
1-
var someVariable =
2-
someObject____________.Property_______________.CallMethod______________________();
1+
var someVariable = someObject____________
2+
.Property_______________
3+
.CallMethod______________________();
34

4-
var someVariable = someObject.Property.CallMethod(someValue =>
5-
someValue.SomeProperty == someOtherValue___________________________
6-
);
5+
var someVariable = someObject
6+
.Property
7+
.CallMethod(someValue => someValue.SomeProperty == someOtherValue___________________________);
78

89
var someVariable = someObject
910
.Property()
1011
.CallMethod(someValue => someValue.SomeProperty == someOtherValue___________________________);
1112

1213
var someVariable = someObject
13-
.Property.CallMethod(someValue =>
14-
someValue.SomeProperty == someOtherValue___________________________
15-
)
14+
.Property
15+
.CallMethod(someValue => someValue.SomeProperty == someOtherValue___________________________)
1616
.CallMethod();
1717

1818
var someVariable = someObject
@@ -31,10 +31,8 @@ var someVariable = this.CallMethod()
3131
var someVariable = this.Array[1]
3232
.CallMethod(someValue => someValue.SomeProperty == someOtherValue___________________________);
3333

34-
var someVariable = this
35-
.Property.CallMethod(someValue =>
36-
someValue.SomeProperty == someOtherValue___________________________
37-
)
34+
var someVariable = this.Property
35+
.CallMethod(someValue => someValue.SomeProperty == someOtherValue___________________________)
3836
.CallMethod();
3937

4038
var someVariable = this.CallMethod()

Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChains.test

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,13 @@ var someValue = someOtherValue!
4848
.Where(o => someLongCondition__________________________);
4949

5050
var someValue = someOtherValue!
51-
.Thing!.Where(o => someLongCondition__________________________)
51+
.Thing!
52+
.Where(o => someLongCondition__________________________)
5253
.Where(o => someLongCondition__________________________);
5354

5455
var someValue = someOtherValue
55-
.Thing.Where(o => someLongCondition__________________________)
56+
.Thing
57+
.Where(o => someLongCondition__________________________)
5658
.Where(o => someLongCondition__________________________);
5759

5860
var someValue = someOtherValue
@@ -77,7 +79,8 @@ roleNames
7779
);
7880

7981
roleNames
80-
.Value.Where(o => o.SomeProperty____________________________________)
82+
.Value
83+
.Where(o => o.SomeProperty____________________________________)
8184
.Select(o => o.SomethingElse);
8285

8386
return someCondition
@@ -105,9 +108,11 @@ CallSomeMethod(
105108
someParameter____________________________________
106109
)!;
107110

108-
var someVariable = someObject.Property.CallMethod(someValue =>
109-
someValue.SomeProperty == someOtherValue___________________________________
110-
);
111+
var someVariable = someObject
112+
.Property
113+
.CallMethod(someValue =>
114+
someValue.SomeProperty == someOtherValue___________________________________
115+
);
111116

112117
CallMethod(
113118
firstParameter________________________________,
@@ -227,41 +232,48 @@ var someValue = CallMethod______________________(
227232
.CallMethod__________________();
228233

229234
someThing_______________________
230-
.Property.CallMethod__________________()
235+
.Property
236+
.CallMethod__________________()
231237
.CallMethod__________________();
232238

233239
someThing_______________________
234-
?.Property.CallMethod__________________()
240+
?.Property
241+
.CallMethod__________________()
235242
.CallMethod__________________();
236243

237244
someThing_______________________
238-
.Property!.CallMethod__________________()
245+
.Property!
246+
.CallMethod__________________()
239247
.CallMethod__________________();
240248

241249
IEnumerable<ValueProviderFactory> valueProviderFactories =
242250
new ModelBinderAttribute_______().GetValueProviderFactories(config);
243251

244-
var something________________________________________ = x
245-
.SomeProperty.CallMethod(longParameter_____________, longParameter_____________)
252+
var something________________________________________ = x.SomeProperty
253+
.CallMethod(longParameter_____________, longParameter_____________)
246254
.CallMethod();
247255

248256
CallMethod(o =>
249-
o.Property.CallMethod_____________________________________________________()
257+
o.Property
258+
.CallMethod_____________________________________________________()
250259
.CallMethod_____________________________________________________()
251260
);
252261

253262
CallMethod(
254263
o,
255-
o.Property.CallMethod_____________________________________________________()
264+
o.Property
265+
.CallMethod_____________________________________________________()
256266
.CallMethod_____________________________________________________()
257267
);
258268

259269
var someValue =
260270
someValue
261-
&& o.Property.CallMethod_____________________________________________________()
271+
&& o.Property
272+
.CallMethod_____________________________________________________()
262273
.CallMethod_____________________________________________________();
263274

264-
o.Property.CallMethod(
275+
o.Property
276+
.CallMethod(
265277
someParameter_____________________________,
266278
someParameter_____________________________
267279
)

Src/CSharpier.Tests/FormattingTests/TestFiles/cs/SpreadElements.test

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ int[] someOtherArray =
1111
List<KeyValuePair<string, string>> list =
1212
[
1313
.. attribute
14-
.Targets.Select(target =>
14+
.Targets
15+
.Select(target =>
1516
KeyValuePair.Create(
1617
target,
1718
context.EntityDefinitions.TryGetValue(target, out var type) ? type.ClassName : null

0 commit comments

Comments
 (0)