Skip to content

Commit 3d6ed7f

Browse files
pdelvopdelvo
committed
Implement SA1131 diagnostic + code fix + fix all provider + tests
1 parent 9a9ef63 commit 3d6ed7f

7 files changed

Lines changed: 371 additions & 4 deletions

File tree

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

Lines changed: 238 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,238 @@
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;
7+
using System.Collections.Generic;
8+
using System.Threading;
9+
using System.Threading.Tasks;
10+
using Analyzers.ReadabilityRules;
11+
using Microsoft.CodeAnalysis.CodeFixes;
12+
using Microsoft.CodeAnalysis.Diagnostics;
13+
using TestHelper;
14+
using Xunit;
15+
16+
public class SA1131UnitTests : CodeFixVerifier
17+
{
18+
[Fact]
19+
public async Task TestYodaComparismAsync()
20+
{
21+
var testCode = @"
22+
using System;
23+
public class TypeName
24+
{
25+
public void Test()
26+
{
27+
int i = 5;
28+
const int j = 6;
29+
if (j == i) { }
30+
}
31+
}";
32+
var fixedCode = @"
33+
using System;
34+
public class TypeName
35+
{
36+
public void Test()
37+
{
38+
int i = 5;
39+
const int j = 6;
40+
if (i == j) { }
41+
}
42+
}";
43+
44+
var expected = new[]
45+
{
46+
this.CSharpDiagnostic().WithLocation(9, 13),
47+
};
48+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
49+
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
50+
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
51+
}
52+
53+
[Fact]
54+
public async Task TestYodaComparismAsAnArgumentAsync()
55+
{
56+
var testCode = @"
57+
using System;
58+
public class TypeName
59+
{
60+
public void Test()
61+
{
62+
int i = 5;
63+
const int j = 6;
64+
Test(j == i);
65+
}
66+
public void Test(bool arg) { }
67+
}";
68+
var fixedCode = @"
69+
using System;
70+
public class TypeName
71+
{
72+
public void Test()
73+
{
74+
int i = 5;
75+
const int j = 6;
76+
Test(i == j);
77+
}
78+
public void Test(bool arg) { }
79+
}";
80+
81+
var expected = new[]
82+
{
83+
this.CSharpDiagnostic().WithLocation(9, 14),
84+
};
85+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
86+
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
87+
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
88+
}
89+
90+
[Fact]
91+
public async Task TestYodaComparismOutsideIfAsync()
92+
{
93+
var testCode = @"
94+
using System;
95+
public class TypeName
96+
{
97+
public void Test()
98+
{
99+
int i = 5;
100+
const int j = 6;
101+
bool b = j == i;
102+
}
103+
}";
104+
var fixedCode = @"
105+
using System;
106+
public class TypeName
107+
{
108+
public void Test()
109+
{
110+
int i = 5;
111+
const int j = 6;
112+
bool b = i == j;
113+
}
114+
}";
115+
var expected = new[]
116+
{
117+
this.CSharpDiagnostic().WithLocation(9, 18),
118+
};
119+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
120+
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
121+
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
122+
}
123+
124+
[Fact]
125+
public async Task TestCorrectComparismAsync()
126+
{
127+
var testCode = @"
128+
using System;
129+
public class TypeName
130+
{
131+
public void Test()
132+
{
133+
const int i = 5;
134+
int j = 6;
135+
if (j == i) { }
136+
}
137+
}";
138+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
139+
}
140+
141+
[Fact]
142+
public async Task TestCorrectComparismOutsideIfAsync()
143+
{
144+
var testCode = @"
145+
using System;
146+
public class TypeName
147+
{
148+
public void Test()
149+
{
150+
const int i = 5;
151+
int j = 6;
152+
bool b = j == i;
153+
}
154+
}";
155+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
156+
}
157+
158+
[Fact]
159+
public async Task TestCorrectComparismNoConstantAsync()
160+
{
161+
var testCode = @"
162+
using System;
163+
public class TypeName
164+
{
165+
public void Test()
166+
{
167+
int i = 5;
168+
int j = 6;
169+
if (j == i) { }
170+
}
171+
}";
172+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
173+
}
174+
175+
[Fact]
176+
public async Task TestCorrectComparismOutsideIfNoConstantAsync()
177+
{
178+
var testCode = @"
179+
using System;
180+
public class TypeName
181+
{
182+
public void Test()
183+
{
184+
int i = 5;
185+
int j = 6;
186+
bool b = j == i;
187+
}
188+
}";
189+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
190+
}
191+
192+
[Fact]
193+
public async Task TestCommentsAsync()
194+
{
195+
var testCode = @"
196+
using System;
197+
public class TypeName
198+
{
199+
public void Test()
200+
{
201+
int i = 5;
202+
const int j = 6;
203+
bool b = /*1*/j/*2*/==/*3*/i/*4*/;
204+
}
205+
}";
206+
var fixedCode = @"
207+
using System;
208+
public class TypeName
209+
{
210+
public void Test()
211+
{
212+
int i = 5;
213+
const int j = 6;
214+
bool b = /*1*/i/*2*/==/*3*/j/*4*/;
215+
}
216+
}";
217+
var expected = new[]
218+
{
219+
this.CSharpDiagnostic().WithLocation(9, 23),
220+
};
221+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
222+
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
223+
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
224+
}
225+
226+
/// <inheritdoc/>
227+
protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
228+
{
229+
yield return new SA1131UseReadableConditions();
230+
}
231+
232+
/// <inheritdoc/>
233+
protected override CodeFixProvider GetCSharpCodeFixProvider()
234+
{
235+
return new SA1131CodeFixProvider();
236+
}
237+
}
238+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,7 @@
310310
<Compile Include="ReadabilityRules\SA1127UnitTests.cs" />
311311
<Compile Include="ReadabilityRules\SA1128UnitTests.cs" />
312312
<Compile Include="ReadabilityRules\SA1129UnitTests.cs" />
313+
<Compile Include="ReadabilityRules\SA1131UnitTests.cs" />
313314
<Compile Include="ReadabilityRules\SA1130UnitTests.cs" />
314315
<Compile Include="ReadabilityRules\SA1132UnitTests.cs" />
315316
<Compile Include="Settings\SettingsFileCodeFixProviderUnitTests.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
@@ -432,11 +432,14 @@
432432
<data name="SA1130Title" xml:space="preserve">
433433
<value>Use lambda syntax</value>
434434
</data>
435+
<data name="SA1131CodeFix" xml:space="preserve">
436+
<value>Switch arguments.</value>
437+
</data>
435438
<data name="SA1131Description" xml:space="preserve">
436439
<value>When a comparison is made between a variable and a literal, the variable should be placed on the left-hand-side to maximize readability.</value>
437440
</data>
438441
<data name="SA1131MessageFormat" xml:space="preserve">
439-
<value />
442+
<value>Constant values should appear on the right-hand side of comparisons</value>
440443
</data>
441444
<data name="SA1131Title" xml:space="preserve">
442445
<value>Use readable conditions</value>

