Skip to content

Commit 82c2d90

Browse files
sharwellsharwell
authored
Merge pull request #3196 from Kevin-Andrew/dowhile
Issue #2801: SA1500 fires for the while clause of do/while statement
2 parents e7af4b3 + c58307c commit 82c2d90

File tree

7 files changed

+215
-9
lines changed

7 files changed

+215
-9
lines changed

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

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,13 @@ private static async Task<Document> GetTransformedDocumentAsync(Document documen
5757

5858
var settings = SettingsHelper.GetStyleCopSettings(document.Project.AnalyzerOptions, syntaxRoot.SyntaxTree, cancellationToken);
5959
var braceToken = syntaxRoot.FindToken(diagnostic.Location.SourceSpan.Start);
60-
var tokenReplacements = GenerateBraceFixes(settings.Indentation, ImmutableArray.Create(braceToken));
60+
var tokenReplacements = GenerateBraceFixes(settings, ImmutableArray.Create(braceToken));
6161

6262
var newSyntaxRoot = syntaxRoot.ReplaceTokens(tokenReplacements.Keys, (originalToken, rewrittenToken) => tokenReplacements[originalToken]);
6363
return document.WithSyntaxRoot(newSyntaxRoot);
6464
}
6565

66-
private static Dictionary<SyntaxToken, SyntaxToken> GenerateBraceFixes(IndentationSettings indentationSettings, ImmutableArray<SyntaxToken> braceTokens)
66+
private static Dictionary<SyntaxToken, SyntaxToken> GenerateBraceFixes(StyleCopSettings settings, ImmutableArray<SyntaxToken> braceTokens)
6767
{
6868
var tokenReplacements = new Dictionary<SyntaxToken, SyntaxToken>();
6969

@@ -72,7 +72,7 @@ private static Dictionary<SyntaxToken, SyntaxToken> GenerateBraceFixes(Indentati
7272
var braceLine = LocationHelpers.GetLineSpan(braceToken).StartLinePosition.Line;
7373
var braceReplacementToken = braceToken;
7474

75-
var indentationSteps = DetermineIndentationSteps(indentationSettings, braceToken);
75+
var indentationSteps = DetermineIndentationSteps(settings.Indentation, braceToken);
7676

7777
var previousToken = braceToken.GetPreviousToken();
7878

@@ -102,19 +102,23 @@ private static Dictionary<SyntaxToken, SyntaxToken> GenerateBraceFixes(Indentati
102102
AddReplacement(tokenReplacements, previousToken, previousToken.WithTrailingTrivia(previousTokenNewTrailingTrivia));
103103
}
104104

105-
braceReplacementToken = braceReplacementToken.WithLeadingTrivia(IndentationHelper.GenerateWhitespaceTrivia(indentationSettings, indentationSteps));
105+
braceReplacementToken = braceReplacementToken.WithLeadingTrivia(IndentationHelper.GenerateWhitespaceTrivia(settings.Indentation, indentationSteps));
106106
}
107107

108108
// Check if we need to apply a fix after the brace. No fix is needed when:
109109
// - The closing brace is followed by a semi-colon or closing paren
110110
// - The closing brace is the last token in the file
111+
// - The closing brace is followed by the while expression of a do/while loop and the
112+
// allowDoWhileOnClosingBrace setting is enabled.
111113
var nextToken = braceToken.GetNextToken();
112114
var nextTokenLine = nextToken.IsKind(SyntaxKind.None) ? -1 : LocationHelpers.GetLineSpan(nextToken).StartLinePosition.Line;
113115
var isMultiDimensionArrayInitializer = braceToken.IsKind(SyntaxKind.OpenBraceToken) && braceToken.Parent.IsKind(SyntaxKind.ArrayInitializerExpression) && braceToken.Parent.Parent.IsKind(SyntaxKind.ArrayInitializerExpression);
116+
var allowDoWhileOnClosingBrace = settings.LayoutRules.AllowDoWhileOnClosingBrace && nextToken.IsKind(SyntaxKind.WhileKeyword) && (braceToken.Parent?.IsKind(SyntaxKind.Block) ?? false) && (braceToken.Parent.Parent?.IsKind(SyntaxKind.DoStatement) ?? false);
114117

115118
if ((nextTokenLine == braceLine) &&
116119
(!braceToken.IsKind(SyntaxKind.CloseBraceToken) || !IsValidFollowingToken(nextToken)) &&
117-
!isMultiDimensionArrayInitializer)
120+
!isMultiDimensionArrayInitializer &&
121+
!allowDoWhileOnClosingBrace)
118122
{
119123
var sharedTrivia = nextToken.LeadingTrivia.WithoutTrailingWhitespace();
120124
var newTrailingTrivia = braceReplacementToken.TrailingTrivia
@@ -135,7 +139,7 @@ private static Dictionary<SyntaxToken, SyntaxToken> GenerateBraceFixes(Indentati
135139
newIndentationSteps = Math.Max(0, newIndentationSteps - 1);
136140
}
137141

138-
AddReplacement(tokenReplacements, nextToken, nextToken.WithLeadingTrivia(IndentationHelper.GenerateWhitespaceTrivia(indentationSettings, newIndentationSteps)));
142+
AddReplacement(tokenReplacements, nextToken, nextToken.WithLeadingTrivia(IndentationHelper.GenerateWhitespaceTrivia(settings.Indentation, newIndentationSteps)));
139143
}
140144

141145
braceReplacementToken = braceReplacementToken.WithTrailingTrivia(newTrailingTrivia);
@@ -284,7 +288,7 @@ protected override async Task<SyntaxNode> FixAllInDocumentAsync(FixAllContext fi
284288

285289
var settings = SettingsHelper.GetStyleCopSettings(document.Project.AnalyzerOptions, syntaxRoot.SyntaxTree, fixAllContext.CancellationToken);
286290

287-
var tokenReplacements = GenerateBraceFixes(settings.Indentation, tokens);
291+
var tokenReplacements = GenerateBraceFixes(settings, tokens);
288292

289293
return syntaxRoot.ReplaceTokens(tokenReplacements.Keys, (originalToken, rewrittenToken) => tokenReplacements[originalToken]);
290294
}

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

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,5 +304,164 @@ private void Bar()
304304

305305
await VerifyCSharpFixAsync(testCode, expectedDiagnostics, fixedTestCode, CancellationToken.None).ConfigureAwait(false);
306306
}
307+
308+
/// <summary>
309+
/// Verifies that no diagnostics are reported for the do/while loop when the <see langword="while"/>
310+
/// expression is on the same line as the closing brace and the setting is enabled.
311+
/// </summary>
312+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
313+
[Fact]
314+
public async Task TestDoWhileOnClosingBraceWithAllowSettingAsync()
315+
{
316+
var testSettings = @"
317+
{
318+
""settings"": {
319+
""layoutRules"": {
320+
""allowDoWhileOnClosingBrace"": true
321+
}
322+
}
323+
}";
324+
325+
var testCode = @"public class Foo
326+
{
327+
private void Bar()
328+
{
329+
var x = 0;
330+
331+
do
332+
{
333+
x = 1;
334+
} while (x == 0);
335+
}
336+
}";
337+
338+
var test = new CSharpTest
339+
{
340+
TestCode = testCode,
341+
Settings = testSettings,
342+
};
343+
344+
await test.RunAsync(CancellationToken.None).ConfigureAwait(false);
345+
}
346+
347+
/// <summary>
348+
/// Verifies that diagnostics will be reported for the invalid <see langword="while"/> loop that
349+
/// is on the same line as the closing brace which is not part of a <c>do/while</c> loop. This
350+
/// ensures that the <c>allowDoWhileOnClosingBrace</c> setting is only applicable to a <c>do/while</c>
351+
/// loop and will not mistakenly allow a plain <see langword="while"/> loop after any arbitrary
352+
/// closing brace.
353+
/// </summary>
354+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
355+
[Fact]
356+
public async Task TestJustWhileLoopOnClosingBraceWithAllowDoWhileOnClosingBraceSettingAsync()
357+
{
358+
var testSettings = @"
359+
{
360+
""settings"": {
361+
""layoutRules"": {
362+
""allowDoWhileOnClosingBrace"": true
363+
}
364+
}
365+
}";
366+
367+
var testCode = @"public class Foo
368+
{
369+
private void Bar()
370+
{
371+
var x = 0;
372+
373+
while (x == 0)
374+
{
375+
x = 1;
376+
[|}|] while (x == 0)
377+
{
378+
x = 1;
379+
}
380+
}
381+
}";
382+
383+
var fixedCode = @"public class Foo
384+
{
385+
private void Bar()
386+
{
387+
var x = 0;
388+
389+
while (x == 0)
390+
{
391+
x = 1;
392+
}
393+
while (x == 0)
394+
{
395+
x = 1;
396+
}
397+
}
398+
}";
399+
400+
var test = new CSharpTest
401+
{
402+
TestCode = testCode,
403+
FixedCode = fixedCode,
404+
Settings = testSettings,
405+
};
406+
407+
await test.RunAsync(CancellationToken.None).ConfigureAwait(false);
408+
}
409+
410+
/// <summary>
411+
/// Verifies that no diagnostics are reported for the do/while loop when the <see langword="while"/>
412+
/// expression is on the same line as the closing brace and the setting is allowed.
413+
/// </summary>
414+
/// <remarks>
415+
/// <para>The "Invalid do ... while #6" code in the <see cref="TestDoWhileInvalidAsync"/> unit test
416+
/// should account for the proper fix when the <c>allowDoWhileOnClosingBrace</c> is <see langword="false"/>,
417+
/// which is the default.</para>
418+
/// </remarks>
419+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
420+
[Fact]
421+
public async Task TestFixDoWhileOnClosingBraceWithAllowSettingAsync()
422+
{
423+
var testSettings = @"
424+
{
425+
""settings"": {
426+
""layoutRules"": {
427+
""allowDoWhileOnClosingBrace"": true
428+
}
429+
}
430+
}";
431+
432+
var testCode = @"public class Foo
433+
{
434+
private void Bar()
435+
{
436+
var x = 0;
437+
438+
do
439+
{
440+
x = 1; [|}|] while (x == 0);
441+
}
442+
}";
443+
444+
var fixedTestCode = @"public class Foo
445+
{
446+
private void Bar()
447+
{
448+
var x = 0;
449+
450+
do
451+
{
452+
x = 1;
453+
} while (x == 0);
454+
}
455+
}";
456+
457+
var test = new CSharpTest
458+
{
459+
TestCode = testCode,
460+
FixedCode = fixedTestCode,
461+
Settings = testSettings,
462+
};
463+
464+
await test.RunAsync(CancellationToken.None).ConfigureAwait(false);
465+
}
307466
}
308467
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ public void VerifySettingsDefaults()
3737

