SONARJAVA-6838: Fix FP on S9354: ignore subtraction of operands with a provably bounded range

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

@@ -2,39 +2,7 @@
 "com.google.guava:guava:src/com/google/common/collect/ExplicitOrdering.java": [
 41
 ],
-"com.google.guava:guava:src/com/google/common/primitives/Booleans.java": [
-297
-],
-"com.google.guava:guava:src/com/google/common/primitives/Chars.java": [
-414
-],
-"com.google.guava:guava:src/com/google/common/primitives/Doubles.java": [
-401
-],
-"com.google.guava:guava:src/com/google/common/primitives/Floats.java": [
-397
-],
-"com.google.guava:guava:src/com/google/common/primitives/Ints.java": [
-462
-],
-"com.google.guava:guava:src/com/google/common/primitives/Longs.java": [
-498
-],
-"com.google.guava:guava:src/com/google/common/primitives/Shorts.java": [
-461
-],
-"com.google.guava:guava:src/com/google/common/primitives/SignedBytes.java": [
-202
-],
 "com.google.guava:guava:src/com/google/common/primitives/UnsignedBytes.java": [
-409,
-420,
-436
-],
-"com.google.guava:guava:src/com/google/common/primitives/UnsignedInts.java": [
-176
-],
-"com.google.guava:guava:src/com/google/common/primitives/UnsignedLongs.java": [
-177
+409
 ]
 }

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

@@ -3,6 +3,7 @@
 import java.io.File;
 import java.util.Comparator;
 import java.util.List;
+import java.util.Map;
 import java.util.function.IntSupplier;
 
 class IntegerSubtractionInComparisonCheckSample {
@@ -225,6 +226,181 @@ public int compareTo(BoxedShortCompareTo other) {
     }
   }
 
