New rule S2198: Silly mathematical comparisons should not be made

analyzers/rspec/cs/S2198_c#.html

@@ -0,0 +1,18 @@
+<p>Certain mathematical comparisons will always return the same value, and should simply not be made.</p>
+<p>This comparison will always return <code>false</code>:</p>
+<ul>
+  <li> comparing a <code>float</code> with a <code>double</code> constant that’s outside the <code>float</code> range </li>
+</ul>
+<p>These will always return <code>true</code>:</p>
+<ul>
+  <li> comparing <code>aByte &lt;= Byte.MaxValue</code> and <code>aByte &gt;= Byte.MinValue</code> </li>
+  <li> comparing <code>anInt &lt;= int.MaxValue</code> and <code>anInt &gt;= int.MinValue</code> </li>
+  <li> comparing <code>aLong &lt;= long.MaxValue</code> and <code>aLong &gt;= long.MinValue</code> </li>
+</ul>
+<h2>Noncompliant Code Example</h2>
+<pre>
+float f = 42.0f;
+const double d = float.MaxValue + 1;
+if (f &lt;= d) { } // Noncompliant
+</pre>
+

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

analyzers/rspec/cs/Sonar_way_profile.json

@@ -66,6 +66,7 @@
     "S2184",
     "S2187",
     "S2190",
+    "S2198",
     "S2201",
     "S2219",
     "S2222",

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

@@ -25,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<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(BitwiseRule, ComparisonRule);
+
+        public SillyBitwiseOperation()
+        {
+            ComparisonRule = Language.CreateDescriptor(ComparisonDiagnosticId, ComparisonMessageFormat, fadeOutCode: true);
+        }
+
         protected override void Initialize(SonarAnalysisContext context)
         {
             context.RegisterNodeAction(
@@ -44,6 +57,13 @@ 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)
@@ -55,7 +75,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 +84,101 @@ 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, binary.Kind(), false))
+            {
+                context.ReportIssue(Diagnostic.Create(BitwiseRule, binary.GetLocation()));
+            }
+
+            if (ShouldRaise(context.SemanticModel, binary.Right, binary.Left, binary.Kind(), true))
+            {
+                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 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;
+            }
+        }
     }
 }

analyzers/src/SonarAnalyzer.Common/Rules/SillyBitwiseOperationBase.cs

@@ -20,19 +20,23 @@
 
 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.";
 
         protected abstract ILanguageFacade Language { get; }
 
-        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
-        protected DiagnosticDescriptor Rule { get; }
+        protected DiagnosticDescriptor BitwiseRule { get; }
 
-        protected SillyBitwiseOperationBase() =>
-            Rule = Language.CreateDescriptor(DiagnosticId, MessageFormat, fadeOutCode: true);
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(BitwiseRule);
+
+        protected SillyBitwiseOperationBase()
+        {
+            BitwiseRule = Language.CreateDescriptor(BitwiseDiagnosticId, BitwiseMessageFormat, fadeOutCode: true);
+        }
 
         protected void CheckBinary(SonarSyntaxNodeReportingContext context, SyntaxNode left, SyntaxToken @operator, SyntaxNode right, int constValueToLookFor)
         {
@@ -53,7 +57,7 @@ protected void CheckBinary(SonarSyntaxNodeReportingContext context, SyntaxNode l
                 return;
             }
 
-            context.ReportIssue(Diagnostic.Create(Rule, location, ImmutableDictionary<string, string>.Empty.Add(IsReportingOnLeftKey, isReportingOnLeftKey.ToString())));
+            context.ReportIssue(Diagnostic.Create(BitwiseRule, location, ImmutableDictionary<string, string>.Empty.Add(IsReportingOnLeftKey, isReportingOnLeftKey.ToString())));
         }
 
         protected int? FindIntConstant(SemanticModel semanticModel, SyntaxNode node) =>
@@ -64,6 +68,17 @@ protected void CheckBinary(SonarSyntaxNodeReportingContext context, SyntaxNode l
                 ? ConversionHelper.TryConvertToInt(value)
                 : null;
 
+        protected T? FindConstant<T>(SemanticModel semanticModel, SyntaxNode node, Func<object, T> 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: { } };
 

analyzers/src/SonarAnalyzer.Common/Rules/SillyBitwiseOperationCodeFixBase.cs

@@ -30,7 +30,7 @@ public abstract class SillyBitwiseOperationCodeFixBase : SonarCodeFix
 
         protected abstract Func<SyntaxNode> CreateNewRoot(SyntaxNode root, TextSpan diagnosticSpan, bool isReportingOnLeft);
 
-        public override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(SillyBitwiseOperationBase.DiagnosticId);
+        public override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(SillyBitwiseOperationBase.BitwiseDiagnosticId);
 
         protected override Task RegisterCodeFixesAsync(SyntaxNode root, CodeFixContext context)
         {

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",

analyzers/tests/SonarAnalyzer.UnitTest/Rules/SillyMathematicalComparison.cs

@@ -0,0 +1,34 @@
+/*
+ * 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;
+
+namespace SonarAnalyzer.UnitTest.Rules
+{
+    [TestClass]
+    public class SillyMathematicalComparisonTest
+    {
+        private readonly VerifierBuilder builderCS = new VerifierBuilder<CS.SillyBitwiseOperation>();
+
+        [TestMethod]
+        public void SillyMathematicalComparison_CS() =>
+            builderCS.AddPaths("SillyMathematicalComparison.cs").Verify();
+    }
+}