Add pattern-case support and generally debug switch CFGs

These were reasonably broken beforehand, due to not taking switch rules into account in enough places, and confusing the expression/statement switch rule distinction with the distinction between switch statements and expressions.

(For example, `switch(x) { 1 -> System.out.println("Hello world") ... }` is a statement, but has a rule expression).

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

@@ -435,7 +435,7 @@ private module ControlFlowGraphImpl {
   }
 
   /**
-   * Gets a SwitchCase's successor SwitchCase, if any.
+   * Holds if `succ` is `pred`'s successor `SwitchCase`.
    */
   private predicate nextSwitchCase(SwitchCase pred, SwitchCase succ) {
     exists(SwitchExpr se, int idx | se.getCase(idx) = pred and se.getCase(idx + 1) = succ)
@@ -723,7 +723,7 @@ private module ControlFlowGraphImpl {
    */
   private predicate last(ControlFlowNode n, ControlFlowNode last, Completion completion) {
     // Exceptions are propagated from any sub-expression.
-    // As are any break, continue, or return completions.
+    // As are any break, yield, continue, or return completions.
     exists(Expr e | e.getParent() = n |
       last(e, last, completion) and not completion instanceof NormalOrBooleanCompletion
     )
@@ -859,7 +859,7 @@ private module ControlFlowGraphImpl {
       // any other abnormal completion is propagated
       last(switch.getAStmt(), last, completion) and
       completion != anonymousBreakCompletion() and
-      completion != NormalCompletion()
+      not completion instanceof NormalOrBooleanCompletion
       or
       // if the last case completes normally, then so does the switch
       last(switch.getStmt(strictcount(switch.getAStmt()) - 1), last, NormalCompletion()) and
@@ -879,32 +879,43 @@ private module ControlFlowGraphImpl {
       // any other abnormal completion is propagated
       last(switch.getAStmt(), last, completion) and
       not completion instanceof YieldCompletion and
-      completion != NormalCompletion()
+      not completion instanceof NormalOrBooleanCompletion
     )
     or
-    // the last node in a case rule is the last node in the right-hand side
-    // if the rhs is a statement we wrap the completion as a break
-    exists(Completion caseCompletion |
-      last(n.(SwitchCase).getRuleStatement(), last, caseCompletion) and
+    // the last node in a case rule in statement context is the last node in the right-hand side.
+    // If the rhs is a statement, we wrap the completion as a break.
+    exists(Completion caseCompletion, SwitchStmt parent, SwitchCase case |
+      case = n and
+      case = parent.getACase() and
+      last(case.getRuleStatementOrExpressionStatement(), last, caseCompletion) and
       if caseCompletion instanceof NormalOrBooleanCompletion
       then completion = anonymousBreakCompletion()
       else completion = caseCompletion
     )
     or
-    // ...and if the rhs is an expression we wrap the completion as a yield
-    exists(Completion caseCompletion |
-      last(n.(SwitchCase).getRuleExpression(), last, caseCompletion) and
-      if caseCompletion instanceof NormalOrBooleanCompletion
-      then completion = YieldCompletion(caseCompletion)
-      else completion = caseCompletion
+    // ...and when a switch occurs in expression context, we wrap the RHS in a yield statement.
+    // Note the wrapping can only occur in the expression case, because a statement would need
+    // to have explicit `yield` statements.
+    exists(SwitchExpr parent, SwitchCase case |
+      case = n and
+      case = parent.getACase() and
+      (
+        exists(Completion caseCompletion |
+          last(case.getRuleExpression(), last, caseCompletion) and
+          if caseCompletion instanceof NormalOrBooleanCompletion
+          then completion = YieldCompletion(caseCompletion)
+          else completion = caseCompletion
+        )
+        or
+        last(case.getRuleStatement(), last, completion)
+      )
     )
     or
-    // The last node in a case could always be a failing pattern check.
-    last = n.(PatternCase) and
-    completion = basicBooleanCompletion(false)
-    or
-    // The last node in a non-rule case is its variable declaration.
+    // The normal last node in a non-rule pattern case is its variable declaration.
+    // Note that either rule or non-rule pattern cases can end with pattern match failure, whereupon
+    // they branch to the next candidate pattern. This is accounted for in the `succ` relation.
     last = n.(PatternCase).getDecl() and
+    not n.(PatternCase).isRule() and
     completion = NormalCompletion()
     or
     // the last statement of a synchronized statement is the last statement of its body
@@ -1231,6 +1242,10 @@ private module ControlFlowGraphImpl {
       )
       or
       // Statements within a switch body execute sequentially.
+      // Note this includes non-rule case statements and the successful pattern match successor
+      // of a non-rule pattern case statement. Rule case statements do not complete normally
+      // (they always break or yield), and the case of pattern matching failure branching to the
+      // next case is specially handled in the `PatternCase` logic below.
       exists(int i |
         last(switch.getStmt(i), n, completion) and result = first(switch.getStmt(i + 1))
       )
@@ -1251,6 +1266,10 @@ private module ControlFlowGraphImpl {
       )
       or
       // Statements within a switch body execute sequentially.
+      // Note this includes non-rule case statements and the successful pattern match successor
+      // of a non-rule pattern case statement. Rule case statements do not complete normally
+      // (they always break or yield), and the case of pattern matching failure branching to the
+      // next case is specially handled in the `PatternCase` logic below.
       exists(int i |
         last(switch.getStmt(i), n, completion) and result = first(switch.getStmt(i + 1))
       )

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

@@ -1514,7 +1514,7 @@ class SwitchExpr extends Expr, StmtParent, @switchexpr {
    * which may be either a normal `case` or a `default`.
    */
   SwitchCase getCase(int i) {
-    result = rank[i](SwitchCase case, int idx | case.isNthChildOf(this, idx) | case order by idx)
+    result = rank[i + 1](SwitchCase case, int idx | case.isNthChildOf(this, idx) | case order by idx)
   }
 
   /**

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

@@ -387,7 +387,8 @@ class SwitchStmt extends Stmt, @switchstmt {
    * which may be either a normal `case` or a `default`.
    */
   SwitchCase getCase(int i) {
-    result = rank[i](SwitchCase case, int idx | case.isNthChildOf(this, idx) | case order by idx)
+    result =
+      rank[i + 1](SwitchCase case, int idx | case.isNthChildOf(this, idx) | case order by idx)
   }
 
   /**
@@ -469,7 +470,17 @@ class SwitchCase extends Stmt, @case {
    * This predicate is mutually exclusive with `getRuleExpression`.
    */
   Stmt getRuleStatement() {
-    result.getParent() = this and result.getIndex() = -1 and not result instanceof ExprStmt
+    result = this.getRuleStatementOrExpressionStatement() and not result instanceof ExprStmt
+  }
+
+  /**
+   * Gets the statement, including an expression statement, on the RHS of the arrow, if any.
+   *
+   * This means this could be an explicit `case e1 -> { s1; ... }` or an implicit
+   * `case e1 -> stmt;` rule.
+   */
+  Stmt getRuleStatementOrExpressionStatement() {
+    result.getParent() = this and result.getIndex() = -1
   }
 }
 

java/ql/test/library-tests/pattern-switch/cfg/Test.java

@@ -0,0 +1,39 @@
+public class Test {
+
+  public static void test(Object thing) {
+
+     switch (thing) {
+       case String s -> System.out.println(s);
+       case Integer i -> System.out.println("An integer: " + i);
+       default -> { }
+     }
+
+     switch (thing) {
+       case String s:
+         System.out.println(s);
+         break;
+       case Integer i:
+         System.out.println("An integer:" + i);
+         break;
+       default:
+         break;
+     }
+
+     var thingAsString = switch(thing) {
+       case String s -> s;
+       case Integer i -> "An integer: " + i;
+       default -> "Something else";
+     };
+
+     var thingAsString2 = switch(thing) {
+       case String s:
+         yield s;
+       case Integer i:
+         yield "An integer: " + i;
+       default:
+         yield "Something else";
+     };
+
+  }
+
+}

java/ql/test/library-tests/pattern-switch/cfg/options

@@ -0,0 +1 @@
+//semmle-extractor-options: --javac-args --release 21

java/ql/test/library-tests/pattern-switch/cfg/test.expected

@@ -0,0 +1,80 @@
+| Test.java:1:14:1:17 | super(...) | Test.java:1:14:1:17 | Test |
+| Test.java:1:14:1:17 | { ... } | Test.java:1:14:1:17 | super(...) |
+| Test.java:3:41:37:3 | { ... } | Test.java:5:6:5:19 | switch (...) |
+| Test.java:5:6:5:19 | switch (...) | Test.java:5:14:5:18 | thing |
+| Test.java:5:14:5:18 | thing | Test.java:6:8:6:23 | case T t ... |
+| Test.java:6:8:6:23 | case T t ... | Test.java:6:20:6:20 | s |
+| Test.java:6:8:6:23 | case T t ... | Test.java:7:8:7:24 | case T t ... |
+| Test.java:6:20:6:20 | s | Test.java:6:25:6:34 | System.out |
+| Test.java:6:25:6:34 | System.out | Test.java:6:44:6:44 | s |
+| Test.java:6:25:6:45 | println(...) | Test.java:11:6:11:19 | switch (...) |
+| Test.java:6:25:6:46 | <Expr>; | Test.java:6:25:6:34 | System.out |
+| Test.java:6:44:6:44 | s | Test.java:6:25:6:45 | println(...) |
+| Test.java:7:8:7:24 | case T t ... | Test.java:7:21:7:21 | i |
+| Test.java:7:8:7:24 | case T t ... | Test.java:8:8:8:17 | default |
+| Test.java:7:21:7:21 | i | Test.java:7:26:7:35 | System.out |
+| Test.java:7:26:7:35 | System.out | Test.java:7:45:7:58 | "An integer: " |
+| Test.java:7:26:7:63 | println(...) | Test.java:11:6:11:19 | switch (...) |
+| Test.java:7:26:7:64 | <Expr>; | Test.java:7:26:7:35 | System.out |
+| Test.java:7:45:7:58 | "An integer: " | Test.java:7:62:7:62 | i |
+| Test.java:7:45:7:62 | ... + ... | Test.java:7:26:7:63 | println(...) |
+| Test.java:7:62:7:62 | i | Test.java:7:45:7:62 | ... + ... |
+| Test.java:8:8:8:17 | default | Test.java:8:19:8:21 | { ... } |
+| Test.java:8:19:8:21 | { ... } | Test.java:11:6:11:19 | switch (...) |
+| Test.java:11:6:11:19 | switch (...) | Test.java:11:14:11:18 | thing |
+| Test.java:11:14:11:18 | thing | Test.java:12:8:12:21 | case T t ... |
+| Test.java:12:8:12:21 | case T t ... | Test.java:12:20:12:20 | s |
+| Test.java:12:8:12:21 | case T t ... | Test.java:15:8:15:22 | case T t ... |
+| Test.java:12:20:12:20 | s | Test.java:13:10:13:31 | <Expr>; |
+| Test.java:13:10:13:19 | System.out | Test.java:13:29:13:29 | s |
+| Test.java:13:10:13:30 | println(...) | Test.java:14:10:14:15 | break |
+| Test.java:13:10:13:31 | <Expr>; | Test.java:13:10:13:19 | System.out |
+| Test.java:13:29:13:29 | s | Test.java:13:10:13:30 | println(...) |
+| Test.java:14:10:14:15 | break | Test.java:22:6:26:7 | var ...; |
+| Test.java:15:8:15:22 | case T t ... | Test.java:15:21:15:21 | i |
+| Test.java:15:8:15:22 | case T t ... | Test.java:18:8:18:15 | default |
+| Test.java:15:21:15:21 | i | Test.java:16:10:16:47 | <Expr>; |
+| Test.java:16:10:16:19 | System.out | Test.java:16:29:16:41 | "An integer:" |
+| Test.java:16:10:16:46 | println(...) | Test.java:17:10:17:15 | break |
+| Test.java:16:10:16:47 | <Expr>; | Test.java:16:10:16:19 | System.out |
+| Test.java:16:29:16:41 | "An integer:" | Test.java:16:45:16:45 | i |
+| Test.java:16:29:16:45 | ... + ... | Test.java:16:10:16:46 | println(...) |
+| Test.java:16:45:16:45 | i | Test.java:16:29:16:45 | ... + ... |
+| Test.java:17:10:17:15 | break | Test.java:22:6:26:7 | var ...; |
+| Test.java:18:8:18:15 | default | Test.java:19:10:19:15 | break |
+| Test.java:19:10:19:15 | break | Test.java:22:6:26:7 | var ...; |
+| Test.java:22:6:26:7 | var ...; | Test.java:22:26:22:38 | switch (...) |
+| Test.java:22:10:22:38 | thingAsString | Test.java:28:6:35:7 | var ...; |
+| Test.java:22:26:22:38 | switch (...) | Test.java:22:33:22:37 | thing |
+| Test.java:22:33:22:37 | thing | Test.java:23:8:23:23 | case T t ... |
+| Test.java:23:8:23:23 | case T t ... | Test.java:23:20:23:20 | s |
+| Test.java:23:8:23:23 | case T t ... | Test.java:24:8:24:24 | case T t ... |
+| Test.java:23:20:23:20 | s | Test.java:23:25:23:25 | s |
+| Test.java:23:25:23:25 | s | Test.java:22:10:22:38 | thingAsString |
+| Test.java:24:8:24:24 | case T t ... | Test.java:24:21:24:21 | i |
+| Test.java:24:8:24:24 | case T t ... | Test.java:25:8:25:17 | default |
+| Test.java:24:21:24:21 | i | Test.java:24:26:24:39 | "An integer: " |
+| Test.java:24:26:24:39 | "An integer: " | Test.java:24:43:24:43 | i |
+| Test.java:24:26:24:43 | ... + ... | Test.java:22:10:22:38 | thingAsString |
+| Test.java:24:43:24:43 | i | Test.java:24:26:24:43 | ... + ... |
+| Test.java:25:8:25:17 | default | Test.java:25:19:25:34 | "Something else" |
+| Test.java:25:19:25:34 | "Something else" | Test.java:22:10:22:38 | thingAsString |
+| Test.java:28:6:35:7 | var ...; | Test.java:28:27:28:39 | switch (...) |
+| Test.java:28:10:28:39 | thingAsString2 | Test.java:3:22:3:25 | test |
+| Test.java:28:27:28:39 | switch (...) | Test.java:28:34:28:38 | thing |
+| Test.java:28:34:28:38 | thing | Test.java:29:8:29:21 | case T t ... |
+| Test.java:29:8:29:21 | case T t ... | Test.java:29:20:29:20 | s |
+| Test.java:29:8:29:21 | case T t ... | Test.java:31:8:31:22 | case T t ... |
+| Test.java:29:20:29:20 | s | Test.java:30:10:30:17 | yield ... |
+| Test.java:30:10:30:17 | yield ... | Test.java:30:16:30:16 | s |
+| Test.java:30:16:30:16 | s | Test.java:28:10:28:39 | thingAsString2 |
+| Test.java:31:8:31:22 | case T t ... | Test.java:31:21:31:21 | i |
+| Test.java:31:8:31:22 | case T t ... | Test.java:33:8:33:15 | default |
+| Test.java:31:21:31:21 | i | Test.java:32:10:32:34 | yield ... |
+| Test.java:32:10:32:34 | yield ... | Test.java:32:16:32:29 | "An integer: " |
+| Test.java:32:16:32:29 | "An integer: " | Test.java:32:33:32:33 | i |
+| Test.java:32:16:32:33 | ... + ... | Test.java:28:10:28:39 | thingAsString2 |
+| Test.java:32:33:32:33 | i | Test.java:32:16:32:33 | ... + ... |
+| Test.java:33:8:33:15 | default | Test.java:34:10:34:32 | yield ... |
+| Test.java:34:10:34:32 | yield ... | Test.java:34:16:34:31 | "Something else" |
+| Test.java:34:16:34:31 | "Something else" | Test.java:28:10:28:39 | thingAsString2 |

java/ql/test/library-tests/pattern-switch/cfg/test.ql

@@ -0,0 +1,5 @@
+import java
+
+from ControlFlowNode cn
+where cn.getFile().getBaseName() = "Test.java"
+select cn, cn.getASuccessor()