Ruby: CFG: make WhenExpr post-order

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

@@ -388,22 +388,23 @@ module Trees {
       first(this.getBranch(0), succ) and
       c instanceof SimpleCompletion
       or
-      exists(int i, WhenTree branch | branch = this.getBranch(i) |
+      exists(int i, WhenTree branch |
+        branch = this.getBranch(i) and
         last(branch.getLastPattern(), pred, c) and
-        first(this.getBranch(i + 1), succ) and
         c.(ConditionalCompletion).getValue() = false
+      |
+        first(this.getBranch(i + 1), succ)
+        or
+        not exists(this.getBranch(i + 1)) and succ = this
       )
       or
       succ = this and
       (
         last(this.getValue(), pred, c) and not exists(this.getABranch())
         or
-        last(this.getABranch().(WhenExpr).getBody(), pred, c)
-        or
-        exists(int i, ControlFlowTree lastBranch |
-          lastBranch = this.getBranch(i) and
-          not exists(this.getBranch(i + 1)) and
-          last(lastBranch, pred, c)
+        exists(int i, ControlFlowTree branch |
+          branch = this.getBranch(i) and
+          last(branch, pred, c)
         )
       )
     }
@@ -1377,8 +1378,13 @@ module Trees {
     final override ControlFlowTree getChildElement(int i) { result = this.getMethodName(i) }
   }
 
-  private class WhenTree extends PreOrderTree, WhenExpr {
-    final override predicate propagatesAbnormal(AstNode child) { child = this.getAPattern() }
+  private class WhenTree extends PostOrderTree, WhenExpr {
+    final override predicate propagatesAbnormal(AstNode child) {
+      child = this.getAPattern() or
+      child = this.getBody()
+    }
+
+    final override predicate first(AstNode first) { first(this.getPattern(0), first) }
 
     final Expr getLastPattern() {
       exists(int i |
@@ -1387,18 +1393,7 @@ module Trees {
       )
     }
 
-    final override predicate last(AstNode last, Completion c) {
-      last(this.getLastPattern(), last, c) and
-      c.(ConditionalCompletion).getValue() = false
-      or
-      last(this.getBody(), last, c)
-    }
-
     final override predicate succ(AstNode pred, AstNode succ, Completion c) {
-      pred = this and
-      first(this.getPattern(0), succ) and
-      c instanceof SimpleCompletion
-      or
       exists(int i, Expr p, boolean b |
         p = this.getPattern(i) and
         last(p, pred, c) and
@@ -1410,6 +1405,15 @@ module Trees {
         b = false and
         first(this.getPattern(i + 1), succ)
       )
+      or
+      last(this.getBody(), pred, c) and
+      succ = this and
+      c instanceof NormalCompletion
+      or
+      not exists(this.getBody()) and
+      last(this.getLastPattern(), pred, c) and
+      succ = this and
+      c.(ConditionalCompletion).getValue() = true
     }
   }
 }

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

@@ -508,20 +508,20 @@ case.rb:
 #-----|  -> exit if_in_case (normal)
 
 #    2| call to x1
-#-----|  -> when ...
+#-----|  -> 1
 
 #    2| self
 #-----|  -> call to x1
 
 #    3| when ...
-#-----|  -> 1
+#-----|  -> case ...
 
 #    3| 1
-#-----| no-match -> when ...
+#-----| no-match -> 2
 #-----| match -> self
 
 #    3| then ...
-#-----|  -> case ...
+#-----|  -> when ...
 
 #    3| ( ... )
 #-----|  -> then ...
@@ -549,14 +549,14 @@ case.rb:
 #-----|  -> call to puts
 
 #    4| when ...
-#-----|  -> 2
+#-----|  -> case ...
 
 #    4| 2
 #-----| no-match -> case ...
 #-----| match -> self
 
 #    4| then ...
-#-----|  -> case ...
+#-----|  -> when ...
 
 #    4| call to puts
 #-----|  -> then ...
@@ -1712,20 +1712,20 @@ cfg.rb:
 #-----|  -> call to puts
 
 #   41| case ...
-#-----|  -> when ...
+#-----|  -> b
 
 #   41| 10
-#-----|  -> when ...
-
-#   42| when ...
 #-----|  -> 1
 
+#   42| when ...
+#-----|  -> case ...
+
 #   42| 1
-#-----| no-match -> when ...
+#-----| no-match -> 2
 #-----| match -> self
 
 #   42| then ...
-#-----|  -> case ...
+#-----|  -> when ...
 
 #   42| call to puts
 #-----|  -> then ...
@@ -1737,7 +1737,7 @@ cfg.rb:
 #-----|  -> call to puts
 
 #   43| when ...
-#-----|  -> 2
+#-----|  -> case ...
 
 #   43| 2
 #-----| no-match -> 3
@@ -1752,7 +1752,7 @@ cfg.rb:
 #-----| no-match -> self
 
 #   43| then ...
-#-----|  -> case ...
+#-----|  -> when ...
 
 #   43| call to puts
 #-----|  -> then ...
@@ -1779,10 +1779,10 @@ cfg.rb:
 #-----|  -> chained
 
 #   48| when ...
-#-----|  -> b
+#-----|  -> case ...
 
 #   48| ... == ...
-#-----| false -> when ...
+#-----| false -> b
 #-----| true -> self
 
 #   48| b
@@ -1792,7 +1792,7 @@ cfg.rb:
 #-----|  -> ... == ...
 
 #   48| then ...
-#-----|  -> case ...
+#-----|  -> when ...
 
 #   48| call to puts
 #-----|  -> then ...
@@ -1804,7 +1804,7 @@ cfg.rb:
 #-----|  -> call to puts
 
 #   49| when ...
-#-----|  -> b
+#-----|  -> case ...
 
 #   49| ... == ...
 #-----| false -> b
@@ -1827,7 +1827,7 @@ cfg.rb:
 #-----|  -> ... > ...
 
 #   49| then ...
-#-----|  -> case ...
+#-----|  -> when ...
 
 #   49| call to puts
 #-----|  -> then ...