New rule S2198: Silly mathematical comparisons should not be made (#6695

)

* Update RSPEC for S2198

* Remove a useless auto-generated json

* Add some testcases

* Remove a useless using

* Initial implementation of the main logic

* Initial working implementation of S2198

* Refactor implementation

* Move some code around

* Add a lot of testcases

* Cut branches: only implement absolute inequality for float

* Rewrite SillyMathematicalComparison, only out-of-range checks for floats for this step

* Apply PR review changes

* Enrichen testcases, add long and ulong to the checked types

* Add 'is' pattern testcases, refactored a .ToString to a .ToDisplayString

* Simple rename

* Implement a constant-in-both-sides version of the rule.

* Apply PR Review changes, some constant-check fixes and more testcases

* Add enum, char and reference type to the testcases

* Simplify the rule, only care about out-of-range mathematical checks

* Add char out-of-range detection

* Added some true negatives for const1 {{operator}} const2

* Bump coverage by adding a case I forgot about

* Apply PR Review changes

* Add an is-pattern edgecase in the testcases

* Fix a logical condition, refactor TryGetRange

* Minor naming refactor

* Minor refactor for OutOfRange check

* Annotate a testcase with an issue

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

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

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

@@ -0,0 +1,93 @@
+/*
+ * 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.
+ */
+
+namespace SonarAnalyzer.Rules.CSharp
+{
+    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    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 static readonly DiagnosticDescriptor MathComparisonRule = DescriptorFactory.Create(DiagnosticId, MathComparisonMessage);
+
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(MathComparisonRule);
+
+        protected override void Initialize(SonarAnalysisContext context) =>
+            context.RegisterNodeAction(
+                CheckComparisonOutOfRange,
+                CSharpFacade.Instance.SyntaxKind.ComparisonKinds);
+
+        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
+               && TryGetRange(typeSymbolOfOther) is { } range
+               && range.IsOutOfRange(constant))
+            {
+                var typeName = typeSymbolOfOther.ToMinimalDisplayString(context.SemanticModel, other.GetLocation().SourceSpan.Start);
+                context.ReportIssue(Diagnostic.Create(MathComparisonRule, other.Parent.GetLocation(), typeName));
+            }
+        }
+
+        private static bool TryGetConstantValue(SemanticModel model, BinaryExpressionSyntax binary, out double constant, out SyntaxNode other)
+        {
+            var optionalLeft = model.GetConstantValue(binary.Left);
+            var optionalRight = model.GetConstantValue(binary.Right);
+
+            if (optionalLeft.HasValue ^ optionalRight.HasValue)
+            {
+                if (optionalLeft.HasValue && TryConvertToDouble(optionalLeft.Value, out constant))
+                {
+                    other = binary.Right;
+                    return true;
+                }
+                else if (optionalRight.HasValue && TryConvertToDouble(optionalRight.Value, out constant))
+                {
+                    other = binary.Left;
+                    return true;
+                }
+            }
+            constant = default;
+            other = default;
+            return false;
+        }
+
+        // 'char' needs to roundtrip {{char -> int -> double}}, can't go {{char -> double}}
+        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) =>
+            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 readonly record struct ValuesRange(double Min, double Max)
+        {
+            public bool IsOutOfRange(double value) =>
+               value < Min || value > Max;
+        }
+    }
+}

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,38 @@
+/*
+ * 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.SillyMathematicalComparison>();
+
+        [TestMethod]
+        public void SillyMathematicalComparison_CS() =>
+            builderCS.AddPaths("SillyMathematicalComparison.cs").Verify();
+
+        [TestMethod]
+        public void SillyMathematicalComparison_CSharp9() =>
+            builderCS.AddPaths("SillyMathematicalComparison.CSharp9.cs").WithOptions(ParseOptionsHelper.FromCSharp9).Verify();
+    }
+}

analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SillyMathematicalComparison.CSharp9.cs

@@ -0,0 +1,25 @@
+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
+
+            const double d = double.MaxValue;
+            float x = 42;
+            _ = x is d; // Error [CS0266] - cannot implicitly convert type 'double' to 'float'
+        }
+    }
+}