Cleaning up the doc tree

belav · Sep 27, 2021 · 483d684 · 483d684
1 parent 8bfd431
commit 483d684
Show file tree

Hide file tree

Showing 11 changed files with 124 additions and 64 deletions.
diff --git a/Src/CSharpier.Tests/DocUtilitiesTests.cs b/Src/CSharpier.Tests/DocUtilitiesTests.cs
@@ -41,7 +41,7 @@ public void RemoveInitialDoubleHardLine_Should_Remove_Simple_Double_HardLine()
         [Test]
         public void RemoveInitialDoubleHardLine_Should_Not_Remove_Concated_HardLine()
         {
-            var concat = Doc.Concat(Doc.HardLine);
+            var concat = ActualConcat(Doc.HardLine);
             var doc = new List<Doc> { concat };
 
             DocUtilities.RemoveInitialDoubleHardLine(doc);
@@ -52,19 +52,19 @@ public void RemoveInitialDoubleHardLine_Should_Not_Remove_Concated_HardLine()
         [Test]
         public void RemoveInitialDoubleHardLine_Should_Remove_Concated_HardLine()
         {
-            var concat = Doc.Concat(Doc.HardLine, Doc.HardLine);
+            var concat = ActualConcat(Doc.HardLine, Doc.HardLine);
             var doc = new List<Doc> { concat };
 
             DocUtilities.RemoveInitialDoubleHardLine(doc);
 
-            concat.Should().BeEquivalentTo(Doc.Concat(Doc.HardLine, Doc.Null));
+            concat.Should().BeEquivalentTo(ActualConcat(Doc.HardLine, Doc.Null));
         }
 
         [Test]
         public void RemoveInitialDoubleHardLine_Should_Not_Remove_Deep_Concated_HardLine()
         {
-            var concat = Doc.Concat(Doc.HardLine);
-            var doc = new List<Doc> { Doc.Concat(concat) };
+            var concat = ActualConcat(Doc.HardLine);
+            var doc = new List<Doc> { ActualConcat(concat) };
 
             DocUtilities.RemoveInitialDoubleHardLine(doc);
 
@@ -74,8 +74,8 @@ public void RemoveInitialDoubleHardLine_Should_Not_Remove_Deep_Concated_HardLine
         [Test]
         public void RemoveInitialDoubleHardLine_Should_Remove_Deep_Concated_HardLine()
         {
-            var concat = Doc.Concat(Doc.HardLine, Doc.HardLine);
-            var doc = new List<Doc> { Doc.Concat(concat) };
+            var concat = ActualConcat(Doc.HardLine, Doc.HardLine);
+            var doc = new List<Doc> { ActualConcat(concat) };
 
             DocUtilities.RemoveInitialDoubleHardLine(doc);
 
@@ -85,8 +85,8 @@ public void RemoveInitialDoubleHardLine_Should_Remove_Deep_Concated_HardLine()
         [Test]
         public void RemoveInitialDoubleHardLine_Should_Remove_Single_HardLine()
         {
-            var concat = Doc.Concat(Doc.HardLine, Doc.HardLine, Doc.HardLine);
-            var doc = new List<Doc> { Doc.Concat(concat) };
+            var concat = ActualConcat(Doc.HardLine, Doc.HardLine, Doc.HardLine);
+            var doc = new List<Doc> { ActualConcat(concat) };
 
             DocUtilities.RemoveInitialDoubleHardLine(doc);
 
@@ -141,27 +141,32 @@ public void RemoveInitialDoubleHardLine_Should_Remove_Grouped_Double_HardLine()
         [Test]
         public void RemoveInitialDoubleHardLine_Should_Only_Remove_Initial_HardLines()
         {
-            var doc = Doc.Concat("1", Doc.HardLine, Doc.HardLine);
+            var doc = ActualConcat("1", Doc.HardLine, Doc.HardLine);
 
             DocUtilities.RemoveInitialDoubleHardLine(doc);
 
-            doc.Should().BeEquivalentTo(Doc.Concat("1", Doc.HardLine, Doc.HardLine));
+            doc.Should().BeEquivalentTo(ActualConcat("1", Doc.HardLine, Doc.HardLine));
         }
 
         [Test]
         public void RemoveInitialDoubleHardLine_Work_With_Doc_Null_Before_String()
         {
-            var doc = Doc.Concat(Doc.HardLine, Doc.Null, "1", Doc.HardLine, "2");
+            var doc = ActualConcat(Doc.HardLine, Doc.Null, "1", Doc.HardLine, "2");
 
             DocUtilities.RemoveInitialDoubleHardLine(doc);
 
             DocSerializer.Serialize(doc)
                 .Should()
                 .Be(
                     DocSerializer.Serialize(
-                        Doc.Concat(Doc.HardLine, Doc.Null, "1", Doc.HardLine, "2")
+                        ActualConcat(Doc.HardLine, Doc.Null, "1", Doc.HardLine, "2")
                     )
                 );
         }
+
+        private Concat ActualConcat(params Doc[] contents)
+        {
+            return new Concat(contents.ToList());
+        }
     }
 }
diff --git a/Src/CSharpier/DocSerializer.cs b/Src/CSharpier/DocSerializer.cs
@@ -16,34 +16,46 @@ public static string Serialize(Doc document)
         }
 
         // this is inefficient without a stringBuilder, but only used for dev work and changing it to use one isn't straightforward.
-        private static string Serialize(Doc document, string indent)
+        private static string Serialize(Doc document, string indent, Doc? parent = null)
         {
             var newLine = Environment.NewLine;
             var nextIndent = indent + "    ";
-            string PrintIndentedDocTree(Doc doc)
+            string PrintIndentedDocTree(Doc doc, Doc? parent = null)
             {
-                return Serialize(doc, nextIndent);
+                return Serialize(doc, nextIndent, parent);
             }
 
             string PrintConcat(Concat concatToPrint)
             {
-                var result = indent + "Doc.Concat(";
-                if (concatToPrint.Contents.Count > 0)
+                var skipConcat =
+                    parent is IHasContents hasContents && hasContents.Contents == concatToPrint;
+
+                var result = "";
+                if (!skipConcat)
                 {
-                    result += newLine;
+                    result += indent + "Doc.Concat(";
+                    if (concatToPrint.Contents.Count > 0)
+                    {
+                        result += newLine;
+                    }
                 }
 
                 for (var x = 0; x < concatToPrint.Contents.Count; x++)
                 {
-                    var printResult = PrintIndentedDocTree(concatToPrint.Contents[x]);
+                    var printResult = skipConcat
+                        ? Serialize(concatToPrint.Contents[x], indent)
+                        : PrintIndentedDocTree(concatToPrint.Contents[x]);
                     result += printResult;
                     if (x < concatToPrint.Contents.Count - 1)
                     {
                         result += "," + newLine;
                     }
                 }
 
-                result += newLine + indent + ")";
+                if (!skipConcat)
+                {
+                    result += newLine + indent + ")";
+                }
                 return result;
             }
 
@@ -76,12 +88,12 @@ Align align
                 ForceFlat forceFlat
                   => $"{indent}Doc.ForceFlat({newLine}{PrintIndentedDocTree(forceFlat.Contents)})",
                 IndentDoc indentDoc
-                  => $"{indent}Doc.Indent({newLine}{PrintIndentedDocTree(indentDoc.Contents)}{newLine}{indent})",
+                  => $"{indent}Doc.Indent({newLine}{PrintIndentedDocTree(indentDoc.Contents, indentDoc)}{newLine}{indent})",
                 ConditionalGroup conditionalGroup
                   => $"{indent}Doc.ConditionalGroup({newLine}{PrintIndentedDocTree(conditionalGroup.Contents)}{newLine}{indent})",
                 Group group
                   => @$"{indent}Doc.Group{(@group.GroupId != null ? "WithId" : string.Empty)}(
-{(@group.GroupId != null ? $"{nextIndent}\"{@group.GroupId}\",{newLine}" : string.Empty)}{PrintIndentedDocTree(@group.Contents)}{newLine}{indent})",
+{(@group.GroupId != null ? $"{nextIndent}\"{@group.GroupId}\",{newLine}" : string.Empty)}{PrintIndentedDocTree(@group.Contents, @group)}{newLine}{indent})",
                 LeadingComment leadingComment
                   => $"{indent}Doc.LeadingComment(\"{leadingComment.Comment}\", CommentType.{(leadingComment.Type == CommentType.SingleLine ? "SingleLine" : "MultiLine")})",
                 TrailingComment trailingComment

