From bbd3433bbc68827d16153cf0e440ff9334d4ab03 Mon Sep 17 00:00:00 2001 From: Antonio Aversa Date: Wed, 1 Feb 2023 14:41:49 +0100 Subject: [PATCH] Improve S1244: Add message to use "IsNaN" instead of "== double.NaN" (#6664) --- .../Rules/EqualityOnFloatingPoint.cs | 203 ++++++++--------- ...OperatorsShouldBeOverloadedConsistently.cs | 73 +++---- .../Helpers/KnownMethods.cs | 14 +- .../Rules/EqualityOnFloatingPointTest.cs | 29 ++- .../EqualityOnFloatingPoint.CSharp11.cs | 19 ++ .../TestCases/EqualityOnFloatingPoint.cs | 206 +++++++++++------- 6 files changed, 298 insertions(+), 246 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/EqualityOnFloatingPoint.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/EqualityOnFloatingPoint.cs index 7747cbee13f..7cde027d1c0 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/EqualityOnFloatingPoint.cs +++ b/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 SupportedDiagnostics { get; } = ImmutableArray.Create(rule); + private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat); + private static readonly Dictionary SpecialMembers = new() + { + { nameof(double.NaN), nameof(double.IsNaN) }, + }; - private static readonly ISet EqualityOperators = new HashSet { "op_Equality", "op_Inequality" }; + public override ImmutableArray 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)); } diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/OperatorsShouldBeOverloadedConsistently.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/OperatorsShouldBeOverloadedConsistently.cs index e9e7f22b87f..9d2413b956d 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/OperatorsShouldBeOverloadedConsistently.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/OperatorsShouldBeOverloadedConsistently.cs @@ -102,56 +102,35 @@ private static IEnumerable FindMissingMethods(INamedTypeSymbol classSymb private static IEnumerable GetImplementedMethods(INamedTypeSymbol classSymbol) { - var classMethods = classSymbol - .GetMembers() - .OfType() - .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().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; - } - } + { MethodKind: not 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 + }; } } diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownMethods.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownMethods.cs index 5ab58467bd1..15313f827a8 100644 --- a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownMethods.cs +++ b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownMethods.cs @@ -157,25 +157,25 @@ public static bool IsDiagnosticDebugMethod(this IMethodSymbol methodSymbol) => methodSymbol != null && methodSymbol.ContainingType.Is(KnownType.System_Diagnostics_Debug); public static bool IsOperatorBinaryPlus(this IMethodSymbol methodSymbol) => - methodSymbol is { MethodKind: MethodKind.UserDefinedOperator, Name: "op_Addition", Parameters: { Length: NumberOfParamsForBinaryOperator } }; + methodSymbol is { MethodKind: MethodKind.BuiltinOperator or MethodKind.UserDefinedOperator, Name: "op_Addition", Parameters.Length: NumberOfParamsForBinaryOperator }; public static bool IsOperatorBinaryMinus(this IMethodSymbol methodSymbol) => - methodSymbol is { MethodKind: MethodKind.UserDefinedOperator, Name: "op_Subtraction", Parameters: { Length: NumberOfParamsForBinaryOperator } }; + methodSymbol is { MethodKind: MethodKind.BuiltinOperator or MethodKind.UserDefinedOperator, Name: "op_Subtraction", Parameters.Length: NumberOfParamsForBinaryOperator }; public static bool IsOperatorBinaryMultiply(this IMethodSymbol methodSymbol) => - methodSymbol is { MethodKind: MethodKind.UserDefinedOperator, Name: "op_Multiply", Parameters: { Length: NumberOfParamsForBinaryOperator } }; + methodSymbol is { MethodKind: MethodKind.BuiltinOperator or MethodKind.UserDefinedOperator, Name: "op_Multiply", Parameters.Length: NumberOfParamsForBinaryOperator }; public static bool IsOperatorBinaryDivide(this IMethodSymbol methodSymbol) => - methodSymbol is { MethodKind: MethodKind.UserDefinedOperator, Name: "op_Division", Parameters: { Length: NumberOfParamsForBinaryOperator } }; + methodSymbol is { MethodKind: MethodKind.BuiltinOperator or MethodKind.UserDefinedOperator, Name: "op_Division", Parameters.Length: NumberOfParamsForBinaryOperator }; public static bool IsOperatorBinaryModulus(this IMethodSymbol methodSymbol) => - methodSymbol is { MethodKind: MethodKind.UserDefinedOperator, Name: "op_Modulus", Parameters: { Length: NumberOfParamsForBinaryOperator } }; + methodSymbol is { MethodKind: MethodKind.BuiltinOperator or MethodKind.UserDefinedOperator, Name: "op_Modulus", Parameters.Length: NumberOfParamsForBinaryOperator }; public static bool IsOperatorEquals(this IMethodSymbol methodSymbol) => - methodSymbol is { MethodKind: MethodKind.UserDefinedOperator, Name: "op_Equality", Parameters: { Length: NumberOfParamsForBinaryOperator } }; + methodSymbol is { MethodKind: MethodKind.BuiltinOperator or MethodKind.UserDefinedOperator, Name: "op_Equality", Parameters.Length: NumberOfParamsForBinaryOperator }; public static bool IsOperatorNotEquals(this IMethodSymbol methodSymbol) => - methodSymbol is { MethodKind: MethodKind.UserDefinedOperator, Name: "op_Inequality", Parameters: { Length: NumberOfParamsForBinaryOperator } }; + methodSymbol is { MethodKind: MethodKind.BuiltinOperator or MethodKind.UserDefinedOperator, Name: "op_Inequality", Parameters.Length: NumberOfParamsForBinaryOperator }; public static bool IsConsoleWriteLine(this IMethodSymbol methodSymbol) => methodSymbol != null diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/EqualityOnFloatingPointTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/EqualityOnFloatingPointTest.cs index 8175193323a..acd28838156 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/EqualityOnFloatingPointTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/EqualityOnFloatingPointTest.cs @@ -20,27 +20,26 @@ using SonarAnalyzer.Rules.CSharp; -namespace SonarAnalyzer.UnitTest.Rules +namespace SonarAnalyzer.UnitTest.Rules; + +[TestClass] +public class EqualityOnFloatingPointTest { - [TestClass] - public class EqualityOnFloatingPointTest - { - private readonly VerifierBuilder builder = new VerifierBuilder(); + private readonly VerifierBuilder builder = new VerifierBuilder(); - [TestMethod] - public void EqualityOnFloatingPoint() => - builder.AddPaths("EqualityOnFloatingPoint.cs").Verify(); + [TestMethod] + public void EqualityOnFloatingPoint() => + builder.AddPaths("EqualityOnFloatingPoint.cs").Verify(); #if NET - [TestMethod] - public void EqualityOnFloatingPoint_CSharp11() => - builder.AddPaths("EqualityOnFloatingPoint.CSharp11.cs") - .AddReferences(new[] { CoreMetadataReference.SystemRuntime }) - .WithOptions(ParseOptionsHelper.FromCSharp11) - .Verify(); + [TestMethod] + public void EqualityOnFloatingPoint_CSharp11() => + builder.AddPaths("EqualityOnFloatingPoint.CSharp11.cs") + .AddReferences(new[] { CoreMetadataReference.SystemRuntime }) + .WithOptions(ParseOptionsHelper.FromCSharp11) + .Verify(); #endif - } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/EqualityOnFloatingPoint.CSharp11.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/EqualityOnFloatingPoint.CSharp11.cs index cced735e821..1bde8f94b44 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/EqualityOnFloatingPoint.CSharp11.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/EqualityOnFloatingPoint.CSharp11.cs @@ -4,6 +4,25 @@ public class EqualityOnFloatingPoint { + void HalfNaN(Half h) + { + _ = h == Half.NaN; // Noncompliant {{Do not check floating point equality with exact values, use 'Half.IsNaN()' instead.}} + // ^^ + h = Half.NaN; // Compliant, not a comparison + } + + void NFloatNaN(NFloat nf) + { + _ = nf == System.Runtime.InteropServices.NFloat.NaN; // Noncompliant {{Do not check floating point equality with exact values, use 'NFloat.IsNaN()' instead.}} + _ = nf == NFloat.NaN; // Noncompliant {{Do not check floating point equality with exact values, use 'NFloat.IsNaN()' instead.}} + } + + public void M(T t) where T : IFloatingPointIeee754 + { + if (t == T.NaN) { } // Noncompliant {{Do not check floating point equality with exact values, use 'T.IsNaN()' instead.}} + if (T.IsNaN(t)) { } // Compliant + } + bool HalfEqual(Half first, Half second) => first == second; // Noncompliant {{Do not check floating point equality with exact values, use a range instead.}} // ^^ diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/EqualityOnFloatingPoint.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/EqualityOnFloatingPoint.cs index c702d51a55c..8bb1069c079 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/EqualityOnFloatingPoint.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/EqualityOnFloatingPoint.cs @@ -2,86 +2,138 @@ using System.Collections.Generic; using System.Runtime.Versioning; -namespace Tests.Diagnostics +public class EqualityOnFloatingPoint { - public class EqualityOnFloatingPoint + void Test(float f, double d1, double d2, dynamic dyn) { - void test(float f, double d1, double d2) + if (dyn == 3.14) { } // FN. {{Do not check floating point equality with exact values, use a range instead.}} + if (f == 3.14F) { } // Noncompliant {{Do not check floating point equality with exact values, use a range instead.}} + // ^^ + if (f != 3.14F) { } // Noncompliant {{Do not check floating point inequality with exact values, use a range instead.}} + if (d1 == d2) { } // Noncompliant + if (d1 < 0 || d1 > 1) { } // Compliant no indirect inequality test + + var b = d1 == 3.14; // Noncompliant + if (true && f >= 3.146) { } // Compliant no indirect equality test + if (f <= 3.146 && ((f >= 3.146))) { } // Noncompliant indirect equality test + if (3.146 >= f && 3.146 <= f) { } // Noncompliant indirect equality test + if (f <= 3.146 && 3.146 <= f) { } // FN. Equivalent to the case above but not detected + if (3.146 >= f && 3.146 < f) { } // Compliant no indirect equality test + if (f <= 3.146 && f > 3.146) { } // Compliant no indirect equality test + + var i = 3; + if (i <= 3 && i >= 3) { } // Compliant: integer + if (i < 4 || i > 4) { } // Compliant: integer + if (f < 3.146 || f > 3.146) { } // Noncompliant indirect inequality test + if (3.146 > f || 3.146 < f) { } // Noncompliant indirect inequality test + if (3.146 > f || f > 3.146) { } // FN. Equivalent to the case above but not detected + if (f < 3.146 || f >= 3.146) { } // Compliant no indirect inequality + if (3.146 > f || 3.146 <= f) { } // Compliant no indirect inequality test + + if (f <= 3.146 && true && f >= 3.146) { } // Not recognized + if (Math.Sign(f) == 0) { } // Compliant + + float f1 = 0.0F; + if ((System.Math.Sign(f1) == 0)) { } // Compliant + } +} + +public class ReportSpecificMessage_NaN +{ + public void WithDoubleEquality(int iPar, double dPar, float fPar) + { + double d = 1.0; + + if (d == double.NaN) { } // Noncompliant {{Do not check floating point equality with exact values, use 'double.IsNaN()' instead.}} + _ = d == double.NaN; // Noncompliant {{Do not check floating point equality with exact values, use 'double.IsNaN()' instead.}} + _ = double.NaN == d; // Noncompliant {{Do not check floating point equality with exact values, use 'double.IsNaN()' instead.}} + _ = double.NaN == double.NaN; // Noncompliant {{Do not check floating point equality with exact values, use 'double.IsNaN()' instead.}} + _ = iPar == double.NaN; // Noncompliant {{Do not check floating point equality with exact values, use 'double.IsNaN()' instead.}} + _ = dPar == double.NaN; // Noncompliant {{Do not check floating point equality with exact values, use 'double.IsNaN()' instead.}} + _ = fPar == double.NaN; // Noncompliant {{Do not check floating point equality with exact values, use 'double.IsNaN()' instead.}} + _ = float.NaN == double.NaN; // Noncompliant {{Do not check floating point equality with exact values, use 'double.IsNaN()' instead.}} + _ = double.NaN == float.NaN; // Noncompliant {{Do not check floating point equality with exact values, use 'double.IsNaN()' instead.}} + } + + public void WithDoubleInequality(int iPar, double dPar, float fPar) + { + double d = 1.0; + + if (d != double.NaN) { } // Noncompliant {{Do not check floating point inequality with exact values, use 'double.IsNaN()' instead.}} + _ = d != double.NaN; // Noncompliant {{Do not check floating point inequality with exact values, use 'double.IsNaN()' instead.}} + _ = double.NaN != d; // Noncompliant {{Do not check floating point inequality with exact values, use 'double.IsNaN()' instead.}} + _ = double.NaN != double.NaN; // Noncompliant {{Do not check floating point inequality with exact values, use 'double.IsNaN()' instead.}} + _ = iPar != double.NaN; // Noncompliant {{Do not check floating point inequality with exact values, use 'double.IsNaN()' instead.}} + _ = dPar != double.NaN; // Noncompliant {{Do not check floating point inequality with exact values, use 'double.IsNaN()' instead.}} + _ = fPar != double.NaN; // Noncompliant {{Do not check floating point inequality with exact values, use 'double.IsNaN()' instead.}} + _ = float.NaN != double.NaN; // Noncompliant {{Do not check floating point inequality with exact values, use 'double.IsNaN()' instead.}} + _ = double.NaN != float.NaN; // Noncompliant {{Do not check floating point inequality with exact values, use 'double.IsNaN()' instead.}} + } + + public void WithFloat(int iPar, float fPar) + { + float f = 1.0f; + _ = f == float.NaN; // Noncompliant {{Do not check floating point equality with exact values, use 'float.IsNaN()' instead.}} + _ = f != float.NaN; // Noncompliant {{Do not check floating point inequality with exact values, use 'float.IsNaN()' instead.}} + _ = iPar == float.NaN; // Noncompliant {{Do not check floating point equality with exact values, use 'float.IsNaN()' instead.}} + _ = fPar == float.NaN; // Noncompliant {{Do not check floating point equality with exact values, use 'float.IsNaN()' instead.}} + _ = iPar != float.NaN; // Noncompliant {{Do not check floating point inequality with exact values, use 'float.IsNaN()' instead.}} + _ = fPar != float.NaN; // Noncompliant {{Do not check floating point inequality with exact values, use 'float.IsNaN()' instead.}} + } + + public void WithDoublePascalCase() + { + Double d = 1.0; + _ = d == Double.NaN; // Noncompliant {{Do not check floating point equality with exact values, use 'double.IsNaN()' instead.}} + _ = d == System.Double.NaN; // Noncompliant {{Do not check floating point equality with exact values, use 'double.IsNaN()' instead.}} + _ = d != Double.NaN; // Noncompliant {{Do not check floating point inequality with exact values, use 'double.IsNaN()' instead.}} + _ = d != System.Double.NaN; // Noncompliant {{Do not check floating point inequality with exact values, use 'double.IsNaN()' instead.}} + } + + public void WithSingle() + { + Single f = 3.14159f; + _ = f == Single.NaN; // Noncompliant {{Do not check floating point equality with exact values, use 'float.IsNaN()' instead.}} + _ = f == System.Single.NaN; // Noncompliant {{Do not check floating point equality with exact values, use 'float.IsNaN()' instead.}} + _ = f != Single.NaN; // Noncompliant {{Do not check floating point inequality with exact values, use 'float.IsNaN()' instead.}} + _ = f != System.Single.NaN; // Noncompliant {{Do not check floating point inequality with exact values, use 'float.IsNaN()' instead.}} + } +} + +namespace TestsWithTypeAliases +{ + using DoubleAlias = Double; + + public class ReportSpecificMessage + { + public void WithDoubleAlias() + { + DoubleAlias d = 1.674927471E-27; + _ = d == DoubleAlias.NaN; // Noncompliant {{Do not check floating point equality with exact values, use 'DoubleAlias.IsNaN()' instead.}} + if (d == double.NaN) { } // Noncompliant {{Do not check floating point equality with exact values, use 'DoubleAlias.IsNaN()' instead.}} + } + } +} + +namespace TestWithUsingStatic +{ + using static System.Double; + + public class ReportSpecificMessage + { + public void WithUsingStatic(double d) + { + _ = d == NaN; // Noncompliant {{Do not check floating point equality with exact values, use 'IsNaN()' instead.}} + _ = NaN == d; // Noncompliant {{Do not check floating point equality with exact values, use 'IsNaN()' instead.}} + _ = NaN == NaN; // Noncompliant {{Do not check floating point equality with exact values, use 'IsNaN()' instead.}} + _ = NaN == float.NaN; // Noncompliant {{Do not check floating point equality with exact values, use 'double.IsNaN()' instead.}} + _ = double.NaN == NaN; // Noncompliant {{Do not check floating point equality with exact values, use 'double.IsNaN()' instead.}} + } + + public void WithLocalVar(double d) { - dynamic din = null; - if (din == null) - { - } - - if (f == 3.14F) //Noncompliant {{Do not check floating point equality with exact values, use a range instead.}} -// ^^ - { - } - - if (f != 3.14F) //Noncompliant {{Do not check floating point inequality with exact values, use a range instead.}} - { } - - if (d1 == d2) //Noncompliant - { } - - var b = d1 == 3.14; //Noncompliant - - if (true && f >= 3.146) - { - } - - if (f <= 3.146 && ((f >= 3.146))) // Noncompliant indirect equality test - { - } - if (3.146 >= f && 3.146 <= f) // Noncompliant indirect equality test - { - } - if (f <= 3.146 && 3.146 <= f) // FN. Equivalent to the case above but not detected - { - } - if (3.146 >= f && 3.146 < f) // Compliant no indirect equality test - { - } - if (f <= 3.146 && f > 3.146) // Compliant no indirect equality test - { - } - var i = 3; - if (i <= 3 && i >= 3) - { - } - - if (i < 4 || i > 4) - { - } - - if (f < 3.146 || f > 3.146) // Noncompliant indirect inequality test - { - } - if (3.146 > f || 3.146 < f) // Noncompliant indirect inequality test - { - } - if (3.146 > f || f > 3.146) // FN. Equivalent to the case above but not detected - { - } - if (f < 3.146 || f >= 3.146) // Compliant no indirect inequality test - { - } - if (3.146 > f || 3.146 <= f) // Compliant no indirect inequality test - { - } - - if (f <= 3.146 && true && f >= 3.146) // Not recognized - { - } - - if (Math.Sign(f) == 0) - { - } - - float f1 = 0.0F; - if ((System.Math.Sign(f1) == 0)) - { - } + var NaN = 5; + _ = d == NaN; // Noncompliant {{Do not check floating point equality with exact values, use a range instead.}} } } }