3838
Assert.NotNull(styleCopSettings.LayoutRules);
3939
Assert.Equal(OptionSetting.Allow, styleCopSettings.LayoutRules.NewlineAtEndOfFile);
40+
Assert.True(styleCopSettings.LayoutRules.AllowConsecutiveUsings);
41+
Assert.False(styleCopSettings.LayoutRules.AllowDoWhileOnClosingBrace);
4042

4143
Assert.NotNull(styleCopSettings.SpacingRules);
4244
Assert.NotNull(styleCopSettings.ReadabilityRules);

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

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ private static void CheckBraces(SyntaxNodeAnalysisContext context, SyntaxToken o
235235
CheckBraceToken(context, openBraceToken);
236236
if (checkCloseBrace)
237237
{
238-
CheckBraceToken(context, closeBraceToken);
238+
CheckBraceToken(context, closeBraceToken, openBraceToken);
239239
}
240240
}
241241

@@ -247,7 +247,7 @@ private static bool InitializerExpressionSharesLine(InitializerExpressionSyntax
247247
return (index > 0) && (parent.Expressions[index - 1].GetEndLine() == parent.Expressions[index].GetLine());
248248
}
249249

250-
private static void CheckBraceToken(SyntaxNodeAnalysisContext context, SyntaxToken token)
250+
private static void CheckBraceToken(SyntaxNodeAnalysisContext context, SyntaxToken token, SyntaxToken openBraceToken = default)
251251
{
252252
if (token.IsMissing)
253253
{
@@ -284,6 +284,21 @@ private static void CheckBraceToken(SyntaxNodeAnalysisContext context, SyntaxTok
284284
// last token of this file
285285
return;
286286

287+
case SyntaxKind.WhileKeyword:
288+
// Because the default Visual Studio code completion snippet for a do-while loop
289+
// places the while expression on the same line as the closing brace, some users
290+
// may want to allow that and not have SA1500 report it as a style error.
291+
if (context.GetStyleCopSettings(context.CancellationToken).LayoutRules.AllowDoWhileOnClosingBrace)
292+
{
293+
if (openBraceToken.Parent.IsKind(SyntaxKind.Block)
294+
&& openBraceToken.Parent.Parent.IsKind(SyntaxKind.DoStatement))
295+
{
296+
return;
297+
}
298+
}
299+
300+
break;
301+
287302
default:
288303
break;
289304
}

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,19 @@ internal class LayoutSettings
1818
/// </summary>
1919
private readonly bool allowConsecutiveUsings;
2020

21+
/// <summary>
22+
/// This is the backing field of the <see cref="AllowDoWhileOnClosingBrace"/> property.
23+
/// </summary>
24+
private readonly bool allowDoWhileOnClosingBrace;
25+
2126
/// <summary>
2227
/// Initializes a new instance of the <see cref="LayoutSettings"/> class.
2328
/// </summary>
2429
protected internal LayoutSettings()
2530
{
2631
this.newlineAtEndOfFile = OptionSetting.Allow;
2732
this.allowConsecutiveUsings = true;
33+
this.allowDoWhileOnClosingBrace = false;
2834
}
2935

3036
/// <summary>
@@ -37,6 +43,7 @@ protected internal LayoutSettings(JsonObject layoutSettingsObject, AnalyzerConfi
3743
{
3844
OptionSetting? newlineAtEndOfFile = null;
3945
bool? allowConsecutiveUsings = null;
46+
bool? allowDoWhileOnClosingBrace = null;
4047

4148
foreach (var kvp in layoutSettingsObject)
4249
{
@@ -50,6 +57,10 @@ protected internal LayoutSettings(JsonObject layoutSettingsObject, AnalyzerConfi
5057
allowConsecutiveUsings = kvp.ToBooleanValue();
5158
break;
5259

60+
case "allowDoWhileOnClosingBrace":
61+
allowDoWhileOnClosingBrace = kvp.ToBooleanValue();
62+
break;
63+
5364
default:
5465
break;
5566
}
@@ -63,15 +74,20 @@ protected internal LayoutSettings(JsonObject layoutSettingsObject, AnalyzerConfi
6374
};
6475

6576
allowConsecutiveUsings ??= AnalyzerConfigHelper.TryGetBooleanValue(analyzerConfigOptions, "stylecop.layout.allowConsecutiveUsings");
77+
allowDoWhileOnClosingBrace ??= AnalyzerConfigHelper.TryGetBooleanValue(analyzerConfigOptions, "stylecop.layout.allowDoWhileOnClosingBrace");
6678

6779
this.newlineAtEndOfFile = newlineAtEndOfFile.GetValueOrDefault(OptionSetting.Allow);
6880
this.allowConsecutiveUsings = allowConsecutiveUsings.GetValueOrDefault(true);
81+
this.allowDoWhileOnClosingBrace = allowDoWhileOnClosingBrace.GetValueOrDefault(false);
6982
}
7083

7184
public OptionSetting NewlineAtEndOfFile =>
7285
this.newlineAtEndOfFile;
7386

7487
public bool AllowConsecutiveUsings =>
7588
this.allowConsecutiveUsings;
89+
90+
public bool AllowDoWhileOnClosingBrace =>
91+
this.allowDoWhileOnClosingBrace;
7692
}
7793
}

‎documentation/Configuration.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,7 @@ The following properties are used to configure layout rules in StyleCop Analyzer
420420
| --- | --- | --- | --- |
421421
| `newlineAtEndOfFile` | `"allow"` | 1.0.0 | Specifies the handling for newline characters which appear at the end of a file |
422422
| `allowConsecutiveUsings` | `true` | 1.1.0 | Specifies if SA1519 will allow consecutive using statements without braces |
423+
| `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 |
423424

424425
### Lines at End of File
425426

@@ -441,6 +442,13 @@ The `allowConsecutiveUsings` property specifies the behavior:
441442
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
442443
require braces to used.
443444

445+
### Do-While Loop Placement
446+
447+
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:
448+
449+
* `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
450+
* `false`: the `while` expression of a `do`/`while` loop must be on a separate line from the closing brace
451+
444452
## Documentation Rules
445453

446454
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.

‎documentation/SA1500.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
The opening or closing brace within a C# statement, element, or expression is not placed on its own line.
2121

22+
> :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.
23+
2224
## Rule description
2325

2426
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:

0 commit comments

Comments
 (0)