Nullness library: recognise switches with null checks

smowton · smowton · commit 6b9aed21dffb · 2023-11-30T11:23:58.000Z
diff --git a/java/ql/lib/semmle/code/java/Expr.qll b/java/ql/lib/semmle/code/java/Expr.qll
@@ -1542,6 +1542,11 @@ class SwitchExpr extends Expr, StmtParent, @switchexpr {
     exists(YieldStmt yield | yield.getTarget() = this and result = yield.getValue())
   }
 
+  /** Holds if this switch has a case handling a null literal. */
+  predicate hasNullCase() {
+    this.getAConstCase().getValue(_) instanceof NullLiteral
+  }
+
   /** Gets a printable representation of this expression. */
   override string toString() { result = "switch (...)" }
 
diff --git a/java/ql/lib/semmle/code/java/Statement.qll b/java/ql/lib/semmle/code/java/Statement.qll
@@ -409,6 +409,11 @@ class SwitchStmt extends Stmt, @switchstmt {
   /** Gets the expression of this `switch` statement. */
   Expr getExpr() { result.getParent() = this }
 
+  /** Holds if this switch has a case handling a null literal. */
+  predicate hasNullCase() {
+    this.getAConstCase().getValue(_) instanceof NullLiteral
+  }
+
   override string pp() { result = "switch (...)" }
 
   override string toString() { result = "switch (...)" }
diff --git a/java/ql/lib/semmle/code/java/dataflow/Nullness.qll b/java/ql/lib/semmle/code/java/dataflow/Nullness.qll
@@ -100,9 +100,9 @@ predicate dereference(Expr e) {
   or
   exists(SynchronizedStmt synch | synch.getExpr() = e)
   or
-  exists(SwitchStmt switch | switch.getExpr() = e)
+  exists(SwitchStmt switch | switch.getExpr() = e and not switch.hasNullCase())
   or
-  exists(SwitchExpr switch | switch.getExpr() = e)
+  exists(SwitchExpr switch | switch.getExpr() = e and not switch.hasNullCase())
   or
   exists(FieldAccess fa, Field f | fa.getQualifier() = e and fa.getField() = f and not f.isStatic())
   or
diff --git a/java/ql/test/query-tests/Nullness/G.java b/java/ql/test/query-tests/Nullness/G.java
@@ -0,0 +1,22 @@
+public class G {
+
+  public static void test(String s) {
+
+    if (s == null) {
+      System.out.println("Is null");
+    }
+
+    switch(s) { // OK; null case means this doesn't throw.
+      case null -> System.out.println("Null");
+      case "foo" -> System.out.println("Foo");
+      default -> System.out.println("Something else");
+    }
+
+    switch(s) { // BAD; lack of a null case means this may throw.
+      case "foo" -> System.out.println("Foo");
+      default -> System.out.println("Something else");
+    }
+
+  }
+
+}
diff --git a/java/ql/test/query-tests/Nullness/NullMaybe.expected b/java/ql/test/query-tests/Nullness/NullMaybe.expected
@@ -35,3 +35,4 @@
 | C.java:233:7:233:8 | xs | Variable $@ may be null at this access because of $@ assignment. | C.java:231:5:231:56 | int[] xs | xs | C.java:231:11:231:55 | xs | this |
 | F.java:11:5:11:7 | obj | Variable $@ may be null at this access as suggested by $@ null guard. | F.java:8:18:8:27 | obj | obj | F.java:9:9:9:19 | ... == ... | this |
 | F.java:17:5:17:7 | obj | Variable $@ may be null at this access as suggested by $@ null guard. | F.java:14:18:14:27 | obj | obj | F.java:15:9:15:19 | ... == ... | this |
+| G.java:15:12:15:12 | s | Variable $@ may be null at this access as suggested by $@ null guard. | G.java:3:27:3:34 | s | s | G.java:5:9:5:17 | ... == ... | this |
diff --git a/java/ql/test/query-tests/Nullness/options b/java/ql/test/query-tests/Nullness/options
@@ -1 +1 @@
-//semmle-extractor-options: --javac-args -cp ${testdir}/../../stubs/junit-4.11:${testdir}/../../stubs/hamcrest-2.2:${testdir}/../../stubs/junit-jupiter-api-5.2.0
+//semmle-extractor-options: --javac-args --release 21 -cp ${testdir}/../../stubs/junit-4.11:${testdir}/../../stubs/hamcrest-2.2:${testdir}/../../stubs/junit-jupiter-api-5.2.0

Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		`-//semmle-extractor-options: --javac-args -cp ${testdir}/../../stubs/junit-4.11:${testdir}/../../stubs/hamcrest-2.2:${testdir}/../../stubs/junit-jupiter-api-5.2.0`
	`1`	`+//semmle-extractor-options: --javac-args --release 21 -cp ${testdir}/../../stubs/junit-4.11:${testdir}/../../stubs/hamcrest-2.2:${testdir}/../../stubs/junit-jupiter-api-5.2.0`