RavenDB-25423 Boolean method negation callsites diagnoser and fixer

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();
+        }
+    }
+}

Raven.CodeAnalysis.nuspec

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="utf-8"?>
 <package xmlns="http://schemas.microsoft.com/packaging/2011/10/nuspec.xsd">
   <metadata minClientVersion="2.12">
-    <version>1.0.12</version>
+    <version>1.0.13</version>
     <id>Raven.CodeAnalysis</id>
     <description>Raven.CodeAnalysis</description>
     <authors>RavenDB</authors>

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<DiagnosticDescriptor> 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));
+        }
+    }
+}

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<string> 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<Document> 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);
+        }
+    }
+}

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";
     }
 }

Raven.CodeAnalysis/DiagnosticDescriptors.cs

@@ -5,7 +5,7 @@
     public static class DiagnosticDescriptors
     {
         public static readonly DiagnosticDescriptor ValueTuple = new DiagnosticDescriptor(
             id: DiagnosticIds.ValueTupleVariablesMustBeUppercase,
             title: "Use PascalCase in named ValueTuples",
             messageFormat: "Use PascalCase in named ValueTuples",
             category: DiagnosticCategories.ValueTuple,
@@ -13,7 +13,7 @@
             isEnabledByDefault: true);
 
         public static readonly DiagnosticDescriptor Logging = new DiagnosticDescriptor(
             id: DiagnosticIds.Logging,
             title: "Wrap Debug and DebugException with IsDebugEnabled condition",
             messageFormat: "Wrap Debug and DebugException with IsDebugEnabled condition",
             category: DiagnosticCategories.Logging,
@@ -21,7 +21,7 @@
             isEnabledByDefault: true);
 
         public static readonly DiagnosticDescriptor ConfigureAwait = new DiagnosticDescriptor(
             id: DiagnosticIds.ConfigureAwait,
             title: "Awaited operations must have ConfigureAwait(false)",
             messageFormat: "Awaited operations must have ConfigureAwait(false)",
             category: DiagnosticCategories.ConfigureAwait,
@@ -29,7 +29,7 @@
             isEnabledByDefault: true);
 
         public static readonly DiagnosticDescriptor GetConfigurationEntryKey = new DiagnosticDescriptor(
             id: DiagnosticIds.GetConfigurationEntryKey,
             title: "Specified property is not a configuration entry",
             messageFormat: "'{0}' property is not decorated with [ConfigurationEntry] attribute",
             category: DiagnosticCategories.Configuration,
@@ -46,7 +46,7 @@
             isEnabledByDefault: true);
 
         public static readonly DiagnosticDescriptor TodoCommentOnExceptionHandler = new DiagnosticDescriptor(
             id: DiagnosticIds.TodoCommentOnExceptionHandler,
             title: "ToDo Comments on Exception Handler",
             messageFormat: "ToDo comments should be resolved and this exception should be properly handled",
             category: DiagnosticCategories.ExceptionBlock,
@@ -76,5 +76,13 @@
             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);
     }
 }