diff --git a/Src/CSharpier/DocTypes/Doc.cs b/Src/CSharpier/DocTypes/Doc.cs
@@ -37,9 +37,11 @@ public static LeadingComment LeadingComment(string comment, CommentType commentT
         public static TrailingComment TrailingComment(string comment, CommentType commentType) =>
             new() { Type = commentType, Comment = comment };
 
-        public static Concat Concat(List<Doc> contents) => new(contents);
+        public static Doc Concat(List<Doc> contents) =>
+            contents.Count == 1 ? contents[0] : new Concat(contents);
 
-        public static Concat Concat(params Doc[] contents) => new(contents);
+        public static Doc Concat(params Doc[] contents) =>
+            contents.Length == 1 ? contents[0] : new Concat(contents);
 
         public static Doc Join(Doc separator, IEnumerable<Doc> array)
         {

diff --git a/Src/CSharpier/SyntaxPrinter/ArgumentListLikeSyntax.cs b/Src/CSharpier/SyntaxPrinter/ArgumentListLikeSyntax.cs
@@ -1,5 +1,7 @@
+using System.Collections.Generic;
 using CSharpier.DocTypes;
 using CSharpier.SyntaxPrinter.SyntaxNodePrinters;
+using CSharpier.Utilities;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 
@@ -11,19 +13,23 @@ public static Doc Print(
             SyntaxToken openParenToken,
             SeparatedSyntaxList<ArgumentSyntax> arguments,
             SyntaxToken closeParenToken
-        ) =>
-            Doc.Concat(
-                Token.Print(openParenToken),
-                arguments.Any()
-                    ? Doc.Concat(
-                          Doc.Indent(
-                              Doc.SoftLine,
-                              SeparatedSyntaxList.Print(arguments, Argument.Print, Doc.Line)
-                          ),
-                          Doc.SoftLine
-                      )
-                    : Doc.Null,
-                Token.Print(closeParenToken)
-            );
+        ) {
+            var docs = new List<Doc> { Token.Print(openParenToken) };
+
+            if (arguments.Any())
+            {
+                docs.Add(
+                    Doc.Indent(
+                        Doc.SoftLine,
+                        SeparatedSyntaxList.Print(arguments, Argument.Print, Doc.Line)
+                    ),
+                    Doc.SoftLine
+                );
+            }
+
+            docs.Add(Token.Print(closeParenToken));
+
+            return Doc.Concat(docs);
+        }
     }
 }
