SONARJAVA-6686 Implement new rule S2198: Unnecessary mathematical comparisons

its/ruling/src/test/resources/diff_S2198.json

@@ -0,0 +1,6 @@
+{
+  "ruleKey": "S2198",
+  "hasTruePositives": true,
+  "falseNegatives": 0,
+  "falsePositives": 0
+}

its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S2198.json

@@ -0,0 +1,6 @@
+{
+"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java": [
+876,
+890
+]
+}

its/ruling/src/test/resources/eclipse-jetty/java-S2198.json

@@ -0,0 +1,17 @@
+{
+"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java": [
+876,
+890
+],
+"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/ByteArrayISO8859Writer.java": [
+107,
+124,
+142,
+167,
+185
+],
+"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/TreeTrie.java": [
+98,
+135
+]
+}

its/ruling/src/test/resources/guava/java-S2198.json

@@ -0,0 +1,5 @@
+{
+"com.google.guava:guava:src/com/google/common/net/InetAddresses.java": [
+907
+]
+}

java-checks-test-sources/default/src/main/java/checks/UselessMathematicalComparisonCheckSample.java

@@ -0,0 +1,116 @@
+package checks;
+
+class UselessMathematicalComparisonCheckSample {
+
+  static final int CONSTANT_200 = 200;
+
+  void byteComparisons(byte b) {
+    if (b > 200) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (b > 127) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (b >= 128) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (b < -200) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (b < -128) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (b <= -129) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (b == 200) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (b == -200) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+
+    if (b < 200) {} // Noncompliant {{Remove this comparison; it will always return true.}}
+    if (b <= 127) {} // Noncompliant {{Remove this comparison; it will always return true.}}
+    if (b >= -128) {} // Noncompliant {{Remove this comparison; it will always return true.}}
+    if (b >= -200) {} // Noncompliant {{Remove this comparison; it will always return true.}}
+    if (b > -129) {} // Noncompliant {{Remove this comparison; it will always return true.}}
+    if (b != 200) {} // Noncompliant {{Remove this comparison; it will always return true.}}
+
+    if (b > 100) {} // Compliant - 100 within byte range
+    if (b < -100) {} // Compliant
+    if (b == 0) {} // Compliant
+    if (b <= 126) {} // Compliant - 126 < max, not always true
+  }
+
+  void shortComparisons(short s) {
+    if (s > 40000) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (s >= 32768) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (s < -32769) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (s <= -32769) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (s == 40000) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+
+    if (s < 32768) {} // Noncompliant {{Remove this comparison; it will always return true.}}
+    if (s <= 32767) {} // Noncompliant {{Remove this comparison; it will always return true.}}
+    if (s >= -32768) {} // Noncompliant {{Remove this comparison; it will always return true.}}
+    if (s != 40000) {} // Noncompliant {{Remove this comparison; it will always return true.}}
+
+    if (s > 30000) {} // Compliant - within range
+    if (s < -30000) {} // Compliant
+  }
+
+  void charComparisons(char c) {
+    if (c < 0) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (c > 65535) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (c >= 65536) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (c < -1) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (c == -1) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (c == 65536) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+
+    if (c >= 0) {} // Noncompliant {{Remove this comparison; it will always return true.}}
+    if (c <= 65535) {} // Noncompliant {{Remove this comparison; it will always return true.}}
+    if (c != -1) {} // Noncompliant {{Remove this comparison; it will always return true.}}
+
+    if (c > 1000) {} // Compliant - within range
+    if (c < 60000) {} // Compliant
+  }
+
+  void intComparisons(int i) {
+    if (i > 2147483648L) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (i >= 2147483648L) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (i < -2147483649L) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (i == 2147483648L) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+
+    if (i < 2147483648L) {} // Noncompliant {{Remove this comparison; it will always return true.}}
+    if (i <= 2147483647L) {} // Noncompliant {{Remove this comparison; it will always return true.}}
+    if (i >= -2147483648L) {} // Noncompliant {{Remove this comparison; it will always return true.}}
+    if (i != 2147483648L) {} // Noncompliant {{Remove this comparison; it will always return true.}}
+
+    if (i > 1000000) {} // Compliant
+    if (i < -1000000) {} // Compliant
+  }
+
+  void reversedOperands(byte b) {
+    if (200 < b) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (200 <= b) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (-200 > b) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (200 > b) {} // Noncompliant {{Remove this comparison; it will always return true.}}
+    if (200 >= b) {} // Noncompliant {{Remove this comparison; it will always return true.}}
+    if (200 == b) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (200 != b) {} // Noncompliant {{Remove this comparison; it will always return true.}}
+  }
+
+  void longVariable(long l) {
+    if (l > 2147483648L) {} // Compliant - long can hold this value
+    if (l < -2147483649L) {} // Compliant
+  }
+
+  void floatVariable(float f) {
+    if (f > 1000000) {} // Compliant - float/double not checked
+  }
+
+  void doubleVariable(double d) {
+    if (d > 1000000) {} // Compliant - float/double not checked
+  }
+
+  void intVariableWithinRange(int b) {
+    if (b > 200) {} // Compliant - int can hold 200
+  }
+
+  void parenthesizedExpressions(byte b) {
+    if ((b) > 200) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+    if (b > (200)) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+  }
+
+  void constantExpressions(byte b) {
+    if (b > CONSTANT_200) {} // Noncompliant {{Remove this comparison; it will always return false.}}
+  }
+
+  void twoVariables(byte a, byte b) {
+    if (a > b) {} // Compliant - comparing two variables
+  }
+}

