Merge pull request #2435 from sharwell/fix-using-trivia

Fix handling of trivia before first using directive

Author: sharwell

StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/OrderingRules/UsingCodeFixProvider.UsingsSorter.cs

@@ -173,20 +173,24 @@ private List<UsingDirectiveSyntax> GenerateUsings(List<UsingDirectiveSyntax> usi
 
                     // when there is a directive trivia, add it (and any trivia before it) to the triviaToMove collection.
                     // when there are leading blank lines for the first entry, add them to the triviaToMove collection.
-                    var previousIsEndOfLine = true;
+                    int triviaToMoveCount = triviaToMove.Count;
+                    var previousIsEndOfLine = false;
                     for (var m = leadingTrivia.Count - 1; m >= 0; m--)
                     {
                         if (leadingTrivia[m].IsDirective)
                         {
-                            triviaToMove.InsertRange(0, leadingTrivia.Take(m + 1));
+                            // When a directive is followed by a blank line, keep the blank line with the directive.
+                            int takeCount = previousIsEndOfLine ? m + 2 : m + 1;
+                            triviaToMove.InsertRange(0, leadingTrivia.Take(takeCount));
                             break;
                         }
 
                         if ((i == 0) && leadingTrivia[m].IsKind(SyntaxKind.EndOfLineTrivia))
                         {
                             if (previousIsEndOfLine)
                             {
-                                triviaToMove.Insert(0, leadingTrivia[m]);
+                                triviaToMove.InsertRange(0, leadingTrivia.Take(m + 2));
+                                break;
                             }
 
                             previousIsEndOfLine = true;
@@ -200,6 +204,56 @@ private List<UsingDirectiveSyntax> GenerateUsings(List<UsingDirectiveSyntax> usi
                     // preserve leading trivia (excluding directive trivia), indenting each line as appropriate
                     var newLeadingTrivia = leadingTrivia.Except(triviaToMove).ToList();
 
+                    // indent the triviaToMove if necessary so it behaves correctly later
+                    bool atStartOfLine = triviaToMoveCount == 0 || triviaToMove.Last().HasBuiltinEndLine();
+                    for (int m = triviaToMoveCount; m < triviaToMove.Count; m++)
+                    {
+                        bool currentAtStartOfLine = atStartOfLine;
+                        atStartOfLine = triviaToMove[m].HasBuiltinEndLine();
+                        if (!currentAtStartOfLine)
+                        {
+                            continue;
+                        }
+
+                        if (triviaToMove[m].IsKind(SyntaxKind.EndOfLineTrivia))
+                        {
+                            // This is a blank line; indenting it would only add trailing whitespace
+                            continue;
+                        }
+
+                        if (triviaToMove[m].IsDirective)
+                        {
+                            // Only #region and #endregion directives get indented
+                            if (!triviaToMove[m].IsKind(SyntaxKind.RegionDirectiveTrivia) && !triviaToMove[m].IsKind(SyntaxKind.EndRegionDirectiveTrivia))
+                            {
+                                // This is a preprocessor line that doesn't need to be indented
+                                continue;
+                            }
+                        }
+
+                        if (triviaToMove[m].IsKind(SyntaxKind.DisabledTextTrivia))
+                        {
+                            // This is text in a '#if false' block; just ignore it
+                            continue;
+                        }
+
+                        if (string.IsNullOrEmpty(indentation))
+                        {
+                            if (triviaToMove[m].IsKind(SyntaxKind.WhitespaceTrivia))
+                            {
+                                // Remove the trivia and analyze the current position again
+                                triviaToMove.RemoveAt(m);
+                                m--;
+                                atStartOfLine = true;
+                            }
+                        }
+                        else
+                        {
+                            triviaToMove.Insert(m, SyntaxFactory.Whitespace(indentation));
+                            m++;
+                        }
+                    }
+
                     // strip any leading whitespace on each line (and also blank lines)
                     var k = 0;
                     var startOfLine = true;

StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/OrderingRules/UsingCodeFixProvider.cs

@@ -352,7 +352,7 @@ private static SyntaxNode StripMultipleBlankLines(SyntaxNode syntaxRoot)
 
         private static ImmutableArray<SyntaxTrivia> GetFileHeader(SyntaxNode syntaxRoot)
         {
-            var onBlankLine = false;
+            var onBlankLine = true;
             var hasHeader = false;
             var fileHeaderBuilder = ImmutableArray.CreateBuilder<SyntaxTrivia>();
 
@@ -369,14 +369,14 @@ private static ImmutableArray<SyntaxTrivia> GetFileHeader(SyntaxNode syntaxRoot)
                 case SyntaxKind.MultiLineCommentTrivia:
                     fileHeaderBuilder.Add(firstTokenLeadingTrivia[i]);
                     onBlankLine = false;
-                    hasHeader = true;
                     break;
 
                 case SyntaxKind.WhitespaceTrivia:
                     fileHeaderBuilder.Add(firstTokenLeadingTrivia[i]);
                     break;
 
                 case SyntaxKind.EndOfLineTrivia:
+                    hasHeader = true;
                     fileHeaderBuilder.Add(firstTokenLeadingTrivia[i]);
 
                     if (onBlankLine)

StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1200OutsideNamespaceUnitTests.cs

@@ -129,10 +129,14 @@ public async Task TestInvalidSimplifiedUsingStatementsInExtensionNamespaceAsync(
 {
     using System.Threading;
     using Reflection;
+    using Assembly = Reflection.Assembly;
+    using List = Collections.Generic.IList<int>;
 }
 ";
             var fixedTestCode = @"using System.Reflection;
 using System.Threading;
+using Assembly = System.Reflection.Assembly;
+using List = System.Collections.Generic.IList<int>;
 
 namespace System.MyExtension
 {
@@ -143,6 +147,8 @@ namespace System.MyExtension
             {
                 this.CSharpDiagnostic(SA1200UsingDirectivesMustBePlacedCorrectly.DescriptorOutside).WithLocation(3, 5),
                 this.CSharpDiagnostic(SA1200UsingDirectivesMustBePlacedCorrectly.DescriptorOutside).WithLocation(4, 5),
+                this.CSharpDiagnostic(SA1200UsingDirectivesMustBePlacedCorrectly.DescriptorOutside).WithLocation(5, 5),
+                this.CSharpDiagnostic(SA1200UsingDirectivesMustBePlacedCorrectly.DescriptorOutside).WithLocation(6, 5),
             };
 
             await this.VerifyCSharpDiagnosticAsync(testCode, expectedResults, CancellationToken.None).ConfigureAwait(false);
@@ -264,6 +270,46 @@ namespace TestNamespace
             await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false);
         }
 
+        [Fact]
+        public async Task TestFileHeaderIsProperlyPreservedWhenMovingUsingStatementsWithCommentsAsync()
+        {
+            var testCode = @"// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved.
+// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information.
+
+namespace TestNamespace
+{
+    // Separated Comment
+
+    using System.Collections;
+    // Comment
+    using System;
+}
+";
+            var fixedTestCode = @"// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved.
+// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information.
+
+// Separated Comment
+
+// Comment
+using System;
+using System.Collections;
+
+namespace TestNamespace
+{
+}
+";
+
+            DiagnosticResult[] expectedResults =
+            {
+                this.CSharpDiagnostic(SA1200UsingDirectivesMustBePlacedCorrectly.DescriptorOutside).WithLocation(8, 5),
+                this.CSharpDiagnostic(SA1200UsingDirectivesMustBePlacedCorrectly.DescriptorOutside).WithLocation(10, 5),
+            };
+
+            await this.VerifyCSharpDiagnosticAsync(testCode, expectedResults, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpDiagnosticAsync(fixedTestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false);
+        }
+
         /// <inheritdoc/>
         protected override string GetSettings()
         {