CFG: Support guarded patterns

2026-04-23 07:45:17 +02:00 · 2023-10-27 18:31:27 +01:00
parent ba0f3cf718
commit 2b16121638
3 changed files with 125 additions and 11 deletions
--- a/java/ql/lib/semmle/code/java/ControlFlowGraph.qll
+++ b/java/ql/lib/semmle/code/java/ControlFlowGraph.qll
@@ -317,6 +317,8 @@ private module ControlFlowGraphImpl {
      whenexpr.getBranch(_).getAResult() = b
    )
    or
+    b = any(PatternCase pc).getGuard()
+    or
    inBooleanContext(b.(ExprStmt).getExpr())
    or
    inBooleanContext(b.(StmtExpr).getStmt())
@@ -911,12 +913,19 @@ private module ControlFlowGraphImpl {
      )
    )
    or
-    // The normal last node in a non-rule pattern case is its variable declaration.
+    // The normal last node in a non-rule pattern case is its variable declaration, or the successful
+    // matching of its guard if it has one.
    // 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()
+    exists(PatternCase pc | n = pc |
+      (
+        if exists(pc.getGuard())
+        then last(pc.getGuard(), last, BooleanCompletion(true, _))
+        else last = pc.getDecl()
+      ) and
+      not pc.isRule() and
+      completion = NormalCompletion()
+    )
    or
    // the last statement of a synchronized statement is the last statement of its body
    last(n.(SynchronizedStmt).getBlock(), last, completion)
@@ -1275,20 +1284,28 @@ private module ControlFlowGraphImpl {
      )
    )
    or
-    // Edge from rule SwitchCases to their body, after any variable assignment if applicable.
+    // Edge from rule SwitchCases to their body, after any variable assignment and/or guard test if applicable.
    // No edges in a non-rule SwitchCase - the constant expression in a ConstCase isn't included in the CFG.
    exists(SwitchCase case, ControlFlowNode preBodyNode |
-      completion = NormalCompletion() and
      if case instanceof PatternCase
-      then preBodyNode = case.(PatternCase).getDecl()
-      else preBodyNode = case
+      then (
+        if exists(case.(PatternCase).getGuard())
+        then (
+          last(case.(PatternCase).getGuard(), preBodyNode, completion) and
+          completion = basicBooleanCompletion(true)
+        ) else (
+          preBodyNode = case.(PatternCase).getDecl() and completion = NormalCompletion()
+        )
+      ) else (
+        preBodyNode = case and completion = NormalCompletion()
+      )
    |
      n = preBodyNode and result = first(case.getRuleExpression())
      or
      n = preBodyNode and result = first(case.getRuleStatement())
    )
    or
-    // A pattern case conducts a type test, then branches to either the next case or the assignment.
+    // A pattern case conducts a type test, then branches to the next case or the assignment.
    exists(PatternCase case |
      n = case and
      (
@@ -1300,6 +1317,23 @@ private module ControlFlowGraphImpl {
      )
    )
    or
+    // A pattern case with a guard evaluates that guard after declaring its pattern variable,
+    // and thereafter if the guard doesn't match will branch to the next case.
+    // The case of a matching guard is accounted for in the case-with-rule logic above, or for
+    // non-rule case statements in `last`.
+    exists(PatternCase case, Expr guard |
+      guard = case.getGuard() and
+      (
+        n = case.getDecl() and
+        result = first(guard) and
+        completion = NormalCompletion()
+        or
+        last(guard, n, completion) and
+        completion = basicBooleanCompletion(false) and
+        nextSwitchCase(case, result)
+      )
+    )
+    or
    // Yield
    exists(YieldStmt yield | completion = NormalCompletion() |
      n = yield and result = first(yield.getValue())
--- a/java/ql/test/library-tests/pattern-switch/cfg/Test.java
+++ b/java/ql/test/library-tests/pattern-switch/cfg/Test.java
@@ -34,6 +34,24 @@ public class Test {
         yield "Something else";
     };

+     switch(thing) {
+       case String s when s.length() == 3:
+         System.out.println("Length 3");
+         break;
+       case String s when s.length() == 5:
+         System.out.println("Length 5");
+         break;
+       default:
+         System.out.println("Anything else");
+         break;
+     }
+
+     switch(thing) {
+       case String s when s.length() == 3 -> System.out.println("Length 3");
+       case String s when s.length() == 5 -> System.out.println("Length 5");
+       default -> { }
+     }
+
  }

 }
--- a/java/ql/test/library-tests/pattern-switch/cfg/test.expected
+++ b/java/ql/test/library-tests/pattern-switch/cfg/test.expected
@@ -1,6 +1,6 @@
 | 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:3:41:55: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 |
@@ -60,7 +60,7 @@
 | 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:10:28:39 | thingAsString2 | Test.java:37:6:37:18 | switch (...) |
 | 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 |
@@ -78,3 +78,65 @@
 | 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 |
