Skip to content

Commit 77d6c72

Browse files
ScooletzScooletz
committed
RavenDB-25423 Boolean method negation callsites diagnoser and fixer
1 parent 8a81a8b commit 77d6c72

File tree

6 files changed

+310
-0
lines changed

6 files changed

+310
-0
lines changed

‎Raven.CodeAnalysis.Test/BooleanMethodNegationTests.cs‎

Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
using Microsoft.CodeAnalysis;
2+
using Microsoft.CodeAnalysis.CodeFixes;
3+
using Microsoft.CodeAnalysis.Diagnostics;
4+
using Microsoft.VisualStudio.TestTools.UnitTesting;
5+
using Raven.CodeAnalysis.BooleanMethodNegation;
6+
using TestHelper;
7+
8+
namespace Raven.CodeAnalysis.Test
9+
{
10+
[TestClass]
11+
public class BooleanMethodNegationTests : CodeFixVerifier
12+
{
13+
[TestMethod]
14+
public void ShouldReportDiagnosticOnNegatedBooleanMethod()
15+
{
16+
const string input = @"
17+
class C
18+
{
19+
private bool HasPermission()
20+
{
21+
return false;
22+
}
23+
24+
void M()
25+
{
26+
if (!HasPermission())
27+
{
28+
}
29+
}
30+
}";
31+
VerifyCSharpDiagnostic(input, new DiagnosticResult
32+
{
33+
Id = DiagnosticIds.BooleanMethodNegation,
34+
Message = "Negated boolean method 'HasPermission' conditions should be rewritten as HasPermission(...) == false",
35+
Severity = DiagnosticSeverity.Error,
36+
Locations = new[]
37+
{
38+
new DiagnosticResultLocation("Test0.cs", 11, 13)
39+
}
40+
});
41+
}
42+
43+
[TestMethod]
44+
public void ShouldReportDiagnosticOnNegatedBooleanMethodWithArguments()
45+
{
46+
const string input = @"
47+
class C
48+
{
49+
private bool IsValid(int number)
50+
{
51+
return number > 0;
52+
}
53+
54+
void M()
55+
{
56+
if (!IsValid(1))
57+
{
58+
}
59+
}
60+
}";
61+
VerifyCSharpDiagnostic(input, new DiagnosticResult
62+
{
63+
Id = DiagnosticIds.BooleanMethodNegation,
64+
Message = "Negated boolean method 'IsValid' conditions should be rewritten as IsValid(...) == false",
65+
Severity = DiagnosticSeverity.Error,
66+
Locations = new[]
67+
{
68+
new DiagnosticResultLocation("Test0.cs", 11, 13)
69+
}
70+
});
71+
}
72+
73+
[TestMethod]
74+
public void ShouldNotReportDiagnosticOnNonNegatedBooleanMethod()
75+
{
76+
const string input = @"
77+
class C
78+
{
79+
private bool HasPermission()
80+
{
81+
return false;
82+
}
83+
84+
void M()
85+
{
86+
if (HasPermission())
87+
{
88+
}
89+
}
90+
}";
91+
VerifyCSharpDiagnostic(input);
92+
}
93+
94+
[TestMethod]
95+
public void ShouldNotReportDiagnosticOnComparisonToFalse()
96+
{
97+
const string input = @"
98+
class C
99+
{
100+
private bool IsValid(int number)
101+
{
102+
return number > 0;
103+
}
104+
105+
void M()
106+
{
107+
if (IsValid(1) == false)
108+
{
109+
}
110+
}
111+
}";
112+
VerifyCSharpDiagnostic(input);
113+
}
114+
115+
[TestMethod]
116+
public void ShouldRewriteNegatedBooleanMethodCondition()
117+
{
118+
const string input = @"
119+
class C
120+
{
121+
private bool HasPermission()
122+
{
123+
return false;
124+
}
125+
126+
void M()
127+
{
128+
if (!HasPermission())
129+
{
130+
}
131+
}
132+
}";
133+
const string output = @"
134+
class C
135+
{
136+
private bool HasPermission()
137+
{
138+
return false;
139+
}
140+
141+
void M()
142+
{
143+
if (HasPermission() == false)
144+
{
145+
}
146+
}
147+
}";
148+
VerifyCSharpFix(input, output);
149+
}
150+
151+
[TestMethod]
152+
public void ShouldRewriteNegatedBooleanMethodConditionWithArguments()
153+
{
154+
const string input = @"
155+
class C
156+
{
157+
private bool IsValid(int number)
158+
{
159+
return number > 0;
160+
}
161+
162+
void M()
163+
{
164+
if (!IsValid(1))
165+
{
166+
}
167+
}
168+
}";
169+
const string output = @"
170+
class C
171+
{
172+
private bool IsValid(int number)
173+
{
174+
return number > 0;
175+
}
176+
177+
void M()
178+
{
179+
if (IsValid(1) == false)
180+
{
181+
}
182+
}
183+
}";
184+
VerifyCSharpFix(input, output);
185+
}
186+
187+
protected override CodeFixProvider GetCSharpCodeFixProvider()
188+
{
189+
return new BooleanMethodNegationCodeFix();
190+
}
191+
192+
protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer()
193+
{
194+
return new BooleanMethodNegationAnalyzer();
195+
}
196+
}
197+
}