java-checks/src/main/java/org/sonar/java/checks/UselessMathematicalComparisonCheck.java

@@ -0,0 +1,177 @@
+/*
+ * SonarQube Java
+ * Copyright (C) SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * You can redistribute and/or modify this program under the terms of
+ * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
+ *
+ * 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 Sonar Source-Available License for more details.
+ *
+ * You should have received a copy of the Sonar Source-Available License
+ * along with this program; if not, see https://sonarsource.com/license/ssal/
+ */
+package org.sonar.java.checks;
+
+import java.util.List;
+import java.util.Map;
+import javax.annotation.Nullable;
+import org.sonar.check.Rule;
+import org.sonar.java.model.ExpressionUtils;
+import org.sonar.java.model.LiteralUtils;
+import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
+import org.sonar.plugins.java.api.semantic.Type;
+import org.sonar.plugins.java.api.tree.BinaryExpressionTree;
+import org.sonar.plugins.java.api.tree.ExpressionTree;
+import org.sonar.plugins.java.api.tree.Tree;
+import org.sonar.plugins.java.api.tree.Tree.Kind;
+
+@Rule(key = "S2198")
+public class UselessMathematicalComparisonCheck extends IssuableSubscriptionVisitor {
+
+  private static final Map<Type.Primitives, long[]> TYPE_BOUNDS = Map.of(
+    Type.Primitives.BYTE, new long[] {Byte.MIN_VALUE, Byte.MAX_VALUE},
+    Type.Primitives.SHORT, new long[] {Short.MIN_VALUE, Short.MAX_VALUE},
+    Type.Primitives.CHAR, new long[] {Character.MIN_VALUE, Character.MAX_VALUE},
+    Type.Primitives.INT, new long[] {Integer.MIN_VALUE, Integer.MAX_VALUE}
+  );
+
+  @Override
+  public List<Kind> nodesToVisit() {
+    return List.of(
+      Kind.GREATER_THAN,
+      Kind.GREATER_THAN_OR_EQUAL_TO,
+      Kind.LESS_THAN,
+      Kind.LESS_THAN_OR_EQUAL_TO,
+      Kind.EQUAL_TO,
+      Kind.NOT_EQUAL_TO);
+  }
+
+  @Override
+  public void visitNode(Tree tree) {
+    BinaryExpressionTree binaryExpression = (BinaryExpressionTree) tree;
+    ExpressionTree leftOperand = ExpressionUtils.skipParentheses(binaryExpression.leftOperand());
+    ExpressionTree rightOperand = ExpressionUtils.skipParentheses(binaryExpression.rightOperand());
+
+    // Try left=variable, right=constant
+    Boolean result = evaluate(leftOperand, rightOperand, binaryExpression.kind(), false);
+    if (result == null) {
+      // Try left=constant, right=variable (reversed)
+      result = evaluate(rightOperand, leftOperand, binaryExpression.kind(), true);
+    }
+
+    if (result != null) {
+      reportIssue(binaryExpression, "Remove this comparison; it will always return " + result + ".");
+    }
+  }
+
+  @Nullable
+  private static Boolean evaluate(ExpressionTree variableCandidate, ExpressionTree constantCandidate, Kind operatorKind, boolean reversed) {
+    long[] bounds = resolveTypeBounds(variableCandidate);
+    if (bounds == null) {
+      return null;
+    }
+    Long constantValue = resolveConstantValue(constantCandidate);
+    if (constantValue == null) {
+      return null;
+    }
+    long min = bounds[0];
+    long max = bounds[1];
+    Kind normalizedKind = reversed ? reverseOperator(operatorKind) : operatorKind;
+    return evaluateComparison(normalizedKind, min, max, constantValue);
+  }
+
+  @Nullable
+  private static long[] resolveTypeBounds(ExpressionTree expression) {
+    Type type = expression.symbolType();
+    for (Map.Entry<Type.Primitives, long[]> entry : TYPE_BOUNDS.entrySet()) {
+      if (type.isPrimitive(entry.getKey())) {
+        return entry.getValue();
+      }
+    }
+    return null;
+  }
+
+  @Nullable
+  private static Long resolveConstantValue(ExpressionTree expression) {
+    Long literal = LiteralUtils.longLiteralValue(expression);
+    if (literal != null) {
+      return literal;
+    }
+    Object constant = ExpressionUtils.resolveAsConstant(expression);
+    if (constant instanceof Integer intVal) {
+      return intVal.longValue();
+    }
+    if (constant instanceof Long longVal) {
+      return longVal;
+    }
+    return null;
+  }
+
+  private static Kind reverseOperator(Kind kind) {
+    return switch (kind) {
+      case GREATER_THAN -> Kind.LESS_THAN;
+      case GREATER_THAN_OR_EQUAL_TO -> Kind.LESS_THAN_OR_EQUAL_TO;
+      case LESS_THAN -> Kind.GREATER_THAN;
+      case LESS_THAN_OR_EQUAL_TO -> Kind.GREATER_THAN_OR_EQUAL_TO;
+      // EQUAL_TO, NOT_EQUAL_TO are symmetric
+      default -> kind;
+    };
+  }
+
+  @Nullable
+  private static Boolean evaluateComparison(Kind normalizedKind, long min, long max, long constant) {
+    return switch (normalizedKind) {
+      case GREATER_THAN -> evaluateGreaterThan(min, max, constant);
+      case GREATER_THAN_OR_EQUAL_TO -> evaluateGreaterThanOrEqual(min, max, constant);
+      case LESS_THAN -> evaluateLessThan(min, max, constant);
+      case LESS_THAN_OR_EQUAL_TO -> evaluateLessThanOrEqual(min, max, constant);
+      case EQUAL_TO -> (constant < min || constant > max) ? Boolean.FALSE : null;
+      case NOT_EQUAL_TO -> (constant < min || constant > max) ? Boolean.TRUE : null;
+      default -> null;
+    };
+  }
+
+  @Nullable
+  private static Boolean evaluateGreaterThan(long min, long max, long constant) {
+    if (constant >= max) {
+      return Boolean.FALSE;
+    } else if (constant < min) {
+      return Boolean.TRUE;
+    }
+    return null;
+  }
+
+  @Nullable
+  private static Boolean evaluateGreaterThanOrEqual(long min, long max, long constant) {
+    if (constant > max) {
+      return Boolean.FALSE;
+    } else if (constant <= min) {
+      return Boolean.TRUE;
+    }
+    return null;
+  }
+
+  @Nullable
+  private static Boolean evaluateLessThan(long min, long max, long constant) {
+    if (constant <= min) {
+      return Boolean.FALSE;
+    } else if (constant > max) {
+      return Boolean.TRUE;
+    }
+    return null;
+  }
+
+  @Nullable
+  private static Boolean evaluateLessThanOrEqual(long min, long max, long constant) {
+    if (constant < min) {
+      return Boolean.FALSE;
+    } else if (constant >= max) {
+      return Boolean.TRUE;
+    }
+    return null;
+  }
+}

java-checks/src/test/java/org/sonar/java/checks/UselessMathematicalComparisonCheckTest.java

@@ -0,0 +1,33 @@
+/*
+ * SonarQube Java
+ * Copyright (C) SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * You can redistribute and/or modify this program under the terms of
+ * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
+ *
+ * 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 Sonar Source-Available License for more details.
+ *
+ * You should have received a copy of the Sonar Source-Available License
+ * along with this program; if not, see https://sonarsource.com/license/ssal/
+ */
+package org.sonar.java.checks;
+
+import org.junit.jupiter.api.Test;
+import org.sonar.java.checks.verifier.CheckVerifier;
+
+import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;
+
+class UselessMathematicalComparisonCheckTest {
+
+  @Test
+  void test() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath("checks/UselessMathematicalComparisonCheckSample.java"))
+      .withCheck(new UselessMathematicalComparisonCheck())
+      .verifyIssues();
+  }
+}