Java: Consolidate Assertions.qll and Preconditions.qll.

java/ql/lib/change-notes/2025-09-11-assertions-cfg.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Improved support for various assertion libraries, in particular JUnit. This affects the control-flow graph slightly, and in turn affects several queries (mainly quality queries). Most queries should see improved precision (new true positives and fewer false positives), in particular `java/constant-comparison`, `java/index-out-of-bounds`, `java/dereferenced-value-may-be-null`, and `java/useless-null-check`. Some medium precision queries like `java/toctou-race-condition` and `java/unreleased-lock` may see mixed result changes (both slight improvements and slight regressions).

java/ql/lib/semmle/code/java/ControlFlowGraph.qll

@@ -365,10 +365,10 @@ private module ControlFlowGraphImpl {
  * Bind `t` to an unchecked exception that may occur in a precondition check or guard wrapper.
  */
  private predicate uncheckedExceptionFromMethod(MethodCall ma, ThrowableType t) {
-conditionCheckArgument(ma, _, _) and
+(methodCallChecksArgument(ma) or methodCallUnconditionallyThrows(ma)) and
  (t instanceof TypeError or t instanceof TypeRuntimeException)
  or
- methodMayThrow(ma.getMethod(), t)
+ methodMayThrow(ma.getMethod().getSourceDeclaration(), t)
  }
 
  /**
@@ -586,6 +586,7 @@ private module ControlFlowGraphImpl {
  * Gets a `MethodCall` that always throws an exception or calls `exit`.
  */
  private MethodCall nonReturningMethodCall() {
+ methodCallUnconditionallyThrows(result) or
  result.getMethod().getSourceDeclaration() = nonReturningMethod() or
  result = likelyNonReturningMethod().getAnAccess()
  }

java/ql/lib/semmle/code/java/controlflow/Guards.qll

@@ -395,11 +395,13 @@ private module LogicInputCommon {
  predicate additionalImpliesStep(
  GuardsImpl::PreGuard g1, GuardValue v1, GuardsImpl::PreGuard g2, GuardValue v2
  ) {
- exists(MethodCall check, int argIndex |
+ exists(MethodCall check |
  g1 = check and
- v1.getDualValue().isThrowsException() and
- conditionCheckArgument(check, argIndex, v2.asBooleanValue()) and
- g2 = check.getArgument(argIndex)
+ v1.getDualValue().isThrowsException()
+ |
+ methodCallChecksBoolean(check, g2, v2.asBooleanValue())
+ or
+ methodCallChecksNotNull(check, g2) and v2.isNonNullValue()
  )
  }
 }

java/ql/lib/semmle/code/java/controlflow/internal/Preconditions.qll

@@ -1,5 +1,5 @@
 /**
- * Provides predicates for identifying precondition checks like
+ * Provides predicates for identifying precondition and assertion checks like
  * `com.google.common.base.Preconditions` and
  * `org.apache.commons.lang3.Validate`.
  */
@@ -9,99 +9,150 @@ module;
 import java
 
 /**
- * Holds if `m` is a non-overridable method that checks that its zero-indexed `argument`
- * is equal to `checkTrue` and throws otherwise.
+ * Holds if `m` is a method that checks that its argument at position `arg` is
+ * equal to true and throws otherwise.
  */
-predicate conditionCheckMethodArgument(Method m, int argument, boolean checkTrue) {
- condtionCheckMethodGooglePreconditions(m, checkTrue) and argument = 0
- or
- conditionCheckMethodApacheCommonsLang3Validate(m, checkTrue) and argument = 0
+private predicate methodCheckTrue(Method m, int arg) {
+ arg = 0 and
+ (
+ m.hasQualifiedName("com.google.common.base", "Preconditions", ["checkArgument", "checkState"]) or
+ m.hasQualifiedName("com.google.common.base", "Verify", "verify") or
+ m.hasQualifiedName("org.apache.commons.lang3", "Validate", ["isTrue", "validState"]) or
+ m.hasQualifiedName("org.junit.jupiter.api", "Assertions", "assertTrue") or
+ m.hasQualifiedName("org.junit.jupiter.api", "Assumptions", "assumeTrue") or
+ m.hasQualifiedName("org.testng", "Assert", "assertTrue")
+ )
  or
- condtionCheckMethodTestingFramework(m, argument, checkTrue)
+ m.getParameter(arg).getType() instanceof BooleanType and
+ (
+ m.hasQualifiedName("org.junit", "Assert", "assertTrue") or
+ m.hasQualifiedName("org.junit", "Assume", "assumeTrue") or
+ m.hasQualifiedName("junit.framework", _, "assertTrue")
+ )
+}
+
+/**
+ * Holds if `m` is a method that checks that its argument at position `arg` is
+ * equal to false and throws otherwise.
+ */
+private predicate methodCheckFalse(Method m, int arg) {
+ arg = 0 and
+ (
+ m.hasQualifiedName("org.junit.jupiter.api", "Assertions", "assertFalse") or
+ m.hasQualifiedName("org.junit.jupiter.api", "Assumptions", "assumeFalse") or
+ m.hasQualifiedName("org.testng", "Assert", "assertFalse")
+ )
  or
- exists(Parameter p, MethodCall ma, int argIndex, boolean ct, Expr arg |
- p = m.getParameter(argument) and
- not m.isOverridable() and
- m.getBody().getStmt(0).(ExprStmt).getExpr() = ma and
- conditionCheckArgument(ma, argIndex, ct) and
- ma.getArgument(argIndex) = arg and
- (
- arg.(LogNotExpr).getExpr().(VarAccess).getVariable() = p and
- checkTrue = ct.booleanNot()
- or
- arg.(VarAccess).getVariable() = p and checkTrue = ct
- )
+ m.getParameter(arg).getType() instanceof BooleanType and
+ (
+ m.hasQualifiedName("org.junit", "Assert", "assertFalse") or
+ m.hasQualifiedName("org.junit", "Assume", "assumeFalse") or
+ m.hasQualifiedName("junit.framework", _, "assertFalse")
+ )
+}
+
+/**
+ * Holds if `m` is a method that checks that its argument at position `arg` is
+ * not null and throws otherwise.
+ */
+private predicate methodCheckNotNull(Method m, int arg) {
+ arg = 0 and
+ (
+ m.hasQualifiedName("com.google.common.base", "Preconditions", "checkNotNull") or
+ m.hasQualifiedName("com.google.common.base", "Verify", "verifyNotNull") or
+ m.hasQualifiedName("org.apache.commons.lang3", "Validate", "notNull") or
+ m.hasQualifiedName("java.util", "Objects", "requireNonNull") or
+ m.hasQualifiedName("org.junit.jupiter.api", "Assertions", "assertNotNull") or
+ m.hasQualifiedName("org.junit", "Assume", "assumeNotNull") or // vararg
+ m.hasQualifiedName("org.testng", "Assert", "assertNotNull")
  )
  or
- exists(Parameter p, IfStmt ifstmt, Expr cond |
- p = m.getParameter(argument) and
- not m.isOverridable() and
- p.getType() instanceof BooleanType and
- m.getBody().getStmt(0) = ifstmt and
- ifstmt.getCondition() = cond and
- (
- cond.(LogNotExpr).getExpr().(VarAccess).getVariable() = p and checkTrue = true
- or
- cond.(VarAccess).getVariable() = p and checkTrue = false
- ) and
- (
- ifstmt.getThen() instanceof ThrowStmt or
- ifstmt.getThen().(SingletonBlock).getStmt() instanceof ThrowStmt
- )
+ arg = m.getNumberOfParameters() - 1 and
+ (
+ m.hasQualifiedName("org.junit", "Assert", "assertNotNull") or
+ m.hasQualifiedName("junit.framework", _, "assertNotNull")
  )
 }
 
-private predicate condtionCheckMethodGooglePreconditions(Method m, boolean checkTrue) {
- m.getDeclaringType().hasQualifiedName("com.google.common.base", "Preconditions") and
- checkTrue = true and
- (m.hasName("checkArgument") or m.hasName("checkState"))
+/**
+ * Holds if `m` is a method that checks that its argument at position `arg`
+ * satisfies a property specified by another argument and throws otherwise.
+ */
+private predicate methodCheckThat(Method m, int arg) {
+ m.getParameter(arg).getType().getErasure() instanceof TypeObject and
+ (
+ m.hasQualifiedName("org.hamcrest", "MatcherAssert", "assertThat") or
+ m.hasQualifiedName("org.junit", "Assert", "assertThat") or
+ m.hasQualifiedName("org.junit", "Assume", "assumeThat")
+ )
 }
 
-private predicate conditionCheckMethodApacheCommonsLang3Validate(Method m, boolean checkTrue) {
- m.getDeclaringType().hasQualifiedName("org.apache.commons.lang3", "Validate") and
- checkTrue = true and
- (m.hasName("isTrue") or m.hasName("validState"))
+/** Holds if `m` is a method that unconditionally throws. */
+private predicate methodUnconditionallyThrows(Method m) {
+ m.hasQualifiedName("org.junit.jupiter.api", "Assertions", "fail") or
+ m.hasQualifiedName("org.junit", "Assert", "fail") or
+ m.hasQualifiedName("junit.framework", _, "fail") or
+ m.hasQualifiedName("org.testng", "Assert", "fail")
 }
 
 /**
- * Holds if `m` is a non-overridable testing framework method that checks that its first argument
- * is equal to `checkTrue` and throws otherwise.
+ * Holds if `mc` is a call to a method that checks that its argument `arg` is
+ * equal to `checkTrue` and throws otherwise.
  */
-private predicate condtionCheckMethodTestingFramework(Method m, int argument, boolean checkTrue) {
- argument = 0 and
- (
- m.getDeclaringType().hasQualifiedName("org.junit", "Assume") and
- checkTrue = true and
- m.hasName("assumeTrue")
+predicate methodCallChecksBoolean(MethodCall mc, Expr arg, boolean checkTrue) {
+ exists(int pos | mc.getArgument(pos) = arg |
+ methodCheckTrue(mc.getMethod().getSourceDeclaration(), pos) and checkTrue = true
  or
- m.getDeclaringType().hasQualifiedName("org.junit.jupiter.api", "Assertions") and
- (
- checkTrue = true and m.hasName("assertTrue")
- or
- checkTrue = false and m.hasName("assertFalse")
- )
- or
- m.getDeclaringType().hasQualifiedName("org.junit.jupiter.api", "Assumptions") and
- (
- checkTrue = true and m.hasName("assumeTrue")
- or
- checkTrue = false and m.hasName("assumeFalse")
- )
+ methodCheckFalse(mc.getMethod().getSourceDeclaration(), pos) and checkTrue = false
  )
- or
- m.getDeclaringType().hasQualifiedName(["org.junit", "org.testng"], "Assert") and
- m.getParameter(argument).getType() instanceof BooleanType and
- (
- checkTrue = true and m.hasName("assertTrue")
+}
+
+/**
+ * Holds if `mc` is a call to a method that checks that its argument `arg` is
+ * not null and throws otherwise.
+ */
+predicate methodCallChecksNotNull(MethodCall mc, Expr arg) {
+ exists(int pos | mc.getArgument(pos) = arg |
+ methodCheckNotNull(mc.getMethod().getSourceDeclaration(), pos)
  or
- checkTrue = false and m.hasName("assertFalse")
+ methodCheckThat(mc.getMethod().getSourceDeclaration(), pos) and
+ mc.getAnArgument().(MethodCall).getMethod().getName() = "notNullValue"
  )
 }
 
 /**
+ * Holds if `mc` is a call to a method that checks one of its arguments in some
+ * way and possibly throws.
+ */
+predicate methodCallChecksArgument(MethodCall mc) {
+ methodCallChecksBoolean(mc, _, _) or
+ methodCallChecksNotNull(mc, _)
+}
+
+/** Holds if `mc` is a call to a method that unconditionally throws. */
+predicate methodCallUnconditionallyThrows(MethodCall mc) {
+ methodUnconditionallyThrows(mc.getMethod().getSourceDeclaration()) or
+ exists(BooleanLiteral b | methodCallChecksBoolean(mc, b, b.getBooleanValue().booleanNot()))
+}
+
+/**
+ * DEPRECATED: Use `methodCallChecksBoolean` instead.
+ *
+ * Holds if `m` is a non-overridable method that checks that its zero-indexed `argument`
+ * is equal to `checkTrue` and throws otherwise.
+ */
+deprecated predicate conditionCheckMethodArgument(Method m, int argument, boolean checkTrue) {
+ methodCheckTrue(m, argument) and checkTrue = true
+ or
+ methodCheckFalse(m, argument) and checkTrue = false
+}
+
+/**
+ * DEPRECATED: Use `methodCallChecksBoolean` instead.
+ *
  * Holds if `ma` is an access to a non-overridable method that checks that its
  * zero-indexed `argument` is equal to `checkTrue` and throws otherwise.
  */
-predicate conditionCheckArgument(MethodCall ma, int argument, boolean checkTrue) {
+deprecated predicate conditionCheckArgument(MethodCall ma, int argument, boolean checkTrue) {
  conditionCheckMethodArgument(ma.getMethod().getSourceDeclaration(), argument, checkTrue)
 }

java/ql/lib/semmle/code/java/dataflow/Nullness.qll

@@ -42,7 +42,7 @@ private import RangeUtils
 private import IntegerGuards
 private import NullGuards
 private import semmle.code.java.Collections
-private import semmle.code.java.frameworks.Assertions
+private import semmle.code.java.controlflow.internal.Preconditions
 
 /** Gets an expression that may be `null`. */
 Expr nullExpr() {
@@ -140,20 +140,11 @@ private ControlFlowNode varDereference(SsaVariable v, VarAccess va) {
  * A `ControlFlowNode` that ensures that the SSA variable is not null in any
  * subsequent use, either by dereferencing it or by an assertion.
  */
-private ControlFlowNode ensureNotNull(SsaVariable v) {
- result = varDereference(v, _)
- or
- exists(AssertTrueMethod m | result.asCall() = m.getACheck(directNullGuard(v, true, false)))
- or
- exists(AssertFalseMethod m | result.asCall() = m.getACheck(directNullGuard(v, false, false)))
- or
- exists(AssertNotNullMethod m | result.asCall() = m.getACheck(v.getAUse()))
- or
- exists(AssertThatMethod m, MethodCall ma |
- result.asCall() = m.getACheck(v.getAUse()) and ma.getControlFlowNode() = result
- |
- ma.getAnArgument().(MethodCall).getMethod().getName() = "notNullValue"
- )
+private ControlFlowNode ensureNotNull(SsaVariable v) { result = varDereference(v, _) }
+
+private predicate assertFail(BasicBlock bb, ControlFlowNode n) {
+ bb = n.getBasicBlock() and
+ methodCallUnconditionallyThrows(n.asExpr())
 }
 
 /**

java/ql/lib/semmle/code/java/frameworks/Assertions.qll

@@ -1,12 +1,14 @@
 /**
+ * DEPRECATED.
+ *
  * A library providing uniform access to various assertion frameworks.
  *
  * Currently supports `org.junit.Assert`, `junit.framework.*`,
  * `org.junit.jupiter.api.Assertions`, `com.google.common.base.Preconditions`,
  * and `java.util.Objects`.
  */
 overlay[local?]
-module;
+deprecated module;
 
 import java
 

java/ql/src/Violations of Best Practice/Dead Code/DeadLocals.qll

@@ -4,7 +4,7 @@
 
 import java
 import semmle.code.java.dataflow.SSA
-private import semmle.code.java.frameworks.Assertions
+private import semmle.code.java.controlflow.internal.Preconditions
 
 private predicate emptyDecl(LocalVariableDeclExpr decl) {
  not exists(decl.getInit()) and
@@ -22,7 +22,19 @@ predicate deadLocal(VariableUpdate upd) {
 /**
  * A dead variable update that is expected to be dead as indicated by an assertion.
  */
-predicate expectedDead(VariableUpdate upd) { assertFail(upd.getBasicBlock(), _) }
+predicate expectedDead(VariableUpdate upd) {
+ exists(BasicBlock bb, ControlFlowNode n |
+ bb = upd.getBasicBlock() and
+ bb = n.getBasicBlock()
+ |
+ methodCallUnconditionallyThrows(n.asExpr())
+ or
+ exists(AssertStmt a |
+ n.asExpr() = a.getExpr() and
+ a.getExpr().(BooleanLiteral).getBooleanValue() = false
+ )
+ )
+}
 
 /**
  * A dead update that is overwritten by a live update.

java/ql/test/library-tests/guards/guardspreconditions.expected

@@ -1,35 +1,19 @@
 | Preconditions.java:8:9:8:31 | assertTrue(...) | exception | Preconditions.java:7:10:7:14 | Exceptional Exit |
 | Preconditions.java:8:9:8:31 | assertTrue(...) | no exception | Preconditions.java:9:9:9:18 | <Expr>; |
-| Preconditions.java:13:9:13:32 | assertTrue(...) | exception | Preconditions.java:12:10:12:14 | Exceptional Exit |
-| Preconditions.java:13:9:13:32 | assertTrue(...) | no exception | Preconditions.java:14:9:14:18 | <Expr>; |
 | Preconditions.java:18:9:18:33 | assertFalse(...) | exception | Preconditions.java:17:10:17:14 | Exceptional Exit |
 | Preconditions.java:18:9:18:33 | assertFalse(...) | no exception | Preconditions.java:19:9:19:18 | <Expr>; |
-| Preconditions.java:23:9:23:32 | assertFalse(...) | exception | Preconditions.java:22:10:22:14 | Exceptional Exit |
-| Preconditions.java:23:9:23:32 | assertFalse(...) | no exception | Preconditions.java:24:9:24:18 | <Expr>; |
 | Preconditions.java:28:9:28:41 | assertTrue(...) | exception | Preconditions.java:27:10:27:14 | Exceptional Exit |
 | Preconditions.java:28:9:28:41 | assertTrue(...) | no exception | Preconditions.java:29:9:29:18 | <Expr>; |
-| Preconditions.java:33:9:33:42 | assertTrue(...) | exception | Preconditions.java:32:10:32:14 | Exceptional Exit |
-| Preconditions.java:33:9:33:42 | assertTrue(...) | no exception | Preconditions.java:34:9:34:18 | <Expr>; |
 | Preconditions.java:38:9:38:43 | assertFalse(...) | exception | Preconditions.java:37:10:37:14 | Exceptional Exit |
 | Preconditions.java:38:9:38:43 | assertFalse(...) | no exception | Preconditions.java:39:9:39:18 | <Expr>; |
-| Preconditions.java:43:9:43:42 | assertFalse(...) | exception | Preconditions.java:42:10:42:14 | Exceptional Exit |
-| Preconditions.java:43:9:43:42 | assertFalse(...) | no exception | Preconditions.java:44:9:44:18 | <Expr>; |
 | Preconditions.java:48:9:48:35 | assertTrue(...) | exception | Preconditions.java:47:10:47:14 | Exceptional Exit |
 | Preconditions.java:48:9:48:35 | assertTrue(...) | no exception | Preconditions.java:49:9:49:18 | <Expr>; |
-| Preconditions.java:53:9:53:36 | assertTrue(...) | exception | Preconditions.java:52:10:52:15 | Exceptional Exit |
-| Preconditions.java:53:9:53:36 | assertTrue(...) | no exception | Preconditions.java:54:9:54:18 | <Expr>; |
 | Preconditions.java:58:9:58:37 | assertFalse(...) | exception | Preconditions.java:57:10:57:15 | Exceptional Exit |
 | Preconditions.java:58:9:58:37 | assertFalse(...) | no exception | Preconditions.java:59:9:59:18 | <Expr>; |
-| Preconditions.java:63:9:63:36 | assertFalse(...) | exception | Preconditions.java:62:10:62:15 | Exceptional Exit |
-| Preconditions.java:63:9:63:36 | assertFalse(...) | no exception | Preconditions.java:64:9:64:18 | <Expr>; |
 | Preconditions.java:68:9:68:45 | assertTrue(...) | exception | Preconditions.java:67:10:67:15 | Exceptional Exit |
 | Preconditions.java:68:9:68:45 | assertTrue(...) | no exception | Preconditions.java:69:9:69:18 | <Expr>; |
-| Preconditions.java:73:9:73:46 | assertTrue(...) | exception | Preconditions.java:72:10:72:15 | Exceptional Exit |
-| Preconditions.java:73:9:73:46 | assertTrue(...) | no exception | Preconditions.java:74:9:74:18 | <Expr>; |
 | Preconditions.java:78:9:78:47 | assertFalse(...) | exception | Preconditions.java:77:10:77:15 | Exceptional Exit |
 | Preconditions.java:78:9:78:47 | assertFalse(...) | no exception | Preconditions.java:79:9:79:18 | <Expr>; |
-| Preconditions.java:83:9:83:46 | assertFalse(...) | exception | Preconditions.java:82:10:82:15 | Exceptional Exit |
-| Preconditions.java:83:9:83:46 | assertFalse(...) | no exception | Preconditions.java:84:9:84:18 | <Expr>; |
 | Preconditions.java:88:9:88:15 | t(...) | exception | Preconditions.java:87:10:87:15 | Exceptional Exit |
 | Preconditions.java:88:9:88:15 | t(...) | no exception | Preconditions.java:89:9:89:18 | <Expr>; |
 | Preconditions.java:93:9:93:16 | t(...) | exception | Preconditions.java:92:10:92:15 | Exceptional Exit |