‎Raven.CodeAnalysis/BooleanMethodNegation/BooleanMethodNegationAnalyzer.cs‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
using System.Collections.Immutable;
2+
using Microsoft.CodeAnalysis;
3+
using Microsoft.CodeAnalysis.CSharp;
4+
using Microsoft.CodeAnalysis.CSharp.Syntax;
5+
using Microsoft.CodeAnalysis.Diagnostics;
6+
7+
namespace Raven.CodeAnalysis.BooleanMethodNegation
8+
{
9+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
10+
internal class BooleanMethodNegationAnalyzer : DiagnosticAnalyzer
11+
{
12+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
13+
ImmutableArray.Create(DiagnosticDescriptors.BooleanMethodNegation);
14+
15+
public override void Initialize(AnalysisContext context)
16+
{
17+
context.RegisterSyntaxNodeAction(AnalyzeSyntax, SyntaxKind.LogicalNotExpression);
18+
}
19+
20+
private static void AnalyzeSyntax(SyntaxNodeAnalysisContext context)
21+
{
22+
var logicalNotExpressionSyntax = (PrefixUnaryExpressionSyntax)context.Node;
23+
24+
var operand = logicalNotExpressionSyntax.Operand;
25+
while (operand is ParenthesizedExpressionSyntax parenthesizedExpressionSyntax)
26+
{
27+
operand = parenthesizedExpressionSyntax.Expression;
28+
}
29+
30+
var invocationExpressionSyntax = operand as InvocationExpressionSyntax;
31+
if (invocationExpressionSyntax == null)
32+
return;
33+
34+
var semanticModel = context.SemanticModel;
35+
var methodSymbol = semanticModel.GetSymbolInfo(invocationExpressionSyntax, context.CancellationToken).Symbol as IMethodSymbol;
36+
37+
if (methodSymbol?.ReturnType?.SpecialType != SpecialType.System_Boolean)
38+
return;
39+
40+
context.ReportDiagnostic(Diagnostic.Create(
41+
DiagnosticDescriptors.BooleanMethodNegation,
42+
logicalNotExpressionSyntax.GetLocation(),
43+
methodSymbol.Name));
44+
}
45+
}
46+
}

