Merge pull request #1440 from sharwell/fix-1437

Fix handling of line endings in FileHeaderCodeFixProvider + SA1635 code fix
DotNetAnalyzers · Sep 10, 2015 · 988ef21 · 988ef21
2 parents b946558 + a53719a
commit 988ef21
Show file tree

Hide file tree

Showing 3 changed files with 95 additions and 31 deletions.
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/NoXmlFileHeaderUnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/NoXmlFileHeaderUnitTests.cs
@@ -7,15 +7,15 @@ namespace StyleCop.Analyzers.Test.DocumentationRules
     using System.Threading;
     using System.Threading.Tasks;
     using Analyzers.DocumentationRules;
-    using Microsoft.CodeAnalysis;
+    using Microsoft.CodeAnalysis.CodeFixes;
     using Microsoft.CodeAnalysis.Diagnostics;
     using TestHelper;
     using Xunit;
 
     /// <summary>
     /// Unit tests for file header that do not follow the XML syntax.
     /// </summary>
-    public class NoXmlFileHeaderUnitTests : DiagnosticVerifier
+    public class NoXmlFileHeaderUnitTests : CodeFixVerifier
     {
         private const string SettingsFileName = "stylecop.json";
         private const string TestSettings = @"
@@ -45,10 +45,19 @@ public virtual async Task TestNoFileHeaderAsync()
             var testCode = @"namespace Foo
 {
 }
+";
+            var fixedCode = @"// Copyright (c) FooCorp. All rights reserved.
+// Licensed under the ??? license. See LICENSE file in the project root for full license information.
+
+namespace Foo
+{
+}
 ";
 
             var expectedDiagnostic = this.CSharpDiagnostic(FileHeaderAnalyzers.SA1633DescriptorMissing).WithLocation(1, 1);
             await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnostic, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
         }
 
         /// <summary>
@@ -87,13 +96,13 @@ namespace Bar
         }
 
         /// <summary>
-        /// Verifies that a valid file header built using multi line comments will not produce a diagnostic message.
+        /// Verifies that a valid file header built using multi-line comments will not produce a diagnostic message.
         /// </summary>
         /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
         [Fact]
-        public async Task TestValidFileHeaderWithMultiLineCommentsAsync()
+        public async Task TestValidFileHeaderWithMultiLineComments1Async()
         {
-            var testCodeFormat1 = @"/* Copyright (c) FooCorp. All rights reserved.
+            var testCode = @"/* Copyright (c) FooCorp. All rights reserved.
  * Licensed under the ??? license. See LICENSE file in the project root for full license information.
  */
 
@@ -102,42 +111,55 @@ namespace Bar
 }
 ";
 