‎StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1131CodeFixProvider.cs‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
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.Immutable;
7+
using System.Composition;
8+
using System.Linq;
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.Syntax;
15+
using StyleCop.Analyzers.Helpers;
16+
17+
/// <summary>
18+
/// Implements a code fix for <see cref="SA1131UseReadableConditions"/>.
19+
/// </summary>
20+
/// <remarks>
21+
/// <para>To fix a violation of this rule, switch the arguments of the comparism.</para>
22+
/// </remarks>
23+
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(SA1131CodeFixProvider))]
24+
[Shared]
25+
internal class SA1131CodeFixProvider : CodeFixProvider
26+
{
27+
/// <inheritdoc/>
28+
public override ImmutableArray<string> FixableDiagnosticIds { get; } =
29+
ImmutableArray.Create(SA1131UseReadableConditions.DiagnosticId);
30+
31+
/// <inheritdoc/>
32+
public override FixAllProvider GetFixAllProvider()
33+
{
34+
return FixAll.Instance;
35+
}
36+
37+
/// <inheritdoc/>
38+
public override Task RegisterCodeFixesAsync(CodeFixContext context)
39+
{
40+
foreach (var diagnostic in context.Diagnostics)
41+
{
42+
context.RegisterCodeFix(
43+
CodeAction.Create(
44+
ReadabilityResources.SA1131CodeFix,
45+
cancellationToken => GetTransformedDocumentAsync(context.Document, diagnostic, cancellationToken),
46+
equivalenceKey: nameof(SA1131CodeFixProvider)),
47+
diagnostic);
48+
}
49+
50+
return SpecializedTasks.CompletedTask;
51+
}
52+
53+
private static async Task<Document> GetTransformedDocumentAsync(Document document, Diagnostic diagnostic, CancellationToken cancellationToken)
54+
{
55+
var syntaxRoot = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
56+
57+
var binaryExpression = (BinaryExpressionSyntax)syntaxRoot.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);
58+
59+
var newBinaryExpression = TransformExpression(binaryExpression);
60+
61+
return document.WithSyntaxRoot(syntaxRoot.ReplaceNode(binaryExpression, newBinaryExpression));
62+
}
63+
64+
private static BinaryExpressionSyntax TransformExpression(BinaryExpressionSyntax binaryExpression)
65+
{
66+
var newLeft = binaryExpression.Right.WithTriviaFrom(binaryExpression.Left);
67+
var newRight = binaryExpression.Left.WithTriviaFrom(binaryExpression.Right);
68+
return binaryExpression.WithLeft(newLeft).WithRight(newRight);
69+
}
70+
71+
private class FixAll : DocumentBasedFixAllProvider
72+
{
73+
public static FixAllProvider Instance { get; } =
74+
new FixAll();
75+
76+
protected override string CodeActionTitle { get; } =
77+
ReadabilityResources.SA1131CodeFix;
78+
79+
protected override async Task<SyntaxNode> FixAllInDocumentAsync(FixAllContext fixAllContext, Document document)
80+
{
81+
var diagnostics = await fixAllContext.GetDocumentDiagnosticsAsync(document).ConfigureAwait(false);
82+
if (diagnostics.IsEmpty)
83+
{
84+
return null;
85+
}
86+
87+
var syntaxRoot = await document.GetSyntaxRootAsync(fixAllContext.CancellationToken).ConfigureAwait(false);
88+
89+
var nodes = diagnostics.Select(diagnostic => (BinaryExpressionSyntax)syntaxRoot.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true));
90+
91+
return syntaxRoot.ReplaceNodes(nodes, (originalNode, rewrittenNode) => TransformExpression(rewrittenNode));
92+
}
93+
}
94+
}
95+
}

0 commit comments

Comments
 (0)