‎Raven.CodeAnalysis/BooleanMethodNegation/BooleanMethodNegationCodeFix.cs‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
using System.Collections.Immutable;
2+
using System.Threading.Tasks;
3+
using Microsoft.CodeAnalysis;
4+
using Microsoft.CodeAnalysis.CodeActions;
5+
using Microsoft.CodeAnalysis.CodeFixes;
6+
using Microsoft.CodeAnalysis.CSharp;
7+
using Microsoft.CodeAnalysis.CSharp.Syntax;
8+
9+
namespace Raven.CodeAnalysis.BooleanMethodNegation
10+
{
11+
[ExportCodeFixProvider(LanguageNames.CSharp, Name = "Rewrite negated boolean method conditions")]
12+
internal class BooleanMethodNegationCodeFix : CodeFixProvider
13+
{
14+
public override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(DiagnosticIds.BooleanMethodNegation);
15+
16+
public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;
17+
18+
public override async Task RegisterCodeFixesAsync(CodeFixContext context)
19+
{
20+
var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken);
21+
var syntaxNode = root.FindNode(context.Span, getInnermostNodeForTie: true) as PrefixUnaryExpressionSyntax;
22+
if (syntaxNode == null)
23+
return;
24+
25+
context.RegisterCodeFix(CodeAction.Create(
26+
"Rewrite boolean method condition to comparison",
27+
token => RewriteAsync(context.Document, syntaxNode, token)),
28+
context.Diagnostics);
29+
}
30+
31+
private static async Task<Document> RewriteAsync(Document document, PrefixUnaryExpressionSyntax logicalNotExpression, System.Threading.CancellationToken token)
32+
{
33+
var operand = logicalNotExpression.Operand;
34+
while (operand is ParenthesizedExpressionSyntax parenthesizedExpressionSyntax)
35+
{
36+
operand = parenthesizedExpressionSyntax.Expression;
37+
}
38+
39+
var invocationExpressionSyntax = operand as InvocationExpressionSyntax;
40+
if (invocationExpressionSyntax == null)
41+
return document;
42+
43+
var newCondition = SyntaxFactory.BinaryExpression(
44+
SyntaxKind.EqualsExpression,
45+
invocationExpressionSyntax,
46+
SyntaxFactory.LiteralExpression(SyntaxKind.FalseLiteralExpression))
47+
.WithTriviaFrom(logicalNotExpression);
48+
49+
var root = await document.GetSyntaxRootAsync(token);
50+
root = root.ReplaceNode(logicalNotExpression, newCondition);
51+
52+
return document.WithSyntaxRoot(root);
53+
}
54+
}
55+
}

‎Raven.CodeAnalysis/DiagnosticCategories.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,7 @@ internal static class DiagnosticCategories
1717
public const string TaskCompletionSource = "TaskCompletionSource";
1818

1919
public const string ReturningTaskInsideUsingStatement = "ReturnTaskInsideUsingStatement";
20+
21+
public const string BooleanMethodNegation = "BooleanMethodNegation";
2022
}
2123
}

‎Raven.CodeAnalysis/DiagnosticDescriptors.cs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,5 +76,13 @@ public static class DiagnosticDescriptors
7676
category: DiagnosticCategories.ReturningTaskInsideUsingStatement,
7777
defaultSeverity: DiagnosticSeverity.Error,
7878
isEnabledByDefault: true);
79+
80+
public static readonly DiagnosticDescriptor BooleanMethodNegation = new DiagnosticDescriptor(
81+
id: DiagnosticIds.BooleanMethodNegation,
82+
title: "Avoid negating boolean method conditions",
83+
messageFormat: "Negated boolean method '{0}' conditions should be rewritten as {0}(...) == false",
84+
category: DiagnosticCategories.BooleanMethodNegation,
85+
defaultSeverity: DiagnosticSeverity.Error,
86+
isEnabledByDefault: true);
7987
}
8088
}

‎Raven.CodeAnalysis/DiagnosticIds.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,7 @@ internal static class DiagnosticIds
1919
public const string TaskCompletionSourceMustHaveRunContinuationsAsynchronouslySet = "RDB0008";
2020

2121
public const string MustNotReturnTaskInsideUsingStatementAnalyzer = "RDB0009";
22+
23+
public const string BooleanMethodNegation = "RDB0010";
2224
}
2325
}

0 commit comments

Comments
 (0)