Issue DotNetAnalyzers#2801: SA1500 fires for the while clause of do/w…

…hile statement

Provide new configuration setting, `layoutRules.allowDoWhileOnClosingBrace` such that when enabled, the following will be allowed:
do
{
    Console.WriteLine("test");
} while (false);

StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/LayoutRules/SA1500CodeFixProvider.cs

@@ -57,13 +57,13 @@ private static async Task<Document> GetTransformedDocumentAsync(Document documen
 
             var settings = SettingsHelper.GetStyleCopSettings(document.Project.AnalyzerOptions, cancellationToken);
             var braceToken = syntaxRoot.FindToken(diagnostic.Location.SourceSpan.Start);
-            var tokenReplacements = GenerateBraceFixes(settings.Indentation, ImmutableArray.Create(braceToken));
+            var tokenReplacements = GenerateBraceFixes(settings, ImmutableArray.Create(braceToken));
 
             var newSyntaxRoot = syntaxRoot.ReplaceTokens(tokenReplacements.Keys, (originalToken, rewrittenToken) => tokenReplacements[originalToken]);
             return document.WithSyntaxRoot(newSyntaxRoot);
         }
 
-        private static Dictionary<SyntaxToken, SyntaxToken> GenerateBraceFixes(IndentationSettings indentationSettings, ImmutableArray<SyntaxToken> braceTokens)
+        private static Dictionary<SyntaxToken, SyntaxToken> GenerateBraceFixes(StyleCopSettings settings, ImmutableArray<SyntaxToken> braceTokens)
         {
             var tokenReplacements = new Dictionary<SyntaxToken, SyntaxToken>();
 
@@ -72,7 +72,7 @@ private static async Task<Document> GetTransformedDocumentAsync(Document documen
                 var braceLine = LocationHelpers.GetLineSpan(braceToken).StartLinePosition.Line;
                 var braceReplacementToken = braceToken;
 
-                var indentationSteps = DetermineIndentationSteps(indentationSettings, braceToken);
+                var indentationSteps = DetermineIndentationSteps(settings.Indentation, braceToken);
 
                 var previousToken = braceToken.GetPreviousToken();
 
@@ -102,19 +102,23 @@ private static async Task<Document> GetTransformedDocumentAsync(Document documen
                             AddReplacement(tokenReplacements, previousToken, previousToken.WithTrailingTrivia(previousTokenNewTrailingTrivia));
                         }
 
-                        braceReplacementToken = braceReplacementToken.WithLeadingTrivia(IndentationHelper.GenerateWhitespaceTrivia(indentationSettings, indentationSteps));
+                        braceReplacementToken = braceReplacementToken.WithLeadingTrivia(IndentationHelper.GenerateWhitespaceTrivia(settings.Indentation, indentationSteps));
                     }
 
                     // Check if we need to apply a fix after the brace. No fix is needed when:
                     // - The closing brace is followed by a semi-colon or closing paren
                     // - The closing brace is the last token in the file
+                    // - The closing brace is followed by the while expression of a do/while loop and the
+                    //   allowDoWhileOnClosingBrace setting is enabled.
                     var nextToken = braceToken.GetNextToken();
                     var nextTokenLine = nextToken.IsKind(SyntaxKind.None) ? -1 : LocationHelpers.GetLineSpan(nextToken).StartLinePosition.Line;
                     var isMultiDimensionArrayInitializer = braceToken.IsKind(SyntaxKind.OpenBraceToken) && braceToken.Parent.IsKind(SyntaxKind.ArrayInitializerExpression) && braceToken.Parent.Parent.IsKind(SyntaxKind.ArrayInitializerExpression);
+                    var allowDoWhileOnClosingBrace = settings.LayoutRules.AllowDoWhileOnClosingBrace && nextToken.IsKind(SyntaxKind.WhileKeyword) && (braceToken.Parent?.IsKind(SyntaxKind.Block) ?? false) && (braceToken.Parent.Parent?.IsKind(SyntaxKind.DoStatement) ?? false);
 
                     if ((nextTokenLine == braceLine) &&
                         (!braceToken.IsKind(SyntaxKind.CloseBraceToken) || !IsValidFollowingToken(nextToken)) &&
-                        !isMultiDimensionArrayInitializer)
+                        !isMultiDimensionArrayInitializer &&
+                        !allowDoWhileOnClosingBrace)
                     {
                         var sharedTrivia = nextToken.LeadingTrivia.WithoutTrailingWhitespace();
                         var newTrailingTrivia = braceReplacementToken.TrailingTrivia
@@ -135,7 +139,7 @@ private static async Task<Document> GetTransformedDocumentAsync(Document documen
                                 newIndentationSteps = Math.Max(0, newIndentationSteps - 1);
                             }
 
-                            AddReplacement(tokenReplacements, nextToken, nextToken.WithLeadingTrivia(IndentationHelper.GenerateWhitespaceTrivia(indentationSettings, newIndentationSteps)));
+                            AddReplacement(tokenReplacements, nextToken, nextToken.WithLeadingTrivia(IndentationHelper.GenerateWhitespaceTrivia(settings.Indentation, newIndentationSteps)));
                         }
 
                         braceReplacementToken = braceReplacementToken.WithTrailingTrivia(newTrailingTrivia);