+  static class ArrayLengthComparator implements Comparator<int[]> {
+    @Override
+    public int compare(int[] left, int[] right) {
+      return left.length - right.length; // Compliant - both operands are array lengths, bounded in [0, Integer.MAX_VALUE]
+    }
+  }
+
+  static class StringLengthComparator implements Comparator<String> {
+    @Override
+    public int compare(String left, String right) {
+      return left.length() - right.length(); // Compliant - both operands are String.length(), bounded in [0, Integer.MAX_VALUE]
+    }
+  }
+
+  static class CollectionSizeComparator implements Comparator<List<Object>> {
+    @Override
+    public int compare(List<Object> left, List<Object> right) {
+      return left.size() - right.size(); // Compliant - both operands are Collection.size(), bounded in [0, Integer.MAX_VALUE]
+    }
+  }
+
+  static class MapSizeComparator implements Comparator<Map<Object, Object>> {
+    @Override
+    public int compare(Map<Object, Object> left, Map<Object, Object> right) {
+      return left.size() - right.size(); // Compliant - both operands are Map.size(), bounded in [0, Integer.MAX_VALUE]
+    }
+  }
+
+  enum Suit {
+    CLUBS, DIAMONDS, HEARTS, SPADES
+  }
+
+  static class OrdinalComparator implements Comparator<Suit> {
+    @Override
+    public int compare(Suit left, Suit right) {
+      return left.ordinal() - right.ordinal(); // Compliant - both operands are Enum.ordinal(), bounded in [0, Integer.MAX_VALUE]
+    }
+  }
+
+  static class BitCountComparator implements Comparator<Integer> {
+    @Override
+    public int compare(Integer left, Integer right) {
+      return Integer.bitCount(left) - Integer.bitCount(right); // Compliant - both operands are bounded in [0, 32]
+    }
+  }
+
+  static class LongBitCountComparator implements Comparator<Long> {
+    @Override
+    public int compare(Long left, Long right) {
+      return Long.bitCount(left) - Long.bitCount(right); // Compliant - both operands are bounded in [0, 64]
+    }
+  }
+
+  static class MaskedByteComparator implements Comparator<byte[]> {
+    @Override
+    public int compare(byte[] left, byte[] right) {
+      int a = left[0] & 0xff;
+      int b = right[0] & 0xff;
+      return a - b; // Compliant - both operands are masked to [0, 255]
+    }
+  }
+
+  static class DirectMaskedByteComparator implements Comparator<byte[]> {
+    @Override
+    public int compare(byte[] left, byte[] right) {
+      return (left[0] & 0xff) - (right[0] & 0xff); // Compliant - both operands are masked to [0, 255]
+    }
+  }
+
+  static class IndirectLengthComparator implements Comparator<String> {
+    @Override
+    public int compare(String left, String right) {
+      int len1 = left.length();
+      int len2 = right.length();
+      return len1 - len2; // Compliant - len1 and len2 are single-write locals holding String.length()
+    }
+  }
+
+  static class LiteralConstantComparator implements Comparator<Object> {
+    @Override
+    public int compare(Object left, Object right) {
+      return 0 - 1; // Compliant - both operands are compile-time constants
+    }
+  }
+
+  static class MixedBoundedAndUnboundedComparator implements Comparable<MixedBoundedAndUnboundedComparator> {
+    private int value;
+
+    @Override
+    public int compareTo(MixedBoundedAndUnboundedComparator other) {
+      // Noncompliant@+1 {{Subtracting numeric values in compareTo can overflow; use Integer.compare instead.}}
+      return this.value - other.getClass().getName().length();
+    }
+  }
+
+  static class MixedUnboundedAndBoundedComparator implements Comparator<String> {
+    @Override
+    public int compare(String left, String right) {
+      // Noncompliant@+1 {{Subtracting numeric values in compare can overflow; use Integer.compare instead.}}
+      return left.length() - right.hashCode();
+    }
+  }
+
+  static class BoundedButUnsafeComparator implements Comparator<byte[]> {
+    @Override
+    public int compare(byte[] left, byte[] right) {
+      int a = left[0] & 0xff;
+      // Noncompliant@+1 {{Subtracting numeric values in compare can overflow; use Integer.compare instead.}}
+      return a - Integer.MIN_VALUE;
+    }
+  }
+
+  static class SignBitLiteralComparator implements Comparator<String> {
+    @Override
+    public int compare(String left, String right) {
+      // 0x80000000 is the int literal for Integer.MIN_VALUE; left.length() is bounded but this still overflows.
+      // Noncompliant@+1 {{Subtracting numeric values in compare can overflow; use Integer.compare instead.}}
+      return left.length() - 0x80000000;
+    }
+  }
+
+  static class MaskedByUnresolvedMaskComparator implements Comparator<Integer> {
+    @Override
+    public int compare(Integer left, Integer right) {
+      int mask = computeMask(left);
+      // mask does not resolve to a constant, so the mask range - and therefore this subtraction - is unknown.
+      // Noncompliant@+1 {{Subtracting numeric values in compare can overflow; use Integer.compare instead.}}
+      return (left & mask) - 0;
+    }
+
+    private static int computeMask(int seed) {
+      return seed > 0 ? 0xff : 0x0f;
+    }
+  }
+
+  static class DeeplyChainedLocalsComparator implements Comparator<String> {
+    @Override
+    public int compare(String left, String right) {
+      int a = left.length();
+      int b = a;
+      int c = b;
+      int d = c;
+      int e = d;
+      // The single-write resolution depth cap keeps this conservative: e is not traced all the way back to
+      // left.length(), so it is treated as unbounded even though it provably isn't.
+      // Noncompliant@+1 {{Subtracting numeric values in compare can overflow; use Integer.compare instead.}}
+      return e - right.length();
+    }
+  }
+
+  static class IncrementedCounterComparator implements Comparator<String[]> {
+    @Override
+    public int compare(String[] left, String[] right) {
+      int leftCount = 0;
+      for (String s : left) {
+        leftCount++;
+      }
+      int rightCount = 0;
+      for (String s : right) {
+        rightCount++;
+      }
+      // leftCount and rightCount are mutated with ++, not a single assignment, so they are not resolved to a
+      // bounded range even though both loops only ever increment their counter.
+      // Noncompliant@+1 {{Subtracting numeric values in compare can overflow; use Integer.compare instead.}}
+      return leftCount - rightCount;
+    }
+  }
+
+  static class UnknownRangeComparator implements Comparable<UnknownRangeComparator> {
+    @Override
+    public int compareTo(UnknownRangeComparator other) {
+      return this.hashCode() - other.hashCode(); // Noncompliant {{Subtracting numeric values in compareTo can overflow; use Integer.compare instead.}}
+    }
+  }
+
   int subtract(int a, int b) {
     return a - b; // Compliant - not in a comparison method
   }

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

@@ -18,6 +18,7 @@
 
 import java.util.List;
 import org.sonar.check.Rule;
+import org.sonar.java.checks.helpers.BoundedIntegerRange;
 import org.sonar.java.checks.helpers.ComparisonMethodUtils;
 import org.sonar.java.model.ExpressionUtils;
 import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
@@ -103,6 +104,11 @@ private static String replacementFor(BinaryExpressionTree tree) {
       return "Long.compare";
     }
     if (isInt(left) || isInt(right)) {
+      if (BoundedIntegerRange.subtractionCannotOverflow(tree.leftOperand(), tree.rightOperand())) {
+        // Both operands have a provable range whose difference is guaranteed to fit in an int,
+        // e.g. lengths, sizes, ordinals, or masked values: no overflow is possible.
+        return null;
+      }
       return "Integer.compare";
     }
     return null;