From 26c251f08060c6ac8eb8b3f7dc8cd954f6b88331 Mon Sep 17 00:00:00 2001
From: Nick Rolfe <nickrolfe@github.com>
Date: Wed, 17 Mar 2021 19:05:41 +0000
Subject: [PATCH 01/20] Order CFG nodes by column as well

---
 .../codeql_ruby/controlflow/internal/Cfg.ql   |   2 +-
 .../controlflow/graph/Cfg.expected            | 326 +++++++++---------
 2 files changed, 164 insertions(+), 164 deletions(-)

diff --git a/ql/src/codeql_ruby/controlflow/internal/Cfg.ql b/ql/src/codeql_ruby/controlflow/internal/Cfg.ql
index ef06ba17a97..ded6b103222 100644
--- a/ql/src/codeql_ruby/controlflow/internal/Cfg.ql
+++ b/ql/src/codeql_ruby/controlflow/internal/Cfg.ql
@@ -14,7 +14,7 @@ query predicate nodes(CfgNode n, string attr, string val) {
           p
           order by
             p.getLocation().getFile().getBaseName(), p.getLocation().getFile().getAbsolutePath(),
-            p.getLocation().getStartLine()
+            p.getLocation().getStartLine(), p.getLocation().getStartColumn()
         )
     ).toString()
 }
diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected
index f106b06d2d8..028f0d08b9f 100644
--- a/ql/test/library-tests/controlflow/graph/Cfg.expected
+++ b/ql/test/library-tests/controlflow/graph/Cfg.expected
@@ -8,9 +8,6 @@ break_ensure.rb:
 #    1| m1
 #-----|  -> m2
 
-#    1| elements
-#-----|  -> elements
-
 #    1| exit m1
 
 #    1| exit break_ensure.rb
@@ -21,6 +18,9 @@ break_ensure.rb:
 #    1| exit break_ensure.rb (normal)
 #-----|  -> exit break_ensure.rb
 
+#    1| elements
+#-----|  -> elements
+
 #    2| for ... in ...
 #-----|  -> ensure ...
 
@@ -75,14 +75,14 @@ break_ensure.rb:
 #   13| m2
 #-----|  -> m3
 
-#   13| elements
-#-----|  -> elements
-
 #   13| exit m2
 
 #   13| exit m2 (normal)
 #-----|  -> exit m2
 
+#   13| elements
+#-----|  -> elements
+
 #   14| for ... in ...
 #-----|  -> exit m2 (normal)
 
@@ -156,14 +156,14 @@ break_ensure.rb:
 #   27| m3
 #-----|  -> m4
 
-#   27| elements
-#-----|  -> elements
-
 #   27| exit m3
 
 #   27| exit m3 (normal)
 #-----|  -> exit m3
 
+#   27| elements
+#-----|  -> elements
+
 #   29| if ...
 #-----|  -> ensure ...
 
@@ -253,14 +253,14 @@ break_ensure.rb:
 #   44| m4
 #-----|  -> exit break_ensure.rb (normal)
 
-#   44| elements
-#-----|  -> elements
-
 #   44| exit m4
 
 #   44| exit m4 (normal)
 #-----|  -> exit m4
 
+#   44| elements
+#-----|  -> elements
+
 #   45| for ... in ...
 #-----|  -> exit m4 (normal)
 
@@ -508,18 +508,23 @@ cfg.rb:
 #   23| 1
 #-----|  -> ... + ...
 
-#   25| enter { ... }
-#-----|  -> x
-
 #   25| call to times
 #-----|  -> :puts
 
 #   25| 2
 #-----|  -> { ... }
 
+#   25| enter { ... }
+#-----|  -> x
+
 #   25| { ... }
 #-----|  -> call to times
 
+#   25| exit { ... }
+
+#   25| exit { ... } (normal)
+#-----|  -> exit { ... }
+
 #   25| x
 #-----|  -> x
 
@@ -529,11 +534,6 @@ cfg.rb:
 #   25| x
 #-----|  -> call to puts
 
-#   25| exit { ... }
-
-#   25| exit { ... } (normal)
-#-----|  -> exit { ... }
-
 #   27| call to puts
 #-----|  -> Proc
 
@@ -543,18 +543,23 @@ cfg.rb:
 #   27| :puts
 #-----|  -> &...
 
-#   29| enter { ... }
-#-----|  -> x
-
 #   29| call to new
 #-----|  -> true
 
 #   29| Proc
 #-----|  -> { ... }
 
+#   29| enter { ... }
+#-----|  -> x
+
 #   29| { ... }
 #-----|  -> call to new
 
+#   29| exit { ... }
+
+#   29| exit { ... } (normal)
+#-----|  -> exit { ... }
+
 #   29| x
 #-----|  -> x
 
@@ -564,11 +569,6 @@ cfg.rb:
 #   29| x
 #-----|  -> call to call
 
-#   29| exit { ... }
-
-#   29| exit { ... } (normal)
-#-----|  -> exit { ... }
-
 #   31| while ...
 #-----|  -> false
 
@@ -811,6 +811,11 @@ cfg.rb:
 #   63| pattern
 #-----|  -> 1
 
+#   63| exit pattern
+
+#   63| exit pattern (normal)
+#-----|  -> exit pattern
+
 #   63| (..., ...)
 #-----|  -> a
 
@@ -820,11 +825,6 @@ cfg.rb:
 #   63| b
 #-----|  -> (..., ...)
 
-#   63| exit pattern
-
-#   63| exit pattern (normal)
-#-----|  -> exit pattern
-
 #   64| call to puts
 #-----|  -> b
 
@@ -1086,6 +1086,11 @@ cfg.rb:
 #  101| parameters
 #-----|  -> "healthy"
 
+#  101| exit parameters
+
+#  101| exit parameters (normal)
+#-----|  -> exit parameters
+
 #  101| value
 #-----| no-match -> 42
 #-----| match -> key
@@ -1099,11 +1104,6 @@ cfg.rb:
 #  101| kwargs
 #-----|  -> value
 
-#  101| exit parameters
-
-#  101| exit parameters (normal)
-#-----|  -> exit parameters
-
 #  102| call to puts
 #-----|  -> kwargs
 
@@ -1201,18 +1201,23 @@ cfg.rb:
 #  117| 10
 #-----|  -> @@static_field
 
-#  120| enter -> { ... }
-#-----|  -> x
-
 #  120| ... = ...
 #-----|  -> M
 
 #  120| swap
 #-----|  -> ... = ...
 
+#  120| enter -> { ... }
+#-----|  -> x
+
 #  120| -> { ... }
 #-----|  -> swap
 
+#  120| exit -> { ... }
+
+#  120| exit -> { ... } (normal)
+#-----|  -> exit -> { ... }
+
 #  120| (..., ...)
 #-----|  -> y
 
@@ -1231,11 +1236,6 @@ cfg.rb:
 #  120| x
 #-----|  -> [...]
 
-#  120| exit -> { ... }
-
-#  120| exit -> { ... } (normal)
-#-----|  -> exit -> { ... }
-
 #  122| M
 #-----|  -> nil
 
@@ -1461,17 +1461,17 @@ cfg.rb:
 #  149| method
 #-----|  -> two_parameters
 
+#  149| exit method
+
+#  149| exit method (normal)
+#-----|  -> exit method
+
 #  149| silly
 #-----|  -> method
 
 #  149| x
 #-----|  -> x
 
-#  149| exit method
-
-#  149| exit method (normal)
-#-----|  -> exit method
-
 #  150| call to puts
 #-----|  -> exit method (normal)
 
@@ -1484,17 +1484,17 @@ cfg.rb:
 #  153| two_parameters
 #-----|  -> 1
 
+#  153| exit two_parameters
+
+#  153| exit two_parameters (normal)
+#-----|  -> exit two_parameters
+
 #  153| a
 #-----|  -> b
 
 #  153| b
 #-----|  -> exit two_parameters (normal)
 
-#  153| exit two_parameters
-
-#  153| exit two_parameters (normal)
-#-----|  -> exit two_parameters
-
 #  155| call to two_parameters
 #-----|  -> call to __FILE__
 
@@ -1801,15 +1801,20 @@ cfg.rb:
 
 #  188| 42
 
-#  191| enter { ... }
-#-----|  -> x
-
 #  191| call to run_block
 #-----|  -> exit cfg.rb (normal)
 
+#  191| enter { ... }
+#-----|  -> x
+
 #  191| { ... }
 #-----|  -> call to run_block
 
+#  191| exit { ... }
+
+#  191| exit { ... } (normal)
+#-----|  -> exit { ... }
+
 #  191| x
 #-----|  -> x
 
@@ -1819,11 +1824,6 @@ cfg.rb:
 #  191| x
 #-----|  -> call to puts
 
-#  191| exit { ... }
-
-#  191| exit { ... } (normal)
-#-----|  -> exit { ... }
-
 exit.rb:
 #    1| enter m1
 #-----|  -> x
@@ -1834,9 +1834,6 @@ exit.rb:
 #    1| m1
 #-----|  -> m2
 
-#    1| x
-#-----|  -> x
-
 #    1| exit m1
 
 #    1| exit exit.rb
@@ -1850,6 +1847,9 @@ exit.rb:
 #    1| exit exit.rb (normal)
 #-----|  -> exit exit.rb
 
+#    1| x
+#-----|  -> x
+
 #    2| if ...
 #-----|  -> "x <= 2"
 
@@ -1881,9 +1881,6 @@ exit.rb:
 #    8| m2
 #-----|  -> exit exit.rb (normal)
 
-#    8| x
-#-----|  -> x
-
 #    8| exit m2
 
 #    8| exit m2 (abnormal)
@@ -1892,6 +1889,9 @@ exit.rb:
 #    8| exit m2 (normal)
 #-----|  -> exit m2
 
+#    8| x
+#-----|  -> x
+
 #    9| if ...
 #-----|  -> "x <= 2"
 
@@ -1956,9 +1956,6 @@ ifs.rb:
 #    1| m1
 #-----|  -> m2
 
-#    1| x
-#-----|  -> x
-
 #    1| exit m1
 
 #    1| exit ifs.rb
@@ -1969,6 +1966,9 @@ ifs.rb:
 #    1| exit ifs.rb (normal)
 #-----|  -> exit ifs.rb
 
+#    1| x
+#-----|  -> x
+
 #    2| if ...
 #-----|  -> exit m1 (normal)
 
@@ -2063,14 +2063,14 @@ ifs.rb:
 #   11| m2
 #-----|  -> m3
 
-#   11| b
-#-----|  -> b
-
 #   11| exit m2
 
 #   11| exit m2 (normal)
 #-----|  -> exit m2
 
+#   11| b
+#-----|  -> b
+
 #   12| if ...
 #-----|  -> 1
 
@@ -2096,14 +2096,14 @@ ifs.rb:
 #   18| m3
 #-----|  -> m4
 
-#   18| x
-#-----|  -> x
-
 #   18| exit m3
 
 #   18| exit m3 (normal)
 #-----|  -> exit m3
 
+#   18| x
+#-----|  -> x
+
 #   19| if ...
 #-----|  -> x
 
@@ -2169,6 +2169,11 @@ ifs.rb:
 #   28| m4
 #-----|  -> m5
 
+#   28| exit m4
+
+#   28| exit m4 (normal)
+#-----|  -> exit m4
+
 #   28| b1
 #-----|  -> b2
 
@@ -2178,11 +2183,6 @@ ifs.rb:
 #   28| b3
 #-----|  -> b1
 
-#   28| exit m4
-
-#   28| exit m4 (normal)
-#-----|  -> exit m4
-
 #   29| return
 #-----| return -> exit m4 (normal)
 
@@ -2225,6 +2225,11 @@ ifs.rb:
 #   32| m5
 #-----|  -> 1
 
+#   32| exit m5
+
+#   32| exit m5 (normal)
+#-----|  -> exit m5
+
 #   32| b1
 #-----|  -> b2
 
@@ -2240,11 +2245,6 @@ ifs.rb:
 #   32| b5
 #-----|  -> b1
 
-#   32| exit m5
-
-#   32| exit m5 (normal)
-#-----|  -> exit m5
-
 #   33| if ...
 #-----|  -> exit m5 (normal)
 
@@ -2352,9 +2352,6 @@ loops.rb:
 #    1| m1
 #-----|  -> m2
 
-#    1| x
-#-----|  -> x
-
 #    1| exit m1
 
 #    1| exit loops.rb
@@ -2365,6 +2362,9 @@ loops.rb:
 #    1| exit loops.rb (normal)
 #-----|  -> exit loops.rb
 
+#    1| x
+#-----|  -> x
+
 #    2| while ...
 #-----|  -> exit m1 (normal)
 
@@ -2399,14 +2399,14 @@ loops.rb:
 #    8| m2
 #-----|  -> m3
 
-#    8| x
-#-----|  -> x
-
 #    8| exit m2
 
 #    8| exit m2 (normal)
 #-----|  -> exit m2
 
+#    8| x
+#-----|  -> x
+
 #    9| while ...
 #-----|  -> "Done"
 
@@ -2506,9 +2506,6 @@ loops.rb:
 #   24| exit m3 (normal)
 #-----|  -> exit m3
 
-#   25| enter do ... end
-#-----|  -> x
-
 #   25| call to each
 #-----|  -> exit m3 (normal)
 
@@ -2524,17 +2521,20 @@ loops.rb:
 #   25| 3
 #-----|  -> [...]
 
+#   25| enter do ... end
+#-----|  -> x
+
 #   25| do ... end
 #-----|  -> call to each
 
-#   25| x
-#-----|  -> x
-
 #   25| exit do ... end
 
 #   25| exit do ... end (normal)
 #-----|  -> exit do ... end
 
+#   25| x
+#-----|  -> x
+
 #   26| call to puts
 #-----|  -> exit do ... end (normal)
 
@@ -2548,14 +2548,14 @@ raise.rb:
 #    1| ExceptionA
 #-----|  -> Exception
 
-#    1| Exception
-#-----|  -> ExceptionB
-
 #    1| exit raise.rb
 
 #    1| exit raise.rb (normal)
 #-----|  -> exit raise.rb
 
+#    1| Exception
+#-----|  -> ExceptionB
+
 #    4| ExceptionB
 #-----|  -> Exception
 
@@ -2568,9 +2568,6 @@ raise.rb:
 #    7| m1
 #-----|  -> m2
 
-#    7| x
-#-----|  -> x
-
 #    7| exit m1
 
 #    7| exit m1 (abnormal)
@@ -2579,6 +2576,9 @@ raise.rb:
 #    7| exit m1 (normal)
 #-----|  -> exit m1
 
+#    7| x
+#-----|  -> x
+
 #    8| if ...
 #-----|  -> "x <= 2"
 
@@ -2610,9 +2610,6 @@ raise.rb:
 #   14| m2
 #-----|  -> m3
 
-#   14| b
-#-----|  -> b
-
 #   14| exit m2
 
 #   14| exit m2 (abnormal)
@@ -2621,6 +2618,9 @@ raise.rb:
 #   14| exit m2 (normal)
 #-----|  -> exit m2
 
+#   14| b
+#-----|  -> b
+
 #   16| if ...
 #-----|  -> "End m2"
 
@@ -2659,14 +2659,14 @@ raise.rb:
 #   25| m3
 #-----|  -> m4
 
-#   25| b
-#-----|  -> b
-
 #   25| exit m3
 
 #   25| exit m3 (normal)
 #-----|  -> exit m3
 
+#   25| b
+#-----|  -> b
+
 #   27| if ...
 #-----|  -> "End m3"
 
@@ -2701,14 +2701,14 @@ raise.rb:
 #   36| m4
 #-----|  -> m5
 
-#   36| b
-#-----|  -> b
-
 #   36| exit m4
 
 #   36| exit m4 (normal)
 #-----|  -> exit m4
 
+#   36| b
+#-----|  -> b
+
 #   38| if ...
 #-----|  -> "End m4"
 
@@ -2746,14 +2746,14 @@ raise.rb:
 #   47| m5
 #-----|  -> m6
 
-#   47| b
-#-----|  -> b
-
 #   47| exit m5
 
 #   47| exit m5 (normal)
 #-----|  -> exit m5
 
+#   47| b
+#-----|  -> b
+
 #   49| if ...
 #-----|  -> "End m5"
 
@@ -2785,9 +2785,6 @@ raise.rb:
 #   57| m6
 #-----|  -> m7
 
-#   57| b
-#-----|  -> b
-
 #   57| exit m6
 
 #   57| exit m6 (abnormal)
@@ -2796,6 +2793,9 @@ raise.rb:
 #   57| exit m6 (normal)
 #-----|  -> exit m6
 
+#   57| b
+#-----|  -> b
+
 #   59| if ...
 #-----|  -> "End m6"
 
@@ -2841,9 +2841,6 @@ raise.rb:
 #   68| m7
 #-----|  -> m8
 
-#   68| x
-#-----|  -> x
-
 #   68| exit m7
 
 #   68| exit m7 (abnormal)
@@ -2852,6 +2849,9 @@ raise.rb:
 #   68| exit m7 (normal)
 #-----|  -> exit m7
 
+#   68| x
+#-----|  -> x
+
 #   69| if ...
 #-----|  -> "0 <= x <= 2"
 
@@ -2929,9 +2929,6 @@ raise.rb:
 #   79| m8
 #-----|  -> m9
 
-#   79| x
-#-----|  -> "Begin m8"
-
 #   79| exit m8
 
 #   79| exit m8 (abnormal)
@@ -2940,6 +2937,9 @@ raise.rb:
 #   79| exit m8 (normal)
 #-----|  -> exit m8
 
+#   79| x
+#-----|  -> "Begin m8"
+
 #   80| call to puts
 #-----|  -> x
 
@@ -3029,6 +3029,14 @@ raise.rb:
 #   94| m9
 #-----|  -> m10
 
+#   94| exit m9
+
+#   94| exit m9 (abnormal)
+#-----|  -> exit m9
+
+#   94| exit m9 (normal)
+#-----|  -> exit m9
+
 #   94| x
 #-----|  -> b1
 
@@ -3038,14 +3046,6 @@ raise.rb:
 #   94| b2
 #-----|  -> "Begin m9"
 
-#   94| exit m9
-
-#   94| exit m9 (abnormal)
-#-----|  -> exit m9
-
-#   94| exit m9 (normal)
-#-----|  -> exit m9
-
 #   95| call to puts
 #-----|  -> x
 
@@ -3294,6 +3294,14 @@ raise.rb:
 #  121| m10
 #-----|  -> m11
 
+#  121| exit m10
+
+#  121| exit m10 (abnormal)
+#-----|  -> exit m10
+
+#  121| exit m10 (normal)
+#-----|  -> exit m10
+
 #  121| p
 #-----| no-match -> "Exception"
 #-----| match -> ensure ...
@@ -3304,14 +3312,6 @@ raise.rb:
 #  121| "Exception"
 #-----|  -> call to raise
 
-#  121| exit m10
-
-#  121| exit m10 (abnormal)
-#-----|  -> exit m10
-
-#  121| exit m10 (normal)
-#-----|  -> exit m10
-
 #  124| ensure ...
 #-----|  -> "Will not get executed if p is..."
 
@@ -3327,9 +3327,6 @@ raise.rb:
 #  128| m11
 #-----|  -> m12
 
-#  128| b
-#-----|  -> b
-
 #  128| exit m11
 
 #  128| exit m11 (abnormal)
@@ -3338,6 +3335,9 @@ raise.rb:
 #  128| exit m11 (normal)
 #-----|  -> exit m11
 
+#  128| b
+#-----|  -> b
+
 #  130| if ...
 #-----|  -> ensure ...
 
@@ -3401,14 +3401,14 @@ raise.rb:
 #  142| m12
 #-----|  -> m13
 
-#  142| b
-#-----|  -> b
-
 #  142| exit m12
 
 #  142| exit m12 (normal)
 #-----|  -> exit m12
 
+#  142| b
+#-----|  -> b
+
 #  143| if ...
 #-----|  -> ensure ...
 
@@ -3460,16 +3460,13 @@ raise.rb:
 #  154| m14
 #-----|  -> exit raise.rb (normal)
 
-#  154| element
-#-----|  -> element
-
 #  154| exit m14
 
 #  154| exit m14 (normal)
 #-----|  -> exit m14
 
-#  155| enter { ... }
-#-----|  -> elem
+#  154| element
+#-----|  -> element
 
 #  155| call to each
 #-----|  -> exit m14 (normal)
@@ -3477,9 +3474,20 @@ raise.rb:
 #  155| element
 #-----|  -> { ... }
 
+#  155| enter { ... }
+#-----|  -> elem
+
 #  155| { ... }
 #-----|  -> call to each
 
+#  155| exit { ... }
+
+#  155| exit { ... } (abnormal)
+#-----|  -> exit { ... }
+
+#  155| exit { ... } (normal)
+#-----|  -> exit { ... }
+
 #  155| elem
 #-----|  -> element
 
@@ -3498,11 +3506,3 @@ raise.rb:
 
 #  155| element
 #-----|  -> call to nil?
-
-#  155| exit { ... }
-
-#  155| exit { ... } (abnormal)
-#-----|  -> exit { ... }
-
-#  155| exit { ... } (normal)
-#-----|  -> exit { ... }

From 32e2b257bff417a82e27654d11a2982f94253d8b Mon Sep 17 00:00:00 2001
From: Nick Rolfe <nickrolfe@github.com>
Date: Mon, 15 Mar 2021 18:43:41 +0000
Subject: [PATCH 02/20] Port CFG implementation to public AST interface

---
 ql/src/codeql_ruby/ast/Control.qll            |   27 +
 ql/src/codeql_ruby/ast/Method.qll             |    2 +-
 ql/src/codeql_ruby/ast/Module.qll             |    3 +-
 ql/src/codeql_ruby/ast/Statement.qll          |    2 +-
 ql/src/codeql_ruby/controlflow/CfgNodes.qll   |    4 +-
 .../controlflow/ControlFlowGraph.qll          |   13 +-
 .../controlflow/internal/AstNodes.qll         |  149 --
 .../controlflow/internal/Completion.qll       |    4 +-
 .../internal/ControlFlowGraphImpl.qll         | 1669 +++++++++--------
 .../controlflow/internal/Splitting.qll        |   58 +-
 .../codeql_ruby/dataflow/internal/SsaImpl.qll |    2 +-
 ql/test/library-tests/ast/calls/calls.ql      |    1 -
 .../ast/modules/toplevel.expected             |    2 +-
 .../controlflow/graph/Cfg.expected            |  205 +-
 14 files changed, 1052 insertions(+), 1089 deletions(-)
 delete mode 100644 ql/src/codeql_ruby/controlflow/internal/AstNodes.qll

diff --git a/ql/src/codeql_ruby/ast/Control.qll b/ql/src/codeql_ruby/ast/Control.qll
index 69588659002..de63484c38d 100644
--- a/ql/src/codeql_ruby/ast/Control.qll
+++ b/ql/src/codeql_ruby/ast/Control.qll
@@ -440,6 +440,9 @@ class ConditionalLoop extends Loop, TConditionalLoop {
     or
     pred = "getCondition" and result = this.getCondition()
   }
+
+  /** Holds if the loop body is entered when the condition is `condValue`. */
+  predicate entersLoopWhenConditionIs(boolean condValue) { none() }
 }
 
 /**
@@ -463,6 +466,12 @@ class WhileExpr extends ConditionalLoop, TWhileExpr {
 
   final override Expr getCondition() { toGenerated(result) = g.getCondition() }
 
+  /**
+   * Holds if the loop body is entered when the condition is `condValue`. For
+   * `while` loops, this holds when `condValue` is true.
+   */
+  final override predicate entersLoopWhenConditionIs(boolean condValue) { condValue = true }
+
   final override string toString() { result = "while ..." }
 }
 
@@ -487,6 +496,12 @@ class UntilExpr extends ConditionalLoop, TUntilExpr {
 
   final override Expr getCondition() { toGenerated(result) = g.getCondition() }
 
+  /**
+   * Holds if the loop body is entered when the condition is `condValue`. For
+   * `until` loops, this holds when `condValue` is false.
+   */
+  final override predicate entersLoopWhenConditionIs(boolean condValue) { condValue = false }
+
   final override string toString() { result = "until ..." }
 }
 
