Skip to content

Commit f044621

Browse files
sharwellsharwell
committed
Merge pull request #2006 from vweijsters/implement-1854
Implemented SA1136 (incl. tests and code fix)
2 parents 1c63d28 + bf3f4ef commit f044621

11 files changed

Lines changed: 449 additions & 0 deletions

File tree

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1136CodeFixProvider.cs‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved.
2+
// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information.
3+
4+
namespace StyleCop.Analyzers.ReadabilityRules
5+
{
6+
using System.Collections.Generic;
7+
using System.Collections.Immutable;
8+
using System.Composition;
9+
using System.Threading;
10+
using System.Threading.Tasks;
11+
using Microsoft.CodeAnalysis;
12+
using Microsoft.CodeAnalysis.CodeActions;
13+
using Microsoft.CodeAnalysis.CodeFixes;
14+
using Microsoft.CodeAnalysis.CSharp;
15+
using Microsoft.CodeAnalysis.CSharp.Syntax;
16+
using StyleCop.Analyzers.Helpers;
17+
18+
/// <summary>
19+
/// Implements a code fix for <see cref="SA1136EnumValuesShouldBeOnSeparateLines"/>.
20+
/// </summary>
21+
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(SA1136CodeFixProvider))]
22+
[Shared]
23+
internal class SA1136CodeFixProvider : CodeFixProvider
24+
{
25+
/// <inheritdoc/>
26+
public override ImmutableArray<string> FixableDiagnosticIds { get; } =
27+
ImmutableArray.Create(SA1136EnumValuesShouldBeOnSeparateLines.DiagnosticId);
28+
29+
/// <inheritdoc/>
30+
public override FixAllProvider GetFixAllProvider()
31+
{
32+
return CustomFixAllProviders.BatchFixer;
33+
}
34+
35+
/// <inheritdoc/>
36+
public override Task RegisterCodeFixesAsync(CodeFixContext context)
37+
{
38+
foreach (var diagnostic in context.Diagnostics)
39+
{
40+
context.RegisterCodeFix(
41+
CodeAction.Create(
42+
ReadabilityResources.SA1136CodeFix,
43+
cancellationToken => GetTransformedDocumentAsync(context.Document, diagnostic, cancellationToken),
44+
nameof(SA1136CodeFixProvider)),
45+
diagnostic);
46+
}
47+
48+
return SpecializedTasks.CompletedTask;
49+
}
50+
51+
private static async Task<Document> GetTransformedDocumentAsync(Document document, Diagnostic diagnostic, CancellationToken cancellationToken)
52+
{
53+
var syntaxRoot = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
54+
var settings = SettingsHelper.GetStyleCopSettings(document.Project.AnalyzerOptions, cancellationToken);
55+
56+
var enumMemberDeclaration = (EnumMemberDeclarationSyntax)syntaxRoot.FindNode(diagnostic.Location.SourceSpan);
57+
var enumDeclaration = (EnumDeclarationSyntax)enumMemberDeclaration.Parent;
58+
59+
var memberIndex = enumDeclaration.Members.IndexOf(enumMemberDeclaration);
60+
var precedingSeparatorToken = enumDeclaration.Members.GetSeparator(memberIndex - 1);
61+
62+
// determine the indentation for enum members (which is parent + 1 step)
63+
var parentIndentationSteps = IndentationHelper.GetIndentationSteps(settings.Indentation, enumDeclaration);
64+
var indentation = IndentationHelper.GenerateWhitespaceTrivia(settings.Indentation, parentIndentationSteps + 1);
65+
66+
// combine all trivia between the separator and the enum member and place them after the separator, followed by a new line.
67+
var enumMemberDeclarationFirstToken = enumMemberDeclaration.GetFirstToken();
68+
var sharedTrivia = TriviaHelper.MergeTriviaLists(precedingSeparatorToken.TrailingTrivia, enumMemberDeclarationFirstToken.LeadingTrivia);
69+
70+
var newTrailingTrivia = SyntaxFactory.TriviaList(sharedTrivia)
71+
.WithoutTrailingWhitespace()
72+
.Add(SyntaxFactory.CarriageReturnLineFeed);
73+
74+
// replace the trivia for the tokens
75+
var replacements = new Dictionary<SyntaxToken, SyntaxToken>
76+
{
77+
[precedingSeparatorToken] = precedingSeparatorToken.WithTrailingTrivia(newTrailingTrivia),
78+
[enumMemberDeclarationFirstToken] = enumMemberDeclarationFirstToken.WithLeadingTrivia(indentation),
79+
};
80+
81+
var newSyntaxRoot = syntaxRoot.ReplaceTokens(replacements.Keys, (original, rewritten) => replacements[original]);
82+
var newDocument = document.WithSyntaxRoot(newSyntaxRoot.WithoutFormatting());
83+
84+
return newDocument;
85+
}
86+
}
87+
}

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/StyleCop.Analyzers.CodeFixes.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@
129129
<Compile Include="ReadabilityRules\SA1132CodeFixProvider.cs" />
130130
<Compile Include="ReadabilityRules\SA1133CodeFixProvider.cs" />
131131
<Compile Include="ReadabilityRules\SA1134CodeFixProvider.cs" />
132+
<Compile Include="ReadabilityRules\SA1136CodeFixProvider.cs" />
132133
<Compile Include="ReadabilityRules\SX1101CodeFixProvider.cs" />
133134
<Compile Include="Settings\SettingsFileCodeFixProvider.cs" />
134135
<Compile Include="SpacingRules\SA1003CodeFixProvider.cs" />

