Improve analyzer ExpressionIsAlwaysEqualToTrueOrFalse (RCS1215)

dotnet · Feb 25, 2020 · 0fdd97f · 0fdd97f
1 parent 5679452
commit 0fdd97f
Show file tree

Hide file tree

Showing 5 changed files with 148 additions and 3 deletions.
diff --git a/src/Analyzers.CodeFixes/CSharp/CodeFixes/ExpressionCodeFixProvider.cs b/src/Analyzers.CodeFixes/CSharp/CodeFixes/ExpressionCodeFixProvider.cs
@@ -7,9 +7,11 @@
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CodeActions;
 using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Roslynator.CodeFixes;
-using Microsoft.CodeAnalysis.CSharp;
+using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory;
+using static Roslynator.CSharp.CSharpFactory;
 
 namespace Roslynator.CSharp.CodeFixes
 {
@@ -45,7 +47,7 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
                             {
                                 var binaryExpression = (BinaryExpressionSyntax)expression;
 
-                                LiteralExpressionSyntax newNode = CSharpFactory.BooleanLiteralExpression(binaryExpression.IsKind(SyntaxKind.GreaterThanOrEqualExpression, SyntaxKind.LessThanOrEqualExpression));
+                                LiteralExpressionSyntax newNode = BooleanLiteralExpression(binaryExpression.IsKind(SyntaxKind.GreaterThanOrEqualExpression, SyntaxKind.LessThanOrEqualExpression));
 
                                 CodeAction codeAction = CodeAction.Create(
                                     $"Replace expression with '{newNode}'",
@@ -54,7 +56,30 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
 
                                 context.RegisterCodeFix(codeAction, diagnostic);
                             }
-                            else
+                            else if (diagnostic.Properties.TryGetValue("DoubleNaN", out string leftOrRight))
+                            {
+                                var binaryExpression = (BinaryExpressionSyntax)expression;
+
+                                CodeAction codeAction = CodeAction.Create(
+                                    $"Call 'IsNaN'",
+                                    ct =>
+                                    {
+                                        ExpressionSyntax newExpression = SimpleMemberInvocationExpression(
+                                            CSharpTypeFactory.DoubleType(),
+                                            IdentifierName("IsNaN"),
+                                            Argument((leftOrRight == "Left") ? binaryExpression.Left.WithoutLeadingTrivia() : binaryExpression.Right.WithoutTrailingTrivia()));
+
+                                        if (binaryExpression.IsKind(SyntaxKind.NotEqualsExpression))
+                                            newExpression = LogicalNotExpression(newExpression);
+
+                                        newExpression = newExpression.Parenthesize().WithTriviaFrom(binaryExpression);
+
+                                        return document.ReplaceNodeAsync(binaryExpression, newExpression, ct);
+                                    },
+                                    GetEquivalenceKey(diagnostic));
+
+                                context.RegisterCodeFix(codeAction, diagnostic);
+                            }
                             {
                                 CodeAction codeAction = CodeAction.Create(
                                     "Remove null check",

diff --git a/src/Analyzers/Analyzers.xml b/src/Analyzers/Analyzers.xml
@@ -4674,6 +4674,10 @@ if (items.Count < 0) // [|Id|]
 {
 }]]></Before>
       </Sample>
+      <Sample>
+        <Before><![CDATA[x == double.NaN]]></Before>
+        <After><![CDATA[double.IsNaN(x)]]></After>
+      </Sample>
     </Samples>
   </Analyzer>
   <Analyzer Identifier="UnnecessaryUnsafeContext">

diff --git a/src/Analyzers/CSharp/Analysis/BinaryOperatorAnalyzer.cs b/src/Analyzers/CSharp/Analysis/BinaryOperatorAnalyzer.cs
@@ -1,6 +1,7 @@
 // Copyright (c) Josef Pihrt. All rights reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Threading;
 using Microsoft.CodeAnalysis;
@@ -44,9 +45,46 @@ public override void Initialize(AnalysisContext context)
                 startContext.RegisterSyntaxNodeAction(AnalyzeLessThanOrEqualExpression, SyntaxKind.LessThanOrEqualExpression);
                 startContext.RegisterSyntaxNodeAction(AnalyzeGreaterThanOrEqualExpression, SyntaxKind.GreaterThanOrEqualExpression);
                 startContext.RegisterSyntaxNodeAction(AnalyzeLogicalOrExpression, SyntaxKind.LogicalOrExpression);
+
+                if (!startContext.IsAnalyzerSuppressed(DiagnosticDescriptors.ExpressionIsAlwaysEqualToTrueOrFalse))
+                {
+                    startContext.RegisterSyntaxNodeAction(AnalyzeSimpleMemberAccessExpression, SyntaxKind.SimpleMemberAccessExpression);
+                }
             });
         }
 
