diff --git a/Raven.CodeAnalysis.Test/BooleanMethodNegationTests.cs b/Raven.CodeAnalysis.Test/BooleanMethodNegationTests.cs new file mode 100644 index 0000000..359170f --- /dev/null +++ b/Raven.CodeAnalysis.Test/BooleanMethodNegationTests.cs @@ -0,0 +1,263 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Raven.CodeAnalysis.BooleanMethodNegation; +using TestHelper; + +namespace Raven.CodeAnalysis.Test +{ + [TestClass] + public class BooleanMethodNegationTests : CodeFixVerifier + { + [TestMethod] + public void ShouldReportDiagnosticOnNegatedBooleanMethod() + { + const string input = @" +class C +{ + private bool HasPermission() + { + return false; + } + + void M() + { + if (!HasPermission()) + { + } + } +}"; + VerifyCSharpDiagnostic(input, new DiagnosticResult + { + Id = DiagnosticIds.BooleanMethodNegation, + Message = "Negated boolean method 'HasPermission' conditions should be rewritten as HasPermission(...) == false", + Severity = DiagnosticSeverity.Error, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 11, 13) + } + }); + } + + [TestMethod] + public void ShouldReportDiagnosticOnNegatedBooleanMethodWithArguments() + { + const string input = @" +class C +{ + private bool IsValid(int number) + { + return number > 0; + } + + void M() + { + if (!IsValid(1)) + { + } + } +}"; + VerifyCSharpDiagnostic(input, new DiagnosticResult + { + Id = DiagnosticIds.BooleanMethodNegation, + Message = "Negated boolean method 'IsValid' conditions should be rewritten as IsValid(...) == false", + Severity = DiagnosticSeverity.Error, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 11, 13) + } + }); + } + + [TestMethod] + public void ShouldReportDiagnosticOnNegatedBooleanMethod_param() + { + const string input = @" +class C +{ + private bool HasPermission() + { + return false; + } + + static void M(C c) + { + if (!c.HasPermission()) + { + } + } +}"; + VerifyCSharpDiagnostic(input, new DiagnosticResult + { + Id = DiagnosticIds.BooleanMethodNegation, + Message = "Negated boolean method 'HasPermission' conditions should be rewritten as HasPermission(...) == false", + Severity = DiagnosticSeverity.Error, + Locations = + [ + new DiagnosticResultLocation("Test0.cs", 11, 13) + ] + }); + } + + [TestMethod] + public void ShouldReportDiagnosticOnNegatedBooleanMethod_field() + { + const string input = @" +class C +{ + internal bool HasPermission() + { + return false; + } +} + +class D +{ + private C _c; + + void M() + { + if (!_c.HasPermission()) + { + } + } +} +"; + VerifyCSharpDiagnostic(input, new DiagnosticResult + { + Id = DiagnosticIds.BooleanMethodNegation, + Message = "Negated boolean method 'HasPermission' conditions should be rewritten as HasPermission(...) == false", + Severity = DiagnosticSeverity.Error, + Locations = + [ + new DiagnosticResultLocation("Test0.cs", 16, 13) + ] + }); + } + + [TestMethod] + public void ShouldNotReportDiagnosticOnNonNegatedBooleanMethod() + { + const string input = @" +class C +{ + private bool HasPermission() + { + return false; + } + + void M() + { + if (HasPermission()) + { + } + } +}"; + VerifyCSharpDiagnostic(input); + } + + [TestMethod] + public void ShouldNotReportDiagnosticOnComparisonToFalse() + { + const string input = @" +class C +{ + private bool IsValid(int number) + { + return number > 0; + } + + void M() + { + if (IsValid(1) == false) + { + } + } +}"; + VerifyCSharpDiagnostic(input); + } + + [TestMethod] + public void ShouldRewriteNegatedBooleanMethodCondition() + { + const string input = @" +class C +{ + private bool HasPermission() + { + return false; + } + + void M() + { + if (!HasPermission()) + { + } + } +}"; + const string output = @" +class C +{ + private bool HasPermission() + { + return false; + } + + void M() + { + if (HasPermission() == false) + { + } + } +}"; + VerifyCSharpFix(input, output); + } + + [TestMethod] + public void ShouldRewriteNegatedBooleanMethodConditionWithArguments() + { + const string input = @" +class C +{ + private bool IsValid(int number) + { + return number > 0; + } + + void M() + { + if (!IsValid(1)) + { + } + } +}"; + const string output = @" +class C +{ + private bool IsValid(int number) + { + return number > 0; + } + + void M() + { + if (IsValid(1) == false) + { + } + } +}"; + VerifyCSharpFix(input, output); + } + + protected override CodeFixProvider GetCSharpCodeFixProvider() + { + return new BooleanMethodNegationCodeFix(); + } + + protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() + { + return new BooleanMethodNegationAnalyzer(); + } + } +} \ No newline at end of file diff --git a/Raven.CodeAnalysis.nuspec b/Raven.CodeAnalysis.nuspec index 067c777..1b733ab 100644 --- a/Raven.CodeAnalysis.nuspec +++ b/Raven.CodeAnalysis.nuspec @@ -1,7 +1,7 @@ - 1.0.12 + 1.0.13 Raven.CodeAnalysis Raven.CodeAnalysis RavenDB diff --git a/Raven.CodeAnalysis/BooleanMethodNegation/BooleanMethodNegationAnalyzer.cs b/Raven.CodeAnalysis/BooleanMethodNegation/BooleanMethodNegationAnalyzer.cs new file mode 100644 index 0000000..3ba004a --- /dev/null +++ b/Raven.CodeAnalysis/BooleanMethodNegation/BooleanMethodNegationAnalyzer.cs @@ -0,0 +1,45 @@ +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Raven.CodeAnalysis.BooleanMethodNegation +{ + [DiagnosticAnalyzer(LanguageNames.CSharp)] + internal class BooleanMethodNegationAnalyzer : DiagnosticAnalyzer + { + public override ImmutableArray SupportedDiagnostics => + ImmutableArray.Create(DiagnosticDescriptors.BooleanMethodNegation); + + public override void Initialize(AnalysisContext context) + { + context.RegisterSyntaxNodeAction(AnalyzeSyntax, SyntaxKind.LogicalNotExpression); + } + + private static void AnalyzeSyntax(SyntaxNodeAnalysisContext context) + { + var logicalNotExpressionSyntax = (PrefixUnaryExpressionSyntax)context.Node; + + var operand = logicalNotExpressionSyntax.Operand; + while (operand is ParenthesizedExpressionSyntax parenthesizedExpressionSyntax) + { + operand = parenthesizedExpressionSyntax.Expression; + } + + if (operand is InvocationExpressionSyntax invocation == false) + return; + + var semanticModel = context.SemanticModel; + var methodSymbol = semanticModel.GetSymbolInfo(invocation, context.CancellationToken).Symbol as IMethodSymbol; + + if (methodSymbol?.ReturnType?.SpecialType != SpecialType.System_Boolean) + return; + + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.BooleanMethodNegation, + logicalNotExpressionSyntax.GetLocation(), + methodSymbol.Name)); + } + } +} \ No newline at end of file diff --git a/Raven.CodeAnalysis/BooleanMethodNegation/BooleanMethodNegationCodeFix.cs b/Raven.CodeAnalysis/BooleanMethodNegation/BooleanMethodNegationCodeFix.cs new file mode 100644 index 0000000..ed9b401 --- /dev/null +++ b/Raven.CodeAnalysis/BooleanMethodNegation/BooleanMethodNegationCodeFix.cs @@ -0,0 +1,55 @@ +using System.Collections.Immutable; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace Raven.CodeAnalysis.BooleanMethodNegation +{ + [ExportCodeFixProvider(LanguageNames.CSharp, Name = "Rewrite negated boolean method conditions")] + internal class BooleanMethodNegationCodeFix : CodeFixProvider + { + public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(DiagnosticIds.BooleanMethodNegation); + + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken); + var syntaxNode = root.FindNode(context.Span, getInnermostNodeForTie: true) as PrefixUnaryExpressionSyntax; + if (syntaxNode == null) + return; + + context.RegisterCodeFix(CodeAction.Create( + "Rewrite boolean method condition to comparison", + token => RewriteAsync(context.Document, syntaxNode, token)), + context.Diagnostics); + } + + private static async Task RewriteAsync(Document document, PrefixUnaryExpressionSyntax logicalNotExpression, System.Threading.CancellationToken token) + { + var operand = logicalNotExpression.Operand; + while (operand is ParenthesizedExpressionSyntax parenthesizedExpressionSyntax) + { + operand = parenthesizedExpressionSyntax.Expression; + } + + var invocationExpressionSyntax = operand as InvocationExpressionSyntax; + if (invocationExpressionSyntax == null) + return document; + + var newCondition = SyntaxFactory.BinaryExpression( + SyntaxKind.EqualsExpression, + invocationExpressionSyntax, + SyntaxFactory.LiteralExpression(SyntaxKind.FalseLiteralExpression)) + .WithTriviaFrom(logicalNotExpression); + + var root = await document.GetSyntaxRootAsync(token); + root = root.ReplaceNode(logicalNotExpression, newCondition); + + return document.WithSyntaxRoot(root); + } + } +} \ No newline at end of file diff --git a/Raven.CodeAnalysis/DiagnosticCategories.cs b/Raven.CodeAnalysis/DiagnosticCategories.cs index 206380c..946c086 100644 --- a/Raven.CodeAnalysis/DiagnosticCategories.cs +++ b/Raven.CodeAnalysis/DiagnosticCategories.cs @@ -17,5 +17,7 @@ internal static class DiagnosticCategories public const string TaskCompletionSource = "TaskCompletionSource"; public const string ReturningTaskInsideUsingStatement = "ReturnTaskInsideUsingStatement"; + + public const string BooleanMethodNegation = "BooleanMethodNegation"; } } diff --git a/Raven.CodeAnalysis/DiagnosticDescriptors.cs b/Raven.CodeAnalysis/DiagnosticDescriptors.cs index ef332e9..879fb68 100644 --- a/Raven.CodeAnalysis/DiagnosticDescriptors.cs +++ b/Raven.CodeAnalysis/DiagnosticDescriptors.cs @@ -76,5 +76,13 @@ public static class DiagnosticDescriptors category: DiagnosticCategories.ReturningTaskInsideUsingStatement, defaultSeverity: DiagnosticSeverity.Error, isEnabledByDefault: true); + + public static readonly DiagnosticDescriptor BooleanMethodNegation = new DiagnosticDescriptor( + id: DiagnosticIds.BooleanMethodNegation, + title: "Avoid negating boolean method conditions", + messageFormat: "Negated boolean method '{0}' conditions should be rewritten as {0}(...) == false", + category: DiagnosticCategories.BooleanMethodNegation, + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true); } } diff --git a/Raven.CodeAnalysis/DiagnosticIds.cs b/Raven.CodeAnalysis/DiagnosticIds.cs index e1a0c81..097a745 100644 --- a/Raven.CodeAnalysis/DiagnosticIds.cs +++ b/Raven.CodeAnalysis/DiagnosticIds.cs @@ -19,5 +19,7 @@ internal static class DiagnosticIds public const string TaskCompletionSourceMustHaveRunContinuationsAsynchronouslySet = "RDB0008"; public const string MustNotReturnTaskInsideUsingStatementAnalyzer = "RDB0009"; + + public const string BooleanMethodNegation = "RDB0010"; } }