diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/LayoutRules/SA1500CodeFixProvider.cs b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/LayoutRules/SA1500CodeFixProvider.cs index 32cccab6b..cc56a77bf 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/LayoutRules/SA1500CodeFixProvider.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/LayoutRules/SA1500CodeFixProvider.cs @@ -57,13 +57,13 @@ private static async Task GetTransformedDocumentAsync(Document documen var settings = SettingsHelper.GetStyleCopSettings(document.Project.AnalyzerOptions, syntaxRoot.SyntaxTree, 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 GenerateBraceFixes(IndentationSettings indentationSettings, ImmutableArray braceTokens) + private static Dictionary GenerateBraceFixes(StyleCopSettings settings, ImmutableArray braceTokens) { var tokenReplacements = new Dictionary(); @@ -72,7 +72,7 @@ private static Dictionary GenerateBraceFixes(Indentati 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 Dictionary GenerateBraceFixes(Indentati 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 Dictionary GenerateBraceFixes(Indentati 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 FixAllInDocumentAsync(FixAllContext fi var settings = SettingsHelper.GetStyleCopSettings(document.Project.AnalyzerOptions, syntaxRoot.SyntaxTree, fixAllContext.CancellationToken); - var tokenReplacements = GenerateBraceFixes(settings.Indentation, tokens); + var tokenReplacements = GenerateBraceFixes(settings, tokens); return syntaxRoot.ReplaceTokens(tokenReplacements.Keys, (originalToken, rewrittenToken) => tokenReplacements[originalToken]); } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/LayoutRules/SA1500/SA1500UnitTests.DoWhiles.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/LayoutRules/SA1500/SA1500UnitTests.DoWhiles.cs index 1420c4f3c..b0a617a91 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/LayoutRules/SA1500/SA1500UnitTests.DoWhiles.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/LayoutRules/SA1500/SA1500UnitTests.DoWhiles.cs @@ -304,5 +304,164 @@ private void Bar() await VerifyCSharpFixAsync(testCode, expectedDiagnostics, fixedTestCode, CancellationToken.None).ConfigureAwait(false); } + + /// + /// Verifies that no diagnostics are reported for the do/while loop when the + /// expression is on the same line as the closing brace and the setting is enabled. + /// + /// A representing the asynchronous unit test. + [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); + } + + /// + /// Verifies that diagnostics will be reported for the invalid loop that + /// is on the same line as the closing brace which is not part of a do/while loop. This + /// ensures that the allowDoWhileOnClosingBrace setting is only applicable to a do/while + /// loop and will not mistakenly allow a plain loop after any arbitrary + /// closing brace. + /// + /// A representing the asynchronous unit test. + [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 fixedCode = @"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, + FixedCode = fixedCode, + Settings = testSettings, + }; + + await test.RunAsync(CancellationToken.None).ConfigureAwait(false); + } + + /// + /// Verifies that no diagnostics are reported for the do/while loop when the + /// expression is on the same line as the closing brace and the setting is allowed. + /// + /// + /// The "Invalid do ... while #6" code in the unit test + /// should account for the proper fix when the allowDoWhileOnClosingBrace is , + /// which is the default. + /// + /// A representing the asynchronous unit test. + [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, + FixedCode = fixedTestCode, + Settings = testSettings, + }; + + await test.RunAsync(CancellationToken.None).ConfigureAwait(false); + } } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/Settings/SettingsUnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/Settings/SettingsUnitTests.cs index f6fd17baf..04d9fb2f0 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/Settings/SettingsUnitTests.cs +++ b/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); diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/LayoutRules/SA1500BracesForMultiLineStatementsMustNotShareLine.cs b/StyleCop.Analyzers/StyleCop.Analyzers/LayoutRules/SA1500BracesForMultiLineStatementsMustNotShareLine.cs index 9e849e50c..101612fcd 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/LayoutRules/SA1500BracesForMultiLineStatementsMustNotShareLine.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/LayoutRules/SA1500BracesForMultiLineStatementsMustNotShareLine.cs @@ -235,7 +235,7 @@ private static void CheckBraces(SyntaxNodeAnalysisContext context, SyntaxToken o CheckBraceToken(context, openBraceToken); if (checkCloseBrace) { - CheckBraceToken(context, closeBraceToken); + CheckBraceToken(context, closeBraceToken, openBraceToken); } } @@ -247,7 +247,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) { @@ -284,6 +284,21 @@ private static void CheckBraceToken(SyntaxNodeAnalysisContext context, SyntaxTok // last token of this file return; + case SyntaxKind.WhileKeyword: + // Because the default Visual Studio code completion snippet for a do-while loop + // places the while expression on the same line as the closing brace, some users + // may want to allow that and not have SA1500 report it as a style error. + if (context.GetStyleCopSettings(context.CancellationToken).LayoutRules.AllowDoWhileOnClosingBrace) + { + if (openBraceToken.Parent.IsKind(SyntaxKind.Block) + && openBraceToken.Parent.Parent.IsKind(SyntaxKind.DoStatement)) + { + return; + } + } + + break; + default: break; } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/Settings/ObjectModel/LayoutSettings.cs b/StyleCop.Analyzers/StyleCop.Analyzers/Settings/ObjectModel/LayoutSettings.cs index fcec1e45b..af534a7df 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/Settings/ObjectModel/LayoutSettings.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/Settings/ObjectModel/LayoutSettings.cs @@ -18,6 +18,11 @@ internal class LayoutSettings /// private readonly bool allowConsecutiveUsings; + /// + /// This is the backing field of the property. + /// + private readonly bool allowDoWhileOnClosingBrace; + /// /// Initializes a new instance of the class. /// @@ -25,6 +30,7 @@ protected internal LayoutSettings() { this.newlineAtEndOfFile = OptionSetting.Allow; this.allowConsecutiveUsings = true; + this.allowDoWhileOnClosingBrace = false; } /// @@ -37,6 +43,7 @@ protected internal LayoutSettings(JsonObject layoutSettingsObject, AnalyzerConfi { OptionSetting? newlineAtEndOfFile = null; bool? allowConsecutiveUsings = null; + bool? allowDoWhileOnClosingBrace = null; foreach (var kvp in layoutSettingsObject) { @@ -50,6 +57,10 @@ protected internal LayoutSettings(JsonObject layoutSettingsObject, AnalyzerConfi allowConsecutiveUsings = kvp.ToBooleanValue(); break; + case "allowDoWhileOnClosingBrace": + allowDoWhileOnClosingBrace = kvp.ToBooleanValue(); + break; + default: break; } @@ -63,9 +74,11 @@ protected internal LayoutSettings(JsonObject layoutSettingsObject, AnalyzerConfi }; allowConsecutiveUsings ??= AnalyzerConfigHelper.TryGetBooleanValue(analyzerConfigOptions, "stylecop.layout.allowConsecutiveUsings"); + allowDoWhileOnClosingBrace ??= AnalyzerConfigHelper.TryGetBooleanValue(analyzerConfigOptions, "stylecop.layout.allowDoWhileOnClosingBrace"); this.newlineAtEndOfFile = newlineAtEndOfFile.GetValueOrDefault(OptionSetting.Allow); this.allowConsecutiveUsings = allowConsecutiveUsings.GetValueOrDefault(true); + this.allowDoWhileOnClosingBrace = allowDoWhileOnClosingBrace.GetValueOrDefault(false); } public OptionSetting NewlineAtEndOfFile => @@ -73,5 +86,8 @@ protected internal LayoutSettings(JsonObject layoutSettingsObject, AnalyzerConfi public bool AllowConsecutiveUsings => this.allowConsecutiveUsings; + + public bool AllowDoWhileOnClosingBrace => + this.allowDoWhileOnClosingBrace; } } diff --git a/documentation/Configuration.md b/documentation/Configuration.md index 773923ad0..8f0ce75b0 100644 --- a/documentation/Configuration.md +++ b/documentation/Configuration.md @@ -420,6 +420,7 @@ The following properties are used to configure layout rules in StyleCop Analyzer | --- | --- | --- | --- | | `newlineAtEndOfFile` | `"allow"` | 1.0.0 | Specifies the handling for newline characters which appear at the end of a file | | `allowConsecutiveUsings` | `true` | 1.1.0 | Specifies if SA1519 will allow consecutive using statements without braces | +| `allowDoWhileOnClosingBrace` | `false` | >1.2.0 | Specifies if SA1500 will allow the `while` expression of a `do`/`while` loop to be on the same line as the closing brace, as is generated by the default code snippet of Visual Studio | ### Lines at End of File @@ -441,6 +442,13 @@ The `allowConsecutiveUsings` property specifies the behavior: This only allows omitting the braces for a using followed by another using statement. A using statement followed by any other type of statement will still require braces to used. +### Do-While Loop Placement + +The behavior of [SA1500](SA1500.md) can be customized regarding the manner in which the `while` expression of a `do`/`while` loop is allowed to be placed. The `allowDoWhileOnClosingBrace` property specified the behavior: + +* `true`: the `while` expression of a `do`/`while` loop may be placed on the same line as the closing brace or on a separate line +* `false`: the `while` expression of a `do`/`while` loop must be on a separate line from the closing brace + ## Documentation Rules This section describes the features of documentation rules which can be configured in **stylecop.json**. Each of the described properties are configured in the `documentationRules` object, which is shown in the following sample file. diff --git a/documentation/SA1500.md b/documentation/SA1500.md index eaa039f65..652728001 100644 --- a/documentation/SA1500.md +++ b/documentation/SA1500.md @@ -19,6 +19,8 @@ The opening or closing brace within a C# statement, element, or expression is not placed on its own line. +> :memo: The behavior of this rule can change based on the configuration of the `allowDoWhileOnClosingBrace` property in **stylecop.json**. See [Configuration.md](Configuration.md#Layout-Rules) for more information. + ## Rule description A violation of this rule occurs when the opening or closing brace within a statement, element, or expression is not placed on its own line. For example: