Merge pull request #1922 from sharwell/fix-1897

Fixed issues with file headers in UsingCodeFixProvider

Author: sharwell

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

@@ -27,7 +27,8 @@ namespace StyleCop.Analyzers.OrderingRules
     internal sealed class UsingCodeFixProvider : CodeFixProvider
     {
         private static readonly List<UsingDirectiveSyntax> EmptyUsingsList = new List<UsingDirectiveSyntax>();
-        private static readonly SyntaxAnnotation UsingCodeFixAnnotation = new SyntaxAnnotation(nameof(UsingCodeFixProvider));
+        private static readonly SyntaxAnnotation UsingCodeFixAnnotation = new SyntaxAnnotation(nameof(UsingCodeFixAnnotation));
+        private static readonly SyntaxAnnotation FileHeaderStrippedAnnotation = new SyntaxAnnotation(nameof(FileHeaderStrippedAnnotation));
 
         /// <inheritdoc/>
         public override ImmutableArray<string> FixableDiagnosticIds { get; } =
@@ -175,6 +176,13 @@ private static async Task<Document> GetTransformedDocumentAsync(Document documen
 
         private static SyntaxNode ReAddFileHeader(SyntaxNode syntaxRoot, SyntaxNode newSyntaxRoot)
         {
+            // Only re-add the file header if it was stripped.
+            var usingDirectives = newSyntaxRoot.GetAnnotatedNodes(FileHeaderStrippedAnnotation);
+            if (!usingDirectives.Any())
+            {
+                return newSyntaxRoot;
+            }
+
             var oldFirstToken = syntaxRoot.GetFirstToken();
             if (!oldFirstToken.HasLeadingTrivia)
             {
@@ -584,12 +592,6 @@ internal static List<SyntaxTrivia> GetFileHeader(SyntaxTriviaList newLeadingTriv
                 return hasHeader ? fileHeader : new List<SyntaxTrivia>();
             }
 
-            private static List<SyntaxTrivia> StripFileHeader(SyntaxTriviaList leadingTrivia)
-            {
-                var fileHeader = GetFileHeader(leadingTrivia);
-                return leadingTrivia.Skip(fileHeader.Count).ToList();
-            }
-
             private List<UsingDirectiveSyntax> GenerateUsings(Dictionary<DirectiveSpan, List<UsingDirectiveSyntax>> usingsGroup, DirectiveSpan directiveSpan, string indentation, List<SyntaxTrivia> triviaToMove, bool qualifyNames)
             {
                 List<UsingDirectiveSyntax> result = new List<UsingDirectiveSyntax>();
@@ -614,15 +616,29 @@ private List<UsingDirectiveSyntax> GenerateUsings(List<UsingDirectiveSyntax> usi
 
                 for (var i = 0; i < usingsList.Count; i++)
                 {
+                    bool strippedFileHeader = false;
                     var currentUsing = usingsList[i];
 
                     if (qualifyNames)
                     {
                         currentUsing = this.QualifyUsingDirective(currentUsing);
                     }
 
+                    // strip the file header, if the using is the first node in the source file.
+                    List<SyntaxTrivia> leadingTrivia;
+                    if ((i == 0) && currentUsing.GetFirstToken().GetPreviousToken().IsMissingOrDefault())
+                    {
+                        var trivia = currentUsing.GetLeadingTrivia();
+                        var fileHeader = GetFileHeader(trivia);
+                        leadingTrivia = trivia.Skip(fileHeader.Count).ToList();
+                        strippedFileHeader = fileHeader.Count > 0;
+                    }
+                    else
+                    {
+                        leadingTrivia = currentUsing.GetLeadingTrivia().ToList();
+                    }
+
                     // when there is a directive trivia, add it (and any trivia before it) to the triviaToMove collection.
-                    var leadingTrivia = (i == 0) ? StripFileHeader(currentUsing.GetLeadingTrivia()) : currentUsing.GetLeadingTrivia().ToList();
                     for (var m = leadingTrivia.Count - 1; m >= 0; m--)
                     {
                         if (leadingTrivia[m].IsDirective)
@@ -689,6 +705,11 @@ private List<UsingDirectiveSyntax> GenerateUsings(List<UsingDirectiveSyntax> usi
                         .WithTrailingTrivia(newTrailingTrivia)
                         .WithAdditionalAnnotations(UsingCodeFixAnnotation);
 
+                    if (strippedFileHeader)
+                    {
+                        processedUsing = processedUsing.WithAdditionalAnnotations(FileHeaderStrippedAnnotation);
+                    }
+
                     // filter duplicate using declarations, preferring to keep the one with an alias
                     var existingUsing = result.Find(u => string.Equals(u.Name.ToNormalizedString(), processedUsing.Name.ToNormalizedString(), StringComparison.Ordinal));
                     if (existingUsing != null)

StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1210UnitTests.cs

@@ -317,6 +317,101 @@ public async Task TestPreprocessorDirectivesAsync()
             await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false);
         }
 
+        /// <summary>
+        /// This is a regression test for DotNetAnalyzers/StyleCopAnalyzers#1897.
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Fact]
+        public async Task TestInvalidOrderedUsingDirectivesInNamespaceDeclarationWithFileHeaderAsync()
+        {
+            var testCode = @"// <copyright file=""VoiceCommandService.cs"" company=""Foo Corporation"">
+// Copyright (c) FooCorporation. All rights reserved.
+// </copyright>
+
+namespace Foo.Garage.XYZ
+{
+    using System;
+    using Newtonsoft.Json;
+    using Foo.Garage.XYZ;
+}
+
+namespace Newtonsoft.Json
+{
+}
+";
+
+            var fixedTestCode = @"// <copyright file=""VoiceCommandService.cs"" company=""Foo Corporation"">
+// Copyright (c) FooCorporation. All rights reserved.
+// </copyright>
+
+namespace Foo.Garage.XYZ
+{
+    using System;
+    using Foo.Garage.XYZ;
+    using Newtonsoft.Json;
+}
+
+namespace Newtonsoft.Json
+{
+}
+";
+
+            // The same diagnostic is reported multiple times due to a bug in Roslyn 1.0
+            DiagnosticResult[] expected =
+            {
+                this.CSharpDiagnostic().WithLocation(8, 5),
+                this.CSharpDiagnostic().WithLocation(8, 5),
+                this.CSharpDiagnostic().WithLocation(8, 5),
+            };
+
+            await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpDiagnosticAsync(fixedTestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpFixAsync(testCode, fixedTestCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
+        }
+
+        /// <summary>
+        /// Verifies that the first using statement will preserve its leading comment.
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Fact]
+        public async Task TestLeadingCommentForFirstUsingInNamespaceIsPreservedAsync()
+        {
+            var testCode = @"namespace TestNamespace
+{
+    // With test comment
+    using System;
+    using TestNamespace;
+    using Newtonsoft.Json;
+}
+
+namespace Newtonsoft.Json
+{
+}
+";
+
+            var fixedTestCode = @"namespace TestNamespace
+{
+    // With test comment
+    using System;
+    using Newtonsoft.Json;
+    using TestNamespace;
+}
+
+namespace Newtonsoft.Json
+{
+}
+";
+
+            DiagnosticResult[] expected =
+            {
+                this.CSharpDiagnostic().WithLocation(5, 5),
+            };
+
+            await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpDiagnosticAsync(fixedTestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpFixAsync(testCode, fixedTestCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
+        }
+
         /// <inheritdoc/>
         protected override IEnumerable<string> GetDisabledDiagnostics()
         {