Fixing a number of edge cases with preprocessor directives

closes #660
belav · May 8, 2022 · 4f30a53 · 4f30a53
1 parent ad5cbf9
commit 4f30a53
Show file tree

Hide file tree

Showing 7 changed files with 207 additions and 56 deletions.
diff --git a/Src/CSharpier.Tests/FormattingTests/TestFiles/Directives_AddAndRemoveLines.cst b/Src/CSharpier.Tests/FormattingTests/TestFiles/Directives_AddAndRemoveLines.cst
@@ -0,0 +1,31 @@
+public class ClassName
+{
+#if NET6_0_OR_GREATER
+    public void Method1() { }
+#endif
+
+
+    public void RemoveLineAbove() { }
+
+#if NET6_0_OR_GREATER
+    public void Method2() { }
+#endif
+    public void AddLineAbove() { }
+
+
+#if NET6_0_OR_GREATER
+    public void RemoveLineAbove() { }
+#endif
+
+    public void Method3() { }
+#if NET6_0_OR_GREATER
+    public void AddLineAbove() { }
+#endif
+
+    public void Method4() { }
+
+#if NET6_0_OR_GREATER
+    public void RemoveLineBelow() { }
+#endif
+
+}
diff --git a/Src/CSharpier.Tests/FormattingTests/TestFiles/Directives_AddAndRemoveLines.expected.cst b/Src/CSharpier.Tests/FormattingTests/TestFiles/Directives_AddAndRemoveLines.expected.cst
@@ -0,0 +1,30 @@
+public class ClassName
+{
+#if NET6_0_OR_GREATER
+    public void Method1() { }
+#endif
+
+    public void RemoveLineAbove() { }
+
+#if NET6_0_OR_GREATER
+    public void Method2() { }
+#endif
+
+    public void AddLineAbove() { }
+
+#if NET6_0_OR_GREATER
+    public void RemoveLineAbove() { }
+#endif
+
+    public void Method3() { }
+
+#if NET6_0_OR_GREATER
+    public void AddLineAbove() { }
+#endif
+
+    public void Method4() { }
+
+#if NET6_0_OR_GREATER
+    public void RemoveLineBelow() { }
+#endif
+}
diff --git a/Src/CSharpier.Tests/Samples/Scratch.cst b/Src/CSharpier.Tests/Samples/Scratch.cst
@@ -1,7 +1,31 @@
-using System.Web.Http;
+public class ClassName
+{
+#if NET6_0_OR_GREATER
+    public void Method1() { }
+#endif
+
+
+    public void RemoveLineAbove() { }
+
+#if NET6_0_OR_GREATER
+    public void Method2() { }
+#endif
+    public void AddLineAbove() { }
+
+
+#if NET6_0_OR_GREATER
+    public void RemoveLineAbove() { }
+#endif
+
+    public void Method3() { }
+#if NET6_0_OR_GREATER
+    public void AddLineAbove() { }
+#endif
 
-#if KEEP_LINE_ABOVE
-using System.Web.Http;
+    public void Method4() { }
+
+#if NET6_0_OR_GREATER
+    public void RemoveLineBelow() { }
 #endif
 
-namespace Namespace { }
+}
diff --git a/Src/CSharpier/SyntaxPrinter/MembersWithForcedLines.cs b/Src/CSharpier/SyntaxPrinter/MembersWithForcedLines.cs
@@ -1,3 +1,5 @@
+using System.Collections.Immutable;
+
 namespace CSharpier.SyntaxPrinter;
 
 internal static class MembersWithForcedLines
@@ -59,48 +61,75 @@ member is MethodDeclarationSyntax methodDeclaration
 
             var addBlankLine = blankLineIsForced || lastMemberForcedBlankLine;
 
