Java: Consolidate Assertions.qll and Preconditions.qll.

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 method that checks that its argument at position `arg` is
+ * equal to true and throws otherwise.
+ */
+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
+  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
+  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
+  arg = m.getNumberOfParameters() - 1 and
+  (
+    m.hasQualifiedName("org.junit", "Assert", "assertNotNull") or
+    m.hasQualifiedName("junit.framework", _, "assertNotNull")
+  )
+}
+
+/**
+ * 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")
+  )
+}
+
+/** 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 `mc` is a call to a method that checks that its argument `arg` is
+ * equal to `checkTrue` and throws otherwise.
+ */
+predicate methodCallChecksBoolean(MethodCall mc, Expr arg, boolean checkTrue) {
+  exists(int pos | mc.getArgument(pos) = arg |
+    methodCheckTrue(mc.getMethod().getSourceDeclaration(), pos) and checkTrue = true
+    or
+    methodCheckFalse(mc.getMethod().getSourceDeclaration(), pos) and checkTrue = false
+  )
+}
+
+/**
+ * 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
+    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.
  */
-predicate conditionCheckMethodArgument(Method m, int argument, boolean checkTrue) {
-  condtionCheckMethodGooglePreconditions(m, checkTrue) and argument = 0
+deprecated predicate conditionCheckMethodArgument(Method m, int argument, boolean checkTrue) {
+  methodCheckTrue(m, argument) and checkTrue = true
   or
-  conditionCheckMethodApacheCommonsLang3Validate(m, checkTrue) and argument = 0
-  or
-  condtionCheckMethodTestingFramework(m, argument, checkTrue)
-  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
-    )
-  )
-  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
-    )
-  )
-}
-
-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"))
-}
-
-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 non-overridable testing framework method that checks that its first argument
- * 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")
-    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")
-    )
-  )
-  or
-  m.getDeclaringType().hasQualifiedName(["org.junit", "org.testng"], "Assert") and
-  m.getParameter(argument).getType() instanceof BooleanType and
-  (
-    checkTrue = true and m.hasName("assertTrue")
-    or
-    checkTrue = false and m.hasName("assertFalse")
-  )
+  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,4 +1,6 @@
 /**
+ * DEPRECATED.
+ *
  * A library providing uniform access to various assertion frameworks.
  *
  * Currently supports `org.junit.Assert`, `junit.framework.*`,
@@ -6,7 +8,7 @@
  * 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.