Skip to content

Commit ffdceb7

Browse files
committed
Implemented SA1136 (incl. tests and code fix)
1 parent cb11b4d commit ffdceb7

10 files changed

Lines changed: 427 additions & 0 deletions

File tree

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
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 indentationOptions = IndentationOptions.FromDocument(document);
55+
56+
var enumMemberDeclaration = (EnumMemberDeclarationSyntax)syntaxRoot.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);
57+
var enumMemberDeclarationFirstToken = enumMemberDeclaration.GetFirstToken();
58+
59+
var enumDeclaration = (EnumDeclarationSyntax)enumMemberDeclaration.Parent;
60+
61+
var memberIndex = enumDeclaration.Members.IndexOf(enumMemberDeclaration);
62+
var precedingSeparatorToken = enumDeclaration.Members.GetSeparator(memberIndex - 1);
63+
64+
var parentIndentationSteps = IndentationHelper.GetIndentationSteps(indentationOptions, enumDeclaration);
65+
var indentation = IndentationHelper.GenerateWhitespaceTrivia(indentationOptions, parentIndentationSteps + 1);
66+
67+
var replacements = new Dictionary<SyntaxToken, SyntaxToken>();
68+
69+
var sharedTrivia = TriviaHelper.MergeTriviaLists(precedingSeparatorToken.TrailingTrivia, enumMemberDeclarationFirstToken.LeadingTrivia);
70+
71+
var newTrailingTrivia = SyntaxFactory.TriviaList(sharedTrivia)
72+
.WithoutTrailingWhitespace()
73+
.Add(SyntaxFactory.CarriageReturnLineFeed);
74+
75+
replacements[precedingSeparatorToken] = precedingSeparatorToken.WithTrailingTrivia(newTrailingTrivia);
76+
replacements[enumMemberDeclarationFirstToken] = enumMemberDeclarationFirstToken.WithLeadingTrivia(indentation);
77+
78+
var newSyntaxRoot = syntaxRoot.ReplaceTokens(replacements.Keys, (original, rewritten) => replacements[original]);
79+
var newDocument = document.WithSyntaxRoot(newSyntaxRoot.WithoutFormatting());
80+
81+
return newDocument;
82+
}
83+
}
84+
}

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

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>
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
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;
7+
using System.Collections.Immutable;
8+
using Microsoft.CodeAnalysis;
9+
using Microsoft.CodeAnalysis.CSharp;
10+
using Microsoft.CodeAnalysis.CSharp.Syntax;
11+
using Microsoft.CodeAnalysis.Diagnostics;
12+
using StyleCop.Analyzers.Helpers;
13+
14+
/// <summary>
15+
/// Enum values should be placed on their own lines for maximum readability.
16+
/// </summary>
17+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
18+
internal class SA1136EnumValuesShouldBeOnSeparateLines : DiagnosticAnalyzer
19+
{
20+
/// <summary>
21+
/// The ID for diagnostics produced by the <see cref="SA1136EnumValuesShouldBeOnSeparateLines"/> analyzer.
22+
/// </summary>
23+
public const string DiagnosticId = "SA1136";
24+
private static readonly LocalizableString Title = new LocalizableResourceString(nameof(ReadabilityResources.SA1136Title), ReadabilityResources.ResourceManager, typeof(ReadabilityResources));
25+
private static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(ReadabilityResources.SA1136MessageFormat), ReadabilityResources.ResourceManager, typeof(ReadabilityResources));
26+
private static readonly LocalizableString Description = new LocalizableResourceString(nameof(ReadabilityResources.SA1136Description), ReadabilityResources.ResourceManager, typeof(ReadabilityResources));
27+
private static readonly string HelpLink = "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1136.md";
28+
29+
private static readonly DiagnosticDescriptor Descriptor =
30+
new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, AnalyzerCategory.ReadabilityRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink);
31+
32+
private static readonly Action<CompilationStartAnalysisContext> CompilationStartAction = HandleCompilationStart;
33+
private static readonly Action<SyntaxNodeAnalysisContext> HandleEnumDeclarationAction = HandleEnumDeclaration;
34+
35+
/// <inheritdoc/>
36+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } =
37+
ImmutableArray.Create(Descriptor);
38+
39+
/// <inheritdoc/>
40+
public override void Initialize(AnalysisContext context)
41+
{
42+
context.RegisterCompilationStartAction(CompilationStartAction);
43+
}
44+
45+
private static void HandleCompilationStart(CompilationStartAnalysisContext context)
46+
{
47+
context.RegisterSyntaxNodeActionHonorExclusions(HandleEnumDeclarationAction, SyntaxKind.EnumDeclaration);
48+
}
49+
50+
private static void HandleEnumDeclaration(SyntaxNodeAnalysisContext context)
51+
{
52+
var enumDeclaration = (EnumDeclarationSyntax)context.Node;
53+
54+
if (enumDeclaration.Members.Count < 2)
55+
{
56+
return;
57+
}
58+
59+
var previousLine = enumDeclaration.Members[0].GetLineSpan().EndLinePosition.Line;
60+
for (var i = 1; i < enumDeclaration.Members.Count; i++)
61+
{
62+
var currentMember = enumDeclaration.Members[i];
63+
var currentLine = currentMember.GetLineSpan().StartLinePosition.Line;
64+
65+
if (currentLine == previousLine)
66+
{
67+
context.ReportDiagnostic(Diagnostic.Create(Descriptor, currentMember.Identifier.GetLocation()));
68+
}
69+
70+
previousLine = currentLine;
71+
}
72+
}
73+
}
74+
}

0 commit comments

Comments
 (0)