diff --git a/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/Argument.cs b/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/Argument.cs
@@ -14,7 +14,11 @@ public static Doc Print(ArgumentSyntax node)
                 docs.Add(NameColon.Print(node.NameColon));
             }
 
-            docs.Add(Token.PrintWithSuffix(node.RefKindKeyword, " "));
+            if (node.RefKindKeyword.RawKind != 0)
+            {
+                docs.Add(Token.PrintWithSuffix(node.RefKindKeyword, " "));
+            }
+
             docs.Add(Node.Print(node.Expression));
             return Doc.Concat(docs);
         }

diff --git a/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/BaseMethodDeclaration.cs b/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/BaseMethodDeclaration.cs
@@ -74,14 +74,14 @@ public static Doc Print(CSharpSyntaxNode node)
 
             var docs = new List<Doc> { ExtraNewLines.Print(node) };
 
-            if (attributeLists.HasValue)
+            if (attributeLists.HasValue && attributeLists.Value.Any())
             {
                 docs.Add(AttributeLists.Print(node, attributeLists.Value));
             }
 
             var declarationGroup = new List<Doc>();
 
-            if (modifiers.HasValue)
+            if (modifiers.HasValue && modifiers.Value.Any())
             {
                 declarationGroup.Add(Modifiers.PrintWithoutLeadingTrivia(modifiers.Value));
             }

diff --git a/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/BaseTypeDeclaration.cs b/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/BaseTypeDeclaration.cs
@@ -68,12 +68,18 @@ public static Doc Print(BaseTypeDeclarationSyntax node)
                 semicolonToken = enumDeclarationSyntax.SemicolonToken;
             }
 