@@ -505,6 +520,12 @@ class WhileModifierExpr extends ConditionalLoop, TWhileModifierExpr {
 
   final override Expr getCondition() { toGenerated(result) = g.getCondition() }
 
+  /**
+   * Holds if the loop body is entered when the condition is `condValue`. For
+   * `while`-modifier loops, this holds when `condValue` is true.
+   */
+  final override predicate entersLoopWhenConditionIs(boolean condValue) { condValue = true }
+
   final override string getAPrimaryQlClass() { result = "WhileModifierExpr" }
 
   final override string toString() { result = "... while ..." }
@@ -525,6 +546,12 @@ class UntilModifierExpr extends ConditionalLoop, TUntilModifierExpr {
 
   final override Expr getCondition() { toGenerated(result) = g.getCondition() }
 
+  /**
+   * Holds if the loop body is entered when the condition is `condValue`. For
+   * `until`-modifier loops, this holds when `condValue` is false.
+   */
+  final override predicate entersLoopWhenConditionIs(boolean condValue) { condValue = false }
+
   final override string getAPrimaryQlClass() { result = "UntilModifierExpr" }
 
   final override string toString() { result = "... until ..." }
diff --git a/ql/src/codeql_ruby/ast/Method.qll b/ql/src/codeql_ruby/ast/Method.qll
index 3836bee596b..7c205f174ab 100644
--- a/ql/src/codeql_ruby/ast/Method.qll
+++ b/ql/src/codeql_ruby/ast/Method.qll
@@ -4,7 +4,7 @@ private import internal.AST
 private import internal.TreeSitter
 
 /** A callable. */
-class Callable extends Expr, CfgScope, TCallable {
+class Callable extends Expr, TCallable {
   /** Gets the number of parameters of this callable. */
   final int getNumberOfParameters() { result = count(this.getAParameter()) }
 
diff --git a/ql/src/codeql_ruby/ast/Module.qll b/ql/src/codeql_ruby/ast/Module.qll
index 568a5d64cac..0eff778dbb8 100644
--- a/ql/src/codeql_ruby/ast/Module.qll
+++ b/ql/src/codeql_ruby/ast/Module.qll
@@ -47,7 +47,8 @@ class Toplevel extends ModuleBase, TToplevel {
    * Gets the `n`th `BEGIN` block.
    */
   final BeginBlock getBeginBlock(int n) {
-    toGenerated(result) = rank[n](int i, Generated::BeginBlock b | b = g.getChild(i) | b order by i)
+    toGenerated(result) =
+      rank[n + 1](int i, Generated::BeginBlock b | b = g.getChild(i) | b order by i)
   }
 
   /**
diff --git a/ql/src/codeql_ruby/ast/Statement.qll b/ql/src/codeql_ruby/ast/Statement.qll
index fcc2ac8c09f..c75d4e07221 100644
--- a/ql/src/codeql_ruby/ast/Statement.qll
+++ b/ql/src/codeql_ruby/ast/Statement.qll
@@ -15,7 +15,7 @@ class Stmt extends AstNode, TStmt {
   CfgNodes::AstCfgNode getAControlFlowNode() { result.getNode() = this }
 
   /** Gets the control-flow scope of this statement, if any. */
-  CfgScope getCfgScope() { result = getCfgScope(toGenerated(this)) }
+  CfgScope getCfgScope() { result = getCfgScope(this) }
 
   /** Gets the enclosing callable, if any. */
   Callable getEnclosingCallable() { result = this.getCfgScope() }
diff --git a/ql/src/codeql_ruby/controlflow/CfgNodes.qll b/ql/src/codeql_ruby/controlflow/CfgNodes.qll
index e2a4badff91..d67849934fc 100644
--- a/ql/src/codeql_ruby/controlflow/CfgNodes.qll
+++ b/ql/src/codeql_ruby/controlflow/CfgNodes.qll
@@ -67,7 +67,7 @@ class AstCfgNode extends CfgNode, TAstCfgNode {
   private Splits splits;
   private AstNode n;
 
-  AstCfgNode() { this = TAstCfgNode(toGenerated(n), splits) }
+  AstCfgNode() { this = TAstCfgNode(n, splits) }
 
   final override AstNode getNode() { result = n }
 
@@ -132,7 +132,7 @@ abstract private class ExprChildMapping extends Expr {
 
   pragma[noinline]
   private BasicBlock getABasicBlockInScope() {
-    result.getANode() = TAstCfgNode(toGenerated(this.getAChildStar()), _)
+    result.getANode() = TAstCfgNode(this.getAChildStar(), _)
   }
 
   pragma[nomagic]
diff --git a/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll b/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll
index d1aca73b49f..30e75576397 100644
--- a/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll
+++ b/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll
@@ -1,8 +1,7 @@
 /** Provides classes representing the control flow graph. */
 
 private import codeql.Locations
-private import codeql_ruby.AST as AST
-private import codeql_ruby.ast.internal.AST as ASTInternal
+private import codeql_ruby.AST
 private import codeql_ruby.controlflow.BasicBlocks
 private import SuccessorTypes
 private import internal.ControlFlowGraphImpl
@@ -10,14 +9,14 @@ private import internal.Splitting
 private import internal.Completion
 
 /** An AST node with an associated control-flow graph. */
-class CfgScope extends AST::AstNode {
-  CfgScope() { ASTInternal::toGenerated(this) instanceof CfgScope::Range_ }
+class CfgScope extends AstNode {
+  CfgScope() { this instanceof CfgScope::Range_ }
 
   /** Gets the CFG scope that this scope is nested under, if any. */
   final CfgScope getOuterCfgScope() {
-    exists(AST::AstNode parent |
+    exists(AstNode parent |
       parent = this.getParent() and
-      result = getCfgScope(ASTInternal::toGenerated(parent))
+      result = getCfgScope(parent)
     )
   }
 }
@@ -35,7 +34,7 @@ class CfgNode extends TCfgNode {
   string toString() { none() }
 
   /** Gets the AST node that this node corresponds to, if any. */
-  AST::AstNode getNode() { none() }
+  AstNode getNode() { none() }
 
   /** Gets the location of this control flow node. */
   Location getLocation() { none() }
diff --git a/ql/src/codeql_ruby/controlflow/internal/AstNodes.qll b/ql/src/codeql_ruby/controlflow/internal/AstNodes.qll
deleted file mode 100644
index a8c5a710b41..00000000000
--- a/ql/src/codeql_ruby/controlflow/internal/AstNodes.qll
+++ /dev/null
@@ -1,149 +0,0 @@
-/**
- * Provides various helper classes for AST nodes. The definitions in this file
- * will likely be part of the hand-written user-facing AST layer.
- */
-
-private import codeql_ruby.ast.internal.TreeSitter::Generated
-private import codeql_ruby.controlflow.internal.Completion
-
-class LogicalNotAstNode extends Unary {
-  AstNode operand;
-
-  LogicalNotAstNode() {
-    this.getOperator().toString() in ["!", "not"] and
-    operand = this.getOperand()
-  }
-}
-
-class LogicalAndAstNode extends Binary {
-  AstNode left;
-  AstNode right;
-
-  LogicalAndAstNode() {
-    this.getOperator().toString() in ["&&", "and"] and
-    left = this.getLeft() and
-    right = this.getRight()
-  }
-
-  AstNode getAnOperand() { result in [left, right] }
-}
-
-class LogicalOrAstNode extends Binary {
-  AstNode left;
-  AstNode right;
-
-  LogicalOrAstNode() {
-    this.getOperator().toString() in ["||", "or"] and
-    left = this.getLeft() and
-    right = this.getRight()
-  }
-
-  AstNode getAnOperand() { result in [left, right] }
-}
-
-private class If_or_elisif =
-  @if or @elsif or @conditional or @if_modifier or @unless or @unless_modifier;
-
-class IfElsifAstNode extends AstNode, If_or_elisif {
-  AstNode getConditionNode() { none() }
-
-  AstNode getBranch(boolean b) { none() }
-}
-
-private class IfAstNode extends IfElsifAstNode, If {
-  override AstNode getConditionNode() { result = this.getCondition() }
-
-  override AstNode getBranch(boolean b) {
-    b = true and result = this.getConsequence()
-    or
-    b = false and result = this.getAlternative()
-  }
-}
-
-private class ElsifAstNode extends IfElsifAstNode, Elsif {
-  override AstNode getConditionNode() { result = this.getCondition() }
-
-  override AstNode getBranch(boolean b) {
-    b = true and result = this.getConsequence()
-    or
-    b = false and result = this.getAlternative()
-  }
-}
-
-private class ConditionalAstNode extends IfElsifAstNode, Conditional {
-  override AstNode getConditionNode() { result = this.getCondition() }
-
-  override AstNode getBranch(boolean b) {
-    b = true and result = this.getConsequence()
-    or
-    b = false and result = this.getAlternative()
-  }
-}
-
-private class IfModifierAstNode extends IfElsifAstNode, IfModifier {
-  override AstNode getConditionNode() { result = this.getCondition() }
-
-  override AstNode getBranch(boolean b) { b = true and result = this.getBody() }
-}
-
-private class UnlessAstNode extends IfElsifAstNode, Unless {
-  override AstNode getConditionNode() { result = this.getCondition() }
-
-  override AstNode getBranch(boolean b) {
-    b = false and result = this.getConsequence()
-    or
-    b = true and result = this.getAlternative()
-  }
-}
-
-private class UnlessModifierAstNode extends IfElsifAstNode, UnlessModifier {
-  override AstNode getConditionNode() { result = this.getCondition() }
-
-  override AstNode getBranch(boolean b) { b = false and result = this.getBody() }
-}
-
-private class CondLoop = @while or @while_modifier or @until or @until_modifier;
-
-class ConditionalLoopAstNode extends AstNode, CondLoop {
-  AstNode getConditionNode() { none() }
-
-  AstNode getBodyNode() { none() }
-
-  predicate continueLoop(BooleanCompletion c) { c instanceof TrueCompletion }
-
-  final predicate endLoop(BooleanCompletion c) { continueLoop(c.getDual()) }
-}
-
-private class WhileLoop extends ConditionalLoopAstNode, While {
-  override UnderscoreStatement getConditionNode() { result = this.getCondition() }
-
-  override Do getBodyNode() { result = this.getBody() }
-}
-
-private class WhileModifierLoop extends ConditionalLoopAstNode, WhileModifier {
-  override AstNode getConditionNode() { result = this.getCondition() }
-
-  override UnderscoreStatement getBodyNode() { result = this.getBody() }
-}
-
-private class UntilLoop extends ConditionalLoopAstNode, Until {
-  override UnderscoreStatement getConditionNode() { result = this.getCondition() }
-
-  override Do getBodyNode() { result = this.getBody() }
-
-  override predicate continueLoop(BooleanCompletion c) { c instanceof FalseCompletion }
-}
-
-private class UntilModifierLoop extends ConditionalLoopAstNode, UntilModifier {
-  override AstNode getConditionNode() { result = this.getCondition() }
-
-  override UnderscoreStatement getBodyNode() { result = this.getBody() }
-
-  override predicate continueLoop(BooleanCompletion c) { c instanceof FalseCompletion }
-}
-
-class ParenthesizedStatement extends ParenthesizedStatements {
-  ParenthesizedStatement() { strictcount(int i | exists(this.getChild(i))) = 1 }
-
-  AstNode getChild() { result = this.getChild(0) }
-}
diff --git a/ql/src/codeql_ruby/controlflow/internal/Completion.qll b/ql/src/codeql_ruby/controlflow/internal/Completion.qll
index ed17cb5f125..b25098b2a06 100644
--- a/ql/src/codeql_ruby/controlflow/internal/Completion.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/Completion.qll
@@ -49,7 +49,7 @@ private predicate nestedEnsureCompletion(Completion outer, int nestLevel) {
     or
     outer = TExitCompletion()
   ) and
-  nestLevel = any(Trees::RescueEnsureBlockTree t).nestLevel()
+  nestLevel = any(Trees::BodyStmtTree t).getNestLevel()
 }
 
 pragma[noinline]
@@ -206,7 +206,7 @@ private predicate inMatchingContext(AstNode n) {
     w.getPattern(_) = n
   )
   or
-  toGenerated(n).(Trees::DefaultValueParameterTree).hasDefaultValue()
+  n.(Trees::DefaultValueParameterTree).hasDefaultValue()
 }
 
 /**
diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
index 89e3a604173..c864d1aee40 100644
--- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
@@ -31,18 +31,16 @@
  * caught up by its surrounding loop and turned into a `NormalCompletion`.
  */
 
-private import codeql_ruby.AST as AST
+private import codeql_ruby.AST
 private import codeql_ruby.ast.internal.AST as ASTInternal
 private import codeql_ruby.ast.internal.Scope
 private import codeql_ruby.ast.Scope
-private import codeql_ruby.ast.internal.TreeSitter::Generated
-private import AstNodes
+private import codeql_ruby.ast.internal.TreeSitter
 private import codeql_ruby.ast.internal.Variable
 private import codeql_ruby.controlflow.ControlFlowGraph
 private import Completion
 private import SuccessorTypes
 private import Splitting
-private import codeql.files.FileSystem
 
 module CfgScope {
   abstract class Range_ extends AstNode {
@@ -51,22 +49,12 @@ module CfgScope {
     abstract predicate exit(AstNode last, Completion c);
   }
 
-  private class ProgramScope extends Range_, Program {
+  private class ProgramScope extends Range_, Toplevel {
     final override predicate entry(AstNode first) { first(this, first) }
 
     final override predicate exit(AstNode last, Completion c) { last(this, last, c) }
   }
 
-  private class BeginBlockScope extends Range_, BeginBlock {
-    final override predicate entry(AstNode first) {
-      first(this.(Trees::BeginBlockTree).getFirstChildNode(), first)
-    }
-
-    final override predicate exit(AstNode last, Completion c) {
-      last(this.(Trees::BeginBlockTree).getLastChildNode(), last, c)
-    }
-  }
-
   private class EndBlockScope extends Range_, EndBlock {
     final override predicate entry(AstNode first) {
       first(this.(Trees::EndBlockTree).getFirstChildNode(), first)
@@ -77,80 +65,50 @@ module CfgScope {
     }
   }
 
-  private class MethodScope extends Range_, AstNode {
-    MethodScope() { this instanceof Method }
-
-    final override predicate entry(AstNode first) {
-      this.(Trees::RescueEnsureBlockTree).firstInner(first)
-    }
+  private class MethodScope extends Range_, Method {
+    final override predicate entry(AstNode first) { this.(Trees::BodyStmtTree).firstInner(first) }
 
     final override predicate exit(AstNode last, Completion c) {
-      this.(Trees::RescueEnsureBlockTree).lastInner(last, c)
+      this.(Trees::BodyStmtTree).lastInner(last, c)
     }
   }
 
-  private class SingletonMethodScope extends Range_, AstNode {
-    SingletonMethodScope() { this instanceof SingletonMethod }
-
-    final override predicate entry(AstNode first) {
-      this.(Trees::RescueEnsureBlockTree).firstInner(first)
-    }
+  private class SingletonMethodScope extends Range_, SingletonMethod {
+    final override predicate entry(AstNode first) { this.(Trees::BodyStmtTree).firstInner(first) }
 
     final override predicate exit(AstNode last, Completion c) {
-      this.(Trees::RescueEnsureBlockTree).lastInner(last, c)
+      this.(Trees::BodyStmtTree).lastInner(last, c)
     }
   }
 
   private class DoBlockScope extends Range_, DoBlock {
-    DoBlockScope() { not this.getParent() instanceof Lambda }
-
-    final override predicate entry(AstNode first) {
-      this.(Trees::RescueEnsureBlockTree).firstInner(first)
-    }
+    final override predicate entry(AstNode first) { this.(Trees::BodyStmtTree).firstInner(first) }
 
     final override predicate exit(AstNode last, Completion c) {
-      this.(Trees::RescueEnsureBlockTree).lastInner(last, c)
+      this.(Trees::BodyStmtTree).lastInner(last, c)
     }
   }
 
-  private class BlockScope extends Range_, Block {
-    BlockScope() { not this.getParent() instanceof Lambda }
-
+  private class BraceBlockScope extends Range_, BraceBlock {
     final override predicate entry(AstNode first) {
-      first(this.(Trees::BlockTree).getFirstChildNode(), first)
+      first(this.(Trees::BraceBlockTree).getFirstChildNode(), first)
     }
 
     final override predicate exit(AstNode last, Completion c) {
-      last(this.(Trees::BlockTree).getLastChildNode(), last, c)
+      last(this.(Trees::BraceBlockTree).getLastChildNode(), last, c)
     }
   }
 
   private class LambdaScope extends Range_, Lambda {
-    final override predicate entry(AstNode first) {
-      first(this.getParameters(), first)
-      or
-      not exists(this.getParameters()) and
-      (
-        this.getBody().(Trees::DoBlockTree).firstInner(first)
-        or
-        first(this.getBody().(Trees::BlockTree).getFirstChildNode(), first)
-      )
-    }
+    final override predicate entry(AstNode first) { this.(Trees::BodyStmtTree).firstInner(first) }
 
     final override predicate exit(AstNode last, Completion c) {
-      last(this.getParameters(), last, c) and
-      not c instanceof NormalCompletion
-      or
-      last(this.getBody().(Trees::BlockTree).getLastChildNode(), last, c)
-      or
-      this.getBody().(Trees::RescueEnsureBlockTree).lastInner(last, c)
-      or
-      not exists(this.getBody()) and last(this.getParameters(), last, c)
+      this.(Trees::BodyStmtTree).lastInner(last, c)
     }
   }
 }
 
-abstract private class ControlFlowTree extends AstNode {
+abstract /*private*/ class ControlFlowTree extends AstNode {
   /**
    * Holds if `first` is the first element executed within this AST node.
    */
@@ -202,11 +160,7 @@ private predicate succImpl(AstNode pred, AstNode succ, Completion c) {
   any(ControlFlowTree cft).succ(pred, succ, c)
 }
 
-private predicate isHidden(ControlFlowTree t) {
-  not t = ASTInternal::toGenerated(_)
-  or
-  t.isHidden()
-}
+private predicate isHidden(ControlFlowTree t) { t.isHidden() }
 
 private predicate succImplIfHidden(AstNode pred, AstNode succ) {
   isHidden(pred) and
@@ -254,10 +208,7 @@ predicate succExit(CfgScope::Range_ scope, AstNode last, Completion c) {
  */
 abstract private class StandardNode extends ControlFlowTree {
   /** Gets the `i`th child node, in order of evaluation. */
-  ControlFlowTree getChildNode(int i) {
-    result = this.getAFieldOrChild() and
-    i = result.getParentIndex()
-  }
+  abstract ControlFlowTree getChildNode(int i);
 
   private AstNode getChildNodeRanked(int i) {
     result = rank[i + 1](AstNode child, int j | child = this.getChildNode(j) | child order by j)
@@ -291,17 +242,21 @@ abstract private class PreOrderTree extends ControlFlowTree {
 }
 
 // TODO: remove this class; it should be replaced with an implicit non AST node
-private class ForIn extends AST::AstNode, ASTInternal::TForIn {
+private class ForIn extends AstNode, ASTInternal::TForIn {
   final override string toString() { result = "In" }
 }
 
 // TODO: remove this class; it should be replaced with an implicit non AST node
-private class ForRange extends AST::ForExpr {
-  override AST::AstNode getAChild(string pred) {
-    result = AST::ForExpr.super.getAChild(pred)
+private class ForRange extends ForExpr {
+  override AstNode getAChild(string pred) {
+    result = ForExpr.super.getAChild(pred)
     or
     pred = "<in>" and
-    result = ASTInternal::TForIn(ASTInternal::toGenerated(this).(For).getValue())
+    result = this.getIn()
+  }
+
+  ForIn getIn() {
+    result = ASTInternal::TForIn(ASTInternal::toGenerated(this).(Generated::For).getValue())
   }
 }
 
@@ -309,7 +264,7 @@ private class ForRange extends AST::ForExpr {
 predicate isValidFor(Completion c, ControlFlowTree node) {
   c instanceof SimpleCompletion and isHidden(node)
   or
-  c.isValidFor(ASTInternal::fromGenerated(node))
+  c.isValidFor(node)
 }
 
 abstract private class StandardPreOrderTree extends StandardNode, PreOrderTree {
@@ -337,53 +292,6 @@ abstract private class PostOrderTree extends ControlFlowTree {
   }
 }
 
-private class LeftToRightPostOrderNodes =
-  @argument_list or @array or @bare_string or @bare_symbol or @binary or @block_argument or
-      @break or @call or @chained_string or @delimited_symbol or @destructured_left_assignment or
-      @destructured_parameter or @element_reference or @exception_variable or @hash or
-      @hash_splat_argument or @interpolation or @left_assignment_list or @next or
-      @operator_assignment or @pair or @parenthesized_statements or @range or @redo or @regex or
-      @rest_assignment or @retry or @return or @right_assignment_list or @scope_resolution or
-      @token_simple_symbol or @splat_argument or @string__ or @string_array or @subshell or
-      @superclass or @symbol_array or @token_hash_key_symbol or @unary or @splat_parameter or
-      @hash_splat_parameter or @block_parameter;
-
-private class LeftToRightPostOrderTree extends StandardPostOrderTree, LeftToRightPostOrderNodes {
-  LeftToRightPostOrderTree() {
-    not this instanceof LogicalNotAstNode and
-    not this instanceof LogicalAndAstNode and
-    not this instanceof LogicalOrAstNode
-  }
-
-  override predicate isHidden() {
-    this instanceof ArgumentList or
-    this instanceof ChainedString or
-    this instanceof ExceptionVariable or
-    this instanceof LeftAssignmentList or
-    this instanceof RightAssignmentList or
-    this instanceof SplatParameter or
-    this instanceof HashSplatParameter or
-    this instanceof BlockParameter
-  }
-}
-
-private class LeftToRightPreOrderNodes =
-  @alias or @block_parameters or @class or @do or @else or @ensure or @lambda_parameters or
-      @method_parameters or @pattern or @program or @then or @undef or @yield;
-
-private class LeftToRightPreOrderTree extends StandardPreOrderTree, LeftToRightPreOrderNodes {
-  override predicate isHidden() {
-    this instanceof BlockParameters or
-    this instanceof Do or
-    this instanceof Else or
-    this instanceof LambdaParameters or
-    this instanceof MethodParameters or
-    this instanceof Pattern or
-    this instanceof Program or
-    this instanceof Then
-  }
-}
-
 abstract private class StandardPostOrderTree extends StandardNode, PostOrderTree {
   final override predicate first(AstNode first) {
     first(this.getFirstChildNode(), first)
@@ -417,590 +325,142 @@ abstract class ScopeTree extends StandardNode, LeafTree {
 
 /** Defines the CFG by dispatch on the various AST types. */
 module Trees {
-  private class AssignmentTree extends StandardPostOrderTree, Assignment {
+  private class AliasStmtTree extends StandardPreOrderTree, AliasStmt {
     final override ControlFlowTree getChildNode(int i) {
-      result = this.getRight() and i = 0
+      result = this.getNewName() and i = 0
       or
-      result = this.getLeft() and i = 1
+      result = this.getOldName() and i = 1
     }
   }
 
-  private class BeginTree extends RescueEnsureBlockTree, PreOrderTree, Begin {
-    final override AstNode getChildNode(int i, boolean rescuable) {
-      result = this.getChild(i) and rescuable = true
-    }
+  private class ArgumentListTree extends StandardPostOrderTree, ArgumentList {
+    final override ControlFlowTree getChildNode(int i) { result = this.getElement(i) }
 
+    override predicate isHidden() { any() }
+  }
+
+  private class ArrayLiteralTree extends StandardPostOrderTree, ArrayLiteral {
+    final override ControlFlowTree getChildNode(int i) { result = this.getElement(i) }
+  }
+
+  private class AssignOperationTree extends StandardPostOrderTree, AssignOperation {
+    final override ControlFlowTree getChildNode(int i) {
+      result = this.getLeftOperand() and i = 0
+      or
+      result = this.getRightOperand() and i = 1
+    }
+  }
+
+  private class AssignmentTree extends StandardPostOrderTree, AssignExpr {
+    final override ControlFlowTree getChildNode(int i) {
+      result = this.getRightOperand() and i = 0
+      or
+      result = this.getLeftOperand() and i = 1
+    }
+  }
+
+  private class BeginTree extends BodyStmtTree, PreOrderTree, BeginExpr {
     final override predicate last(AstNode last, Completion c) { this.lastInner(last, c) }
 
     override predicate isHidden() { any() }
   }
 
-  class BeginBlockTree extends ScopeTree, BeginBlock {
-    final override ControlFlowTree getChildNode(int i) { result = this.getChild(i) }
-  }
+  private class BinaryOperationTree extends StandardPostOrderTree, BinaryOperation {
+    // Logical AND and OR are handled separately
+    BinaryOperationTree() { not this instanceof BinaryLogicalOperation }
 
-  class BlockTree extends ScopeTree, Block {
     final override ControlFlowTree getChildNode(int i) {
-      result = this.getParameters() and i = 0
+      result = this.getLeftOperand() and i = 0
       or
-      result = this.getChild(i - 1)
+      result = this.getRightOperand() and i = 1
     }
   }
 
-  private class CaseTree extends PreOrderTree, Case {
-    final override predicate propagatesAbnormal(AstNode child) {
-      child = this.getValue() or child = this.getChild(_)
-    }
-
-    final override predicate last(AstNode last, Completion c) {
-      last(this.getValue(), last, c) and not exists(this.getChild(_))
-      or
-      last(this.getChild(_).(When).getBody(), last, c)
-      or
-      exists(int i, ControlFlowTree lastBranch |
-        lastBranch = this.getChild(i) and
-        not exists(this.getChild(i + 1)) and
-        last(lastBranch, last, c)
-      )
-    }
-
-    final override predicate succ(AstNode pred, AstNode succ, Completion c) {
-      exists(AstNode next |
-        pred = this and
-        first(next, succ) and
-        c instanceof SimpleCompletion
-      |
-        next = this.getValue()
-        or
-        not exists(this.getValue()) and
-        next = this.getChild(0)
-      )
-      or
-      last(this.getValue(), pred, c) and
-      first(this.getChild(0), succ) and
-      c instanceof SimpleCompletion
-      or
-      exists(int i, WhenTree branch | branch = this.getChild(i) |
-        last(branch.getLastPattern(), pred, c) and
-        first(this.getChild(i + 1), succ) and
-        c.(ConditionalCompletion).getValue() = false
-      )
-    }
+  private class BlockArgumentTree extends StandardPostOrderTree, BlockArgument {
+    final override ControlFlowTree getChildNode(int i) { result = this.getValue() and i = 0 }
   }
 
-  private class CharacterTree extends LeafTree, Character { }
-
-  private class ClassTree extends RescueEnsureBlockTree, PreOrderTree, Class {
-    final override AstNode getChildNode(int i, boolean rescuable) {
-      result = this.getName() and i = 0 and rescuable = false
-      or
-      result = this.getSuperclass() and i = 1 and rescuable = true
-      or
-      result = this.getChild(i - 2) and rescuable = true
-    }
-
-    final override predicate last(AstNode last, Completion c) { this.lastInner(last, c) }
-  }
-
-  private class ClassVariableTree extends LeafTree, ClassVariable { }
-
-  private class ComplexTree extends LeafTree, Complex { }
-
-  private class ConstantTree extends LeafTree, Constant { }
-
-  /** A parameter that may have a default value. */
-  abstract class DefaultValueParameterTree extends ControlFlowTree {
-    abstract AstNode getDefaultValue();
-
-    abstract AstNode getAccessNode();
-
-    predicate hasDefaultValue() { exists(this.getDefaultValue()) }
-
-    final override predicate propagatesAbnormal(AstNode child) {
-      child = this.getDefaultValue() or child = this.getAccessNode()
-    }
-
-    final override predicate first(AstNode first) { first = this.getAccessNode() }
-
-    final override predicate last(AstNode last, Completion c) {
-      last(this.getDefaultValue(), last, c) and
-      c instanceof NormalCompletion
-      or
-      last = this.getAccessNode() and
-      (
-        not this.hasDefaultValue() and
-        c instanceof SimpleCompletion
-        or
-        this.hasDefaultValue() and
-        c.(MatchingCompletion).getValue() = true
-      )
-    }
-
-    final override predicate succ(AstNode pred, AstNode succ, Completion c) {
-      pred = this.getAccessNode() and
-      first(this.getDefaultValue(), succ) and
-      c.(MatchingCompletion).getValue() = false
+  private class BlockParameterTree extends StandardPostOrderTree, BlockParameter {
+    final override ControlFlowTree getChildNode(int i) {
+      result = this.getVariable().getDefiningAccess() and i = 0
     }
   }
 
-  class DoBlockTree extends RescueEnsureBlockTree, PostOrderTree, DoBlock {
-    final override predicate first(AstNode first) { first = this }
-
-    final override AstNode getChildNode(int i, boolean rescuable) {
-      result = this.getParameters() and i = 0 and rescuable = false
-      or
-      result = this.getChild(i - 1) and rescuable = true
-    }
-  }
-
-  private class EmptyStatementTree extends LeafTree, EmptyStatement { }
-
-  class EndBlockTree extends ScopeTree, EndBlock {
-    final override ControlFlowTree getChildNode(int i) { result = this.getChild(i) }
-  }
-
-  private class ExceptionsTree extends PreOrderTree, Exceptions {
-    final override predicate propagatesAbnormal(AstNode child) { none() }
-
-    final override predicate last(AstNode last, Completion c) {
-      last(this.getChild(_), last, c) and
-      c.(MatchingCompletion).getValue() = true
-      or
-      exists(int lst |
-        last(this.getChild(lst), last, c) and
-        not exists(this.getChild(lst + 1))
-      )
-    }
-
-    final override predicate succ(AstNode pred, AstNode succ, Completion c) {
-      pred = this and
-      first(this.getChild(0), succ) and
-      c instanceof SimpleCompletion
-      or
-      exists(int i |
-        last(this.getChild(i), pred, c) and
-        c.(MatchingCompletion).getValue() = false and
-        first(this.getChild(i + 1), succ)
-      )
-    }
-
-    override predicate isHidden() { any() }
-  }
-
-  private class FalseTree extends LeafTree, False { }
-
-  private class FloatTree extends LeafTree, Float { }
-
   /**
-   * Control flow of a for-in loop
-   *
-   * For example, this program fragment:
-   *
-   * ```rb
-   * for arg in args do
-   *   puts arg
-   * end
-   * puts "done";
-   * ```
-   *
-   * has the following control flow graph:
-   *
-   * ```
-   *           args
-   *            |
-   *           in------<-----
-   *           / \           \
-   *          /   \          |
-   *         /     \         |
-   *        /       \        |
-   *     empty    non-empty  |
-   *       |          \      |
-   *      for          \     |
-   *       |          arg    |
-   *       |            |    |
-   *  puts "done"   puts arg |
-   *                     \___/
-   * ```
+   * TODO: make all StmtSequence tree classes post-order, and simplify class
+   * hierarchy.
    */
-  private class ForTree extends PostOrderTree, For {
-    final override predicate propagatesAbnormal(AstNode child) {
-      child = this.getPattern() or child = this.getArray()
-    }
-
-    final override predicate first(AstNode first) { first(this.getArray(), first) }
-
-    private In getIn() { result = this.getValue() }
-
-    private UnderscoreArg getArray() { result = this.getValue().getChild() }
-
-    /**
-     * for pattern in array do body end
-     * ```
-     * array +-> in +--[non empty]--> pattern -> body -> in
-     *              |--[empty]--> for
-     * ```
-     */
-    final override predicate succ(AstNode pred, AstNode succ, Completion c) {
-      last(this.getArray(), pred, c) and
-      first(this.getIn(), succ) and
-      c instanceof SimpleCompletion
+  abstract class BodyStmtTree extends StmtSequenceTree, BodyStmt {
+    predicate firstInner(AstNode first) {
+      first(this.getBodyChild(0, _), first)
       or
-      last(this.getIn(), pred, c) and
-      first(this.getPattern(), succ) and
-      c.(EmptinessCompletion).getValue() = false
-      or
-      last(this.getPattern(), pred, c) and
-      first(this.getBody(), succ) and
-      c instanceof NormalCompletion
-      or
-      last(this.getBody(), pred, c) and
-      first(this.getIn(), succ) and
-      c.continuesLoop()
-      or
-      last(this.getBody(), pred, c) and
-      first(this.getBody(), succ) and
-      c instanceof RedoCompletion
-      or
-      succ = this and
+      not exists(this.getBodyChild(_, _)) and
       (
-        last(this.getIn(), pred, c) and
-        c.(EmptinessCompletion).getValue() = true
+        first(this.getRescue(_), first)
         or
-        last(this.getBody(), pred, c) and
-        not c.continuesLoop() and
-        not c instanceof BreakCompletion and
-        not c instanceof RedoCompletion
-        or
-        last(this.getBody(), pred, c.(NestedBreakCompletion).getAnInnerCompatibleCompletion())
+        not exists(this.getRescue(_)) and
+        first(this.getEnsure(), first)
       )
     }
-  }
 
-  private class GlobalVariableTree extends LeafTree, GlobalVariable { }
-
-  private HeredocBody heredoc(HeredocBeginning start) {
-    exists(int i, File f |
-      start =
-        rank[i](HeredocBeginning b |
-          f = b.getLocation().getFile()
-        |
-          b order by b.getLocation().getStartLine(), b.getLocation().getStartColumn()
-        ) and
-      result =
-        rank[i](HeredocBody b |
-          f = b.getLocation().getFile()
-        |
-          b order by b.getLocation().getStartLine(), b.getLocation().getStartColumn()
+    predicate lastInner(AstNode last, Completion c) {
+      exists(boolean ensurable | last = this.getAnEnsurePredecessor(c, ensurable) |
+        not this.hasEnsure()
+        or
+        ensurable = false
+      )
+      or
+      // If the body completes normally, take the completion from the `ensure` block
+      this.lastEnsure(last, c, any(NormalCompletion nc), _)
+      or
+      // If the `ensure` block completes normally, it inherits any non-normal
+      // completion from the body
+      c =
+        any(NestedEnsureCompletion nec |
+          this.lastEnsure(last, nec.getAnInnerCompatibleCompletion(), nec.getOuterCompletion(),
+            nec.getNestLevel())
         )
-    )
-  }
-
-  private class HeredocBeginningTree extends StandardPreOrderTree, HeredocBeginning {
-    final override ControlFlowTree getChildNode(int i) { result = heredoc(this).getChild(i) }
-  }
-
-  private class IdentifierTree extends LeafTree, Identifier { }
-
-  private class IfElsifTree extends PostOrderTree, IfElsifAstNode {
-    final override predicate propagatesAbnormal(AstNode child) {
-      child = this.getConditionNode() or child = this.getBranch(_)
-    }
-
-    final override predicate first(AstNode first) { first(this.getConditionNode(), first) }
-
-    final override predicate succ(AstNode pred, AstNode succ, Completion c) {
-      exists(boolean b |
-        last(this.getConditionNode(), pred, c) and
-        b = c.(BooleanCompletion).getValue()
-      |
-        first(this.getBranch(b), succ)
-        or
-        not exists(this.getBranch(b)) and
-        succ = this
-      )
       or
-      last(this.getBranch(_), pred, c) and
-      succ = this and
-      c instanceof NormalCompletion
-    }
-  }
-
-  private class InTree extends LeafTree, In { }
-
-  private class InstanceVariableTree extends LeafTree, InstanceVariable { }
-
-  private class IntegerTree extends LeafTree, Integer { }
-
-  private class KeywordParameterTree extends DefaultValueParameterTree, KeywordParameter {
-    final override AstNode getDefaultValue() { result = this.getValue() }
-
-    final override AstNode getAccessNode() { result = this.getName() }
-  }
-
-  class LambdaTree extends LeafTree, Lambda {
-    final override predicate succ(AstNode pred, AstNode succ, Completion c) {
-      last(this.getParameters(), pred, c) and
-      c instanceof NormalCompletion and
-      (
-        this.getBody().(DoBlockTree).firstInner(succ)
-        or
-        first(this.getBody().(BlockTree).getFirstChildNode(), succ)
-      )
-    }
-  }
-
-  class LogicalAndTree extends PostOrderTree, LogicalAndAstNode {
-    final override predicate propagatesAbnormal(AstNode child) { child in [left, right] }
-
-    final override predicate first(AstNode first) { first(left, first) }
-
-    final override predicate succ(AstNode pred, AstNode succ, Completion c) {
-      last(left, pred, c) and
-      c instanceof TrueCompletion and
-      first(right, succ)
+      not exists(this.getBodyChild(_, _)) and
+      not exists(this.getRescue(_)) and
+      this.lastEnsure0(last, c)
       or
-      last(left, pred, c) and
-      c instanceof FalseCompletion and
-      succ = this
-      or
-      last(right, pred, c) and
-      c instanceof NormalCompletion and
-      succ = this
+      last([this.getEnsure().(AstNode), this.getBodyChild(_, false)], last, c) and
+      not c instanceof NormalCompletion
     }
-  }
-
-  class LogicalOrTree extends PostOrderTree, LogicalOrAstNode {
-    final override predicate propagatesAbnormal(AstNode child) { child in [left, right] }
-
-    final override predicate first(AstNode first) { first(left, first) }
-
-    final override predicate succ(AstNode pred, AstNode succ, Completion c) {
-      last(left, pred, c) and
-      c instanceof FalseCompletion and
-      first(right, succ)
-      or
-      last(left, pred, c) and
-      c instanceof TrueCompletion and
-      succ = this
-      or
-      last(right, pred, c) and
-      c instanceof NormalCompletion and
-      succ = this
-    }
-  }
-
-  class LogicalNotTree extends PostOrderTree, LogicalNotAstNode {
-    final override predicate propagatesAbnormal(AstNode child) { child = operand }
-
-    final override predicate first(AstNode first) { first(operand, first) }
-
-    final override predicate succ(AstNode pred, AstNode succ, Completion c) {
-      succ = this and
-      last(operand, pred, c) and
-      c instanceof NormalCompletion
-    }
-  }
-
-  private class MethodTree extends RescueEnsureBlockTree, PostOrderTree, Method {
-    final override AstNode getChildNode(int i, boolean rescuable) {
-      result = this.getParameters() and i = 0 and rescuable = false
-      or
-      result = this.getChild(i - 1) and rescuable = true
-    }
-
-    final override predicate first(AstNode first) { first(this.getName(), first) }
 
     override predicate succ(AstNode pred, AstNode succ, Completion c) {
-      RescueEnsureBlockTree.super.succ(pred, succ, c)
+      this instanceof PreOrderTree and
+      pred = this and
+      c instanceof SimpleCompletion and
+      this.firstInner(succ)
       or
-      last(this.getName(), pred, c) and
-      succ = this and
+      // Normal left-to-right evaluation in the body
+      exists(int i |
+        last(this.getBodyChild(i, _), pred, c) and
+        first(this.getBodyChild(i + 1, _), succ) and
+        c instanceof NormalCompletion
+      )
+      or
+      // Exceptional flow from body to first `rescue`
+      this.lastBody(pred, c, true) and
+      first(this.getRescue(0), succ) and
+      c instanceof RaiseCompletion
+      or
+      // Flow from one `rescue` clause to the next when there is no match
+      exists(RescueTree rescue, int i | rescue = this.getRescue(i) |
+        rescue.lastNoMatch(pred, c) and
+        first(this.getRescue(i + 1), succ)
+      )
+      or
+      // Flow from body to `else` block when no exception
+      this.lastBody(pred, c, _) and
+      first(this.getElse(), succ) and
       c instanceof NormalCompletion
-    }
-  }
-
-  private class ModuleTree extends RescueEnsureBlockTree, PreOrderTree, Module {
-    final override AstNode getChildNode(int i, boolean rescuable) {
-      result = this.getName() and i = 0 and rescuable = false
       or
-      result = this.getChild(i - 1) and rescuable = true
-    }
-
-    final override predicate last(AstNode last, Completion c) { this.lastInner(last, c) }
-  }
-
-  private class NilTree extends LeafTree, Nil { }
-
-  private class OptionalParameterTree extends DefaultValueParameterTree, OptionalParameter {
-    final override AstNode getDefaultValue() { result = this.getValue() }
-
-    final override AstNode getAccessNode() { result = this.getName() }
-  }
-
-  private class RationalTree extends LeafTree, Rational { }
-
-  private class RescueTree extends PreOrderTree, Rescue {
-    final override predicate propagatesAbnormal(AstNode child) { child = this.getExceptions() }
-
-    predicate lastMatch(AstNode last, Completion c) {
-      last(this.getBody(), last, c)
-      or
-      not exists(this.getBody()) and
-      (
-        last(this.getVariable(), last, c)
-        or
-        not exists(this.getVariable()) and
-        (
-          last(this.getExceptions(), last, c) and
-          c.(MatchingCompletion).getValue() = true
-          or
-          not exists(this.getExceptions()) and
-          last = this and
-          isValidFor(c, this)
-        )
-      )
-    }
-
-    predicate lastNoMatch(AstNode last, Completion c) {
-      last(this.getExceptions(), last, c) and
-      c.(MatchingCompletion).getValue() = false
-    }
-
-    final override predicate last(AstNode last, Completion c) {
-      this.lastNoMatch(last, c)
-      or
-      this.lastMatch(last, c)
-    }
-
-    final override predicate succ(AstNode pred, AstNode succ, Completion c) {
-      exists(AstNode next |
-        pred = this and
-        first(next, succ) and
-        c instanceof SimpleCompletion
-      |
-        next = this.getExceptions()
-        or
-        not exists(this.getExceptions()) and
-        (
-          next = this.getVariable()
-          or
-          not exists(this.getVariable()) and
-          next = this.getBody()
-        )
-      )
-      or
-      exists(AstNode next |
-        last(this.getExceptions(), pred, c) and
-        first(next, succ) and
-        c.(MatchingCompletion).getValue() = true
-      |
-        next = this.getVariable()
-        or
-        not exists(this.getVariable()) and
-        next = this.getBody()
-      )
-      or
-      last(this.getVariable(), pred, c) and
-      first(this.getBody(), succ) and
-      c instanceof NormalCompletion
-    }
-  }
-
-  /** Gets a child of `n` that is in CFG scope `scope`. */
-  pragma[noinline]
-  private AstNode getAChildInScope(AstNode n, CfgScope scope) {
-    result.getParent() = n and
-    scope = getCfgScope(result)
-  }
-
-  /** A block that may contain `rescue`/`ensure`. */
-  abstract class RescueEnsureBlockTree extends ControlFlowTree {
-    /**
-     * Gets the `i`th child of this block. `rescuable` indicates whether exceptional
-     * execution of the child can be caught by `rescue`/`ensure`.
-     */
-    abstract AstNode getChildNode(int i, boolean rescuable);
-
-    /** Gets the `i`th child in the body of this block. */
-    final private AstNode getBodyChild(int i, boolean rescuable) {
-      result = this.getChildNode(_, rescuable) and
-      result =
-        rank[i + 1](AstNode child, int j |
-          child = this.getChildNode(j, _) and
-          not result instanceof Rescue and
-          not result instanceof Ensure and
-          not result instanceof Else
-        |
-          child order by j
-        )
-    }
-
-    /** Gets the `i`th `rescue` block in this block. */
-    final Rescue getRescue(int i) {
-      result = rank[i + 1](Rescue s | s = this.getAFieldOrChild() | s order by s.getParentIndex())
-    }
-
-    /** Gets the `else` block in this block, if any. */
-    final private Else getElse() { result = unique(Else s | s = this.getAFieldOrChild()) }
-
-    /** Gets the `ensure` block in this block, if any. */
-    final Ensure getEnsure() { result = unique(Ensure s | s = this.getAFieldOrChild()) }
-
-    final private predicate hasEnsure() { exists(this.getEnsure()) }
-
-    final override predicate propagatesAbnormal(AstNode child) { none() }
-
-    /**
-     * Gets a descendant that belongs to the `ensure` block of this block, if any.
-     * Nested `ensure` blocks are not included.
-     */
-    AstNode getAnEnsureDescendant() {
-      result = this.getEnsure()
-      or
-      exists(AstNode mid |
-        mid = this.getAnEnsureDescendant() and
-        result = getAChildInScope(mid, getCfgScope(mid)) and
-        not exists(RescueEnsureBlockTree nestedBlock |
-          result = nestedBlock.getEnsure() and
-          nestedBlock != this
-        )
-      )
-    }
-
-    /**
-     * Holds if `innerBlock` has an `ensure` block and is immediately nested inside the
-     * `ensure` block of this block.
-     */
-    private predicate nestedEnsure(RescueEnsureBlockTree innerBlock) {
-      exists(Ensure innerEnsure |
-        innerEnsure = getAChildInScope(this.getAnEnsureDescendant(), getCfgScope(this)) and
-        innerEnsure = innerBlock.getEnsure()
-      )
-    }
-
-    /**
-     * Gets the `ensure`-nesting level of this block. That is, the number of `ensure`
-     * blocks that this block is nested under.
-     */
-    int nestLevel() { result = count(RescueEnsureBlockTree outer | outer.nestedEnsure+(this)) }
-
-    /**
-     * Holds if `last` is a last element in the body of this block. `ensurable`
-     * indicates whether `last` may be a predecessor of an `ensure` block.
-     */
-    pragma[nomagic]
-    private predicate lastBody(AstNode last, Completion c, boolean ensurable) {
-      exists(boolean rescuable |
-        if c instanceof RaiseCompletion then ensurable = rescuable else ensurable = true
-      |
-        last(this.getBodyChild(_, rescuable), last, c) and
-        not c instanceof NormalCompletion
-        or
-        exists(int lst |
-          last(this.getBodyChild(lst, rescuable), last, c) and
-          not exists(this.getBodyChild(lst + 1, _))
-        )
-      )
+      // Flow into `ensure` block
+      pred = getAnEnsurePredecessor(c, true) and
+      first(this.getEnsure(), succ)
     }
 
     /**
@@ -1048,6 +508,40 @@ module Trees {
     pragma[nomagic]
     private predicate lastEnsure0(AstNode last, Completion c) { last(this.getEnsure(), last, c) }
 
+    /**
+     * Gets a descendant that belongs to the `ensure` block of this block, if any.
+     * Nested `ensure` blocks are not included.
+     */
+    AstNode getAnEnsureDescendant() {
+      result = this.getEnsure()
+      or
+      exists(AstNode mid |
+        mid = this.getAnEnsureDescendant() and
+        result = getAChildInScope(mid, getCfgScope(mid)) and
+        not exists(BodyStmt nestedBlock |
+          result = nestedBlock.getEnsure() and
+          nestedBlock != this
+        )
+      )
+    }
+
+    /**
+     * Holds if `innerBlock` has an `ensure` block and is immediately nested inside the
+     * `ensure` block of this block.
+     */
+    private predicate nestedEnsure(BodyStmtTree innerBlock) {
+      exists(StmtSequence innerEnsure |
+        innerEnsure = getAChildInScope(this.getAnEnsureDescendant(), getCfgScope(this)) and
+        innerEnsure = innerBlock.(BodyStmt).getEnsure()
+      )
+    }
+
+    /**
+     * Gets the `ensure`-nesting level of this block. That is, the number of `ensure`
+     * blocks that this block is nested under.
+     */
+    int getNestLevel() { result = count(BodyStmtTree outer | outer.nestedEnsure+(this)) }
+
     pragma[nomagic]
     private predicate lastEnsure(
       AstNode last, NormalCompletion ensure, Completion outer, int nestLevel
@@ -1056,83 +550,472 @@ module Trees {
       exists(
         this.getAnEnsurePredecessor(any(Completion c0 | outer = c0.getOuterCompletion()), true)
       ) and
-      nestLevel = this.nestLevel()
+      nestLevel = this.getNestLevel()
     }
 
-    predicate lastInner(AstNode last, Completion c) {
-      exists(boolean ensurable | last = this.getAnEnsurePredecessor(c, ensurable) |
-        not this.hasEnsure()
+    /**
+     * Holds if `last` is a last element in the body of this block. `ensurable`
+     * indicates whether `last` may be a predecessor of an `ensure` block.
+     */
+    pragma[nomagic]
+    private predicate lastBody(AstNode last, Completion c, boolean ensurable) {
+      exists(boolean rescuable |
+        if c instanceof RaiseCompletion then ensurable = rescuable else ensurable = true
+      |
+        last(this.getBodyChild(_, rescuable), last, c) and
+        not c instanceof NormalCompletion
         or
-        ensurable = false
-      )
-      or
-      // If the body completes normally, take the completion from the `ensure` block
-      this.lastEnsure(last, c, any(NormalCompletion nc), _)
-      or
-      // If the `ensure` block completes normally, it inherits any non-normal
-      // completion from the body
-      c =
-        any(NestedEnsureCompletion nec |
-          this.lastEnsure(last, nec.getAnInnerCompatibleCompletion(), nec.getOuterCompletion(),
-            nec.getNestLevel())
+        exists(int lst |
+          last(this.getBodyChild(lst, rescuable), last, c) and
+          not exists(this.getBodyChild(lst + 1, _))
         )
-      or
-      not exists(this.getBodyChild(_, _)) and
-      not exists(this.getRescue(_)) and
-      this.lastEnsure0(last, c)
-      or
-      last([this.getEnsure(), this.getBodyChild(_, false)], last, c) and
-      not c instanceof NormalCompletion
-    }
-
-    predicate firstInner(AstNode first) {
-      first(this.getBodyChild(0, _), first)
-      or
-      not exists(this.getBodyChild(_, _)) and
-      (
-        first(this.getRescue(_), first)
-        or
-        not exists(this.getRescue(_)) and
-        first(this.getEnsure(), first)
       )
     }
-
-    override predicate succ(AstNode pred, AstNode succ, Completion c) {
-      this instanceof PreOrderTree and
-      pred = this and
-      c instanceof SimpleCompletion and
-      this.firstInner(succ)
-      or
-      // Normal left-to-right evaluation in the body
-      exists(int i |
-        last(this.getBodyChild(i, _), pred, c) and
-        first(this.getBodyChild(i + 1, _), succ) and
-        c instanceof NormalCompletion
-      )
-      or
-      // Exceptional flow from body to first `rescue`
-      this.lastBody(pred, c, true) and
-      first(this.getRescue(0), succ) and
-      c instanceof RaiseCompletion
-      or
-      // Flow from one `rescue` clause to the next when there is no match
-      exists(RescueTree rescue, int i | rescue = this.getRescue(i) |
-        rescue.lastNoMatch(pred, c) and
-        first(this.getRescue(i + 1), succ)
-      )
-      or
-      // Flow from body to `else` block when no exception
-      this.lastBody(pred, c, _) and
-      first(this.getElse(), succ) and
-      c instanceof NormalCompletion
-      or
-      // Flow into `ensure` block
-      pred = getAnEnsurePredecessor(c, true) and
-      first(this.getEnsure(), succ)
-    }
   }
 
-  private class RescueModifierTree extends PreOrderTree, RescueModifier {
+  private class BooleanLiteralTree extends LeafTree, BooleanLiteral { }
+
+  class BraceBlockTree extends ScopeTree, BraceBlock {
+    final override ControlFlowTree getChildNode(int i) {
+      result = this.getParameter(i)
+      or
+      result = this.getStmt(i - this.getNumberOfParameters())
+    }
+  }
+
+  private class CaseTree extends PreOrderTree, CaseExpr {
+    final override predicate propagatesAbnormal(AstNode child) {
+      child = this.getValue() or child = this.getABranch()
+    }
+
+    final override predicate last(AstNode last, Completion c) {
+      last(this.getValue(), last, c) and not exists(this.getABranch())
+      or
+      last(this.getAWhenBranch().getBody(), last, c)
+      or
+      exists(int i, ControlFlowTree lastBranch |
+        lastBranch = this.getBranch(i) and
+        not exists(this.getBranch(i + 1)) and
+        last(lastBranch, last, c)
+      )
+    }
+
+    final override predicate succ(AstNode pred, AstNode succ, Completion c) {
+      exists(AstNode next |
+        pred = this and
+        first(next, succ) and
+        c instanceof SimpleCompletion
+      |
+        next = this.getValue()
+        or
+        not exists(this.getValue()) and
+        next = this.getBranch(0)
+      )
+      or
+      last(this.getValue(), pred, c) and
+      first(this.getBranch(0), succ) and
+      c instanceof SimpleCompletion
+      or
+      exists(int i, WhenTree branch | branch = this.getBranch(i) |
+        last(branch.getLastPattern(), pred, c) and
+        first(this.getBranch(i + 1), succ) and
+        c.(ConditionalCompletion).getValue() = false
+      )
+    }
+  }
+
+  private class CharacterTree extends LeafTree, CharacterLiteral { }
+
+  private class ClassTree extends BodyStmtTree, PreOrderTree, Class {
+    /** Gets the `i`th child in the body of this block. */
+    final override AstNode getBodyChild(int i, boolean rescuable) {
+      result = this.getScopeExpr() and i = 0 and rescuable = false
+      or
+      result = this.getSuperclassExpr() and
+      i = count(this.getScopeExpr()) and
+      rescuable = true
+      or
+      result = this.getStmt(i - count(this.getScopeExpr()) - count(this.getSuperclassExpr())) and
+      rescuable = true
+    }
+
+    final override predicate last(AstNode last, Completion c) { this.lastInner(last, c) }
+  }
+
+  private class ClassVariableTree extends LeafTree, ClassVariableAccess { }
+
+  private class ConditionalExprTree extends PostOrderTree, ConditionalExpr {
+    final override predicate propagatesAbnormal(AstNode child) {
+      child = this.getCondition() or child = this.getBranch(_)
+    }
+
+    final override predicate first(AstNode first) { first(this.getCondition(), first) }
+
+    final override predicate succ(AstNode pred, AstNode succ, Completion c) {
+      exists(boolean b |
+        last(this.getCondition(), pred, c) and
+        b = c.(BooleanCompletion).getValue()
+      |
+        first(this.getBranch(b), succ)
+        or
+        not exists(this.getBranch(b)) and
+        succ = this
+      )
+      or
+      last(this.getBranch(_), pred, c) and
+      succ = this and
+      c instanceof NormalCompletion
+    }
+  }
+
+  private class ConditionalLoopTree extends PostOrderTree, ConditionalLoop {
+    final override predicate propagatesAbnormal(AstNode child) { child = this.getCondition() }
+
+    final override predicate first(AstNode first) { first(this.getCondition(), first) }
+
+    final override predicate succ(AstNode pred, AstNode succ, Completion c) {
+      last(this.getCondition(), pred, c) and
+      this.entersLoopWhenConditionIs(c.(BooleanCompletion).getValue()) and
+      first(this.getBody(), succ)
+      or
+      last(this.getBody(), pred, c) and
+      first(this.getCondition(), succ) and
+      c.continuesLoop()
+      or
+      last(this.getBody(), pred, c) and
+      first(this.getBody(), succ) and
+      c instanceof RedoCompletion
+      or
+      succ = this and
+      (
+        last(this.getCondition(), pred, c) and
+        this.entersLoopWhenConditionIs(c.(BooleanCompletion).getValue().booleanNot())
+        or
+        last(this.getBody(), pred, c) and
+        not c.continuesLoop() and
+        not c instanceof BreakCompletion and
+        not c instanceof RedoCompletion
+        or
+        last(this.getBody(), pred, c.(NestedBreakCompletion).getAnInnerCompatibleCompletion())
+      )
+    }
+  }
+
+  private class ConstantAccessTree extends PostOrderTree, ConstantAccess {
+    ConstantAccessTree() {
+      not this instanceof Class and
+      not this instanceof Module
+    }
+
+    final override predicate propagatesAbnormal(AstNode child) { child = this.getScopeExpr() }
+
+    final override predicate first(AstNode first) {
+      first(this.getScopeExpr(), first)
+      or
+      not exists(this.getScopeExpr()) and
+      first = this
+    }
+
+    final override predicate succ(AstNode pred, AstNode succ, Completion c) {
+      last(this.getScopeExpr(), pred, c) and
+      succ = this and
+      c instanceof NormalCompletion
+    }
+  }
+
+  /** A parameter that may have a default value. */
+  abstract class DefaultValueParameterTree extends ControlFlowTree {
+    abstract Expr getDefaultValueExpr();
+
+    abstract AstNode getAccessNode();
+
+    predicate hasDefaultValue() { exists(this.getDefaultValueExpr()) }
+
+    final override predicate propagatesAbnormal(AstNode child) {
+      child = this.getDefaultValueExpr() or child = this.getAccessNode()
+    }
+
+    final override predicate first(AstNode first) { first = this.getAccessNode() }
+
+    final override predicate last(AstNode last, Completion c) {
+      last(this.getDefaultValueExpr(), last, c) and
+      c instanceof NormalCompletion
+      or
+      last = this.getAccessNode() and
+      (
+        not this.hasDefaultValue() and
+        c instanceof SimpleCompletion
+        or
+        this.hasDefaultValue() and
+        c.(MatchingCompletion).getValue() = true
+      )
+    }
+
+    final override predicate succ(AstNode pred, AstNode succ, Completion c) {
+      pred = this.getAccessNode() and
+      first(this.getDefaultValueExpr(), succ) and
+      c.(MatchingCompletion).getValue() = false
+    }
+  }
+
+  private class DoBlockTree extends BodyStmtTree, PostOrderTree, DoBlock {
+    final override predicate first(AstNode first) { first = this }
+
+    /** Gets the `i`th child in the body of this block. */
+    final override AstNode getBodyChild(int i, boolean rescuable) {
+      result = this.getParameter(i) and rescuable = false
+      or
+      result = BodyStmtTree.super.getBodyChild(i - this.getNumberOfParameters(), rescuable)
+    }
+  }
+
+  private class EmptyStatementTree extends LeafTree, EmptyStmt { }
+
+  class EndBlockTree extends ScopeTree, EndBlock {
+    final override ControlFlowTree getChildNode(int i) { result = this.getStmt(i) }
+  }
+
+  private class ForInTree extends LeafTree, ForIn { }
+
+  /**
+   * Control flow of a for-in loop
+   *
+   * For example, this program fragment:
+   *
+   * ```rb
+   * for arg in args do
+   *   puts arg
+   * end
+   * puts "done";
+   * ```
+   *
+   * has the following control flow graph:
+   *
+   * ```
+   *           args
+   *            |
+   *           in------<-----
+   *           / \           \
+   *          /   \          |
+   *         /     \         |
+   *        /       \        |
+   *     empty    non-empty  |
+   *       |          \      |
+   *      for          \     |
+   *       |          arg    |
+   *       |            |    |
+   *  puts "done"   puts arg |
+   *                     \___/
+   * ```
+   */
+  private class ForTree extends PostOrderTree, ForRange {
+    final override predicate propagatesAbnormal(AstNode child) {
+      child = this.getPattern() or child = this.getValue()
+    }
+
+    final override predicate first(AstNode first) { first(this.getValue(), first) }
+
+    /**
+     * for pattern in array do body end
+     * ```
+     * array +-> in +--[non empty]--> pattern -> body -> in
+     *              |--[empty]--> for
+     * ```
+     */
+    final override predicate succ(AstNode pred, AstNode succ, Completion c) {
+      last(this.getValue(), pred, c) and
+      first(this.getIn(), succ) and
+      c instanceof SimpleCompletion
+      or
+      last(this.getIn(), pred, c) and
+      first(this.getPattern(), succ) and
+      c.(EmptinessCompletion).getValue() = false
+      or
+      last(this.getPattern(), pred, c) and
+      first(this.getBody(), succ) and
+      c instanceof NormalCompletion
+      or
+      last(this.getBody(), pred, c) and
+      first(this.getIn(), succ) and
+      c.continuesLoop()
+      or
+      last(this.getBody(), pred, c) and
+      first(this.getBody(), succ) and
+      c instanceof RedoCompletion
+      or
+      succ = this and
+      (
+        last(this.getIn(), pred, c) and
+        c.(EmptinessCompletion).getValue() = true
+        or
+        last(this.getBody(), pred, c) and
+        not c.continuesLoop() and
+        not c instanceof BreakCompletion and
+        not c instanceof RedoCompletion
+        or
+        last(this.getBody(), pred, c.(NestedBreakCompletion).getAnInnerCompatibleCompletion())
+      )
+    }
+  }
+
+  private class GlobalVariableTree extends LeafTree, GlobalVariableAccess { }
+
+  private class HashLiteralTree extends StandardPostOrderTree, HashLiteral {
+    final override ControlFlowTree getChildNode(int i) { result = this.getElement(i) }
+  }
+
+  private class HashSplatArgumentTree extends StandardPostOrderTree, HashSplatArgument {
+    final override ControlFlowTree getChildNode(int i) { result = this.getValue() and i = 0 }
+  }
+
+  private class HashSplatParameterTree extends StandardPostOrderTree, HashSplatParameter {
+    final override ControlFlowTree getChildNode(int i) {
+      result = this.getVariable().getDefiningAccess() and i = 0
+    }
+  }
+
+  private class HereDocTree extends StandardPreOrderTree, HereDoc {
+    final override ControlFlowTree getChildNode(int i) { result = this.getComponent(i) }
+  }
+
+  private class InstanceVariableTree extends LeafTree, InstanceVariableAccess { }
+
+  private class KeywordParameterTree extends DefaultValueParameterTree, KeywordParameter {
+    final override Expr getDefaultValueExpr() { result = this.getDefaultValue() }
+
+    final override AstNode getAccessNode() { result = this.getDefiningAccess() }
+  }
+
+  private class LambdaTree extends BodyStmtTree, PostOrderTree, Lambda {
+    final override predicate first(AstNode first) { first = this }
+
+    /** Gets the `i`th child in the body of this block. */
+    final override AstNode getBodyChild(int i, boolean rescuable) {
+      result = this.getParameter(i) and rescuable = false
+      or
+      result = BodyStmtTree.super.getBodyChild(i - this.getNumberOfParameters(), rescuable)
+    }
+  }
+
+  private class LocalVariableAccessTree extends LeafTree, LocalVariableAccess { }
+
+  private class LogicalAndTree extends PostOrderTree, LogicalAndExpr {
+    final override predicate propagatesAbnormal(AstNode child) { child = this.getAnOperand() }
+
+    final override predicate first(AstNode first) { first(this.getLeftOperand(), first) }
+
+    final override predicate succ(AstNode pred, AstNode succ, Completion c) {
+      last(this.getLeftOperand(), pred, c) and
+      c instanceof TrueCompletion and
+      first(this.getRightOperand(), succ)
+      or
+      last(this.getLeftOperand(), pred, c) and
+      c instanceof FalseCompletion and
+      succ = this
+      or
+      last(this.getRightOperand(), pred, c) and
+      c instanceof NormalCompletion and
+      succ = this
+    }
+  }
+
+  private class LogicalNotTree extends PostOrderTree, NotExpr {
+    final override predicate propagatesAbnormal(AstNode child) { child = this.getOperand() }
+
+    final override predicate first(AstNode first) { first(this.getOperand(), first) }
+
+    final override predicate succ(AstNode pred, AstNode succ, Completion c) {
+      succ = this and
+      last(this.getOperand(), pred, c) and
+      c instanceof NormalCompletion
+    }
+  }
+
+  private class LogicalOrTree extends PostOrderTree, LogicalOrExpr {
+    final override predicate propagatesAbnormal(AstNode child) { child = this.getAnOperand() }
+
+    final override predicate first(AstNode first) { first(this.getLeftOperand(), first) }
+
+    final override predicate succ(AstNode pred, AstNode succ, Completion c) {
+      last(this.getLeftOperand(), pred, c) and
+      c instanceof FalseCompletion and
+      first(this.getRightOperand(), succ)
+      or
+      last(this.getLeftOperand(), pred, c) and
+      c instanceof TrueCompletion and
+      succ = this
+      or
+      last(this.getRightOperand(), pred, c) and
+      c instanceof NormalCompletion and
+      succ = this
+    }
+  }
+
+  private class MethodCallTree extends StandardPostOrderTree, MethodCall {
+    final override ControlFlowTree getChildNode(int i) {
+      result = this.getReceiver() and i = 0
+      or
+      result = this.getArgument(i - count(this.getReceiver()))
+      or
+      result = this.getBlock() and i = count(this.getReceiver()) + this.getNumberOfArguments()
+    }
+  }
+
+  private class MethodNameTree extends LeafTree, MethodName { }
+
+  private class MethodTree extends BodyStmtTree, PostOrderTree, Method {
+    final override predicate first(AstNode first) { first = this }
+
+    /** Gets the `i`th child in the body of this block. */
+    final override AstNode getBodyChild(int i, boolean rescuable) {
+      result = this.getParameter(i) and rescuable = false
+      or
+      result = BodyStmtTree.super.getBodyChild(i - this.getNumberOfParameters(), rescuable)
+    }
+  }
+
+  private class ModuleTree extends BodyStmtTree, PreOrderTree, Module {
+    /** Gets the `i`th child in the body of this block. */
+    final override AstNode getBodyChild(int i, boolean rescuable) {
+      result = this.getScopeExpr() and i = 0 and rescuable = false
+      or
+      result = BodyStmtTree.super.getBodyChild(i - count(this.getScopeExpr()), rescuable)
+    }
+
+    final override predicate last(AstNode last, Completion c) { this.lastInner(last, c) }
+  }
+
+  private class NilTree extends LeafTree, NilLiteral { }
+
+  private class NumericLiteralTree extends LeafTree, NumericLiteral { }
+
+  private class OptionalParameterTree extends DefaultValueParameterTree, OptionalParameter {
+    final override Expr getDefaultValueExpr() { result = this.getDefaultValue() }
+
+    final override AstNode getAccessNode() { result = this.getDefiningAccess() }
+  }
+
+  /** Contains partridges */
+  private class PairTree extends StandardPostOrderTree, Pair {
+    final override ControlFlowTree getChildNode(int i) {
+      result = this.getKey() and i = 0
+      or
+      result = this.getValue() and i = 1
+    }
+  }
+
+  private class RangeLiteralTree extends StandardPostOrderTree, RangeLiteral {
+    final override ControlFlowTree getChildNode(int i) {
+      result = this.getBegin() and i = 0
+      or
+      result = this.getEnd() and i = 1
+    }
+  }
+
+  private class RedoStmtTree extends LeafTree, RedoStmt { }
+
+  /** A block that may contain `rescue`/`ensure`. */
+  private class RescueModifierTree extends PreOrderTree, RescueModifierExpr {
     final override predicate propagatesAbnormal(AstNode child) { child = this.getHandler() }
 
     final override predicate last(AstNode last, Completion c) {
@@ -1153,56 +1036,277 @@ module Trees {
     }
   }
 
+  private class RescueTree extends PreOrderTree, RescueClause {
+    final override predicate propagatesAbnormal(AstNode child) { child = this.getAnException() }
+
+    private Expr getLastException() {
+      exists(int i | result = this.getException(i) and not exists(this.getException(i + 1)))
+    }
+
+    predicate lastMatch(AstNode last, Completion c) {
+      last(this.getBody(), last, c)
+      or
+      not exists(this.getBody()) and
+      (
+        last(this.getVariableExpr(), last, c)
+        or
+        not exists(this.getVariableExpr()) and
+        (
+          last(this.getAnException(), last, c) and
+          c.(MatchingCompletion).getValue() = true
+          or
+          not exists(this.getAnException()) and
+          last = this and
+          isValidFor(c, this)
+        )
+      )
+    }
+
+    predicate lastNoMatch(AstNode last, Completion c) {
+      last(this.getLastException(), last, c) and
+      c.(MatchingCompletion).getValue() = false
+    }
+
+    final override predicate last(AstNode last, Completion c) {
+      this.lastNoMatch(last, c)
+      or
+      this.lastMatch(last, c)
+    }
+
+    final override predicate succ(AstNode pred, AstNode succ, Completion c) {
+      exists(AstNode next |
+        pred = this and
+        first(next, succ) and
+        c instanceof SimpleCompletion
+      |
+        next = this.getException(0)
+        or
+        not exists(this.getException(0)) and
+        (
+          next = this.getVariableExpr()
+          or
+          not exists(this.getVariableExpr()) and
+          next = this.getBody()
+        )
+      )
+      or
+      exists(AstNode next |
+        last(this.getAnException(), pred, c) and
+        first(next, succ) and
+        c.(MatchingCompletion).getValue() = true
+      |
+        next = this.getVariableExpr()
+        or
+        not exists(this.getVariableExpr()) and
+        next = this.getBody()
+      )
+      or
+      exists(int i |
+        last(this.getException(i), pred, c) and
+        c.(MatchingCompletion).getValue() = false and
+        first(this.getException(i + 1), succ)
+      )
+      or
+      last(this.getVariableExpr(), pred, c) and
+      first(this.getBody(), succ) and
+      c instanceof NormalCompletion
+    }
+  }
+
+  private class RetryStmtTree extends LeafTree, RetryStmt { }
+
+  private class ReturningStmtTree extends StandardPostOrderTree, ReturningStmt {
+    final override ControlFlowTree getChildNode(int i) { result = this.getValue() and i = 0 }
+  }
+
   private class SelfTree extends LeafTree, Self { }
 
-  private class SetterTree extends LeafTree, Setter { }
+  private class SimpleParameterTree extends LeafTree, SimpleParameter { }
 
-  private class SingletonClassTree extends RescueEnsureBlockTree, PreOrderTree, SingletonClass {
-    final override AstNode getChildNode(int i, boolean rescuable) {
-      rescuable = true and
+  /**
+   * Control-flow tree for any post-order StmtSequence that doesn't have a more
+   * specific implementation.
+   * TODO: make all StmtSequence tree classes post-order, and simplify class
+   * hierarchy.
+   */
+  private class SimplePostOrderStmtSequenceTree extends StmtSequenceTree, PostOrderTree {
+    SimplePostOrderStmtSequenceTree() {
+      this instanceof StringInterpolationComponent or
+      this instanceof ParenthesizedExpr
+    }
+
+    final override predicate first(AstNode first) { first(this.getStmt(0), first) }
+
+    override predicate propagatesAbnormal(AstNode child) { child = this.getAStmt() }
+  }
+
+  /**
+   * Control-flow tree for any pre-order StmtSequence that doesn't have a more
+   * specific implementation.
+   * TODO: make all StmtSequence tree classes post-order, and simplify class
+   * hierarchy.
+   */
+  private class SimplePreOrderStmtSequenceTree extends StmtSequenceTree, PreOrderTree {
+    SimplePreOrderStmtSequenceTree() {
+      not this instanceof BodyStmtTree and
+      not this instanceof EndBlock and
+      not this instanceof StringInterpolationComponent and
+      not this instanceof Block and
+      not this instanceof ParenthesizedExpr
+    }
+
+    override predicate propagatesAbnormal(AstNode child) { child = this.getAStmt() }
+
+    override predicate isHidden() {
+      // TODO: unhide, or avoid using Generated types
+      ASTInternal::toGenerated(this) instanceof Generated::Else or
+      ASTInternal::toGenerated(this) instanceof Generated::Then or
+      ASTInternal::toGenerated(this) instanceof Generated::Do
+    }
+
+    final AstNode getLastChildNode() { result = this.getStmt(this.getNumberOfStatements() - 1) }
+
+    final override predicate last(AstNode last, Completion c) {
+      last(this.getLastChildNode(), last, c)
+      or
+      not exists(this.getLastChildNode()) and
+      isValidFor(c, this) and
+      last = this
+    }
+
+    override predicate succ(AstNode pred, AstNode succ, Completion c) {
+      StmtSequenceTree.super.succ(pred, succ, c)
+    }
+  }
+
+  private class SingletonClassTree extends BodyStmtTree, PreOrderTree, SingletonClass {
+    /** Gets the `i`th child in the body of this block. */
+    final override AstNode getBodyChild(int i, boolean rescuable) {
       (
-        result = this.getValue() and i = 0
+        result = this.getValue() and i = 0 and rescuable = false
         or
-        result = this.getChild(i - 1)
+        result = BodyStmtTree.super.getBodyChild(i - 1, rescuable)
       )
     }
 
     final override predicate last(AstNode last, Completion c) { this.lastInner(last, c) }
   }
 
-  private class SingletonMethodTree extends RescueEnsureBlockTree, PostOrderTree, SingletonMethod {
-    final override AstNode getChildNode(int i, boolean rescuable) {
-      result = this.getParameters() and
-      i = 0 and
-      rescuable = false
+  private class SingletonMethodTree extends BodyStmtTree, PostOrderTree, SingletonMethod {
+    /** Gets the `i`th child in the body of this block. */
+    final override AstNode getBodyChild(int i, boolean rescuable) {
+      result = this.getParameter(i) and rescuable = false
       or
-      result = this.getChild(i - 1) and
-      rescuable = true
+      result = BodyStmtTree.super.getBodyChild(i - this.getNumberOfParameters(), rescuable)
     }
 
     final override predicate first(AstNode first) { first(this.getObject(), first) }
 
     override predicate succ(AstNode pred, AstNode succ, Completion c) {
-      RescueEnsureBlockTree.super.succ(pred, succ, c)
+      BodyStmtTree.super.succ(pred, succ, c)
       or
       last(this.getObject(), pred, c) and
-      first(this.getName(), succ) and
-      c instanceof NormalCompletion
-      or
-      last(this.getName(), pred, c) and
       succ = this and
       c instanceof NormalCompletion
     }
   }
 
-  private class SuperTree extends LeafTree, Super { }
+  private class SplatArgumentTree extends StandardPostOrderTree, SplatArgument {
+    final override ControlFlowTree getChildNode(int i) { result = this.getValue() and i = 0 }
+  }
 
-  private class TrueTree extends LeafTree, True { }
+  private class SplatParameterTree extends StandardPostOrderTree, SplatParameter {
+    final override ControlFlowTree getChildNode(int i) {
+      result = this.getVariable().getDefiningAccess() and i = 0
+    }
+  }
 
-  private class WhenTree extends PreOrderTree, When {
-    final override predicate propagatesAbnormal(AstNode child) { child = this.getPattern(_) }
+  abstract class StmtSequenceTree extends ControlFlowTree, StmtSequence {
+    override predicate propagatesAbnormal(AstNode child) { none() }
 
-    final Pattern getLastPattern() {
+    /** Gets the `i`th child in the body of this body statement. */
+    AstNode getBodyChild(int i, boolean rescuable) {
+      result = this.getStmt(i) and
+      rescuable = true
+    }
+
+    AstNode getLastBodyChild() {
+      exists(int i |
+        result = this.getBodyChild(i, _) and
+        not exists(this.getBodyChild(i + 1, _))
+      )
+    }
+
+    override predicate succ(AstNode pred, AstNode succ, Completion c) {
+      this instanceof PreOrderTree and
+      pred = this and
+      first(this.getBodyChild(0, _), succ) and
+      c instanceof SimpleCompletion
+      or
+      this instanceof PostOrderTree and
+      succ = this and
+      last(this.getLastBodyChild(), pred, c)
+      or
+      // Normal left-to-right evaluation in the body
+      exists(int i |
+        last(this.getBodyChild(i, _), pred, c) and
+        first(this.getBodyChild(i + 1, _), succ) and
+        c instanceof NormalCompletion
+      )
+    }
+  }
+
+  private class StringConcatenationTree extends StandardPostOrderTree, StringConcatenation {
+    final override ControlFlowTree getChildNode(int i) { result = this.getString(i) }
+
+    override predicate isHidden() { any() }
+  }
+
+  private class StringTextComponentTree extends LeafTree, StringTextComponent {
+    override predicate isHidden() { any() }
+  }
+
+  private class StringlikeLiteralTree extends StandardPostOrderTree, StringlikeLiteral {
+    StringlikeLiteralTree() { not this instanceof HereDoc }
+
+    final override ControlFlowTree getChildNode(int i) { result = this.getComponent(i) }
+  }
+
+  private class SuperCallTree extends StandardPostOrderTree, SuperCall {
+    final override ControlFlowTree getChildNode(int i) { result = this.getArgument(i) }
+  }
+
+  private class ToplevelTree extends BodyStmtTree, PreOrderTree, Toplevel {
+    final override AstNode getBodyChild(int i, boolean rescuable) {
+      result = this.getBeginBlock(i) and rescuable = true
+      or
+      result = BodyStmtTree.super.getBodyChild(i - count(this.getABeginBlock()), rescuable)
+    }
+
+    final override predicate last(AstNode last, Completion c) { this.lastInner(last, c) }
+
+    override predicate isHidden() { any() }
+  }
+
+  private class TuplePatternTree extends StandardPostOrderTree, TuplePattern {
+    final override ControlFlowTree getChildNode(int i) { result = this.getElement(i) }
+  }
+
+  private class UnaryOperationTree extends StandardPostOrderTree, UnaryOperation {
+    // Logical NOT is handled separately
+    UnaryOperationTree() { not this instanceof NotExpr }
+
+    final override ControlFlowTree getChildNode(int i) { result = this.getOperand() and i = 0 }
+  }
+
+  private class UndefStmtTree extends StandardPreOrderTree, UndefStmt {
+    final override ControlFlowTree getChildNode(int i) { result = this.getMethodName(i) }
+  }
+
+  private class WhenTree extends PreOrderTree, WhenExpr {
+    final override predicate propagatesAbnormal(AstNode child) { child = this.getAPattern() }
+
+    final Expr getLastPattern() {
       exists(int i |
         result = this.getPattern(i) and
         not exists(this.getPattern(i + 1))
@@ -1221,7 +1325,7 @@ module Trees {
       first(this.getPattern(0), succ) and
       c instanceof SimpleCompletion
       or
-      exists(int i, Pattern p, boolean b |
+      exists(int i, Expr p, boolean b |
         p = this.getPattern(i) and
         last(p, pred, c) and
         b = c.(ConditionalCompletion).getValue()
@@ -1235,41 +1339,20 @@ module Trees {
     }
   }
 
-  private class ConditionalLoopTree extends PostOrderTree, ConditionalLoopAstNode {
-    final override predicate propagatesAbnormal(AstNode child) { child = this.getConditionNode() }
+  // TODO: make post-order
+  private class YieldCallTree extends StandardPreOrderTree, YieldCall {
+    final override ControlFlowTree getChildNode(int i) { result = this.getArgument(i) }
+  }
 
-    final override predicate first(AstNode first) { first(this.getConditionNode(), first) }
-
-    final override predicate succ(AstNode pred, AstNode succ, Completion c) {
-      last(this.getConditionNode(), pred, c) and
-      this.continueLoop(c) and
-      first(this.getBodyNode(), succ)
-      or
-      last(this.getBodyNode(), pred, c) and
-      first(this.getConditionNode(), succ) and
-      c.continuesLoop()
-      or
-      last(this.getBodyNode(), pred, c) and
-      first(this.getBodyNode(), succ) and
-      c instanceof RedoCompletion
-      or
-      succ = this and
-      (
-        last(this.getConditionNode(), pred, c) and
-        this.endLoop(c)
-        or
-        last(this.getBodyNode(), pred, c) and
-        not c.continuesLoop() and
-        not c instanceof BreakCompletion and
-        not c instanceof RedoCompletion
-        or
-        last(this.getBodyNode(), pred, c.(NestedBreakCompletion).getAnInnerCompatibleCompletion())
-      )
-    }
+  /** Gets a child of `n` that is in CFG scope `scope`. */
+  pragma[noinline]
+  private AstNode getAChildInScope(AstNode n, CfgScope scope) {
+    result.getParent() = n and
+    scope = getCfgScope(result)
   }
 }
 
-private Scope::Range parent(Scope::Range n) {
+private Scope parent(Scope n) {
   result = n.getOuterScope() and
   not n instanceof CfgScope::Range_
 }
@@ -1278,7 +1361,9 @@ cached
 private module Cached {
   /** Gets the CFG scope of node `n`. */
   cached
-  CfgScope getCfgScope(AstNode n) { ASTInternal::toGenerated(result) = parent*(scopeOf(n)) }
+  CfgScope getCfgScope(AstNode n) {
+    result = parent*(ASTInternal::fromGenerated(scopeOf(ASTInternal::toGenerated(n))))
+  }
 
   private predicate isAbnormalExitType(SuccessorType t) {
     t instanceof RaiseSuccessor or t instanceof ExitSuccessor
@@ -1359,5 +1444,5 @@ import Cached
 
 /** An AST node that is split into multiple control flow nodes. */
 class SplitAstNode extends AstNode {
-  SplitAstNode() { strictcount(CfgNode n | ASTInternal::toGenerated(n.getNode()) = this) > 1 }
+  SplitAstNode() { strictcount(CfgNode n | n.getNode() = this) > 1 }
 }
diff --git a/ql/src/codeql_ruby/controlflow/internal/Splitting.qll b/ql/src/codeql_ruby/controlflow/internal/Splitting.qll
index e1cf24a2815..7a81548ed5d 100644
--- a/ql/src/codeql_ruby/controlflow/internal/Splitting.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/Splitting.qll
@@ -2,9 +2,7 @@
  * Provides classes and predicates relevant for splitting the control flow graph.
  */
 
-private import codeql_ruby.ast.internal.TreeSitter::Generated
-private import codeql_ruby.ast.internal.AST as ASTInternal
-private import codeql_ruby.AST as AST
+private import codeql_ruby.AST
 private import Completion
 private import ControlFlowGraphImpl
 private import SuccessorTypes
@@ -18,13 +16,13 @@ private module Cached {
   cached
   newtype TSplitKind =
     TConditionalCompletionSplitKind() or
-    TEnsureSplitKind(int nestLevel) { nestLevel = any(Trees::RescueEnsureBlockTree t).nestLevel() }
+    TEnsureSplitKind(int nestLevel) { nestLevel = any(Trees::BodyStmtTree t).getNestLevel() }
 
   cached
   newtype TSplit =
     TConditionalCompletionSplit(ConditionalCompletion c) or
     TEnsureSplit(EnsureSplitting::EnsureSplitType type, int nestLevel) {
-      nestLevel = any(Trees::RescueEnsureBlockTree t).nestLevel()
+      nestLevel = any(Trees::BodyStmtTree t).getNestLevel()
     }
 
   cached
@@ -219,28 +217,19 @@ private module ConditionalCompletionSplitting {
       succ(pred, succ, c) and
       last(succ, _, completion) and
       (
-        last(ASTInternal::toGenerated(ASTInternal::fromGenerated(succ).(AST::NotExpr).getOperand()),
-          pred, c) and
+        last(succ.(NotExpr).getOperand(), pred, c) and
         completion.(BooleanCompletion).getDual() = c
         or
-        last(ASTInternal::toGenerated(ASTInternal::fromGenerated(succ)
-                .(AST::LogicalAndExpr)
-                .getAnOperand()), pred, c) and
+        last(succ.(LogicalAndExpr).getAnOperand(), pred, c) and
         completion = c
         or
-        last(ASTInternal::toGenerated(ASTInternal::fromGenerated(succ)
-                .(AST::LogicalOrExpr)
-                .getAnOperand()), pred, c) and
+        last(succ.(LogicalOrExpr).getAnOperand(), pred, c) and
         completion = c
         or
-        last(ASTInternal::toGenerated(ASTInternal::fromGenerated(succ)
-                .(AST::ParenthesizedExpr)
-                .getLastExpr()), pred, c) and
+        last(succ.(ParenthesizedExpr).getLastExpr(), pred, c) and
         completion = c
         or
-        last(ASTInternal::toGenerated(ASTInternal::fromGenerated(succ)
-                .(AST::ConditionalExpr)
-                .getBranch(_)), pred, c) and
+        last(succ.(ConditionalExpr).getBranch(_), pred, c) and
         completion = c
       )
     }
@@ -255,7 +244,7 @@ private module ConditionalCompletionSplitting {
 
     override predicate hasExitScope(CfgScope scope, AstNode last, Completion c) {
       this.appliesTo(last) and
-      succExit(ASTInternal::toGenerated(scope), last, c) and
+      succExit(scope, last, c) and
       if c instanceof ConditionalCompletion then completion = c else any()
     }
 
@@ -288,12 +277,11 @@ module EnsureSplitting {
 
   /** A node that belongs to an `ensure` block. */
   private class EnsureNode extends AstNode {
-    private Trees::RescueEnsureBlockTree block;
+    private Trees::BodyStmtTree block;
 
     EnsureNode() { this = block.getAnEnsureDescendant() }
 
-    /** Gets the immediate block that this node belongs to. */
-    Trees::RescueEnsureBlockTree getBlock() { result = block }
+    int getNestLevel() { result = block.getNestLevel() }
 
     /** Holds if this node is the entry node in the `ensure` block it belongs to. */
     predicate isEntryNode() { first(block.getEnsure(), this) }
@@ -366,7 +354,7 @@ module EnsureSplitting {
   pragma[noinline]
   private predicate hasEntry0(AstNode pred, EnsureNode succ, int nestLevel, Completion c) {
     succ.isEntryNode() and
-    nestLevel = succ.getBlock().nestLevel() and
+    nestLevel = succ.getNestLevel() and
     succ(pred, succ, c)
   }
 
@@ -389,11 +377,9 @@ module EnsureSplitting {
     }
 
     pragma[noinline]
-    private predicate exit0(
-      AstNode pred, Trees::RescueEnsureBlockTree block, int nestLevel, Completion c
-    ) {
+    private predicate exit0(AstNode pred, Trees::BodyStmtTree block, int nestLevel, Completion c) {
       this.appliesToPredecessor(pred) and
-      nestLevel = block.nestLevel() and
+      nestLevel = block.getNestLevel() and
       block.lastInner(pred, c)
     }
 
@@ -402,9 +388,7 @@ module EnsureSplitting {
      * `inherited` indicates whether `c` is an inherited completion from the
      * body.
      */
-    private predicate exit(
-      Trees::RescueEnsureBlockTree block, AstNode pred, Completion c, boolean inherited
-    ) {
+    private predicate exit(Trees::BodyStmtTree block, AstNode pred, Completion c, boolean inherited) {
       exists(EnsureSplitType type |
         exit0(pred, block, this.getNestLevel(), c) and
         type = this.getType()
@@ -472,7 +456,7 @@ module EnsureSplitting {
     }
 
     override predicate hasExitScope(CfgScope scope, AstNode last, Completion c) {
-      succExit(ASTInternal::toGenerated(scope), last, c) and
+      succExit(scope, last, c) and
       (
         exit(_, last, c, _)
         or
@@ -488,10 +472,10 @@ module EnsureSplitting {
           if en.isEntryNode()
           then
             // entering a nested `ensure` block
-            en.getBlock().nestLevel() > this.getNestLevel()
+            en.getNestLevel() > this.getNestLevel()
           else
             // staying in the same (possibly nested) `ensure` block as `pred`
-            en.getBlock().nestLevel() >= this.getNestLevel()
+            en.getNestLevel() >= this.getNestLevel()
         )
     }
   }
@@ -517,7 +501,7 @@ class Splits extends TSplits {
 
 private predicate succEntrySplitsFromRank(CfgScope pred, AstNode succ, Splits splits, int rnk) {
   splits = TSplitsNil() and
-  succEntry(ASTInternal::toGenerated(pred), succ) and
+  succEntry(pred, succ) and
   rnk = 0
   or
   exists(SplitImpl head, Splits tail | succEntrySplitsCons(pred, succ, head, tail, rnk) |
@@ -540,7 +524,7 @@ private predicate succEntrySplitsCons(
 pragma[noinline]
 predicate succEntrySplits(CfgScope pred, AstNode succ, Splits succSplits, SuccessorType t) {
   exists(int rnk |
-    succEntry(ASTInternal::toGenerated(pred), succ) and
+    succEntry(pred, succ) and
     t instanceof NormalSuccessor and
     succEntrySplitsFromRank(pred, succ, succSplits, rnk)
   |
@@ -559,7 +543,7 @@ predicate succExitSplits(AstNode last, Splits predSplits, CfgScope scope, Succes
   exists(Reachability::SameSplitsBlock b, Completion c | last = b.getANode() |
     b.isReachable(predSplits) and
     t = c.getAMatchingSuccessorType() and
-    succExit(ASTInternal::toGenerated(scope), last, c) and
+    succExit(scope, last, c) and
     forall(SplitImpl predSplit | predSplit = predSplits.getASplit() |
       predSplit.hasExitScope(scope, last, c)
     )
diff --git a/ql/src/codeql_ruby/dataflow/internal/SsaImpl.qll b/ql/src/codeql_ruby/dataflow/internal/SsaImpl.qll
index bdc3c813ca0..ad2d2932044 100644
--- a/ql/src/codeql_ruby/dataflow/internal/SsaImpl.qll
+++ b/ql/src/codeql_ruby/dataflow/internal/SsaImpl.qll
@@ -49,7 +49,7 @@ private predicate capturedExitRead(AnnotatedExitBasicBlock bb, int i, LocalVaria
   i = bb.length()
 }
 
-private CfgScope getCaptureOuterCfgScope(Callable scope) {
+private CfgScope getCaptureOuterCfgScope(CfgScope scope) {
   result = scope.getOuterCfgScope() and
   (
     scope instanceof Block
diff --git a/ql/test/library-tests/ast/calls/calls.ql b/ql/test/library-tests/ast/calls/calls.ql
index 32b1183317f..4d244c2cf4c 100644
--- a/ql/test/library-tests/ast/calls/calls.ql
+++ b/ql/test/library-tests/ast/calls/calls.ql
@@ -1,5 +1,4 @@
 import ruby
-import codeql_ruby.ast.internal.TreeSitter
 
 private string getMethodName(Call c) {
   result = c.(MethodCall).getMethodName()
diff --git a/ql/test/library-tests/ast/modules/toplevel.expected b/ql/test/library-tests/ast/modules/toplevel.expected
index 0eca47938a3..9ea0f83b3df 100644
--- a/ql/test/library-tests/ast/modules/toplevel.expected
+++ b/ql/test/library-tests/ast/modules/toplevel.expected
@@ -3,6 +3,6 @@ toplevel
 | modules.rb:1:1:61:3 | modules.rb | Toplevel |
 | toplevel.rb:1:1:5:23 | toplevel.rb | Toplevel |
 beginBlocks
-| toplevel.rb:1:1:5:23 | toplevel.rb | 1 | toplevel.rb:5:1:5:22 | BEGIN { ... } |
+| toplevel.rb:1:1:5:23 | toplevel.rb | 0 | toplevel.rb:5:1:5:22 | BEGIN { ... } |
 endBlocks
 | toplevel.rb:1:1:5:23 | toplevel.rb | toplevel.rb:3:1:3:18 | END { ... } |
diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected
index 028f0d08b9f..5481f1a2439 100644
--- a/ql/test/library-tests/controlflow/graph/Cfg.expected
+++ b/ql/test/library-tests/controlflow/graph/Cfg.expected
@@ -38,8 +38,8 @@ break_ensure.rb:
 #-----|  -> In
 
 #    3| ... > ...
-#-----| false -> if ...
 #-----| true -> break
+#-----| false -> if ...
 
 #    3| element
 #-----|  -> 0
@@ -57,8 +57,8 @@ break_ensure.rb:
 #-----|  -> exit m1 (normal)
 
 #    8| call to nil?
-#-----| false -> if ...
 #-----| true -> "elements nil"
+#-----| false -> if ...
 
 #    8| elements
 #-----|  -> call to nil?
@@ -100,8 +100,8 @@ break_ensure.rb:
 #-----|  -> ensure ...
 
 #   16| ... > ...
-#-----| false -> if ...
 #-----| true -> break
+#-----| false -> if ...
 
 #   16| element
 #-----|  -> 0
@@ -125,12 +125,12 @@ break_ensure.rb:
 #-----| break -> for ... in ...
 
 #   20| call to nil?
-#-----| false -> if ...
 #-----| true -> "elements nil"
+#-----| false -> if ...
 
 #   20| [ensure: break] call to nil?
-#-----| false -> [ensure: break] if ...
 #-----| true -> [ensure: break] "elements nil"
+#-----| false -> [ensure: break] if ...
 
 #   20| elements
 #-----|  -> call to nil?
@@ -168,8 +168,8 @@ break_ensure.rb:
 #-----|  -> ensure ...
 
 #   29| call to nil?
-#-----| false -> if ...
 #-----| true -> return
+#-----| false -> if ...
 
 #   29| elements
 #-----|  -> call to nil?
@@ -216,12 +216,12 @@ break_ensure.rb:
 #-----|  -> [ensure: return] In
 
 #   35| ... > ...
-#-----| false -> if ...
 #-----| true -> break
+#-----| false -> if ...
 
 #   35| [ensure: return] ... > ...
-#-----| false -> [ensure: return] if ...
 #-----| true -> [ensure: return] break
+#-----| false -> [ensure: return] if ...
 
 #   35| call to x
 #-----|  -> 0
@@ -278,8 +278,8 @@ break_ensure.rb:
 #-----|  -> ensure ...
 
 #   47| ... > ...
-#-----| false -> if ...
 #-----| true -> ""
+#-----| false -> if ...
 
 #   47| element
 #-----|  -> 1
@@ -306,12 +306,12 @@ break_ensure.rb:
 #-----| raise -> for ... in ...
 
 #   51| ... > ...
-#-----| false -> if ...
 #-----| true -> 10
+#-----| false -> if ...
 
 #   51| [ensure: raise] ... > ...
-#-----| false -> [ensure: raise] if ...
 #-----| true -> [ensure: raise] 10
+#-----| false -> [ensure: raise] if ...
 
 #   51| element
 #-----|  -> 0
@@ -377,8 +377,8 @@ case.rb:
 #-----|  -> (if ...)
 
 #    3| call to x2
-#-----| false -> if ...
 #-----| true -> "x2"
+#-----| false -> if ...
 
 #    3| call to puts
 #-----|  -> if ...
@@ -401,7 +401,7 @@ case.rb:
 
 cfg.rb:
 #    1| enter cfg.rb
-#-----|  -> bar
+#-----|  -> BEGIN { ... }
 
 #    1| bar
 #-----|  -> alias ...
@@ -460,24 +460,16 @@ cfg.rb:
 #-----|  -> %w(...)
 
 #   12| call to puts
-#-----|  -> BEGIN { ... }
+#-----|  -> END { ... }
 
 #   12| 4
 #-----|  -> call to puts
 
-#   15| enter BEGIN { ... }
+#   15| BEGIN { ... }
 #-----|  -> "hello"
 
-#   15| BEGIN { ... }
-#-----|  -> END { ... }
-
-#   15| exit BEGIN { ... }
-
-#   15| exit BEGIN { ... } (normal)
-#-----|  -> exit BEGIN { ... }
-
 #   16| call to puts
-#-----|  -> exit BEGIN { ... } (normal)
+#-----|  -> bar
 
 #   16| "hello"
 #-----|  -> call to puts
@@ -560,9 +552,12 @@ cfg.rb:
 #   29| exit { ... } (normal)
 #-----|  -> exit { ... }
 
-#   29| x
+#   29| &x
 #-----|  -> x
 
+#   29| x
+#-----|  -> &x
+
 #   29| call to call
 #-----|  -> exit { ... } (normal)
 
@@ -587,12 +582,12 @@ cfg.rb:
 #   35| false
 #-----| false -> if ...
 
-#   39| call to puts
-#-----|  -> case ...
-
 #   39| self
 #-----|  -> 42
 
+#   39| call to puts
+#-----|  -> case ...
+
 #   39| 42
 #-----|  -> call to puts
 
@@ -619,12 +614,12 @@ cfg.rb:
 #-----|  -> 2
 
 #   43| 2
-#-----| no-match -> 3
 #-----| match -> "some"
+#-----| no-match -> 3
 
 #   43| 3
-#-----| no-match -> 4
 #-----| match -> "some"
+#-----| no-match -> 4
 
 #   43| 4
 #-----| match -> "some"
@@ -668,8 +663,8 @@ cfg.rb:
 #-----|  -> b
 
 #   49| ... == ...
-#-----| false -> b
 #-----| true -> "some"
+#-----| false -> b
 
 #   49| b
 #-----|  -> 0
@@ -775,6 +770,9 @@ cfg.rb:
 #   62| ... = ...
 #-----|  -> pattern
 
+#   62| (..., ...)
+#-----|  -> (..., ...)
+
 #   62| (..., ...)
 #-----|  -> ... = ...
 
@@ -987,8 +985,8 @@ cfg.rb:
 #-----|  -> x
 
 #   91| ... > ...
-#-----| false -> if ...
 #-----| true -> next
+#-----| false -> if ...
 
 #   91| x
 #-----|  -> 3
@@ -1023,21 +1021,21 @@ cfg.rb:
 #   97| {...}
 #-----|  -> map1
 
-#   97| Pair
-#-----|  -> "c"
-
 #   97| "a"
 #-----|  -> "b"
 
+#   97| Pair
+#-----|  -> "c"
+
 #   97| "b"
 #-----|  -> Pair
 
-#   97| Pair
-#-----|  -> :e
-
 #   97| "c"
 #-----|  -> "d"
 
+#   97| Pair
+#-----|  -> :e
+
 #   97| "d"
 #-----|  -> Pair
 
@@ -1065,12 +1063,12 @@ cfg.rb:
 #   98| map1
 #-----|  -> **...
 
-#   98| Pair
-#-----|  -> map1
-
 #   98| "x"
 #-----|  -> "y"
 
+#   98| Pair
+#-----|  -> map1
+
 #   98| "y"
 #-----|  -> Pair
 
@@ -1101,9 +1099,12 @@ cfg.rb:
 #  101| key
 #-----|  -> kwargs
 
-#  101| kwargs
+#  101| **kwargs
 #-----|  -> value
 
+#  101| kwargs
+#-----|  -> **kwargs
+
 #  102| call to puts
 #-----|  -> kwargs
 
@@ -1161,18 +1162,18 @@ cfg.rb:
 #  110| call to type
 #-----|  -> #{...}
 
-#  113| ... if ...
-#-----|  -> C
-
 #  113| call to puts
 #-----|  -> ... if ...
 
+#  113| ... if ...
+#-----|  -> C
+
 #  113| "hi"
 #-----|  -> call to puts
 
 #  113| ... > ...
-#-----| false -> ... if ...
 #-----| true -> "hi"
+#-----| false -> ... if ...
 
 #  113| b
 #-----|  -> 10
@@ -1372,6 +1373,9 @@ cfg.rb:
 #  135| ... = ...
 #-----|  -> M
 
+#  135| (..., ...)
+#-----|  -> (..., ...)
+
 #  135| (..., ...)
 #-----|  -> ... = ...
 
@@ -1390,14 +1394,11 @@ cfg.rb:
 #  135| 3
 #-----|  -> init
 
-#  137| Constant
-#-----|  -> M
-
 #  137| M
 #-----|  -> Constant
 
-#  138| Constant
-#-----|  -> class << ...
+#  137| Constant
+#-----|  -> M
 
 #  138| call to itself
 #-----|  -> Constant
@@ -1405,6 +1406,9 @@ cfg.rb:
 #  138| M
 #-----|  -> call to itself
 
+#  138| Constant
+#-----|  -> class << ...
+
 #  140| class << ...
 #-----|  -> Silly
 
@@ -1469,9 +1473,12 @@ cfg.rb:
 #  149| silly
 #-----|  -> method
 
-#  149| x
+#  149| *x
 #-----|  -> x
 
+#  149| x
+#-----|  -> *x
+
 #  150| call to puts
 #-----|  -> exit method (normal)
 
@@ -1619,18 +1626,18 @@ cfg.rb:
 #  169| "bye"
 #-----|  -> call to puts
 
-#  171| ... unless ...
-#-----|  -> x
-
 #  171| call to puts
 #-----|  -> ... unless ...
 
+#  171| ... unless ...
+#-----|  -> x
+
 #  171| "hi"
 #-----|  -> call to puts
 
 #  171| ... == ...
-#-----| true -> ... unless ...
 #-----| false -> "hi"
+#-----| true -> ... unless ...
 
 #  171| x
 #-----|  -> 0
@@ -1675,12 +1682,12 @@ cfg.rb:
 #  175| 0
 #-----|  -> i
 
-#  176| ... until ...
-#-----|  -> 0
-
 #  176| (...; ...)
 #-----|  -> i
 
+#  176| ... until ...
+#-----|  -> 0
+
 #  176| call to puts
 #-----|  -> i
 
@@ -1697,8 +1704,8 @@ cfg.rb:
 #-----|  -> ... += ...
 
 #  176| ... == ...
-#-----| true -> ... until ...
 #-----| false -> "hello"
+#-----| true -> ... until ...
 
 #  176| i
 #-----|  -> 10
@@ -1719,8 +1726,8 @@ cfg.rb:
 #-----|  -> i
 
 #  179| ... < ...
-#-----| false -> while ...
 #-----| true -> x
+#-----| false -> while ...
 
 #  179| x
 #-----|  -> 10
@@ -1741,8 +1748,8 @@ cfg.rb:
 #-----|  -> x
 
 #  181| ... == ...
-#-----| false -> if ...
 #-----| true -> redo
+#-----| false -> if ...
 
 #  181| x
 #-----|  -> 5
@@ -1759,12 +1766,12 @@ cfg.rb:
 #  182| x
 #-----|  -> call to puts
 
-#  185| ... while ...
-#-----|  -> run_block
-
 #  185| (...; ...)
 #-----|  -> i
 
+#  185| ... while ...
+#-----|  -> run_block
+
 #  185| call to puts
 #-----|  -> i
 
@@ -1781,8 +1788,8 @@ cfg.rb:
 #-----|  -> ... -= ...
 
 #  185| ... != ...
-#-----| false -> ... while ...
 #-----| true -> "hello"
+#-----| false -> ... while ...
 
 #  185| i
 #-----|  -> 0
@@ -1796,10 +1803,16 @@ cfg.rb:
 #  187| run_block
 #-----|  -> { ... }
 
+#  187| exit run_block
+
+#  187| exit run_block (normal)
+#-----|  -> exit run_block
+
 #  188| yield ...
 #-----|  -> 42
 
 #  188| 42
+#-----|  -> exit run_block (normal)
 
 #  191| call to run_block
 #-----|  -> exit cfg.rb (normal)
@@ -1854,8 +1867,8 @@ exit.rb:
 #-----|  -> "x <= 2"
 
 #    2| ... > ...
-#-----| false -> if ...
 #-----| true -> 1
+#-----| false -> if ...
 
 #    2| x
 #-----|  -> 2
@@ -1896,8 +1909,8 @@ exit.rb:
 #-----|  -> "x <= 2"
 
 #    9| ... > ...
-#-----| false -> if ...
 #-----| true -> "abort!"
+#-----| false -> if ...
 
 #    9| x
 #-----|  -> 2
@@ -2075,8 +2088,8 @@ ifs.rb:
 #-----|  -> 1
 
 #   12| b
-#-----| false -> if ...
 #-----| true -> 0
+#-----| false -> if ...
 
 #   13| return
 #-----| return -> exit m2 (normal)
@@ -2295,12 +2308,12 @@ ifs.rb:
 #   36| enter conditional_method_def
 #-----|  -> "bla"
 
-#   36| ... unless ...
-#-----|  -> constant_condition
-
 #   36| conditional_method_def
 #-----|  -> ... unless ...
 
+#   36| ... unless ...
+#-----|  -> constant_condition
+
 #   36| exit conditional_method_def
 
 #   36| exit conditional_method_def (normal)
@@ -2313,8 +2326,8 @@ ifs.rb:
 #-----|  -> call to puts
 
 #   38| ... == ...
-#-----| true -> ... unless ...
 #-----| false -> conditional_method_def
+#-----| true -> ... unless ...
 
 #   38| 1
 #-----|  -> 2
@@ -2369,8 +2382,8 @@ loops.rb:
 #-----|  -> exit m1 (normal)
 
 #    2| ... >= ...
-#-----| false -> while ...
 #-----| true -> x
+#-----| false -> while ...
 
 #    2| x
 #-----|  -> 0
@@ -2411,8 +2424,8 @@ loops.rb:
 #-----|  -> "Done"
 
 #    9| ... >= ...
-#-----| false -> while ...
 #-----| true -> x
+#-----| false -> while ...
 
 #    9| x
 #-----|  -> 0
@@ -2471,8 +2484,8 @@ loops.rb:
 #-----|  -> elsif ...
 
 #   16| ... > ...
-#-----| false -> elsif ...
 #-----| true -> redo
+#-----| false -> elsif ...
 
 #   16| x
 #-----|  -> 10
@@ -2506,12 +2519,12 @@ loops.rb:
 #   24| exit m3 (normal)
 #-----|  -> exit m3
 
-#   25| call to each
-#-----|  -> exit m3 (normal)
-
 #   25| [...]
 #-----|  -> do ... end
 
+#   25| call to each
+#-----|  -> exit m3 (normal)
+
 #   25| 1
 #-----|  -> 2
 
@@ -2583,8 +2596,8 @@ raise.rb:
 #-----|  -> "x <= 2"
 
 #    8| ... > ...
-#-----| false -> if ...
 #-----| true -> "x > 2"
+#-----| false -> if ...
 
 #    8| x
 #-----|  -> 2
@@ -2875,8 +2888,8 @@ raise.rb:
 #-----|  -> if ...
 
 #   71| ... < ...
-#-----| false -> elsif ...
 #-----| true -> "x < 0"
+#-----| false -> elsif ...
 
 #   71| x
 #-----|  -> 0
@@ -2969,8 +2982,8 @@ raise.rb:
 #-----|  -> if ...
 
 #   84| ... < ...
-#-----| false -> elsif ...
 #-----| true -> "x < 0"
+#-----| false -> elsif ...
 
 #   84| x
 #-----|  -> 0
@@ -3075,8 +3088,8 @@ raise.rb:
 #-----|  -> if ...
 
 #   99| ... < ...
-#-----| false -> elsif ...
 #-----| true -> "x < 0"
+#-----| false -> elsif ...
 
 #   99| x
 #-----|  -> 0
@@ -3133,16 +3146,16 @@ raise.rb:
 #-----|  -> [ensure: raise] ensure ...
 
 #  106| b1
-#-----| false -> if ...
 #-----| true -> "b1 is true"
+#-----| false -> if ...
 
 #  106| [ensure: return] b1
-#-----| false -> [ensure: return] if ...
 #-----| true -> [ensure: return] "b1 is true"
+#-----| false -> [ensure: return] if ...
 
 #  106| [ensure: raise] b1
-#-----| false -> [ensure: raise] if ...
 #-----| true -> [ensure: raise] "b1 is true"
+#-----| false -> [ensure: raise] if ...
 
 #  107| call to raise
 #-----| raise -> [ensure(1): raise] ensure ...
@@ -3259,16 +3272,16 @@ raise.rb:
 #-----| raise -> exit m9 (abnormal)
 
 #  116| b2
-#-----| false -> if ...
 #-----| true -> "b2 is true"
+#-----| false -> if ...
 
 #  116| [ensure: return] b2
-#-----| false -> [ensure: return] if ...
 #-----| true -> [ensure: return] "b2 is true"
+#-----| false -> [ensure: return] if ...
 
 #  116| [ensure: raise] b2
-#-----| false -> [ensure: raise] if ...
 #-----| true -> [ensure: raise] "b2 is true"
+#-----| false -> [ensure: raise] if ...
 
 #  117| call to raise
 #-----| raise -> exit m9 (abnormal)
@@ -3306,7 +3319,11 @@ raise.rb:
 #-----| no-match -> "Exception"
 #-----| match -> ensure ...
 
+#  121| (call to raise)
+#-----|  -> ensure ...
+
 #  121| call to raise
+#-----| raise -> (call to raise)
 #-----| raise -> exit m10 (abnormal)
 
 #  121| "Exception"
@@ -3413,8 +3430,8 @@ raise.rb:
 #-----|  -> ensure ...
 
 #  143| b
-#-----| false -> if ...
 #-----| true -> ""
+#-----| false -> if ...
 
 #  144| call to raise
 #-----| raise -> [ensure: raise] ensure ...
@@ -3491,18 +3508,18 @@ raise.rb:
 #  155| elem
 #-----|  -> element
 
-#  155| ... if ...
-#-----|  -> exit { ... } (normal)
-
 #  155| call to raise
 #-----| raise -> exit { ... } (abnormal)
 
+#  155| ... if ...
+#-----|  -> exit { ... } (normal)
+
 #  155| ""
 #-----|  -> call to raise
 
 #  155| call to nil?
-#-----| false -> ... if ...
 #-----| true -> ""
+#-----| false -> ... if ...
 
 #  155| element
 #-----|  -> call to nil?

From d4030c66d8372981b436aefc1a5bd257568b1f89 Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Thu, 18 Mar 2021 09:54:44 +0100
Subject: [PATCH 03/20] Update Consistency.qll

---
 ql/src/codeql_ruby/controlflow/internal/Consistency.qll | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ql/src/codeql_ruby/controlflow/internal/Consistency.qll b/ql/src/codeql_ruby/controlflow/internal/Consistency.qll
index ccec52fd56e..0b8e07dd77c 100644
--- a/ql/src/codeql_ruby/controlflow/internal/Consistency.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/Consistency.qll
@@ -1,4 +1,4 @@
-private import codeql_ruby.ast.internal.TreeSitter::Generated
+private import codeql_ruby.AST
 private import codeql_ruby.CFG
 private import Completion
 private import Splitting

From 3bb2c529a59c116f8dafc3feba2e56a51812c4a9 Mon Sep 17 00:00:00 2001
From: Tom Hvitved <hvitved@github.com>
Date: Thu, 18 Mar 2021 10:43:10 +0100
Subject: [PATCH 04/20] CFG: Revert change to mandatory parameters

---
 .../internal/ControlFlowGraphImpl.qll         | 38 ++++++++++++-------
 .../controlflow/graph/Cfg.expected            | 15 ++------
 2 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
index c864d1aee40..119d2c4e7cf 100644
--- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
@@ -380,12 +380,24 @@ module Trees {
     final override ControlFlowTree getChildNode(int i) { result = this.getValue() and i = 0 }
   }
 
-  private class BlockParameterTree extends StandardPostOrderTree, BlockParameter {
-    final override ControlFlowTree getChildNode(int i) {
-      result = this.getVariable().getDefiningAccess() and i = 0
+  abstract private class MandatoryParameterTree extends ControlFlowTree, NamedParameter {
+    final override predicate first(AstNode first) {
+      this.getDefiningAccess().(ControlFlowTree).first(first)
     }
+
+    final override predicate last(AstNode last, Completion c) {
+      this.getDefiningAccess().(ControlFlowTree).last(last, c)
+    }
+
+    override predicate propagatesAbnormal(AstNode child) {
+      this.getDefiningAccess().(ControlFlowTree).propagatesAbnormal(child)
+    }
+
+    final override predicate succ(AstNode pred, AstNode succ, Completion c) { none() }
   }
 
+  private class BlockParameterTree extends MandatoryParameterTree, BlockParameter { }
+
   /**
    * TODO: make all StmtSequence tree classes post-order, and simplify class
    * hierarchy.
@@ -868,11 +880,7 @@ module Trees {
     final override ControlFlowTree getChildNode(int i) { result = this.getValue() and i = 0 }
   }
 
-  private class HashSplatParameterTree extends StandardPostOrderTree, HashSplatParameter {
-    final override ControlFlowTree getChildNode(int i) {
-      result = this.getVariable().getDefiningAccess() and i = 0
-    }
-  }
+  private class HashSplatParameterTree extends MandatoryParameterTree, HashSplatParameter { }
 
   private class HereDocTree extends StandardPreOrderTree, HereDoc {
     final override ControlFlowTree getChildNode(int i) { result = this.getComponent(i) }
@@ -1121,7 +1129,13 @@ module Trees {
 
   private class SelfTree extends LeafTree, Self { }
 
-  private class SimpleParameterTree extends LeafTree, SimpleParameter { }
+  private class SimpleParameterTree extends MandatoryParameterTree, SimpleParameter { }
+
+  // Corner case: For duplicated '_' parameters, only the first occurence has a defining
+  // access. For subsequent parameters we simply include the parameter itself in the CFG
+  private class SimpleParameterTreeDupUnderscore extends LeafTree, SimpleParameter {
+    SimpleParameterTreeDupUnderscore() { not exists(this.getDefiningAccess()) }
+  }
 
   /**
    * Control-flow tree for any post-order StmtSequence that doesn't have a more
@@ -1215,11 +1229,7 @@ module Trees {
     final override ControlFlowTree getChildNode(int i) { result = this.getValue() and i = 0 }
   }
 
-  private class SplatParameterTree extends StandardPostOrderTree, SplatParameter {
-    final override ControlFlowTree getChildNode(int i) {
-      result = this.getVariable().getDefiningAccess() and i = 0
-    }
-  }
+  private class SplatParameterTree extends MandatoryParameterTree, SplatParameter { }
 
   abstract class StmtSequenceTree extends ControlFlowTree, StmtSequence {
     override predicate propagatesAbnormal(AstNode child) { none() }
diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected
index 5481f1a2439..40746ef4ce4 100644
--- a/ql/test/library-tests/controlflow/graph/Cfg.expected
+++ b/ql/test/library-tests/controlflow/graph/Cfg.expected
@@ -552,11 +552,8 @@ cfg.rb:
 #   29| exit { ... } (normal)
 #-----|  -> exit { ... }
 
-#   29| &x
-#-----|  -> x
-
 #   29| x
-#-----|  -> &x
+#-----|  -> x
 
 #   29| call to call
 #-----|  -> exit { ... } (normal)
@@ -1099,11 +1096,8 @@ cfg.rb:
 #  101| key
 #-----|  -> kwargs
 
-#  101| **kwargs
-#-----|  -> value
-
 #  101| kwargs
-#-----|  -> **kwargs
+#-----|  -> value
 
 #  102| call to puts
 #-----|  -> kwargs
@@ -1473,11 +1467,8 @@ cfg.rb:
 #  149| silly
 #-----|  -> method
 
-#  149| *x
-#-----|  -> x
-
 #  149| x
-#-----|  -> *x
+#-----|  -> x
 
 #  150| call to puts
 #-----|  -> exit method (normal)

From c8eab42c1db8ff5c8d1902a751913791a6b370d2 Mon Sep 17 00:00:00 2001
From: Nick Rolfe <nickrolfe@github.com>
Date: Thu, 18 Mar 2021 11:09:21 +0000
Subject: [PATCH 05/20] Minor comment fixes

---
 .../codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll  | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
index 119d2c4e7cf..125c60296ef 100644
--- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
@@ -108,7 +108,7 @@ module CfgScope {
   }
 }
 
-abstract /*private*/ class ControlFlowTree extends AstNode {
+abstract private class ControlFlowTree extends AstNode {
   /**
    * Holds if `first` is the first element executed within this AST node.
    */
@@ -1003,7 +1003,6 @@ module Trees {
     final override AstNode getAccessNode() { result = this.getDefiningAccess() }
   }
 
-  /** Contains partridges */
   private class PairTree extends StandardPostOrderTree, Pair {
     final override ControlFlowTree getChildNode(int i) {
       result = this.getKey() and i = 0

From ceda7c8fd2a7795cb84876fcf75fa946512e6cc6 Mon Sep 17 00:00:00 2001
From: Nick Rolfe <nickrolfe@github.com>
Date: Thu, 18 Mar 2021 11:21:11 +0000
Subject: [PATCH 06/20] Generalise splitting of parenthesized exprs to all
 statement sequences

---
 ql/src/codeql_ruby/ast/Expr.qll                          | 4 ++--
 ql/src/codeql_ruby/controlflow/CfgNodes.qll              | 6 +++---
 ql/src/codeql_ruby/controlflow/internal/Completion.qll   | 2 +-
 ql/src/codeql_ruby/controlflow/internal/Splitting.qll    | 2 +-
 ql/src/codeql_ruby/dataflow/internal/DataFlowPrivate.qll | 2 +-
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/ql/src/codeql_ruby/ast/Expr.qll b/ql/src/codeql_ruby/ast/Expr.qll
index f584b7f96f7..6f27b0462bc 100644
--- a/ql/src/codeql_ruby/ast/Expr.qll
+++ b/ql/src/codeql_ruby/ast/Expr.qll
@@ -73,8 +73,8 @@ class StmtSequence extends Expr, TStmtSequence {
   /** Gets a statement in this sequence. */
   final Stmt getAStmt() { result = this.getStmt(_) }
 
-  /** Gets the last expression in this sequence, if any. */
-  final Expr getLastExpr() { result = this.getStmt(this.getNumberOfStatements() - 1) }
+  /** Gets the last statement in this sequence, if any. */
+  final Stmt getLastStmt() { result = this.getStmt(this.getNumberOfStatements() - 1) }
 
   /** Gets the number of statements in this sequence. */
   final int getNumberOfStatements() { result = count(this.getAStmt()) }
diff --git a/ql/src/codeql_ruby/controlflow/CfgNodes.qll b/ql/src/codeql_ruby/controlflow/CfgNodes.qll
index d67849934fc..3a80fffbae4 100644
--- a/ql/src/codeql_ruby/controlflow/CfgNodes.qll
+++ b/ql/src/codeql_ruby/controlflow/CfgNodes.qll
@@ -284,7 +284,7 @@ module ExprNodes {
   }
 
   private class StmtSequenceChildMapping extends ExprChildMapping, StmtSequence {
-    override predicate relevantChild(Expr e) { e = this.getLastExpr() }
+    override predicate relevantChild(Expr e) { e = this.getLastStmt() }
   }
 
   /** A control-flow node that wraps a `StmtSequence` AST expression. */
@@ -293,8 +293,8 @@ module ExprNodes {
 
     final override StmtSequence getExpr() { result = ExprCfgNode.super.getExpr() }
 
-    /** Gets the last expression in this sequence, if any. */
-    final ExprCfgNode getLastExpr() { e.hasCfgChild(e.getLastExpr(), this, result) }
+    /** Gets the last statement in this sequence, if any. */
+    final ExprCfgNode getLastStmt() { e.hasCfgChild(e.getLastStmt(), this, result) }
   }
 
   private class ForExprChildMapping extends ExprChildMapping, ForExpr {
diff --git a/ql/src/codeql_ruby/controlflow/internal/Completion.qll b/ql/src/codeql_ruby/controlflow/internal/Completion.qll
index b25098b2a06..dc80bbb98f2 100644
--- a/ql/src/codeql_ruby/controlflow/internal/Completion.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/Completion.qll
@@ -176,7 +176,7 @@ private predicate inBooleanContext(AstNode n) {
   or
   n = any(NotExpr parent | inBooleanContext(parent)).getOperand()
   or
-  n = any(StmtSequence parent | inBooleanContext(parent)).getLastExpr()
+  n = any(StmtSequence parent | inBooleanContext(parent)).getLastStmt()
   or
   exists(CaseExpr c, WhenExpr w |
     not exists(c.getValue()) and
diff --git a/ql/src/codeql_ruby/controlflow/internal/Splitting.qll b/ql/src/codeql_ruby/controlflow/internal/Splitting.qll
index 7a81548ed5d..8543c64f055 100644
--- a/ql/src/codeql_ruby/controlflow/internal/Splitting.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/Splitting.qll
@@ -226,7 +226,7 @@ private module ConditionalCompletionSplitting {
         last(succ.(LogicalOrExpr).getAnOperand(), pred, c) and
         completion = c
         or
-        last(succ.(ParenthesizedExpr).getLastExpr(), pred, c) and
+        last(succ.(StmtSequence).getLastStmt(), pred, c) and
         completion = c
         or
         last(succ.(ConditionalExpr).getBranch(_), pred, c) and
diff --git a/ql/src/codeql_ruby/dataflow/internal/DataFlowPrivate.qll b/ql/src/codeql_ruby/dataflow/internal/DataFlowPrivate.qll
index 222fa8ac7bb..2352f9c4693 100644
--- a/ql/src/codeql_ruby/dataflow/internal/DataFlowPrivate.qll
+++ b/ql/src/codeql_ruby/dataflow/internal/DataFlowPrivate.qll
@@ -126,7 +126,7 @@ private module Cached {
     or
     nodeFrom.asExpr() = nodeTo.asExpr().(CfgNodes::ExprNodes::AssignExprCfgNode).getRhs()
     or
-    nodeFrom.asExpr() = nodeTo.asExpr().(CfgNodes::ExprNodes::StmtSequenceCfgNode).getLastExpr()
+    nodeFrom.asExpr() = nodeTo.asExpr().(CfgNodes::ExprNodes::StmtSequenceCfgNode).getLastStmt()
     or
     nodeFrom.asExpr() = nodeTo.asExpr().(CfgNodes::ExprNodes::ConditionalExprCfgNode).getBranch(_)
     or

From 4ce7faf868bc0b755862493730d5362612c7e3e3 Mon Sep 17 00:00:00 2001
From: Nick Rolfe <nickrolfe@github.com>
Date: Thu, 18 Mar 2021 13:01:16 +0000
Subject: [PATCH 07/20] Fix erroneous flow from 'raise' call to StmtSequence

---
 .../codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll | 3 ++-
 ql/test/library-tests/controlflow/graph/Cfg.expected          | 4 ----
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
index 125c60296ef..4a551dddd86 100644
--- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
@@ -1254,7 +1254,8 @@ module Trees {
       or
       this instanceof PostOrderTree and
       succ = this and
-      last(this.getLastBodyChild(), pred, c)
+      last(this.getLastBodyChild(), pred, c) and
+      c instanceof NormalCompletion
       or
       // Normal left-to-right evaluation in the body
       exists(int i |
diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected
index 40746ef4ce4..a3136e61ede 100644
--- a/ql/test/library-tests/controlflow/graph/Cfg.expected
+++ b/ql/test/library-tests/controlflow/graph/Cfg.expected
@@ -3310,11 +3310,7 @@ raise.rb:
 #-----| no-match -> "Exception"
 #-----| match -> ensure ...
 
-#  121| (call to raise)
-#-----|  -> ensure ...
-
 #  121| call to raise
-#-----| raise -> (call to raise)
 #-----| raise -> exit m10 (abnormal)
 
 #  121| "Exception"

From 434d9e54a18c011445cf01cfa929a6720f08471a Mon Sep 17 00:00:00 2001
From: Nick Rolfe <nickrolfe@github.com>
Date: Thu, 18 Mar 2021 14:48:08 +0000
Subject: [PATCH 08/20] Fix complex symbols having multiple ControlFlowTree
 implementations

---
 .../controlflow/internal/ControlFlowGraphImpl.qll           | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
index 4a551dddd86..613b8687596 100644
--- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
@@ -969,7 +969,7 @@ module Trees {
     }
   }
 
-  private class MethodNameTree extends LeafTree, MethodName { }
+  private class MethodNameTree extends LeafTree, MethodName, ASTInternal::TTokenMethodName { }
 
   private class MethodTree extends BodyStmtTree, PostOrderTree, Method {
     final override predicate first(AstNode first) { first = this }
@@ -1276,6 +1276,10 @@ module Trees {
     override predicate isHidden() { any() }
   }
 
+  private class StringEscapeSequenceComponentTree extends LeafTree, StringEscapeSequenceComponent {
+    override predicate isHidden() { any() }
+  }
+
   private class StringlikeLiteralTree extends StandardPostOrderTree, StringlikeLiteral {
     StringlikeLiteralTree() { not this instanceof HereDoc }
 

From 37435764a0cb41adeb0769c0be1a288937c4ba53 Mon Sep 17 00:00:00 2001
From: Nick Rolfe <nickrolfe@github.com>
Date: Thu, 18 Mar 2021 18:58:40 +0000
Subject: [PATCH 09/20] Fix control-flow for empty classes and modules

---
 .../internal/ControlFlowGraphImpl.qll         | 68 +++++++++----------
 1 file changed, 33 insertions(+), 35 deletions(-)

diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
index 613b8687596..9ffbd14ab79 100644
--- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
@@ -359,9 +359,7 @@ module Trees {
     }
   }
 
-  private class BeginTree extends BodyStmtTree, PreOrderTree, BeginExpr {
-    final override predicate last(AstNode last, Completion c) { this.lastInner(last, c) }
-
+  private class BeginTree extends BodyStmtPreOrderTree, BeginExpr {
     override predicate isHidden() { any() }
   }
 
@@ -437,7 +435,7 @@ module Trees {
       not exists(this.getRescue(_)) and
       this.lastEnsure0(last, c)
       or
-      last([this.getEnsure().(AstNode), this.getBodyChild(_, false)], last, c) and
+      last([this.getEnsure(), this.getBodyChild(_, false)], last, c) and
       not c instanceof NormalCompletion
     }
 
@@ -585,6 +583,20 @@ module Trees {
     }
   }
 
+  abstract class BodyStmtPreOrderTree extends BodyStmtTree, PreOrderTree {
+    final override predicate last(AstNode last, Completion c) {
+      this.lastInner(last, c)
+      or
+      not exists(this.getAChild(_)) and
+      last = this and
+      isValidFor(c, this)
+    }
+  }
+
+  abstract class BodyStmtPostOrderTree extends BodyStmtTree, PostOrderTree {
+    override predicate first(AstNode first) { first = this }
+  }
+
   private class BooleanLiteralTree extends LeafTree, BooleanLiteral { }
 
   class BraceBlockTree extends ScopeTree, BraceBlock {
@@ -638,7 +650,7 @@ module Trees {
 
   private class CharacterTree extends LeafTree, CharacterLiteral { }
 
-  private class ClassTree extends BodyStmtTree, PreOrderTree, Class {
+  private class ClassTree extends BodyStmtPreOrderTree, Class {
     /** Gets the `i`th child in the body of this block. */
     final override AstNode getBodyChild(int i, boolean rescuable) {
       result = this.getScopeExpr() and i = 0 and rescuable = false
@@ -650,8 +662,6 @@ module Trees {
       result = this.getStmt(i - count(this.getScopeExpr()) - count(this.getSuperclassExpr())) and
       rescuable = true
     }
-
-    final override predicate last(AstNode last, Completion c) { this.lastInner(last, c) }
   }
 
   private class ClassVariableTree extends LeafTree, ClassVariableAccess { }
@@ -770,14 +780,12 @@ module Trees {
     }
   }
 
-  private class DoBlockTree extends BodyStmtTree, PostOrderTree, DoBlock {
-    final override predicate first(AstNode first) { first = this }
-
+  private class DoBlockTree extends BodyStmtPostOrderTree, DoBlock {
     /** Gets the `i`th child in the body of this block. */
     final override AstNode getBodyChild(int i, boolean rescuable) {
       result = this.getParameter(i) and rescuable = false
       or
-      result = BodyStmtTree.super.getBodyChild(i - this.getNumberOfParameters(), rescuable)
+      result = BodyStmtPostOrderTree.super.getBodyChild(i - this.getNumberOfParameters(), rescuable)
     }
   }
 
@@ -894,14 +902,12 @@ module Trees {
     final override AstNode getAccessNode() { result = this.getDefiningAccess() }
   }
 
-  private class LambdaTree extends BodyStmtTree, PostOrderTree, Lambda {
-    final override predicate first(AstNode first) { first = this }
-
+  private class LambdaTree extends BodyStmtPostOrderTree, Lambda {
     /** Gets the `i`th child in the body of this block. */
     final override AstNode getBodyChild(int i, boolean rescuable) {
       result = this.getParameter(i) and rescuable = false
       or
-      result = BodyStmtTree.super.getBodyChild(i - this.getNumberOfParameters(), rescuable)
+      result = BodyStmtPostOrderTree.super.getBodyChild(i - this.getNumberOfParameters(), rescuable)
     }
   }
 
@@ -971,26 +977,22 @@ module Trees {
 
   private class MethodNameTree extends LeafTree, MethodName, ASTInternal::TTokenMethodName { }
 
-  private class MethodTree extends BodyStmtTree, PostOrderTree, Method {
-    final override predicate first(AstNode first) { first = this }
-
+  private class MethodTree extends BodyStmtPostOrderTree, Method {
     /** Gets the `i`th child in the body of this block. */
     final override AstNode getBodyChild(int i, boolean rescuable) {
       result = this.getParameter(i) and rescuable = false
       or
-      result = BodyStmtTree.super.getBodyChild(i - this.getNumberOfParameters(), rescuable)
+      result = BodyStmtPostOrderTree.super.getBodyChild(i - this.getNumberOfParameters(), rescuable)
     }
   }
 
-  private class ModuleTree extends BodyStmtTree, PreOrderTree, Module {
+  private class ModuleTree extends BodyStmtPreOrderTree, Module {
     /** Gets the `i`th child in the body of this block. */
     final override AstNode getBodyChild(int i, boolean rescuable) {
       result = this.getScopeExpr() and i = 0 and rescuable = false
       or
-      result = BodyStmtTree.super.getBodyChild(i - count(this.getScopeExpr()), rescuable)
+      result = BodyStmtPreOrderTree.super.getBodyChild(i - count(this.getScopeExpr()), rescuable)
     }
-
-    final override predicate last(AstNode last, Completion c) { this.lastInner(last, c) }
   }
 
   private class NilTree extends LeafTree, NilLiteral { }
@@ -1192,31 +1194,29 @@ module Trees {
     }
   }
 
-  private class SingletonClassTree extends BodyStmtTree, PreOrderTree, SingletonClass {
+  private class SingletonClassTree extends BodyStmtPreOrderTree, SingletonClass {
     /** Gets the `i`th child in the body of this block. */
     final override AstNode getBodyChild(int i, boolean rescuable) {
       (
         result = this.getValue() and i = 0 and rescuable = false
         or
-        result = BodyStmtTree.super.getBodyChild(i - 1, rescuable)
+        result = BodyStmtPreOrderTree.super.getBodyChild(i - 1, rescuable)
       )
     }
-
-    final override predicate last(AstNode last, Completion c) { this.lastInner(last, c) }
   }
 
-  private class SingletonMethodTree extends BodyStmtTree, PostOrderTree, SingletonMethod {
+  private class SingletonMethodTree extends BodyStmtPostOrderTree, SingletonMethod {
     /** Gets the `i`th child in the body of this block. */
     final override AstNode getBodyChild(int i, boolean rescuable) {
       result = this.getParameter(i) and rescuable = false
       or
-      result = BodyStmtTree.super.getBodyChild(i - this.getNumberOfParameters(), rescuable)
+      result = BodyStmtPostOrderTree.super.getBodyChild(i - this.getNumberOfParameters(), rescuable)
     }
 
-    final override predicate first(AstNode first) { first(this.getObject(), first) }
+    override predicate first(AstNode first) { first(this.getObject(), first) }
 
     override predicate succ(AstNode pred, AstNode succ, Completion c) {
-      BodyStmtTree.super.succ(pred, succ, c)
+      BodyStmtPostOrderTree.super.succ(pred, succ, c)
       or
       last(this.getObject(), pred, c) and
       succ = this and
@@ -1290,15 +1290,13 @@ module Trees {
     final override ControlFlowTree getChildNode(int i) { result = this.getArgument(i) }
   }
 
-  private class ToplevelTree extends BodyStmtTree, PreOrderTree, Toplevel {
+  private class ToplevelTree extends BodyStmtPreOrderTree, Toplevel {
     final override AstNode getBodyChild(int i, boolean rescuable) {
       result = this.getBeginBlock(i) and rescuable = true
       or
-      result = BodyStmtTree.super.getBodyChild(i - count(this.getABeginBlock()), rescuable)
+      result = BodyStmtPreOrderTree.super.getBodyChild(i - count(this.getABeginBlock()), rescuable)
     }
 
-    final override predicate last(AstNode last, Completion c) { this.lastInner(last, c) }
-
     override predicate isHidden() { any() }
   }
 

From 9493997e9d00c8a812409c07c4ab1faa54033a83 Mon Sep 17 00:00:00 2001
From: Nick Rolfe <nickrolfe@github.com>
Date: Thu, 18 Mar 2021 19:01:11 +0000
Subject: [PATCH 10/20] Make space in CFG test for two new lines in the middle

Commented out to make it easier to ignore the noise from line number
changes.
---
 .../controlflow/graph/Cfg.expected            | 376 +++++++++---------
 .../library-tests/controlflow/graph/cfg.rb    |   3 +
 2 files changed, 191 insertions(+), 188 deletions(-)

diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected
index a3136e61ede..b691caa8394 100644
--- a/ql/test/library-tests/controlflow/graph/Cfg.expected
+++ b/ql/test/library-tests/controlflow/graph/Cfg.expected
@@ -1345,487 +1345,487 @@ cfg.rb:
 #  130| 5
 #-----|  -> Constant
 
-#  133| ... rescue ...
+#  136| ... rescue ...
 #-----|  -> 1
 
-#  133| ... / ...
+#  136| ... / ...
 #-----| raise -> "div by zero"
 #-----|  -> 1
 
-#  133| 1
+#  136| 1
 #-----|  -> 0
 
-#  133| 0
+#  136| 0
 #-----|  -> ... / ...
 
-#  133| call to puts
+#  136| call to puts
 #-----|  -> 1
 
-#  133| "div by zero"
+#  136| "div by zero"
 #-----|  -> call to puts
 
-#  135| ... = ...
+#  138| ... = ...
 #-----|  -> M
 
-#  135| (..., ...)
+#  138| (..., ...)
 #-----|  -> (..., ...)
 
-#  135| (..., ...)
+#  138| (..., ...)
 #-----|  -> ... = ...
 
-#  135| init
+#  138| init
 #-----|  -> last
 
-#  135| last
+#  138| last
 #-----|  -> (..., ...)
 
-#  135| 1
+#  138| 1
 #-----|  -> 2
 
-#  135| 2
+#  138| 2
 #-----|  -> 3
 
-#  135| 3
+#  138| 3
 #-----|  -> init
 
-#  137| M
+#  140| M
 #-----|  -> Constant
 
-#  137| Constant
+#  140| Constant
 #-----|  -> M
 
-#  138| call to itself
+#  141| call to itself
 #-----|  -> Constant
 
-#  138| M
+#  141| M
 #-----|  -> call to itself
 
-#  138| Constant
+#  141| Constant
 #-----|  -> class << ...
 
-#  140| class << ...
+#  143| class << ...
 #-----|  -> Silly
 
-#  140| call to itself
+#  143| call to itself
 #-----|  -> setter=
 
-#  140| Silly
+#  143| Silly
 #-----|  -> call to itself
 
-#  141| setter=
+#  144| setter=
 #-----|  -> print
 
-#  142| enter print
+#  145| enter print
 #-----|  -> "singleton"
 
-#  142| print
+#  145| print
 #-----|  -> Silly
 
-#  142| exit print
+#  145| exit print
 
-#  142| exit print (normal)
+#  145| exit print (normal)
 #-----|  -> exit print
 
-#  143| call to puts
+#  146| call to puts
 #-----|  -> call to super
 
-#  143| "singleton"
+#  146| "singleton"
 #-----|  -> call to puts
 
-#  144| call to puts
+#  147| call to puts
 #-----|  -> exit print (normal)
 
-#  144| call to print
+#  147| call to print
 #-----|  -> call to puts
 
-#  144| call to super
+#  147| call to super
 #-----|  -> call to print
 
-#  148| ... = ...
+#  151| ... = ...
 #-----|  -> silly
 
-#  148| silly
+#  151| silly
 #-----|  -> ... = ...
 
-#  148| call to new
+#  151| call to new
 #-----|  -> silly
 
-#  148| Silly
+#  151| Silly
 #-----|  -> call to new
 
-#  149| enter method
+#  152| enter method
 #-----|  -> x
 
-#  149| method
+#  152| method
 #-----|  -> two_parameters
 
-#  149| exit method
+#  152| exit method
 
-#  149| exit method (normal)
+#  152| exit method (normal)
 #-----|  -> exit method
 
-#  149| silly
+#  152| silly
 #-----|  -> method
 
-#  149| x
+#  152| x
 #-----|  -> x
 
-#  150| call to puts
+#  153| call to puts
 #-----|  -> exit method (normal)
 
-#  150| x
+#  153| x
 #-----|  -> call to puts
 
-#  153| enter two_parameters
+#  156| enter two_parameters
 #-----|  -> a
 
-#  153| two_parameters
+#  156| two_parameters
 #-----|  -> 1
 
-#  153| exit two_parameters
+#  156| exit two_parameters
 
-#  153| exit two_parameters (normal)
+#  156| exit two_parameters (normal)
 #-----|  -> exit two_parameters
 
-#  153| a
+#  156| a
 #-----|  -> b
 
-#  153| b
+#  156| b
 #-----|  -> exit two_parameters (normal)
 
-#  155| call to two_parameters
+#  158| call to two_parameters
 #-----|  -> call to __FILE__
 
-#  155| *...
+#  158| *...
 #-----|  -> call to two_parameters
 
-#  155| [...]
+#  158| [...]
 #-----|  -> *...
 
-#  155| 1
+#  158| 1
 #-----|  -> 2
 
-#  155| 2
+#  158| 2
 #-----|  -> [...]
 
-#  157| ... = ...
+#  160| ... = ...
 #-----|  -> :hello
 
-#  157| scriptfile
+#  160| scriptfile
 #-----|  -> ... = ...
 
-#  157| `cat "#{...}"`
+#  160| `cat "#{...}"`
 #-----|  -> scriptfile
 
-#  157| #{...}
+#  160| #{...}
 #-----|  -> `cat "#{...}"`
 
-#  157| call to __FILE__
+#  160| call to __FILE__
 #-----|  -> #{...}
 
-#  159| ... = ...
+#  162| ... = ...
 #-----|  -> 12
 
-#  159| symbol
+#  162| symbol
 #-----|  -> ... = ...
 
-#  159| :hello
+#  162| :hello
 #-----|  -> symbol
 
-#  161| ... = ...
+#  164| ... = ...
 #-----|  -> true
 
-#  161| delimited_symbol
+#  164| delimited_symbol
 #-----|  -> ... = ...
 
-#  161| :"goodbye-#{...}"
+#  164| :"goodbye-#{...}"
 #-----|  -> delimited_symbol
 
-#  161| #{...}
+#  164| #{...}
 #-----|  -> :"goodbye-#{...}"
 
-#  161| ... + ...
+#  164| ... + ...
 #-----|  -> #{...}
 
-#  161| 12
+#  164| 12
 #-----|  -> 13
 
-#  161| 13
+#  164| 13
 #-----|  -> ... + ...
 
-#  163| ... = ...
+#  166| ... = ...
 #-----|  -> true
 
-#  163| x
+#  166| x
 #-----|  -> ... = ...
 
-#  163| true
+#  166| true
 #-----|  -> x
 
-#  164| ... = ...
+#  167| ... = ...
 #-----|  -> 42
 
-#  164| x
+#  167| x
 #-----|  -> ... = ...
 
-#  164| ! ...
+#  167| ! ...
 #-----|  -> x
 
-#  164| true
+#  167| true
 #-----|  -> ! ...
 
-#  165| ... = ...
+#  168| ... = ...
 #-----|  -> undef ...
 
-#  165| x
+#  168| x
 #-----|  -> ... = ...
 
-#  165| - ...
+#  168| - ...
 #-----|  -> x
 
-#  165| 42
+#  168| 42
 #-----|  -> - ...
 
-#  167| undef ...
+#  170| undef ...
 #-----|  -> two_parameters
 
-#  167| two_parameters
+#  170| two_parameters
 #-----|  -> x
 
-#  169| unless ...
+#  172| unless ...
 #-----|  -> x
 
-#  169| ... == ...
+#  172| ... == ...
 #-----| false -> "hi"
 #-----| true -> "bye"
 
-#  169| x
+#  172| x
 #-----|  -> 10
 
-#  169| 10
+#  172| 10
 #-----|  -> ... == ...
 
-#  169| call to puts
+#  172| call to puts
 #-----|  -> unless ...
 
-#  169| "hi"
+#  172| "hi"
 #-----|  -> call to puts
 
-#  169| call to puts
+#  172| call to puts
 #-----|  -> unless ...
 
-#  169| "bye"
+#  172| "bye"
 #-----|  -> call to puts
 
-#  171| call to puts
+#  174| call to puts
 #-----|  -> ... unless ...
 
-#  171| ... unless ...
+#  174| ... unless ...
 #-----|  -> x
 
-#  171| "hi"
+#  174| "hi"
 #-----|  -> call to puts
 
-#  171| ... == ...
+#  174| ... == ...
 #-----| false -> "hi"
 #-----| true -> ... unless ...
 
-#  171| x
+#  174| x
 #-----|  -> 0
 
-#  171| 0
+#  174| 0
 #-----|  -> ... == ...
 
-#  173| until ...
+#  176| until ...
 #-----|  -> 0
 
-#  173| ... > ...
+#  176| ... > ...
 #-----| true -> until ...
 #-----| false -> x
 
-#  173| x
+#  176| x
 #-----|  -> 10
 
-#  173| 10
+#  176| 10
 #-----|  -> ... > ...
 
-#  173| ... += ...
+#  176| ... += ...
 #-----|  -> "hello"
 
-#  173| x
+#  176| x
 #-----|  -> 10
 
-#  173| 10
+#  176| 10
 #-----|  -> ... += ...
 
-#  173| call to puts
-#-----|  -> x
-
-#  173| "hello"
-#-----|  -> call to puts
-
-#  175| ... = ...
-#-----|  -> i
-
-#  175| i
-#-----|  -> ... = ...
-
-#  175| 0
-#-----|  -> i
-
-#  176| (...; ...)
-#-----|  -> i
-
-#  176| ... until ...
-#-----|  -> 0
-
 #  176| call to puts
-#-----|  -> i
+#-----|  -> x
 
 #  176| "hello"
 #-----|  -> call to puts
 
-#  176| ... += ...
-#-----|  -> (...; ...)
-
-#  176| i
-#-----|  -> 1
-
-#  176| 1
-#-----|  -> ... += ...
-
-#  176| ... == ...
-#-----| false -> "hello"
-#-----| true -> ... until ...
-
-#  176| i
-#-----|  -> 10
-
-#  176| 10
-#-----|  -> ... == ...
-
 #  178| ... = ...
-#-----|  -> x
+#-----|  -> i
 
-#  178| x
+#  178| i
 #-----|  -> ... = ...
 
 #  178| 0
-#-----|  -> x
-
-#  179| while ...
 #-----|  -> i
 
-#  179| ... < ...
-#-----| true -> x
-#-----| false -> while ...
+#  179| (...; ...)
+#-----|  -> i
 
-#  179| x
+#  179| ... until ...
+#-----|  -> 0
+
+#  179| call to puts
+#-----|  -> i
+
+#  179| "hello"
+#-----|  -> call to puts
+
+#  179| ... += ...
+#-----|  -> (...; ...)
+
+#  179| i
+#-----|  -> 1
+
+#  179| 1
+#-----|  -> ... += ...
+
+#  179| ... == ...
+#-----| false -> "hello"
+#-----| true -> ... until ...
+
+#  179| i
 #-----|  -> 10
 
 #  179| 10
+#-----|  -> ... == ...
+
+#  181| ... = ...
+#-----|  -> x
+
+#  181| x
+#-----|  -> ... = ...
+
+#  181| 0
+#-----|  -> x
+
+#  182| while ...
+#-----|  -> i
+
+#  182| ... < ...
+#-----| true -> x
+#-----| false -> while ...
+
+#  182| x
+#-----|  -> 10
+
+#  182| 10
 #-----|  -> ... < ...
 
-#  180| ... += ...
+#  183| ... += ...
 #-----|  -> x
 
-#  180| x
+#  183| x
 #-----|  -> 1
 
-#  180| 1
+#  183| 1
 #-----|  -> ... += ...
 
-#  181| if ...
+#  184| if ...
 #-----|  -> x
 
-#  181| ... == ...
+#  184| ... == ...
 #-----| true -> redo
 #-----| false -> if ...
 
-#  181| x
+#  184| x
 #-----|  -> 5
 
-#  181| 5
+#  184| 5
 #-----|  -> ... == ...
 
-#  181| redo
+#  184| redo
 #-----| redo -> x
 
-#  182| call to puts
+#  185| call to puts
 #-----|  -> x
 
-#  182| x
+#  185| x
 #-----|  -> call to puts
 
-#  185| (...; ...)
+#  188| (...; ...)
 #-----|  -> i
 
-#  185| ... while ...
+#  188| ... while ...
 #-----|  -> run_block
 
-#  185| call to puts
+#  188| call to puts
 #-----|  -> i
 
-#  185| "hello"
+#  188| "hello"
 #-----|  -> call to puts
 
-#  185| ... -= ...
+#  188| ... -= ...
 #-----|  -> (...; ...)
 
-#  185| i
+#  188| i
 #-----|  -> 1
 
-#  185| 1
+#  188| 1
 #-----|  -> ... -= ...
 
-#  185| ... != ...
+#  188| ... != ...
 #-----| true -> "hello"
 #-----| false -> ... while ...
 
-#  185| i
+#  188| i
 #-----|  -> 0
 
-#  185| 0
+#  188| 0
 #-----|  -> ... != ...
 
-#  187| enter run_block
+#  190| enter run_block
 #-----|  -> yield ...
 
-#  187| run_block
+#  190| run_block
 #-----|  -> { ... }
 
-#  187| exit run_block
+#  190| exit run_block
 
-#  187| exit run_block (normal)
+#  190| exit run_block (normal)
 #-----|  -> exit run_block
 
-#  188| yield ...
+#  191| yield ...
 #-----|  -> 42
 
-#  188| 42
+#  191| 42
 #-----|  -> exit run_block (normal)
 
-#  191| call to run_block
+#  194| call to run_block
 #-----|  -> exit cfg.rb (normal)
 
-#  191| enter { ... }
+#  194| enter { ... }
 #-----|  -> x
 
-#  191| { ... }
+#  194| { ... }
 #-----|  -> call to run_block
 
-#  191| exit { ... }
+#  194| exit { ... }
 
-#  191| exit { ... } (normal)
+#  194| exit { ... } (normal)
 #-----|  -> exit { ... }
 
-#  191| x
+#  194| x
 #-----|  -> x
 
-#  191| call to puts
+#  194| call to puts
 #-----|  -> exit { ... } (normal)
 
-#  191| x
+#  194| x
 #-----|  -> call to puts
 
 exit.rb:
diff --git a/ql/test/library-tests/controlflow/graph/cfg.rb b/ql/test/library-tests/controlflow/graph/cfg.rb
index ff3305df60a..65f6a927b10 100644
--- a/ql/test/library-tests/controlflow/graph/cfg.rb
+++ b/ql/test/library-tests/controlflow/graph/cfg.rb
@@ -130,6 +130,9 @@ module M
  Constant = 5
 end
 
+#class EmptyClass; end
+#module EmptyModule; end
+
 1/0 rescue puts "div by zero"
 
 (*init, last) = 1, 2, 3

From 6bcc433af31331e9b3023464427a2470160584f4 Mon Sep 17 00:00:00 2001
From: Nick Rolfe <nickrolfe@github.com>
Date: Thu, 18 Mar 2021 19:02:32 +0000
Subject: [PATCH 11/20] Uncomment empty class and module in CFG test

---
 ql/test/library-tests/controlflow/graph/Cfg.expected | 8 +++++++-
 ql/test/library-tests/controlflow/graph/cfg.rb       | 4 ++--
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected
index b691caa8394..7140bb4c6be 100644
--- a/ql/test/library-tests/controlflow/graph/Cfg.expected
+++ b/ql/test/library-tests/controlflow/graph/Cfg.expected
@@ -1337,7 +1337,7 @@ cfg.rb:
 #-----|  -> #{...}
 
 #  130| ... = ...
-#-----|  -> ... rescue ...
+#-----|  -> EmptyClass
 
 #  130| Constant
 #-----|  -> ... = ...
@@ -1345,6 +1345,12 @@ cfg.rb:
 #  130| 5
 #-----|  -> Constant
 
+#  133| EmptyClass
+#-----|  -> EmptyModule
+
+#  134| EmptyModule
+#-----|  -> ... rescue ...
+
 #  136| ... rescue ...
 #-----|  -> 1
 
diff --git a/ql/test/library-tests/controlflow/graph/cfg.rb b/ql/test/library-tests/controlflow/graph/cfg.rb
index 65f6a927b10..aa43ec6c9ae 100644
--- a/ql/test/library-tests/controlflow/graph/cfg.rb
+++ b/ql/test/library-tests/controlflow/graph/cfg.rb
@@ -130,8 +130,8 @@ module M
  Constant = 5
 end
 
-#class EmptyClass; end
-#module EmptyModule; end
+class EmptyClass; end
+module EmptyModule; end
 
 1/0 rescue puts "div by zero"
 

From c0636bef29097b02928c0cad2b1da9dd24d2f458 Mon Sep 17 00:00:00 2001
From: Nick Rolfe <nickrolfe@github.com>
Date: Thu, 18 Mar 2021 19:08:51 +0000
Subject: [PATCH 12/20] Make CfgScope extend Scope

---
 ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll b/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll
index 30e75576397..519d2b30dad 100644
--- a/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll
+++ b/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll
@@ -9,7 +9,7 @@ private import internal.Splitting
 private import internal.Completion
 
 /** An AST node with an associated control-flow graph. */
-class CfgScope extends AstNode {
+class CfgScope extends Scope {
   CfgScope() { this instanceof CfgScope::Range_ }
 
   /** Gets the CFG scope that this scope is nested under, if any. */

From e17551329341f4a0469132784fc4d91965d4650f Mon Sep 17 00:00:00 2001
From: Tom Hvitved <hvitved@github.com>
Date: Fri, 19 Mar 2021 10:23:31 +0100
Subject: [PATCH 13/20] Remove duplicate tuple patterns

---
 ql/src/codeql_ruby/ast/Pattern.qll                   | 9 ++++++++-
 ql/src/codeql_ruby/ast/internal/AST.qll              | 4 +++-
 ql/test/library-tests/ast/Ast.expected               | 5 ++---
 ql/test/library-tests/ast/control/Loop.expected      | 3 ++-
 ql/test/library-tests/controlflow/graph/Cfg.expected | 6 ------
 5 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/ql/src/codeql_ruby/ast/Pattern.qll b/ql/src/codeql_ruby/ast/Pattern.qll
index 8cf30a3ea4b..3f5d335ec05 100644
--- a/ql/src/codeql_ruby/ast/Pattern.qll
+++ b/ql/src/codeql_ruby/ast/Pattern.qll
@@ -63,7 +63,14 @@ class TuplePattern extends Pattern, TTuplePattern {
     or
     result = toGenerated(this).(Generated::DestructuredLeftAssignment).getChild(i)
     or
-    result = toGenerated(this).(Generated::LeftAssignmentList).getChild(i)
+    toGenerated(this) =
+      any(Generated::LeftAssignmentList lal |
+        if
+          strictcount(int j | exists(lal.getChild(j))) = 1 and
+          lal.getChild(0) instanceof Generated::DestructuredLeftAssignment
+        then result = lal.getChild(0).(Generated::DestructuredLeftAssignment).getChild(i)
+        else result = lal.getChild(i)
+      )
   }
 
   /** Gets the `i`th pattern in this tuple pattern. */
diff --git a/ql/src/codeql_ruby/ast/internal/AST.qll b/ql/src/codeql_ruby/ast/internal/AST.qll
index 6cf9696c510..37ef071d507 100644
--- a/ql/src/codeql_ruby/ast/internal/AST.qll
+++ b/ql/src/codeql_ruby/ast/internal/AST.qll
@@ -90,7 +90,9 @@ private module Cached {
     TComplexLiteral(Generated::Complex g) or
     TDefinedExpr(Generated::Unary g) { g instanceof @unary_definedquestion } or
     TDelimitedSymbolLiteral(Generated::DelimitedSymbol g) or
-    TDestructuredLeftAssignment(Generated::DestructuredLeftAssignment g) or
+    TDestructuredLeftAssignment(Generated::DestructuredLeftAssignment g) {
+      not strictcount(int i | exists(g.getParent().(Generated::LeftAssignmentList).getChild(i))) = 1
+    } or
     TDivExpr(Generated::Binary g) { g instanceof @binary_slash } or
     TDo(Generated::Do g) or
     TDoBlock(Generated::DoBlock g) { not g.getParent() instanceof Generated::Lambda } or
diff --git a/ql/test/library-tests/ast/Ast.expected b/ql/test/library-tests/ast/Ast.expected
index 246512d1ec6..8b74d275522 100644
--- a/ql/test/library-tests/ast/Ast.expected
+++ b/ql/test/library-tests/ast/Ast.expected
@@ -1185,9 +1185,8 @@ control/loops.rb:
 #   24|         getAnOperand/getRightOperand: [LocalVariableAccess] value
 #   28|   getStmt: [ForExpr] for ... in ...
 #   28|     getPattern: [TuplePattern] (..., ...)
-#   28|       getElement: [TuplePattern] (..., ...)
-#   28|         getElement: [LocalVariableAccess] key
-#   28|         getElement: [LocalVariableAccess] value
+#   28|       getElement: [LocalVariableAccess] key
+#   28|       getElement: [LocalVariableAccess] value
 #   28|     <in>: [???] In
 #   28|     getValue: [HashLiteral] {...}
 #   28|       getElement: [Pair] Pair
diff --git a/ql/test/library-tests/ast/control/Loop.expected b/ql/test/library-tests/ast/control/Loop.expected
index f93e73f5ee8..6ed5a497ec4 100644
--- a/ql/test/library-tests/ast/control/Loop.expected
+++ b/ql/test/library-tests/ast/control/Loop.expected
@@ -29,7 +29,8 @@ forExprs
 forExprsTuplePatterns
 | loops.rb:22:1:25:3 | for ... in ... | loops.rb:22:5:22:14 | (..., ...) | 0 | loops.rb:22:5:22:7 | key |
 | loops.rb:22:1:25:3 | for ... in ... | loops.rb:22:5:22:14 | (..., ...) | 1 | loops.rb:22:10:22:14 | value |
-| loops.rb:28:1:32:3 | for ... in ... | loops.rb:28:5:28:16 | (..., ...) | 0 | loops.rb:28:5:28:16 | (..., ...) |
+| loops.rb:28:1:32:3 | for ... in ... | loops.rb:28:5:28:16 | (..., ...) | 0 | loops.rb:28:6:28:8 | key |
+| loops.rb:28:1:32:3 | for ... in ... | loops.rb:28:5:28:16 | (..., ...) | 1 | loops.rb:28:11:28:15 | value |
 whileExprs
 | loops.rb:35:1:39:3 | while ... | loops.rb:35:7:35:11 | ... < ... | loops.rb:35:12:39:3 | ...; ... | 0 | loops.rb:36:3:36:8 | ... += ... |
 | loops.rb:35:1:39:3 | while ... | loops.rb:35:7:35:11 | ... < ... | loops.rb:35:12:39:3 | ...; ... | 1 | loops.rb:37:3:37:8 | ... += ... |
diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected
index 7140bb4c6be..18ff3d01381 100644
--- a/ql/test/library-tests/controlflow/graph/Cfg.expected
+++ b/ql/test/library-tests/controlflow/graph/Cfg.expected
@@ -767,9 +767,6 @@ cfg.rb:
 #   62| ... = ...
 #-----|  -> pattern
 
-#   62| (..., ...)
-#-----|  -> (..., ...)
-
 #   62| (..., ...)
 #-----|  -> ... = ...
 
@@ -1373,9 +1370,6 @@ cfg.rb:
 #  138| ... = ...
 #-----|  -> M
 
-#  138| (..., ...)
-#-----|  -> (..., ...)
-
 #  138| (..., ...)
 #-----|  -> ... = ...
 

From 5cedf7ee86185f0ffb027bc7f9628a78db9c718a Mon Sep 17 00:00:00 2001
From: Nick Rolfe <nickrolfe@github.com>
Date: Fri, 19 Mar 2021 13:59:02 +0000
Subject: [PATCH 14/20] Remove unused import

---
 ql/src/codeql_ruby/controlflow/CfgNodes.qll | 1 -
 1 file changed, 1 deletion(-)

diff --git a/ql/src/codeql_ruby/controlflow/CfgNodes.qll b/ql/src/codeql_ruby/controlflow/CfgNodes.qll
index 3a80fffbae4..10449ed7c91 100644
--- a/ql/src/codeql_ruby/controlflow/CfgNodes.qll
+++ b/ql/src/codeql_ruby/controlflow/CfgNodes.qll
@@ -1,7 +1,6 @@
 /** Provides classes representing nodes in a control flow graph. */
 
 private import codeql_ruby.AST
-private import codeql_ruby.ast.internal.AST
 private import codeql_ruby.controlflow.BasicBlocks
 private import ControlFlowGraph
 private import internal.ControlFlowGraphImpl

From f381f94bc2a2968f757c8c29a87f9312262e8442 Mon Sep 17 00:00:00 2001
From: Nick Rolfe <nickrolfe@github.com>
Date: Fri, 19 Mar 2021 14:02:54 +0000
Subject: [PATCH 15/20] Rename ProgramScope to ToplevelScope

---
 .../codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
index 9ffbd14ab79..c3e7d79d64d 100644
--- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
@@ -49,7 +49,7 @@ module CfgScope {
     abstract predicate exit(AstNode last, Completion c);
   }
 
-  private class ProgramScope extends Range_, Toplevel {
+  private class ToplevelScope extends Range_, Toplevel {
     final override predicate entry(AstNode first) { first(this, first) }
 
     final override predicate exit(AstNode last, Completion c) { last(this, last, c) }

From c6958f64e49409f6f8b9acb4f7d8136fefaa4e1d Mon Sep 17 00:00:00 2001
From: Nick Rolfe <nickrolfe@github.com>
Date: Fri, 19 Mar 2021 14:25:38 +0000
Subject: [PATCH 16/20] Make CFG for AssignExpr visit left operand before right

---
 .../internal/ControlFlowGraphImpl.qll         |   4 +-
 .../controlflow/graph/Cfg.expected            | 266 +++++++++---------
 2 files changed, 135 insertions(+), 135 deletions(-)

diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
index c3e7d79d64d..241fe949329 100644
--- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
@@ -353,9 +353,9 @@ module Trees {
 
   private class AssignmentTree extends StandardPostOrderTree, AssignExpr {
     final override ControlFlowTree getChildNode(int i) {
-      result = this.getRightOperand() and i = 0
+      result = this.getLeftOperand() and i = 0
       or
-      result = this.getLeftOperand() and i = 1
+      result = this.getRightOperand() and i = 1
     }
   }
 
diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected
index 18ff3d01381..b585b8ac4f0 100644
--- a/ql/test/library-tests/controlflow/graph/Cfg.expected
+++ b/ql/test/library-tests/controlflow/graph/Cfg.expected
@@ -418,16 +418,16 @@ cfg.rb:
 #-----|  -> bar
 
 #    3| bar
-#-----|  -> 42
+#-----|  -> b
 
 #    5| ... = ...
 #-----|  -> b
 
 #    5| b
-#-----|  -> ... = ...
+#-----|  -> 42
 
 #    5| 42
-#-----|  -> b
+#-----|  -> ... = ...
 
 #    7| %i(...)
 #-----|  -> b
@@ -651,7 +651,7 @@ cfg.rb:
 #-----|  -> ... == ...
 
 #   48| call to puts
-#-----|  -> "a"
+#-----|  -> chained
 
 #   48| "one"
 #-----|  -> call to puts
@@ -671,7 +671,7 @@ cfg.rb:
 
 #   49| ... > ...
 #-----| true -> "some"
-#-----| false -> "a"
+#-----| false -> chained
 
 #   49| b
 #-----|  -> 1
@@ -680,16 +680,16 @@ cfg.rb:
 #-----|  -> ... > ...
 
 #   49| call to puts
-#-----|  -> "a"
+#-----|  -> chained
 
 #   49| "some"
 #-----|  -> call to puts
 
 #   52| ... = ...
-#-----|  -> ?\x40
+#-----|  -> character
 
 #   52| chained
-#-----|  -> ... = ...
+#-----|  -> "a"
 
 #   52| "a"
 #-----|  -> chained
@@ -704,40 +704,40 @@ cfg.rb:
 #-----|  -> #{...}
 
 #   52| "string"
-#-----|  -> chained
+#-----|  -> ... = ...
 
 #   54| ... = ...
 #-----|  -> Silly
 
 #   54| character
-#-----|  -> ... = ...
+#-----|  -> ?\x40
 
 #   54| ?\x40
-#-----|  -> character
+#-----|  -> ... = ...
 
 #   58| Silly
 #-----|  -> Object
 
 #   58| Object
-#-----|  -> 10-2i
-
-#   59| ... = ...
-#-----|  -> call to b
-
-#   59| complex
-#-----|  -> ... = ...
-
-#   59| 10-2i
 #-----|  -> complex
 
-#   60| ... = ...
-#-----|  -> "constant"
+#   59| ... = ...
+#-----|  -> conditional
 
-#   60| conditional
+#   59| complex
+#-----|  -> 10-2i
+
+#   59| 10-2i
 #-----|  -> ... = ...
 
+#   60| ... = ...
+#-----|  -> C
+
+#   60| conditional
+#-----|  -> call to b
+
 #   60| ... ? ... : ...
-#-----|  -> conditional
+#-----|  -> ... = ...
 
 #   60| ... < ...
 #-----| true -> "hello"
@@ -756,19 +756,19 @@ cfg.rb:
 #-----|  -> ... ? ... : ...
 
 #   61| ... = ...
-#-----|  -> 1
+#-----|  -> x
 
 #   61| C
-#-----|  -> ... = ...
+#-----|  -> "constant"
 
 #   61| "constant"
-#-----|  -> C
+#-----|  -> ... = ...
 
 #   62| ... = ...
 #-----|  -> pattern
 
 #   62| (..., ...)
-#-----|  -> ... = ...
+#-----|  -> 1
 
 #   62| x
 #-----|  -> y
@@ -783,7 +783,7 @@ cfg.rb:
 #-----|  -> (..., ...)
 
 #   62| [...]
-#-----|  -> x
+#-----|  -> ... = ...
 
 #   62| 1
 #-----|  -> 2
@@ -801,7 +801,7 @@ cfg.rb:
 #-----|  -> a
 
 #   63| pattern
-#-----|  -> 1
+#-----|  -> items
 
 #   63| exit pattern
 
@@ -833,10 +833,10 @@ cfg.rb:
 #-----|  -> items
 
 #   67| items
-#-----|  -> ... = ...
+#-----|  -> 1
 
 #   67| [...]
-#-----|  -> items
+#-----|  -> ... = ...
 
 #   67| 1
 #-----|  -> 2
@@ -863,7 +863,7 @@ cfg.rb:
 #-----|  -> "silly"
 
 #   69| print
-#-----|  -> 42
+#-----|  -> x
 
 #   69| exit print
 
@@ -880,10 +880,10 @@ cfg.rb:
 #-----|  -> x
 
 #   74| x
-#-----|  -> ... = ...
+#-----|  -> 42
 
 #   74| 42
-#-----|  -> x
+#-----|  -> ... = ...
 
 #   75| if ...
 #-----|  -> ;
@@ -933,7 +933,7 @@ cfg.rb:
 #-----|  -> "end"
 
 #   85| call to puts
-#-----|  -> x
+#-----|  -> escape
 
 #   85| "end"
 #-----|  -> call to puts
@@ -942,10 +942,10 @@ cfg.rb:
 #-----|  -> 1.4
 
 #   88| escape
-#-----|  -> ... = ...
+#-----|  -> x
 
 #   88| "\u1234#{...}\n"
-#-----|  -> escape
+#-----|  -> ... = ...
 
 #   88| #{...}
 #-----|  -> "\u1234#{...}\n"
@@ -954,7 +954,7 @@ cfg.rb:
 #-----|  -> #{...}
 
 #   90| for ... in ...
-#-----|  -> 42
+#-----|  -> $global
 
 #   90| x
 #-----|  -> x
@@ -998,22 +998,22 @@ cfg.rb:
 #-----|  -> call to puts
 
 #   95| ... = ...
-#-----|  -> "a"
+#-----|  -> map1
 
 #   95| $global
-#-----|  -> ... = ...
+#-----|  -> 42
 
 #   95| 42
-#-----|  -> $global
-
-#   97| ... = ...
-#-----|  -> map1
-
-#   97| map1
 #-----|  -> ... = ...
 
+#   97| ... = ...
+#-----|  -> map2
+
+#   97| map1
+#-----|  -> "a"
+
 #   97| {...}
-#-----|  -> map1
+#-----|  -> ... = ...
 
 #   97| "a"
 #-----|  -> "b"
@@ -1046,10 +1046,10 @@ cfg.rb:
 #-----|  -> parameters
 
 #   98| map2
-#-----|  -> ... = ...
+#-----|  -> map1
 
 #   98| {...}
-#-----|  -> map2
+#-----|  -> ... = ...
 
 #   98| **...
 #-----|  -> "x"
@@ -1076,7 +1076,7 @@ cfg.rb:
 #-----|  -> value
 
 #  101| parameters
-#-----|  -> "healthy"
+#-----|  -> type
 
 #  101| exit parameters
 
@@ -1115,22 +1115,22 @@ cfg.rb:
 #-----|  -> ...[...]
 
 #  106| ... = ...
-#-----|  -> "food"
+#-----|  -> table
 
 #  106| type
-#-----|  -> ... = ...
+#-----|  -> "healthy"
 
 #  106| "healthy"
-#-----|  -> type
+#-----|  -> ... = ...
 
 #  107| ... = ...
 #-----|  -> <<SQL
 
 #  107| table
-#-----|  -> ... = ...
+#-----|  -> "food"
 
 #  107| "food"
-#-----|  -> table
+#-----|  -> ... = ...
 
 #  108| call to puts
 #-----|  -> b
@@ -1173,37 +1173,37 @@ cfg.rb:
 #-----|  -> ... > ...
 
 #  115| C
-#-----|  -> 42
-
-#  116| ... = ...
-#-----|  -> 10
-
-#  116| @field
-#-----|  -> ... = ...
-
-#  116| 42
 #-----|  -> @field
 
-#  117| ... = ...
-#-----|  -> -> { ... }
+#  116| ... = ...
+#-----|  -> @@static_field
 
-#  117| @@static_field
+#  116| @field
+#-----|  -> 42
+
+#  116| 42
 #-----|  -> ... = ...
 
+#  117| ... = ...
+#-----|  -> swap
+
+#  117| @@static_field
+#-----|  -> 10
+
 #  117| 10
-#-----|  -> @@static_field
+#-----|  -> ... = ...
 
 #  120| ... = ...
 #-----|  -> M
 
 #  120| swap
-#-----|  -> ... = ...
+#-----|  -> -> { ... }
 
 #  120| enter -> { ... }
 #-----|  -> x
 
 #  120| -> { ... }
-#-----|  -> swap
+#-----|  -> ... = ...
 
 #  120| exit -> { ... }
 
@@ -1229,28 +1229,28 @@ cfg.rb:
 #-----|  -> [...]
 
 #  122| M
-#-----|  -> nil
+#-----|  -> nothing
 
 #  123| ... = ...
-#-----|  -> 2
+#-----|  -> some
 
 #  123| nothing
-#-----|  -> ... = ...
+#-----|  -> nil
 
 #  123| nil
-#-----|  -> nothing
+#-----|  -> ... = ...
 
 #  124| ... = ...
 #-----|  -> some
 
 #  124| some
-#-----|  -> ... = ...
+#-----|  -> 2
 
 #  124| 2
-#-----|  -> some
+#-----|  -> ... = ...
 
 #  125| ... += ...
-#-----|  -> 2
+#-----|  -> last
 
 #  125| some
 #-----|  -> 10
@@ -1259,13 +1259,13 @@ cfg.rb:
 #-----|  -> ... += ...
 
 #  126| ... = ...
-#-----|  -> 0
+#-----|  -> range
 
 #  126| last
-#-----|  -> ... = ...
+#-----|  -> 2
 
 #  126| (...; ...)
-#-----|  -> last
+#-----|  -> ... = ...
 
 #  126| 2
 #-----|  -> 4
@@ -1277,13 +1277,13 @@ cfg.rb:
 #-----|  -> (...; ...)
 
 #  127| ... = ...
-#-----|  -> 1
+#-----|  -> half
 
 #  127| range
-#-----|  -> ... = ...
+#-----|  -> 0
 
 #  127| _ .. _
-#-----|  -> range
+#-----|  -> ... = ...
 
 #  127| 0
 #-----|  -> 9
@@ -1292,13 +1292,13 @@ cfg.rb:
 #-----|  -> _ .. _
 
 #  128| ... = ...
-#-----|  -> range
+#-----|  -> regex
 
 #  128| half
-#-----|  -> ... = ...
+#-----|  -> 1
 
 #  128| ... + ...
-#-----|  -> half
+#-----|  -> ... = ...
 
 #  128| ... / ...
 #-----|  -> 1
@@ -1319,13 +1319,13 @@ cfg.rb:
 #-----|  -> ... / ...
 
 #  129| ... = ...
-#-----|  -> 5
+#-----|  -> Constant
 
 #  129| regex
-#-----|  -> ... = ...
+#-----|  -> range
 
 #  129| /hello\s+[#{...}]/
-#-----|  -> regex
+#-----|  -> ... = ...
 
 #  129| #{...}
 #-----|  -> /hello\s+[#{...}]/
@@ -1337,10 +1337,10 @@ cfg.rb:
 #-----|  -> EmptyClass
 
 #  130| Constant
-#-----|  -> ... = ...
+#-----|  -> 5
 
 #  130| 5
-#-----|  -> Constant
+#-----|  -> ... = ...
 
 #  133| EmptyClass
 #-----|  -> EmptyModule
@@ -1353,7 +1353,7 @@ cfg.rb:
 
 #  136| ... / ...
 #-----| raise -> "div by zero"
-#-----|  -> 1
+#-----|  -> init
 
 #  136| 1
 #-----|  -> 0
@@ -1362,7 +1362,7 @@ cfg.rb:
 #-----|  -> ... / ...
 
 #  136| call to puts
-#-----|  -> 1
+#-----|  -> init
 
 #  136| "div by zero"
 #-----|  -> call to puts
@@ -1371,7 +1371,7 @@ cfg.rb:
 #-----|  -> M
 
 #  138| (..., ...)
-#-----|  -> ... = ...
+#-----|  -> 1
 
 #  138| init
 #-----|  -> last
@@ -1386,7 +1386,7 @@ cfg.rb:
 #-----|  -> 3
 
 #  138| 3
-#-----|  -> init
+#-----|  -> ... = ...
 
 #  140| M
 #-----|  -> Constant
@@ -1419,7 +1419,7 @@ cfg.rb:
 #-----|  -> "singleton"
 
 #  145| print
-#-----|  -> Silly
+#-----|  -> silly
 
 #  145| exit print
 
@@ -1445,10 +1445,10 @@ cfg.rb:
 #-----|  -> silly
 
 #  151| silly
-#-----|  -> ... = ...
+#-----|  -> Silly
 
 #  151| call to new
-#-----|  -> silly
+#-----|  -> ... = ...
 
 #  151| Silly
 #-----|  -> call to new
@@ -1494,7 +1494,7 @@ cfg.rb:
 #-----|  -> exit two_parameters (normal)
 
 #  158| call to two_parameters
-#-----|  -> call to __FILE__
+#-----|  -> scriptfile
 
 #  158| *...
 #-----|  -> call to two_parameters
@@ -1509,13 +1509,13 @@ cfg.rb:
 #-----|  -> [...]
 
 #  160| ... = ...
-#-----|  -> :hello
+#-----|  -> symbol
 
 #  160| scriptfile
-#-----|  -> ... = ...
+#-----|  -> call to __FILE__
 
 #  160| `cat "#{...}"`
-#-----|  -> scriptfile
+#-----|  -> ... = ...
 
 #  160| #{...}
 #-----|  -> `cat "#{...}"`
@@ -1524,22 +1524,22 @@ cfg.rb:
 #-----|  -> #{...}
 
 #  162| ... = ...
-#-----|  -> 12
+#-----|  -> delimited_symbol
 
 #  162| symbol
-#-----|  -> ... = ...
+#-----|  -> :hello
 
 #  162| :hello
-#-----|  -> symbol
-
-#  164| ... = ...
-#-----|  -> true
-
-#  164| delimited_symbol
 #-----|  -> ... = ...
 
+#  164| ... = ...
+#-----|  -> x
+
+#  164| delimited_symbol
+#-----|  -> 12
+
 #  164| :"goodbye-#{...}"
-#-----|  -> delimited_symbol
+#-----|  -> ... = ...
 
 #  164| #{...}
 #-----|  -> :"goodbye-#{...}"
@@ -1554,23 +1554,23 @@ cfg.rb:
 #-----|  -> ... + ...
 
 #  166| ... = ...
-#-----|  -> true
+#-----|  -> x
 
 #  166| x
-#-----|  -> ... = ...
+#-----|  -> true
 
 #  166| true
-#-----|  -> x
-
-#  167| ... = ...
-#-----|  -> 42
-
-#  167| x
 #-----|  -> ... = ...
 
-#  167| ! ...
+#  167| ... = ...
 #-----|  -> x
 
+#  167| x
+#-----|  -> true
+
+#  167| ! ...
+#-----|  -> ... = ...
+
 #  167| true
 #-----|  -> ! ...
 
@@ -1578,10 +1578,10 @@ cfg.rb:
 #-----|  -> undef ...
 
 #  168| x
-#-----|  -> ... = ...
+#-----|  -> 42
 
 #  168| - ...
-#-----|  -> x
+#-----|  -> ... = ...
 
 #  168| 42
 #-----|  -> - ...
@@ -1637,7 +1637,7 @@ cfg.rb:
 #-----|  -> ... == ...
 
 #  176| until ...
-#-----|  -> 0
+#-----|  -> i
 
 #  176| ... > ...
 #-----| true -> until ...
@@ -1668,16 +1668,16 @@ cfg.rb:
 #-----|  -> i
 
 #  178| i
-#-----|  -> ... = ...
+#-----|  -> 0
 
 #  178| 0
-#-----|  -> i
+#-----|  -> ... = ...
 
 #  179| (...; ...)
 #-----|  -> i
 
 #  179| ... until ...
-#-----|  -> 0
+#-----|  -> x
 
 #  179| call to puts
 #-----|  -> i
@@ -1708,10 +1708,10 @@ cfg.rb:
 #-----|  -> x
 
 #  181| x
-#-----|  -> ... = ...
+#-----|  -> 0
 
 #  181| 0
-#-----|  -> x
+#-----|  -> ... = ...
 
 #  182| while ...
 #-----|  -> i
@@ -2125,10 +2125,10 @@ ifs.rb:
 #-----|  -> x
 
 #   20| x
-#-----|  -> ... = ...
+#-----|  -> x
 
 #   20| - ...
-#-----|  -> x
+#-----|  -> ... = ...
 
 #   20| x
 #-----|  -> - ...
@@ -2150,10 +2150,10 @@ ifs.rb:
 #-----|  -> if ...
 
 #   22| x
-#-----|  -> ... = ...
+#-----|  -> x
 
 #   22| ... - ...
-#-----|  -> x
+#-----|  -> ... = ...
 
 #   22| x
 #-----|  -> 1

From f37c862c92b791747fa92892d37a2d567bcbd3d0 Mon Sep 17 00:00:00 2001
From: Nick Rolfe <nickrolfe@github.com>
Date: Fri, 19 Mar 2021 14:27:29 +0000
Subject: [PATCH 17/20] Rename MandatoryParameterTree to
 NonDefaultValueParameterTree

---
 .../controlflow/internal/ControlFlowGraphImpl.qll      | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
index 241fe949329..5c6816e0287 100644
--- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
@@ -378,7 +378,7 @@ module Trees {
     final override ControlFlowTree getChildNode(int i) { result = this.getValue() and i = 0 }
   }
 
-  abstract private class MandatoryParameterTree extends ControlFlowTree, NamedParameter {
+  abstract private class NonDefaultValueParameterTree extends ControlFlowTree, NamedParameter {
     final override predicate first(AstNode first) {
       this.getDefiningAccess().(ControlFlowTree).first(first)
     }
@@ -394,7 +394,7 @@ module Trees {
     final override predicate succ(AstNode pred, AstNode succ, Completion c) { none() }
   }
 
-  private class BlockParameterTree extends MandatoryParameterTree, BlockParameter { }
+  private class BlockParameterTree extends NonDefaultValueParameterTree, BlockParameter { }
 
   /**
    * TODO: make all StmtSequence tree classes post-order, and simplify class
@@ -888,7 +888,7 @@ module Trees {
     final override ControlFlowTree getChildNode(int i) { result = this.getValue() and i = 0 }
   }
 
-  private class HashSplatParameterTree extends MandatoryParameterTree, HashSplatParameter { }
+  private class HashSplatParameterTree extends NonDefaultValueParameterTree, HashSplatParameter { }
 
   private class HereDocTree extends StandardPreOrderTree, HereDoc {
     final override ControlFlowTree getChildNode(int i) { result = this.getComponent(i) }
@@ -1130,7 +1130,7 @@ module Trees {
 
   private class SelfTree extends LeafTree, Self { }
 
-  private class SimpleParameterTree extends MandatoryParameterTree, SimpleParameter { }
+  private class SimpleParameterTree extends NonDefaultValueParameterTree, SimpleParameter { }
 
   // Corner case: For duplicated '_' parameters, only the first occurence has a defining
   // access. For subsequent parameters we simply include the parameter itself in the CFG
@@ -1228,7 +1228,7 @@ module Trees {
     final override ControlFlowTree getChildNode(int i) { result = this.getValue() and i = 0 }
   }
 
-  private class SplatParameterTree extends MandatoryParameterTree, SplatParameter { }
+  private class SplatParameterTree extends NonDefaultValueParameterTree, SplatParameter { }
 
   abstract class StmtSequenceTree extends ControlFlowTree, StmtSequence {
     override predicate propagatesAbnormal(AstNode child) { none() }

From 21192bf43cf29df4f7e4cdeac387354f21f33654 Mon Sep 17 00:00:00 2001
From: Nick Rolfe <nickrolfe@github.com>
Date: Fri, 19 Mar 2021 14:28:26 +0000
Subject: [PATCH 18/20] Remove outdated comment

---
 ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll | 1 -
 1 file changed, 1 deletion(-)

diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
index 5c6816e0287..822bd8ced4a 100644
--- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
@@ -1023,7 +1023,6 @@ module Trees {
 
   private class RedoStmtTree extends LeafTree, RedoStmt { }
 
-  /** A block that may contain `rescue`/`ensure`. */
   private class RescueModifierTree extends PreOrderTree, RescueModifierExpr {
     final override predicate propagatesAbnormal(AstNode child) { child = this.getHandler() }
 

From 7667606b89474b33e992a2253250df787d75a49e Mon Sep 17 00:00:00 2001
From: Nick Rolfe <nickrolfe@github.com>
Date: Fri, 19 Mar 2021 14:31:17 +0000
Subject: [PATCH 19/20] Replace some uses of Generated types

---
 .../controlflow/internal/ControlFlowGraphImpl.qll          | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
index 822bd8ced4a..bf1448acdc4 100644
--- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
@@ -1172,10 +1172,9 @@ module Trees {
     override predicate propagatesAbnormal(AstNode child) { child = this.getAStmt() }
 
     override predicate isHidden() {
-      // TODO: unhide, or avoid using Generated types
-      ASTInternal::toGenerated(this) instanceof Generated::Else or
-      ASTInternal::toGenerated(this) instanceof Generated::Then or
-      ASTInternal::toGenerated(this) instanceof Generated::Do
+      this instanceof ASTInternal::TElse or
+      this instanceof ASTInternal::TThen or
+      this instanceof ASTInternal::TDo
     }
 
     final AstNode getLastChildNode() { result = this.getStmt(this.getNumberOfStatements() - 1) }

From cf7ce911bccc4ed049216d07d80e57a24f5d69fc Mon Sep 17 00:00:00 2001
From: Nick Rolfe <nickrolfe@github.com>
Date: Fri, 19 Mar 2021 16:08:43 +0000
Subject: [PATCH 20/20] =?UTF-8?q?Combine=20CfgScope=20classes=20for=20Body?=
 =?UTF-8?q?Stmt=20=E2=88=A9=20Callable?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 .../internal/ControlFlowGraphImpl.qll         | 26 +------------------
 1 file changed, 1 insertion(+), 25 deletions(-)

diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
index bf1448acdc4..83091c2f87b 100644
--- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
@@ -65,23 +65,7 @@ module CfgScope {
     }
   }
 
-  private class MethodScope extends Range_, Method {
-    final override predicate entry(AstNode first) { this.(Trees::BodyStmtTree).firstInner(first) }
-
-    final override predicate exit(AstNode last, Completion c) {
-      this.(Trees::BodyStmtTree).lastInner(last, c)
-    }
-  }
-
-  private class SingletonMethodScope extends Range_, SingletonMethod {
-    final override predicate entry(AstNode first) { this.(Trees::BodyStmtTree).firstInner(first) }
-
-    final override predicate exit(AstNode last, Completion c) {
-      this.(Trees::BodyStmtTree).lastInner(last, c)
-    }
-  }
-
-  private class DoBlockScope extends Range_, DoBlock {
+  private class BodyStmtCallableScope extends Range_, ASTInternal::TBodyStmt, Callable {
     final override predicate entry(AstNode first) { this.(Trees::BodyStmtTree).firstInner(first) }
 
     final override predicate exit(AstNode last, Completion c) {
@@ -98,14 +82,6 @@ module CfgScope {
       last(this.(Trees::BraceBlockTree).getLastChildNode(), last, c)
     }
   }
-
-  private class LambdaScope extends Range_, Lambda {
-    final override predicate entry(AstNode first) { this.(Trees::BodyStmtTree).firstInner(first) }
-
-    final override predicate exit(AstNode last, Completion c) {
-      this.(Trees::BodyStmtTree).lastInner(last, c)
-    }
-  }
 }
 
 abstract private class ControlFlowTree extends AstNode {