Java: Fix FPs for concurrent modification checks.

Author: aschackmull

change-notes/1.19/analysis-java.md

@@ -13,6 +13,7 @@
 |----------------------------|------------------------|------------------------------------------------------------------|
 | Array index out of bounds (`java/index-out-of-bounds`) | Fewer false positive results | False positives involving arrays with a length evenly divisible by 3 or some greater number and an index being increased with a similar stride length are no longer reported. |
 | Unreachable catch clause (`java/unreachable-catch-clause`) | Fewer false positive results | This rule now accounts for calls to generic methods that throw generic exceptions. |
+| Useless comparison test (`java/constant-comparison`) | Fewer false positive results | Constant comparisons guarding `java.util.ConcurrentModificationException` are no longer reported, as they are intended to always be false in the absence of API misuse. |
 
 ## Changes to QL libraries
 

java/ql/src/Likely Bugs/Comparison/UselessComparisonTest.ql

@@ -167,6 +167,15 @@ predicate overFlowTest(ComparisonExpr comp) {
   comp.getGreaterOperand().(IntegerLiteral).getIntValue() = 0
 }
 
+predicate concurrentModificationTest(BinaryExpr test) {
+  exists(IfStmt ifstmt, ThrowStmt throw, RefType exc |
+    ifstmt.getCondition() = test and
+    (ifstmt.getThen() = throw or ifstmt.getThen().(SingletonBlock).getStmt() = throw) and
+    throw.getExpr().(ClassInstanceExpr).getConstructedType() = exc and
+    exc.hasQualifiedName("java.util", "ConcurrentModificationException")
+  )
+}
+
 /**
  * Holds if `test` and `guard` are equality tests of the same integral variable v with constants `c1` and `c2`.
  */
@@ -202,13 +211,13 @@ where
       )
     else
       if constCondSimple(test, _)
-      then (
-        constCondSimple(test, testIsTrue) and reason = "" and reasonElem = test // dummy reason element
-      ) else
+      then constCondSimple(test, testIsTrue) and reason = "" and reasonElem = test // dummy reason element
+      else
         exists(CondReason r |
           constCond(test, testIsTrue, r) and reason = ", because of $@" and reasonElem = r.getCond()
         )
   ) and
   not overFlowTest(test) and
+  not concurrentModificationTest(test) and
   not exists(AssertStmt assert | assert.getExpr() = test.getParent*())
 select test, "Test is always " + testIsTrue + reason + ".", reasonElem, "this condition"