Merge pull request #2502 from vweijsters/improve-SA1009-codefix

Fixed issues in SA1009 code fix

Author: sharwell

StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/SpacingRules/TokenSpacingCodeFixProvider.cs

@@ -113,6 +113,7 @@ private static void UpdateReplaceMap(Dictionary<SyntaxToken, SyntaxToken> replac
             }
 
             SyntaxTriviaList triviaList;
+            SyntaxToken nextToken;
             switch (location)
             {
             case TokenSpacingProperties.LocationPreceding:
@@ -188,7 +189,18 @@ private static void UpdateReplaceMap(Dictionary<SyntaxToken, SyntaxToken> replac
                     else
                     {
                         SyntaxTriviaList trailingTrivia = triviaList.AddRange(token.TrailingTrivia.WithoutLeadingWhitespace(endOfLineIsWhitespace: false));
-                        replaceMap[token] = token.WithLeadingTrivia().WithTrailingTrivia(trailingTrivia);
+
+                        nextToken = token.GetNextToken();
+                        if (nextToken.IsKind(SyntaxKind.SemicolonToken))
+                        {
+                            // make the semicolon 'sticky'
+                            replaceMap[token] = token.WithLeadingTrivia().WithTrailingTrivia();
+                            replaceMap[nextToken] = nextToken.WithLeadingTrivia().WithTrailingTrivia(trailingTrivia.WithoutTrailingWhitespace());
+                        }
+                        else
+                        {
+                            replaceMap[token] = token.WithLeadingTrivia().WithTrailingTrivia(trailingTrivia);
+                        }
                     }
 
                     break;
@@ -219,7 +231,7 @@ private static void UpdateReplaceMap(Dictionary<SyntaxToken, SyntaxToken> replac
                 break;
 
             case TokenSpacingProperties.LocationFollowing:
-                var nextToken = token.GetNextToken();
+                nextToken = token.GetNextToken();
                 switch (action)
                 {
                 case TokenSpacingProperties.ActionInsert:

StyleCop.Analyzers/StyleCop.Analyzers.Test/SpacingRules/SA1009UnitTests.cs

@@ -532,8 +532,7 @@ public void TestMethod2()
         // This is a regression test for https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/1206
         TestMethod3(
             true,
-            (false || true)) // comment
-        ;
+            (false || true)); // comment
 
         // This is a regression test for https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/1206
         if (
@@ -923,6 +922,81 @@ public void TestMethod()
             await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
         }
 
+        [Fact]
+        [WorkItem(2473, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2473")]
+        public async Task TestCodefixBehaviorWithCommentAndSemiColonAsync()
+        {
+            var testCode = @"using System.Threading.Tasks;
+
+public class TestClass
+{
+    public async void TestMethod()
+    {
+        await Task.Delay(1000 // Comment
+            );
+    }
+}
+";
+
+            var fixedCode = @"using System.Threading.Tasks;
+
+public class TestClass
+{
+    public async void TestMethod()
+    {
+        await Task.Delay(1000); // Comment
+    }
+}
+";
+
+            DiagnosticResult[] expected =
+            {
+                this.CSharpDiagnostic().WithLocation(8, 13).WithArguments(" not", "preceded"),
+            };
+
+            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);
+        }
+
+        [Fact]
+        [WorkItem(2474, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2474")]
+        public async Task TestCodefixBehaviorWithMemberAccessAsync()
+        {
+            var testCode = @"using System.Threading.Tasks;
+
+public class TestClass
+{
+    public async void TestMethod()
+    {
+        await Task.Delay(1000
+            ).ConfigureAwait(false);
+    }
+}
+";
+
+            var fixedCode = @"using System.Threading.Tasks;
+
+public class TestClass
+{
+    public async void TestMethod()
+    {
+        await Task.Delay(1000)
+            .ConfigureAwait(false);
+    }
+}
+";
+
+            DiagnosticResult[] expected =
+            {
+                this.CSharpDiagnostic().WithLocation(8, 13).WithArguments(" not", "preceded"),
+            };
+
+            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/SpacingRules/SA1009ClosingParenthesisMustBeSpacedCorrectly.cs

@@ -81,6 +81,7 @@ private static void HandleCloseParenToken(SyntaxTreeAnalysisContext context, Syn
             bool lastInLine = token.IsLastInLine();
             bool precedesStickyCharacter;
             bool allowEndOfLine = false;
+            bool preserveLayout = false;
 
             bool suppressFollowingSpaceError = false;
 
@@ -141,8 +142,15 @@ private static void HandleCloseParenToken(SyntaxTreeAnalysisContext context, Syn
                 break;
 
             case SyntaxKind.DotToken:
+                // allow a space for this case, but only if the ')' character is the last on the line
+                allowEndOfLine = true;
+                precedesStickyCharacter = true;
+
+                preserveLayout = nextToken.Parent.IsKind(SyntaxKind.SimpleMemberAccessExpression);
+                break;
+
             case SyntaxKind.MinusGreaterThanToken:
-                // allow a space for these cases, but only if the ')' character is the last on the line
+                // allow a space for this case, but only if the ')' character is the last on the line
                 allowEndOfLine = true;
                 precedesStickyCharacter = true;
                 break;
@@ -198,9 +206,19 @@ private static void HandleCloseParenToken(SyntaxTreeAnalysisContext context, Syn
             if (precededBySpace)
             {
                 // Closing parenthesis should{ not} be {preceded} by a space.
-                var properties = token.IsFirstInLine()
-                    ? TokenSpacingProperties.RemovePreceding
-                    : TokenSpacingProperties.RemoveImmediatePreceding;
+                ImmutableDictionary<string, string> properties;
+
+                if (preserveLayout)
+                {
+                    properties = TokenSpacingProperties.RemovePrecedingPreserveLayout;
+                }
+                else
+                {
+                    properties = token.IsFirstInLine()
+                        ? TokenSpacingProperties.RemovePreceding
+                        : TokenSpacingProperties.RemoveImmediatePreceding;
+                }
+
                 context.ReportDiagnostic(Diagnostic.Create(Descriptor, token.GetLocation(), properties, " not", "preceded"));
             }