Merge pull request #8611 from github/smowton/doc/switch-expr-accessors

Java: make SwitchCase.getRuleExpression/Statement more consistent

Author: aschackmull

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

@@ -438,13 +438,27 @@ class SwitchCase extends Stmt, @case {
 
   /**
    * Gets the expression on the right-hand side of the arrow, if any.
+   *
+   * Note, this predicate gets a value when this switch case is of the form
+   * `case e1 -> e2`, where `e2` is neither a block nor a throw statement.
+   * This predicate is mutually exclusive with `getRuleStatement`.
    */
-  Expr getRuleExpression() { result.getParent() = this and result.getIndex() = -1 }
+  Expr getRuleExpression() {
+    result.getParent() = this and result.getIndex() = -1
+    or
+    exists(ExprStmt es | es.getParent() = this and es.getIndex() = -1 | result = es.getExpr())
+  }
 
   /**
    * Gets the statement on the right-hand side of the arrow, if any.
+   *
+   * Note, this predicate gets a value when this switch case is of the form
+   * `case e1 -> { s1; s2; ... }` or `case e1 -> throw ...`.
+   * This predicate is mutually exclusive with `getRuleExpression`.
    */
-  Stmt getRuleStatement() { result.getParent() = this and result.getIndex() = -1 }
+  Stmt getRuleStatement() {
+    result.getParent() = this and result.getIndex() = -1 and not result instanceof ExprStmt
+  }
 }
 
 /** A constant `case` of a switch statement. */

java/ql/src/change-notes/2022-04-01-switch-rule-expressions.md

@@ -0,0 +1,5 @@
+---
+category: minorAnalysis
+---
+* The `SwitchCase.getRuleExpression()` predicate now gets expressions for case rules with an expression on the right-hand side of the arrow belonging to both `SwitchStmt` and `SwitchExpr`, and the corresponding `getRuleStatement()` no longer returns an `ExprStmt` in either case. Previously `SwitchStmt` and `SwitchExpr` behaved differently in 
+this respect.

java/ql/test/library-tests/dataflow/switchexpr/TestSwitchExprStmtConsistency.java

@@ -0,0 +1,23 @@
+public class TestSwitchExprStmtConsistency {
+
+  static int f() { return 0; }
+
+  public static void test(int x) {
+
+    // Test that getRuleExpression() and getRuleStatement() behave alike for switch expressions and statements using arrow rules.
+
+    switch(x) {
+      case 1 -> f();
+      case 2 -> f();
+      default -> f();
+    }
+
+    int result = switch(x) {
+      case 1 -> f();
+      case 2 -> f();
+      default -> f();
+    };
+
+  }
+
+}

java/ql/test/library-tests/dataflow/switchexpr/switchcasearrows.expected

@@ -0,0 +1,12 @@
+exprs
+| TestSwitchExpr.java:9:13:9:41 | case ... | TestSwitchExpr.java:9:43:9:46 | null |
+| TestSwitchExpr.java:10:13:10:22 | default | TestSwitchExpr.java:10:24:10:25 | x1 |
+| TestSwitchExprStmtConsistency.java:10:7:10:15 | case ... | TestSwitchExprStmtConsistency.java:10:17:10:19 | f(...) |
+| TestSwitchExprStmtConsistency.java:11:7:11:15 | case ... | TestSwitchExprStmtConsistency.java:11:17:11:19 | f(...) |
+| TestSwitchExprStmtConsistency.java:12:7:12:16 | default | TestSwitchExprStmtConsistency.java:12:18:12:20 | f(...) |
+| TestSwitchExprStmtConsistency.java:16:7:16:15 | case ... | TestSwitchExprStmtConsistency.java:16:17:16:19 | f(...) |
+| TestSwitchExprStmtConsistency.java:17:7:17:15 | case ... | TestSwitchExprStmtConsistency.java:17:17:17:19 | f(...) |
+| TestSwitchExprStmtConsistency.java:18:7:18:16 | default | TestSwitchExprStmtConsistency.java:18:18:18:20 | f(...) |
+stmts
+| TestSwitchExpr.java:13:13:13:28 | case ... | TestSwitchExpr.java:13:30:13:42 | { ... } |
+| TestSwitchExpr.java:14:13:14:22 | default | TestSwitchExpr.java:14:24:14:52 | throw ... |

java/ql/test/library-tests/dataflow/switchexpr/switchcasearrows.ql

@@ -0,0 +1,5 @@
+import java
+
+query predicate exprs(SwitchCase sc, Expr e) { e = sc.getRuleExpression() }
+
+query predicate stmts(SwitchCase sc, Stmt s) { s = sc.getRuleStatement() }