Improve S1244: Add message to use "IsNaN" instead of "== double.NaN"

analyzers/src/SonarAnalyzer.CSharp/Rules/EqualityOnFloatingPoint.cs

@@ -18,116 +18,119 @@
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 
-namespace SonarAnalyzer.Rules.CSharp
-{
-    [DiagnosticAnalyzer(LanguageNames.CSharp)]
-    public sealed class EqualityOnFloatingPoint : SonarDiagnosticAnalyzer
-    {
-        internal const string DiagnosticId = "S1244";
-        private const string MessageFormat = "Do not check floating point {0} with exact values, use a range instead.";
+namespace SonarAnalyzer.Rules.CSharp;
 
-        private static readonly DiagnosticDescriptor rule =
-            DescriptorFactory.Create(DiagnosticId, MessageFormat);
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class EqualityOnFloatingPoint : SonarDiagnosticAnalyzer
+{
+    private const string DiagnosticId = "S1244";
+    private const string MessageFormat = "Do not check floating point {0} with exact values, use {1} instead.";
 
-        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(rule);
+    private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat);
+    private static readonly Dictionary<string, string> SpecialMembers = new()
+    {
+        { nameof(double.NaN), nameof(double.IsNaN) },
+    };
 
-        private static readonly ISet<string> EqualityOperators = new HashSet<string> { "op_Equality", "op_Inequality" };
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);
 
-        protected override void Initialize(SonarAnalysisContext context)
-        {
-            context.RegisterNodeAction(
-                CheckEquality,
-                SyntaxKind.EqualsExpression,
-                SyntaxKind.NotEqualsExpression);
-
-            context.RegisterNodeAction(
-                CheckLogicalExpression,
-                SyntaxKind.LogicalAndExpression,
-                SyntaxKind.LogicalOrExpression);
-        }
+    protected override void Initialize(SonarAnalysisContext context)
+    {
+        context.RegisterNodeAction(
+            CheckEquality,
+            SyntaxKind.EqualsExpression,
+            SyntaxKind.NotEqualsExpression);
+
+        context.RegisterNodeAction(
+            CheckLogicalExpression,
+            SyntaxKind.LogicalAndExpression,
+            SyntaxKind.LogicalOrExpression);
+    }
 
-        private static void CheckLogicalExpression(SonarSyntaxNodeReportingContext context)
+    private static void CheckLogicalExpression(SonarSyntaxNodeReportingContext context)
+    {
+        var binaryExpression = (BinaryExpressionSyntax)context.Node;
+
+        if (TryGetBinaryExpression(binaryExpression.Left) is { } left
+            && TryGetBinaryExpression(binaryExpression.Right) is { } right
+            && CSharpEquivalenceChecker.AreEquivalent(right.Right, left.Right)
+            && CSharpEquivalenceChecker.AreEquivalent(right.Left, left.Left)
+            && IsIndirectEquality(context.SemanticModel, binaryExpression, left, right) is var isEquality
+            && IsIndirectInequality(context.SemanticModel, binaryExpression, left, right) is var isInequality
+            && (isEquality || isInequality))
         {
-            var binaryExpression = (BinaryExpressionSyntax)context.Node;
-            var left = TryGetBinaryExpression(binaryExpression.Left);
-            var right = TryGetBinaryExpression(binaryExpression.Right);
-
-            if (right == null || left == null)
-            {
-                return;
-            }
-
-            var eqRight = CSharpEquivalenceChecker.AreEquivalent(right.Right, left.Right);
-            var eqLeft = CSharpEquivalenceChecker.AreEquivalent(right.Left, left.Left);
-            if (!eqRight || !eqLeft)
-            {
-                return;
-            }
-
-            var isEquality = IsIndirectEquality(context.SemanticModel, binaryExpression, left, right);
-
-            if (isEquality || IsIndirectInequality(context.SemanticModel, binaryExpression, left, right))
-            {
-                var messageEqualityPart = GetMessageEqualityPart(isEquality);
-
-                context.ReportIssue(Diagnostic.Create(rule, binaryExpression.GetLocation(), messageEqualityPart));
-            }
+            context.ReportIssue(Diagnostic.Create(Rule, binaryExpression.GetLocation(), MessageEqualityPart(isEquality), "a range"));
         }
+    }
 
-        private static string GetMessageEqualityPart(bool isEquality) =>
-            isEquality ? "equality" : "inequality";
+    private static string MessageEqualityPart(bool isEquality) =>
+        isEquality ? "equality" : "inequality";
 
-        private static void CheckEquality(SonarSyntaxNodeReportingContext context)
+    private static void CheckEquality(SonarSyntaxNodeReportingContext context)
+    {
+        var equals = (BinaryExpressionSyntax)context.Node;
+        if (context.SemanticModel.GetSymbolInfo(equals).Symbol is IMethodSymbol { ContainingType: { } container } method
+            && IsFloatingPointType(container)
+            && (method.IsOperatorEquals() || method.IsOperatorNotEquals()))
         {
-            var equals = (BinaryExpressionSyntax)context.Node;
-
-            if (context.SemanticModel.GetSymbolInfo(equals).Symbol is IMethodSymbol { ContainingType: { } container, Name: { } equalitySymbolName }
-                && IsFloatingPointNumberType(container)
-                && EqualityOperators.Contains(equalitySymbolName))
-            {
-                var messageEqualityPart = GetMessageEqualityPart(equals.IsKind(SyntaxKind.EqualsExpression));
-
-                context.ReportIssue(Diagnostic.Create(rule, equals.OperatorToken.GetLocation(), messageEqualityPart));
-            }
+            var messageEqualityPart = MessageEqualityPart(equals.IsKind(SyntaxKind.EqualsExpression));
+            var proposed = ProposedMessageForMemberAccess(context, equals.Right)
+                ?? ProposedMessageForMemberAccess(context, equals.Left)
+                ?? ProposedMessageForIdentifier(context, equals.Right)
+                ?? ProposedMessageForIdentifier(context, equals.Left)
+                ?? "a range";
+            context.ReportIssue(Diagnostic.Create(Rule, equals.OperatorToken.GetLocation(), messageEqualityPart, proposed));
         }
-
-        // Returns true for the floating point types that suffer from equivalence problems. All .NET floating point types have this problem except `decimal.`
-        // - Reason for excluding `decimal`: the documentation for the `decimal.Equals()` method does not have a "Precision in Comparisons" section as the other .NET floating point types.
-        // - Power-2-based types like `double` implement `IFloatingPointIeee754`, but power-10-based `decimal` implements `IFloatingPoint`.
-        // - `IFloatingPointIeee754` defines `Epsilon` which indicates problems with equivalence checking.
-        private static bool IsFloatingPointNumberType(ITypeSymbol type) =>
-            type.IsAny(KnownType.FloatingPointNumbers)
-            || (type.Is(KnownType.System_Numerics_IEqualityOperators_TSelf_TOther_TResult) // The operator originates from a virtual static member
-                && type is INamedTypeSymbol { TypeArguments: { } typeArguments }           // Arguments of TSelf, TOther, TResult
-                && typeArguments.Any(IsFloatingPointNumberType))
-            || (type is ITypeParameterSymbol { ConstraintTypes: { } constraintTypes }      // constraints of TSelf or of TSelf, TOther, TResult from IEqualityOperators
-                && constraintTypes.Any(constraint => constraint.DerivesOrImplements(KnownType.System_Numerics_IFloatingPointIeee754_TSelf)));
-
-        private static BinaryExpressionSyntax TryGetBinaryExpression(ExpressionSyntax expression) =>
-            expression.RemoveParentheses() as BinaryExpressionSyntax;
-
-        private static bool IsIndirectInequality(SemanticModel semanticModel, BinaryExpressionSyntax binaryExpression, BinaryExpressionSyntax left, BinaryExpressionSyntax right) =>
-            binaryExpression.IsKind(SyntaxKind.LogicalOrExpression)
-                && HasAppropriateOperatorsForInequality(left, right)
-                && HasFloatingType(semanticModel, right.Left, right.Right);
-
-        private static bool IsIndirectEquality(SemanticModel semanticModel, BinaryExpressionSyntax binaryExpression, BinaryExpressionSyntax left, BinaryExpressionSyntax right) =>
-            binaryExpression.IsKind(SyntaxKind.LogicalAndExpression)
-                && HasAppropriateOperatorsForEquality(left, right)
-                && HasFloatingType(semanticModel, right.Left, right.Right);
-
-        private static bool HasFloatingType(SemanticModel semanticModel, ExpressionSyntax left, ExpressionSyntax right) =>
-            IsExpressionFloatingType(semanticModel, right) || IsExpressionFloatingType(semanticModel, left);
-
-        private static bool IsExpressionFloatingType(SemanticModel semanticModel, ExpressionSyntax expression) =>
-            IsFloatingPointNumberType(semanticModel.GetTypeInfo(expression).Type);
-
-        private static bool HasAppropriateOperatorsForEquality(BinaryExpressionSyntax left, BinaryExpressionSyntax right) =>
-            (left.OperatorToken.Kind() is SyntaxKind.GreaterThanEqualsToken && right.OperatorToken.Kind() is SyntaxKind.LessThanEqualsToken)
-            || (left.OperatorToken.Kind() is SyntaxKind.LessThanEqualsToken && right.OperatorToken.Kind() is SyntaxKind.GreaterThanEqualsToken);
-
-        private static bool HasAppropriateOperatorsForInequality(BinaryExpressionSyntax left, BinaryExpressionSyntax right) =>
-            (left.OperatorToken.Kind() is SyntaxKind.GreaterThanToken && right.OperatorToken.Kind() is SyntaxKind.LessThanToken)
-            || (left.OperatorToken.Kind() is SyntaxKind.LessThanToken && right.OperatorToken.Kind() is SyntaxKind.GreaterThanToken);
     }
+
+    private static string ProposedMessageForMemberAccess(SonarSyntaxNodeReportingContext context, ExpressionSyntax expression) =>
+        expression is MemberAccessExpressionSyntax memberAccess
+        && SpecialMembers.TryGetValue(memberAccess.GetName(), out var proposedMethod)
+        && context.SemanticModel.GetTypeInfo(memberAccess).ConvertedType is { } type
+        && IsFloatingPointType(type)
+            ? $"'{type.ToMinimalDisplayString(context.SemanticModel, memberAccess.SpanStart)}.{proposedMethod}()'"
+            : null;
+
+    private static string ProposedMessageForIdentifier(SonarSyntaxNodeReportingContext context, ExpressionSyntax expression) =>
+        expression is IdentifierNameSyntax identifier
+        && SpecialMembers.TryGetValue(identifier.GetName(), out var proposedMethod)
+        && context.SemanticModel.GetSymbolInfo(identifier).Symbol is { ContainingType: { } type }
+        && IsFloatingPointType(type)
+            ? $"'{proposedMethod}()'"
+            : null;
+
+    // Returns true for the floating point types that suffer from equivalence problems. All .NET floating point types have this problem except `decimal.`
+    // - Reason for excluding `decimal`: the documentation for the `decimal.Equals()` method does not have a "Precision in Comparisons" section as the other .NET floating point types.
+    // - Power-2-based types like `double` implement `IFloatingPointIeee754`, but power-10-based `decimal` implements `IFloatingPoint`.
+    // - `IFloatingPointIeee754` defines `Epsilon` which indicates problems with equivalence checking.
+    private static bool IsFloatingPointType(ITypeSymbol type) =>
+        type.IsAny(KnownType.FloatingPointNumbers)
+        || (type.Is(KnownType.System_Numerics_IEqualityOperators_TSelf_TOther_TResult) // The operator originates from a virtual static member
+            && type is INamedTypeSymbol { TypeArguments: { } typeArguments }           // Arguments of TSelf, TOther, TResult
+            && typeArguments.Any(IsFloatingPointType))
+        || (type is ITypeParameterSymbol { ConstraintTypes: { } constraintTypes }      // constraints of TSelf or of TSelf, TOther, TResult from IEqualityOperators
+            && constraintTypes.Any(x => x.DerivesOrImplements(KnownType.System_Numerics_IFloatingPointIeee754_TSelf)));
+
+    private static BinaryExpressionSyntax TryGetBinaryExpression(ExpressionSyntax expression) =>
+        expression.RemoveParentheses() as BinaryExpressionSyntax;
+
+    private static bool IsIndirectInequality(SemanticModel semanticModel, BinaryExpressionSyntax binaryExpression, BinaryExpressionSyntax left, BinaryExpressionSyntax right) =>
+        binaryExpression.IsKind(SyntaxKind.LogicalOrExpression)
+        && IsOperatorPair(left, right, SyntaxKind.GreaterThanToken, SyntaxKind.LessThanToken)
+        && HasFloatingType(semanticModel, right);
+
+    private static bool IsIndirectEquality(SemanticModel semanticModel, BinaryExpressionSyntax binaryExpression, BinaryExpressionSyntax left, BinaryExpressionSyntax right) =>
+        binaryExpression.IsKind(SyntaxKind.LogicalAndExpression)
+        && IsOperatorPair(left, right, SyntaxKind.GreaterThanEqualsToken, SyntaxKind.LessThanEqualsToken)
+        && HasFloatingType(semanticModel, right);
+
+    private static bool HasFloatingType(SemanticModel semanticModel, BinaryExpressionSyntax binary) =>
+        IsExpressionFloatingType(semanticModel, binary.Right) || IsExpressionFloatingType(semanticModel, binary.Left);
+
+    private static bool IsExpressionFloatingType(SemanticModel semanticModel, ExpressionSyntax expression) =>
+        IsFloatingPointType(semanticModel.GetTypeInfo(expression).Type);
+
+    private static bool IsOperatorPair(BinaryExpressionSyntax left, BinaryExpressionSyntax right, SyntaxKind first, SyntaxKind second) =>
+        (left.OperatorToken.IsKind(first) && right.OperatorToken.IsKind(second))
+        || (left.OperatorToken.IsKind(second) && right.OperatorToken.IsKind(first));
 }

