Skip to content

Commit 784cfff

Browse files
sharwellsharwell
committed
Merge pull request #1817 from vweijsters/implement-SA1133
Implemented SA1133 (incl. tests + codefix)
2 parents dc7e71a + ee1bb96 commit 784cfff

7 files changed

Lines changed: 302 additions & 3 deletions

File tree

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

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 Helpers;
12+
using Microsoft.CodeAnalysis;
13+
using Microsoft.CodeAnalysis.CodeActions;
14+
using Microsoft.CodeAnalysis.CodeFixes;
15+
using Microsoft.CodeAnalysis.CSharp;
16+
using Microsoft.CodeAnalysis.CSharp.Syntax;
17+
18+
/// <summary>
19+
/// Implements a code fix for <see cref="SA1133DoNotCombineAttributes"/>.
20+
/// </summary>
21+
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(SA1133CodeFixProvider))]
22+
[Shared]
23+
internal class SA1133CodeFixProvider : CodeFixProvider
24+
{
25+
/// <inheritdoc/>
26+
public override ImmutableArray<string> FixableDiagnosticIds { get; } =
27+
ImmutableArray.Create(SA1133DoNotCombineAttributes.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.SA1133CodeFix,
43+
cancellationToken => GetTransformedDocumentAsync(context.Document, diagnostic, cancellationToken),
44+
nameof(SA1133CodeFixProvider)),
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 violatingAttribute = (AttributeSyntax)syntaxRoot.FindNode(diagnostic.Location.SourceSpan).Parent;
55+
var attributeList = (AttributeListSyntax)violatingAttribute.Parent;
56+
var newAttributeLists = new List<AttributeListSyntax>();
57+
58+
var indentationOptions = IndentationOptions.FromDocument(document);
59+
var indentationSteps = IndentationHelper.GetIndentationSteps(indentationOptions, attributeList);
60+
var indentationTrivia = IndentationHelper.GenerateWhitespaceTrivia(indentationOptions, indentationSteps);
61+
62+
for (var i = 0; i < attributeList.Attributes.Count; i++)
63+
{
64+
var newAttributes = SyntaxFactory.SingletonSeparatedList(attributeList.Attributes[i]);
65+
var newAttributeList = SyntaxFactory.AttributeList(attributeList.Target, newAttributes);
66+
67+
newAttributeList = (i == 0)
68+
? newAttributeList.WithLeadingTrivia(attributeList.GetLeadingTrivia())
69+
: newAttributeList.WithLeadingTrivia(indentationTrivia);
70+
71+
newAttributeList = (i == (attributeList.Attributes.Count - 1))
72+
? newAttributeList.WithTrailingTrivia(attributeList.GetTrailingTrivia())
73+
: newAttributeList.WithTrailingTrivia(SyntaxFactory.CarriageReturnLineFeed);
74+
75+
newAttributeLists.Add(newAttributeList);
76+
}
77+
78+
var newSyntaxRoot = syntaxRoot.ReplaceNode(attributeList, newAttributeLists);
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
@@ -127,6 +127,7 @@
127127
<Compile Include="ReadabilityRules\SA1129CodeFixProvider.cs" />
128128
<Compile Include="ReadabilityRules\SA1131CodeFixProvider.cs" />
129129
<Compile Include="ReadabilityRules\SA1132CodeFixProvider.cs" />
130+
<Compile Include="ReadabilityRules\SA1133CodeFixProvider.cs" />
130131
<Compile Include="ReadabilityRules\SX1101CodeFixProvider.cs" />
131132
<Compile Include="Settings\SettingsFileCodeFixProvider.cs" />
132133
<Compile Include="SpacingRules\SA1003CodeFixProvider.cs" />

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

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
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+
/// This class contains unit tests for the <see cref="SA1133DoNotCombineAttributes"/> class.
17+
/// </summary>
18+
public class SA1133UnitTests : CodeFixVerifier
19+
{
20+
/// <summary>
21+
/// Verifies that a single attribute will not produce a diagnostic.
22+
/// </summary>
23+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
24+
[Fact]
25+
public async Task VerifyThatSingleAttributesDoNotProduceDiagnosticAsync()
26+
{
27+
var testCode = @"using System.ComponentModel;
28+
29+
[EditorBrowsable(EditorBrowsableState.Never)]
30+
public class TestClass
31+
{
32+
}
33+
";
34+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
35+
}
36+
37+
/// <summary>
38+
/// Verifies that a multiple attributes on the same line will not produce a diagnostic.
39+
/// </summary>
40+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
41+
[Fact]
42+
public async Task VerifyThatMultipleAttributesOnSameLineDoNotProduceDiagnosticAsync()
43+
{
44+
var testCode = @"using System.ComponentModel;
45+
46+
[EditorBrowsable(EditorBrowsableState.Never)][DesignOnly(true)]
47+
public class TestClass
48+
{
49+
}
50+
";
51+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
52+
}
53+
54+
/// <summary>
55+
/// Verifies that an attribute list will produce the required diagnostics.
56+
/// </summary>
57+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
58+
[Fact]
59+
public async Task VerifyThatAttributeListProducesDiagnosticAsync()
60+
{
61+
var testCode = @"using System.ComponentModel;
62+
63+
[EditorBrowsable(EditorBrowsableState.Never), DesignOnly(true)]
64+
public class TestClass
65+
{
66+
/// <summary>
67+
/// Test method.
68+
/// </summary>
69+
[EditorBrowsable(EditorBrowsableState.Never), DesignOnly(true), DisplayName(""Test"")] // test comment
70+
public void TestMethod()
71+
{
72+
}
73+
}
74+
";
75+
76+
var fixedTestCode = @"using System.ComponentModel;
77+
78+
[EditorBrowsable(EditorBrowsableState.Never)]
79+
[DesignOnly(true)]
80+
public class TestClass
81+
{
82+
/// <summary>
83+
/// Test method.
84+
/// </summary>
85+
[EditorBrowsable(EditorBrowsableState.Never)]
86+
[DesignOnly(true)]
87+
[DisplayName(""Test"")] // test comment
88+
public void TestMethod()
89+
{
90+
}
91+
}
92+
";
93+
94+
DiagnosticResult[] expected =
95+
{
96+
this.CSharpDiagnostic().WithLocation(3, 47),
97+
this.CSharpDiagnostic().WithLocation(9, 51)
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 multiple attribute list are handled correctly.
107+
/// </summary>
108+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
109+
[Fact]
110+
public async Task VerifyThatMultipleAttributeListsAreHandledCorrectlyAsync()
111+
{
112+
var testCode = @"using System.ComponentModel;
113+
114+
[EditorBrowsable(EditorBrowsableState.Never), DesignOnly(true)]
115+
#if false
116+
[DataObject(true), Browsable(true)]
117+
#else
118+
[DataObject(true), Browsable(false)]
119+
#endif
120+
public class TestClass
121+
{
122+
/// <summary>
123+
/// Test method.
124+
/// </summary>
125+
[EditorBrowsable(EditorBrowsableState.Never), DesignOnly(true), DisplayName(""Test"")] // test comment
126+
public void TestMethod()
127+
{
128+
}
129+
}
130+
";
131+
132+
var fixedTestCode = @"using System.ComponentModel;
133+
134+
[EditorBrowsable(EditorBrowsableState.Never)]
135+
[DesignOnly(true)]
136+
#if false
137+
[DataObject(true), Browsable(true)]
138+
#else
139+
[DataObject(true)]
140+
[Browsable(false)]
141+
#endif
142+
public class TestClass
143+
{
144+
/// <summary>
145+
/// Test method.
146+
/// </summary>
147+
[EditorBrowsable(EditorBrowsableState.Never)]
148+
[DesignOnly(true)]
149+
[DisplayName(""Test"")] // test comment
150+
public void TestMethod()
151+
{
152+
}
153+
}
154+
";
155+
156+
DiagnosticResult[] expected =
157+
{
158+
this.CSharpDiagnostic().WithLocation(3, 47),
159+
this.CSharpDiagnostic().WithLocation(7, 20),
160+
this.CSharpDiagnostic().WithLocation(14, 51)
161+
};
162+
163+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
164+
await this.VerifyCSharpDiagnosticAsync(fixedTestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
165+
await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false);
166+
}
167+
168+
/// <inheritdoc/>
169+
protected override CodeFixProvider GetCSharpCodeFixProvider()
170+
{
171+
return new SA1133CodeFixProvider();
172+
}
173+
174+
/// <inheritdoc/>
175+
protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
176+
{
177+
yield return new SA1133DoNotCombineAttributes();
178+
}
179+
}
180+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,7 @@
323323
<Compile Include="ReadabilityRules\SA1130UnitTests.cs" />
324324
<Compile Include="ReadabilityRules\SA1131UnitTests.cs" />
325325
<Compile Include="ReadabilityRules\SA1132UnitTests.cs" />
326+
<Compile Include="ReadabilityRules\SA1133UnitTests.cs" />
326327
<Compile Include="ReadabilityRules\SX1101UnitTests.cs" />
327328
<Compile Include="Settings\SettingsFileCodeFixProviderUnitTests.cs" />
328329
<Compile Include="Settings\SettingsUnitTests.cs" />

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

Lines changed: 10 additions & 1 deletion
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: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -459,11 +459,14 @@
459459
<data name="SA1132Title" xml:space="preserve">
460460
<value>Do not combine fields</value>
461461
</data>
462+
<data name="SA1133CodeFix" xml:space="preserve">
463+
<value>Give each attribute its own square brackets</value>
464+
</data>
462465
<data name="SA1133Description" xml:space="preserve">
463466
<value>Each attribute usage should be placed in its own set of square brackets for maximum readability.</value>
464467
</data>
465468
<data name="SA1133MessageFormat" xml:space="preserve">
466-
<value />
469+
<value>Each attribute should be placed in its own set of square brackets.</value>
467470
</data>
468471
<data name="SA1133Title" xml:space="preserve">
469472
<value>Do not combine attributes</value>

‎StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1133DoNotCombineAttributes.cs‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,11 @@
33

44
namespace StyleCop.Analyzers.ReadabilityRules
55
{
6+
using System;
67
using System.Collections.Immutable;
78
using Microsoft.CodeAnalysis;
9+
using Microsoft.CodeAnalysis.CSharp;
10+
using Microsoft.CodeAnalysis.CSharp.Syntax;
811
using Microsoft.CodeAnalysis.Diagnostics;
912

1013
/// <summary>
@@ -25,14 +28,32 @@ internal class SA1133DoNotCombineAttributes : DiagnosticAnalyzer
2528
private static readonly DiagnosticDescriptor Descriptor =
2629
new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, AnalyzerCategory.ReadabilityRules, DiagnosticSeverity.Warning, AnalyzerConstants.DisabledNoTests, Description, HelpLink);
2730

31+
private static readonly Action<CompilationStartAnalysisContext> CompilationStartAction = HandleCompilationStart;
32+
private static readonly Action<SyntaxNodeAnalysisContext> HandleAttributeListAction = HandleAttributeList;
33+
2834
/// <inheritdoc/>
2935
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } =
3036
ImmutableArray.Create(Descriptor);
3137

3238
/// <inheritdoc/>
3339
public override void Initialize(AnalysisContext context)
3440
{
35-
// TODO: Implement analysis
41+
context.RegisterCompilationStartAction(CompilationStartAction);
42+
}
43+
44+
private static void HandleCompilationStart(CompilationStartAnalysisContext context)
45+
{
46+
context.RegisterSyntaxNodeActionHonorExclusions(HandleAttributeListAction, SyntaxKind.AttributeList);
47+
}
48+
49+
private static void HandleAttributeList(SyntaxNodeAnalysisContext context)
50+
{
51+
AttributeListSyntax attributeList = (AttributeListSyntax)context.Node;
52+
53+
if (attributeList.Attributes.Count > 1)
54+
{
55+
context.ReportDiagnostic(Diagnostic.Create(Descriptor, attributeList.Attributes[1].Name.GetLocation()));
56+
}
3657
}
3758
}
3859
}

0 commit comments

Comments
 (0)