From d623f47ba02754f41228d5e8947bc7ab322f444b Mon Sep 17 00:00:00 2001
From: Nick Rolfe <nickrolfe@github.com>
Date: Thu, 6 May 2021 15:36:25 +0100
Subject: [PATCH] Make ClassDeclarationTree post-order

---
 .../internal/ControlFlowGraphImpl.qll         | 16 ++++++++++++++-
 .../controlflow/graph/Cfg.expected            | 20 +++++++++----------
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
index 4cb35175592..75309f5abf9 100644
--- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
@@ -602,7 +602,21 @@ module Trees {
 
   private class CharacterTree extends LeafTree, CharacterLiteral { }
 
-  private class ClassDeclarationTree extends BodyStmtPreOrderTree, ClassDeclaration {
+  private class ClassDeclarationTree extends BodyStmtPostOrderTree, ClassDeclaration {
+    override predicate first(AstNode first) {
+      this.firstInner(first)
+      or
+      not exists(this.getAChild(_)) and
+      first = this
+    }
+
+    final override predicate succ(AstNode pred, AstNode succ, Completion c) {
+      BodyStmtPostOrderTree.super.succ(pred, succ, c)
+      or
+      succ = this and
+      this.lastInner(pred, c)
+    }
+
     /** 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
diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected
index 025f50e62e4..ffad06e886c 100644
--- a/ql/test/library-tests/controlflow/graph/Cfg.expected
+++ b/ql/test/library-tests/controlflow/graph/Cfg.expected
@@ -845,7 +845,7 @@ cfg.rb:
 #-----|  -> ... = ...
 
 #   54| ... = ...
-#-----|  -> Silly
+#-----|  -> Object
 
 #   54| character
 #-----|  -> ?\x40
@@ -854,7 +854,7 @@ cfg.rb:
 #-----|  -> ... = ...
 
 #   58| Silly
-#-----|  -> Object
+#-----|  -> x
 
 #   58| Object
 #-----|  -> complex
@@ -1013,7 +1013,7 @@ cfg.rb:
 #-----|  -> self
 
 #   69| print
-#-----|  -> x
+#-----|  -> Silly
 
 #   69| exit print
 
@@ -1346,7 +1346,7 @@ cfg.rb:
 #-----|  -> call to type
 
 #  113| ... if ...
-#-----|  -> C
+#-----|  -> @field
 
 #  113| call to puts
 #-----|  -> ... if ...
@@ -1368,7 +1368,7 @@ cfg.rb:
 #-----|  -> ... > ...
 
 #  115| C
-#-----|  -> @field
+#-----|  -> swap
 
 #  116| ... = ...
 #-----|  -> @@static_field
@@ -1380,7 +1380,7 @@ cfg.rb:
 #-----|  -> ... = ...
 
 #  117| ... = ...
-#-----|  -> swap
+#-----|  -> C
 
 #  117| @@static_field
 #-----|  -> 10
@@ -2979,7 +2979,7 @@ loops.rb:
 
 raise.rb:
 #    1| enter raise.rb
-#-----|  -> ExceptionA
+#-----|  -> Exception
 
 #    1| ExceptionA
 #-----|  -> Exception
@@ -2990,13 +2990,13 @@ raise.rb:
 #-----|  -> exit raise.rb
 
 #    1| Exception
-#-----|  -> ExceptionB
+#-----|  -> ExceptionA
 
 #    4| ExceptionB
-#-----|  -> Exception
+#-----|  -> m1
 
 #    4| Exception
-#-----|  -> m1
+#-----|  -> ExceptionB
 
 #    7| enter m1
 #-----|  -> x