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

[nathsou]

Summary

A Peach false-positive run sampled 366 S9354 issues and found 60 false positives (16.4%). About 45 of them subtract two operands whose value range is provably bounded, so the subtraction cannot overflow: array/String lengths, Collection/Map.size(), Enum.ordinal(), Integer/Long.bitCount() and related bit-scan intrinsics, values masked with a non-negative bitwise AND constant, and compile-time constants. The check previously decided purely from operand type (int/long), so every one of these was reported.

• Add BoundedIntegerRange, computing a best-effort [lo, hi] range for an expression and testing whether two ranges make an int subtraction safe (hi - otherLo <= Integer.MAX_VALUE && lo - otherHi >= Integer.MIN_VALUE). This subsumes the existing byte/short/char exemption and generalizes to any bounded int value, including single-write locals resolved via the existing ExpressionsHelper.getSingleWriteUsage.
• Consult it in IntegerSubtractionInComparisonCheck.replacementFor only for the Integer.compare branch (both operands int), leaving Long.compare cases unaffected — none of the sampled false positives involve long operands, and exempting bounded longs (e.g. java.time nanosecond fields) would require API invariants outside the type system.
• Add compliant samples for each recognized pattern (array length, String.length(), Collection.size(), Map.size(), Enum.ordinal(), Integer.bitCount(), masked bytes direct and via locals, single-write local length, literal constants) and noncompliant guard cases for a mixed bounded/unbounded subtraction and an unknown-range hashCode() subtraction, to catch a regression that would weaken the "both operands must be bounded" requirement to "either operand".

Intentionally out of scope, and still reported: indexOf() results ([-1, size-1], so Integer.MAX_VALUE - (-1) overflows in principle), java.time.Duration/Instant internal nanosecond fields, java.lang.Enum.ordinal raw field access (only reachable in JDK source, unlike the .ordinal() method call which is now recognized), values from unrelated helper methods, and values bounded only by runtime assertions.

Links

• Jira: https://sonarsource.atlassian.net/browse/SONARJAVA-6838
• RSPEC PR: https://github.com/SonarSource/rspec/pull/7950

AI disclosure

• LLM model used for implementation: Claude Sonnet 5

[hashicorp-vault-sonar-prod]

SONARJAVA-6838

[github-actions]

❌ Ruling needs updating. A fix PR has been created: #6003

Please review and merge it into your branch.

[github-actions]

Ruling Diff Summary

Detected changes in 1 rule files: 12 issues removed, 0 issues added.

S9354 (java) on guava - 12 issues removed, 0 issues added

Removed src/com/google/common/primitives/Booleans.java (line 297)

       292 |         int result = Booleans.compare(left[i], right[i]);
       293 |         if (result != 0) {
       294 |           return result;
       295 |         }
       296 |       }