+        // x == double.NaN >>> double.IsNaN(x)
+        // x != double.NaN >>> !double.IsNaN(x)
+        private void AnalyzeSimpleMemberAccessExpression(SyntaxNodeAnalysisContext context)
+        {
+            var simpleMemberAccess = (MemberAccessExpressionSyntax)context.Node;
+
+            if (!(simpleMemberAccess.Name is IdentifierNameSyntax identifierName))
+                return;
+
+            if (identifierName.Identifier.ValueText != "NaN")
+                return;
+
+            ExpressionSyntax expression = simpleMemberAccess.WalkUpParentheses();
+
+            SyntaxNode binaryExpression = expression.Parent;
+
+            if (!binaryExpression.IsKind(SyntaxKind.EqualsExpression, SyntaxKind.NotEqualsExpression))
+                return;
+
+            ISymbol symbol = context.SemanticModel.GetSymbol(simpleMemberAccess, context.CancellationToken);
+
+            if (symbol?.ContainingType?.SpecialType != SpecialType.System_Double)
+                return;
+
+            DiagnosticHelpers.ReportDiagnostic(
+                context,
+                DiagnosticDescriptors.ExpressionIsAlwaysEqualToTrueOrFalse,
+                binaryExpression.GetLocation(),
+                ImmutableDictionary.CreateRange(new KeyValuePair<string, string>[] { new KeyValuePair<string, string>("DoubleNaN", (((BinaryExpressionSyntax)binaryExpression).Left == expression) ? "Right" : "Left") }),
+                (binaryExpression.IsKind(SyntaxKind.EqualsExpression)) ? "false" : "true");
+        }
+
         // x:
         // byte
         // ushort

diff --git a/src/CSharp.Workspaces/CSharp/CSharpTypeFactory.cs b/src/CSharp.Workspaces/CSharp/CSharpTypeFactory.cs
@@ -9,6 +9,7 @@ internal static class CSharpTypeFactory
     {
         private static TypeSyntax _boolType;
         private static TypeSyntax _intType;
+        private static TypeSyntax _doubleType;
         private static TypeSyntax _stringType;
         private static TypeSyntax _objectType;
         private static TypeSyntax _notImplementedException;
@@ -24,6 +25,11 @@ public static TypeSyntax IntType()
             return _intType ?? (_intType = Parse("System.Int32"));
         }
 
+        public static TypeSyntax DoubleType()
+        {
+            return _doubleType ?? (_doubleType = Parse("System.Double"));
+        }
+
         public static TypeSyntax StringType()
         {
             return _stringType ?? (_stringType = Parse("System.String"));

diff --git a/src/Tests/Analyzers.Tests/RCS1215ExpressionIsAlwaysEqualToTrueOrFalseTests.cs b/src/Tests/Analyzers.Tests/RCS1215ExpressionIsAlwaysEqualToTrueOrFalseTests.cs
@@ -203,6 +203,78 @@ void M()
 ");
         }
 
+        [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ExpressionIsAlwaysEqualToTrueOrFalse)]
+        public async Task Test_EqualsToDoubleNaN()
+        {
+            await VerifyDiagnosticAndFixAsync(@"
+class C
+{
+    void M()
+    {
+        double x = default;
+        if ([|x == double.NaN|]) { }
+    }
+}
+", @"
+class C
+{
+    void M()
+    {
+        double x = default;
+        if (double.IsNaN(x)) { }
+    }
+}
+");
+        }
+
+        [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ExpressionIsAlwaysEqualToTrueOrFalse)]
+        public async Task Test_EqualsToDoubleNaN_Right()
+        {
+            await VerifyDiagnosticAndFixAsync(@"
+class C
+{
+    void M()
+    {
+        double x = default;
+        if ([|double.NaN == x|]) { }
+    }
+}
+", @"
+class C
+{
+    void M()
+    {
+        double x = default;
+        if (double.IsNaN(x)) { }
+    }
+}
+");
+        }
+
+        [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ExpressionIsAlwaysEqualToTrueOrFalse)]
+        public async Task Test_NotEqualsToDoubleNaN()
+        {
+            await VerifyDiagnosticAndFixAsync(@"
+class C
+{
+    void M()
+    {
+        double x = default;
+        if ([|x != double.NaN|]) { }
+    }
+}
+", @"
+class C
+{
+    void M()
+    {
+        double x = default;
+        if (!double.IsNaN(x)) { }
+    }
+}
+");
+        }
+
         [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ExpressionIsAlwaysEqualToTrueOrFalse)]
         public async Task TestNoDiagnostic_ReversedForStatement()
         {