Merge pull request #1577 from vweijsters/fix-1556

Author: sharwell

StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1106UnitTests.cs

@@ -340,6 +340,83 @@ public async Task TestMemberAsync(string declaration)
             await this.VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
         }
 
+        /// <summary>
+        /// Verifies that the code fix will remove all unnecessary whitespace.
+        /// This is a regression for #1556
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Fact]
+        public async Task VerifyCodeFixWillRemoveUnnecessaryWhitespaceAsync()
+        {
+            var testCode = @"
+class TestClass
+{
+    public void TestMethod1()
+    {
+        throw new System.NotImplementedException(); ;
+    }
+
+    public void TestMethod2()
+    {
+        throw new System.NotImplementedException(); /* c1 */ ;
+    }
+}";
+            var fixedCode = @"
+class TestClass
+{
+    public void TestMethod1()
+    {
+        throw new System.NotImplementedException();
+    }
+
+    public void TestMethod2()
+    {
+        throw new System.NotImplementedException(); /* c1 */
+    }
+}";
+
+            DiagnosticResult[] expected =
+            {
+                this.CSharpDiagnostic().WithLocation(6, 53),
+                this.CSharpDiagnostic().WithLocation(11, 62)
+            };
+
+            await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
+        }
+
+        /// <summary>
+        /// Verifies that the code fix will not remove relevant trivia.
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Fact]
+        public async Task VerifyCodeFixWillNotRemoveTriviaAsync()
+        {
+            var testCode = @"
+class TestClass
+{
+    public void TestMethod()
+    {
+        /* do nothing */ ;
+    }
+}";
+            var fixedCode = @"
+class TestClass
+{
+    public void TestMethod()
+    {
+        /* do nothing */
+    }
+}";
+
+            var expected = this.CSharpDiagnostic().WithLocation(6, 26);
+
+            await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
+        }
+
         /// <inheritdoc/>
         protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
         {

StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1106CodeFixProvider.cs

@@ -3,10 +3,8 @@
 
 namespace StyleCop.Analyzers.ReadabilityRules
 {
-    using System.Collections.Generic;
     using System.Collections.Immutable;
     using System.Composition;
-    using System.Linq;
     using System.Threading;
     using System.Threading.Tasks;
     using Microsoft.CodeAnalysis;
@@ -16,7 +14,6 @@ namespace StyleCop.Analyzers.ReadabilityRules
     using Microsoft.CodeAnalysis.CSharp.Syntax;
     using Microsoft.CodeAnalysis.Text;
     using StyleCop.Analyzers.Helpers;
-    using StyleCop.Analyzers.SpacingRules;
 
     /// <summary>
     /// This class provides a code fix for <see cref="SA1106CodeMustNotContainEmptyStatements"/>.
@@ -25,11 +22,9 @@ namespace StyleCop.Analyzers.ReadabilityRules
     [Shared]
     internal class SA1106CodeFixProvider : CodeFixProvider
     {
-        private static readonly ImmutableArray<string> FixableDiagnostics =
-            ImmutableArray.Create(SA1106CodeMustNotContainEmptyStatements.DiagnosticId);
-
         /// <inheritdoc/>
-        public override ImmutableArray<string> FixableDiagnosticIds => FixableDiagnostics;
+        public override ImmutableArray<string> FixableDiagnosticIds { get; }
+            = ImmutableArray.Create(SA1106CodeMustNotContainEmptyStatements.DiagnosticId);
 
         /// <inheritdoc/>
         public override FixAllProvider GetFixAllProvider()
@@ -42,11 +37,6 @@ public override Task RegisterCodeFixesAsync(CodeFixContext context)
         {
             foreach (var diagnostic in context.Diagnostics)
             {
-                if (!diagnostic.Id.Equals(SA1106CodeMustNotContainEmptyStatements.DiagnosticId))
-                {
-                    continue;
-                }
-
                 context.RegisterCodeFix(
                     CodeAction.Create(
                         ReadabilityResources.SA1106CodeFix,
@@ -62,10 +52,6 @@ private static async Task<Document> GetTransformedDocumentAsync(Document documen
         {
             var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
             var token = root.FindToken(diagnostic.Location.SourceSpan.Start);
-            if (token.IsMissingOrDefault())
-            {
-                return document;
-            }
 
             if (!token.Parent.IsKind(SyntaxKind.EmptyStatement))
             {
@@ -106,20 +92,39 @@ private static async Task<Document> RemoveEmptyStatementAsync(Document document,
 
         private static async Task<Document> RemoveSemicolonTextAsync(Document document, SyntaxToken token, CancellationToken cancellationToken)
         {
+            TextChange textChange;
+
             SourceText sourceText = await document.GetTextAsync(cancellationToken).ConfigureAwait(false);
             TextLine line = sourceText.Lines.GetLineFromPosition(token.SpanStart);
             if (sourceText.ToString(line.Span).Trim() == token.Text)
             {
                 // remove the line containing the semicolon token
-                TextChange textChange = new TextChange(line.SpanIncludingLineBreak, string.Empty);
+                textChange = new TextChange(line.SpanIncludingLineBreak, string.Empty);
                 return document.WithText(sourceText.WithChanges(textChange));
             }
+
+            TextSpan spanToRemove;
+            var whitespaceIndex = TriviaHelper.IndexOfTrailingWhitespace(token.LeadingTrivia);
+            if (whitespaceIndex >= 0)
+            {
+                spanToRemove = TextSpan.FromBounds(token.LeadingTrivia[whitespaceIndex].Span.Start, token.Span.End);
+            }
             else
             {
-                // remove just the semicolon
-                TextChange textChange = new TextChange(token.Span, string.Empty);
-                return document.WithText(sourceText.WithChanges(textChange));
+                var previousToken = token.GetPreviousToken();
+                whitespaceIndex = TriviaHelper.IndexOfTrailingWhitespace(previousToken.TrailingTrivia);
+                if (whitespaceIndex >= 0)
+                {
+                    spanToRemove = TextSpan.FromBounds(previousToken.TrailingTrivia[whitespaceIndex].Span.Start, token.Span.End);
+                }
+                else
+                {
+                    spanToRemove = token.Span;
+                }
             }
+
+            textChange = new TextChange(spanToRemove, string.Empty);
+            return document.WithText(sourceText.WithChanges(textChange));
         }
     }
-}
+}