-            var testCodeFormat2 = @"/* Copyright (c) FooCorp. All rights reserved.
+            await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
+        /// <summary>
+        /// Verifies that a valid file header built using multi-line comments will not produce a diagnostic message.
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Fact]
+        public async Task TestValidFileHeaderWithMultiLineComments2Async()
+        {
+            var testCode = @"/* Copyright (c) FooCorp. All rights reserved.
    Licensed under the ??? license. See LICENSE file in the project root for full license information. */
 
 namespace Bar
 {
 }
 ";
 
-            await this.VerifyCSharpDiagnosticAsync(testCodeFormat1, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
-            await this.VerifyCSharpDiagnosticAsync(testCodeFormat2, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
         }
 
         /// <summary>
         /// Verifies that a file header without text / only whitespace will produce the expected diagnostic message.
         /// </summary>
+        /// <param name="comment">The comment text.</param>
         /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
-        [Fact]
-        public async Task TestInvalidFileHeaderWithoutTextAsync()
+        [Theory]
+        [InlineData("//")]
+        [InlineData("//    ")]
+        public async Task TestInvalidFileHeaderWithoutTextAsync(string comment)
         {
-            var testCodeFormat1 = @"//
+            var testCode = $@"{comment}
 
 namespace Bar
-{
-}
+{{
+}}
 ";
-
-            var testCodeFormat2 = "//    " + @"
+            var fixedCode = @"// Copyright (c) FooCorp. All rights reserved.
+// Licensed under the ??? license. See LICENSE file in the project root for full license information.
 
 namespace Bar
 {
 }
 ";
 
             var expected = this.CSharpDiagnostic(FileHeaderAnalyzers.SA1635Descriptor).WithLocation(1, 1);
-            await this.VerifyCSharpDiagnosticAsync(testCodeFormat1, expected, CancellationToken.None).ConfigureAwait(false);
-            await this.VerifyCSharpDiagnosticAsync(testCodeFormat2, expected, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
         }
 
         /// <summary>
@@ -150,12 +172,21 @@ public async Task TestInvalidFileHeaderWithWrongTextAsync()
             var testCode = @"// Copyright (c) BarCorp. All rights reserved.
 // Licensed under the ??? license. See LICENSE file in the project root for full license information.
 
+namespace Bar
+{
+}
+";
+            var fixedCode = @"// Copyright (c) FooCorp. All rights reserved.
+// Licensed under the ??? license. See LICENSE file in the project root for full license information.
+
 namespace Bar
 {
 }
 ";
             var expected = this.CSharpDiagnostic(FileHeaderAnalyzers.SA1636Descriptor).WithLocation(1, 1);
             await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
         }
 
         /// <inheritdoc/>
@@ -169,5 +200,11 @@ protected sealed override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAna
         {
             yield return new FileHeaderAnalyzers();
         }
+
+        /// <inheritdoc/>
+        protected override CodeFixProvider GetCSharpCodeFixProvider()
+        {
+            return new FileHeaderCodeFixProvider();
+        }
     }
 }
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1635UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1635UnitTests.cs
@@ -23,13 +23,23 @@ public async Task TestFileHeaderWithShorthandCopyrightAsync()
         {
             var testCode = @"// <copyright file=""Test0.cs"" company=""FooCorp""/>
 
+namespace Bar
+{
+}
+";
+            var fixedCode = @"// <copyright file=""Test0.cs"" company=""FooCorp"">
+// Copyright (c) FooCorp. All rights reserved.
+// </copyright>
+
 namespace Bar
 {
 }
 ";
 
             var expectedDiagnostic = this.CSharpDiagnostic(FileHeaderAnalyzers.SA1635Descriptor).WithLocation(1, 4);
             await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnostic, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
         }
 
         /// <summary>
@@ -47,14 +57,25 @@ public async Task TestFileHeaderWithWhitespaceOnlyCopyrightAsync()
                 "namespace Bar\r\n" +
                 "{\r\n" +
                 "}\r\n";
+            string fixedCode =
+                "// <copyright file=\"Test0.cs\" company=\"FooCorp\">\r\n" +
+                "// Copyright (c) FooCorp. All rights reserved.\r\n" +
+                "// </copyright>\r\n" +
+                "\r\n" +
+                "namespace Bar\r\n" +
+                "{\r\n" +
+                "}\r\n";
 
             var expectedDiagnostic = this.CSharpDiagnostic(FileHeaderAnalyzers.SA1635Descriptor).WithLocation(1, 4);
             await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnostic, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
         }
 
+        /// <inheritdoc/>
         protected override CodeFixProvider GetCSharpCodeFixProvider()
         {
-            throw new System.NotImplementedException();
+            return new FileHeaderCodeFixProvider();
         }
     }
 }
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/FileHeaderCodeFixProvider.cs b/StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/FileHeaderCodeFixProvider.cs
@@ -12,6 +12,7 @@ namespace StyleCop.Analyzers.DocumentationRules
     using Microsoft.CodeAnalysis.CodeActions;
     using Microsoft.CodeAnalysis.CodeFixes;
     using Microsoft.CodeAnalysis.CSharp;
+    using Microsoft.CodeAnalysis.Formatting;
     using StyleCop.Analyzers.Helpers;
     using StyleCop.Analyzers.Settings.ObjectModel;
 
@@ -29,6 +30,7 @@ public class FileHeaderCodeFixProvider : CodeFixProvider
         public override ImmutableArray<string> FixableDiagnosticIds { get; }
             = ImmutableArray.Create(
                 FileHeaderAnalyzers.SA1633DescriptorMissing.Id,
+                FileHeaderAnalyzers.SA1635Descriptor.Id,
                 FileHeaderAnalyzers.SA1636Descriptor.Id);
 
         /// <inheritdoc/>
