Ruby: Change the CFG for while clauses

The `when` node now acts as a join point for patterns in the when
clause, with match/no-match completions. This is similar to how `or`
expressions work.

The result of this is that the `when` clause "controls" the body of the
`when`, which allows us to model barrier guards for multi-pattern when
clauses.

For this code

case x
when 1, 2
  y
end

The old CFG was

x --> when --> 1 --no-match--> 2 ---no-match---> case
                \               \                  ^
                  \               \                |
                   \                --match----+   |
                     \                         |   |
                       \                       |   |
                         ------match---------> y --+

The new CFG is

x --> 1 --no-match--> 2 --no-match--> [no-match] when --no-match--> case
       \               \                                             ^
         \               \                                           |
           \               --match--> [match] when --match--> y -----+
             \                       /
               \                   /
                 -------match-----

i.e. all patterns flow to the `when` node, which is split based on
whether the pattern matched or not. The body of the when clause then has
a single predecessor `[match] when`, which acts as condition block that
controls `y`.

ruby/ql/lib/codeql/ruby/controlflow/CfgNodes.qll

@@ -446,7 +446,7 @@ module ExprNodes {
     final ExprCfgNode getBody() { e.hasCfgChild(e.getBody(), this, result) }
 
     /** Gets the `i`th pattern this `when`-clause. */
-    final ExprCfgNode getPattern(int i) { e.hasCfgChild(e.getPattern(i), this, result) }
+    final ExprCfgNode getPattern(int i) { result.getExpr() = e.getPattern(i) }
   }
 
   /** A control-flow node that wraps a `CasePattern`. */

ruby/ql/lib/codeql/ruby/controlflow/internal/Completion.qll