analyzers/src/SonarAnalyzer.CSharp/Rules/OperatorsShouldBeOverloadedConsistently.cs

@@ -102,56 +102,35 @@ private static IEnumerable<string> FindMissingMethods(INamedTypeSymbol classSymb
 
         private static IEnumerable<string> GetImplementedMethods(INamedTypeSymbol classSymbol)
         {
-            var classMethods = classSymbol
-                .GetMembers()
-                .OfType<IMethodSymbol>()
-                .Where(m => !m.IsConstructor())
-                .ToList();
-
-            if (classMethods.Any(KnownMethods.IsOperatorBinaryPlus))
-            {
-                yield return MethodName.OperatorPlus;
-            }
-
-            if (classMethods.Any(KnownMethods.IsOperatorBinaryMinus))
-            {
-                yield return MethodName.OperatorMinus;
-            }
-
-            if (classMethods.Any(KnownMethods.IsOperatorBinaryMultiply))
-            {
-                yield return MethodName.OperatorMultiply;
-            }
-
-            if (classMethods.Any(KnownMethods.IsOperatorBinaryDivide))
-            {
-                yield return MethodName.OperatorDivide;
-            }
-
-            if (classMethods.Any(KnownMethods.IsOperatorBinaryModulus))
-            {
-                yield return MethodName.OperatorReminder;
-            }
-
-            if (classMethods.Any(KnownMethods.IsOperatorEquals))
+            foreach (var member in classSymbol.GetMembers().OfType<IMethodSymbol>().Where(x => !x.IsConstructor()))
             {
-                yield return MethodName.OperatorEquals;
-            }
-
-            if (classMethods.Any(KnownMethods.IsOperatorNotEquals))
-            {
-                yield return MethodName.OperatorNotEquals;
-            }
-
-            if (classMethods.Any(KnownMethods.IsObjectEquals))
-            {
-                yield return MethodName.ObjectEquals;
+                if (ImplementedOperator(member) is { } name)
+                {
+                    yield return name;
+                }
+                else if (KnownMethods.IsObjectEquals(member))
+                {
+                    yield return MethodName.ObjectEquals;
+                }
+                else if (KnownMethods.IsObjectGetHashCode(member))
+                {
+                    yield return MethodName.ObjectGetHashCode;
+                }
             }
+        }
 
-            if (classMethods.Any(KnownMethods.IsObjectGetHashCode))
+        private static string ImplementedOperator(IMethodSymbol member) =>
+            member switch
             {
-                yield return MethodName.ObjectGetHashCode;
-            }
-        }
+                _ when member.MethodKind != MethodKind.UserDefinedOperator => null,
+                _ when KnownMethods.IsOperatorBinaryPlus(member) => MethodName.OperatorPlus,
+                _ when KnownMethods.IsOperatorBinaryMinus(member) => MethodName.OperatorMinus,
+                _ when KnownMethods.IsOperatorBinaryMultiply(member) => MethodName.OperatorMultiply,
+                _ when KnownMethods.IsOperatorBinaryDivide(member) => MethodName.OperatorDivide,
+                _ when KnownMethods.IsOperatorBinaryModulus(member) => MethodName.OperatorReminder,
+                _ when KnownMethods.IsOperatorEquals(member) => MethodName.OperatorEquals,
+                _ when KnownMethods.IsOperatorNotEquals(member) => MethodName.OperatorNotEquals,
+                _ => null
+            };
     }
 }