Fixing issue with trailing commas that have a trailing comment (#1301)

closes #1300 --------- Co-authored-by: Lasath Fernando <[email protected]>
belav · Aug 13, 2024 · 04ce002 · 04ce002
1 parent 29f9db4
commit 04ce002
Show file tree

Hide file tree

Showing 8 changed files with 74 additions and 24 deletions.
diff --git a/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/InitializerExpressions.test b/Src/CSharpier.Tests/FormattingTests/TestFiles/cs/InitializerExpressions.test
@@ -2,6 +2,11 @@ class ClassName
 {
     int[] array = { 1, 2 };
 
+    int[] array =
+    [
+        1, // some comment
+    ];
+
     int[] array =
     {
         "someLongValue_____________________________________",
@@ -58,6 +63,10 @@ class ClassName
 
         var x = new OneProperty { One = true };
         var x = new TwoProperties { One = true, Two = true };
+        var x = new TwoProperties
+        {
+            One = true, // comment
+        };
         var x = new ThreeProperties
         {
             One = true,

diff --git a/Src/CSharpier/SyntaxPrinter/SeparatedSyntaxList.cs b/Src/CSharpier/SyntaxPrinter/SeparatedSyntaxList.cs
@@ -7,8 +7,35 @@ public static Doc Print<T>(
         Func<T, FormattingContext, Doc> printFunc,
         Doc afterSeparator,
         FormattingContext context,
-        int startingIndex = 0,
-        Doc? trailingSeparator = null
+        int startingIndex = 0
+    )
+        where T : SyntaxNode
+    {
+        return Print(list, printFunc, afterSeparator, context, startingIndex, null);
+    }
+
+    public static Doc PrintWithTrailingComma<T>(
+        SeparatedSyntaxList<T> list,
+        Func<T, FormattingContext, Doc> printFunc,
+        Doc afterSeparator,
+        FormattingContext context,
+        SyntaxToken? closingToken = null
+    )
+        where T : SyntaxNode
+    {
+        return Print(list, printFunc, afterSeparator, context, 0, closingToken);
+    }
+
+    // the names above aren't totally accurate
+    // sometimes there are trailing commas with calls to Print (some patterns do that)
+    // and if you pass null to PrintWithTrailingComma it won't add a trailing comma if there isn't one
+    private static Doc Print<T>(
+        SeparatedSyntaxList<T> list,
+        Func<T, FormattingContext, Doc> printFunc,
+        Doc afterSeparator,
+        FormattingContext context,
+        int startingIndex,
+        SyntaxToken? closingToken
     )
         where T : SyntaxNode
     {
@@ -17,21 +44,35 @@ public static Doc Print<T>(
         {
             docs.Add(printFunc(list[x], context));
 
-            var isTrailingSeparator = x == list.Count - 1;
-
+            // if the syntax tree doesn't have a trailing comma but we want want, then add it
             if (x >= list.SeparatorCount)
             {
-                if (isTrailingSeparator && trailingSeparator != null)
+                if (closingToken != null)
                 {
-                    docs.Add(trailingSeparator);
+                    docs.Add(TrailingComma.Print(closingToken.Value, context));
                 }
 
                 continue;
             }
 
+            var isTrailingSeparator = x == list.Count - 1;
+
             if (isTrailingSeparator)
             {
-                docs.Add(Doc.IfBreak(Token.Print(list.GetSeparator(x), context), Doc.Null));
+                var trailingSeparatorToken = list.GetSeparator(x);
+                // when the trailing separator has trailing comments, we have to print it normally to prevent it from collapsing
+                if (trailingSeparatorToken.TrailingTrivia.Any(o => o.IsComment()))
+                {
+                    docs.Add(Token.Print(trailingSeparatorToken, context));
+                }
+                else if (closingToken != null)
+                {
+                    docs.Add(TrailingComma.Print(closingToken.Value, context));
+                }
+                else
+                {
+                    docs.Add(Doc.IfBreak(Token.Print(list.GetSeparator(x), context), Doc.Null));
+                }
             }
             else
             {

diff --git a/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/AnonymousObjectCreationExpression.cs b/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/AnonymousObjectCreationExpression.cs
@@ -12,12 +12,12 @@ public static Doc Print(AnonymousObjectCreationExpressionSyntax node, Formatting
             node.Initializers.Any()
                 ? Doc.Indent(
                     alwaysBreak ? Doc.HardLine : Doc.Line,
-                    SeparatedSyntaxList.Print(
+                    SeparatedSyntaxList.PrintWithTrailingComma(
                         node.Initializers,
                         AnonymousObjectMemberDeclarator.Print,
                         Doc.Line,
                         context,
-                        trailingSeparator: TrailingComma.Print(node.CloseBraceToken, context)
+                        node.CloseBraceToken
                     )
                 )
                 : Doc.Null,

diff --git a/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/CollectionExpression.cs b/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/CollectionExpression.cs
@@ -32,12 +32,12 @@ or ExpressionStatementSyntax
             node.Elements.Any()
                 ? Doc.Indent(
                     alwaysBreak ? Doc.HardLine : Doc.IfBreak(Doc.Line, Doc.Null),
-                    SeparatedSyntaxList.Print(
+                    SeparatedSyntaxList.PrintWithTrailingComma(
                         node.Elements,
                         Node.Print,
                         alwaysBreak ? Doc.HardLine : Doc.Line,
                         context,
-                        trailingSeparator: TrailingComma.Print(node.CloseBracketToken, context)
+                        node.CloseBracketToken
                     )
                 )
                 : Doc.Null,

diff --git a/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/InitializerExpression.cs b/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/InitializerExpression.cs
@@ -34,15 +34,15 @@ is SyntaxKind.ArrayInitializerExpression
                 : Doc.Concat(
                     Doc.Indent(
                         alwaysBreak ? Doc.HardLine : Doc.Line,
-                        SeparatedSyntaxList.Print(
+                        SeparatedSyntaxList.PrintWithTrailingComma(
                             node.Expressions,
                             Node.Print,
                             alwaysBreak ? Doc.HardLine : Doc.Line,
                             context,
-                            trailingSeparator: node.Kind()
+                            closingToken: node.Kind()
                                 is not SyntaxKind.ComplexElementInitializerExpression
-                                ? TrailingComma.Print(node.CloseBraceToken, context)
-                                : Doc.Null
+                                ? node.CloseBraceToken
+                                : null
                         )
                     ),
                     alwaysBreak ? Doc.HardLine : Doc.Line

diff --git a/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/ListPattern.cs b/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/ListPattern.cs
@@ -8,12 +8,12 @@ public static Doc Print(ListPatternSyntax node, FormattingContext context)
             Token.Print(node.OpenBracketToken, context),
             Doc.Indent(
                 Doc.SoftLine,
-                SeparatedSyntaxList.Print(
+                SeparatedSyntaxList.PrintWithTrailingComma(
                     node.Patterns,
                     Node.Print,
                     Doc.Line,
                     context,
-                    trailingSeparator: TrailingComma.Print(node.CloseBracketToken, context)
+                    node.CloseBracketToken
                 )
             ),
             Doc.SoftLine,

diff --git a/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/SwitchExpression.cs b/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/SwitchExpression.cs
@@ -7,7 +7,7 @@ public static Doc Print(SwitchExpressionSyntax node, FormattingContext context)
         var sections = Doc.Group(
             Doc.Indent(
                 Doc.HardLine,
-                SeparatedSyntaxList.Print(
+                SeparatedSyntaxList.PrintWithTrailingComma(
                     node.Arms,
                     (o, _) =>
                         Doc.Concat(
@@ -34,7 +34,7 @@ public static Doc Print(SwitchExpressionSyntax node, FormattingContext context)
                         ),
                     Doc.HardLine,
                     context,
-                    trailingSeparator: TrailingComma.Print(node.CloseBraceToken, context)
+                    node.CloseBraceToken
                 )
             ),
             Doc.HardLine

diff --git a/Src/CSharpier/SyntaxPrinter/TrailingComma.cs b/Src/CSharpier/SyntaxPrinter/TrailingComma.cs
@@ -2,13 +2,13 @@ namespace CSharpier.SyntaxPrinter;
 
 internal static class TrailingComma
 {
-    public static Doc Print(SyntaxToken nextSyntaxToken, FormattingContext context)
+    public static Doc Print(SyntaxToken closingToken, FormattingContext context)
     {
-        return !nextSyntaxToken.LeadingTrivia.Any(o => o.IsDirective)
-            ? Doc.IfBreak(
+        return closingToken.LeadingTrivia.Any(o => o.IsDirective)
+            ? Doc.Null
+            : Doc.IfBreak(
                 Token.Print(SyntaxFactory.Token(SyntaxKind.CommaToken), context),
                 Doc.Null
-            )
-            : Doc.Null;
+            );
     }
 }