@@ -284,7 +288,7 @@ protected override async Task<SyntaxNode> FixAllInDocumentAsync(FixAllContext fi
 
                 var settings = SettingsHelper.GetStyleCopSettings(document.Project.AnalyzerOptions, fixAllContext.CancellationToken);
 
-                var tokenReplacements = GenerateBraceFixes(settings.Indentation, tokens);
+                var tokenReplacements = GenerateBraceFixes(settings, tokens);
 
                 return syntaxRoot.ReplaceTokens(tokenReplacements.Keys, (originalToken, rewrittenToken) => tokenReplacements[originalToken]);
             }

StyleCop.Analyzers/StyleCop.Analyzers.Test/LayoutRules/SA1500/SA1500UnitTests.DoWhiles.cs

@@ -305,5 +305,154 @@ private void Bar()
 
             await VerifyCSharpFixAsync(testCode, expectedDiagnostics, fixedTestCode, CancellationToken.None).ConfigureAwait(false);
         }
+
+        /// <summary>
+        /// Verifies that no diagnostics are reported for the do/while loop when the <see langword="while"/>
+        /// expression is on the same line as the closing brace and the setting is enabled.
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Fact]
+        public async Task TestDoWhileOnClosingBraceWithAllowSettingAsync()
+        {
+            var testSettings = @"
+{
+    ""settings"": {
+        ""layoutRules"": {
+            ""allowDoWhileOnClosingBrace"": true
+        }
+    }
+}";
+
+            var testCode = @"public class Foo
+{
+    private void Bar()
+    {
+        var x = 0;
+
+        do
+        {
+            x = 1;
+        } while (x == 0);
+    }
+}";
+
+            var test = new CSharpTest
+            {
+                TestCode = testCode,
+                Settings = testSettings,
+            };
+
+            await test.RunAsync(CancellationToken.None).ConfigureAwait(false);
+        }
+
+        /// <summary>
+        /// Verifies that diagnostics will be reported for the invalid <see langword="while"/> loop that
+        /// is on the same line as the closing brace which is not part of a <c>do/while</c> loop. This
+        /// ensures that the <c>allowDoWhileOnClosingBrace</c> setting is only applicable to a <c>do/while</c>
+        /// loop and will not mistakenly allow a plain <see langword="while"/> loop after any arbitrary
+        /// closing brace.
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Fact]
+        public async Task TestJustWhileLoopOnClosingBraceWithAllowDoWhileOnClosingBraceSettingAsync()
+        {
+            var testSettings = @"
+{
+    ""settings"": {
+        ""layoutRules"": {
+            ""allowDoWhileOnClosingBrace"": true
+        }
+    }
+}";
+
+            var testCode = @"public class Foo
+{
+    private void Bar()
+    {
+        var x = 0;
+
+        while (x == 0)
+        {
+            x = 1;
+        } while (x == 0)
+        {
+            x = 1;
+        }
+    }
+}";
+
+            var test = new CSharpTest
+            {
+                TestCode = testCode,
+                ExpectedDiagnostics =
+                {
+                    Diagnostic().WithLocation(10, 9),
+                },
+                Settings = testSettings,
+            };
+
+            await test.RunAsync(CancellationToken.None).ConfigureAwait(false);
+        }
+
+        /// <summary>
+        /// Verifies that no diagnostics are reported for the do/while loop when the <see langword="while"/>
+        /// expression is on the same line as the closing brace and the setting is allowed.
+        /// </summary>
+        /// <remarks>
+        /// <para>The "Invalid do ... while #6" code in the <see cref="TestDoWhileInvalidAsync"/> unit test
+        /// should account for the proper fix when the <c>allowDoWhileOnClosingBrace</c> is <see langword="false"/>,
+        /// which is the default.</para>
+        /// </remarks>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Fact]
+        public async Task TestFixDoWhileOnClosingBraceWithAllowSettingAsync()
+        {
+            var testSettings = @"
+{
+    ""settings"": {
+        ""layoutRules"": {
+            ""allowDoWhileOnClosingBrace"": true
+        }
+    }
+}";
+
+            var testCode = @"public class Foo
+{
+    private void Bar()
+    {
+        var x = 0;
+
+        do
+        {
+            x = 1; } while (x == 0);
+    }
+}";
+
+            var fixedTestCode = @"public class Foo
+{
+    private void Bar()
+    {
+        var x = 0;
+
+        do
+        {
+            x = 1;
+        } while (x == 0);
+    }
+}";
+
+            var test = new CSharpTest
+            {
+                TestCode = testCode,
+                ExpectedDiagnostics =
+                {
+                    Diagnostic().WithLocation(9, 20),
+                },
+                FixedCode = fixedTestCode,
+                Settings = testSettings,
+            };
+
+            await test.RunAsync(CancellationToken.None).ConfigureAwait(false);
+        }
     }
 }

