Merge pull request #14336 from aschackmull/java/switch-rule-stmt-cfg

Java: Fix CFG for case rule statements.

Author: aschackmull

java/ql/lib/change-notes/2023-09-28-case-rule-stmt-cfg-fix.md

@@ -0,0 +1,5 @@
+---
+category: minorAnalysis
+---
+* Fixed a control-flow bug where case rule statements would incorrectly include a fall-through edge.
+* Added support for default cases as proper guards in switch expressions to match switch statements.

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

@@ -871,7 +871,13 @@ private module ControlFlowGraphImpl {
     )
     or
     // the last node in a case rule is the last node in the right-hand side
-    last(n.(SwitchCase).getRuleStatement(), last, completion)
+    // if the rhs is a statement we wrap the completion as a break
+    exists(Completion caseCompletion |
+      last(n.(SwitchCase).getRuleStatement(), 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 |

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

@@ -57,6 +57,8 @@ predicate implies_v1(Guard g1, boolean b1, Guard g2, boolean b2) {
   or
   g1.(DefaultCase).getSwitch().getAConstCase() = g2 and b1 = true and b2 = false
   or
+  g1.(DefaultCase).getSwitchExpr().getAConstCase() = g2 and b1 = true and b2 = false
+  or
   exists(MethodAccess check, int argIndex | check = g1 |
     conditionCheckArgument(check, argIndex, _) and
     g2 = check.getArgument(argIndex) and

java/ql/test/library-tests/guards12/PrintAst.expected

@@ -19,5 +19,22 @@ Test.java:
 #    5|               1: [ConstCase] case ...
 #    5|                 -1: [IntegerLiteral] 2
 #    5|                 0: [StringLiteral] "c"
-#    6|               2: [DefaultCase] default
-#    6|                 -1: [IntegerLiteral] 3
+#    6|               2: [ConstCase] case ...
+#    6|                 -1: [IntegerLiteral] 2
+#    6|                 0: [StringLiteral] "d"
+#    7|               3: [DefaultCase] default
+#    7|                 -1: [IntegerLiteral] 3
+#    9|         1: [SwitchStmt] switch (...)
+#    9|           -1: [VarAccess] s
+#   10|           0: [ConstCase] case ...
+#   10|             -1: [BlockStmt] { ... }
+#   10|             0: [StringLiteral] "a"
+#   10|             1: [StringLiteral] "b"
+#   11|           1: [ConstCase] case ...
+#   11|             -1: [BlockStmt] { ... }
+#   11|             0: [StringLiteral] "c"
+#   12|           2: [ConstCase] case ...
+#   12|             -1: [BlockStmt] { ... }
+#   12|             0: [StringLiteral] "d"
+#   13|           3: [DefaultCase] default
+#   13|             -1: [BlockStmt] { ... }

java/ql/test/library-tests/guards12/Test.java

@@ -3,7 +3,14 @@ void foo(String s) {
     int x = switch(s) {
       case "a", "b" -> 1;
       case "c" -> 2;
+      case "d" -> 2;
       default -> 3;
     };
+    switch (s) {
+      case "a", "b" -> { }
+      case "c" -> { }
+      case "d" -> { }
+      default -> { }
+    }
   }
 }

java/ql/test/library-tests/guards12/guard.expected

@@ -1 +1,8 @@
+| Test.java:5:7:5:17 | case ... | Test.java:3:20:3:20 | s | Test.java:5:12:5:14 | "c" | true | false | Test.java:7:7:7:16 | default |
 | Test.java:5:7:5:17 | case ... | Test.java:3:20:3:20 | s | Test.java:5:12:5:14 | "c" | true | true | Test.java:5:7:5:17 | case ... |
+| Test.java:6:7:6:17 | case ... | Test.java:3:20:3:20 | s | Test.java:6:12:6:14 | "d" | true | false | Test.java:7:7:7:16 | default |
+| Test.java:6:7:6:17 | case ... | Test.java:3:20:3:20 | s | Test.java:6:12:6:14 | "d" | true | true | Test.java:6:7:6:17 | case ... |
+| Test.java:11:7:11:17 | case ... | Test.java:9:13:9:13 | s | Test.java:11:12:11:14 | "c" | true | false | Test.java:13:7:13:16 | default |
+| Test.java:11:7:11:17 | case ... | Test.java:9:13:9:13 | s | Test.java:11:12:11:14 | "c" | true | true | Test.java:11:7:11:17 | case ... |
+| Test.java:12:7:12:17 | case ... | Test.java:9:13:9:13 | s | Test.java:12:12:12:14 | "d" | true | false | Test.java:13:7:13:16 | default |
+| Test.java:12:7:12:17 | case ... | Test.java:9:13:9:13 | s | Test.java:12:12:12:14 | "d" | true | true | Test.java:12:7:12:17 | case ... |