@@ -54,7 +56,7 @@ private static async Task<Document> GetTransformedDocumentAsync(Document documen
             var settings = document.Project.AnalyzerOptions.GetStyleCopSettings();
 
             var fileHeader = FileHeaderHelpers.ParseFileHeader(root);
-            var newSyntaxRoot = fileHeader.IsMissing ? AddHeader(root, document.Name, settings) : ReplaceHeader(document, root, settings);
+            var newSyntaxRoot = fileHeader.IsMissing ? AddHeader(document, root, document.Name, settings) : ReplaceHeader(document, root, settings);
 
             return document.WithSyntaxRoot(newSyntaxRoot);
         }
@@ -104,36 +106,40 @@ private static SyntaxNode ReplaceHeader(Document document, SyntaxNode root, Styl
                 }
             }
 
-            return root.WithLeadingTrivia(CreateNewHeader(document.Name, settings).Add(SyntaxFactory.CarriageReturnLineFeed).Add(SyntaxFactory.CarriageReturnLineFeed).AddRange(trivia));
+            string newLineText = document.Project.Solution.Workspace.Options.GetOption(FormattingOptions.NewLine, LanguageNames.CSharp);
+            return root.WithLeadingTrivia(CreateNewHeader(document.Name, settings, newLineText).Add(SyntaxFactory.CarriageReturnLineFeed).Add(SyntaxFactory.CarriageReturnLineFeed).AddRange(trivia));
         }
 
-        private static SyntaxNode AddHeader(SyntaxNode root, string name, StyleCopSettings settings)
+        private static SyntaxNode AddHeader(Document document, SyntaxNode root, string name, StyleCopSettings settings)
         {
-            var newTrivia = CreateNewHeader(name, settings).Add(SyntaxFactory.CarriageReturnLineFeed).Add(SyntaxFactory.CarriageReturnLineFeed);
+            string newLineText = document.Project.Solution.Workspace.Options.GetOption(FormattingOptions.NewLine, LanguageNames.CSharp);
+            var newTrivia = CreateNewHeader(name, settings, newLineText).Add(SyntaxFactory.CarriageReturnLineFeed).Add(SyntaxFactory.CarriageReturnLineFeed);
             newTrivia = newTrivia.AddRange(root.GetLeadingTrivia());
 
             return root.WithLeadingTrivia(newTrivia);
         }
 
-        private static SyntaxTriviaList CreateNewHeader(string filename, StyleCopSettings settings)
+        private static SyntaxTriviaList CreateNewHeader(string filename, StyleCopSettings settings, string newLineText)
         {
-            var copyrightText = "// " + GetCopyrightText(settings.DocumentationRules.CopyrightText);
+            var copyrightText = "// " + GetCopyrightText(settings.DocumentationRules.CopyrightText, newLineText);
             var newHeader = settings.DocumentationRules.XmlHeader
-                ? WrapInXmlComment(copyrightText, filename, settings)
+                ? WrapInXmlComment(copyrightText, filename, settings, newLineText)
                 : copyrightText;
             return SyntaxFactory.ParseLeadingTrivia(newHeader);
         }
 
-        private static string WrapInXmlComment(string copyrightText, string filename, StyleCopSettings settings)
+        private static string WrapInXmlComment(string copyrightText, string filename, StyleCopSettings settings, string newLineText)
         {
-            return $@"// <copyright file=""{filename}"" company=""{settings.DocumentationRules.CompanyName}"">
-{copyrightText}
-// </copyright>";
+            return
+                $"// <copyright file=\"{filename}\" company=\"{settings.DocumentationRules.CompanyName}\">" + newLineText
+                + copyrightText + newLineText
+                + "// </copyright>";
         }
 
-        private static string GetCopyrightText(string copyrightText)
+        private static string GetCopyrightText(string copyrightText, string newLineText)
         {
-            return string.Join("\n// ", copyrightText.Split('\n'));
+            copyrightText = copyrightText.Replace("\r\n", "\n");
+            return string.Join(newLineText + "// ", copyrightText.Split('\n'));
         }
     }
 }