StyleCop.Analyzers/StyleCop.Analyzers.Test/Settings/SettingsUnitTests.cs

@@ -37,6 +37,8 @@ public void VerifySettingsDefaults()
 
             Assert.NotNull(styleCopSettings.LayoutRules);
             Assert.Equal(OptionSetting.Allow, styleCopSettings.LayoutRules.NewlineAtEndOfFile);
+            Assert.True(styleCopSettings.LayoutRules.AllowConsecutiveUsings);
+            Assert.False(styleCopSettings.LayoutRules.AllowDoWhileOnClosingBrace);
 
             Assert.NotNull(styleCopSettings.SpacingRules);
             Assert.NotNull(styleCopSettings.ReadabilityRules);

StyleCop.Analyzers/StyleCop.Analyzers/LayoutRules/SA1500BracesForMultiLineStatementsMustNotShareLine.cs

@@ -230,7 +230,7 @@ private static void CheckBraces(SyntaxNodeAnalysisContext context, SyntaxToken o
             CheckBraceToken(context, openBraceToken);
             if (checkCloseBrace)
             {
-                CheckBraceToken(context, closeBraceToken);
+                CheckBraceToken(context, closeBraceToken, openBraceToken);
             }
         }
 
@@ -242,7 +242,7 @@ private static bool InitializerExpressionSharesLine(InitializerExpressionSyntax
             return (index > 0) && (parent.Expressions[index - 1].GetEndLine() == parent.Expressions[index].GetLine());
         }
 
-        private static void CheckBraceToken(SyntaxNodeAnalysisContext context, SyntaxToken token)
+        private static void CheckBraceToken(SyntaxNodeAnalysisContext context, SyntaxToken token, SyntaxToken openBraceToken = default)
         {
             if (token.IsMissing)
             {
@@ -279,6 +279,19 @@ private static void CheckBraceToken(SyntaxNodeAnalysisContext context, SyntaxTok
                     // last token of this file
                     return;
 
+                case SyntaxKind.WhileKeyword:
+                    if (context.Options.GetStyleCopSettings(context.CancellationToken).LayoutRules.AllowDoWhileOnClosingBrace)
+                    {
+                        var openBracePreviousToken = openBraceToken.GetPreviousToken(includeZeroWidth: true);
+
+                        if (!openBracePreviousToken.IsMissing && openBracePreviousToken.IsKind(SyntaxKind.DoKeyword))
+                        {
+                            return;
+                        }
+                    }
+
+                    break;
+
                 default:
                     break;
                 }

StyleCop.Analyzers/StyleCop.Analyzers/Settings/ObjectModel/LayoutSettings.cs

@@ -17,13 +17,19 @@ internal class LayoutSettings
         /// </summary>
         private readonly bool allowConsecutiveUsings;
 
+        /// <summary>
+        /// This is the backing field of the <see cref="AllowDoWhileOnClosingBrace"/> property.
+        /// </summary>
+        private readonly bool allowDoWhileOnClosingBrace;
+
         /// <summary>
         /// Initializes a new instance of the <see cref="LayoutSettings"/> class.
         /// </summary>
         protected internal LayoutSettings()
         {
             this.newlineAtEndOfFile = OptionSetting.Allow;
             this.allowConsecutiveUsings = true;
+            this.allowDoWhileOnClosingBrace = false;
         }
 
         /// <summary>
@@ -45,6 +51,10 @@ protected internal LayoutSettings(JsonObject layoutSettingsObject)
                     this.allowConsecutiveUsings = kvp.ToBooleanValue();
                     break;
 
+                case "allowDoWhileOnClosingBrace":
+                    this.allowDoWhileOnClosingBrace = kvp.ToBooleanValue();
+                    break;
+
                 default:
                     break;
                 }
@@ -56,5 +66,8 @@ protected internal LayoutSettings(JsonObject layoutSettingsObject)
 
         public bool AllowConsecutiveUsings =>
             this.allowConsecutiveUsings;
+
+        public bool AllowDoWhileOnClosingBrace =>
+            this.allowDoWhileOnClosingBrace;
     }
 }