[Java]: Add precondition support for testing library asserts

Author: JLLeitschuh

java/ql/lib/change-notes/2022-03-18-test-assertion-guards.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added guard preconditon support for assertion methods for popular testing libraries (eg. Junit 4, Junit 5, TestNG).

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

@@ -185,7 +185,7 @@ private module ControlFlowGraphImpl {
    * Bind `t` to an unchecked exception that may occur in a precondition check.
    */
   private predicate uncheckedExceptionFromMethod(MethodAccess ma, ThrowableType t) {
-    conditionCheck(ma, _) and
+    conditionCheck(ma, _, _) and
     (t instanceof TypeError or t instanceof TypeRuntimeException)
   }
 

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

@@ -86,7 +86,7 @@ class Guard extends ExprParent {
     or
     this instanceof SwitchCase
     or
-    conditionCheck(this, _)
+    conditionCheck(this, _, _)
   }
 
   /** Gets the immediately enclosing callable whose body contains this guard. */

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

@@ -57,9 +57,9 @@ predicate implies_v1(Guard g1, boolean b1, Guard g2, boolean b2) {
   or
   g1.(DefaultCase).getSwitch().getAConstCase() = g2 and b1 = true and b2 = false
   or
-  exists(MethodAccess check | check = g1 |
-    conditionCheck(check, _) and
-    g2 = check.getArgument(0) and
+  exists(MethodAccess check, int argIndex | check = g1 |
+    conditionCheck(check, argIndex, _) and
+    g2 = check.getArgument(argIndex) and
     b1 = [true, false] and
     b2 = b1
   )

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

@@ -11,6 +11,8 @@ import java
  * is equal to `checkTrue` and throws otherwise.
  */
 predicate conditionCheckMethod(Method m, boolean checkTrue) {
+  conditionCheckMethod(m, 0, checkTrue)
+  or
   m.getDeclaringType().hasQualifiedName("com.google.common.base", "Preconditions") and
   checkTrue = true and
   (m.hasName("checkArgument") or m.hasName("checkState"))
@@ -19,6 +21,24 @@ predicate conditionCheckMethod(Method m, boolean checkTrue) {
   checkTrue = true and
   (m.hasName("isTrue") or m.hasName("validState"))
   or
+  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
   exists(Parameter p, IfStmt ifstmt, Expr cond |
     p = m.getParameter(0) and
     not m.isOverridable() and
@@ -35,13 +55,29 @@ predicate conditionCheckMethod(Method m, boolean checkTrue) {
       ifstmt.getThen().(SingletonBlock).getStmt() instanceof ThrowStmt
     )
   )
+}
+
+/**
+ * Holds if `m` is a non-overridable method that checks that its zero-indexed `argument`
+ * is equal to `checkTrue` and throws otherwise.
+ */
+predicate conditionCheckMethod(Method m, int argument, boolean checkTrue) {
+  conditionCheckMethod(m, checkTrue) and argument = 0
+  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")
+  )
   or
-  exists(Parameter p, MethodAccess ma, boolean ct, Expr arg |
-    p = m.getParameter(0) and
+  exists(Parameter p, MethodAccess ma, int argIndex, boolean ct, Expr arg |
+    p = m.getParameter(argument) and
     not m.isOverridable() and
     m.getBody().getStmt(0).(ExprStmt).getExpr() = ma and
-    conditionCheck(ma, ct) and
-    ma.getArgument(0) = arg and
+    conditionCheck(ma, argIndex, ct) and
+    ma.getArgument(argIndex) = arg and
     (
       arg.(LogNotExpr).getExpr().(VarAccess).getVariable() = p and
       checkTrue = ct.booleanNot()
@@ -58,3 +94,11 @@ predicate conditionCheckMethod(Method m, boolean checkTrue) {
 predicate conditionCheck(MethodAccess ma, boolean checkTrue) {
   conditionCheckMethod(ma.getMethod().getSourceDeclaration(), checkTrue)
 }
+
+/**
+ * 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 conditionCheck(MethodAccess ma, int argument, boolean checkTrue) {
+  conditionCheckMethod(ma.getMethod().getSourceDeclaration(), argument, checkTrue)
+}

java/ql/test/library-tests/guards/Preconditions.java

@@ -0,0 +1,95 @@
+import org.junit.Assert;
+import org.junit.jupiter.api.Assertions;
+
+public class Preconditions {
+    public static void guarded() {}
+
+    void test1() {
+        Assert.assertTrue(true);
+        guarded();
+    }
+
+    void test2() {
+        Assert.assertTrue(false);
+        guarded();
+    }
+
+    void test3() {
+        Assert.assertFalse(false);
+        guarded();
+    }
+
+    void test4() {
+        Assert.assertFalse(true);
+        guarded();
+    }
+
+    void test5() {
+        Assert.assertTrue("Reason", true);
+        guarded();
+    }
+
+    void test6() {
+        Assert.assertTrue("Reason", false);
+        guarded();
+    }
+
+    void test7() {
+        Assert.assertFalse("Reason", false);
+        guarded();
+    }
+
+    void test8() {
+        Assert.assertFalse("Reason", true);
+        guarded();
+    }
+
+    void test9() {
+        r(true);
+        guarded();
+    }
+
+    static void r(boolean b) {
+        Assert.assertTrue("Unified Reason", b);
+    }
+
+    void test10() {
+        Assertions.assertTrue(true);
+        guarded();
+    }
+
+    void test11() {
+        Assertions.assertTrue(false);
+        guarded();
+    }
+
+    void test12() {
+        Assertions.assertFalse(false);
+        guarded();
+    }
+
+    void test13() {
+        Assertions.assertFalse(true);
+        guarded();
+    }
+
+    void test14() {
+        Assertions.assertTrue(true, "Reason");
+        guarded();
+    }
+
+    void test15() {
+        Assertions.assertTrue(false, "Reason");
+        guarded();
+    }
+
+    void test16() {
+        Assertions.assertFalse(false, "Reason");
+        guarded();
+    }
+
+    void test17() {
+        Assertions.assertFalse(true, "Reason");
+        guarded();
+    }
+}

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

@@ -0,0 +1,13 @@
+| Preconditions.java:8:9:8:31 | assertTrue(...) | true | Preconditions.java:9:9:9:18 | <Expr>; |
+| Preconditions.java:13:9:13:32 | assertTrue(...) | true | Preconditions.java:14:9:14:18 | <Expr>; |
+| Preconditions.java:18:9:18:33 | assertFalse(...) | false | Preconditions.java:19:9:19:18 | <Expr>; |
+| Preconditions.java:23:9:23:32 | assertFalse(...) | false | Preconditions.java:24:9:24:18 | <Expr>; |
+| Preconditions.java:48:9:48:15 | r(...) | true | Preconditions.java:49:9:49:18 | <Expr>; |
+| Preconditions.java:57:9:57:35 | assertTrue(...) | true | Preconditions.java:58:9:58:18 | <Expr>; |
+| Preconditions.java:62:9:62:36 | assertTrue(...) | true | Preconditions.java:63:9:63:18 | <Expr>; |
+| Preconditions.java:67:9:67:37 | assertFalse(...) | false | Preconditions.java:68:9:68:18 | <Expr>; |
+| Preconditions.java:72:9:72:36 | assertFalse(...) | false | Preconditions.java:73:9:73:18 | <Expr>; |
+| Preconditions.java:77:9:77:45 | assertTrue(...) | true | Preconditions.java:78:9:78:18 | <Expr>; |
+| Preconditions.java:82:9:82:46 | assertTrue(...) | true | Preconditions.java:83:9:83:18 | <Expr>; |
+| Preconditions.java:87:9:87:47 | assertFalse(...) | false | Preconditions.java:88:9:88:18 | <Expr>; |
+| Preconditions.java:92:9:92:46 | assertFalse(...) | false | Preconditions.java:93:9:93:18 | <Expr>; |

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

@@ -0,0 +1,8 @@
+import java
+import semmle.code.java.controlflow.Guards
+
+from Guard g, BasicBlock bb, boolean branch
+where
+  g.controls(bb, branch) and
+  g.getEnclosingCallable().getDeclaringType().hasName("Preconditions")
+select g, branch, bb

java/ql/test/library-tests/guards/options

@@ -0,0 +1 @@
+//semmle-extractor-options: --javac-args -cp ${testdir}/../../stubs/junit-4.11/:${testdir}/../../stubs/junit-jupiter-api-5.2.0/