‎StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1136UnitTests.cs‎

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved.
2+
// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information.
3+
4+
namespace StyleCop.Analyzers.Test.ReadabilityRules
5+
{
6+
using System.Collections.Generic;
7+
using System.Threading;
8+
using System.Threading.Tasks;
9+
using Microsoft.CodeAnalysis;
10+
using Microsoft.CodeAnalysis.CodeFixes;
11+
using Microsoft.CodeAnalysis.Diagnostics;
12+
using StyleCop.Analyzers.ReadabilityRules;
13+
using TestHelper;
14+
using Xunit;
15+
16+
/// <summary>
17+
/// Unit tests for the <see cref="SA1136EnumValuesShouldBeOnSeparateLines"/> analyzer.
18+
/// </summary>
19+
public class SA1136UnitTests : CodeFixVerifier
20+
{
21+
/// <summary>
22+
/// Verifies that an enum declaration with all values on the same line will return the expected diagnostics.
23+
/// This also verifies that an enum declaration with all values on separate lines will not produce diagnostics.
24+
/// </summary>
25+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
26+
[Fact]
27+
public async Task TestSingleLineEnumDeclarationAsync()
28+
{
29+
var testCode = @"
30+
public enum TestEnum
31+
{
32+
FirstValue, SecondValue, ThirdValue
33+
}
34+
";
35+
36+
var fixedTestCode = @"
37+
public enum TestEnum
38+
{
39+
FirstValue,
40+
SecondValue,
41+
ThirdValue
42+
}
43+
";
44+
45+
DiagnosticResult[] expected =
46+
{
47+
this.CSharpDiagnostic().WithLocation(4, 17),
48+
this.CSharpDiagnostic().WithLocation(4, 30),
49+
};
50+
51+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
52+
await this.VerifyCSharpDiagnosticAsync(fixedTestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
53+
await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false);
54+
}
55+
56+
/// <summary>
57+
/// Verifies that comments after a enum value declaration will not produce diagnostics.
58+
/// </summary>
59+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
60+
[Fact]
61+
public async Task TestEndOfLineCommentsAsync()
62+
{
63+
var testCode = @"
64+
public enum TestEnum
65+
{
66+
FirstValue, /* comment 1 */
67+
SecondValue, // comment 2
68+
}
69+
";
70+
71+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
72+
}
73+
74+
/// <summary>
75+
/// Verifies that inline comments between enum value declarations are handled properly.
76+
/// </summary>
77+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
78+
[Fact]
79+
public async Task TestInlineCommentsAsync()
80+
{
81+
var testCode = @"
82+
public enum TestEnum
83+
{
84+
FirstValue, /* comment */ SecondValue,
85+
}
86+
";
87+
88+
var fixedTestCode = @"
89+
public enum TestEnum
90+
{
91+
FirstValue, /* comment */
92+
SecondValue,
93+
}
94+
";
95+
96+
DiagnosticResult[] expected =
97+
{
98+
this.CSharpDiagnostic().WithLocation(4, 31),
99+
};
100+
101+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
102+
await this.VerifyCSharpDiagnosticAsync(fixedTestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
103+
await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false);
104+
}
105+
106+
/// <summary>
107+
/// Verifies that values on the same line within a directive trivia are handled correctly.
108+
/// </summary>
109+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
110+
[Fact]
111+
public async Task TestDirectiveTriviaAreHandledProperlyAsync()
112+
{
113+
var testCode = @"
114+
public enum TestEnum
115+
{
116+
FirstValue,
117+
#if !TEST
118+
SecondValue, ThirdValue,
119+
#endif
120+
FourthValue
121+
}
122+
";
123+
124+
var fixedTestCode = @"
125+
public enum TestEnum
126+
{
127+
FirstValue,
128+
#if !TEST
129+
SecondValue,
130+
ThirdValue,
131+
#endif
132+
FourthValue
133+
}
134+
";
135+
136+
DiagnosticResult[] expected =
137+
{
138+
this.CSharpDiagnostic().WithLocation(6, 18),
139+
};
140+
141+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
142+
await this.VerifyCSharpDiagnosticAsync(fixedTestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
143+
await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false);
144+
}
145+
146+
/// <summary>
147+
/// Verifies that an enum declaration without a block is handled correctly.
148+
/// </summary>
149+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
150+
[Fact]
151+
public async Task TestEnumWithoutBlockAsync()
152+
{
153+
var testCode = @"
154+
public enum TestEnum
155+
";
156+
157+
DiagnosticResult[] expected =
158+
{
159+
this.CSharpCompilerError("CS1513").WithMessage("} expected").WithLocation(2, 21),
160+
this.CSharpCompilerError("CS1514").WithMessage("{ expected").WithLocation(2, 21),
161+
};
162+
163+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
164+
}
165+
166+
/// <inheritdoc/>
167+
protected override CodeFixProvider GetCSharpCodeFixProvider()
168+
{
169+
return new SA1136CodeFixProvider();
170+
}
171+
172+
/// <inheritdoc/>
173+
protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
174+
{
175+
yield return new SA1136EnumValuesShouldBeOnSeparateLines();
176+
}
177+
}
178+
}