-            var docs = new List<Doc>
+            var docs = new List<Doc>();
+            docs.AddIfNotNull(ExtraNewLines.Print(node));
+            if (node.AttributeLists.Any())
             {
-                ExtraNewLines.Print(node),
-                AttributeLists.Print(node, node.AttributeLists),
-                Modifiers.Print(node.Modifiers)
-            };
+                docs.Add(AttributeLists.Print(node, node.AttributeLists));
+            }
+
+            if (node.Modifiers.Any())
+            {
+                docs.Add(Modifiers.Print(node.Modifiers));
+            }
+
             if (keyword != null)
             {
                 docs.Add(Token.PrintWithSuffix(keyword.Value, " "));

diff --git a/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/ConditionalExpression.cs b/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/ConditionalExpression.cs
@@ -18,8 +18,6 @@ public static Doc Print(ConditionalExpressionSyntax node)
                 Doc.Align(2, Node.Print(node.WhenFalse))
             };
 
-            var groupId = Guid.NewGuid().ToString();
-
             return Doc.Group(
                 node.Parent is ReturnStatementSyntax
                 && node.Condition is BinaryExpressionSyntax or IsPatternExpressionSyntax

diff --git a/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/Parameter.cs b/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/Parameter.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using CSharpier.DocTypes;
 using CSharpier.Utilities;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -12,12 +13,18 @@ public static Doc Print(ParameterSyntax node)
         {
             var hasAttribute = node.AttributeLists.Any();
             var groupId = hasAttribute ? Guid.NewGuid().ToString() : string.Empty;
-            var docs = new List<Doc>
+            var docs = new List<Doc>();
+
+            if (hasAttribute)
             {
-                AttributeLists.Print(node, node.AttributeLists),
-                hasAttribute ? Doc.IndentIfBreak(Doc.Line, groupId) : Doc.Null,
-                Modifiers.Print(node.Modifiers)
-            };
+                docs.Add(AttributeLists.Print(node, node.AttributeLists));
+                docs.Add(Doc.IndentIfBreak(Doc.Line, groupId));
+            }
+
+            if (node.Modifiers.Any())
+            {
+                docs.Add(Modifiers.Print(node.Modifiers));
+            }
 
             if (node.Type != null)
             {
@@ -30,7 +37,9 @@ public static Doc Print(ParameterSyntax node)
                 docs.Add(EqualsValueClause.Print(node.Default));
             }
 
-            return hasAttribute ? Doc.GroupWithId(groupId, docs) : Doc.Concat(docs);
+            return hasAttribute
+                ? Doc.GroupWithId(groupId, docs)
+                : docs.Count == 1 ? docs[0] : Doc.Concat(docs);
         }
     }
 }
diff --git a/Src/CSharpier/Utilities/ListExtensions.cs b/Src/CSharpier/Utilities/ListExtensions.cs
@@ -0,0 +1,27 @@
+using System;
+using System.Collections.Generic;
+using System.Threading.Tasks.Sources;
+using CSharpier.DocTypes;
+
+namespace CSharpier.Utilities
+{
+    public static class ListExtensions
+    {
+        public static void Add(this List<Doc> list, params Doc[] values)
+        {
+            if (values.Length == 1 && values[0] == Doc.Null)
+            {
+                return;
+            }
+            list.AddRange(values);
+        }
+
+        public static void AddIfNotNull(this List<Doc> value, Doc doc)
+        {
+            if (doc != Doc.Null)
+            {
+                value.Add(doc);
+            }
+        }
+    }
+}
diff --git a/Src/CSharpier/Utilities/StringExtensions.cs b/Src/CSharpier/Utilities/StringExtensions.cs
@@ -40,15 +40,6 @@ public static bool IsBlank(this string? value)
             return value == null || string.IsNullOrEmpty(value.Trim());
         }
 
-        public static void Add(this List<Doc> list, params Doc[] values)
-        {
-            if (values.Length == 1 && values[0] == Doc.Null)
-            {
-                return;
-            }
-            list.AddRange(values);
-        }
-
         // this will eventually deal with the visual width not being the same as the code width https://github.com/belav/csharpier/issues/260
         public static int GetPrintedWidth(this string value)
         {