Merge pull request #13509 from hvitved/cfg-pack

Convert shared CFG construction library to a parameterized module

ruby/ql/consistency-queries/CfgConsistency.ql

@@ -1,8 +1,8 @@
-import codeql.ruby.controlflow.internal.ControlFlowGraphImplShared::Consistency
+import codeql.ruby.controlflow.internal.ControlFlowGraphImpl::Consistency
 import codeql.ruby.AST
 import codeql.ruby.CFG
 import codeql.ruby.controlflow.internal.Completion
-import codeql.ruby.controlflow.internal.ControlFlowGraphImpl
+import codeql.ruby.controlflow.internal.ControlFlowGraphImpl as CfgImpl
 
 /**
  * All `Expr` nodes are `PostOrderTree`s
@@ -14,7 +14,7 @@ query predicate nonPostOrderExpr(Expr e, string cls) {
   not e instanceof Namespace and
   not e instanceof Toplevel and
   exists(AstNode last, Completion c |
-    last(e, last, c) and
+    CfgImpl::last(e, last, c) and
     last != e and
     c instanceof NormalCompletion
   )

ruby/ql/lib/codeql/ruby/ast/Statement.qll

@@ -3,7 +3,7 @@ private import codeql.ruby.CFG
 private import internal.AST
 private import internal.TreeSitter
 private import internal.Variable
-private import codeql.ruby.controlflow.internal.ControlFlowGraphImpl
+private import codeql.ruby.controlflow.internal.ControlFlowGraphImpl as CfgImpl
 
 /**
  * A statement.
@@ -12,13 +12,13 @@ private import codeql.ruby.controlflow.internal.ControlFlowGraphImpl
  */
 class Stmt extends AstNode, TStmt {
   /** Gets a control-flow node for this statement, if any. */
-  CfgNodes::AstCfgNode getAControlFlowNode() { result.getNode() = this }
+  CfgNodes::AstCfgNode getAControlFlowNode() { result.getAstNode() = this }
 
   /** Gets a control-flow entry node for this statement, if any */
-  AstNode getAControlFlowEntryNode() { result = getAControlFlowEntryNode(this) }
+  AstNode getAControlFlowEntryNode() { result = CfgImpl::getAControlFlowEntryNode(this) }
 
   /** Gets the control-flow scope of this statement, if any. */
-  CfgScope getCfgScope() { result = getCfgScope(this) }
+  CfgScope getCfgScope() { result = CfgImpl::getCfgScope(this) }
 
   /** Gets the enclosing callable, if any. */
   Callable getEnclosingCallable() { result = this.getCfgScope() }

ruby/ql/lib/codeql/ruby/ast/internal/Constant.qll

@@ -214,7 +214,7 @@ private module Propagation {
         any(StringComponentCfgNode c |
           isString(c, result)
           or
-          result = c.getNode().(StringComponentImpl).getValue()
+          result = c.getAstNode().(StringComponentImpl).getValue()
         )
     }
 

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

@@ -4,7 +4,6 @@ private import codeql.ruby.AST
 private import codeql.ruby.ast.internal.AST
 private import codeql.ruby.ast.internal.TreeSitter
 private import codeql.ruby.controlflow.ControlFlowGraph
-private import internal.ControlFlowGraphImpl
 private import CfgNodes
 private import SuccessorTypes
 
@@ -390,7 +389,7 @@ private module JoinBlockPredecessors {
   private predicate idOf(Ruby::AstNode x, int y) = equivalenceRelation(id/2)(x, y)
 
   int getId(JoinBlockPredecessor jbp) {
-    idOf(toGeneratedInclSynth(jbp.getFirstNode().(AstCfgNode).getNode()), result)
+    idOf(toGeneratedInclSynth(jbp.getFirstNode().(AstCfgNode).getAstNode()), result)
     or
     idOf(toGeneratedInclSynth(jbp.(EntryBasicBlock).getScope()), result)
   }

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

@@ -6,62 +6,26 @@ private import codeql.ruby.dataflow.SSA
 private import codeql.ruby.ast.internal.Constant
 private import codeql.ruby.ast.internal.Literal
 private import ControlFlowGraph
-private import internal.ControlFlowGraphImpl
+private import internal.ControlFlowGraphImpl as CfgImpl
 private import internal.Splitting
 
 /** An entry node for a given scope. */
-class EntryNode extends CfgNode, TEntryNode {
+class EntryNode extends CfgNode, CfgImpl::EntryNode {
   override string getAPrimaryQlClass() { result = "EntryNode" }
 
-  private CfgScope scope;
-
-  EntryNode() { this = TEntryNode(scope) }
-
   final override EntryBasicBlock getBasicBlock() { result = super.getBasicBlock() }
-
-  final override Location getLocation() { result = scope.getLocation() }
-
-  final override string toString() { result = "enter " + scope }
 }
 
 /** An exit node for a given scope, annotated with the type of exit. */
-class AnnotatedExitNode extends CfgNode, TAnnotatedExitNode {
+class AnnotatedExitNode extends CfgNode, CfgImpl::AnnotatedExitNode {
   override string getAPrimaryQlClass() { result = "AnnotatedExitNode" }
 
-  private CfgScope scope;
-  private boolean normal;
-
-  AnnotatedExitNode() { this = TAnnotatedExitNode(scope, normal) }
-
-  /** Holds if this node represent a normal exit. */
-  final predicate isNormal() { normal = true }
-
   final override AnnotatedExitBasicBlock getBasicBlock() { result = super.getBasicBlock() }
-
-  final override Location getLocation() { result = scope.getLocation() }
-
-  final override string toString() {
-    exists(string s |
-      normal = true and s = "normal"
-      or
-      normal = false and s = "abnormal"
-    |
-      result = "exit " + scope + " (" + s + ")"
-    )
-  }
 }
 
 /** An exit node for a given scope. */
-class ExitNode extends CfgNode, TExitNode {
+class ExitNode extends CfgNode, CfgImpl::ExitNode {
   override string getAPrimaryQlClass() { result = "ExitNode" }
-
-  private CfgScope scope;
-
-  ExitNode() { this = TExitNode(scope) }
-
-  final override Location getLocation() { result = scope.getLocation() }
-
-  final override string toString() { result = "exit " + scope }
 }
 
 /**
@@ -71,44 +35,18 @@ class ExitNode extends CfgNode, TExitNode {
  * (dead) code or not important for control flow, and multiple when there are different
  * splits for the AST node.
  */
-class AstCfgNode extends CfgNode, TElementNode {
+class AstCfgNode extends CfgNode, CfgImpl::AstCfgNode {
   /** Gets the name of the primary QL class for this node. */
   override string getAPrimaryQlClass() { result = "AstCfgNode" }
-
-  private Splits splits;
-  AstNode e;
-
-  AstCfgNode() { this = TElementNode(_, e, splits) }
-
-  final override AstNode getNode() { result = e }
-
-  override Location getLocation() { result = e.getLocation() }
-
-  final override string toString() {
-    exists(string s | s = e.toString() |
-      result = "[" + this.getSplitsString() + "] " + s
-      or
-      not exists(this.getSplitsString()) and result = s
-    )
-  }
-
-  /** Gets a comma-separated list of strings for each split in this node, if any. */
-  final string getSplitsString() {
-    result = splits.toString() and
-    result != ""
-  }
-
-  /** Gets a split for this control flow node, if any. */
-  final Split getASplit() { result = splits.getASplit() }
 }
 
 /** A control-flow node that wraps an AST expression. */
 class ExprCfgNode extends AstCfgNode {
   override string getAPrimaryQlClass() { result = "ExprCfgNode" }
 
-  override Expr e;
+  Expr e;
 
-  ExprCfgNode() { e = this.getNode() }
+  ExprCfgNode() { e = this.getAstNode() }
 
   /** Gets the underlying expression. */
   Expr getExpr() { result = e }
@@ -123,12 +61,12 @@ class ReturningCfgNode extends AstCfgNode {
 
   ReturningStmt s;
 
-  ReturningCfgNode() { s = this.getNode() }
+  ReturningCfgNode() { s = this.getAstNode() }
 
   /** Gets the node of the returned value, if any. */
   ExprCfgNode getReturnedValueNode() {
     result = this.getAPredecessor() and
-    result.getNode() = s.getValue()
+    result.getAstNode() = s.getValue()
   }
 }
 
@@ -136,17 +74,19 @@ class ReturningCfgNode extends AstCfgNode {
 class StringComponentCfgNode extends AstCfgNode {
   override string getAPrimaryQlClass() { result = "StringComponentCfgNode" }
 
-  StringComponentCfgNode() { this.getNode() instanceof StringComponent }
+  StringComponentCfgNode() { this.getAstNode() instanceof StringComponent }
 
   /** Gets the constant value of this string component. */
-  ConstantValue getConstantValue() { result = this.getNode().(StringComponent).getConstantValue() }
+  ConstantValue getConstantValue() {
+    result = this.getAstNode().(StringComponent).getConstantValue()
+  }
 }
 
 /** A control-flow node that wraps a `RegExpComponent` AST expression. */
 class RegExpComponentCfgNode extends StringComponentCfgNode {
   override string getAPrimaryQlClass() { result = "RegExpComponentCfgNode" }
 
-  RegExpComponentCfgNode() { e instanceof RegExpComponent }
+  RegExpComponentCfgNode() { this.getAstNode() instanceof RegExpComponent }
 }
 
 private AstNode desugar(AstNode n) {
@@ -179,7 +119,7 @@ abstract private class ChildMapping extends AstNode {
   cached
   predicate hasCfgChild(AstNode child, CfgNode cfn, CfgNode cfnChild) {
     this.reachesBasicBlock(child, cfn, cfnChild.getBasicBlock()) and
-    cfnChild.getNode() = desugar(child)
+    cfnChild.getAstNode() = desugar(child)
   }
 }
 
@@ -196,7 +136,7 @@ abstract private class ExprChildMapping extends Expr, ChildMapping {
     exists(BasicBlock mid |
       this.reachesBasicBlock(child, cfn, mid) and
       bb = mid.getAPredecessor() and
-      not mid.getANode().getNode() = child
+      not mid.getANode().getAstNode() = child
     )
   }
 }
@@ -210,13 +150,13 @@ abstract private class NonExprChildMapping extends ChildMapping {
   pragma[nomagic]
   override predicate reachesBasicBlock(AstNode child, CfgNode cfn, BasicBlock bb) {
     this.relevantChild(child) and
-    cfn.getNode() = this and
+    cfn.getAstNode() = this and
     bb.getANode() = cfn
     or
     exists(BasicBlock mid |
       this.reachesBasicBlock(child, cfn, mid) and
       bb = mid.getASuccessor() and
-      not mid.getANode().getNode() = child
+      not mid.getANode().getAstNode() = child
     )
   }
 }
@@ -440,9 +380,11 @@ module ExprNodes {
 
   /** A control-flow node that wraps an `InClause` AST expression. */
   class InClauseCfgNode extends AstCfgNode {
-    override string getAPrimaryQlClass() { result = "InClauseCfgNode" }
+    private InClauseChildMapping e;
 
-    override InClauseChildMapping e;
+    InClauseCfgNode() { e = this.getAstNode() }
+
+    override string getAPrimaryQlClass() { result = "InClauseCfgNode" }
 
     /** Gets the pattern in this `in`-clause. */
     final AstCfgNode getPattern() { e.hasCfgChild(e.getPattern(), this, result) }
@@ -460,56 +402,60 @@ module ExprNodes {
     predicate patternReachesBasicBlock(int i, CfgNode cfnPattern, BasicBlock bb) {
       exists(Expr pattern |
         pattern = this.getPattern(i) and
-        cfnPattern.getNode() = pattern and
+        cfnPattern.getAstNode() = pattern and
         bb.getANode() = cfnPattern
       )
       or
       exists(BasicBlock mid |
         this.patternReachesBasicBlock(i, cfnPattern, mid) and
         bb = mid.getASuccessor() and
-        not mid.getANode().getNode() = this
+        not mid.getANode().getAstNode() = this
       )
     }
 
     predicate bodyReachesBasicBlock(CfgNode cfnBody, BasicBlock bb) {
       exists(Stmt body |
         body = this.getBody() and
-        cfnBody.getNode() = body and
+        cfnBody.getAstNode() = body and
         bb.getANode() = cfnBody
       )
       or
       exists(BasicBlock mid |
         this.bodyReachesBasicBlock(cfnBody, mid) and
         bb = mid.getAPredecessor() and
-        not mid.getANode().getNode() = this
+        not mid.getANode().getAstNode() = this
       )
     }
   }
 
   /** A control-flow node that wraps a `WhenClause` AST expression. */
   class WhenClauseCfgNode extends AstCfgNode {
-    override string getAPrimaryQlClass() { result = "WhenClauseCfgNode" }
+    private WhenClauseChildMapping e;
 
-    override WhenClauseChildMapping e;
+    WhenClauseCfgNode() { e = this.getAstNode() }
+
+    override string getAPrimaryQlClass() { result = "WhenClauseCfgNode" }
 
     /** Gets the body of this `when`-clause. */
     final ExprCfgNode getBody() {
-      result.getNode() = desugar(e.getBody()) and
+      result.getAstNode() = desugar(e.getBody()) and
       e.bodyReachesBasicBlock(result, this.getBasicBlock())
     }
 
     /** Gets the `i`th pattern this `when`-clause. */
     final ExprCfgNode getPattern(int i) {
-      result.getNode() = desugar(e.getPattern(i)) and
+      result.getAstNode() = desugar(e.getPattern(i)) and
       e.patternReachesBasicBlock(i, result, this.getBasicBlock())
     }
   }
 
   /** A control-flow node that wraps a `CasePattern`. */
   class CasePatternCfgNode extends AstCfgNode {
-    override string getAPrimaryQlClass() { result = "CasePatternCfgNode" }
+    CasePattern e;
 
-    override CasePattern e;
+    CasePatternCfgNode() { e = this.getAstNode() }
+
+    override string getAPrimaryQlClass() { result = "CasePatternCfgNode" }
   }
 
   private class ArrayPatternChildMapping extends NonExprChildMapping, ArrayPattern {
@@ -871,7 +817,7 @@ module ExprNodes {
 
   /** A control-flow node that wraps an `InstanceVariableReadAccess` AST expression. */
   class InstanceVariableReadAccessCfgNode extends InstanceVariableAccessCfgNode {
-    InstanceVariableReadAccessCfgNode() { this.getNode() instanceof InstanceVariableReadAccess }
+    InstanceVariableReadAccessCfgNode() { this.getAstNode() instanceof InstanceVariableReadAccess }
 
     override string getAPrimaryQlClass() { result = "InstanceVariableReadAccessCfgNode" }
 
@@ -880,7 +826,9 @@ module ExprNodes {
 
   /** A control-flow node that wraps an `InstanceVariableWriteAccess` AST expression. */
   class InstanceVariableWriteAccessCfgNode extends InstanceVariableAccessCfgNode {
-    InstanceVariableWriteAccessCfgNode() { this.getNode() instanceof InstanceVariableWriteAccess }
+    InstanceVariableWriteAccessCfgNode() {
+      this.getAstNode() instanceof InstanceVariableWriteAccess
+    }
 
     override string getAPrimaryQlClass() { result = "InstanceVariableWriteAccessCfgNode" }
 
@@ -891,7 +839,9 @@ module ExprNodes {
   class StringInterpolationComponentCfgNode extends StringComponentCfgNode, StmtSequenceCfgNode {
     override string getAPrimaryQlClass() { result = "StringInterpolationComponentCfgNode" }
 
-    StringInterpolationComponentCfgNode() { this.getNode() instanceof StringInterpolationComponent }
+    StringInterpolationComponentCfgNode() {
+      this.getAstNode() instanceof StringInterpolationComponent
+    }
 
     final override ConstantValue getConstantValue() {
       result = StmtSequenceCfgNode.super.getConstantValue()
@@ -902,7 +852,9 @@ module ExprNodes {
   class RegExpInterpolationComponentCfgNode extends RegExpComponentCfgNode, StmtSequenceCfgNode {
     override string getAPrimaryQlClass() { result = "RegExpInterpolationComponentCfgNode" }
 
-    RegExpInterpolationComponentCfgNode() { this.getNode() instanceof RegExpInterpolationComponent }
+    RegExpInterpolationComponentCfgNode() {
+      this.getAstNode() instanceof RegExpInterpolationComponent
+    }
 
     final override ConstantValue getConstantValue() {
       result = StmtSequenceCfgNode.super.getConstantValue()

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

@@ -3,7 +3,7 @@
 private import codeql.ruby.AST
 private import codeql.ruby.controlflow.BasicBlocks
 private import SuccessorTypes
-private import internal.ControlFlowGraphImpl
+private import internal.ControlFlowGraphImpl as CfgImpl
 private import internal.Splitting
 private import internal.Completion
 
@@ -15,12 +15,12 @@ private import internal.Completion
  * Note that module declarations are not themselves CFG scopes, as they are part of
  * the CFG of the enclosing top-level or callable.
  */
-class CfgScope extends Scope instanceof CfgScopeImpl {
+class CfgScope extends Scope instanceof CfgImpl::CfgScopeImpl {
   /** Gets the CFG scope that this scope is nested under, if any. */
   final CfgScope getOuterCfgScope() {
     exists(AstNode parent |
       parent = this.getParent() and
-      result = getCfgScope(parent)
+      result = CfgImpl::getCfgScope(parent)
     )
   }
 }
@@ -33,33 +33,18 @@ class CfgScope extends Scope instanceof CfgScopeImpl {
  *
  * Only nodes that can be reached from an entry point are included in the CFG.
  */
-class CfgNode extends TCfgNode {
+class CfgNode extends CfgImpl::Node {
   /** Gets the name of the primary QL class for this node. */
   string getAPrimaryQlClass() { none() }
 
-  /** Gets a textual representation of this control flow node. */
-  string toString() { none() }
-
-  /** Gets the AST node that this node corresponds to, if any. */
-  AstNode getNode() { none() }
-
-  /** Gets the location of this control flow node. */
-  Location getLocation() { none() }
-
   /** Gets the file of this control flow node. */
   final File getFile() { result = this.getLocation().getFile() }
 
-  /** Holds if this control flow node has conditional successors. */
-  final predicate isCondition() { exists(this.getASuccessor(any(ConditionalSuccessor bs))) }
-
-  /** Gets the scope of this node. */
-  final CfgScope getScope() { result = getNodeCfgScope(this) }
-
-  /** Gets the basic block that this control flow node belongs to. */
-  BasicBlock getBasicBlock() { result.getANode() = this }
+  /** DEPRECATED: Use `getAstNode` instead. */
+  deprecated AstNode getNode() { result = this.getAstNode() }
 
   /** Gets a successor node of a given type, if any. */
-  final CfgNode getASuccessor(SuccessorType t) { result = getASuccessor(this, t) }
+  final CfgNode getASuccessor(SuccessorType t) { result = super.getASuccessor(t) }
 
   /** Gets an immediate successor, if any. */
   final CfgNode getASuccessor() { result = this.getASuccessor(_) }
@@ -70,15 +55,12 @@ class CfgNode extends TCfgNode {
   /** Gets an immediate predecessor, if any. */
   final CfgNode getAPredecessor() { result = this.getAPredecessor(_) }
 
-  /** Holds if this node has more than one predecessor. */
-  final predicate isJoin() { strictcount(this.getAPredecessor()) > 1 }
-
-  /** Holds if this node has more than one successor. */
-  final predicate isBranch() { strictcount(this.getASuccessor()) > 1 }
+  /** Gets the basic block that this control flow node belongs to. */
+  BasicBlock getBasicBlock() { result.getANode() = this }
 }
 
 /** The type of a control flow successor. */
-class SuccessorType extends TSuccessorType {
+class SuccessorType extends CfgImpl::TSuccessorType {
   /** Gets a textual representation of successor type. */
   string toString() { none() }
 }
@@ -86,7 +68,7 @@ class SuccessorType extends TSuccessorType {
 /** Provides different types of control flow successor types. */
 module SuccessorTypes {
   /** A normal control flow successor. */
-  class NormalSuccessor extends SuccessorType, TSuccessorSuccessor {
+  class NormalSuccessor extends SuccessorType, CfgImpl::TSuccessorSuccessor {
     final override string toString() { result = "successor" }
   }
 
@@ -99,9 +81,9 @@ module SuccessorTypes {
     boolean value;
 
     ConditionalSuccessor() {
-      this = TBooleanSuccessor(value) or
-      this = TEmptinessSuccessor(value) or
-      this = TMatchingSuccessor(value)
+      this = CfgImpl::TBooleanSuccessor(value) or
+      this = CfgImpl::TEmptinessSuccessor(value) or
+      this = CfgImpl::TMatchingSuccessor(value)
     }
 
     /** Gets the Boolean value of this successor. */
@@ -125,7 +107,7 @@ module SuccessorTypes {
    *
    * `x >= 0` has both a `true` successor and a `false` successor.
    */
-  class BooleanSuccessor extends ConditionalSuccessor, TBooleanSuccessor { }
+  class BooleanSuccessor extends ConditionalSuccessor, CfgImpl::TBooleanSuccessor { }
 
   /**
    * An emptiness control flow successor.
@@ -158,7 +140,7 @@ module SuccessorTypes {
    *                     \___/
    * ```
    */
-  class EmptinessSuccessor extends ConditionalSuccessor, TEmptinessSuccessor {
+  class EmptinessSuccessor extends ConditionalSuccessor, CfgImpl::TEmptinessSuccessor {
     override string toString() { if value = true then result = "empty" else result = "non-empty" }
   }
 
@@ -189,7 +171,7 @@ module SuccessorTypes {
    *  puts "one"  puts "not one"
    * ```
    */
-  class MatchingSuccessor extends ConditionalSuccessor, TMatchingSuccessor {
+  class MatchingSuccessor extends ConditionalSuccessor, CfgImpl::TMatchingSuccessor {
     override string toString() { if value = true then result = "match" else result = "no-match" }
   }
 
@@ -207,7 +189,7 @@ module SuccessorTypes {
    * The exit node of `sum` is a `return` successor of the `return x + y`
    * statement.
    */
-  class ReturnSuccessor extends SuccessorType, TReturnSuccessor {
+  class ReturnSuccessor extends SuccessorType, CfgImpl::TReturnSuccessor {
     final override string toString() { result = "return" }
   }
 
@@ -230,7 +212,7 @@ module SuccessorTypes {
    *
    * The node `puts "done"` is `break` successor of the node `break`.
    */
-  class BreakSuccessor extends SuccessorType, TBreakSuccessor {
+  class BreakSuccessor extends SuccessorType, CfgImpl::TBreakSuccessor {
     final override string toString() { result = "break" }
   }
 
@@ -253,7 +235,7 @@ module SuccessorTypes {
    *
    * The node `x >= 0` is `next` successor of the node `next`.
    */
-  class NextSuccessor extends SuccessorType, TNextSuccessor {
+  class NextSuccessor extends SuccessorType, CfgImpl::TNextSuccessor {
     final override string toString() { result = "next" }
   }
 
@@ -278,7 +260,7 @@ module SuccessorTypes {
    *
    * The node `x -= 1` is `redo` successor of the node `redo`.
    */
-  class RedoSuccessor extends SuccessorType, TRedoSuccessor {
+  class RedoSuccessor extends SuccessorType, CfgImpl::TRedoSuccessor {
     final override string toString() { result = "redo" }
   }
 
@@ -302,7 +284,7 @@ module SuccessorTypes {
    *
    * The node `puts "Retry"` is `retry` successor of the node `retry`.
    */
-  class RetrySuccessor extends SuccessorType, TRetrySuccessor {
+  class RetrySuccessor extends SuccessorType, CfgImpl::TRetrySuccessor {
     final override string toString() { result = "retry" }
   }
 
@@ -323,7 +305,7 @@ module SuccessorTypes {
    * The exit node of `m` is an exceptional successor of the node
    * `raise "x > 2"`.
    */
-  class RaiseSuccessor extends SuccessorType, TRaiseSuccessor {
+  class RaiseSuccessor extends SuccessorType, CfgImpl::TRaiseSuccessor {
     final override string toString() { result = "raise" }
   }
 
@@ -344,7 +326,7 @@ module SuccessorTypes {
    * The exit node of `m` is an exit successor of the node
    * `exit 1`.
    */
-  class ExitSuccessor extends SuccessorType, TExitSuccessor {
+  class ExitSuccessor extends SuccessorType, CfgImpl::TExitSuccessor {
     final override string toString() { result = "exit" }
   }
 }

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

@@ -8,7 +8,7 @@ private import codeql.ruby.AST
 private import codeql.ruby.ast.internal.AST
 private import codeql.ruby.ast.internal.Control
 private import codeql.ruby.controlflow.ControlFlowGraph
-private import ControlFlowGraphImpl
+private import ControlFlowGraphImpl as CfgImpl
 private import NonReturning
 private import SuccessorTypes
 
@@ -53,7 +53,7 @@ private predicate nestedEnsureCompletion(TCompletion outer, int nestLevel) {
     or
     outer = TExitCompletion()
   ) and
-  nestLevel = any(Trees::BodyStmtTree t).getNestLevel()
+  nestLevel = any(CfgImpl::Trees::BodyStmtTree t).getNestLevel()
 }
 
 pragma[noinline]
@@ -72,7 +72,7 @@ private predicate completionIsValidForStmt(AstNode n, Completion c) {
 }
 
 private AstNode getARescuableBodyChild() {
-  exists(Trees::BodyStmtTree bst | result = bst.getBodyChild(_, true) |
+  exists(CfgImpl::Trees::BodyStmtTree bst | result = bst.getBodyChild(_, true) |
     exists(bst.getARescue())
     or
     exists(bst.getEnsure())
@@ -247,7 +247,7 @@ private predicate inMatchingContext(AstNode n) {
   or
   n = any(ReferencePattern p).getExpr()
   or
-  n.(Trees::DefaultValueParameterTree).hasDefaultValue()
+  n.(CfgImpl::Trees::DefaultValueParameterTree).hasDefaultValue()
 }
 
 /**

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

@@ -1,74 +0,0 @@
-private import codeql.ruby.AST as RB
-private import ControlFlowGraphImpl as Impl
-private import Completion as Comp
-private import codeql.ruby.ast.internal.Synthesis
-private import Splitting as Splitting
-private import codeql.ruby.CFG as Cfg
-
-/** The base class for `ControlFlowTree`. */
-class ControlFlowTreeBase extends RB::AstNode {
-  ControlFlowTreeBase() { not any(Synthesis s).excludeFromControlFlowTree(this) }
-}
-
-class ControlFlowElement = RB::AstNode;
-
-class Completion = Comp::Completion;
-
-/**
- * Hold if `c` represents normal evaluation of a statement or an
- * expression.
- */
-predicate completionIsNormal(Completion c) { c instanceof Comp::NormalCompletion }
-
-/**
- * Hold if `c` represents simple (normal) evaluation of a statement or an
- * expression.
- */
-predicate completionIsSimple(Completion c) { c instanceof Comp::SimpleCompletion }
-
-/** Holds if `c` is a valid completion for `e`. */
-predicate completionIsValidFor(Completion c, ControlFlowElement e) { c.isValidFor(e) }
-
-class CfgScope = Cfg::CfgScope;
-
-predicate getCfgScope = Impl::getCfgScope/1;
-
-/** Holds if `first` is first executed when entering `scope`. */
-predicate scopeFirst(CfgScope scope, ControlFlowElement first) {
-  scope.(Impl::CfgScopeImpl).entry(first)
-}
-
-/** Holds if `scope` is exited when `last` finishes with completion `c`. */
-predicate scopeLast(CfgScope scope, ControlFlowElement last, Completion c) {
-  scope.(Impl::CfgScopeImpl).exit(last, c)
-}
-
-/** The maximum number of splits allowed for a given node. */
-int maxSplits() { result = 5 }
-
-class SplitKindBase = Splitting::TSplitKind;
-
-class Split = Splitting::Split;
-
-class SuccessorType = Cfg::SuccessorType;
-
-/** Gets a successor type that matches completion `c`. */
-SuccessorType getAMatchingSuccessorType(Completion c) { result = c.getAMatchingSuccessorType() }
-
-/**
- * Hold if `c` represents simple (normal) evaluation of a statement or an
- * expression.
- */
-predicate successorTypeIsSimple(SuccessorType t) {
-  t instanceof Cfg::SuccessorTypes::NormalSuccessor
-}
-
-/** Holds if `t` is an abnormal exit type out of a CFG scope. */
-predicate isAbnormalExitType(SuccessorType t) {
-  t instanceof Cfg::SuccessorTypes::RaiseSuccessor or
-  t instanceof Cfg::SuccessorTypes::ExitSuccessor
-}
-
-class Location = RB::Location;
-
-class Node = Cfg::CfgNode;

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

@@ -2,7 +2,7 @@
  * Provides classes and predicates relevant for splitting the control flow graph.
  */
 
-private import codeql.ruby.AST
+private import codeql.ruby.AST as Ast
 private import Completion
 private import ControlFlowGraphImpl
 private import SuccessorTypes
@@ -64,31 +64,36 @@ private module ConditionalCompletionSplitting {
 
   int getNextListOrder() { result = 1 }
 
-  private class ConditionalCompletionSplitImpl extends SplitImpl, ConditionalCompletionSplit {
+  private class ConditionalCompletionSplitImpl extends SplitImpl instanceof ConditionalCompletionSplit
+  {
+    ConditionalCompletion completion;
+
+    ConditionalCompletionSplitImpl() { this = TConditionalCompletionSplit(completion) }
+
     override ConditionalCompletionSplitKind getKind() { any() }
 
     override predicate hasEntry(AstNode pred, AstNode succ, Completion c) {
       succ(pred, succ, c) and
       last(succ, _, completion) and
       (
-        last(succ.(NotExpr).getOperand(), pred, c) and
+        last(succ.(Ast::NotExpr).getOperand(), pred, c) and
         completion.(BooleanCompletion).getDual() = c
         or
-        last(succ.(LogicalAndExpr).getAnOperand(), pred, c) and
+        last(succ.(Ast::LogicalAndExpr).getAnOperand(), pred, c) and
         completion = c
         or
-        last(succ.(LogicalOrExpr).getAnOperand(), pred, c) and
+        last(succ.(Ast::LogicalOrExpr).getAnOperand(), pred, c) and
         completion = c
         or
-        last(succ.(StmtSequence).getLastStmt(), pred, c) and
+        last(succ.(Ast::StmtSequence).getLastStmt(), pred, c) and
         completion = c
         or
-        last(succ.(ConditionalExpr).getBranch(_), pred, c) and
+        last(succ.(Ast::ConditionalExpr).getBranch(_), pred, c) and
         completion = c
       )
       or
       succ(pred, succ, c) and
-      succ instanceof WhenClause and
+      succ instanceof Ast::WhenClause and
       completion = c
     }
 
@@ -144,7 +149,7 @@ module EnsureSplitting {
     /** Holds if this node is the entry node in the `ensure` block it belongs to. */
     predicate isEntryNode() { first(block.getEnsure(), this) }
 
-    BodyStmt getBlock() { result = block }
+    Ast::BodyStmt getBlock() { result = block }
 
     pragma[noinline]
     predicate isEntered(AstNode pred, int nestLevel, Completion c) {
@@ -221,12 +226,12 @@ module EnsureSplitting {
     override string toString() { result = "ensure (" + nestLevel + ")" }
   }
 
-  private class EnsureSplitImpl extends SplitImpl, EnsureSplit {
-    override EnsureSplitKind getKind() { result.getNestLevel() = this.getNestLevel() }
+  private class EnsureSplitImpl extends SplitImpl instanceof EnsureSplit {
+    override EnsureSplitKind getKind() { result.getNestLevel() = super.getNestLevel() }
 
     override predicate hasEntry(AstNode pred, AstNode succ, Completion c) {
-      succ.(EnsureNode).isEntered(pred, this.getNestLevel(), c) and
-      this.getType().isSplitForEntryCompletion(c)
+      succ.(EnsureNode).isEntered(pred, super.getNestLevel(), c) and
+      super.getType().isSplitForEntryCompletion(c)
     }
 
     override predicate hasEntryScope(CfgScope scope, AstNode first) { none() }
@@ -253,8 +258,8 @@ module EnsureSplitting {
      */
     private predicate exit(AstNode pred, Completion c, boolean inherited) {
       exists(Trees::BodyStmtTree block, EnsureSplitType type |
-        this.exit0(pred, block, this.getNestLevel(), c) and
-        type = this.getType()
+        this.exit0(pred, block, super.getNestLevel(), c) and
+        type = super.getType()
       |
         if last(block.getEnsure(), pred, c)
         then
@@ -302,9 +307,9 @@ module EnsureSplitting {
       // split must be able to exit with a `return` completion.
       this.appliesToPredecessor(pred) and
       exists(EnsureSplitImpl outer |
-        outer.getNestLevel() = this.getNestLevel() - 1 and
+        outer.(EnsureSplit).getNestLevel() = super.getNestLevel() - 1 and
         outer.exit(pred, c, inherited) and
-        this.getType() instanceof NormalSuccessor and
+        super.getType() instanceof NormalSuccessor and
         inherited = true
       )
     }
@@ -335,10 +340,10 @@ module EnsureSplitting {
           if en.isEntryNode() and en.getBlock() != pred.(EnsureNode).getBlock()
           then
             // entering a nested `ensure` block
-            en.getNestLevel() > this.getNestLevel()
+            en.getNestLevel() > super.getNestLevel()
           else
             // staying in the same (possibly nested) `ensure` block as `pred`
-            en.getNestLevel() >= this.getNestLevel()
+            en.getNestLevel() >= super.getNestLevel()
         )
     }
   }

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll

@@ -264,7 +264,7 @@ private predicate selfInToplevel(SelfVariable self, Module m) {
 private predicate asModulePattern(SsaDefinitionExtNode def, Module m) {
   exists(AsPattern ap |
     m = resolveConstantReadAccess(ap.getPattern()) and
-    def.getDefinitionExt().(Ssa::WriteDefinition).getWriteAccess().getNode() =
+    def.getDefinitionExt().(Ssa::WriteDefinition).getWriteAccess().getAstNode() =
       ap.getVariableAccess()
   )
 }

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll

@@ -118,7 +118,7 @@ module LocalFlow {
   /** Gets the SSA definition node corresponding to parameter `p`. */
   SsaDefinitionExtNode getParameterDefNode(NamedParameter p) {
     exists(BasicBlock bb, int i |
-      bb.getNode(i).getNode() = p.getDefiningAccess() and
+      bb.getNode(i).getAstNode() = p.getDefiningAccess() and
       result.getDefinitionExt().definesAt(_, bb, i, _)
     )
   }
@@ -203,8 +203,8 @@ module LocalFlow {
     exists(CfgNodes::ExprCfgNode exprTo, ReturningStatementNode n |
       nodeFrom = n and
       exprTo = nodeTo.asExpr() and
-      n.getReturningNode().getNode() instanceof BreakStmt and
-      exprTo.getNode() instanceof Loop and
+      n.getReturningNode().getAstNode() instanceof BreakStmt and
+      exprTo.getAstNode() instanceof Loop and
       nodeTo.asExpr().getAPredecessor(any(SuccessorTypes::BreakSuccessor s)) = n.getReturningNode()
     )
     or
@@ -926,7 +926,7 @@ abstract class SourceReturnNode extends ReturnNode {
 private module ReturnNodes {
   private predicate isValid(CfgNodes::ReturningCfgNode node) {
     exists(ReturningStmt stmt, Callable scope |
-      stmt = node.getNode() and
+      stmt = node.getAstNode() and
       scope = node.getScope()
     |
       stmt instanceof ReturnStmt and
@@ -952,7 +952,7 @@ private module ReturnNodes {
     }
 
     override ReturnKind getKindSource() {
-      if n.getNode() instanceof BreakStmt
+      if n.getAstNode() instanceof BreakStmt
       then result instanceof BreakReturnKind
       else
         exists(CfgScope scope | scope = this.getCfgScope() |

ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll

@@ -1,7 +1,7 @@
 private import codeql.ssa.Ssa as SsaImplCommon
 private import codeql.ruby.AST
 private import codeql.ruby.CFG as Cfg
-private import codeql.ruby.controlflow.internal.ControlFlowGraphImplShared as ControlFlowGraphImplShared
+private import codeql.ruby.controlflow.internal.ControlFlowGraphImpl as ControlFlowGraphImpl
 private import codeql.ruby.dataflow.SSA
 private import codeql.ruby.ast.Variable
 private import Cfg::CfgNodes::ExprNodes
@@ -31,7 +31,7 @@ private module SsaInput implements SsaImplCommon::InputSig {
         i = 0
         or
         // ...or a class or module block.
-        bb.getNode(i).getNode() = scope.(ModuleBase).getAControlFlowEntryNode() and
+        bb.getNode(i).getAstNode() = scope.(ModuleBase).getAControlFlowEntryNode() and
         not scope instanceof Toplevel // handled by case above
       )
       or
@@ -124,10 +124,10 @@ private predicate capturedExitRead(Cfg::AnnotatedExitBasicBlock bb, int i, Local
 private predicate namespaceSelfExitRead(Cfg::AnnotatedExitBasicBlock bb, int i, SelfVariable v) {
   exists(Namespace ns, AstNode last |
     v.getDeclaringScope() = ns and
-    last = ControlFlowGraphImplShared::getAControlFlowExitNode(ns) and
+    last = ControlFlowGraphImpl::getAControlFlowExitNode(ns) and
     if last = ns
-    then bb.getNode(i).getAPredecessor().getNode() = last
-    else bb.getNode(i).getNode() = last
+    then bb.getNode(i).getAPredecessor().getAstNode() = last
+    else bb.getNode(i).getAstNode() = last
   )
 }
 
@@ -183,7 +183,7 @@ private predicate capturedCallRead(CallCfgNode call, Cfg::BasicBlock bb, int i,
 private predicate variableReadActual(Cfg::BasicBlock bb, int i, LocalVariable v) {
   exists(VariableReadAccess read |
     read.getVariable() = v and
-    read = bb.getNode(i).getNode()
+    read = bb.getNode(i).getAstNode()
   )
 }
 

ruby/ql/lib/codeql/ruby/security/HardcodedDataInterpretedAsCodeCustomizations.qll

@@ -79,7 +79,7 @@ module HardcodedDataInterpretedAsCode {
       forex(StringComponentCfgNode c |
         c = this.asExpr().(ExprNodes::StringlikeLiteralCfgNode).getAComponent()
       |
-        c.getNode().(Ast::StringEscapeSequenceComponent).getRawText().matches("\\x%")
+        c.getAstNode().(Ast::StringEscapeSequenceComponent).getRawText().matches("\\x%")
       )
     }
   }

ruby/ql/lib/codeql/ruby/typetracking/TypeTrackerSpecific.qll

@@ -230,7 +230,7 @@ predicate returnStep(Node nodeFrom, Node nodeTo) {
     // deliberately do not include `getInitializeTarget`, since calls to `new` should not
     // get the return value from `initialize`. Any fields being set in the initializer
     // will reach all reads via `callStep` and `localFieldStep`.
-    nodeTo.asExpr().getNode() = call.getNode()
+    nodeTo.asExpr().getAstNode() = call.getAstNode()
   )
 }
 

ruby/ql/lib/ide-contextual-queries/printCfg.ql

@@ -7,7 +7,7 @@
  * @tags ide-contextual-queries/print-cfg
  */
 
-private import codeql.ruby.controlflow.internal.ControlFlowGraphImplShared::TestOutput
+private import codeql.ruby.controlflow.internal.ControlFlowGraphImpl::TestOutput
 private import codeql.IDEContextual
 
 /**

ruby/ql/lib/qlpack.yml

@@ -6,6 +6,7 @@ dbscheme: ruby.dbscheme
 upgrades: upgrades
 library: true
 dependencies:
+  codeql/controlflow: ${workspace}
   codeql/dataflow: ${workspace}
   codeql/mad: ${workspace}
   codeql/regex: ${workspace}

ruby/ql/src/experimental/performance/UseDetect.ql

@@ -42,12 +42,12 @@ class EndCall extends MethodCall {
 }
 
 Expr getUniqueRead(Expr e) {
-  forex(CfgNode eNode | eNode.getNode() = e |
+  forex(CfgNode eNode | eNode.getAstNode() = e |
     exists(Ssa::WriteDefinition def |
       def.assigns(eNode) and
       strictcount(def.getARead()) = 1 and
       not def = any(Ssa::PhiNode phi).getAnInput() and
-      def.getARead().getNode() = result
+      def.getARead().getAstNode() = result
     )
   )
 }

ruby/ql/src/queries/analysis/Definitions.ql

@@ -48,7 +48,7 @@ newtype DefLoc =
   /** A local variable. */
   LocalVariableLoc(VariableReadAccess read, VariableWriteAccess write) {
     exists(Ssa::WriteDefinition w |
-      write = w.getWriteAccess().getNode() and
+      write = w.getWriteAccess().getAstNode() and
       read = w.getARead().getExpr() and
       not read.isSynthesized()
     )

ruby/ql/src/queries/variables/DeadStoreOfLocal.ql

@@ -24,5 +24,5 @@ from RelevantLocalVariableWriteAccess write, LocalVariable v
 where
   v = write.getVariable() and
   exists(write.getAControlFlowNode()) and
-  not exists(Ssa::WriteDefinition def | def.getWriteAccess().getNode() = write)
+  not exists(Ssa::WriteDefinition def | def.getWriteAccess().getAstNode() = write)
 select write, "This assignment to $@ is useless, since its value is never read.", v, v.getName()

ruby/ql/src/queries/variables/UnusedParameter.ql

@@ -23,5 +23,6 @@ class RelevantParameterVariable extends LocalVariable {
 }
 
 from RelevantParameterVariable v
-where not exists(Ssa::WriteDefinition def | def.getWriteAccess().getNode() = v.getDefiningAccess())
+where
+  not exists(Ssa::WriteDefinition def | def.getWriteAccess().getAstNode() = v.getDefiningAccess())
 select v, "The parameter '" + v.getName() + "' is never used."