‎StyleCop.Analyzers/StyleCop.Analyzers.Test/StyleCop.Analyzers.Test.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,7 @@
329329
<Compile Include="ReadabilityRules\SA1132UnitTests.cs" />
330330
<Compile Include="ReadabilityRules\SA1133UnitTests.cs" />
331331
<Compile Include="ReadabilityRules\SA1134UnitTests.cs" />
332+
<Compile Include="ReadabilityRules\SA1136UnitTests.cs" />
332333
<Compile Include="ReadabilityRules\SX1101UnitTests.cs" />
333334
<Compile Include="Settings\SettingsFileCodeFixProviderUnitTests.cs" />
334335
<Compile Include="Settings\SettingsUnitTests.cs" />

‎StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/ReadabilityResources.Designer.cs‎

Lines changed: 36 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/ReadabilityResources.resx‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,18 @@
483483
<data name="SA1134Title" xml:space="preserve">
484484
<value>Attributes must not share line</value>
485485
</data>
486+
<data name="SA1136CodeFix" xml:space="preserve">
487+
<value>Place enum values own their own lines</value>
488+
</data>
489+
<data name="SA1136Description" xml:space="preserve">
490+
<value>Enum values should be placed on their own lines for maximum readability.</value>
491+
</data>
492+
<data name="SA1136MessageFormat" xml:space="preserve">
493+
<value>Enum values should be on separate lines</value>
494+
</data>
495+
<data name="SA1136Title" xml:space="preserve">
496+
<value>Enum values should be on separate lines</value>
497+
</data>
486498
<data name="SX1101CodeFix" xml:space="preserve">
487499
<value>Remove 'this.' prefix</value>
488500
</data>

0 commit comments

Comments
 (0)