From e76d32a8aa4f9cdd1537146fb272ab6edd0f63bd Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Wed, 1 Feb 2023 10:11:56 +0100 Subject: [PATCH 01/28] Update RSPEC for S2198 --- .../cs/MISRA_C++_2008_recommended_profile.json | 6 ++++++ analyzers/rspec/cs/S2198_c#.html | 18 ++++++++++++++++++ analyzers/rspec/cs/S2198_c#.json | 17 +++++++++++++++++ analyzers/rspec/cs/Sonar_way_profile.json | 1 + .../PackagingTests/RuleTypeMappingCS.cs | 2 +- 5 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 analyzers/rspec/cs/MISRA_C++_2008_recommended_profile.json create mode 100644 analyzers/rspec/cs/S2198_c#.html create mode 100644 analyzers/rspec/cs/S2198_c#.json diff --git a/analyzers/rspec/cs/MISRA_C++_2008_recommended_profile.json b/analyzers/rspec/cs/MISRA_C++_2008_recommended_profile.json new file mode 100644 index 00000000000..8d8e08e9951 --- /dev/null +++ b/analyzers/rspec/cs/MISRA_C++_2008_recommended_profile.json @@ -0,0 +1,6 @@ +{ + "name": "MISRA C++ 2008 recommended", + "ruleKeys": [ + "S2198" + ] +} diff --git a/analyzers/rspec/cs/S2198_c#.html b/analyzers/rspec/cs/S2198_c#.html new file mode 100644 index 00000000000..d0d070dc92b --- /dev/null +++ b/analyzers/rspec/cs/S2198_c#.html @@ -0,0 +1,18 @@ +

Certain mathematical comparisons will always return the same value, and should simply not be made.

+

This comparison will always return false:

+ +

These will always return true:

+ +

Noncompliant Code Example

+
+float f = 42.0f;
+const double d = float.MaxValue + 1;
+if (f <= d) { } // Noncompliant
+
+ diff --git a/analyzers/rspec/cs/S2198_c#.json b/analyzers/rspec/cs/S2198_c#.json new file mode 100644 index 00000000000..d7e7b900946 --- /dev/null +++ b/analyzers/rspec/cs/S2198_c#.json @@ -0,0 +1,17 @@ +{ + "title": "Silly mathematical comparisons should not be made", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "15min" + }, + "tags": [ + "suspicious" + ], + "defaultSeverity": "Critical", + "ruleSpecification": "RSPEC-2198", + "sqKey": "S2198", + "scope": "Main", + "quickfix": "unknown" +} diff --git a/analyzers/rspec/cs/Sonar_way_profile.json b/analyzers/rspec/cs/Sonar_way_profile.json index d16b8c34168..fc2b8e583f2 100644 --- a/analyzers/rspec/cs/Sonar_way_profile.json +++ b/analyzers/rspec/cs/Sonar_way_profile.json @@ -66,6 +66,7 @@ "S2184", "S2187", "S2190", + "S2198", "S2201", "S2219", "S2222", diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs b/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs index b85c5fa1f6b..e560a541143 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs @@ -2122,7 +2122,7 @@ internal static class RuleTypeMappingCS // ["S2195"], // ["S2196"], ["S2197"] = "CODE_SMELL", - // ["S2198"], + ["S2198"] = "CODE_SMELL", // ["S2199"], // ["S2200"], ["S2201"] = "BUG", From 93391b2eada398747132b80fb9cff2f037bceefc Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Wed, 1 Feb 2023 10:26:26 +0100 Subject: [PATCH 02/28] Remove a useless auto-generated json --- analyzers/rspec/cs/MISRA_C++_2008_recommended_profile.json | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 analyzers/rspec/cs/MISRA_C++_2008_recommended_profile.json diff --git a/analyzers/rspec/cs/MISRA_C++_2008_recommended_profile.json b/analyzers/rspec/cs/MISRA_C++_2008_recommended_profile.json deleted file mode 100644 index 8d8e08e9951..00000000000 --- a/analyzers/rspec/cs/MISRA_C++_2008_recommended_profile.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "name": "MISRA C++ 2008 recommended", - "ruleKeys": [ - "S2198" - ] -} From ececfa2e4055f16879c1b18720fcc496bc83a0fc Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Wed, 1 Feb 2023 16:57:17 +0100 Subject: [PATCH 03/28] Add some testcases --- .../Rules/SillyMathematicalComparison.cs | 35 +++++++++ .../TestCases/SillyMathematicalComparison.cs | 75 +++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/Rules/SillyMathematicalComparison.cs create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SillyMathematicalComparison.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SillyMathematicalComparison.cs new file mode 100644 index 00000000000..2cb1e7b8463 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SillyMathematicalComparison.cs @@ -0,0 +1,35 @@ +/* + * SonarAnalyzer for .NET + * Copyright (C) 2015-2023 SonarSource SA + * mailto: contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +using CS = SonarAnalyzer.Rules.CSharp; +using VB = SonarAnalyzer.Rules.VisualBasic; + +namespace SonarAnalyzer.UnitTest.Rules +{ + [TestClass] + public class SillyMathematicalComparisonTest + { + private readonly VerifierBuilder builderCS = new VerifierBuilder(); + + [TestMethod] + public void SillyMathematicalComparison_CS() => + builderCS.AddPaths("SillyMathematicalComparison.cs").Verify(); + } +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs new file mode 100644 index 00000000000..c346200b61a --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs @@ -0,0 +1,75 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace SonarAnalyzer.UnitTest.TestCases +{ + internal class SillyMathematicalComparison + { + public void Bytes() + { + //const sbyte smallSByte = byte.MinValue; + //const sbyte bigSByte = byte.MaxValue; + + const byte smallByte = byte.MinValue; + const byte bigByte = byte.MaxValue; + + const short smallShort = byte.MinValue; + const short bigShort = byte.MaxValue; + + const ushort smallUShort = byte.MinValue; + const ushort bigUShort = byte.MaxValue; + + const int smallInt = byte.MinValue; + const int bigInt = byte.MaxValue; + + const uint smallUInt = byte.MinValue; + const uint bigUInt = byte.MaxValue; + + const long smallLong = byte.MinValue; + const long bigLong = byte.MaxValue; + + const ulong smallULong = byte.MinValue; + const ulong bigULong = byte.MaxValue; + + //const nint smallNint = byte.MinValue; + //const nint bigNint = byte.MaxValue; + + //const nuint smallNuint = byte.MinValue; + //const nuint bigNuint = byte.MaxValue; + + const float smallFloat = byte.MinValue; + const float bigFloat = byte.MaxValue; + + const double smallDouble = byte.MinValue; + const double bigDouble = byte.MaxValue; + + const decimal smallDecimal = byte.MinValue; + const decimal bigDecimal = byte.MaxValue; + + byte b = 42; + + _ = b <= bigByte; // Noncompliant + _ = b <= bigShort; // Noncompliant + _ = b >= smallByte; // Noncompliant + _ = b >= smallShort; // Noncompliant + + _ = bigByte >= b; // Noncompliant + _ = bigShort >= b; // Noncompliant + _ = smallByte <= b; // Noncompliant + _ = smallShort <= b; // Noncompliant + + _ = b >= bigByte; // Compliant, not (always true) or (always false) + _ = b >= bigShort; // Compliant, not (always true) or (always false) + _ = b <= smallByte; // Compliant, not (always true) or (always false) + _ = b <= smallShort; // Compliant, not (always true) or (always false) + + _ = bigByte <= b; // Compliant, not (always true) or (always false) + _ = bigShort <= b; // Compliant, not (always true) or (always false) + _ = smallByte >= b; // Compliant, not (always true) or (always false) + _ = smallShort >= b; // Compliant, not (always true) or (always false) + } + } +} From 46f3d70c41bcc34e87261c5e44382746c8b16665 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Wed, 1 Feb 2023 17:00:13 +0100 Subject: [PATCH 04/28] Remove a useless using --- .../Rules/SillyBitwiseOperationBase.cs | 35 +++++++++++++++---- .../Rules/SillyMathematicalComparison.cs | 1 - 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/SillyBitwiseOperationBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/SillyBitwiseOperationBase.cs index 73628e8b9d5..64a7863fdd7 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/SillyBitwiseOperationBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/SillyBitwiseOperationBase.cs @@ -20,19 +20,29 @@ namespace SonarAnalyzer.Rules { + // FIXME: Rename this to something like SillyNumericalOperatorUsage public abstract class SillyBitwiseOperationBase : SonarDiagnosticAnalyzer { - internal const string DiagnosticId = "S2437"; + internal const string BitwiseDiagnosticId = "S2437"; internal const string IsReportingOnLeftKey = "IsReportingOnLeft"; - private const string MessageFormat = "Remove this silly bit operation."; + private const string BitwiseMessageFormat = "Remove this silly bit operation."; + + // No codefix (yet?) + private const string ComparisonDiagnosticId = "S2198"; + private const string ComparisonMessageFormat = "Remove this silly mathematical comparison."; protected abstract ILanguageFacade Language { get; } - public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); - protected DiagnosticDescriptor Rule { get; } + protected DiagnosticDescriptor BitwiseRule { get; } + protected DiagnosticDescriptor ComparisonRule { get; } + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(BitwiseRule, ComparisonRule); - protected SillyBitwiseOperationBase() => - Rule = Language.CreateDescriptor(DiagnosticId, MessageFormat, fadeOutCode: true); + protected SillyBitwiseOperationBase() + { + BitwiseRule = Language.CreateDescriptor(BitwiseDiagnosticId, BitwiseMessageFormat, fadeOutCode: true); + ComparisonRule = Language.CreateDescriptor(ComparisonDiagnosticId, ComparisonMessageFormat, fadeOutCode: true); + } protected void CheckBinary(SonarSyntaxNodeReportingContext context, SyntaxNode left, SyntaxToken @operator, SyntaxNode right, int constValueToLookFor) { @@ -53,7 +63,7 @@ protected void CheckBinary(SonarSyntaxNodeReportingContext context, SyntaxNode l return; } - context.ReportIssue(Diagnostic.Create(Rule, location, ImmutableDictionary.Empty.Add(IsReportingOnLeftKey, isReportingOnLeftKey.ToString()))); + context.ReportIssue(Diagnostic.Create(BitwiseRule, location, ImmutableDictionary.Empty.Add(IsReportingOnLeftKey, isReportingOnLeftKey.ToString()))); } protected int? FindIntConstant(SemanticModel semanticModel, SyntaxNode node) => @@ -64,6 +74,17 @@ protected void CheckBinary(SonarSyntaxNodeReportingContext context, SyntaxNode l ? ConversionHelper.TryConvertToInt(value) : null; + protected T? FindConstant(SemanticModel semanticModel, SyntaxNode node, Func converter) where T : struct + { + return semanticModel.GetSymbolInfo(node).Symbol is var symbol + && !IsFieldOrPropertyOutsideSystemNamespace(symbol) + && !IsEnum(symbol) + && Language.FindConstantValue(semanticModel, node) is { } value + && ConversionHelper.TryConvertWith(value, converter, out T typedValue) + ? typedValue + : null; + } + private static bool IsEnum(ISymbol symbol) => symbol.GetSymbolType() is INamedTypeSymbol { EnumUnderlyingType: { } }; diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SillyMathematicalComparison.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SillyMathematicalComparison.cs index 2cb1e7b8463..d51615b280b 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SillyMathematicalComparison.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SillyMathematicalComparison.cs @@ -19,7 +19,6 @@ */ using CS = SonarAnalyzer.Rules.CSharp; -using VB = SonarAnalyzer.Rules.VisualBasic; namespace SonarAnalyzer.UnitTest.Rules { From 5bdd084d0abd6a5db7ccdb5508e5b6a022436f8e Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Wed, 1 Feb 2023 17:00:29 +0100 Subject: [PATCH 05/28] Initial implementation of the main logic --- .../Rules/SillyBitwiseOperation.cs | 57 ++++++++++++++++++- .../Rules/SillyBitwiseOperationCodeFixBase.cs | 2 +- 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs index 5b91a764a9f..3e87fa61d20 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs @@ -18,6 +18,8 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +using Microsoft.CodeAnalysis; + namespace SonarAnalyzer.Rules.CSharp { [DiagnosticAnalyzer(LanguageNames.CSharp)] @@ -27,6 +29,7 @@ public sealed class SillyBitwiseOperation : SillyBitwiseOperationBase protected override void Initialize(SonarAnalysisContext context) { + // FIRST RULE context.RegisterNodeAction( c => CheckBinary(c, -1), SyntaxKind.BitwiseAndExpression); @@ -44,6 +47,15 @@ protected override void Initialize(SonarAnalysisContext context) c => CheckAssignment(c, 0), SyntaxKind.OrAssignmentExpression, SyntaxKind.ExclusiveOrAssignmentExpression); + + // SECOND RULE + context.RegisterNodeAction( + CheckLessOrEqualExpression, + SyntaxKind.LessThanOrEqualExpression); + + context.RegisterNodeAction( + CheckGreaterOrEqualExpression, + SyntaxKind.GreaterThanOrEqualExpression); } private void CheckAssignment(SonarSyntaxNodeReportingContext context, int constValueToLookFor) @@ -55,7 +67,7 @@ private void CheckAssignment(SonarSyntaxNodeReportingContext context, int constV var location = assignment.Parent is StatementSyntax ? assignment.Parent.GetLocation() : assignment.OperatorToken.CreateLocation(assignment.Right); - context.ReportIssue(Diagnostic.Create(Rule, location)); + context.ReportIssue(Diagnostic.Create(BitwiseRule, location)); } } @@ -64,5 +76,48 @@ private void CheckBinary(SonarSyntaxNodeReportingContext context, int constValue var binary = (BinaryExpressionSyntax)context.Node; CheckBinary(context, binary.Left, binary.OperatorToken, binary.Right, constValueToLookFor); } + + private void CheckLessOrEqualExpression(SonarSyntaxNodeReportingContext context) // x <= y + { + var binary = (BinaryExpressionSyntax)context.Node; + + if (ShouldRaise(context.SemanticModel, binary.Left, binary.Right, x => x.MinValue)) // minConstant <= actualValue + { + context.ReportIssue(Diagnostic.Create(ComparisonRule, binary.GetLocation())); + } + else if (ShouldRaise(context.SemanticModel, binary.Right, binary.Left, x => x.MaxValue)) // actualValue <= maxConstant + { + context.ReportIssue(Diagnostic.Create(ComparisonRule, binary.GetLocation())); + } + } + + private void CheckGreaterOrEqualExpression(SonarSyntaxNodeReportingContext context) // x >= y + { + var binary = (BinaryExpressionSyntax)context.Node; + + if (ShouldRaise(context.SemanticModel, binary.Right, binary.Left, x => x.MinValue)) // actualValue >= minConstant + { + context.ReportIssue(Diagnostic.Create(ComparisonRule, binary.GetLocation())); + } + else if (ShouldRaise(context.SemanticModel, binary.Left, binary.Right, x => x.MaxValue)) // maxConstant >= actualValue + { + context.ReportIssue(Diagnostic.Create(ComparisonRule, binary.GetLocation())); + } + } + + private bool ShouldRaise(SemanticModel semanticModel, SyntaxNode expectedConstant, SyntaxNode other, Func getSide) => + FindConstant(semanticModel, expectedConstant, Convert.ToDouble) is { } constant + && semanticModel.GetSymbolInfo(other).Symbol.GetSymbolType() is { } symbol + && Ranges.FirstOrDefault(x => symbol.Is(x.Type)) is { } typeMetadata + && constant == getSide(typeMetadata); + + private Thingy[] Ranges = new[] + { + new Thingy(KnownType.System_Byte, byte.MinValue, byte.MaxValue), + new Thingy(KnownType.System_Int32, int.MinValue, int.MaxValue), + new Thingy(KnownType.System_Int64, long.MinValue, long.MaxValue), + }; + + private record struct Thingy(KnownType Type, double MinValue, double MaxValue); } } diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/SillyBitwiseOperationCodeFixBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/SillyBitwiseOperationCodeFixBase.cs index f60002a146a..e1b6a4bd135 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/SillyBitwiseOperationCodeFixBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/SillyBitwiseOperationCodeFixBase.cs @@ -30,7 +30,7 @@ public abstract class SillyBitwiseOperationCodeFixBase : SonarCodeFix protected abstract Func CreateNewRoot(SyntaxNode root, TextSpan diagnosticSpan, bool isReportingOnLeft); - public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(SillyBitwiseOperationBase.DiagnosticId); + public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(SillyBitwiseOperationBase.BitwiseDiagnosticId); protected override Task RegisterCodeFixesAsync(SyntaxNode root, CodeFixContext context) { From fee626d29112b5eedef1ff73d928a211a4da5b0f Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Thu, 2 Feb 2023 14:02:31 +0100 Subject: [PATCH 06/28] Initial working implementation of S2198 --- .../Rules/SillyBitwiseOperation.cs | 115 +++++++++++++----- .../TestCases/SillyMathematicalComparison.cs | 54 +++++++- 2 files changed, 133 insertions(+), 36 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs index 3e87fa61d20..6adf5657552 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs @@ -29,7 +29,6 @@ public sealed class SillyBitwiseOperation : SillyBitwiseOperationBase protected override void Initialize(SonarAnalysisContext context) { - // FIRST RULE context.RegisterNodeAction( c => CheckBinary(c, -1), SyntaxKind.BitwiseAndExpression); @@ -48,14 +47,12 @@ protected override void Initialize(SonarAnalysisContext context) SyntaxKind.OrAssignmentExpression, SyntaxKind.ExclusiveOrAssignmentExpression); - // SECOND RULE context.RegisterNodeAction( - CheckLessOrEqualExpression, - SyntaxKind.LessThanOrEqualExpression); - - context.RegisterNodeAction( - CheckGreaterOrEqualExpression, - SyntaxKind.GreaterThanOrEqualExpression); + CheckComparison, + SyntaxKind.GreaterThanExpression, + SyntaxKind.GreaterThanOrEqualExpression, + SyntaxKind.LessThanExpression, + SyntaxKind.LessThanOrEqualExpression); } private void CheckAssignment(SonarSyntaxNodeReportingContext context, int constValueToLookFor) @@ -77,47 +74,105 @@ private void CheckBinary(SonarSyntaxNodeReportingContext context, int constValue CheckBinary(context, binary.Left, binary.OperatorToken, binary.Right, constValueToLookFor); } - private void CheckLessOrEqualExpression(SonarSyntaxNodeReportingContext context) // x <= y + private void CheckComparison(SonarSyntaxNodeReportingContext context) { var binary = (BinaryExpressionSyntax)context.Node; - if (ShouldRaise(context.SemanticModel, binary.Left, binary.Right, x => x.MinValue)) // minConstant <= actualValue + var normalComparer = GetNormalComparer(binary.Kind()); + var inverseComparer = GetReverseComparer(binary.Kind()); + + if (ShouldRaise(context.SemanticModel, binary.Left, binary.Right, normalComparer)) // x >= constant { context.ReportIssue(Diagnostic.Create(ComparisonRule, binary.GetLocation())); } - else if (ShouldRaise(context.SemanticModel, binary.Right, binary.Left, x => x.MaxValue)) // actualValue <= maxConstant + else if (ShouldRaise(context.SemanticModel, binary.Right, binary.Left, inverseComparer)) // constant >= x inversed: x <= constant { context.ReportIssue(Diagnostic.Create(ComparisonRule, binary.GetLocation())); } } - private void CheckGreaterOrEqualExpression(SonarSyntaxNodeReportingContext context) // x >= y - { - var binary = (BinaryExpressionSyntax)context.Node; - - if (ShouldRaise(context.SemanticModel, binary.Right, binary.Left, x => x.MinValue)) // actualValue >= minConstant + private Func GetNormalComparer(SyntaxKind kind) => + kind switch { - context.ReportIssue(Diagnostic.Create(ComparisonRule, binary.GetLocation())); - } - else if (ShouldRaise(context.SemanticModel, binary.Left, binary.Right, x => x.MaxValue)) // maxConstant >= actualValue + SyntaxKind.LessThanExpression => (tm, constant) => tm.Less(constant), + SyntaxKind.LessThanOrEqualExpression => (tm, constant) => tm.LessOrEqual(constant), + SyntaxKind.GreaterThanExpression => (tm, constant) => tm.Greater(constant), + SyntaxKind.GreaterThanOrEqualExpression => (tm, constant) => tm.GreaterOrEqual(constant), + }; + + private Func GetReverseComparer(SyntaxKind kind) => + kind switch { - context.ReportIssue(Diagnostic.Create(ComparisonRule, binary.GetLocation())); - } - } + SyntaxKind.LessThanExpression => (tm, constant) => tm.Greater(constant), + SyntaxKind.LessThanOrEqualExpression => (tm, constant) => tm.GreaterOrEqual(constant), + SyntaxKind.GreaterThanExpression => (tm, constant) => tm.Less(constant), + SyntaxKind.GreaterThanOrEqualExpression => (tm, constant) => tm.LessOrEqual(constant), + }; - private bool ShouldRaise(SemanticModel semanticModel, SyntaxNode expectedConstant, SyntaxNode other, Func getSide) => + private bool ShouldRaise(SemanticModel semanticModel, SyntaxNode value, SyntaxNode expectedConstant, Func checkConstant) => FindConstant(semanticModel, expectedConstant, Convert.ToDouble) is { } constant - && semanticModel.GetSymbolInfo(other).Symbol.GetSymbolType() is { } symbol + && semanticModel.GetSymbolInfo(value).Symbol.GetSymbolType() is { } symbol && Ranges.FirstOrDefault(x => symbol.Is(x.Type)) is { } typeMetadata - && constant == getSide(typeMetadata); + && checkConstant(typeMetadata, constant); - private Thingy[] Ranges = new[] + private static readonly TypeMetadata[] Ranges = new TypeMetadata[] // FIXME: Add more types here { - new Thingy(KnownType.System_Byte, byte.MinValue, byte.MaxValue), - new Thingy(KnownType.System_Int32, int.MinValue, int.MaxValue), - new Thingy(KnownType.System_Int64, long.MinValue, long.MaxValue), + new LimitedFunctionalityTypeMetadata(KnownType.System_Byte, byte.MinValue, byte.MaxValue), + new LimitedFunctionalityTypeMetadata(KnownType.System_Int32, int.MinValue, int.MaxValue), + new LimitedFunctionalityTypeMetadata(KnownType.System_Int64, long.MinValue, long.MaxValue), + new FullFunctionalityTypeMetadata(KnownType.System_Single, long.MinValue, long.MaxValue), }; - private record struct Thingy(KnownType Type, double MinValue, double MaxValue); + private abstract class TypeMetadata + { + public KnownType Type { get; init; } + public double MinValue { get; init; } + public double MaxValue { get; init; } + + protected TypeMetadata(KnownType type, double minValue, double maxValue) + { + Type = type; + MinValue = minValue; + MaxValue = maxValue; + } + + public virtual bool Less(double constant) => false; + public virtual bool Greater(double constant) => false; + + public abstract bool LessOrEqual(double constant); + public abstract bool GreaterOrEqual(double constant); + } + + private class LimitedFunctionalityTypeMetadata : TypeMetadata // For types that are already semi-covered by CS0652 + { + public LimitedFunctionalityTypeMetadata(KnownType type, double minValue, double maxValue) + : base(type, minValue, maxValue) + { } + + public override bool GreaterOrEqual(double constant) => // something >= constant + constant == MinValue; + + public override bool LessOrEqual(double constant) => // something <= constant + constant == MaxValue; + } + + private class FullFunctionalityTypeMetadata : TypeMetadata // For types that are not covered by CS0652 + { + public FullFunctionalityTypeMetadata(KnownType type, double minValue, double maxValue) + : base(type, minValue, maxValue) + { } + + public override bool Greater(double constant) => // something > constant + constant < MinValue; + + public override bool Less(double constant) => // something < constant + constant > MaxValue; + + public override bool GreaterOrEqual(double constant) => // something >= constant + constant <= MinValue; + + public override bool LessOrEqual(double constant) => // something <= constant + constant >= MaxValue; + } } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs index c346200b61a..a814b39ba9e 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs @@ -34,12 +34,6 @@ public void Bytes() const ulong smallULong = byte.MinValue; const ulong bigULong = byte.MaxValue; - //const nint smallNint = byte.MinValue; - //const nint bigNint = byte.MaxValue; - - //const nuint smallNuint = byte.MinValue; - //const nuint bigNuint = byte.MaxValue; - const float smallFloat = byte.MinValue; const float bigFloat = byte.MaxValue; @@ -70,6 +64,54 @@ public void Bytes() _ = bigShort <= b; // Compliant, not (always true) or (always false) _ = smallByte >= b; // Compliant, not (always true) or (always false) _ = smallShort >= b; // Compliant, not (always true) or (always false) + + const int veryBig = byte.MaxValue + 1; + const int verySmall = byte.MinValue - 1; + + _ = b < veryBig; // Compliant, raised by CS0652 + _ = veryBig > b; // Compliant, raised by CS0652 + _ = b > verySmall; // Compliant, raised by CS0652 + _ = verySmall < b; // Compliant, raised by CS0652 + } + + public void Floats() + { + const float smallFloat = float.MinValue; + const float bigFloat = float.MaxValue; + + const double smallDouble = float.MinValue; + const double bigDouble = float.MaxValue; + + const double veryBig = float.MaxValue + 1; + const double verySmall = float.MinValue - 1; + + float f = 42; + + _ = f <= bigFloat; // Noncompliant + _ = f <= bigDouble; // Noncompliant + _ = f >= smallFloat; // Noncompliant + _ = f >= smallDouble; // Noncompliant + + _ = bigFloat >= f; // Noncompliant + _ = bigDouble >= f; // Noncompliant + _ = smallFloat <= f; // Noncompliant + _ = smallDouble <= f; // Noncompliant + + _ = f >= bigFloat; // Compliant, not (always true) or (always false) + _ = f >= bigDouble; // Compliant, not (always true) or (always false) + _ = f <= smallFloat; // Compliant, not (always true) or (always false) + _ = f <= smallDouble; // Compliant, not (always true) or (always false) + + _ = bigFloat <= f; // Compliant, not (always true) or (always false) + _ = bigDouble <= f; // Compliant, not (always true) or (always false) + _ = smallFloat >= f; // Compliant, not (always true) or (always false) + _ = smallDouble >= f; // Compliant, not (always true) or (always false) + + _ = f < veryBig; // Noncompliant + _ = veryBig > f; // Noncompliant + _ = f > verySmall; // Noncompliant + _ = verySmall < f; // Noncompliant } + } } From 36214a278235b4faa1051b427facdf9257cf869f Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Thu, 2 Feb 2023 17:44:04 +0100 Subject: [PATCH 07/28] Refactor implementation --- .../Rules/SillyBitwiseOperation.cs | 142 +++++++++--------- .../TestCases/SillyMathematicalComparison.cs | 4 +- 2 files changed, 70 insertions(+), 76 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs index 6adf5657552..d793ea14344 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs @@ -18,6 +18,7 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +using System.Numerics; using Microsoft.CodeAnalysis; namespace SonarAnalyzer.Rules.CSharp @@ -77,102 +78,95 @@ private void CheckBinary(SonarSyntaxNodeReportingContext context, int constValue private void CheckComparison(SonarSyntaxNodeReportingContext context) { var binary = (BinaryExpressionSyntax)context.Node; + if (ShouldRaise(context.SemanticModel, binary.Left, binary.Right, binary.Kind(), false)) + { + context.ReportIssue(Diagnostic.Create(BitwiseRule, binary.GetLocation())); + } - var normalComparer = GetNormalComparer(binary.Kind()); - var inverseComparer = GetReverseComparer(binary.Kind()); - - if (ShouldRaise(context.SemanticModel, binary.Left, binary.Right, normalComparer)) // x >= constant + if (ShouldRaise(context.SemanticModel, binary.Right, binary.Left, binary.Kind(), true)) { - context.ReportIssue(Diagnostic.Create(ComparisonRule, binary.GetLocation())); + context.ReportIssue(Diagnostic.Create(BitwiseRule, binary.GetLocation())); } - else if (ShouldRaise(context.SemanticModel, binary.Right, binary.Left, inverseComparer)) // constant >= x inversed: x <= constant + } + + private bool ShouldRaise(SemanticModel semanticModel, SyntaxNode value, SyntaxNode expectedConstant, SyntaxKind kind, bool constantIsLeft) + { + if (FindConstant(semanticModel, expectedConstant, Convert.ToDouble) is { } constant + && semanticModel.GetSymbolInfo(value).Symbol.GetSymbolType() is { } symbol + && Ranges.FirstOrDefault(x => symbol.Is(x.Type)) is { } range) { - context.ReportIssue(Diagnostic.Create(ComparisonRule, binary.GetLocation())); + // Implement out-of-range checks for the types that CS0652 does not. + if (symbol.IsAny(FullyImplementedTypes) && (constant < range.MinValue || constant > range.MaxValue)) + { + return true; + } + + // Implement threshold checks for every type. + return GetThreshold(kind, constantIsLeft, range) is { } threshold && constant == threshold; } + + return false; } - private Func GetNormalComparer(SyntaxKind kind) => - kind switch + private static double? GetThreshold(SyntaxKind operation, bool constantIsLeft, TypeRange range) + { + if (constantIsLeft) { - SyntaxKind.LessThanExpression => (tm, constant) => tm.Less(constant), - SyntaxKind.LessThanOrEqualExpression => (tm, constant) => tm.LessOrEqual(constant), - SyntaxKind.GreaterThanExpression => (tm, constant) => tm.Greater(constant), - SyntaxKind.GreaterThanOrEqualExpression => (tm, constant) => tm.GreaterOrEqual(constant), - }; - - private Func GetReverseComparer(SyntaxKind kind) => - kind switch + return operation switch + { + SyntaxKind.LessThanExpression => range.MaxValue, // T.MaxValue < x + SyntaxKind.LessThanOrEqualExpression => range.MinValue, // T.MinValue <= x + SyntaxKind.GreaterThanExpression => range.MinValue, // T.MinValue > x + SyntaxKind.GreaterThanOrEqualExpression => range.MaxValue, // T.MaxValue >= x + _ => null + }; + } + else { - SyntaxKind.LessThanExpression => (tm, constant) => tm.Greater(constant), - SyntaxKind.LessThanOrEqualExpression => (tm, constant) => tm.GreaterOrEqual(constant), - SyntaxKind.GreaterThanExpression => (tm, constant) => tm.Less(constant), - SyntaxKind.GreaterThanOrEqualExpression => (tm, constant) => tm.LessOrEqual(constant), - }; - - private bool ShouldRaise(SemanticModel semanticModel, SyntaxNode value, SyntaxNode expectedConstant, Func checkConstant) => - FindConstant(semanticModel, expectedConstant, Convert.ToDouble) is { } constant - && semanticModel.GetSymbolInfo(value).Symbol.GetSymbolType() is { } symbol - && Ranges.FirstOrDefault(x => symbol.Is(x.Type)) is { } typeMetadata - && checkConstant(typeMetadata, constant); - - private static readonly TypeMetadata[] Ranges = new TypeMetadata[] // FIXME: Add more types here + + return operation switch + { + SyntaxKind.LessThanExpression => range.MinValue, // x < T.MinValue + SyntaxKind.LessThanOrEqualExpression => range.MaxValue, // x <= T.MaxValue + SyntaxKind.GreaterThanExpression => range.MaxValue, // x > T.MaxValue + SyntaxKind.GreaterThanOrEqualExpression => range.MinValue, // x >= T.MinValue + _ => null + }; + } + } + + private static readonly KnownType[] FullyImplementedTypes = new[] + { + KnownType.System_Half, + KnownType.System_Single, + }; + + private static readonly TypeRange[] Ranges = new[] { - new LimitedFunctionalityTypeMetadata(KnownType.System_Byte, byte.MinValue, byte.MaxValue), - new LimitedFunctionalityTypeMetadata(KnownType.System_Int32, int.MinValue, int.MaxValue), - new LimitedFunctionalityTypeMetadata(KnownType.System_Int64, long.MinValue, long.MaxValue), - new FullFunctionalityTypeMetadata(KnownType.System_Single, long.MinValue, long.MaxValue), + new TypeRange(KnownType.System_SByte, sbyte.MinValue, sbyte.MaxValue), + new TypeRange(KnownType.System_Byte, byte.MinValue, byte.MaxValue), + new TypeRange(KnownType.System_Int16, short.MinValue, short.MaxValue), + new TypeRange(KnownType.System_UInt16, ushort.MinValue, ushort.MaxValue), + new TypeRange(KnownType.System_Int32, int.MinValue, int.MaxValue), + new TypeRange(KnownType.System_UInt32, uint.MinValue, uint.MaxValue), + new TypeRange(KnownType.System_Int64, long.MinValue, long.MaxValue), + new TypeRange(KnownType.System_UInt64, ulong.MinValue, ulong.MaxValue), + new TypeRange(KnownType.System_Single, float.MinValue, float.MaxValue), + //new TypeRange(KnownType.System_Half, half.MinValue, half.MaxValue), }; - private abstract class TypeMetadata + private class TypeRange { public KnownType Type { get; init; } public double MinValue { get; init; } public double MaxValue { get; init; } - protected TypeMetadata(KnownType type, double minValue, double maxValue) + public TypeRange(KnownType type, double minValue, double maxValue) { Type = type; MinValue = minValue; MaxValue = maxValue; } - - public virtual bool Less(double constant) => false; - public virtual bool Greater(double constant) => false; - - public abstract bool LessOrEqual(double constant); - public abstract bool GreaterOrEqual(double constant); - } - - private class LimitedFunctionalityTypeMetadata : TypeMetadata // For types that are already semi-covered by CS0652 - { - public LimitedFunctionalityTypeMetadata(KnownType type, double minValue, double maxValue) - : base(type, minValue, maxValue) - { } - - public override bool GreaterOrEqual(double constant) => // something >= constant - constant == MinValue; - - public override bool LessOrEqual(double constant) => // something <= constant - constant == MaxValue; - } - - private class FullFunctionalityTypeMetadata : TypeMetadata // For types that are not covered by CS0652 - { - public FullFunctionalityTypeMetadata(KnownType type, double minValue, double maxValue) - : base(type, minValue, maxValue) - { } - - public override bool Greater(double constant) => // something > constant - constant < MinValue; - - public override bool Less(double constant) => // something < constant - constant > MaxValue; - - public override bool GreaterOrEqual(double constant) => // something >= constant - constant <= MinValue; - - public override bool LessOrEqual(double constant) => // something <= constant - constant >= MaxValue; } } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs index a814b39ba9e..653694064a6 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs @@ -82,8 +82,8 @@ public void Floats() const double smallDouble = float.MinValue; const double bigDouble = float.MaxValue; - const double veryBig = float.MaxValue + 1; - const double verySmall = float.MinValue - 1; + const double veryBig = double.MaxValue; + const double verySmall = double.MinValue; float f = 42; From 2983cc741f4f5b51816732265cbc0546078c8109 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Thu, 2 Feb 2023 17:55:39 +0100 Subject: [PATCH 08/28] Move some code around --- .../Rules/SillyBitwiseOperation.cs | 16 +++++++++++++--- .../Rules/SillyBitwiseOperationBase.cs | 8 +------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs index d793ea14344..ea690f84f08 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs @@ -18,9 +18,6 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using System.Numerics; -using Microsoft.CodeAnalysis; - namespace SonarAnalyzer.Rules.CSharp { [DiagnosticAnalyzer(LanguageNames.CSharp)] @@ -28,6 +25,19 @@ public sealed class SillyBitwiseOperation : SillyBitwiseOperationBase { protected override ILanguageFacade Language => CSharpFacade.Instance; + // No codefix (yet?) + private const string ComparisonDiagnosticId = "S2198"; + private const string ComparisonMessageFormat = "Remove this silly mathematical comparison."; + + protected DiagnosticDescriptor ComparisonRule { get; set; } + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(BitwiseRule, ComparisonRule); + + public SillyBitwiseOperation() + { + ComparisonRule = Language.CreateDescriptor(ComparisonDiagnosticId, ComparisonMessageFormat, fadeOutCode: true); + } + protected override void Initialize(SonarAnalysisContext context) { context.RegisterNodeAction( diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/SillyBitwiseOperationBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/SillyBitwiseOperationBase.cs index 64a7863fdd7..32023a70859 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/SillyBitwiseOperationBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/SillyBitwiseOperationBase.cs @@ -27,21 +27,15 @@ public abstract class SillyBitwiseOperationBase : SonarDiagnosticAnalyzer internal const string IsReportingOnLeftKey = "IsReportingOnLeft"; private const string BitwiseMessageFormat = "Remove this silly bit operation."; - // No codefix (yet?) - private const string ComparisonDiagnosticId = "S2198"; - private const string ComparisonMessageFormat = "Remove this silly mathematical comparison."; - protected abstract ILanguageFacade Language { get; } protected DiagnosticDescriptor BitwiseRule { get; } - protected DiagnosticDescriptor ComparisonRule { get; } - public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(BitwiseRule, ComparisonRule); + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(BitwiseRule); protected SillyBitwiseOperationBase() { BitwiseRule = Language.CreateDescriptor(BitwiseDiagnosticId, BitwiseMessageFormat, fadeOutCode: true); - ComparisonRule = Language.CreateDescriptor(ComparisonDiagnosticId, ComparisonMessageFormat, fadeOutCode: true); } protected void CheckBinary(SonarSyntaxNodeReportingContext context, SyntaxNode left, SyntaxToken @operator, SyntaxNode right, int constValueToLookFor) From 3374a3597d2b66ac2b346b1ec6882ec1832bb1d6 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Fri, 3 Feb 2023 11:04:01 +0100 Subject: [PATCH 09/28] Add a lot of testcases --- .../Rules/SillyBitwiseOperation.cs | 2 + .../TestCases/SillyMathematicalComparison.cs | 482 ++++++++++++++++-- 2 files changed, 433 insertions(+), 51 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs index ea690f84f08..5ef22f827b2 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs @@ -147,6 +147,8 @@ private bool ShouldRaise(SemanticModel semanticModel, SyntaxNode value, SyntaxNo private static readonly KnownType[] FullyImplementedTypes = new[] { + KnownType.System_Int64, + KnownType.System_UInt64, KnownType.System_Half, KnownType.System_Single, }; diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs index 653694064a6..f21ab2e9758 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs @@ -8,65 +8,139 @@ namespace SonarAnalyzer.UnitTest.TestCases { internal class SillyMathematicalComparison { - public void Bytes() + public void SBytes() { - //const sbyte smallSByte = byte.MinValue; - //const sbyte bigSByte = byte.MaxValue; + const sbyte smallSByte = sbyte.MinValue; + const short smallShort = sbyte.MinValue; + const int smallInt = sbyte.MinValue; + const long smallLong = sbyte.MinValue; + const float smallFloat = sbyte.MinValue; + const double smallDouble = sbyte.MinValue; + const decimal smallDecimal = sbyte.MinValue; - const byte smallByte = byte.MinValue; - const byte bigByte = byte.MaxValue; + const sbyte bigSByte = sbyte.MaxValue; + const short bigShort = sbyte.MaxValue; + const int bigInt = sbyte.MaxValue; + const long bigLong = sbyte.MaxValue; + const float bigFloat = sbyte.MaxValue; + const double bigDouble = sbyte.MaxValue; + const decimal bigDecimal = sbyte.MaxValue; - const short smallShort = byte.MinValue; - const short bigShort = byte.MaxValue; + sbyte sb = 42; - const ushort smallUShort = byte.MinValue; - const ushort bigUShort = byte.MaxValue; + _ = sb >= smallSByte; // Noncompliant + _ = sb >= smallShort; // Noncompliant + _ = sb >= smallInt; // Noncompliant + _ = smallLong <= sb; // Noncompliant + _ = smallFloat <= sb; // Noncompliant + _ = smallDouble <= sb; // Noncompliant + _ = smallDecimal <= sb; // Noncompliant - const int smallInt = byte.MinValue; - const int bigInt = byte.MaxValue; + _ = sb <= bigSByte; // Noncompliant + _ = sb <= bigShort; // Noncompliant + _ = sb <= bigInt; // Noncompliant + _ = bigLong >= sb; // Noncompliant + _ = bigFloat >= sb; // Noncompliant + _ = bigDouble >= sb; // Noncompliant + _ = bigDecimal >= sb; // Noncompliant - const uint smallUInt = byte.MinValue; - const uint bigUInt = byte.MaxValue; + _ = sb >= bigSByte; // Compliant, not (always true) or (always false) + _ = sb >= bigShort; // Compliant, not (always true) or (always false) + _ = sb >= bigInt; // Compliant, not (always true) or (always false) + _ = bigLong <= sb; // Compliant, not (always true) or (always false) + _ = bigFloat <= sb; // Compliant, not (always true) or (always false) + _ = bigDouble <= sb; // Compliant, not (always true) or (always false) + _ = bigDecimal <= sb; // Compliant, not (always true) or (always false) - const long smallLong = byte.MinValue; - const long bigLong = byte.MaxValue; + _ = sb <= smallSByte; // Compliant, not (always true) or (always false) + _ = sb <= smallShort; // Compliant, not (always true) or (always false) + _ = sb <= smallInt; // Compliant, not (always true) or (always false) + _ = smallLong >= sb; // Compliant, not (always true) or (always false) + _ = smallFloat >= sb; // Compliant, not (always true) or (always false) + _ = smallDouble >= sb; // Compliant, not (always true) or (always false) + _ = smallDecimal >= sb; // Compliant, not (always true) or (always false) - const ulong smallULong = byte.MinValue; - const ulong bigULong = byte.MaxValue; + const int veryBig = int.MaxValue; + const long verySmall = long.MinValue; - const float smallFloat = byte.MinValue; - const float bigFloat = byte.MaxValue; + _ = sb < veryBig; // Compliant, raised by CS0652 + _ = veryBig > sb; // Compliant, raised by CS0652 + _ = sb > verySmall; // Compliant, raised by CS0652 + _ = verySmall < sb; // Compliant, raised by CS0652 + } + public void Bytes() + { + const byte smallByte = byte.MinValue; + const short smallShort = byte.MinValue; + const ushort smallUShort = byte.MinValue; + const int smallInt = byte.MinValue; + const uint smallUInt = byte.MinValue; + const long smallLong = byte.MinValue; + const ulong smallULong = byte.MinValue; + const float smallFloat = byte.MinValue; const double smallDouble = byte.MinValue; - const double bigDouble = byte.MaxValue; - const decimal smallDecimal = byte.MinValue; + + const byte bigByte = byte.MaxValue; + const short bigShort = byte.MaxValue; + const ushort bigUShort = byte.MaxValue; + const int bigInt = byte.MaxValue; + const uint bigUInt = byte.MaxValue; + const long bigLong = byte.MaxValue; + const ulong bigULong = byte.MaxValue; + const float bigFloat = byte.MaxValue; + const double bigDouble = byte.MaxValue; const decimal bigDecimal = byte.MaxValue; byte b = 42; - _ = b <= bigByte; // Noncompliant - _ = b <= bigShort; // Noncompliant _ = b >= smallByte; // Noncompliant _ = b >= smallShort; // Noncompliant + _ = b >= smallUShort; // Noncompliant + _ = b >= smallInt; // Noncompliant + _ = b >= smallUInt; // Noncompliant + _ = smallLong <= b; // Noncompliant + _ = smallULong <= b; // Noncompliant + _ = smallFloat <= b; // Noncompliant + _ = smallDouble <= b; // Noncompliant + _ = smallDecimal <= b; // Noncompliant - _ = bigByte >= b; // Noncompliant - _ = bigShort >= b; // Noncompliant - _ = smallByte <= b; // Noncompliant - _ = smallShort <= b; // Noncompliant + _ = b <= bigByte; // Noncompliant + _ = b <= bigShort; // Noncompliant + _ = b <= bigUShort; // Noncompliant + _ = b <= bigInt; // Noncompliant + _ = b <= bigUInt; // Noncompliant + _ = bigLong >= b; // Noncompliant + _ = bigULong >= b; // Noncompliant + _ = bigFloat >= b; // Noncompliant + _ = bigDouble >= b; // Noncompliant + _ = bigDecimal >= b; // Noncompliant _ = b >= bigByte; // Compliant, not (always true) or (always false) _ = b >= bigShort; // Compliant, not (always true) or (always false) + _ = b >= bigUShort; // Compliant, not (always true) or (always false) + _ = b >= bigInt; // Compliant, not (always true) or (always false) + _ = b >= bigUInt; // Compliant, not (always true) or (always false) + _ = bigLong <= b; // Compliant, not (always true) or (always false) + _ = bigULong <= b; // Compliant, not (always true) or (always false) + _ = bigFloat <= b; // Compliant, not (always true) or (always false) + _ = bigDouble <= b; // Compliant, not (always true) or (always false) + _ = bigDecimal <= b; // Compliant, not (always true) or (always false) + _ = b <= smallByte; // Compliant, not (always true) or (always false) _ = b <= smallShort; // Compliant, not (always true) or (always false) + _ = b <= smallUShort; // Compliant, not (always true) or (always false) + _ = b <= smallInt; // Compliant, not (always true) or (always false) + _ = b <= smallUInt; // Compliant, not (always true) or (always false) + _ = smallLong >= b; // Compliant, not (always true) or (always false) + _ = smallULong >= b; // Compliant, not (always true) or (always false) + _ = smallFloat >= b; // Compliant, not (always true) or (always false) + _ = smallDouble >= b; // Compliant, not (always true) or (always false) + _ = smallDecimal >= b; // Compliant, not (always true) or (always false) - _ = bigByte <= b; // Compliant, not (always true) or (always false) - _ = bigShort <= b; // Compliant, not (always true) or (always false) - _ = smallByte >= b; // Compliant, not (always true) or (always false) - _ = smallShort >= b; // Compliant, not (always true) or (always false) - - const int veryBig = byte.MaxValue + 1; - const int verySmall = byte.MinValue - 1; + const int veryBig = int.MaxValue; + const short verySmall = short.MinValue; _ = b < veryBig; // Compliant, raised by CS0652 _ = veryBig > b; // Compliant, raised by CS0652 @@ -74,44 +148,350 @@ public void Bytes() _ = verySmall < b; // Compliant, raised by CS0652 } - public void Floats() + public void Shorts() { - const float smallFloat = float.MinValue; - const float bigFloat = float.MaxValue; + const short smallShort = short.MinValue; + const int smallInt = short.MinValue; + const long smallLong = short.MinValue; + const float smallFloat = short.MinValue; + const double smallDouble = short.MinValue; + const decimal smallDecimal = short.MinValue; - const double smallDouble = float.MinValue; - const double bigDouble = float.MaxValue; + const short bigShort = short.MaxValue; + const int bigInt = short.MaxValue; + const long bigLong = short.MaxValue; + const float bigFloat = short.MaxValue; + const double bigDouble = short.MaxValue; + const decimal bigDecimal = short.MaxValue; + + short s = 42; + + _ = s >= smallShort; // Noncompliant + _ = s >= smallInt; // Noncompliant + _ = smallLong <= s; // Noncompliant + _ = smallFloat <= s; // Noncompliant + _ = smallDouble <= s; // Noncompliant + _ = smallDecimal <= s; // Noncompliant + + _ = s <= bigShort; // Noncompliant + _ = s <= bigInt; // Noncompliant + _ = bigLong >= s; // Noncompliant + _ = bigFloat >= s; // Noncompliant + _ = bigDouble >= s; // Noncompliant + _ = bigDecimal >= s; // Noncompliant + + _ = s >= bigShort; // Compliant, not (always true) or (always false) + _ = s >= bigInt; // Compliant, not (always true) or (always false) + _ = bigLong <= s; // Compliant, not (always true) or (always false) + _ = bigFloat <= s; // Compliant, not (always true) or (always false) + _ = bigDouble <= s; // Compliant, not (always true) or (always false) + _ = bigDecimal <= s; // Compliant, not (always true) or (always false) + + _ = s <= smallShort; // Compliant, not (always true) or (always false) + _ = s <= smallInt; // Compliant, not (always true) or (always false) + _ = smallLong >= s; // Compliant, not (always true) or (always false) + _ = smallFloat >= s; // Compliant, not (always true) or (always false) + _ = smallDouble >= s; // Compliant, not (always true) or (always false) + _ = smallDecimal >= s; // Compliant, not (always true) or (always false) + + const int veryBig = int.MaxValue; + const long verySmall = long.MinValue; + + _ = s < veryBig; // Compliant, raised by CS0652 + _ = veryBig > s; // Compliant, raised by CS0652 + _ = s > verySmall; // Compliant, raised by CS0652 + _ = verySmall < s; // Compliant, raised by CS0652 + } + + public void UShorts() + { + const ushort smallUShort = ushort.MinValue; + const int smallInt = ushort.MinValue; + const uint smallUInt = ushort.MinValue; + const long smallLong = ushort.MinValue; + const ulong smallULong = ushort.MinValue; + const float smallFloat = ushort.MinValue; + const double smallDouble = ushort.MinValue; + const decimal smallDecimal = ushort.MinValue; + + const ushort bigUShort = ushort.MaxValue; + const int bigInt = ushort.MaxValue; + const uint bigUInt = ushort.MaxValue; + const long bigLong = ushort.MaxValue; + const ulong bigULong = ushort.MaxValue; + const float bigFloat = ushort.MaxValue; + const double bigDouble = ushort.MaxValue; + const decimal bigDecimal = ushort.MaxValue; + + ushort us = 42; + + _ = us >= smallUShort; // Noncompliant + _ = us >= smallInt; // Noncompliant + _ = us >= smallUInt; // Noncompliant + _ = smallLong <= us; // Noncompliant + _ = smallULong <= us; // Noncompliant + _ = smallFloat <= us; // Noncompliant + _ = smallDouble <= us; // Noncompliant + _ = smallDecimal <= us; // Noncompliant + + _ = us <= bigUShort; // Noncompliant + _ = us <= bigInt; // Noncompliant + _ = us <= bigUInt; // Noncompliant + _ = bigLong >= us; // Noncompliant + _ = bigULong >= us; // Noncompliant + _ = bigFloat >= us; // Noncompliant + _ = bigDouble >= us; // Noncompliant + _ = bigDecimal >= us; // Noncompliant + + _ = us >= bigUShort; // Compliant, not (always true) or (always false) + _ = us >= bigInt; // Compliant, not (always true) or (always false) + _ = us >= bigUInt; // Compliant, not (always true) or (always false) + _ = bigLong <= us; // Compliant, not (always true) or (always false) + _ = bigULong <= us; // Compliant, not (always true) or (always false) + _ = bigFloat <= us; // Compliant, not (always true) or (always false) + _ = bigDouble <= us; // Compliant, not (always true) or (always false) + _ = bigDecimal <= us; // Compliant, not (always true) or (always false) + + _ = us <= smallUShort; // Compliant, not (always true) or (always false) + _ = us <= smallInt; // Compliant, not (always true) or (always false) + _ = us <= smallUInt; // Compliant, not (always true) or (always false) + _ = smallLong >= us; // Compliant, not (always true) or (always false) + _ = smallULong >= us; // Compliant, not (always true) or (always false) + _ = smallFloat >= us; // Compliant, not (always true) or (always false) + _ = smallDouble >= us; // Compliant, not (always true) or (always false) + _ = smallDecimal >= us; // Compliant, not (always true) or (always false) + + const int veryBig = int.MaxValue; + const long verySmall = long.MinValue; + + _ = us < veryBig; // Compliant, raised by CS0652 + _ = veryBig > us; // Compliant, raised by CS0652 + _ = us > verySmall; // Compliant, raised by CS0652 + _ = verySmall < us; // Compliant, raised by CS0652 + } + + public void Ints() + { + const int smallInt = int.MinValue; + const long smallLong = int.MinValue; + const float smallFloat = int.MinValue; + const double smallDouble = int.MinValue; + const decimal smallDecimal = int.MinValue; + + const int bigInt = int.MaxValue; + const uint bigUInt = int.MaxValue; + const long bigLong = int.MaxValue; + const float bigFloat = int.MaxValue; + const double bigDouble = int.MaxValue; + const decimal bigDecimal = int.MaxValue; + + int i = 42; + + _ = i >= smallInt; // Noncompliant + _ = smallLong <= i; // Noncompliant + _ = smallFloat <= i; // Noncompliant + _ = smallDouble <= i; // Noncompliant + _ = smallDecimal <= i; // Noncompliant + + _ = i <= bigInt; // Noncompliant + _ = i <= bigUInt; // Noncompliant + _ = bigLong >= i; // Noncompliant + _ = bigFloat >= i; // Compliant, "bigFloat" has lost precision here and is not int.MaxValue + _ = bigDouble >= i; // Noncompliant + _ = bigDecimal >= i; // Noncompliant + + _ = i >= bigInt; // Compliant, not (always true) or (always false) + _ = i >= bigUInt; // Compliant, not (always true) or (always false) + _ = bigLong <= i; // Compliant, not (always true) or (always false) + _ = bigFloat <= i; // Compliant, not (always true) or (always false) + _ = bigDouble <= i; // Compliant, not (always true) or (always false) + _ = bigDecimal <= i; // Compliant, not (always true) or (always false) + + _ = i <= smallInt; // Compliant, not (always true) or (always false) + _ = smallLong >= i; // Compliant, not (always true) or (always false) + _ = smallFloat >= i; // Compliant, not (always true) or (always false) + _ = smallDouble >= i; // Compliant, not (always true) or (always false) + _ = smallDecimal >= i; // Compliant, not (always true) or (always false) + + const long veryBig = long.MaxValue; + const long verySmall = long.MinValue; + + _ = i < veryBig; // Compliant, raised by CS0652 + _ = veryBig > i; // Compliant, raised by CS0652 + _ = i > verySmall; // Compliant, raised by CS0652 + _ = verySmall < i; // Compliant, raised by CS0652 + } + + public void UInts() + { + const uint smallUInt = uint.MinValue; + const long smallLong = uint.MinValue; + const ulong smallULong = uint.MinValue; + const float smallFloat = uint.MinValue; + const double smallDouble = uint.MinValue; + const decimal smallDecimal = uint.MinValue; + + const uint bigUInt = uint.MaxValue; + const long bigLong = uint.MaxValue; + const ulong bigULong = uint.MaxValue; + const float bigFloat = uint.MaxValue; + const double bigDouble = uint.MaxValue; + const decimal bigDecimal = uint.MaxValue; + + uint ui = 42; + + _ = ui >= smallUInt; // Noncompliant + _ = smallLong <= ui; // Noncompliant + _ = smallULong <= ui; // Noncompliant + _ = smallFloat <= ui; // Noncompliant + _ = smallDouble <= ui; // Noncompliant + _ = smallDecimal <= ui; // Noncompliant + + _ = ui <= bigUInt; // Noncompliant + _ = bigLong >= ui; // Noncompliant + _ = bigULong >= ui; // Noncompliant + _ = bigFloat >= ui; // Compliant, "bigFloat" has lost precision here and is not uint.MaxValue + _ = bigDouble >= ui; // Noncompliant + _ = bigDecimal >= ui; // Noncompliant + + _ = ui >= bigUInt; // Compliant, not (always true) or (always false) + _ = bigLong <= ui; // Compliant, not (always true) or (always false) + _ = bigULong <= ui; // Compliant, not (always true) or (always false) + _ = bigFloat <= ui; // Compliant, not (always true) or (always false) + _ = bigDouble <= ui; // Compliant, not (always true) or (always false) + _ = bigDecimal <= ui; // Compliant, not (always true) or (always false) + + _ = ui <= smallUInt; // Compliant, not (always true) or (always false) + _ = smallLong >= ui; // Compliant, not (always true) or (always false) + _ = smallULong >= ui; // Compliant, not (always true) or (always false) + _ = smallFloat >= ui; // Compliant, not (always true) or (always false) + _ = smallDouble >= ui; // Compliant, not (always true) or (always false) + _ = smallDecimal >= ui; // Compliant, not (always true) or (always false) + + const long veryBig = long.MaxValue; + const long verySmall = long.MinValue; + + _ = ui < veryBig; // Compliant, raised by CS0652 + _ = veryBig > ui; // Compliant, raised by CS0652 + _ = ui > verySmall; // Compliant, raised by CS0652 + _ = verySmall < ui; // Compliant, raised by CS0652 + } + + public void Longs() + { + const long smallLong = long.MinValue; + const float smallFloat = long.MinValue; + const double smallDouble = long.MinValue; + const decimal smallDecimal = long.MinValue; + + const long bigLong = long.MaxValue; + const float bigFloat = long.MaxValue; + const double bigDouble = long.MaxValue; + const decimal bigDecimal = long.MaxValue; + + long l = 42; + + _ = smallLong <= l; // Noncompliant + _ = smallFloat <= l; // Noncompliant + _ = smallDouble <= l; // Noncompliant + _ = smallDecimal <= l; // Noncompliant + + _ = bigLong >= l; // Noncompliant + _ = bigFloat >= l; // Noncompliant + _ = bigDouble >= l; // Noncompliant + _ = bigDecimal >= l; // Noncompliant + + _ = bigLong <= l; // Compliant, not (always true) or (always false) + _ = bigFloat <= l; // Compliant, not (always true) or (always false) + _ = bigDouble <= l; // Compliant, not (always true) or (always false) + _ = bigDecimal <= l; // Compliant, not (always true) or (always false) + + _ = smallLong >= l; // Compliant, not (always true) or (always false) + _ = smallFloat >= l; // Compliant, not (always true) or (always false) + _ = smallDouble >= l; // Compliant, not (always true) or (always false) + _ = smallDecimal >= l; // Compliant, not (always true) or (always false) const double veryBig = double.MaxValue; const double verySmall = double.MinValue; - float f = 42; + _ = l < veryBig; // Noncompliant + _ = veryBig > l; // Noncompliant + _ = l > verySmall; // Noncompliant + _ = verySmall < l; // Noncompliant + } + + public void ULongs() + { + const ulong smallULong = ulong.MinValue; + const float smallFloat = ulong.MinValue; + const double smallDouble = ulong.MinValue; + const decimal smallDecimal = ulong.MinValue; - _ = f <= bigFloat; // Noncompliant - _ = f <= bigDouble; // Noncompliant - _ = f >= smallFloat; // Noncompliant - _ = f >= smallDouble; // Noncompliant + const ulong bigULong = ulong.MaxValue; + const float bigFloat = ulong.MaxValue; + const double bigDouble = ulong.MaxValue; + const decimal bigDecimal = ulong.MaxValue; + + ulong ul = 42; + + _ = smallULong <= ul; // Noncompliant + _ = smallFloat <= ul; // Noncompliant + _ = smallDouble <= ul; // Noncompliant + _ = smallDecimal <= ul; // Noncompliant + + _ = bigULong >= ul; // Noncompliant + _ = bigFloat >= ul; // Noncompliant + _ = bigDouble >= ul; // Noncompliant + _ = bigDecimal >= ul; // Noncompliant + + _ = bigULong <= ul; // Compliant, not (always true) or (always false) + _ = bigFloat <= ul; // Compliant, not (always true) or (always false) + _ = bigDouble <= ul; // Compliant, not (always true) or (always false) + _ = bigDecimal <= ul; // Compliant, not (always true) or (always false) + + _ = smallULong >= ul; // Compliant, not (always true) or (always false) + _ = smallFloat >= ul; // Compliant, not (always true) or (always false) + _ = smallDouble >= ul; // Compliant, not (always true) or (always false) + _ = smallDecimal >= ul; // Compliant, not (always true) or (always false) + + const float veryBig = float.MaxValue; + const float verySmall = float.MinValue; + + _ = ul < veryBig; // Noncompliant + _ = veryBig > ul; // Noncompliant + _ = ul > verySmall; // Noncompliant + _ = verySmall < ul; // Noncompliant + } + + public void Floats() + { + const float smallFloat = float.MinValue; + const double smallDouble = float.MinValue; + + const float bigFloat = float.MaxValue; + const double bigDouble = float.MaxValue; + + float f = 42; - _ = bigFloat >= f; // Noncompliant - _ = bigDouble >= f; // Noncompliant _ = smallFloat <= f; // Noncompliant _ = smallDouble <= f; // Noncompliant - _ = f >= bigFloat; // Compliant, not (always true) or (always false) - _ = f >= bigDouble; // Compliant, not (always true) or (always false) - _ = f <= smallFloat; // Compliant, not (always true) or (always false) - _ = f <= smallDouble; // Compliant, not (always true) or (always false) + _ = bigFloat >= f; // Noncompliant + _ = bigDouble >= f; // Noncompliant _ = bigFloat <= f; // Compliant, not (always true) or (always false) _ = bigDouble <= f; // Compliant, not (always true) or (always false) + _ = smallFloat >= f; // Compliant, not (always true) or (always false) _ = smallDouble >= f; // Compliant, not (always true) or (always false) + const double veryBig = double.MaxValue; + const double verySmall = double.MinValue; + _ = f < veryBig; // Noncompliant _ = veryBig > f; // Noncompliant _ = f > verySmall; // Noncompliant _ = verySmall < f; // Noncompliant } - } } From 3564d52e1a8946f6b0d4973f84c098ba79a1ca17 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Fri, 3 Feb 2023 15:29:56 +0100 Subject: [PATCH 10/28] Cut branches: only implement absolute inequality for float --- .../Rules/SillyBitwiseOperation.cs | 84 +--- .../TestCases/SillyMathematicalComparison.cs | 472 ------------------ 2 files changed, 8 insertions(+), 548 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs index 5ef22f827b2..4bc96cdbf9a 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs @@ -88,97 +88,29 @@ private void CheckBinary(SonarSyntaxNodeReportingContext context, int constValue private void CheckComparison(SonarSyntaxNodeReportingContext context) { var binary = (BinaryExpressionSyntax)context.Node; - if (ShouldRaise(context.SemanticModel, binary.Left, binary.Right, binary.Kind(), false)) + if (ShouldRaise(context.SemanticModel, binary.Left, binary.Right)) { context.ReportIssue(Diagnostic.Create(BitwiseRule, binary.GetLocation())); } - if (ShouldRaise(context.SemanticModel, binary.Right, binary.Left, binary.Kind(), true)) + if (ShouldRaise(context.SemanticModel, binary.Right, binary.Left)) { context.ReportIssue(Diagnostic.Create(BitwiseRule, binary.GetLocation())); } } - private bool ShouldRaise(SemanticModel semanticModel, SyntaxNode value, SyntaxNode expectedConstant, SyntaxKind kind, bool constantIsLeft) - { - if (FindConstant(semanticModel, expectedConstant, Convert.ToDouble) is { } constant - && semanticModel.GetSymbolInfo(value).Symbol.GetSymbolType() is { } symbol - && Ranges.FirstOrDefault(x => symbol.Is(x.Type)) is { } range) - { - // Implement out-of-range checks for the types that CS0652 does not. - if (symbol.IsAny(FullyImplementedTypes) && (constant < range.MinValue || constant > range.MaxValue)) - { - return true; - } - - // Implement threshold checks for every type. - return GetThreshold(kind, constantIsLeft, range) is { } threshold && constant == threshold; - } - - return false; - } - - private static double? GetThreshold(SyntaxKind operation, bool constantIsLeft, TypeRange range) - { - if (constantIsLeft) - { - return operation switch - { - SyntaxKind.LessThanExpression => range.MaxValue, // T.MaxValue < x - SyntaxKind.LessThanOrEqualExpression => range.MinValue, // T.MinValue <= x - SyntaxKind.GreaterThanExpression => range.MinValue, // T.MinValue > x - SyntaxKind.GreaterThanOrEqualExpression => range.MaxValue, // T.MaxValue >= x - _ => null - }; - } - else - { - - return operation switch - { - SyntaxKind.LessThanExpression => range.MinValue, // x < T.MinValue - SyntaxKind.LessThanOrEqualExpression => range.MaxValue, // x <= T.MaxValue - SyntaxKind.GreaterThanExpression => range.MaxValue, // x > T.MaxValue - SyntaxKind.GreaterThanOrEqualExpression => range.MinValue, // x >= T.MinValue - _ => null - }; - } - } - - private static readonly KnownType[] FullyImplementedTypes = new[] - { - KnownType.System_Int64, - KnownType.System_UInt64, - KnownType.System_Half, - KnownType.System_Single, - }; + private bool ShouldRaise(SemanticModel semanticModel, SyntaxNode value, SyntaxNode expectedConstant) => + FindConstant(semanticModel, expectedConstant, Convert.ToDouble) is { } constant + && semanticModel.GetSymbolInfo(value).Symbol.GetSymbolType() is { } symbol + && Ranges.FirstOrDefault(x => symbol.Is(x.Type)) is { } range + && (constant < range.MinValue || constant > range.MaxValue); private static readonly TypeRange[] Ranges = new[] { - new TypeRange(KnownType.System_SByte, sbyte.MinValue, sbyte.MaxValue), - new TypeRange(KnownType.System_Byte, byte.MinValue, byte.MaxValue), - new TypeRange(KnownType.System_Int16, short.MinValue, short.MaxValue), - new TypeRange(KnownType.System_UInt16, ushort.MinValue, ushort.MaxValue), - new TypeRange(KnownType.System_Int32, int.MinValue, int.MaxValue), - new TypeRange(KnownType.System_UInt32, uint.MinValue, uint.MaxValue), - new TypeRange(KnownType.System_Int64, long.MinValue, long.MaxValue), - new TypeRange(KnownType.System_UInt64, ulong.MinValue, ulong.MaxValue), new TypeRange(KnownType.System_Single, float.MinValue, float.MaxValue), //new TypeRange(KnownType.System_Half, half.MinValue, half.MaxValue), }; - private class TypeRange - { - public KnownType Type { get; init; } - public double MinValue { get; init; } - public double MaxValue { get; init; } - - public TypeRange(KnownType type, double minValue, double maxValue) - { - Type = type; - MinValue = minValue; - MaxValue = maxValue; - } - } + private sealed record TypeRange(KnownType Type, double MinValue, double MaxValue); } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs index f21ab2e9758..d8b1b5ae06b 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs @@ -8,483 +8,11 @@ namespace SonarAnalyzer.UnitTest.TestCases { internal class SillyMathematicalComparison { - public void SBytes() - { - const sbyte smallSByte = sbyte.MinValue; - const short smallShort = sbyte.MinValue; - const int smallInt = sbyte.MinValue; - const long smallLong = sbyte.MinValue; - const float smallFloat = sbyte.MinValue; - const double smallDouble = sbyte.MinValue; - const decimal smallDecimal = sbyte.MinValue; - - const sbyte bigSByte = sbyte.MaxValue; - const short bigShort = sbyte.MaxValue; - const int bigInt = sbyte.MaxValue; - const long bigLong = sbyte.MaxValue; - const float bigFloat = sbyte.MaxValue; - const double bigDouble = sbyte.MaxValue; - const decimal bigDecimal = sbyte.MaxValue; - - sbyte sb = 42; - - _ = sb >= smallSByte; // Noncompliant - _ = sb >= smallShort; // Noncompliant - _ = sb >= smallInt; // Noncompliant - _ = smallLong <= sb; // Noncompliant - _ = smallFloat <= sb; // Noncompliant - _ = smallDouble <= sb; // Noncompliant - _ = smallDecimal <= sb; // Noncompliant - - _ = sb <= bigSByte; // Noncompliant - _ = sb <= bigShort; // Noncompliant - _ = sb <= bigInt; // Noncompliant - _ = bigLong >= sb; // Noncompliant - _ = bigFloat >= sb; // Noncompliant - _ = bigDouble >= sb; // Noncompliant - _ = bigDecimal >= sb; // Noncompliant - - _ = sb >= bigSByte; // Compliant, not (always true) or (always false) - _ = sb >= bigShort; // Compliant, not (always true) or (always false) - _ = sb >= bigInt; // Compliant, not (always true) or (always false) - _ = bigLong <= sb; // Compliant, not (always true) or (always false) - _ = bigFloat <= sb; // Compliant, not (always true) or (always false) - _ = bigDouble <= sb; // Compliant, not (always true) or (always false) - _ = bigDecimal <= sb; // Compliant, not (always true) or (always false) - - _ = sb <= smallSByte; // Compliant, not (always true) or (always false) - _ = sb <= smallShort; // Compliant, not (always true) or (always false) - _ = sb <= smallInt; // Compliant, not (always true) or (always false) - _ = smallLong >= sb; // Compliant, not (always true) or (always false) - _ = smallFloat >= sb; // Compliant, not (always true) or (always false) - _ = smallDouble >= sb; // Compliant, not (always true) or (always false) - _ = smallDecimal >= sb; // Compliant, not (always true) or (always false) - - const int veryBig = int.MaxValue; - const long verySmall = long.MinValue; - - _ = sb < veryBig; // Compliant, raised by CS0652 - _ = veryBig > sb; // Compliant, raised by CS0652 - _ = sb > verySmall; // Compliant, raised by CS0652 - _ = verySmall < sb; // Compliant, raised by CS0652 - } - - public void Bytes() - { - const byte smallByte = byte.MinValue; - const short smallShort = byte.MinValue; - const ushort smallUShort = byte.MinValue; - const int smallInt = byte.MinValue; - const uint smallUInt = byte.MinValue; - const long smallLong = byte.MinValue; - const ulong smallULong = byte.MinValue; - const float smallFloat = byte.MinValue; - const double smallDouble = byte.MinValue; - const decimal smallDecimal = byte.MinValue; - - const byte bigByte = byte.MaxValue; - const short bigShort = byte.MaxValue; - const ushort bigUShort = byte.MaxValue; - const int bigInt = byte.MaxValue; - const uint bigUInt = byte.MaxValue; - const long bigLong = byte.MaxValue; - const ulong bigULong = byte.MaxValue; - const float bigFloat = byte.MaxValue; - const double bigDouble = byte.MaxValue; - const decimal bigDecimal = byte.MaxValue; - - byte b = 42; - - _ = b >= smallByte; // Noncompliant - _ = b >= smallShort; // Noncompliant - _ = b >= smallUShort; // Noncompliant - _ = b >= smallInt; // Noncompliant - _ = b >= smallUInt; // Noncompliant - _ = smallLong <= b; // Noncompliant - _ = smallULong <= b; // Noncompliant - _ = smallFloat <= b; // Noncompliant - _ = smallDouble <= b; // Noncompliant - _ = smallDecimal <= b; // Noncompliant - - _ = b <= bigByte; // Noncompliant - _ = b <= bigShort; // Noncompliant - _ = b <= bigUShort; // Noncompliant - _ = b <= bigInt; // Noncompliant - _ = b <= bigUInt; // Noncompliant - _ = bigLong >= b; // Noncompliant - _ = bigULong >= b; // Noncompliant - _ = bigFloat >= b; // Noncompliant - _ = bigDouble >= b; // Noncompliant - _ = bigDecimal >= b; // Noncompliant - - _ = b >= bigByte; // Compliant, not (always true) or (always false) - _ = b >= bigShort; // Compliant, not (always true) or (always false) - _ = b >= bigUShort; // Compliant, not (always true) or (always false) - _ = b >= bigInt; // Compliant, not (always true) or (always false) - _ = b >= bigUInt; // Compliant, not (always true) or (always false) - _ = bigLong <= b; // Compliant, not (always true) or (always false) - _ = bigULong <= b; // Compliant, not (always true) or (always false) - _ = bigFloat <= b; // Compliant, not (always true) or (always false) - _ = bigDouble <= b; // Compliant, not (always true) or (always false) - _ = bigDecimal <= b; // Compliant, not (always true) or (always false) - - _ = b <= smallByte; // Compliant, not (always true) or (always false) - _ = b <= smallShort; // Compliant, not (always true) or (always false) - _ = b <= smallUShort; // Compliant, not (always true) or (always false) - _ = b <= smallInt; // Compliant, not (always true) or (always false) - _ = b <= smallUInt; // Compliant, not (always true) or (always false) - _ = smallLong >= b; // Compliant, not (always true) or (always false) - _ = smallULong >= b; // Compliant, not (always true) or (always false) - _ = smallFloat >= b; // Compliant, not (always true) or (always false) - _ = smallDouble >= b; // Compliant, not (always true) or (always false) - _ = smallDecimal >= b; // Compliant, not (always true) or (always false) - - const int veryBig = int.MaxValue; - const short verySmall = short.MinValue; - - _ = b < veryBig; // Compliant, raised by CS0652 - _ = veryBig > b; // Compliant, raised by CS0652 - _ = b > verySmall; // Compliant, raised by CS0652 - _ = verySmall < b; // Compliant, raised by CS0652 - } - - public void Shorts() - { - const short smallShort = short.MinValue; - const int smallInt = short.MinValue; - const long smallLong = short.MinValue; - const float smallFloat = short.MinValue; - const double smallDouble = short.MinValue; - const decimal smallDecimal = short.MinValue; - - const short bigShort = short.MaxValue; - const int bigInt = short.MaxValue; - const long bigLong = short.MaxValue; - const float bigFloat = short.MaxValue; - const double bigDouble = short.MaxValue; - const decimal bigDecimal = short.MaxValue; - - short s = 42; - - _ = s >= smallShort; // Noncompliant - _ = s >= smallInt; // Noncompliant - _ = smallLong <= s; // Noncompliant - _ = smallFloat <= s; // Noncompliant - _ = smallDouble <= s; // Noncompliant - _ = smallDecimal <= s; // Noncompliant - - _ = s <= bigShort; // Noncompliant - _ = s <= bigInt; // Noncompliant - _ = bigLong >= s; // Noncompliant - _ = bigFloat >= s; // Noncompliant - _ = bigDouble >= s; // Noncompliant - _ = bigDecimal >= s; // Noncompliant - - _ = s >= bigShort; // Compliant, not (always true) or (always false) - _ = s >= bigInt; // Compliant, not (always true) or (always false) - _ = bigLong <= s; // Compliant, not (always true) or (always false) - _ = bigFloat <= s; // Compliant, not (always true) or (always false) - _ = bigDouble <= s; // Compliant, not (always true) or (always false) - _ = bigDecimal <= s; // Compliant, not (always true) or (always false) - - _ = s <= smallShort; // Compliant, not (always true) or (always false) - _ = s <= smallInt; // Compliant, not (always true) or (always false) - _ = smallLong >= s; // Compliant, not (always true) or (always false) - _ = smallFloat >= s; // Compliant, not (always true) or (always false) - _ = smallDouble >= s; // Compliant, not (always true) or (always false) - _ = smallDecimal >= s; // Compliant, not (always true) or (always false) - - const int veryBig = int.MaxValue; - const long verySmall = long.MinValue; - - _ = s < veryBig; // Compliant, raised by CS0652 - _ = veryBig > s; // Compliant, raised by CS0652 - _ = s > verySmall; // Compliant, raised by CS0652 - _ = verySmall < s; // Compliant, raised by CS0652 - } - - public void UShorts() - { - const ushort smallUShort = ushort.MinValue; - const int smallInt = ushort.MinValue; - const uint smallUInt = ushort.MinValue; - const long smallLong = ushort.MinValue; - const ulong smallULong = ushort.MinValue; - const float smallFloat = ushort.MinValue; - const double smallDouble = ushort.MinValue; - const decimal smallDecimal = ushort.MinValue; - - const ushort bigUShort = ushort.MaxValue; - const int bigInt = ushort.MaxValue; - const uint bigUInt = ushort.MaxValue; - const long bigLong = ushort.MaxValue; - const ulong bigULong = ushort.MaxValue; - const float bigFloat = ushort.MaxValue; - const double bigDouble = ushort.MaxValue; - const decimal bigDecimal = ushort.MaxValue; - - ushort us = 42; - - _ = us >= smallUShort; // Noncompliant - _ = us >= smallInt; // Noncompliant - _ = us >= smallUInt; // Noncompliant - _ = smallLong <= us; // Noncompliant - _ = smallULong <= us; // Noncompliant - _ = smallFloat <= us; // Noncompliant - _ = smallDouble <= us; // Noncompliant - _ = smallDecimal <= us; // Noncompliant - - _ = us <= bigUShort; // Noncompliant - _ = us <= bigInt; // Noncompliant - _ = us <= bigUInt; // Noncompliant - _ = bigLong >= us; // Noncompliant - _ = bigULong >= us; // Noncompliant - _ = bigFloat >= us; // Noncompliant - _ = bigDouble >= us; // Noncompliant - _ = bigDecimal >= us; // Noncompliant - - _ = us >= bigUShort; // Compliant, not (always true) or (always false) - _ = us >= bigInt; // Compliant, not (always true) or (always false) - _ = us >= bigUInt; // Compliant, not (always true) or (always false) - _ = bigLong <= us; // Compliant, not (always true) or (always false) - _ = bigULong <= us; // Compliant, not (always true) or (always false) - _ = bigFloat <= us; // Compliant, not (always true) or (always false) - _ = bigDouble <= us; // Compliant, not (always true) or (always false) - _ = bigDecimal <= us; // Compliant, not (always true) or (always false) - - _ = us <= smallUShort; // Compliant, not (always true) or (always false) - _ = us <= smallInt; // Compliant, not (always true) or (always false) - _ = us <= smallUInt; // Compliant, not (always true) or (always false) - _ = smallLong >= us; // Compliant, not (always true) or (always false) - _ = smallULong >= us; // Compliant, not (always true) or (always false) - _ = smallFloat >= us; // Compliant, not (always true) or (always false) - _ = smallDouble >= us; // Compliant, not (always true) or (always false) - _ = smallDecimal >= us; // Compliant, not (always true) or (always false) - - const int veryBig = int.MaxValue; - const long verySmall = long.MinValue; - - _ = us < veryBig; // Compliant, raised by CS0652 - _ = veryBig > us; // Compliant, raised by CS0652 - _ = us > verySmall; // Compliant, raised by CS0652 - _ = verySmall < us; // Compliant, raised by CS0652 - } - - public void Ints() - { - const int smallInt = int.MinValue; - const long smallLong = int.MinValue; - const float smallFloat = int.MinValue; - const double smallDouble = int.MinValue; - const decimal smallDecimal = int.MinValue; - - const int bigInt = int.MaxValue; - const uint bigUInt = int.MaxValue; - const long bigLong = int.MaxValue; - const float bigFloat = int.MaxValue; - const double bigDouble = int.MaxValue; - const decimal bigDecimal = int.MaxValue; - - int i = 42; - - _ = i >= smallInt; // Noncompliant - _ = smallLong <= i; // Noncompliant - _ = smallFloat <= i; // Noncompliant - _ = smallDouble <= i; // Noncompliant - _ = smallDecimal <= i; // Noncompliant - - _ = i <= bigInt; // Noncompliant - _ = i <= bigUInt; // Noncompliant - _ = bigLong >= i; // Noncompliant - _ = bigFloat >= i; // Compliant, "bigFloat" has lost precision here and is not int.MaxValue - _ = bigDouble >= i; // Noncompliant - _ = bigDecimal >= i; // Noncompliant - - _ = i >= bigInt; // Compliant, not (always true) or (always false) - _ = i >= bigUInt; // Compliant, not (always true) or (always false) - _ = bigLong <= i; // Compliant, not (always true) or (always false) - _ = bigFloat <= i; // Compliant, not (always true) or (always false) - _ = bigDouble <= i; // Compliant, not (always true) or (always false) - _ = bigDecimal <= i; // Compliant, not (always true) or (always false) - - _ = i <= smallInt; // Compliant, not (always true) or (always false) - _ = smallLong >= i; // Compliant, not (always true) or (always false) - _ = smallFloat >= i; // Compliant, not (always true) or (always false) - _ = smallDouble >= i; // Compliant, not (always true) or (always false) - _ = smallDecimal >= i; // Compliant, not (always true) or (always false) - - const long veryBig = long.MaxValue; - const long verySmall = long.MinValue; - - _ = i < veryBig; // Compliant, raised by CS0652 - _ = veryBig > i; // Compliant, raised by CS0652 - _ = i > verySmall; // Compliant, raised by CS0652 - _ = verySmall < i; // Compliant, raised by CS0652 - } - - public void UInts() - { - const uint smallUInt = uint.MinValue; - const long smallLong = uint.MinValue; - const ulong smallULong = uint.MinValue; - const float smallFloat = uint.MinValue; - const double smallDouble = uint.MinValue; - const decimal smallDecimal = uint.MinValue; - - const uint bigUInt = uint.MaxValue; - const long bigLong = uint.MaxValue; - const ulong bigULong = uint.MaxValue; - const float bigFloat = uint.MaxValue; - const double bigDouble = uint.MaxValue; - const decimal bigDecimal = uint.MaxValue; - - uint ui = 42; - - _ = ui >= smallUInt; // Noncompliant - _ = smallLong <= ui; // Noncompliant - _ = smallULong <= ui; // Noncompliant - _ = smallFloat <= ui; // Noncompliant - _ = smallDouble <= ui; // Noncompliant - _ = smallDecimal <= ui; // Noncompliant - - _ = ui <= bigUInt; // Noncompliant - _ = bigLong >= ui; // Noncompliant - _ = bigULong >= ui; // Noncompliant - _ = bigFloat >= ui; // Compliant, "bigFloat" has lost precision here and is not uint.MaxValue - _ = bigDouble >= ui; // Noncompliant - _ = bigDecimal >= ui; // Noncompliant - - _ = ui >= bigUInt; // Compliant, not (always true) or (always false) - _ = bigLong <= ui; // Compliant, not (always true) or (always false) - _ = bigULong <= ui; // Compliant, not (always true) or (always false) - _ = bigFloat <= ui; // Compliant, not (always true) or (always false) - _ = bigDouble <= ui; // Compliant, not (always true) or (always false) - _ = bigDecimal <= ui; // Compliant, not (always true) or (always false) - - _ = ui <= smallUInt; // Compliant, not (always true) or (always false) - _ = smallLong >= ui; // Compliant, not (always true) or (always false) - _ = smallULong >= ui; // Compliant, not (always true) or (always false) - _ = smallFloat >= ui; // Compliant, not (always true) or (always false) - _ = smallDouble >= ui; // Compliant, not (always true) or (always false) - _ = smallDecimal >= ui; // Compliant, not (always true) or (always false) - - const long veryBig = long.MaxValue; - const long verySmall = long.MinValue; - - _ = ui < veryBig; // Compliant, raised by CS0652 - _ = veryBig > ui; // Compliant, raised by CS0652 - _ = ui > verySmall; // Compliant, raised by CS0652 - _ = verySmall < ui; // Compliant, raised by CS0652 - } - - public void Longs() - { - const long smallLong = long.MinValue; - const float smallFloat = long.MinValue; - const double smallDouble = long.MinValue; - const decimal smallDecimal = long.MinValue; - - const long bigLong = long.MaxValue; - const float bigFloat = long.MaxValue; - const double bigDouble = long.MaxValue; - const decimal bigDecimal = long.MaxValue; - - long l = 42; - - _ = smallLong <= l; // Noncompliant - _ = smallFloat <= l; // Noncompliant - _ = smallDouble <= l; // Noncompliant - _ = smallDecimal <= l; // Noncompliant - - _ = bigLong >= l; // Noncompliant - _ = bigFloat >= l; // Noncompliant - _ = bigDouble >= l; // Noncompliant - _ = bigDecimal >= l; // Noncompliant - - _ = bigLong <= l; // Compliant, not (always true) or (always false) - _ = bigFloat <= l; // Compliant, not (always true) or (always false) - _ = bigDouble <= l; // Compliant, not (always true) or (always false) - _ = bigDecimal <= l; // Compliant, not (always true) or (always false) - - _ = smallLong >= l; // Compliant, not (always true) or (always false) - _ = smallFloat >= l; // Compliant, not (always true) or (always false) - _ = smallDouble >= l; // Compliant, not (always true) or (always false) - _ = smallDecimal >= l; // Compliant, not (always true) or (always false) - - const double veryBig = double.MaxValue; - const double verySmall = double.MinValue; - - _ = l < veryBig; // Noncompliant - _ = veryBig > l; // Noncompliant - _ = l > verySmall; // Noncompliant - _ = verySmall < l; // Noncompliant - } - - public void ULongs() - { - const ulong smallULong = ulong.MinValue; - const float smallFloat = ulong.MinValue; - const double smallDouble = ulong.MinValue; - const decimal smallDecimal = ulong.MinValue; - - const ulong bigULong = ulong.MaxValue; - const float bigFloat = ulong.MaxValue; - const double bigDouble = ulong.MaxValue; - const decimal bigDecimal = ulong.MaxValue; - - ulong ul = 42; - - _ = smallULong <= ul; // Noncompliant - _ = smallFloat <= ul; // Noncompliant - _ = smallDouble <= ul; // Noncompliant - _ = smallDecimal <= ul; // Noncompliant - - _ = bigULong >= ul; // Noncompliant - _ = bigFloat >= ul; // Noncompliant - _ = bigDouble >= ul; // Noncompliant - _ = bigDecimal >= ul; // Noncompliant - - _ = bigULong <= ul; // Compliant, not (always true) or (always false) - _ = bigFloat <= ul; // Compliant, not (always true) or (always false) - _ = bigDouble <= ul; // Compliant, not (always true) or (always false) - _ = bigDecimal <= ul; // Compliant, not (always true) or (always false) - - _ = smallULong >= ul; // Compliant, not (always true) or (always false) - _ = smallFloat >= ul; // Compliant, not (always true) or (always false) - _ = smallDouble >= ul; // Compliant, not (always true) or (always false) - _ = smallDecimal >= ul; // Compliant, not (always true) or (always false) - - const float veryBig = float.MaxValue; - const float verySmall = float.MinValue; - - _ = ul < veryBig; // Noncompliant - _ = veryBig > ul; // Noncompliant - _ = ul > verySmall; // Noncompliant - _ = verySmall < ul; // Noncompliant - } public void Floats() { - const float smallFloat = float.MinValue; - const double smallDouble = float.MinValue; - - const float bigFloat = float.MaxValue; - const double bigDouble = float.MaxValue; - float f = 42; - _ = smallFloat <= f; // Noncompliant - _ = smallDouble <= f; // Noncompliant - - _ = bigFloat >= f; // Noncompliant - _ = bigDouble >= f; // Noncompliant - - _ = bigFloat <= f; // Compliant, not (always true) or (always false) - _ = bigDouble <= f; // Compliant, not (always true) or (always false) - - _ = smallFloat >= f; // Compliant, not (always true) or (always false) - _ = smallDouble >= f; // Compliant, not (always true) or (always false) - const double veryBig = double.MaxValue; const double verySmall = double.MinValue; From 86696dcf18f2bd7880304ceacb92bcbec07306f3 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Mon, 6 Feb 2023 11:57:51 +0100 Subject: [PATCH 11/28] Rewrite SillyMathematicalComparison, only out-of-range checks for floats for this step --- .../Rules/SillyBitwiseOperation.cs | 50 +------- .../Rules/SillyMathematicalComparison.cs | 108 ++++++++++++++++++ .../Rules/SillyBitwiseOperationBase.cs | 29 ++--- .../Rules/SillyBitwiseOperationCodeFixBase.cs | 2 +- .../Rules/SillyMathematicalComparison.cs | 2 +- .../TestCases/SillyMathematicalComparison.cs | 1 - 6 files changed, 118 insertions(+), 74 deletions(-) create mode 100644 analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs index 4bc96cdbf9a..5b91a764a9f 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyBitwiseOperation.cs @@ -25,19 +25,6 @@ public sealed class SillyBitwiseOperation : SillyBitwiseOperationBase { protected override ILanguageFacade Language => CSharpFacade.Instance; - // No codefix (yet?) - private const string ComparisonDiagnosticId = "S2198"; - private const string ComparisonMessageFormat = "Remove this silly mathematical comparison."; - - protected DiagnosticDescriptor ComparisonRule { get; set; } - - public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(BitwiseRule, ComparisonRule); - - public SillyBitwiseOperation() - { - ComparisonRule = Language.CreateDescriptor(ComparisonDiagnosticId, ComparisonMessageFormat, fadeOutCode: true); - } - protected override void Initialize(SonarAnalysisContext context) { context.RegisterNodeAction( @@ -57,13 +44,6 @@ protected override void Initialize(SonarAnalysisContext context) c => CheckAssignment(c, 0), SyntaxKind.OrAssignmentExpression, SyntaxKind.ExclusiveOrAssignmentExpression); - - context.RegisterNodeAction( - CheckComparison, - SyntaxKind.GreaterThanExpression, - SyntaxKind.GreaterThanOrEqualExpression, - SyntaxKind.LessThanExpression, - SyntaxKind.LessThanOrEqualExpression); } private void CheckAssignment(SonarSyntaxNodeReportingContext context, int constValueToLookFor) @@ -75,7 +55,7 @@ private void CheckAssignment(SonarSyntaxNodeReportingContext context, int constV var location = assignment.Parent is StatementSyntax ? assignment.Parent.GetLocation() : assignment.OperatorToken.CreateLocation(assignment.Right); - context.ReportIssue(Diagnostic.Create(BitwiseRule, location)); + context.ReportIssue(Diagnostic.Create(Rule, location)); } } @@ -84,33 +64,5 @@ private void CheckBinary(SonarSyntaxNodeReportingContext context, int constValue var binary = (BinaryExpressionSyntax)context.Node; CheckBinary(context, binary.Left, binary.OperatorToken, binary.Right, constValueToLookFor); } - - private void CheckComparison(SonarSyntaxNodeReportingContext context) - { - var binary = (BinaryExpressionSyntax)context.Node; - if (ShouldRaise(context.SemanticModel, binary.Left, binary.Right)) - { - context.ReportIssue(Diagnostic.Create(BitwiseRule, binary.GetLocation())); - } - - if (ShouldRaise(context.SemanticModel, binary.Right, binary.Left)) - { - context.ReportIssue(Diagnostic.Create(BitwiseRule, binary.GetLocation())); - } - } - - private bool ShouldRaise(SemanticModel semanticModel, SyntaxNode value, SyntaxNode expectedConstant) => - FindConstant(semanticModel, expectedConstant, Convert.ToDouble) is { } constant - && semanticModel.GetSymbolInfo(value).Symbol.GetSymbolType() is { } symbol - && Ranges.FirstOrDefault(x => symbol.Is(x.Type)) is { } range - && (constant < range.MinValue || constant > range.MaxValue); - - private static readonly TypeRange[] Ranges = new[] - { - new TypeRange(KnownType.System_Single, float.MinValue, float.MaxValue), - //new TypeRange(KnownType.System_Half, half.MinValue, half.MaxValue), - }; - - private sealed record TypeRange(KnownType Type, double MinValue, double MaxValue); } } diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs new file mode 100644 index 00000000000..0a47ecc4e94 --- /dev/null +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs @@ -0,0 +1,108 @@ +/* + * SonarAnalyzer for .NET + * Copyright (C) 2015-2023 SonarSource SA + * mailto: contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +using SonarAnalyzer.Helpers; + +namespace SonarAnalyzer.Rules.CSharp +{ + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public sealed class SillyMathematicalComparison : SonarDiagnosticAnalyzer + { + private const string DiagnosticId = "S2198"; + private const string MessageFormat = "Comparison to integral constant is useless; the constant is outside the range of type '{0}'"; + + private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + + protected override void Initialize(SonarAnalysisContext context) => + context.RegisterNodeAction( + CheckBinary, + SyntaxKind.GreaterThanExpression, + SyntaxKind.GreaterThanOrEqualExpression, + SyntaxKind.LessThanExpression, + SyntaxKind.LessThanOrEqualExpression); + + private static void CheckBinary(SonarSyntaxNodeReportingContext context) + { + var binary = (BinaryExpressionSyntax)context.Node; + var typeIdentifier = string.Empty; + if (ShouldRaise(context.SemanticModel, binary.Left, binary.Right, ref typeIdentifier)) + { + context.ReportIssue(Diagnostic.Create(Rule, binary.GetLocation(), typeIdentifier)); + } + + if (ShouldRaise(context.SemanticModel, binary.Right, binary.Left, ref typeIdentifier)) + { + context.ReportIssue(Diagnostic.Create(Rule, binary.GetLocation(), typeIdentifier)); + } + } + + private static bool ShouldRaise(SemanticModel model, SyntaxNode expectedConstant, SyntaxNode other, ref string typeIdentifier) => + TryGetConstant(model, expectedConstant, out var constant) + && model.GetSymbolInfo(other).Symbol.GetSymbolType() is { } symbol + && TryGetRange(symbol, out typeIdentifier, out var min, out var max) + && (constant < min || constant > max); + + private static bool TryGetConstant(SemanticModel model, SyntaxNode expectedConstant, out double constant) + { + if (FindConstant(model, expectedConstant, Convert.ToDouble) is { } constValue) + { + constant = constValue; + return true; + } + + constant = default; + return false; + } + + private static T? FindConstant(SemanticModel semanticModel, SyntaxNode node, Func converter) where T : struct + => semanticModel.GetSymbolInfo(node).Symbol is var symbol + && !IsFieldOrPropertyOutsideSystemNamespace(symbol) + && !IsEnum(symbol) + && CSharpFacade.Instance.FindConstantValue(semanticModel, node) is { } value + && ConversionHelper.TryConvertWith(value, converter, out T typedValue) + ? typedValue + : null; + + private static bool TryGetRange(ITypeSymbol typeSymbol, out string typeIdentifier, out double min, out double max) + { + if (typeSymbol.Is(KnownType.System_Single)) + { + typeIdentifier = "float"; + min = float.MinValue; + max = float.MaxValue; + return true; + } + + typeIdentifier = default; + min = default; + max = default; + return false; + } + + private static bool IsFieldOrPropertyOutsideSystemNamespace(ISymbol symbol) => + symbol is { Kind: SymbolKind.Field or SymbolKind.Property } + && symbol.ContainingNamespace.Name != nameof(System); + + private static bool IsEnum(ISymbol symbol) => + symbol.GetSymbolType() is INamedTypeSymbol { EnumUnderlyingType: { } }; + } +} diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/SillyBitwiseOperationBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/SillyBitwiseOperationBase.cs index 32023a70859..73628e8b9d5 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/SillyBitwiseOperationBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/SillyBitwiseOperationBase.cs @@ -20,23 +20,19 @@ namespace SonarAnalyzer.Rules { - // FIXME: Rename this to something like SillyNumericalOperatorUsage public abstract class SillyBitwiseOperationBase : SonarDiagnosticAnalyzer { - internal const string BitwiseDiagnosticId = "S2437"; + internal const string DiagnosticId = "S2437"; internal const string IsReportingOnLeftKey = "IsReportingOnLeft"; - private const string BitwiseMessageFormat = "Remove this silly bit operation."; + private const string MessageFormat = "Remove this silly bit operation."; protected abstract ILanguageFacade Language { get; } - protected DiagnosticDescriptor BitwiseRule { get; } + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + protected DiagnosticDescriptor Rule { get; } - public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(BitwiseRule); - - protected SillyBitwiseOperationBase() - { - BitwiseRule = Language.CreateDescriptor(BitwiseDiagnosticId, BitwiseMessageFormat, fadeOutCode: true); - } + protected SillyBitwiseOperationBase() => + Rule = Language.CreateDescriptor(DiagnosticId, MessageFormat, fadeOutCode: true); protected void CheckBinary(SonarSyntaxNodeReportingContext context, SyntaxNode left, SyntaxToken @operator, SyntaxNode right, int constValueToLookFor) { @@ -57,7 +53,7 @@ protected void CheckBinary(SonarSyntaxNodeReportingContext context, SyntaxNode l return; } - context.ReportIssue(Diagnostic.Create(BitwiseRule, location, ImmutableDictionary.Empty.Add(IsReportingOnLeftKey, isReportingOnLeftKey.ToString()))); + context.ReportIssue(Diagnostic.Create(Rule, location, ImmutableDictionary.Empty.Add(IsReportingOnLeftKey, isReportingOnLeftKey.ToString()))); } protected int? FindIntConstant(SemanticModel semanticModel, SyntaxNode node) => @@ -68,17 +64,6 @@ protected void CheckBinary(SonarSyntaxNodeReportingContext context, SyntaxNode l ? ConversionHelper.TryConvertToInt(value) : null; - protected T? FindConstant(SemanticModel semanticModel, SyntaxNode node, Func converter) where T : struct - { - return semanticModel.GetSymbolInfo(node).Symbol is var symbol - && !IsFieldOrPropertyOutsideSystemNamespace(symbol) - && !IsEnum(symbol) - && Language.FindConstantValue(semanticModel, node) is { } value - && ConversionHelper.TryConvertWith(value, converter, out T typedValue) - ? typedValue - : null; - } - private static bool IsEnum(ISymbol symbol) => symbol.GetSymbolType() is INamedTypeSymbol { EnumUnderlyingType: { } }; diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/SillyBitwiseOperationCodeFixBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/SillyBitwiseOperationCodeFixBase.cs index e1b6a4bd135..f60002a146a 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/SillyBitwiseOperationCodeFixBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/SillyBitwiseOperationCodeFixBase.cs @@ -30,7 +30,7 @@ public abstract class SillyBitwiseOperationCodeFixBase : SonarCodeFix protected abstract Func CreateNewRoot(SyntaxNode root, TextSpan diagnosticSpan, bool isReportingOnLeft); - public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(SillyBitwiseOperationBase.BitwiseDiagnosticId); + public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(SillyBitwiseOperationBase.DiagnosticId); protected override Task RegisterCodeFixesAsync(SyntaxNode root, CodeFixContext context) { diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SillyMathematicalComparison.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SillyMathematicalComparison.cs index d51615b280b..b1aa67313d2 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SillyMathematicalComparison.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SillyMathematicalComparison.cs @@ -25,7 +25,7 @@ namespace SonarAnalyzer.UnitTest.Rules [TestClass] public class SillyMathematicalComparisonTest { - private readonly VerifierBuilder builderCS = new VerifierBuilder(); + private readonly VerifierBuilder builderCS = new VerifierBuilder(); [TestMethod] public void SillyMathematicalComparison_CS() => diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs index d8b1b5ae06b..af396da16b6 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs @@ -8,7 +8,6 @@ namespace SonarAnalyzer.UnitTest.TestCases { internal class SillyMathematicalComparison { - public void Floats() { float f = 42; From 99f3b2778cb4a6fd807b1a7e499bc9ed44242440 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Tue, 7 Feb 2023 11:38:22 +0100 Subject: [PATCH 12/28] Apply PR review changes --- .../Rules/SillyMathematicalComparison.cs | 51 ++++++++----------- 1 file changed, 20 insertions(+), 31 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs index 0a47ecc4e94..be993dfb26d 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs @@ -18,15 +18,13 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using SonarAnalyzer.Helpers; - namespace SonarAnalyzer.Rules.CSharp { [DiagnosticAnalyzer(LanguageNames.CSharp)] public sealed class SillyMathematicalComparison : SonarDiagnosticAnalyzer { private const string DiagnosticId = "S2198"; - private const string MessageFormat = "Comparison to integral constant is useless; the constant is outside the range of type '{0}'"; + private const string MessageFormat = "Comparison to this constant is useless; the constant is outside the range of type '{0}'"; private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat); @@ -38,32 +36,35 @@ public sealed class SillyMathematicalComparison : SonarDiagnosticAnalyzer SyntaxKind.GreaterThanExpression, SyntaxKind.GreaterThanOrEqualExpression, SyntaxKind.LessThanExpression, - SyntaxKind.LessThanOrEqualExpression); + SyntaxKind.LessThanOrEqualExpression, + SyntaxKind.EqualsExpression, + SyntaxKind.NotEqualsExpression); private static void CheckBinary(SonarSyntaxNodeReportingContext context) { var binary = (BinaryExpressionSyntax)context.Node; - var typeIdentifier = string.Empty; - if (ShouldRaise(context.SemanticModel, binary.Left, binary.Right, ref typeIdentifier)) - { - context.ReportIssue(Diagnostic.Create(Rule, binary.GetLocation(), typeIdentifier)); - } + CheckOneSide(context, binary.Left, binary.Right); + CheckOneSide(context, binary.Right, binary.Left); + } - if (ShouldRaise(context.SemanticModel, binary.Right, binary.Left, ref typeIdentifier)) + private static void CheckOneSide(SonarSyntaxNodeReportingContext context, SyntaxNode first, SyntaxNode second) + { + if (ShouldRaise(context.SemanticModel, first, second)) { - context.ReportIssue(Diagnostic.Create(Rule, binary.GetLocation(), typeIdentifier)); + var typeName = context.SemanticModel.GetSymbolInfo(second).Symbol.ToMinimalDisplayString(context.SemanticModel, second.GetLocation().SourceSpan.Start); + context.ReportIssue(Diagnostic.Create(Rule, first.Parent.GetLocation(), typeName)); } } - private static bool ShouldRaise(SemanticModel model, SyntaxNode expectedConstant, SyntaxNode other, ref string typeIdentifier) => + private static bool ShouldRaise(SemanticModel model, SyntaxNode expectedConstant, SyntaxNode other) => TryGetConstant(model, expectedConstant, out var constant) - && model.GetSymbolInfo(other).Symbol.GetSymbolType() is { } symbol - && TryGetRange(symbol, out typeIdentifier, out var min, out var max) + && model.GetTypeInfo(other).Type is { } symbol + && TryGetRange(symbol, out var min, out var max) && (constant < min || constant > max); private static bool TryGetConstant(SemanticModel model, SyntaxNode expectedConstant, out double constant) { - if (FindConstant(model, expectedConstant, Convert.ToDouble) is { } constValue) + if (FindConstant(model, expectedConstant) is { } constValue) { constant = constValue; return true; @@ -73,36 +74,24 @@ private static bool TryGetConstant(SemanticModel model, SyntaxNode expectedConst return false; } - private static T? FindConstant(SemanticModel semanticModel, SyntaxNode node, Func converter) where T : struct - => semanticModel.GetSymbolInfo(node).Symbol is var symbol - && !IsFieldOrPropertyOutsideSystemNamespace(symbol) - && !IsEnum(symbol) - && CSharpFacade.Instance.FindConstantValue(semanticModel, node) is { } value - && ConversionHelper.TryConvertWith(value, converter, out T typedValue) + private static double? FindConstant(SemanticModel semanticModel, SyntaxNode node) => + node.FindConstantValue(semanticModel) is { } value + && ConversionHelper.TryConvertWith(value, Convert.ToDouble, out var typedValue) ? typedValue : null; - private static bool TryGetRange(ITypeSymbol typeSymbol, out string typeIdentifier, out double min, out double max) + private static bool TryGetRange(ITypeSymbol typeSymbol, out double min, out double max) { if (typeSymbol.Is(KnownType.System_Single)) { - typeIdentifier = "float"; min = float.MinValue; max = float.MaxValue; return true; } - typeIdentifier = default; min = default; max = default; return false; } - - private static bool IsFieldOrPropertyOutsideSystemNamespace(ISymbol symbol) => - symbol is { Kind: SymbolKind.Field or SymbolKind.Property } - && symbol.ContainingNamespace.Name != nameof(System); - - private static bool IsEnum(ISymbol symbol) => - symbol.GetSymbolType() is INamedTypeSymbol { EnumUnderlyingType: { } }; } } From ee45a5c0595f4c39027407b19ac6642a5c7e1bdc Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Tue, 7 Feb 2023 11:54:00 +0100 Subject: [PATCH 13/28] Enrichen testcases, add long and ulong to the checked types --- .../Rules/SillyMathematicalComparison.cs | 14 +- .../TestCases/SillyMathematicalComparison.cs | 378 +++++++++++++++++- 2 files changed, 386 insertions(+), 6 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs index be993dfb26d..753e0c69bce 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs @@ -51,7 +51,7 @@ private static void CheckOneSide(SonarSyntaxNodeReportingContext context, Syntax { if (ShouldRaise(context.SemanticModel, first, second)) { - var typeName = context.SemanticModel.GetSymbolInfo(second).Symbol.ToMinimalDisplayString(context.SemanticModel, second.GetLocation().SourceSpan.Start); + var typeName = context.SemanticModel.GetTypeInfo(second).Type.ToString(); context.ReportIssue(Diagnostic.Create(Rule, first.Parent.GetLocation(), typeName)); } } @@ -88,6 +88,18 @@ private static bool TryGetRange(ITypeSymbol typeSymbol, out double min, out doub max = float.MaxValue; return true; } + if (typeSymbol.Is(KnownType.System_Int64)) + { + min = long.MinValue; + max = long.MaxValue; + return true; + } + if (typeSymbol.Is(KnownType.System_UInt64)) + { + min = ulong.MinValue; + max = ulong.MaxValue; + return true; + } min = default; max = default; diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs index af396da16b6..7d97c8f9f5f 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs @@ -8,17 +8,385 @@ namespace SonarAnalyzer.UnitTest.TestCases { internal class SillyMathematicalComparison { + public void SBytes() + { + const sbyte smallSByte = sbyte.MinValue; + const short smallShort = sbyte.MinValue; + const int smallInt = sbyte.MinValue; + const long smallLong = sbyte.MinValue; + const float smallFloat = sbyte.MinValue; + const double smallDouble = sbyte.MinValue; + const decimal smallDecimal = sbyte.MinValue; + + const sbyte bigSByte = sbyte.MaxValue; + const short bigShort = sbyte.MaxValue; + const int bigInt = sbyte.MaxValue; + const long bigLong = sbyte.MaxValue; + const float bigFloat = sbyte.MaxValue; + const double bigDouble = sbyte.MaxValue; + const decimal bigDecimal = sbyte.MaxValue; + + sbyte sb = 42; + + _ = sb >= bigSByte; // Compliant, not (always true) or (always false) + _ = sb >= bigShort; // Compliant, not (always true) or (always false) + _ = sb >= bigInt; // Compliant, not (always true) or (always false) + _ = bigLong <= sb; // Compliant, not (always true) or (always false) + _ = bigFloat <= sb; // Compliant, not (always true) or (always false) + _ = bigDouble <= sb; // Compliant, not (always true) or (always false) + _ = bigDecimal <= sb; // Compliant, not (always true) or (always false) + + _ = sb <= smallSByte; // Compliant, not (always true) or (always false) + _ = sb <= smallShort; // Compliant, not (always true) or (always false) + _ = sb <= smallInt; // Compliant, not (always true) or (always false) + _ = smallLong >= sb; // Compliant, not (always true) or (always false) + _ = smallFloat >= sb; // Compliant, not (always true) or (always false) + _ = smallDouble >= sb; // Compliant, not (always true) or (always false) + _ = smallDecimal >= sb; // Compliant, not (always true) or (always false) + + const int veryBig = int.MaxValue; + const long verySmall = long.MinValue; + + _ = sb < veryBig; // Compliant, raised by CS0652 + _ = veryBig > sb; // Compliant, raised by CS0652 + _ = sb > verySmall; // Compliant, raised by CS0652 + _ = verySmall < sb; // Compliant, raised by CS0652 + _ = verySmall == sb; // Compliant, raised by CS0652 + _ = sb == veryBig; // Compliant, raised by CS0652 + } + + public void Bytes() + { + const byte smallByte = byte.MinValue; + const short smallShort = byte.MinValue; + const ushort smallUShort = byte.MinValue; + const int smallInt = byte.MinValue; + const uint smallUInt = byte.MinValue; + const long smallLong = byte.MinValue; + const ulong smallULong = byte.MinValue; + const float smallFloat = byte.MinValue; + const double smallDouble = byte.MinValue; + const decimal smallDecimal = byte.MinValue; + + const byte bigByte = byte.MaxValue; + const short bigShort = byte.MaxValue; + const ushort bigUShort = byte.MaxValue; + const int bigInt = byte.MaxValue; + const uint bigUInt = byte.MaxValue; + const long bigLong = byte.MaxValue; + const ulong bigULong = byte.MaxValue; + const float bigFloat = byte.MaxValue; + const double bigDouble = byte.MaxValue; + const decimal bigDecimal = byte.MaxValue; + + byte b = 42; + + _ = b >= bigByte; // Compliant, not (always true) or (always false) + _ = b >= bigShort; // Compliant, not (always true) or (always false) + _ = b >= bigUShort; // Compliant, not (always true) or (always false) + _ = b >= bigInt; // Compliant, not (always true) or (always false) + _ = b >= bigUInt; // Compliant, not (always true) or (always false) + _ = bigLong <= b; // Compliant, not (always true) or (always false) + _ = bigULong <= b; // Compliant, not (always true) or (always false) + _ = bigFloat <= b; // Compliant, not (always true) or (always false) + _ = bigDouble <= b; // Compliant, not (always true) or (always false) + _ = bigDecimal <= b; // Compliant, not (always true) or (always false) + + _ = b <= smallByte; // Compliant, not (always true) or (always false) + _ = b <= smallShort; // Compliant, not (always true) or (always false) + _ = b <= smallUShort; // Compliant, not (always true) or (always false) + _ = b <= smallInt; // Compliant, not (always true) or (always false) + _ = b <= smallUInt; // Compliant, not (always true) or (always false) + _ = smallLong >= b; // Compliant, not (always true) or (always false) + _ = smallULong >= b; // Compliant, not (always true) or (always false) + _ = smallFloat >= b; // Compliant, not (always true) or (always false) + _ = smallDouble >= b; // Compliant, not (always true) or (always false) + _ = smallDecimal >= b; // Compliant, not (always true) or (always false) + + const int veryBig = int.MaxValue; + const short verySmall = short.MinValue; + + _ = b < veryBig; // Compliant, raised by CS0652 + _ = veryBig > b; // Compliant, raised by CS0652 + _ = b > verySmall; // Compliant, raised by CS0652 + _ = verySmall < b; // Compliant, raised by CS0652 + _ = verySmall == b; // Compliant, raised by CS0652 + _ = b == veryBig; // Compliant, raised by CS0652 + } + + public void Shorts() + { + const short smallShort = short.MinValue; + const int smallInt = short.MinValue; + const long smallLong = short.MinValue; + const float smallFloat = short.MinValue; + const double smallDouble = short.MinValue; + const decimal smallDecimal = short.MinValue; + + const short bigShort = short.MaxValue; + const int bigInt = short.MaxValue; + const long bigLong = short.MaxValue; + const float bigFloat = short.MaxValue; + const double bigDouble = short.MaxValue; + const decimal bigDecimal = short.MaxValue; + + short s = 42; + + _ = s >= bigShort; // Compliant, not (always true) or (always false) + _ = s >= bigInt; // Compliant, not (always true) or (always false) + _ = bigLong <= s; // Compliant, not (always true) or (always false) + _ = bigFloat <= s; // Compliant, not (always true) or (always false) + _ = bigDouble <= s; // Compliant, not (always true) or (always false) + _ = bigDecimal <= s; // Compliant, not (always true) or (always false) + + _ = s <= smallShort; // Compliant, not (always true) or (always false) + _ = s <= smallInt; // Compliant, not (always true) or (always false) + _ = smallLong >= s; // Compliant, not (always true) or (always false) + _ = smallFloat >= s; // Compliant, not (always true) or (always false) + _ = smallDouble >= s; // Compliant, not (always true) or (always false) + _ = smallDecimal >= s; // Compliant, not (always true) or (always false) + + const int veryBig = int.MaxValue; + const long verySmall = long.MinValue; + + _ = s < veryBig; // Compliant, raised by CS0652 + _ = veryBig > s; // Compliant, raised by CS0652 + _ = s > verySmall; // Compliant, raised by CS0652 + _ = verySmall < s; // Compliant, raised by CS0652 + _ = verySmall == s; // Compliant, raised by CS0652 + _ = s == veryBig; // Compliant, raised by CS0652 + } + + public void UShorts() + { + const ushort smallUShort = ushort.MinValue; + const int smallInt = ushort.MinValue; + const uint smallUInt = ushort.MinValue; + const long smallLong = ushort.MinValue; + const ulong smallULong = ushort.MinValue; + const float smallFloat = ushort.MinValue; + const double smallDouble = ushort.MinValue; + const decimal smallDecimal = ushort.MinValue; + + const ushort bigUShort = ushort.MaxValue; + const int bigInt = ushort.MaxValue; + const uint bigUInt = ushort.MaxValue; + const long bigLong = ushort.MaxValue; + const ulong bigULong = ushort.MaxValue; + const float bigFloat = ushort.MaxValue; + const double bigDouble = ushort.MaxValue; + const decimal bigDecimal = ushort.MaxValue; + + ushort us = 42; + + _ = us >= bigUShort; // Compliant, not (always true) or (always false) + _ = us >= bigInt; // Compliant, not (always true) or (always false) + _ = us >= bigUInt; // Compliant, not (always true) or (always false) + _ = bigLong <= us; // Compliant, not (always true) or (always false) + _ = bigULong <= us; // Compliant, not (always true) or (always false) + _ = bigFloat <= us; // Compliant, not (always true) or (always false) + _ = bigDouble <= us; // Compliant, not (always true) or (always false) + _ = bigDecimal <= us; // Compliant, not (always true) or (always false) + + _ = us <= smallUShort; // Compliant, not (always true) or (always false) + _ = us <= smallInt; // Compliant, not (always true) or (always false) + _ = us <= smallUInt; // Compliant, not (always true) or (always false) + _ = smallLong >= us; // Compliant, not (always true) or (always false) + _ = smallULong >= us; // Compliant, not (always true) or (always false) + _ = smallFloat >= us; // Compliant, not (always true) or (always false) + _ = smallDouble >= us; // Compliant, not (always true) or (always false) + _ = smallDecimal >= us; // Compliant, not (always true) or (always false) + + const int veryBig = int.MaxValue; + const long verySmall = long.MinValue; + + _ = us < veryBig; // Compliant, raised by CS0652 + _ = veryBig > us; // Compliant, raised by CS0652 + _ = us > verySmall; // Compliant, raised by CS0652 + _ = verySmall < us; // Compliant, raised by CS0652 + _ = verySmall == us; // Compliant, raised by CS0652 + _ = us == veryBig; // Compliant, raised by CS0652 + } + + public void Ints() + { + const int smallInt = int.MinValue; + const long smallLong = int.MinValue; + const float smallFloat = int.MinValue; + const double smallDouble = int.MinValue; + const decimal smallDecimal = int.MinValue; + + const int bigInt = int.MaxValue; + const uint bigUInt = int.MaxValue; + const long bigLong = int.MaxValue; + const float bigFloat = int.MaxValue; + const double bigDouble = int.MaxValue; + const decimal bigDecimal = int.MaxValue; + + int i = 42; + + _ = i >= bigInt; // Compliant, not (always true) or (always false) + _ = i >= bigUInt; // Compliant, not (always true) or (always false) + _ = bigLong <= i; // Compliant, not (always true) or (always false) + _ = bigFloat <= i; // Compliant, not (always true) or (always false) + _ = bigDouble <= i; // Compliant, not (always true) or (always false) + _ = bigDecimal <= i; // Compliant, not (always true) or (always false) + + _ = i <= smallInt; // Compliant, not (always true) or (always false) + _ = smallLong >= i; // Compliant, not (always true) or (always false) + _ = smallFloat >= i; // Compliant, not (always true) or (always false) + _ = smallDouble >= i; // Compliant, not (always true) or (always false) + _ = smallDecimal >= i; // Compliant, not (always true) or (always false) + + const long veryBig = long.MaxValue; + const long verySmall = long.MinValue; + + _ = i < veryBig; // Compliant, raised by CS0652 + _ = veryBig > i; // Compliant, raised by CS0652 + _ = i > verySmall; // Compliant, raised by CS0652 + _ = verySmall < i; // Compliant, raised by CS0652 + _ = verySmall == i; // Compliant, raised by CS0652 + _ = i == veryBig; // Compliant, raised by CS0652 + } + + public void UInts() + { + const uint smallUInt = uint.MinValue; + const long smallLong = uint.MinValue; + const ulong smallULong = uint.MinValue; + const float smallFloat = uint.MinValue; + const double smallDouble = uint.MinValue; + const decimal smallDecimal = uint.MinValue; + + const uint bigUInt = uint.MaxValue; + const long bigLong = uint.MaxValue; + const ulong bigULong = uint.MaxValue; + const float bigFloat = uint.MaxValue; + const double bigDouble = uint.MaxValue; + const decimal bigDecimal = uint.MaxValue; + + uint ui = 42; + + _ = ui >= bigUInt; // Compliant, not (always true) or (always false) + _ = bigLong <= ui; // Compliant, not (always true) or (always false) + _ = bigULong <= ui; // Compliant, not (always true) or (always false) + _ = bigFloat <= ui; // Compliant, not (always true) or (always false) + _ = bigDouble <= ui; // Compliant, not (always true) or (always false) + _ = bigDecimal <= ui; // Compliant, not (always true) or (always false) + + _ = ui <= smallUInt; // Compliant, not (always true) or (always false) + _ = smallLong >= ui; // Compliant, not (always true) or (always false) + _ = smallULong >= ui; // Compliant, not (always true) or (always false) + _ = smallFloat >= ui; // Compliant, not (always true) or (always false) + _ = smallDouble >= ui; // Compliant, not (always true) or (always false) + _ = smallDecimal >= ui; // Compliant, not (always true) or (always false) + + const long veryBig = long.MaxValue; + const long verySmall = long.MinValue; + + _ = ui < veryBig; // Compliant, raised by CS0652 + _ = veryBig > ui; // Compliant, raised by CS0652 + _ = ui > verySmall; // Compliant, raised by CS0652 + _ = verySmall < ui; // Compliant, raised by CS0652 + _ = verySmall == ui; // Compliant, raised by CS0652 + _ = ui == veryBig; // Compliant, raised by CS0652 + } + + public void Longs() + { + const long smallLong = long.MinValue; + const float smallFloat = long.MinValue; + const double smallDouble = long.MinValue; + const decimal smallDecimal = long.MinValue; + + const long bigLong = long.MaxValue; + const float bigFloat = long.MaxValue; + const double bigDouble = long.MaxValue; + const decimal bigDecimal = long.MaxValue; + + long l = 42; + + _ = bigLong <= l; // Compliant, not (always true) or (always false) + _ = bigFloat <= l; // Compliant, not (always true) or (always false) + _ = bigDouble <= l; // Compliant, not (always true) or (always false) + _ = bigDecimal <= l; // Compliant, not (always true) or (always false) + + _ = smallLong >= l; // Compliant, not (always true) or (always false) + _ = smallFloat >= l; // Compliant, not (always true) or (always false) + _ = smallDouble >= l; // Compliant, not (always true) or (always false) + _ = smallDecimal >= l; // Compliant, not (always true) or (always false) + + const double veryBig = double.MaxValue; + const double verySmall = double.MinValue; + + _ = l < veryBig; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'long'}} + _ = veryBig > l; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'long'}} + _ = l > verySmall; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'long'}} + _ = verySmall < l; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'long'}} + _ = verySmall == l; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'long'}} + _ = l == veryBig; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'long'}} + } + + public void ULongs() + { + const ulong smallULong = ulong.MinValue; + const float smallFloat = ulong.MinValue; + const double smallDouble = ulong.MinValue; + const decimal smallDecimal = ulong.MinValue; + + const ulong bigULong = ulong.MaxValue; + const float bigFloat = ulong.MaxValue; + const double bigDouble = ulong.MaxValue; + const decimal bigDecimal = ulong.MaxValue; + + ulong ul = 42; + + _ = bigULong <= ul; // Compliant, not (always true) or (always false) + _ = bigFloat <= ul; // Compliant, not (always true) or (always false) + _ = bigDouble <= ul; // Compliant, not (always true) or (always false) + _ = bigDecimal <= ul; // Compliant, not (always true) or (always false) + + _ = smallULong >= ul; // Compliant, not (always true) or (always false) + _ = smallFloat >= ul; // Compliant, not (always true) or (always false) + _ = smallDouble >= ul; // Compliant, not (always true) or (always false) + _ = smallDecimal >= ul; // Compliant, not (always true) or (always false) + + const float veryBig = float.MaxValue; + const float verySmall = float.MinValue; + + _ = ul < veryBig; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'ulong'}} + _ = veryBig > ul; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'ulong'}} + _ = ul > verySmall; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'ulong'}} + _ = verySmall < ul; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'ulong'}} + _ = verySmall == ul; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'ulong'}} + _ = ul == veryBig; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'ulong'}} + } + public void Floats() { - float f = 42; + const float smallFloat = float.MinValue; + const double smallDouble = float.MinValue; + + const float bigFloat = float.MaxValue; + const double bigDouble = float.MaxValue; + + System.Single f = 42; + + _ = bigFloat <= f; // Compliant, not (always true) or (always false) + _ = bigDouble <= f; // Compliant, not (always true) or (always false) + + _ = smallFloat >= f; // Compliant, not (always true) or (always false) + _ = smallDouble >= f; // Compliant, not (always true) or (always false) const double veryBig = double.MaxValue; const double verySmall = double.MinValue; - _ = f < veryBig; // Noncompliant - _ = veryBig > f; // Noncompliant - _ = f > verySmall; // Noncompliant - _ = verySmall < f; // Noncompliant + _ = f < veryBig; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'float'}} + _ = veryBig > f; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'float'}} + _ = f > verySmall; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'float'}} + _ = verySmall < f; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'float'}} + _ = verySmall == f; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'float'}} + _ = f == veryBig; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'float'}} } } } From ba6589f8ab9f71523c975387c0252c28112f775c Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Tue, 7 Feb 2023 16:23:15 +0100 Subject: [PATCH 14/28] Add 'is' pattern testcases, refactored a .ToString to a .ToDisplayString --- .../Rules/SillyMathematicalComparison.cs | 2 +- .../Rules/SillyMathematicalComparison.cs | 4 ++++ .../SillyMathematicalComparison.CSharp9.cs | 21 +++++++++++++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.CSharp9.cs diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs index 753e0c69bce..a1bef122e47 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs @@ -51,7 +51,7 @@ private static void CheckOneSide(SonarSyntaxNodeReportingContext context, Syntax { if (ShouldRaise(context.SemanticModel, first, second)) { - var typeName = context.SemanticModel.GetTypeInfo(second).Type.ToString(); + var typeName = context.SemanticModel.GetTypeInfo(second).Type.ToDisplayString(); context.ReportIssue(Diagnostic.Create(Rule, first.Parent.GetLocation(), typeName)); } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SillyMathematicalComparison.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SillyMathematicalComparison.cs index b1aa67313d2..ae4d9ab8f52 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SillyMathematicalComparison.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SillyMathematicalComparison.cs @@ -30,5 +30,9 @@ public class SillyMathematicalComparisonTest [TestMethod] public void SillyMathematicalComparison_CS() => builderCS.AddPaths("SillyMathematicalComparison.cs").Verify(); + + [TestMethod] + public void SillyMathematicalComparison_CSharp9() => + builderCS.AddPaths("SillyMathematicalComparison.CSharp9.cs").WithOptions(ParseOptionsHelper.FromCSharp9).Verify(); } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.CSharp9.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.CSharp9.cs new file mode 100644 index 00000000000..afd99a0c7b7 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.CSharp9.cs @@ -0,0 +1,21 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace SonarAnalyzer.UnitTest.TestCases +{ + internal class SillyMathematicalComparison + { + public void IsPatterns() + { + _ = 33 is 55; // Compliant + _ = 33 is < 55; // Compliant + _ = 33 is <= 55; // Compliant + _ = 33 is > 55; // Compliant + _ = 33 is >= 55; // Compliant + _ = 33 is not 55; // Compliant + } + } +} From d46386435766ca1078f5d79a518634b18a7d6673 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Tue, 7 Feb 2023 16:27:07 +0100 Subject: [PATCH 15/28] Simple rename --- .../SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs index a1bef122e47..4df3e64352d 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs @@ -58,8 +58,8 @@ private static void CheckOneSide(SonarSyntaxNodeReportingContext context, Syntax private static bool ShouldRaise(SemanticModel model, SyntaxNode expectedConstant, SyntaxNode other) => TryGetConstant(model, expectedConstant, out var constant) - && model.GetTypeInfo(other).Type is { } symbol - && TryGetRange(symbol, out var min, out var max) + && model.GetTypeInfo(other).Type is { } typeSymbolOfOther + && TryGetRange(typeSymbolOfOther, out var min, out var max) && (constant < min || constant > max); private static bool TryGetConstant(SemanticModel model, SyntaxNode expectedConstant, out double constant) From 4f0720a9e69636aeed9904a1673c03df13f957ff Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Tue, 7 Feb 2023 16:55:16 +0100 Subject: [PATCH 16/28] Implement a constant-in-both-sides version of the rule. --- .../Rules/SillyMathematicalComparison.cs | 58 +++++++++---------- .../TestCases/SillyMathematicalComparison.cs | 39 ++++++++++--- 2 files changed, 58 insertions(+), 39 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs index 4df3e64352d..a0fbb420401 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs @@ -24,11 +24,13 @@ namespace SonarAnalyzer.Rules.CSharp public sealed class SillyMathematicalComparison : SonarDiagnosticAnalyzer { private const string DiagnosticId = "S2198"; - private const string MessageFormat = "Comparison to this constant is useless; the constant is outside the range of type '{0}'"; + private const string MathComparisonMessage = "Comparison to this constant is useless; the constant is outside the range of type '{0}'"; + private const string ConstantComparisonMessage = "Don't compare constant values"; - private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat); + private static readonly DiagnosticDescriptor MathComparisonRule = DescriptorFactory.Create(DiagnosticId, MathComparisonMessage); + private static readonly DiagnosticDescriptor ConstantComparisonRule = DescriptorFactory.Create(DiagnosticId, ConstantComparisonMessage); - public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(MathComparisonRule, ConstantComparisonRule); protected override void Initialize(SonarAnalysisContext context) => context.RegisterNodeAction( @@ -43,43 +45,39 @@ public sealed class SillyMathematicalComparison : SonarDiagnosticAnalyzer private static void CheckBinary(SonarSyntaxNodeReportingContext context) { var binary = (BinaryExpressionSyntax)context.Node; - CheckOneSide(context, binary.Left, binary.Right); - CheckOneSide(context, binary.Right, binary.Left); - } - private static void CheckOneSide(SonarSyntaxNodeReportingContext context, SyntaxNode first, SyntaxNode second) - { - if (ShouldRaise(context.SemanticModel, first, second)) + var constantLeft = binary.Left.FindConstantValue(context.SemanticModel); + var constantRight = binary.Right.FindConstantValue(context.SemanticModel); + + if (constantLeft is not null) + { + if (constantRight is not null) // both are constants + { + context.ReportIssue(Diagnostic.Create(ConstantComparisonRule, binary.GetLocation())); + } + else // left is constant + { + CheckOneSide(context, constantLeft, binary.Right); + } + } + else if (constantRight is not null) // right is constant { - var typeName = context.SemanticModel.GetTypeInfo(second).Type.ToDisplayString(); - context.ReportIssue(Diagnostic.Create(Rule, first.Parent.GetLocation(), typeName)); + CheckOneSide(context, constantRight, binary.Left); } } - private static bool ShouldRaise(SemanticModel model, SyntaxNode expectedConstant, SyntaxNode other) => - TryGetConstant(model, expectedConstant, out var constant) - && model.GetTypeInfo(other).Type is { } typeSymbolOfOther - && TryGetRange(typeSymbolOfOther, out var min, out var max) - && (constant < min || constant > max); - - private static bool TryGetConstant(SemanticModel model, SyntaxNode expectedConstant, out double constant) + private static void CheckOneSide(SonarSyntaxNodeReportingContext context, object constant, SyntaxNode other) { - if (FindConstant(model, expectedConstant) is { } constValue) + if (ConversionHelper.TryConvertWith(constant, Convert.ToDouble, out var doubleConstant) + && context.SemanticModel.GetTypeInfo(other).Type is { } typeSymbolOfOther + && TryGetRange(typeSymbolOfOther, out var min, out var max) + && (doubleConstant < min || doubleConstant > max)) { - constant = constValue; - return true; + var typeName = context.SemanticModel.GetTypeInfo(other).Type.ToDisplayString(); + context.ReportIssue(Diagnostic.Create(MathComparisonRule, other.Parent.GetLocation(), typeName)); } - - constant = default; - return false; } - private static double? FindConstant(SemanticModel semanticModel, SyntaxNode node) => - node.FindConstantValue(semanticModel) is { } value - && ConversionHelper.TryConvertWith(value, Convert.ToDouble, out var typedValue) - ? typedValue - : null; - private static bool TryGetRange(ITypeSymbol typeSymbol, out double min, out double max) { if (typeSymbol.Is(KnownType.System_Single)) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs index 7d97c8f9f5f..7eadd67d66b 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs @@ -8,6 +8,8 @@ namespace SonarAnalyzer.UnitTest.TestCases { internal class SillyMathematicalComparison { + private T GetValue() => default(T); + public void SBytes() { const sbyte smallSByte = sbyte.MinValue; @@ -26,7 +28,7 @@ public void SBytes() const double bigDouble = sbyte.MaxValue; const decimal bigDecimal = sbyte.MaxValue; - sbyte sb = 42; + var sb = GetValue(); _ = sb >= bigSByte; // Compliant, not (always true) or (always false) _ = sb >= bigShort; // Compliant, not (always true) or (always false) @@ -79,7 +81,7 @@ public void Bytes() const double bigDouble = byte.MaxValue; const decimal bigDecimal = byte.MaxValue; - byte b = 42; + var b = GetValue(); _ = b >= bigByte; // Compliant, not (always true) or (always false) _ = b >= bigShort; // Compliant, not (always true) or (always false) @@ -130,7 +132,7 @@ public void Shorts() const double bigDouble = short.MaxValue; const decimal bigDecimal = short.MaxValue; - short s = 42; + var s = GetValue(); _ = s >= bigShort; // Compliant, not (always true) or (always false) _ = s >= bigInt; // Compliant, not (always true) or (always false) @@ -177,7 +179,7 @@ public void UShorts() const double bigDouble = ushort.MaxValue; const decimal bigDecimal = ushort.MaxValue; - ushort us = 42; + var us = GetValue(); _ = us >= bigUShort; // Compliant, not (always true) or (always false) _ = us >= bigInt; // Compliant, not (always true) or (always false) @@ -223,7 +225,7 @@ public void Ints() const double bigDouble = int.MaxValue; const decimal bigDecimal = int.MaxValue; - int i = 42; + var i = GetValue(); _ = i >= bigInt; // Compliant, not (always true) or (always false) _ = i >= bigUInt; // Compliant, not (always true) or (always false) @@ -265,7 +267,7 @@ public void UInts() const double bigDouble = uint.MaxValue; const decimal bigDecimal = uint.MaxValue; - uint ui = 42; + var ui = GetValue(); _ = ui >= bigUInt; // Compliant, not (always true) or (always false) _ = bigLong <= ui; // Compliant, not (always true) or (always false) @@ -304,7 +306,7 @@ public void Longs() const double bigDouble = long.MaxValue; const decimal bigDecimal = long.MaxValue; - long l = 42; + var l = GetValue(); _ = bigLong <= l; // Compliant, not (always true) or (always false) _ = bigFloat <= l; // Compliant, not (always true) or (always false) @@ -339,7 +341,7 @@ public void ULongs() const double bigDouble = ulong.MaxValue; const decimal bigDecimal = ulong.MaxValue; - ulong ul = 42; + var ul = GetValue(); _ = bigULong <= ul; // Compliant, not (always true) or (always false) _ = bigFloat <= ul; // Compliant, not (always true) or (always false) @@ -370,7 +372,7 @@ public void Floats() const float bigFloat = float.MaxValue; const double bigDouble = float.MaxValue; - System.Single f = 42; + System.Single f = GetValue(); _ = bigFloat <= f; // Compliant, not (always true) or (always false) _ = bigDouble <= f; // Compliant, not (always true) or (always false) @@ -388,5 +390,24 @@ public void Floats() _ = verySmall == f; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'float'}} _ = f == veryBig; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'float'}} } + + public void ConstantInBothOperands() + { + _ = "Happy" == "Sad"; // Noncompliant {{Don't compare constant values}} + _ = "Ipsum" != "Lorem"; // Noncompliant {{Don't compare constant values}} + + var george = true; + var boole = false; + + _ = george == boole; // Noncompliant {{Don't compare constant values}} + + var number1 = 128; + var number2 = 42; + + _ = 42 == 256; // Noncompliant {{Don't compare constant values}} + _ = number1 >= 256; // Noncompliant {{Don't compare constant values}} + _ = 42 <= number2; // Noncompliant {{Don't compare constant values}} + _ = number1 != number2; // Noncompliant {{Don't compare constant values}} + } } } From 0070a96f389595db8ef4f0dff2f3ab0540a1f287 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Wed, 8 Feb 2023 11:03:45 +0100 Subject: [PATCH 17/28] Apply PR Review changes, some constant-check fixes and more testcases --- .../Rules/SillyMathematicalComparison.cs | 25 ++++---- .../TestCases/SillyMathematicalComparison.cs | 59 +++++++++++++++---- 2 files changed, 59 insertions(+), 25 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs index a0fbb420401..7e9df3c3300 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs @@ -46,23 +46,20 @@ private static void CheckBinary(SonarSyntaxNodeReportingContext context) { var binary = (BinaryExpressionSyntax)context.Node; - var constantLeft = binary.Left.FindConstantValue(context.SemanticModel); - var constantRight = binary.Right.FindConstantValue(context.SemanticModel); + var constantLeft = context.SemanticModel.GetConstantValue(binary.Left); + var constantRight = context.SemanticModel.GetConstantValue(binary.Right); - if (constantLeft is not null) + if (constantLeft.HasValue && constantRight.HasValue) { - if (constantRight is not null) // both are constants - { - context.ReportIssue(Diagnostic.Create(ConstantComparisonRule, binary.GetLocation())); - } - else // left is constant - { - CheckOneSide(context, constantLeft, binary.Right); - } + context.ReportIssue(Diagnostic.Create(ConstantComparisonRule, binary.GetLocation())); } - else if (constantRight is not null) // right is constant + else if (constantLeft.HasValue) { - CheckOneSide(context, constantRight, binary.Left); + CheckOneSide(context, constantLeft.Value, binary.Right); + } + else if (constantRight.HasValue) + { + CheckOneSide(context, constantRight.Value, binary.Left); } } @@ -73,7 +70,7 @@ private static void CheckOneSide(SonarSyntaxNodeReportingContext context, object && TryGetRange(typeSymbolOfOther, out var min, out var max) && (doubleConstant < min || doubleConstant > max)) { - var typeName = context.SemanticModel.GetTypeInfo(other).Type.ToDisplayString(); + var typeName = context.SemanticModel.GetTypeInfo(other).Type.ToMinimalDisplayString(context.SemanticModel, other.GetLocation().SourceSpan.Start); context.ReportIssue(Diagnostic.Create(MathComparisonRule, other.Parent.GetLocation(), typeName)); } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs index 7eadd67d66b..0022a3e1ff6 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs @@ -341,7 +341,7 @@ public void ULongs() const double bigDouble = ulong.MaxValue; const decimal bigDecimal = ulong.MaxValue; - var ul = GetValue(); + ulong ul = 42; _ = bigULong <= ul; // Compliant, not (always true) or (always false) _ = bigFloat <= ul; // Compliant, not (always true) or (always false) @@ -393,21 +393,58 @@ public void Floats() public void ConstantInBothOperands() { - _ = "Happy" == "Sad"; // Noncompliant {{Don't compare constant values}} + const string s1 = "Yes"; + const string s2 = "No"; + const bool b1 = false; + const bool b2 = true; + const float f1 = 42; + const float f2 = 256; + + _ = s1 == s2; // Noncompliant {{Don't compare constant values}} + _ = s1 != s2; // Noncompliant {{Don't compare constant values}} + _ = s1 != "Lorum"; // Noncompliant {{Don't compare constant values}} + _ = "Ipsem" != s2; // Noncompliant {{Don't compare constant values}} + _ = "Ipsem" == "Lorum"; // Noncompliant {{Don't compare constant values}} _ = "Ipsum" != "Lorem"; // Noncompliant {{Don't compare constant values}} - var george = true; - var boole = false; + _ = b1 == b2; // Noncompliant {{Don't compare constant values}} + _ = b1 != b2; // Noncompliant {{Don't compare constant values}} + _ = true != b2; // Noncompliant {{Don't compare constant values}} + _ = b1 == false; // Noncompliant {{Don't compare constant values}} - _ = george == boole; // Noncompliant {{Don't compare constant values}} + _ = f1 != f2; // Noncompliant {{Don't compare constant values}} + _ = f1 == f2; // Noncompliant {{Don't compare constant values}} + _ = f1 <= 0; // Noncompliant {{Don't compare constant values}} + _ = 128 >= f2; // Noncompliant {{Don't compare constant values}} - var number1 = 128; - var number2 = 42; + _ = 42 != 300; // Noncompliant {{Don't compare constant values}} + _ = 42 != 256; // Noncompliant {{Don't compare constant values}} + } - _ = 42 == 256; // Noncompliant {{Don't compare constant values}} - _ = number1 >= 256; // Noncompliant {{Don't compare constant values}} - _ = 42 <= number2; // Noncompliant {{Don't compare constant values}} - _ = number1 != number2; // Noncompliant {{Don't compare constant values}} + public void NotConstantInBothOperands() + { + string s1 = "Yes"; + bool b1 = false; + float f1 = GetValue(); + + const string s2 = "No"; + const bool b2 = true; + const float f2 = 256; + + _ = s1 == s2; // Compliant + _ = s1 != s2; // Compliant + _ = s1 != "Lorum"; // Compliant + _ = "Ipsem" == s1; // Compliant + + _ = b1 == b2; // Compliant + _ = b1 != b2; // Compliant + _ = true != b1; // Compliant + _ = b1 == false; // Compliant + + _ = f1 != f2; // Compliant + _ = f1 == f2; // Compliant + _ = f1 <= 0; // Compliant + _ = 128 >= f1; // Compliant } } } From e3b6f2422a7335c499fb2711f2c202dae3d8b0fe Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Wed, 8 Feb 2023 11:49:58 +0100 Subject: [PATCH 18/28] Add enum, char and reference type to the testcases --- .../TestCases/SillyMathematicalComparison.cs | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs index 0022a3e1ff6..22ef6439a40 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs @@ -399,6 +399,13 @@ public void ConstantInBothOperands() const bool b2 = true; const float f1 = 42; const float f2 = 256; + const MyEnum e1 = MyEnum.One; + const MyEnum e2 = MyEnum.Two; + const char c1 = 'a'; + const char c2 = '1'; + + const Thingy constReference1 = null; + const Thingy constReference2 = null; _ = s1 == s2; // Noncompliant {{Don't compare constant values}} _ = s1 != s2; // Noncompliant {{Don't compare constant values}} @@ -419,6 +426,21 @@ public void ConstantInBothOperands() _ = 42 != 300; // Noncompliant {{Don't compare constant values}} _ = 42 != 256; // Noncompliant {{Don't compare constant values}} + + _ = e1 != e2; // Noncompliant {{Don't compare constant values}} + _ = e1 == e2; // Noncompliant {{Don't compare constant values}} + _ = e1 <= 0; // Noncompliant {{Don't compare constant values}} + _ = MyEnum.One >= e2; // Noncompliant {{Don't compare constant values}} + + _ = c1 != c2; // Noncompliant {{Don't compare constant values}} + _ = c1 == c2; // Noncompliant {{Don't compare constant values}} + _ = c1 <= 0; // Noncompliant {{Don't compare constant values}} + _ = 'b' >= c2; // Noncompliant {{Don't compare constant values}} + + _ = constReference1 == constReference2; // Noncompliant {{Don't compare constant values}} + _ = constReference1 != constReference2; // Noncompliant {{Don't compare constant values}} + _ = null != constReference2; // Noncompliant {{Don't compare constant values}} + _ = constReference1 == null; // Noncompliant {{Don't compare constant values}} } public void NotConstantInBothOperands() @@ -426,10 +448,17 @@ public void NotConstantInBothOperands() string s1 = "Yes"; bool b1 = false; float f1 = GetValue(); + MyEnum e1 = GetValue(); + char c1 = 'a'; + Thingy reference = new(); + const string s2 = "No"; const bool b2 = true; const float f2 = 256; + const MyEnum e2 = MyEnum.Two; + const char c2 = '1'; + const Thingy constReference = null; _ = s1 == s2; // Compliant _ = s1 != s2; // Compliant @@ -445,6 +474,24 @@ public void NotConstantInBothOperands() _ = f1 == f2; // Compliant _ = f1 <= 0; // Compliant _ = 128 >= f1; // Compliant + + _ = e1 != e2; // Compliant + _ = e1 == e2; // Compliant + _ = e1 <= 0; // Compliant + _ = MyEnum.One >= e1; // Compliant + + _ = c1 != c2; // Compliant + _ = c1 == c2; // Compliant + _ = c1 <= 0; // Compliant + _ = 'b' >= c1; // Compliant + + _ = reference == constReference; // Compliant + _ = constReference != reference; // Compliant + _ = null != reference; // Compliant + _ = reference == null; // Compliant } + + private enum MyEnum { One, Two } + private sealed class Thingy { } } } From a84d4413cd89db6402d03cb2976e47941b8488ce Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Wed, 8 Feb 2023 15:13:22 +0100 Subject: [PATCH 19/28] Simplify the rule, only care about out-of-range mathematical checks --- .../Rules/SillyMathematicalComparison.cs | 52 ++++----- .../TestCases/SillyMathematicalComparison.cs | 103 ------------------ 2 files changed, 27 insertions(+), 128 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs index 7e9df3c3300..723281ab6c7 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs @@ -25,12 +25,10 @@ public sealed class SillyMathematicalComparison : SonarDiagnosticAnalyzer { private const string DiagnosticId = "S2198"; private const string MathComparisonMessage = "Comparison to this constant is useless; the constant is outside the range of type '{0}'"; - private const string ConstantComparisonMessage = "Don't compare constant values"; private static readonly DiagnosticDescriptor MathComparisonRule = DescriptorFactory.Create(DiagnosticId, MathComparisonMessage); - private static readonly DiagnosticDescriptor ConstantComparisonRule = DescriptorFactory.Create(DiagnosticId, ConstantComparisonMessage); - public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(MathComparisonRule, ConstantComparisonRule); + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(MathComparisonRule); protected override void Initialize(SonarAnalysisContext context) => context.RegisterNodeAction( @@ -44,35 +42,39 @@ public sealed class SillyMathematicalComparison : SonarDiagnosticAnalyzer private static void CheckBinary(SonarSyntaxNodeReportingContext context) { - var binary = (BinaryExpressionSyntax)context.Node; - - var constantLeft = context.SemanticModel.GetConstantValue(binary.Left); - var constantRight = context.SemanticModel.GetConstantValue(binary.Right); - - if (constantLeft.HasValue && constantRight.HasValue) - { - context.ReportIssue(Diagnostic.Create(ConstantComparisonRule, binary.GetLocation())); - } - else if (constantLeft.HasValue) - { - CheckOneSide(context, constantLeft.Value, binary.Right); - } - else if (constantRight.HasValue) + if (TryGetConstantValue(context, (BinaryExpressionSyntax)context.Node, out var constant, out var other) + && context.SemanticModel.GetTypeInfo(other).Type is { } typeSymbolOfOther + && TryGetRange(typeSymbolOfOther, out var min, out var max) + && (constant < min || constant > max)) { - CheckOneSide(context, constantRight.Value, binary.Left); + var typeName = context.SemanticModel.GetTypeInfo(other).Type.ToMinimalDisplayString(context.SemanticModel, other.GetLocation().SourceSpan.Start); + context.ReportIssue(Diagnostic.Create(MathComparisonRule, other.Parent.GetLocation(), typeName)); } } - private static void CheckOneSide(SonarSyntaxNodeReportingContext context, object constant, SyntaxNode other) + private static bool TryGetConstantValue(SonarSyntaxNodeReportingContext context, BinaryExpressionSyntax binary, out double constant, out SyntaxNode other) { - if (ConversionHelper.TryConvertWith(constant, Convert.ToDouble, out var doubleConstant) - && context.SemanticModel.GetTypeInfo(other).Type is { } typeSymbolOfOther - && TryGetRange(typeSymbolOfOther, out var min, out var max) - && (doubleConstant < min || doubleConstant > max)) + constant = default; + other = default; + var maybeLeft = context.SemanticModel.GetConstantValue(binary.Left); + var maybeRight = context.SemanticModel.GetConstantValue(binary.Right); + + if (maybeLeft.HasValue ^ maybeRight.HasValue) { - var typeName = context.SemanticModel.GetTypeInfo(other).Type.ToMinimalDisplayString(context.SemanticModel, other.GetLocation().SourceSpan.Start); - context.ReportIssue(Diagnostic.Create(MathComparisonRule, other.Parent.GetLocation(), typeName)); + if (maybeLeft.HasValue) + { + ConversionHelper.TryConvertWith(maybeLeft.Value, Convert.ToDouble, out constant); + other = binary.Right; + return true; + } + else + { + ConversionHelper.TryConvertWith(maybeRight.Value, Convert.ToDouble, out constant); + other = binary.Left; + return true; + } } + return false; } private static bool TryGetRange(ITypeSymbol typeSymbol, out double min, out double max) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs index 22ef6439a40..b7757c31341 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs @@ -390,108 +390,5 @@ public void Floats() _ = verySmall == f; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'float'}} _ = f == veryBig; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'float'}} } - - public void ConstantInBothOperands() - { - const string s1 = "Yes"; - const string s2 = "No"; - const bool b1 = false; - const bool b2 = true; - const float f1 = 42; - const float f2 = 256; - const MyEnum e1 = MyEnum.One; - const MyEnum e2 = MyEnum.Two; - const char c1 = 'a'; - const char c2 = '1'; - - const Thingy constReference1 = null; - const Thingy constReference2 = null; - - _ = s1 == s2; // Noncompliant {{Don't compare constant values}} - _ = s1 != s2; // Noncompliant {{Don't compare constant values}} - _ = s1 != "Lorum"; // Noncompliant {{Don't compare constant values}} - _ = "Ipsem" != s2; // Noncompliant {{Don't compare constant values}} - _ = "Ipsem" == "Lorum"; // Noncompliant {{Don't compare constant values}} - _ = "Ipsum" != "Lorem"; // Noncompliant {{Don't compare constant values}} - - _ = b1 == b2; // Noncompliant {{Don't compare constant values}} - _ = b1 != b2; // Noncompliant {{Don't compare constant values}} - _ = true != b2; // Noncompliant {{Don't compare constant values}} - _ = b1 == false; // Noncompliant {{Don't compare constant values}} - - _ = f1 != f2; // Noncompliant {{Don't compare constant values}} - _ = f1 == f2; // Noncompliant {{Don't compare constant values}} - _ = f1 <= 0; // Noncompliant {{Don't compare constant values}} - _ = 128 >= f2; // Noncompliant {{Don't compare constant values}} - - _ = 42 != 300; // Noncompliant {{Don't compare constant values}} - _ = 42 != 256; // Noncompliant {{Don't compare constant values}} - - _ = e1 != e2; // Noncompliant {{Don't compare constant values}} - _ = e1 == e2; // Noncompliant {{Don't compare constant values}} - _ = e1 <= 0; // Noncompliant {{Don't compare constant values}} - _ = MyEnum.One >= e2; // Noncompliant {{Don't compare constant values}} - - _ = c1 != c2; // Noncompliant {{Don't compare constant values}} - _ = c1 == c2; // Noncompliant {{Don't compare constant values}} - _ = c1 <= 0; // Noncompliant {{Don't compare constant values}} - _ = 'b' >= c2; // Noncompliant {{Don't compare constant values}} - - _ = constReference1 == constReference2; // Noncompliant {{Don't compare constant values}} - _ = constReference1 != constReference2; // Noncompliant {{Don't compare constant values}} - _ = null != constReference2; // Noncompliant {{Don't compare constant values}} - _ = constReference1 == null; // Noncompliant {{Don't compare constant values}} - } - - public void NotConstantInBothOperands() - { - string s1 = "Yes"; - bool b1 = false; - float f1 = GetValue(); - MyEnum e1 = GetValue(); - char c1 = 'a'; - Thingy reference = new(); - - - const string s2 = "No"; - const bool b2 = true; - const float f2 = 256; - const MyEnum e2 = MyEnum.Two; - const char c2 = '1'; - const Thingy constReference = null; - - _ = s1 == s2; // Compliant - _ = s1 != s2; // Compliant - _ = s1 != "Lorum"; // Compliant - _ = "Ipsem" == s1; // Compliant - - _ = b1 == b2; // Compliant - _ = b1 != b2; // Compliant - _ = true != b1; // Compliant - _ = b1 == false; // Compliant - - _ = f1 != f2; // Compliant - _ = f1 == f2; // Compliant - _ = f1 <= 0; // Compliant - _ = 128 >= f1; // Compliant - - _ = e1 != e2; // Compliant - _ = e1 == e2; // Compliant - _ = e1 <= 0; // Compliant - _ = MyEnum.One >= e1; // Compliant - - _ = c1 != c2; // Compliant - _ = c1 == c2; // Compliant - _ = c1 <= 0; // Compliant - _ = 'b' >= c1; // Compliant - - _ = reference == constReference; // Compliant - _ = constReference != reference; // Compliant - _ = null != reference; // Compliant - _ = reference == null; // Compliant - } - - private enum MyEnum { One, Two } - private sealed class Thingy { } } } From 4856e966eac0dca62fce55feafa54e829f655797 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Wed, 8 Feb 2023 15:34:10 +0100 Subject: [PATCH 20/28] Add char out-of-range detection --- .../Rules/SillyMathematicalComparison.cs | 18 ++++-- .../TestCases/SillyMathematicalComparison.cs | 62 ++++++++++++++++--- 2 files changed, 67 insertions(+), 13 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs index 723281ab6c7..bdbb6672cf2 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs @@ -63,22 +63,32 @@ private static bool TryGetConstantValue(SonarSyntaxNodeReportingContext context, { if (maybeLeft.HasValue) { - ConversionHelper.TryConvertWith(maybeLeft.Value, Convert.ToDouble, out constant); other = binary.Right; - return true; + return TryConvertToDouble(maybeLeft.Value, out constant); } else { - ConversionHelper.TryConvertWith(maybeRight.Value, Convert.ToDouble, out constant); other = binary.Left; - return true; + return TryConvertToDouble(maybeRight.Value, out constant); } } return false; } + // 'char' needs to roundtrip {{char -> int -> double}}, can't go {{char -> double}} + private static bool TryConvertToDouble(object constant, out double typedConstant) => + constant is char + ? ConversionHelper.TryConvertWith(Convert.ToInt32(constant), Convert.ToDouble, out typedConstant) + : ConversionHelper.TryConvertWith(constant, Convert.ToDouble, out typedConstant); + private static bool TryGetRange(ITypeSymbol typeSymbol, out double min, out double max) { + if (typeSymbol.Is(KnownType.System_Char)) + { + min = char.MinValue; + max = char.MaxValue; + return true; + } if (typeSymbol.Is(KnownType.System_Single)) { min = float.MinValue; diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs index b7757c31341..1ad14326fa5 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Security.Cryptography.X509Certificates; using System.Text; using System.Threading.Tasks; @@ -10,7 +11,46 @@ internal class SillyMathematicalComparison { private T GetValue() => default(T); - public void SBytes() + public void Chars() // Rule applies + { + const int smallInt = char.MinValue; + const long smallLong = char.MinValue; + const float smallFloat = char.MinValue; + const double smallDouble = char.MinValue; + const decimal smallDecimal = char.MinValue; + + const int bigInt = char.MaxValue; + const long bigLong = char.MaxValue; + const float bigFloat = char.MaxValue; + const double bigDouble = char.MaxValue; + const decimal bigDecimal = char.MaxValue; + + var c = GetValue(); + + _ = c >= bigInt; // Compliant, not (always true) or (always false) + _ = bigLong <= c; // Compliant, not (always true) or (always false) + _ = bigFloat <= c; // Compliant, not (always true) or (always false) + _ = bigDouble <= c; // Compliant, not (always true) or (always false) + _ = bigDecimal <= c; // Compliant, not (always true) or (always false) + + _ = c <= smallInt; // Compliant, not (always true) or (always false) + _ = smallLong >= c; // Compliant, not (always true) or (always false) + _ = smallFloat >= c; // Compliant, not (always true) or (always false) + _ = smallDouble >= c; // Compliant, not (always true) or (always false) + _ = smallDecimal >= c; // Compliant, not (always true) or (always false) + + const int veryBig = int.MaxValue; + const long verySmall = long.MinValue; + + _ = c < veryBig; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'char'}} + _ = veryBig > c; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'char'}} + _ = c > verySmall; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'char'}} + _ = verySmall < c; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'char'}} + _ = verySmall == c; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'char'}} + _ = c == veryBig; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'char'}} + } + + public void SBytes() // CS0652 { const sbyte smallSByte = sbyte.MinValue; const short smallShort = sbyte.MinValue; @@ -57,7 +97,7 @@ public void SBytes() _ = sb == veryBig; // Compliant, raised by CS0652 } - public void Bytes() + public void Bytes() // CS0652 { const byte smallByte = byte.MinValue; const short smallShort = byte.MinValue; @@ -116,7 +156,7 @@ public void Bytes() _ = b == veryBig; // Compliant, raised by CS0652 } - public void Shorts() + public void Shorts() // CS0652 { const short smallShort = short.MinValue; const int smallInt = short.MinValue; @@ -159,7 +199,7 @@ public void Shorts() _ = s == veryBig; // Compliant, raised by CS0652 } - public void UShorts() + public void UShorts() // CS0652 { const ushort smallUShort = ushort.MinValue; const int smallInt = ushort.MinValue; @@ -210,7 +250,7 @@ public void UShorts() _ = us == veryBig; // Compliant, raised by CS0652 } - public void Ints() + public void Ints() // CS0652 { const int smallInt = int.MinValue; const long smallLong = int.MinValue; @@ -251,7 +291,7 @@ public void Ints() _ = i == veryBig; // Compliant, raised by CS0652 } - public void UInts() + public void UInts() // CS0652 { const uint smallUInt = uint.MinValue; const long smallLong = uint.MinValue; @@ -294,7 +334,7 @@ public void UInts() _ = ui == veryBig; // Compliant, raised by CS0652 } - public void Longs() + public void Longs() // Rule applies { const long smallLong = long.MinValue; const float smallFloat = long.MinValue; @@ -329,7 +369,7 @@ public void Longs() _ = l == veryBig; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'long'}} } - public void ULongs() + public void ULongs() // Rule applies { const ulong smallULong = ulong.MinValue; const float smallFloat = ulong.MinValue; @@ -364,7 +404,7 @@ public void ULongs() _ = ul == veryBig; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'ulong'}} } - public void Floats() + public void Floats() // Rule applies { const float smallFloat = float.MinValue; const double smallDouble = float.MinValue; @@ -372,6 +412,10 @@ public void Floats() const float bigFloat = float.MaxValue; const double bigDouble = float.MaxValue; + + const long l = long.MaxValue; + var i = 42; + System.Single f = GetValue(); _ = bigFloat <= f; // Compliant, not (always true) or (always false) From 51d7b9c4b7a5ae7ef989c155b0ddebac3051d1bb Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Wed, 8 Feb 2023 15:47:55 +0100 Subject: [PATCH 21/28] Added some true negatives for const1 {{operator}} const2 --- .../TestCases/SillyMathematicalComparison.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs index 1ad14326fa5..04169bee714 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs @@ -434,5 +434,19 @@ internal class SillyMathematicalComparison _ = verySmall == f; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'float'}} _ = f == veryBig; // Noncompliant {{Comparison to this constant is useless; the constant is outside the range of type 'float'}} } + + public void ConstantInBothOperands() + { + const float number1 = 42; + const double number2 = double.MaxValue; + + _ = number1 != number2; // Compliant, constant in both operands + _ = number1 == number2; // Compliant + _ = number1 <= double.MaxValue; // Compliant + _ = double.MaxValue >= number2; // Compliant + + _ = 42f != double.MaxValue; // Compliant + _ = 42f <= double.MaxValue; // Compliant + } } } From f79b42ae7d5b412007b41c621d62842c33d15517 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Wed, 8 Feb 2023 15:54:21 +0100 Subject: [PATCH 22/28] Bump coverage by adding a case I forgot about --- .../TestCases/SillyMathematicalComparison.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs index 04169bee714..803fb591ee5 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs @@ -13,12 +13,14 @@ internal class SillyMathematicalComparison public void Chars() // Rule applies { + const char smallChar = char.MinValue; const int smallInt = char.MinValue; const long smallLong = char.MinValue; const float smallFloat = char.MinValue; const double smallDouble = char.MinValue; const decimal smallDecimal = char.MinValue; + const char bigChar = char.MaxValue; const int bigInt = char.MaxValue; const long bigLong = char.MaxValue; const float bigFloat = char.MaxValue; @@ -27,12 +29,14 @@ internal class SillyMathematicalComparison var c = GetValue(); + _ = c >= bigChar; // Compliant, not (always true) or (always false) _ = c >= bigInt; // Compliant, not (always true) or (always false) _ = bigLong <= c; // Compliant, not (always true) or (always false) _ = bigFloat <= c; // Compliant, not (always true) or (always false) _ = bigDouble <= c; // Compliant, not (always true) or (always false) _ = bigDecimal <= c; // Compliant, not (always true) or (always false) + _ = c <= smallChar; // Compliant, not (always true) or (always false) _ = c <= smallInt; // Compliant, not (always true) or (always false) _ = smallLong >= c; // Compliant, not (always true) or (always false) _ = smallFloat >= c; // Compliant, not (always true) or (always false) From 083ae472e1d02598abe7989e2f509ea7420a3067 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Thu, 9 Feb 2023 15:47:28 +0100 Subject: [PATCH 23/28] Apply PR Review changes --- .../Rules/SillyMathematicalComparison.cs | 76 ++++++------------- 1 file changed, 25 insertions(+), 51 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs index bdbb6672cf2..0f4d6127124 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs @@ -28,48 +28,51 @@ public sealed class SillyMathematicalComparison : SonarDiagnosticAnalyzer private static readonly DiagnosticDescriptor MathComparisonRule = DescriptorFactory.Create(DiagnosticId, MathComparisonMessage); + private static readonly Dictionary ValuesRanges = new() + { + {KnownType.System_Char, new ValuesRange(char.MinValue, char.MaxValue) }, + {KnownType.System_Single, new ValuesRange(float.MinValue, float.MaxValue) }, + {KnownType.System_Int64, new ValuesRange(long.MinValue, long.MaxValue) }, + {KnownType.System_UInt64, new ValuesRange(ulong.MinValue, ulong.MaxValue) }, + }; + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(MathComparisonRule); protected override void Initialize(SonarAnalysisContext context) => context.RegisterNodeAction( CheckBinary, - SyntaxKind.GreaterThanExpression, - SyntaxKind.GreaterThanOrEqualExpression, - SyntaxKind.LessThanExpression, - SyntaxKind.LessThanOrEqualExpression, - SyntaxKind.EqualsExpression, - SyntaxKind.NotEqualsExpression); + CSharpFacade.Instance.SyntaxKind.ComparisonKinds); private static void CheckBinary(SonarSyntaxNodeReportingContext context) { - if (TryGetConstantValue(context, (BinaryExpressionSyntax)context.Node, out var constant, out var other) + if (TryGetConstantValue(context.SemanticModel, (BinaryExpressionSyntax)context.Node, out var constant, out var other) && context.SemanticModel.GetTypeInfo(other).Type is { } typeSymbolOfOther - && TryGetRange(typeSymbolOfOther, out var min, out var max) - && (constant < min || constant > max)) + && TryGetRange(typeSymbolOfOther) is { } range + && (constant < range.Min || constant > range.Max)) { - var typeName = context.SemanticModel.GetTypeInfo(other).Type.ToMinimalDisplayString(context.SemanticModel, other.GetLocation().SourceSpan.Start); + var typeName = typeSymbolOfOther.ToMinimalDisplayString(context.SemanticModel, other.GetLocation().SourceSpan.Start); context.ReportIssue(Diagnostic.Create(MathComparisonRule, other.Parent.GetLocation(), typeName)); } } - private static bool TryGetConstantValue(SonarSyntaxNodeReportingContext context, BinaryExpressionSyntax binary, out double constant, out SyntaxNode other) + private static bool TryGetConstantValue(SemanticModel model, BinaryExpressionSyntax binary, out double constant, out SyntaxNode other) { constant = default; other = default; - var maybeLeft = context.SemanticModel.GetConstantValue(binary.Left); - var maybeRight = context.SemanticModel.GetConstantValue(binary.Right); + var optionalLeft = model.GetConstantValue(binary.Left); + var optionalRight = model.GetConstantValue(binary.Right); - if (maybeLeft.HasValue ^ maybeRight.HasValue) + if (optionalLeft.HasValue ^ optionalRight.HasValue) { - if (maybeLeft.HasValue) + if (optionalLeft.HasValue && TryConvertToDouble(optionalLeft.Value, out constant)) { other = binary.Right; - return TryConvertToDouble(maybeLeft.Value, out constant); + return true; } - else + else if (TryConvertToDouble(optionalRight.Value, out constant)) { other = binary.Left; - return TryConvertToDouble(maybeRight.Value, out constant); + return true; } } return false; @@ -77,40 +80,11 @@ private static bool TryGetConstantValue(SonarSyntaxNodeReportingContext context, // 'char' needs to roundtrip {{char -> int -> double}}, can't go {{char -> double}} private static bool TryConvertToDouble(object constant, out double typedConstant) => - constant is char - ? ConversionHelper.TryConvertWith(Convert.ToInt32(constant), Convert.ToDouble, out typedConstant) - : ConversionHelper.TryConvertWith(constant, Convert.ToDouble, out typedConstant); + ConversionHelper.TryConvertWith(constant is char ? Convert.ToInt32(constant) : constant, Convert.ToDouble, out typedConstant); - private static bool TryGetRange(ITypeSymbol typeSymbol, out double min, out double max) - { - if (typeSymbol.Is(KnownType.System_Char)) - { - min = char.MinValue; - max = char.MaxValue; - return true; - } - if (typeSymbol.Is(KnownType.System_Single)) - { - min = float.MinValue; - max = float.MaxValue; - return true; - } - if (typeSymbol.Is(KnownType.System_Int64)) - { - min = long.MinValue; - max = long.MaxValue; - return true; - } - if (typeSymbol.Is(KnownType.System_UInt64)) - { - min = ulong.MinValue; - max = ulong.MaxValue; - return true; - } + private static ValuesRange TryGetRange(ITypeSymbol typeSymbol) => + ValuesRanges.FirstOrDefault(x => typeSymbol.Is(x.Key)).Value; - min = default; - max = default; - return false; - } + private sealed record ValuesRange(double Min, double Max); } } From 0980aa48250d2c05286b81fe558c7881d60a38a1 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Thu, 9 Feb 2023 15:55:47 +0100 Subject: [PATCH 24/28] Add an is-pattern edgecase in the testcases --- .../TestCases/SillyMathematicalComparison.CSharp9.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.CSharp9.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.CSharp9.cs index afd99a0c7b7..bc0f781da1a 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.CSharp9.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.CSharp9.cs @@ -16,6 +16,10 @@ public void IsPatterns() _ = 33 is > 55; // Compliant _ = 33 is >= 55; // Compliant _ = 33 is not 55; // Compliant + + const double d = double.MaxValue; + float x = 42; + _ = x is d; // Error [CS0266] - cannot implicitly convert type 'double' to 'float' } } } From 16c517c9c7165d502973d8199b6fe31596d382d3 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Thu, 9 Feb 2023 18:29:13 +0100 Subject: [PATCH 25/28] Fix a logical condition, refactor TryGetRange --- .../Rules/SillyMathematicalComparison.cs | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs index 0f4d6127124..ade5e9e4050 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs @@ -28,14 +28,6 @@ public sealed class SillyMathematicalComparison : SonarDiagnosticAnalyzer private static readonly DiagnosticDescriptor MathComparisonRule = DescriptorFactory.Create(DiagnosticId, MathComparisonMessage); - private static readonly Dictionary ValuesRanges = new() - { - {KnownType.System_Char, new ValuesRange(char.MinValue, char.MaxValue) }, - {KnownType.System_Single, new ValuesRange(float.MinValue, float.MaxValue) }, - {KnownType.System_Int64, new ValuesRange(long.MinValue, long.MaxValue) }, - {KnownType.System_UInt64, new ValuesRange(ulong.MinValue, ulong.MaxValue) }, - }; - public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(MathComparisonRule); protected override void Initialize(SonarAnalysisContext context) => @@ -57,8 +49,6 @@ private static void CheckBinary(SonarSyntaxNodeReportingContext context) private static bool TryGetConstantValue(SemanticModel model, BinaryExpressionSyntax binary, out double constant, out SyntaxNode other) { - constant = default; - other = default; var optionalLeft = model.GetConstantValue(binary.Left); var optionalRight = model.GetConstantValue(binary.Right); @@ -69,12 +59,14 @@ private static bool TryGetConstantValue(SemanticModel model, BinaryExpressionSyn other = binary.Right; return true; } - else if (TryConvertToDouble(optionalRight.Value, out constant)) + else if (optionalRight.HasValue && TryConvertToDouble(optionalRight.Value, out constant)) { other = binary.Left; return true; } } + constant = default; + other = default; return false; } @@ -82,9 +74,16 @@ private static bool TryGetConstantValue(SemanticModel model, BinaryExpressionSyn private static bool TryConvertToDouble(object constant, out double typedConstant) => ConversionHelper.TryConvertWith(constant is char ? Convert.ToInt32(constant) : constant, Convert.ToDouble, out typedConstant); - private static ValuesRange TryGetRange(ITypeSymbol typeSymbol) => - ValuesRanges.FirstOrDefault(x => typeSymbol.Is(x.Key)).Value; + private static ValuesRange? TryGetRange(ITypeSymbol typeSymbol) => + typeSymbol switch + { + _ when typeSymbol.Is(KnownType.System_Char) => new(char.MinValue, char.MaxValue), + _ when typeSymbol.Is(KnownType.System_Single) => new(float.MinValue, float.MaxValue), + _ when typeSymbol.Is(KnownType.System_Int64) => new(long.MinValue, long.MaxValue), + _ when typeSymbol.Is(KnownType.System_UInt64) => new(ulong.MinValue, ulong.MaxValue), + _ => null, + }; - private sealed record ValuesRange(double Min, double Max); + private readonly record struct ValuesRange(double Min, double Max); } } From a0fd1cefeb77714140dd836311ba46cccd72f3a9 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Thu, 9 Feb 2023 18:31:47 +0100 Subject: [PATCH 26/28] Minor naming refactor --- .../SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs index ade5e9e4050..081a60df311 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs @@ -32,10 +32,10 @@ public sealed class SillyMathematicalComparison : SonarDiagnosticAnalyzer protected override void Initialize(SonarAnalysisContext context) => context.RegisterNodeAction( - CheckBinary, + CheckComparisonOutOfRange, CSharpFacade.Instance.SyntaxKind.ComparisonKinds); - private static void CheckBinary(SonarSyntaxNodeReportingContext context) + private static void CheckComparisonOutOfRange(SonarSyntaxNodeReportingContext context) { if (TryGetConstantValue(context.SemanticModel, (BinaryExpressionSyntax)context.Node, out var constant, out var other) && context.SemanticModel.GetTypeInfo(other).Type is { } typeSymbolOfOther From 339ac499bcb66f0458014a29e6088d3f62c82aeb Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Fri, 10 Feb 2023 13:30:49 +0100 Subject: [PATCH 27/28] Minor refactor for OutOfRange check --- .../Rules/SillyMathematicalComparison.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs index 081a60df311..9f42bb27b65 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SillyMathematicalComparison.cs @@ -40,7 +40,7 @@ private static void CheckComparisonOutOfRange(SonarSyntaxNodeReportingContext co if (TryGetConstantValue(context.SemanticModel, (BinaryExpressionSyntax)context.Node, out var constant, out var other) && context.SemanticModel.GetTypeInfo(other).Type is { } typeSymbolOfOther && TryGetRange(typeSymbolOfOther) is { } range - && (constant < range.Min || constant > range.Max)) + && range.IsOutOfRange(constant)) { var typeName = typeSymbolOfOther.ToMinimalDisplayString(context.SemanticModel, other.GetLocation().SourceSpan.Start); context.ReportIssue(Diagnostic.Create(MathComparisonRule, other.Parent.GetLocation(), typeName)); @@ -84,6 +84,10 @@ typeSymbol switch _ => null, }; - private readonly record struct ValuesRange(double Min, double Max); + private readonly record struct ValuesRange(double Min, double Max) + { + public bool IsOutOfRange(double value) => + value < Min || value > Max; + } } } From a8f25d77fe40bad16a4f46cd2f8f70d0d9d6103f Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Fri, 10 Feb 2023 13:44:26 +0100 Subject: [PATCH 28/28] Annotate a testcase with an issue --- .../TestCases/SillyMathematicalComparison.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs index 803fb591ee5..238e34dcd72 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.cs @@ -444,6 +444,7 @@ public void ConstantInBothOperands() const float number1 = 42; const double number2 = double.MaxValue; + // https://github.com/SonarSource/sonar-dotnet/issues/6745 _ = number1 != number2; // Compliant, constant in both operands _ = number1 == number2; // Compliant _ = number1 <= double.MaxValue; // Compliant