>>>    297 |       return left.length - right.length;
       298 |     }
       299 |   }
       300 | 
       301 |   /**
       302 |    * Copies a collection of {@code Boolean} instances into a new array of

Removed src/com/google/common/primitives/Chars.java (line 414)

       409 |         int result = Chars.compare(left[i], right[i]);
       410 |         if (result != 0) {
       411 |           return result;
       412 |         }
       413 |       }
>>>    414 |       return left.length - right.length;
       415 |     }
       416 |   }
       417 | 
       418 |   /**
       419 |    * Copies a collection of {@code Character} instances into a new array of

Removed src/com/google/common/primitives/Doubles.java (line 401)

       396 |         int result = Double.compare(left[i], right[i]);
       397 |         if (result != 0) {
       398 |           return result;
       399 |         }
       400 |       }
>>>    401 |       return left.length - right.length;
       402 |     }
       403 |   }
       404 | 
       405 |   /**
       406 |    * Returns an array containing each value of {@code collection}, converted to

Removed src/com/google/common/primitives/Floats.java (line 397)

       392 |         int result = Float.compare(left[i], right[i]);
       393 |         if (result != 0) {
       394 |           return result;
       395 |         }
       396 |       }
>>>    397 |       return left.length - right.length;
       398 |     }
       399 |   }
       400 | 
       401 |   /**
       402 |    * Returns an array containing each value of {@code collection}, converted to

Removed src/com/google/common/primitives/Ints.java (line 462)

       457 |         int result = Ints.compare(left[i], right[i]);
       458 |         if (result != 0) {
       459 |           return result;
       460 |         }
       461 |       }
>>>    462 |       return left.length - right.length;
       463 |     }
       464 |   }
       465 | 
       466 |   /**
       467 |    * Returns an array containing each value of {@code collection}, converted to

Removed src/com/google/common/primitives/Longs.java (line 498)

       493 |         int result = Longs.compare(left[i], right[i]);
       494 |         if (result != 0) {
       495 |           return result;
       496 |         }
       497 |       }
>>>    498 |       return left.length - right.length;
       499 |     }
       500 |   }
       501 | 
       502 |   /**
       503 |    * Returns an array containing each value of {@code collection}, converted to

Removed src/com/google/common/primitives/Shorts.java (line 461)

       456 |         int result = Shorts.compare(left[i], right[i]);
       457 |         if (result != 0) {
       458 |           return result;
       459 |         }
       460 |       }
>>>    461 |       return left.length - right.length;
       462 |     }
       463 |   }
       464 | 
       465 |   /**
       466 |    * Returns an array containing each value of {@code collection}, converted to

Removed src/com/google/common/primitives/SignedBytes.java (line 202)

       197 |         int result = SignedBytes.compare(left[i], right[i]);
       198 |         if (result != 0) {
       199 |           return result;
       200 |         }
       201 |       }
>>>    202 |       return left.length - right.length;
       203 |     }
       204 |   }
       205 | }

Removed src/com/google/common/primitives/UnsignedBytes.java (line 420)

       415 |           int result = UnsignedBytes.compare(left[i], right[i]);
       416 |           if (result != 0) {
       417 |             return result;
       418 |           }
       419 |         }
>>>    420 |         return left.length - right.length;
       421 |       }
       422 |     }
       423 | 
       424 |     enum PureJavaComparator implements Comparator<byte[]> {
       425 |       INSTANCE;

Removed src/com/google/common/primitives/UnsignedBytes.java (line 436)

       431 |           int result = UnsignedBytes.compare(left[i], right[i]);
       432 |           if (result != 0) {
       433 |             return result;
       434 |           }
       435 |         }
>>>    436 |         return left.length - right.length;
       437 |       }
       438 |     }
       439 | 
       440 |     /**
       441 |      * Returns the Unsafe-using Comparator, or falls back to the pure-Java

Removed src/com/google/common/primitives/UnsignedInts.java (line 176)

       171 |       for (int i = 0; i < minLength; i++) {
       172 |         if (left[i] != right[i]) {
       173 |           return UnsignedInts.compare(left[i], right[i]);
       174 |         }
       175 |       }
>>>    176 |       return left.length - right.length;
       177 |     }
       178 |   }
       179 | 
       180 |   /**
       181 |    * Returns dividend / divisor, where the dividend and divisor are treated as unsigned 32-bit

Removed src/com/google/common/primitives/UnsignedLongs.java (line 177)

       172 |       for (int i = 0; i < minLength; i++) {
       173 |         if (left[i] != right[i]) {
       174 |           return UnsignedLongs.compare(left[i], right[i]);
       175 |         }
       176 |       }
>>>    177 |       return left.length - right.length;
       178 |     }
       179 |   }
       180 | 
       181 |   /**
       182 |    * Returns dividend / divisor, where the dividend and divisor are treated as unsigned 64-bit

[github-actions]

❌ Ruling needs updating. A fix PR has been created: #6003

Please review and merge it into your branch.

[gitar-bot]

Code Review ✅ Approved 1 resolved / 1 findings

Fixes false positives in S9354 by ignoring subtractions of operands with provably bounded ranges, addressing the bitwiseAndRange unsoundness finding.

✅ 1 resolved

✅ Edge Case: bitwiseAndRange unsound for int masks with sign bit set

📄 java-checks/src/main/java/org/sonar/java/checks/helpers/BoundedIntegerRange.java:139-153
For an int-typed x & C, the range [0, C] is only valid when C is non-negative as an int. But LiteralUtils.longLiteralValue decodes hex literals by magnitude (Long.decode("0x80000000") -> 2147483648, 0xFFFFFFFF -> 4294967295), so a mask whose bit 31 is set (a negative int) is read as a large positive long. x & 0x80000000 actually yields 0 or Integer.MIN_VALUE, yet the code derives [0, 2147483648]. This makes the helper unsound in the negative direction: e.g. (x & 0x80000000) - 1 is judged non-overflowing and silently exempted, missing a genuine overflow. The common & 0xff/& 0x7fffffff cases are unaffected since those constants are < Integer.MAX_VALUE. Guard the mask so a sign-bit-set int constant is treated as unbounded.

Implementation Status 🟡 0 / 1 issues implemented

⬜ SONARJAVA-6838 — 0 / 8 objectives

The PR contains no file diffs to evaluate the objectives.

• ⬜ Recognize operations composing with value sources including ternary conditions, Math.min/max, bitwise AND with constants, unsigned right shift, modulo, Math.floorMod, compile-time constants, and single-write local variables
• ⬜ Ensure patterns like Math.abs, file/system sizes, hashCodes, compare methods, and Matcher start/end remain reportable
• ⬜ Recognize specific non-negative value sources bounded by Integer.MAX_VALUE such as array length, CharSequence length, Collection/Map size, etc.
• ⬜ Recognize values bounded to smaller explicit ranges such as Enum.ordinal, bitCount, leading/trailing zeros, signum, code point methods, character count, and java.time accessors
• ⬜ Anchor API entries on their owning JDK types using MethodMatchers
• ⬜ Do not raise S9354 when the range of both operands can be established and their difference provably fits in an int
• ⬜ Add helper returning an optional interval for an expression and consult it in IntegerSubtractionInComparisonCheck using long arithmetic
• ⬜ Extend IntegerSubtractionInComparisonCheckSample with compliant cases for accepted forms and noncompliant boundary cases

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
95.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube