Merge pull request #143 from jaredpar/issue-83

Fix double new lines with extern directives

Author: jaredpar

src/Microsoft.DotNet.CodeFormatting.Tests/Rules/NewLineAboveRuleTests.cs

@@ -58,8 +58,8 @@ public void UsingWithOnlyDirectiveAbove()
             var text = @"#pragma warning disable 1591
 using System;";
 
-            var expected = @"
-#pragma warning disable 1591
+            var expected = @"#pragma warning disable 1591
+
 using System;";
 
             Verify(text, expected);
@@ -158,8 +158,8 @@ public void TestNewLineBeforeFirstUsing03()
 ";
             var expected = @"
 // copyright comment
-
 #pragma warning disable 1591
+
 using System;
 ";
             Verify(text, expected);
@@ -228,6 +228,7 @@ namespace N { }
 using System;
 
 #pragma warning disable 1591
+
 namespace N { }
 ";
             Verify(text, expected);
@@ -245,5 +246,23 @@ namespace N { }
 ";
             Verify(text, text);
         }
+
+        [Fact]
+        public void Issue83()
+        {
+            var text = @"
+extern alias PDB;
+using System;
+namespace N { }";
+
+            var expected = @"
+extern alias PDB;
+
+using System;
+
+namespace N { }";
+
+            Verify(text, expected);
+        }
     }
 }

src/Microsoft.DotNet.CodeFormatting/Rules/NewLineAboveRule.cs

@@ -5,6 +5,7 @@
 using System.Collections;
 using System.Collections.Generic;
 using System.ComponentModel.Composition;
+using System.Diagnostics;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
@@ -80,8 +81,8 @@ private SyntaxNode ProcessCore<TNode>(SyntaxNode syntaxRoot, TNode node) where T
                 return syntaxRoot;
             }
 
-            SyntaxTriviaList newTriviaList;
-            if (!TryGetNewLeadingTrivia(node, out newTriviaList))
+            var newTriviaList = GetNewLeadingTrivia(node);
+            if (newTriviaList == node.GetLeadingTrivia())
             {
                 return syntaxRoot;
             }
@@ -93,69 +94,104 @@ private SyntaxNode ProcessCore<TNode>(SyntaxNode syntaxRoot, TNode node) where T
         /// Get the new leading trivia list that will add the double blank line that we are looking
         /// for.
         /// </summary>