@@ -233,8 +233,12 @@ private predicate inMatchingContext(AstNode n) {
   or
   exists(CaseExpr c, WhenClause w |
     exists(c.getValue()) and
-    c.getABranch() = w and
-    w.getPattern(_) = n
+    (
+      c.getABranch() = w and
+      w.getPattern(_) = n
+      or
+      w = n
+    )
   )
   or
   n instanceof CasePattern

ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImpl.qll

@@ -400,7 +400,7 @@ module Trees {
       c instanceof SimpleCompletion
       or
       exists(int i, WhenTree branch | branch = this.getBranch(i) |
-        last(branch.getLastPattern(), pred, c) and
+        pred = branch and
         first(this.getBranch(i + 1), succ) and
         c.(ConditionalCompletion).getValue() = false
       )
@@ -1397,7 +1397,7 @@ module Trees {
     final override ControlFlowTree getChildElement(int i) { result = this.getMethodName(i) }
   }
 
-  private class WhenTree extends PreOrderTree, WhenClause {
+  private class WhenTree extends ControlFlowTree, WhenClause {
     final override predicate propagatesAbnormal(AstNode child) { child = this.getAPattern() }
 
     final Expr getLastPattern() {
@@ -1407,17 +1407,21 @@ module Trees {
       )
     }
 
+    final override predicate first(AstNode first) { first(this.getPattern(0), first) }
+
     final override predicate last(AstNode last, Completion c) {
-      last(this.getLastPattern(), last, c) and
+      last = this and
       c.(ConditionalCompletion).getValue() = false
       or
-      last(this.getBody(), last, c)
+      last(this.getBody(), last, c) and
+      c instanceof NormalCompletion
     }
 
     final override predicate succ(AstNode pred, AstNode succ, Completion c) {
       pred = this and
-      first(this.getPattern(0), succ) and
-      c instanceof SimpleCompletion
+      c.isValidFor(this) and
+      c.(ConditionalCompletion).getValue() = true and
+      first(this.getBody(), succ)
       or
       exists(int i, Expr p, boolean b |
         p = this.getPattern(i) and
@@ -1425,10 +1429,13 @@ module Trees {
         b = c.(ConditionalCompletion).getValue()
       |
         b = true and
-        first(this.getBody(), succ)
+        succ = this
         or
         b = false and
         first(this.getPattern(i + 1), succ)
+        or
+        not exists(this.getPattern(i + 1)) and
+        succ = this
       )
     }
   }

ruby/ql/lib/codeql/ruby/controlflow/internal/Splitting.qll

@@ -85,7 +85,14 @@ private module ConditionalCompletionSplitting {
         or
         last(succ.(ConditionalExpr).getBranch(_), pred, c) and
         completion = c
+        or
+        last(succ.(WhenClause).getAPattern(), pred, c) and completion = c
       )
+      or
+      succ(pred, succ, c) and
+      succ(succ, _, completion) and
+      succ instanceof WhenClause and
+      completion = c
     }
 
     override predicate hasEntryScope(CfgScope scope, AstNode succ) { none() }

ruby/ql/lib/codeql/ruby/dataflow/BarrierGuards.qll

@@ -186,8 +186,8 @@ private predicate stringConstCaseCompare(
     case.getValue() = testedNode and
     (
       exists(CfgNodes::ExprNodes::WhenClauseCfgNode branchNode |
+        guard = branchNode and
         branchNode = case.getBranch(_) and
-        guard = branchNode.getPattern(_) and
         // For simplicity, consider patterns that contain only string literals or arrays of string literals
         forall(ExprCfgNode pattern | pattern = branchNode.getPattern(_) |
           // when "foo"

ruby/ql/test/library-tests/controlflow/graph/Cfg.expected

@@ -526,17 +526,20 @@ case.rb:
 #-----|  -> exit if_in_case (normal)
 
 #    2| call to x1
-#-----|  -> when ...
+#-----|  -> 1
 
 #    2| self
 #-----|  -> call to x1
 
-#    3| when ...
-#-----|  -> 1
+#    3| [match] when ...
+#-----| match -> self
+
+#    3| [no-match] when ...
+#-----| no-match -> 2
 
 #    3| 1
-#-----| match -> self
-#-----| no-match -> when ...
+#-----| match -> [match] when ...
+#-----| no-match -> [no-match] when ...
 
 #    3| then ...
 #-----|  -> case ...
@@ -569,12 +572,15 @@ case.rb:
 #    3| x2
 #-----|  -> "x2"
 
-#    4| when ...
-#-----|  -> 2
+#    4| [match] when ...
+#-----| match -> self
+
+#    4| [no-match] when ...
+#-----| no-match -> case ...
 
 #    4| 2
-#-----| no-match -> case ...
-#-----| match -> self
+#-----| match -> [match] when ...
+#-----| no-match -> [no-match] when ...
 
 #    4| then ...
 #-----|  -> case ...
@@ -1826,17 +1832,20 @@ cfg.rb:
 #-----|  -> call to puts
 
 #   41| case ...
-#-----|  -> when ...
+#-----|  -> b
 
 #   41| 10
-#-----|  -> when ...
-
-#   42| when ...
 #-----|  -> 1
 
-#   42| 1
+#   42| [match] when ...
 #-----| match -> self
-#-----| no-match -> when ...
+
+#   42| [no-match] when ...
+#-----| no-match -> 2
+
+#   42| 1
+#-----| match -> [match] when ...
+#-----| no-match -> [no-match] when ...
 
 #   42| then ...
 #-----|  -> case ...
@@ -1853,20 +1862,23 @@ cfg.rb:
 #   42| one
 #-----|  -> "one"
 
-#   43| when ...
-#-----|  -> 2
+#   43| [match] when ...
+#-----| match -> self
+
+#   43| [no-match] when ...
+#-----| no-match -> self
 
 #   43| 2
+#-----| match -> [match] when ...
 #-----| no-match -> 3
-#-----| match -> self
 
 #   43| 3
+#-----| match -> [match] when ...
 #-----| no-match -> 4
-#-----| match -> self
 
 #   43| 4
-#-----| match -> self
-#-----| no-match -> self
+#-----| match -> [match] when ...
+#-----| no-match -> [no-match] when ...
 
 #   43| then ...
 #-----|  -> case ...
@@ -1901,15 +1913,19 @@ cfg.rb:
 #   47| case ...
 #-----|  -> chained
 
+#   48| [false] when ...
+#-----| false -> b
+
 #   48| when ...
-#-----|  -> b
+#-----| match -> self
+#-----| no-match -> b
 
 #   48| b
 #-----|  -> 1
 
 #   48| ... == ...
-#-----| true -> self
-#-----| false -> when ...
+#-----| false -> [false] when ...
+#-----| true -> when ...
 
 #   48| 1
 #-----|  -> ... == ...
@@ -1929,15 +1945,19 @@ cfg.rb:
 #   48| one
 #-----|  -> "one"
 
+#   49| [false] when ...
+#-----| false -> case ...
+
 #   49| when ...
-#-----|  -> b
+#-----| no-match -> case ...
+#-----| match -> self
 
 #   49| b
 #-----|  -> 0
 
 #   49| ... == ...
+#-----| true -> when ...
 #-----| false -> b
-#-----| true -> self
 
 #   49| 0
 #-----|  -> ... == ...
@@ -1946,8 +1966,8 @@ cfg.rb:
 #-----|  -> 1
 
 #   49| ... > ...
-#-----| false -> case ...
-#-----| true -> self
+#-----| false -> [false] when ...
+#-----| true -> when ...
 
 #   49| 1
 #-----|  -> ... > ...

ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.rb

@@ -137,9 +137,9 @@ end
 
 case foo
 when "foo", "bar"
-    foo # $ MISSING: guarded
+    foo # $ guarded
 when "baz", "quux"
-    foo # $ MISSING: guarded
+    foo # $ guarded
 else
     foo
 end