+            var triviaContainsCommentOrNewLine = false;
+            var printExtraNewLines = false;
+            var triviaContainsEndIfOrRegion = false;
+
+            var leadingTrivia = member
+                .GetLeadingTrivia()
+                .Select(o => o.RawSyntaxKind())
+                .ToImmutableHashSet();
+
+            foreach (var syntaxTrivia in leadingTrivia)
+            {
+                if (syntaxTrivia is SyntaxKind.EndOfLineTrivia || syntaxTrivia.IsComment())
+                {
+                    triviaContainsCommentOrNewLine = true;
+                }
+                else if (
+                    syntaxTrivia
+                    is SyntaxKind.PragmaWarningDirectiveTrivia
+                        or SyntaxKind.PragmaChecksumDirectiveTrivia
+                        or SyntaxKind.IfDirectiveTrivia
+                        or SyntaxKind.EndRegionDirectiveTrivia
+                )
+                {
+                    printExtraNewLines = true;
+                }
+                else if (
+                    syntaxTrivia
+                    is SyntaxKind.EndIfDirectiveTrivia
+                        or SyntaxKind.EndRegionDirectiveTrivia
+                )
+                {
+                    triviaContainsEndIfOrRegion = true;
+                }
+            }
+
             if (!addBlankLine)
             {
-                addBlankLine =
-                    member.AttributeLists.Any()
-                    || (
-                        member
-                            .GetLeadingTrivia()
-                            .Any(
-                                o =>
-                                    o.RawSyntaxKind() is SyntaxKind.EndOfLineTrivia || o.IsComment()
-                            )
-                    );
+                addBlankLine = member.AttributeLists.Any() || triviaContainsCommentOrNewLine;
+            }
+
+            if (printExtraNewLines)
+            {
+                result.Add(ExtraNewLines.Print(member));
+            }
+            else if (addBlankLine && !triviaContainsEndIfOrRegion)
+            {
+                result.Add(Doc.HardLine);
             }
 
             if (
-                member
-                    .GetLeadingTrivia()
-                    .Any(
-                        o =>
-                            o.RawSyntaxKind()
-                                is SyntaxKind.PragmaWarningDirectiveTrivia
-                                    or SyntaxKind.PragmaChecksumDirectiveTrivia
-                                    or SyntaxKind.IfDirectiveTrivia
-                                    or SyntaxKind.EndRegionDirectiveTrivia
-                    )
+                addBlankLine
+                && !triviaContainsEndIfOrRegion
+                && leadingTrivia.Contains(SyntaxKind.IfDirectiveTrivia)
+                && !leadingTrivia.Contains(SyntaxKind.EndOfLineTrivia)
             )
             {
-                result.Add(ExtraNewLines.Print(member));
+                Token.NextTriviaNeedsLine = true;
             }
             else if (
                 addBlankLine
-                && !member
-                    .GetLeadingTrivia()
-                    .Any(
-                        o =>
-                            o.RawSyntaxKind()
-                                is SyntaxKind.EndIfDirectiveTrivia
-                                    or SyntaxKind.EndRegionDirectiveTrivia
-                    )
+                && triviaContainsEndIfOrRegion
+                && !leadingTrivia.Contains(SyntaxKind.IfDirectiveTrivia)
+                && !leadingTrivia.Contains(SyntaxKind.ElifDirectiveTrivia)
+                && !leadingTrivia.Contains(SyntaxKind.ElseDirectiveTrivia)
+                && !leadingTrivia.Contains(SyntaxKind.EndOfLineTrivia)
+                && !printExtraNewLines
             )
             {
-                result.Add(Doc.HardLine);
+                Token.NextTriviaNeedsLine = true;
             }
 
             result.Add(Doc.HardLine, Node.Print(member));

diff --git a/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/BaseMethodDeclaration.cs b/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/BaseMethodDeclaration.cs
@@ -9,7 +9,7 @@ public static Doc Print(CSharpSyntaxNode node)
         TypeSyntax? returnType = null;
         ExplicitInterfaceSpecifierSyntax? explicitInterfaceSpecifier = null;
         TypeParameterListSyntax? typeParameterList = null;
-        Doc identifier = Doc.Null;
+        Func<Doc>? identifier = null;
         SyntaxList<TypeParameterConstraintClauseSyntax>? constraintClauses = null;
         ParameterListSyntax? parameterList = null;
         ConstructorInitializerSyntax? constructorInitializer = null;
@@ -30,20 +30,21 @@ public static Doc Print(CSharpSyntaxNode node)
             {
                 returnType = methodDeclarationSyntax.ReturnType;
                 explicitInterfaceSpecifier = methodDeclarationSyntax.ExplicitInterfaceSpecifier;
-                identifier = Token.Print(methodDeclarationSyntax.Identifier);
+                identifier = () => Token.Print(methodDeclarationSyntax.Identifier);
                 typeParameterList = methodDeclarationSyntax.TypeParameterList;
                 constraintClauses = methodDeclarationSyntax.ConstraintClauses;
             }
             else if (node is DestructorDeclarationSyntax destructorDeclarationSyntax)
             {
-                identifier = Doc.Concat(
-                    Token.Print(destructorDeclarationSyntax.TildeToken),
-                    Token.Print(destructorDeclarationSyntax.Identifier)
-                );
+                identifier = () =>
+                    Doc.Concat(
+                        Token.Print(destructorDeclarationSyntax.TildeToken),
+                        Token.Print(destructorDeclarationSyntax.Identifier)
+                    );
             }
             else if (node is ConstructorDeclarationSyntax constructorDeclarationSyntax)
             {
-                identifier = Token.Print(constructorDeclarationSyntax.Identifier);
+                identifier = () => Token.Print(constructorDeclarationSyntax.Identifier);
                 constructorInitializer = constructorDeclarationSyntax.Initializer;
             }
 
@@ -54,7 +55,7 @@ public static Doc Print(CSharpSyntaxNode node)
             attributeLists = localFunctionStatementSyntax.AttributeLists;
             modifiers = localFunctionStatementSyntax.Modifiers;
             returnType = localFunctionStatementSyntax.ReturnType;
-            identifier = Token.Print(localFunctionStatementSyntax.Identifier);
+            identifier = () => Token.Print(localFunctionStatementSyntax.Identifier);
             typeParameterList = localFunctionStatementSyntax.TypeParameterList;
             parameterList = localFunctionStatementSyntax.ParameterList;
             constraintClauses = localFunctionStatementSyntax.ConstraintClauses;
@@ -64,6 +65,7 @@ public static Doc Print(CSharpSyntaxNode node)
         }
 
         var docs = new List<Doc>();
+
         if (node is LocalFunctionStatementSyntax)
         {
             docs.Add(ExtraNewLines.Print(node));
@@ -74,6 +76,15 @@ public static Doc Print(CSharpSyntaxNode node)
             docs.Add(AttributeLists.Print(node, attributeLists.Value));
         }
 
+        if (modifiers is { Count: > 0 })
+        {
+            docs.Add(Token.PrintLeadingTrivia(modifiers.Value[0]));
+        }
+        else if (returnType != null)
+        {
+            docs.Add(Token.PrintLeadingTrivia(returnType.GetLeadingTrivia()));
+        }
+
         var declarationGroup = new List<Doc>();
 
         if (modifiers.HasValue && modifiers.Value.Any())
@@ -83,7 +94,7 @@ public static Doc Print(CSharpSyntaxNode node)
 
         if (returnType != null)
         {
-            if (!(modifiers.HasValue && modifiers.Value.Count > 0))
+            if (modifiers is not { Count: > 0 })
             {
                 Token.ShouldSkipNextLeadingTrivia = true;
             }
@@ -100,9 +111,9 @@ public static Doc Print(CSharpSyntaxNode node)
             );
         }
 
-        if (identifier != Doc.Null)
+        if (identifier != null)
         {
-            declarationGroup.Add(identifier);
+            declarationGroup.Add(identifier());
         }
 
         if (node is ConversionOperatorDeclarationSyntax conversionOperatorDeclarationSyntax)
@@ -181,15 +192,6 @@ public static Doc Print(CSharpSyntaxNode node)
             }
         }
 
-        if (modifiers is { Count: > 0 })
-        {
-            docs.Add(Token.PrintLeadingTrivia(modifiers.Value[0]));
-        }
-        else if (returnType != null)
-        {
-            docs.Add(Token.PrintLeadingTrivia(returnType.GetLeadingTrivia()));
-        }
-
         docs.Add(Doc.Group(declarationGroup));
 
         if (constraintClauses != null)

diff --git a/Src/CSharpier/SyntaxPrinter/Token.cs b/Src/CSharpier/SyntaxPrinter/Token.cs
@@ -5,6 +5,9 @@ internal static class Token
     [ThreadStatic]
     public static bool ShouldSkipNextLeadingTrivia;
 
+    [ThreadStatic]
+    public static bool NextTriviaNeedsLine;
+
     public static Doc PrintWithoutLeadingTrivia(SyntaxToken syntaxToken)
     {
         return PrintSyntaxToken(syntaxToken, null, skipLeadingTrivia: true);
@@ -198,16 +201,43 @@ void AddLeadingComment(CommentType commentType)
                     Doc.Directive(triviaText),
                     Doc.HardLineSkipBreakIfFirstInGroup
                 );
+
+                if (kind is SyntaxKind.EndIfDirectiveTrivia)
+                {
+                    if (
+                        x + 1 < leadingTrivia.Count
+                        && leadingTrivia[x + 1].RawSyntaxKind() is SyntaxKind.EndOfLineTrivia
+                    )
+                    {
+                        x++;
+                        docs.Add(Doc.HardLineSkipBreakIfFirstInGroup);
+                    }
+                    printNewLines = false;
+                }
             }
 
             PreprocessorSymbols.AddSymbolSet(trivia);
         }
 
-        if (skipLastHardline && docs.Any() && docs.Last() is HardLine)
+        while (skipLastHardline && docs.Any() && docs.Last() is HardLine)
         {
             docs.RemoveAt(docs.Count - 1);
         }
 
+        if (NextTriviaNeedsLine)
+        {
+            if (leadingTrivia.Any(o => o.RawSyntaxKind() is SyntaxKind.IfDirectiveTrivia))
+            {
+                docs.Insert(0, Doc.HardLineSkipBreakIfFirstInGroup);
+            }
+            else
+            {
+                docs.Add(Doc.HardLineSkipBreakIfFirstInGroup);
+            }
+
+            NextTriviaNeedsLine = false;
+        }
+
         return docs.Count > 0 ? Doc.Concat(docs) : Doc.Null;
     }
 

diff --git a/Src/CSharpier/SyntaxTriviaExtensions.cs b/Src/CSharpier/SyntaxTriviaExtensions.cs
@@ -4,7 +4,12 @@ public static class SyntaxTriviaExtensions
 {
     public static bool IsComment(this SyntaxTrivia syntaxTrivia)
     {
-        return syntaxTrivia.RawSyntaxKind()
+        return syntaxTrivia.RawSyntaxKind().IsComment();
+    }
+
+    public static bool IsComment(this SyntaxKind syntaxKind)
+    {
+        return syntaxKind
             is SyntaxKind.SingleLineCommentTrivia
                 or SyntaxKind.MultiLineCommentTrivia
                 or SyntaxKind.SingleLineDocumentationCommentTrivia