-        private bool TryGetNewLeadingTrivia(SyntaxNode node, out SyntaxTriviaList newTriviaList)
+        private SyntaxTriviaList GetNewLeadingTrivia(SyntaxNode node)
         {
-            var newLineTrivia = SyntaxUtil.GetBestNewLineTrivia(node);
             var list = node.GetLeadingTrivia();
-            var index = list.Count - 1;
+            var searchIndex = 0;
+            var newLineTrivia = SyntaxUtil.GetBestNewLineTrivia(node);
+            var prev = node.FindPreviousNodeInParent();
 
-            MoveBackwardsPastWhitespace(list, ref index);
-            if (index < 0 || !list[index].IsAnyEndOfLine())
+            if (prev == null)
             {
-                // There is no newline before the using at all.  Add a double newline to 
-                // get the blank we are looking for
-                newTriviaList = list.InsertRange(index + 1, new[] { newLineTrivia, newLineTrivia });
-                return true;
+                if (node.Span.Start == 0)
+                {
+                    // First item in the file.  Do nothing for this case.  
+                    return list;
+                }
+            }
+            else
+            {
+                // If there is no new line in the trailing trivia of the previous node then need to add 
+                // one to put this node on the next line. 
+                if (prev.GetTrailingTrivia().Count == 0 || !prev.GetTrailingTrivia().Last().IsAnyEndOfLine())
+                {
+                    list = list.Insert(0, newLineTrivia);
+                    searchIndex = 1;
+                }
             }
 
-            var wasDirective = list[index].IsDirective;
-            index--;
-
-            // Move past any directives that are above the token.  The newline needs to 
-            // be above them.
-            while (index >= 0 && list[index].IsDirective)
+            // Ensure there are blank above #pragma directives here.  This is an attempt to maintain compatibility
+            // with the original design of this rule which had special spacing rules for #pragma.  No reason
+            // was given for the special casing, only tests.  
+            if (searchIndex < list.Count && list[0].IsKind(SyntaxKind.PragmaWarningDirectiveTrivia) && list[0].FullSpan.Start != 0)
             {
-                index--;
+                list = list.Insert(searchIndex, newLineTrivia);
+                searchIndex++;
             }
 
-            if (wasDirective)
+            EnsureHasBlankLineAtEnd(ref list, searchIndex, newLineTrivia);
+
+            return list;
+        }
+
+        /// <summary>
+        /// Ensure the trivia list has a blank line at the end.  Both the second to last
+        /// and final line may contain spaces. 
+        ///
+        /// Note: This function assumes the trivia token before <param name="startIndex" />
+        /// is an end of line trivia.  
+        /// </summary>
+        private static void EnsureHasBlankLineAtEnd(ref SyntaxTriviaList list, int startIndex, SyntaxTrivia newLineTrivia)
+        {
+            const int StateNone = 0;
+            const int StateEol = 1;
+            const int StateBlankLine = 2;
+
+            var state = StateEol;
+            var index = startIndex;
+            var eolIndex = startIndex - 1;
+
+            while (index < list.Count)
             {
-                // There was a directive above the using and index now points directly before 
-                // that.  This token must be a new line.  
-                if (index < 0 || !list[index].IsKind(SyntaxKind.EndOfLineTrivia))
+                var current = list[index];
+                if (current.IsKind(SyntaxKind.WhitespaceTrivia))
                 {
-                    newTriviaList = list.Insert(index + 1, newLineTrivia);
-                    return true;
+                    index++;
+                    continue;
                 }
 
-                index--;
-            }
-
-            // In the logical line above the using.  Need to see <blank><eol> in order for the 
-            // using to be correct
-            var insertIndex = index + 1;
-            MoveBackwardsPastWhitespace(list, ref index);
-            if (index < 0 || !list[index].IsAnyEndOfLine())
-            {
-                // If this is the first item in the file then there is no need for a double 
-                // blank line.  
-                if (index >= 0 || node.FullSpan.Start != 0)
+                var isStateAnyEol = (state == StateEol || state == StateBlankLine);
+                if (isStateAnyEol && current.IsKind(SyntaxKind.EndOfLineTrivia))
                 {
-                    newTriviaList = list.Insert(insertIndex, newLineTrivia);
-                    return true;
+                    state = StateBlankLine;
+                }
+                else if (current.IsAnyEndOfLine())
+                {
+                    eolIndex = index;
+                    state = StateEol;
+                }
+                else
+                {
+                    state = StateNone;
                 }
-            }
 
-            // The using is well formed so there is no work to be done. 
-            newTriviaList = SyntaxTriviaList.Empty;
-            return false;
-        }
+                index++;
+            }
 
-        private static void MoveBackwardsPastWhitespace(SyntaxTriviaList list, ref int index)
-        {
-            while (index >= 0 && list[index].IsKind(SyntaxKind.WhitespaceTrivia))
+            switch (state)
             {
-                index--;
+                case StateNone:
+                    list = list.InsertRange(list.Count, new[] { newLineTrivia, newLineTrivia });
+                    break;
+                case StateEol:
+                    list = list.Insert(eolIndex + 1, newLineTrivia);
+                    break;
+                case StateBlankLine:
+                    // Nothing to do. 
+                    break;
+                default:
+                    Debug.Assert(false);
+                    break;
             }
         }
     }

src/Microsoft.DotNet.CodeFormatting/SyntaxUtil.cs

@@ -91,5 +91,20 @@ internal static bool IsAnyEndOfLine(this SyntaxTrivia trivia)
         {
             return trivia.IsKind(SyntaxKind.EndOfLineTrivia) || trivia.IsDirective;
         }
+
+        /// <summary>
+        /// Find the node directly before this in the parent.  Returns null in the case it 
+        /// cannot be found.
+        /// </summary>
+        internal static SyntaxNode FindPreviousNodeInParent(this SyntaxNode node)
+        {
+            var parent = node.Parent;
+            if (parent == null)
+            {
+                return null;
+            }
+
+            return parent.ChildNodes().Where(x => x.FullSpan.End == node.FullSpan.Start).FirstOrDefault();
+        }
     }
 }