+| Test.java:37:6:37:18 | switch (...) | Test.java:37:13:37:17 | thing |
+| Test.java:37:13:37:17 | thing | Test.java:38:8:38:42 | case T t ... |
+| Test.java:38:8:38:42 | case T t ... | Test.java:38:20:38:20 | s |
+| Test.java:38:8:38:42 | case T t ... | Test.java:41:8:41:42 | case T t ... |
+| Test.java:38:20:38:20 | s | Test.java:38:27:38:27 | s |
+| Test.java:38:27:38:27 | s | Test.java:38:27:38:36 | length(...) |
+| Test.java:38:27:38:36 | length(...) | Test.java:38:41:38:41 | 3 |
+| Test.java:38:27:38:41 | ... == ... | Test.java:39:10:39:40 | <Expr>; |
+| Test.java:38:27:38:41 | ... == ... | Test.java:41:8:41:42 | case T t ... |
+| Test.java:38:41:38:41 | 3 | Test.java:38:27:38:41 | ... == ... |
+| Test.java:39:10:39:19 | System.out | Test.java:39:29:39:38 | "Length 3" |
+| Test.java:39:10:39:39 | println(...) | Test.java:40:10:40:15 | break |
+| Test.java:39:10:39:40 | <Expr>; | Test.java:39:10:39:19 | System.out |
+| Test.java:39:29:39:38 | "Length 3" | Test.java:39:10:39:39 | println(...) |
+| Test.java:40:10:40:15 | break | Test.java:49:6:49:18 | switch (...) |
+| Test.java:41:8:41:42 | case T t ... | Test.java:41:20:41:20 | s |
+| Test.java:41:8:41:42 | case T t ... | Test.java:44:8:44:15 | default |
+| Test.java:41:20:41:20 | s | Test.java:41:27:41:27 | s |
+| Test.java:41:27:41:27 | s | Test.java:41:27:41:36 | length(...) |
+| Test.java:41:27:41:36 | length(...) | Test.java:41:41:41:41 | 5 |
+| Test.java:41:27:41:41 | ... == ... | Test.java:42:10:42:40 | <Expr>; |
+| Test.java:41:27:41:41 | ... == ... | Test.java:44:8:44:15 | default |
+| Test.java:41:41:41:41 | 5 | Test.java:41:27:41:41 | ... == ... |
+| Test.java:42:10:42:19 | System.out | Test.java:42:29:42:38 | "Length 5" |
+| Test.java:42:10:42:39 | println(...) | Test.java:43:10:43:15 | break |
+| Test.java:42:10:42:40 | <Expr>; | Test.java:42:10:42:19 | System.out |
+| Test.java:42:29:42:38 | "Length 5" | Test.java:42:10:42:39 | println(...) |
+| Test.java:43:10:43:15 | break | Test.java:49:6:49:18 | switch (...) |
+| Test.java:44:8:44:15 | default | Test.java:45:10:45:45 | <Expr>; |
+| Test.java:45:10:45:19 | System.out | Test.java:45:29:45:43 | "Anything else" |
+| Test.java:45:10:45:44 | println(...) | Test.java:46:10:46:15 | break |
+| Test.java:45:10:45:45 | <Expr>; | Test.java:45:10:45:19 | System.out |
+| Test.java:45:29:45:43 | "Anything else" | Test.java:45:10:45:44 | println(...) |
+| Test.java:46:10:46:15 | break | Test.java:49:6:49:18 | switch (...) |
+| Test.java:49:6:49:18 | switch (...) | Test.java:49:13:49:17 | thing |
+| Test.java:49:13:49:17 | thing | Test.java:50:8:50:44 | case T t ... |
+| Test.java:50:8:50:44 | case T t ... | Test.java:50:20:50:20 | s |
+| Test.java:50:8:50:44 | case T t ... | Test.java:51:8:51:44 | case T t ... |
+| Test.java:50:20:50:20 | s | Test.java:50:27:50:27 | s |
+| Test.java:50:27:50:27 | s | Test.java:50:27:50:36 | length(...) |
+| Test.java:50:27:50:36 | length(...) | Test.java:50:41:50:41 | 3 |
+| Test.java:50:27:50:41 | ... == ... | Test.java:50:46:50:55 | System.out |
+| Test.java:50:27:50:41 | ... == ... | Test.java:51:8:51:44 | case T t ... |
+| Test.java:50:41:50:41 | 3 | Test.java:50:27:50:41 | ... == ... |
+| Test.java:50:46:50:55 | System.out | Test.java:50:65:50:74 | "Length 3" |
+| Test.java:50:46:50:75 | println(...) | Test.java:3:22:3:25 | test |
+| Test.java:50:46:50:76 | <Expr>; | Test.java:50:46:50:55 | System.out |
+| Test.java:50:65:50:74 | "Length 3" | Test.java:50:46:50:75 | println(...) |
+| Test.java:51:8:51:44 | case T t ... | Test.java:51:20:51:20 | s |
+| Test.java:51:8:51:44 | case T t ... | Test.java:52:8:52:17 | default |
+| Test.java:51:20:51:20 | s | Test.java:51:27:51:27 | s |
+| Test.java:51:27:51:27 | s | Test.java:51:27:51:36 | length(...) |
+| Test.java:51:27:51:36 | length(...) | Test.java:51:41:51:41 | 5 |
+| Test.java:51:27:51:41 | ... == ... | Test.java:51:46:51:55 | System.out |
+| Test.java:51:27:51:41 | ... == ... | Test.java:52:8:52:17 | default |
+| Test.java:51:41:51:41 | 5 | Test.java:51:27:51:41 | ... == ... |
+| Test.java:51:46:51:55 | System.out | Test.java:51:65:51:74 | "Length 5" |
+| Test.java:51:46:51:75 | println(...) | Test.java:3:22:3:25 | test |
+| Test.java:51:46:51:76 | <Expr>; | Test.java:51:46:51:55 | System.out |
+| Test.java:51:65:51:74 | "Length 5" | Test.java:51:46:51:75 | println(...) |
+| Test.java:52:8:52:17 | default | Test.java:52:19:52:21 | { ... } |
+| Test.java:52:19:52:21 | { ... } | Test.java:3:22:3:25 | test |