From a2778eee75d9640a4c8c98e56f4bbc7e93406ac7 Mon Sep 17 00:00:00 2001
From: Anders Schack-Mulligen <aschackmull@github.com>
Date: Wed, 14 May 2025 11:22:58 +0200
Subject: [PATCH] Java: Refactor clearlyNotNullExpr into a base case that does
 not rely on SSA.

---
 .../semmle/code/java/dataflow/NullGuards.qll  | 50 +++++++++++--------
 1 file changed, 30 insertions(+), 20 deletions(-)

diff --git a/java/ql/lib/semmle/code/java/dataflow/NullGuards.qll b/java/ql/lib/semmle/code/java/dataflow/NullGuards.qll
index 2dd72d78a2e..971fc3cf072 100644
--- a/java/ql/lib/semmle/code/java/dataflow/NullGuards.qll
+++ b/java/ql/lib/semmle/code/java/dataflow/NullGuards.qll
@@ -40,35 +40,45 @@ EqualityTest varEqualityTestExpr(SsaVariable v1, SsaVariable v2, boolean isEqual
   isEqualExpr = result.polarity()
 }
 
-/** Gets an expression that is provably not `null`. */
-Expr clearlyNotNullExpr(Expr reason) {
-  result instanceof ClassInstanceExpr and reason = result
+Expr baseNotNullExpr() {
+  result instanceof ClassInstanceExpr
   or
-  result instanceof ArrayCreationExpr and reason = result
+  result instanceof ArrayCreationExpr
   or
-  result instanceof TypeLiteral and reason = result
+  result instanceof TypeLiteral
   or
-  result instanceof ThisAccess and reason = result
+  result instanceof ThisAccess
   or
-  result instanceof StringLiteral and reason = result
+  result instanceof StringLiteral
   or
-  result instanceof AddExpr and result.getType() instanceof TypeString and reason = result
+  result instanceof AddExpr and result.getType() instanceof TypeString
   or
   exists(Field f |
     result = f.getAnAccess() and
     (f.hasName("TRUE") or f.hasName("FALSE")) and
-    f.getDeclaringType().hasQualifiedName("java.lang", "Boolean") and
-    reason = result
+    f.getDeclaringType().hasQualifiedName("java.lang", "Boolean")
   )
   or
+  result = any(EnumConstant c).getAnAccess()
+  or
+  result instanceof ImplicitNotNullExpr
+  or
+  result instanceof ImplicitCoercionToUnitExpr
+  or
+  result
+      .(MethodCall)
+      .getMethod()
+      .hasQualifiedName("com.google.common.base", "Strings", "nullToEmpty")
+}
+
+/** Gets an expression that is provably not `null`. */
+Expr clearlyNotNullExpr(Expr reason) {
+  result = baseNotNullExpr() and reason = result
+  or
   result.(CastExpr).getExpr() = clearlyNotNullExpr(reason)
   or
   result.(ImplicitCastExpr).getExpr() = clearlyNotNullExpr(reason)
   or
-  result instanceof ImplicitNotNullExpr and reason = result
-  or
-  result instanceof ImplicitCoercionToUnitExpr and reason = result
-  or
   result.(AssignExpr).getSource() = clearlyNotNullExpr(reason)
   or
   exists(ConditionalExpr c, Expr r1, Expr r2 |
@@ -83,14 +93,14 @@ Expr clearlyNotNullExpr(Expr reason) {
     guard.controls(rval.getBasicBlock(), branch) and
     reason = guard and
     rval = v.getAUse() and
-    result = rval
+    result = rval and
+    not result = baseNotNullExpr()
   )
   or
-  exists(SsaVariable v | clearlyNotNull(v, reason) and result = v.getAUse())
-  or
-  exists(Method m | m = result.(MethodCall).getMethod() and reason = result |
-    m.getDeclaringType().hasQualifiedName("com.google.common.base", "Strings") and
-    m.hasName("nullToEmpty")
+  exists(SsaVariable v |
+    clearlyNotNull(v, reason) and
+    result = v.getAUse() and
+    not result = baseNotNullExpr()
   )
 }