From 5724112513af697dcc3c1a4f985a20c4ef92beaf Mon Sep 17 00:00:00 2001
From: Tom Hvitved <hvitved@github.com>
Date: Tue, 16 Mar 2021 20:39:49 +0100
Subject: [PATCH] Address review comments

---
 ql/src/codeql_ruby/AST.qll                    |  8 +-
 ql/src/codeql_ruby/ast/Call.qll               | 25 ++----
 ql/src/codeql_ruby/ast/Constant.qll           |  8 +-
 ql/src/codeql_ruby/ast/Control.qll            | 72 ++++++++--------
 ql/src/codeql_ruby/ast/Expr.qll               | 42 ++++-----
 ql/src/codeql_ruby/ast/Literal.qll            | 86 ++++++++++++-------
 ql/src/codeql_ruby/ast/Method.qll             | 35 ++++----
 ql/src/codeql_ruby/ast/Module.qll             | 24 +++---
 ql/src/codeql_ruby/ast/Operation.qll          | 38 ++++----
 ql/src/codeql_ruby/ast/Parameter.qll          | 28 +++---
 ql/src/codeql_ruby/ast/Pattern.qll            |  4 +-
 ql/src/codeql_ruby/ast/Statement.qll          | 14 ++-
 ql/src/codeql_ruby/ast/Variable.qll           |  8 +-
 .../internal/ControlFlowGraphImpl.qll         |  8 +-
 ql/src/codeql_ruby/printAst.qll               |  2 +-
 15 files changed, 200 insertions(+), 202 deletions(-)

diff --git a/ql/src/codeql_ruby/AST.qll b/ql/src/codeql_ruby/AST.qll
index ce383d239fe..375ab3d806b 100644
--- a/ql/src/codeql_ruby/AST.qll
+++ b/ql/src/codeql_ruby/AST.qll
@@ -36,15 +36,15 @@ class AstNode extends TAstNode {
   Location getLocation() { result = toTreeSitter(this).getLocation() }
 
   /** Gets a child node of this `AstNode`. */
-  final AstNode getAChild() { this.child(_, result) }
+  final AstNode getAChild() { result = this.getAChild(_) }
 
   /** Gets the parent of this `AstNode`, if this node is not a root node. */
   final AstNode getParent() { result.getAChild() = this }
 
   /**
-   * Holds if `child` is a child of this node, and `child` can be retrieved using
-   * a predicate named `label`.
+   * Gets a child of this node, which can also be retrieved using a predicate
+   * named `pred`.
    */
   cached
-  predicate child(string label, AstNode child) { none() }
+  AstNode getAChild(string pred) { none() }
 }
diff --git a/ql/src/codeql_ruby/ast/Call.qll b/ql/src/codeql_ruby/ast/Call.qll
index a2bf4f6e24a..2bbb5885f58 100644
--- a/ql/src/codeql_ruby/ast/Call.qll
+++ b/ql/src/codeql_ruby/ast/Call.qll
@@ -48,9 +48,7 @@ class Call extends Expr, TCall {
    */
   final int getNumberOfArguments() { result = count(this.getAnArgument()) }
 
-  override predicate child(string label, AstNode child) {
-    label = "getArgument" and child = getArgument(_)
-  }
+  override AstNode getAChild(string pred) { pred = "getArgument" and result = this.getArgument(_) }
 }
 
 bindingset[s]
@@ -108,12 +106,12 @@ class MethodCall extends Call, TMethodCall {
 
   override string toString() { result = "call to " + concat(this.getMethodName(), "/") }
 
-  final override predicate child(string label, AstNode child) {
-    Call.super.child(label, child)
+  final override AstNode getAChild(string pred) {
+    result = Call.super.getAChild(pred)
     or
-    label = "getReceiver" and child = getReceiver()
+    pred = "getReceiver" and result = this.getReceiver()
     or
-    label = "getBlock" and child = getBlock()
+    pred = "getBlock" and result = this.getBlock()
   }
 }
 
@@ -161,7 +159,6 @@ private class RegularMethodCall extends MethodCall, TRegularMethodCall {
   final override Expr getArgument(int n) {
     toTreeSitter(result) = g.getArguments().getChild(n)
     or
-    not exists(g.getArguments()) and
     toTreeSitter(result) = g.getMethod().(Generated::ArgumentList).getChild(n)
   }
 
@@ -279,9 +276,7 @@ class BlockArgument extends Expr, TBlockArgument {
 
   final override string toString() { result = "&..." }
 
-  final override predicate child(string label, AstNode child) {
-    label = "getValue" and child = getValue()
-  }
+  final override AstNode getAChild(string pred) { pred = "getValue" and result = this.getValue() }
 }
 
 /**
@@ -308,9 +303,7 @@ class SplatArgument extends Expr, TSplatArgument {
 
   final override string toString() { result = "*..." }
 
-  final override predicate child(string label, AstNode child) {
-    label = "getValue" and child = getValue()
-  }
+  final override AstNode getAChild(string pred) { pred = "getValue" and result = this.getValue() }
 }
 
 /**
@@ -337,7 +330,5 @@ class HashSplatArgument extends Expr, THashSplatArgument {
 
   final override string toString() { result = "**..." }
 
-  final override predicate child(string label, AstNode child) {
-    label = "getValue" and child = getValue()
-  }
+  final override AstNode getAChild(string pred) { pred = "getValue" and result = this.getValue() }
 }
diff --git a/ql/src/codeql_ruby/ast/Constant.qll b/ql/src/codeql_ruby/ast/Constant.qll
index f3749d1b42b..423ff6afb88 100644
--- a/ql/src/codeql_ruby/ast/Constant.qll
+++ b/ql/src/codeql_ruby/ast/Constant.qll
@@ -37,9 +37,7 @@ class ConstantAccess extends Expr, TConstantAccess {
 
   override string toString() { result = this.getName() }
 
-  override predicate child(string label, AstNode child) {
-    label = "getScopeExpr" and child = getScopeExpr()
-  }
+  override AstNode getAChild(string pred) { pred = "getScopeExpr" and result = this.getScopeExpr() }
 }
 
 /**
@@ -85,7 +83,7 @@ private class ScopeResolutionConstantReadAccess extends ConstantReadAccess,
 
   final override Expr getScopeExpr() { toTreeSitter(result) = g.getScope() }
 
-  final override predicate hasGlobalScope() { not exists(this.getScopeExpr()) }
+  final override predicate hasGlobalScope() { not exists(g.getScope()) }
 }
 
 /**
@@ -127,7 +125,7 @@ private class ScopeResolutionConstantWriteAccess extends ConstantWriteAccess,
 
   final override Expr getScopeExpr() { toTreeSitter(result) = g.getScope() }
 
-  final override predicate hasGlobalScope() { not exists(this.getScopeExpr()) }
+  final override predicate hasGlobalScope() { not exists(g.getScope()) }
 }
 
 /**
diff --git a/ql/src/codeql_ruby/ast/Control.qll b/ql/src/codeql_ruby/ast/Control.qll
index 688726a328e..64977515a25 100644
--- a/ql/src/codeql_ruby/ast/Control.qll
+++ b/ql/src/codeql_ruby/ast/Control.qll
@@ -34,10 +34,10 @@ class ConditionalExpr extends ControlExpr, TConditionalExpr {
    */
   Stmt getBranch(boolean cond) { none() }
 
-  override predicate child(string label, AstNode child) {
-    label = "getCondition" and child = this.getCondition()
+  override AstNode getAChild(string pred) {
+    pred = "getCondition" and result = this.getCondition()
     or
-    label = "getBranch" and child = this.getBranch(_)
+    pred = "getBranch" and result = this.getBranch(_)
   }
 }
 
@@ -99,12 +99,12 @@ class IfExpr extends ConditionalExpr, TIfExpr {
     cond = false and result = this.getElse()
   }
 
-  override predicate child(string label, AstNode child) {
-    ConditionalExpr.super.child(label, child)
+  override AstNode getAChild(string pred) {
+    result = ConditionalExpr.super.getAChild(pred)
     or
-    label = "getThen" and child = this.getThen()
+    pred = "getThen" and result = this.getThen()
     or
-    label = "getElse" and child = this.getElse()
+    pred = "getElse" and result = this.getElse()
   }
 }
 
@@ -189,12 +189,12 @@ class UnlessExpr extends ConditionalExpr, TUnlessExpr {
 
   final override string toString() { result = "unless ..." }
 
-  override predicate child(string label, AstNode child) {
-    ConditionalExpr.super.child(label, child)
+  override AstNode getAChild(string pred) {
+    result = ConditionalExpr.super.getAChild(pred)
     or
-    label = "getThen" and child = this.getThen()
+    pred = "getThen" and result = this.getThen()
     or
-    label = "getElse" and child = this.getElse()
+    pred = "getElse" and result = this.getElse()
   }
 }
 
@@ -226,10 +226,10 @@ class IfModifierExpr extends ConditionalExpr, TIfModifierExpr {
 
   final override string toString() { result = "... if ..." }
 
-  override predicate child(string label, AstNode child) {
-    ConditionalExpr.super.child(label, child)
+  override AstNode getAChild(string pred) {
+    result = ConditionalExpr.super.getAChild(pred)
     or
-    label = "getBody" and child = this.getBody()
+    pred = "getBody" and result = this.getBody()
   }
 }
 
@@ -261,10 +261,10 @@ class UnlessModifierExpr extends ConditionalExpr, TUnlessModifierExpr {
 
   final override string toString() { result = "... unless ..." }
 
-  override predicate child(string label, AstNode child) {
-    ConditionalExpr.super.child(label, child)
+  override AstNode getAChild(string pred) {
+    result = ConditionalExpr.super.getAChild(pred)
     or
-    label = "getBody" and child = this.getBody()
+    pred = "getBody" and result = this.getBody()
   }
 }
 
@@ -297,12 +297,12 @@ class TernaryIfExpr extends ConditionalExpr, TTernaryIfExpr {
 
   final override string toString() { result = "... ? ... : ..." }
 
-  override predicate child(string label, AstNode child) {
-    ConditionalExpr.super.child(label, child)
+  override AstNode getAChild(string pred) {
+    result = ConditionalExpr.super.getAChild(pred)
     or
-    label = "getThen" and child = this.getThen()
+    pred = "getThen" and result = this.getThen()
     or
-    label = "getElse" and child = this.getElse()
+    pred = "getElse" and result = this.getElse()
   }
 }
 
@@ -359,10 +359,10 @@ class CaseExpr extends ControlExpr, TCaseExpr {
 
   final override string toString() { result = "case ..." }
 
-  override predicate child(string label, AstNode child) {
-    label = "getValue" and child = this.getValue()
+  override AstNode getAChild(string pred) {
+    pred = "getValue" and result = this.getValue()
     or
-    label = "getBranch" and child = this.getBranch(_)
+    pred = "getBranch" and result = this.getBranch(_)
   }
 }
 
@@ -409,10 +409,10 @@ class WhenExpr extends Expr, TWhenExpr {
 
   final override string toString() { result = "when ..." }
 
-  override predicate child(string label, AstNode child) {
-    label = "getBody" and child = this.getBody()
+  override AstNode getAChild(string pred) {
+    pred = "getBody" and result = this.getBody()
     or
-    label = "getPattern" and child = this.getPattern(_)
+    pred = "getPattern" and result = this.getPattern(_)
   }
 }
 
@@ -424,9 +424,7 @@ class Loop extends ControlExpr, TLoop {
   /** Gets the body of this loop. */
   Stmt getBody() { none() }
 
-  override predicate child(string label, AstNode child) {
-    label = "getBody" and child = this.getBody()
-  }
+  override AstNode getAChild(string pred) { pred = "getBody" and result = this.getBody() }
 }
 
 /**
@@ -437,10 +435,10 @@ class ConditionalLoop extends Loop, TConditionalLoop {
   /** Gets the condition expression of this loop. */
   Expr getCondition() { none() }
 
-  override predicate child(string label, AstNode child) {
-    Loop.super.child(label, child)
+  override AstNode getAChild(string pred) {
+    result = Loop.super.getAChild(pred)
     or
-    label = "getCondition" and child = this.getCondition()
+    pred = "getCondition" and result = this.getCondition()
   }
 }
 
@@ -566,11 +564,11 @@ class ForExpr extends Loop, TForExpr {
 
   final override string toString() { result = "for ... in ..." }
 
-  override predicate child(string label, AstNode child) {
-    Loop.super.child(label, child)
+  override AstNode getAChild(string pred) {
+    result = Loop.super.getAChild(pred)
     or
-    label = "getPattern" and child = this.getPattern()
+    pred = "getPattern" and result = this.getPattern()
     or
-    label = "getValue" and child = this.getValue()
+    pred = "getValue" and result = this.getValue()
   }
 }
diff --git a/ql/src/codeql_ruby/ast/Expr.qll b/ql/src/codeql_ruby/ast/Expr.qll
index 2d14acaa28d..f71d111c5d8 100644
--- a/ql/src/codeql_ruby/ast/Expr.qll
+++ b/ql/src/codeql_ruby/ast/Expr.qll
@@ -48,8 +48,8 @@ class ArgumentList extends Expr, TArgumentList {
 
   final override string toString() { result = "..., ..." }
 
-  final override predicate child(string label, AstNode child) {
-    label = "getElement" and child = this.getElement(_)
+  final override AstNode getAChild(string pred) {
+    pred = "getElement" and result = this.getElement(_)
   }
 }
 
@@ -82,9 +82,7 @@ class StmtSequence extends Expr, TStmtSequence {
   /** Holds if this sequence has no statements. */
   final predicate isEmpty() { this.getNumberOfStatements() = 0 }
 
-  override predicate child(string label, AstNode child) {
-    label = "getStmt" and child = this.getStmt(_)
-  }
+  override AstNode getAChild(string pred) { pred = "getStmt" and result = this.getStmt(_) }
 }
 
 private class Then extends StmtSequence, TThen {
@@ -181,14 +179,14 @@ class BodyStmt extends StmtSequence, TBodyStmt {
 
   final predicate hasEnsure() { exists(this.getEnsure()) }
 
-  override predicate child(string label, AstNode child) {
-    StmtSequence.super.child(label, child)
+  override AstNode getAChild(string pred) {
+    result = StmtSequence.super.getAChild(pred)
     or
-    label = "getRescue" and child = this.getRescue(_)
+    pred = "getRescue" and result = this.getRescue(_)
     or
-    label = "getElse" and child = this.getElse()
+    pred = "getElse" and result = this.getElse()
     or
-    label = "getEnsure" and child = this.getEnsure()
+    pred = "getEnsure" and result = this.getEnsure()
   }
 }
 
@@ -266,10 +264,10 @@ class Pair extends Expr, TPair {
 
   final override string toString() { result = "Pair" }
 
-  override predicate child(string label, AstNode child) {
-    label = "getKey" and child = this.getKey()
+  override AstNode getAChild(string pred) {
+    pred = "getKey" and result = this.getKey()
     or
-    label = "getValue" and child = this.getValue()
+    pred = "getValue" and result = this.getValue()
   }
 }
 
@@ -333,12 +331,12 @@ class RescueClause extends Expr, TRescueClause {
 
   final override string toString() { result = "rescue ..." }
 
-  override predicate child(string label, AstNode child) {
-    label = "getException" and child = this.getException(_)
+  override AstNode getAChild(string pred) {
+    pred = "getException" and result = this.getException(_)
     or
-    label = "getVariableExpr" and child = this.getVariableExpr()
+    pred = "getVariableExpr" and result = this.getVariableExpr()
     or
-    label = "getBody" and child = this.getBody()
+    pred = "getBody" and result = this.getBody()
   }
 }
 
@@ -373,10 +371,10 @@ class RescueModifierExpr extends Expr, TRescueModifierExpr {
 
   final override string toString() { result = "... rescue ..." }
 
-  override predicate child(string label, AstNode child) {
-    label = "getBody" and child = this.getBody()
+  override AstNode getAChild(string pred) {
+    pred = "getBody" and result = this.getBody()
     or
-    label = "getHandler" and child = this.getHandler()
+    pred = "getHandler" and result = this.getHandler()
   }
 }
 
@@ -432,7 +430,5 @@ class StringConcatenation extends Expr, TStringConcatenation {
 
   final override string toString() { result = "\"...\" \"...\"" }
 
-  override predicate child(string label, AstNode child) {
-    label = "getString" and child = this.getString(_)
-  }
+  override AstNode getAChild(string pred) { pred = "getString" and result = this.getString(_) }
 }
diff --git a/ql/src/codeql_ruby/ast/Literal.qll b/ql/src/codeql_ruby/ast/Literal.qll
index cb3684e6f88..57bc28dbb99 100644
--- a/ql/src/codeql_ruby/ast/Literal.qll
+++ b/ql/src/codeql_ruby/ast/Literal.qll
@@ -271,11 +271,51 @@ class StringlikeLiteral extends Literal, TStringlikeLiteral {
    */
   final int getNumberOfComponents() { result = count(this.getComponent(_)) }
 
-  string getStartDelimiter() { result = "" }
+  private string getStartDelimiter() {
+    this instanceof TStringLiteral and
+    result = "\""
+    or
+    this instanceof TRegexLiteral and
+    result = "/"
+    or
+    this instanceof TSimpleSymbolLiteral and
+    result = ":"
+    or
+    this instanceof TComplexSymbolLiteral and
+    result = ":\""
+    or
+    this instanceof THashKeySymbolLiteral and
+    result = ""
+    or
+    this instanceof TSubshellLiteral and
+    result = "`"
+    or
+    this instanceof THereDoc and
+    result = ""
+  }
 
-  string getEndDelimiter() { result = "" }
-
-  final predicate isSimple() { count(this.getComponent(_)) <= 1 }
+  private string getEndDelimiter() {
+    this instanceof TStringLiteral and
+    result = "\""
+    or
+    this instanceof TRegexLiteral and
+    result = "/"
+    or
+    this instanceof TSimpleSymbolLiteral and
+    result = ""
+    or
+    this instanceof TComplexSymbolLiteral and
+    result = "\""
+    or
+    this instanceof THashKeySymbolLiteral and
+    result = ""
+    or
+    this instanceof TSubshellLiteral and
+    result = "`"
+    or
+    this instanceof THereDoc and
+    result = ""
+  }
 
   override string getValueText() {
     // 0 components should result in the empty string
@@ -312,8 +352,8 @@ class StringlikeLiteral extends Literal, TStringlikeLiteral {
     )
   }
 
-  final override predicate child(string label, AstNode child) {
-    label = "getComponent" and child = this.getComponent(_)
+  final override AstNode getAChild(string pred) {
+    pred = "getComponent" and result = this.getComponent(_)
   }
 }
 
@@ -326,10 +366,6 @@ class StringlikeLiteral extends Literal, TStringlikeLiteral {
  * ```
  */
 class StringLiteral extends StringlikeLiteral, TStringLiteral {
-  final override string getStartDelimiter() { result = "\"" }
-
-  final override string getEndDelimiter() { result = "\"" }
-
   final override string getAPrimaryQlClass() { result = "StringLiteral" }
 }
 
@@ -365,10 +401,6 @@ class RegexLiteral extends StringlikeLiteral, TRegexLiteral {
 
   final override StringComponent getComponent(int i) { toTreeSitter(result) = g.getChild(i) }
 
-  final override string getStartDelimiter() { result = "/" }
-
-  final override string getEndDelimiter() { result = "/" }
-
   /**
    * Gets the regex flags as a string.
    *
@@ -419,19 +451,13 @@ private class SimpleSymbolLiteral extends SymbolLiteral, TSimpleSymbolLiteral {
 
   SimpleSymbolLiteral() { this = TSimpleSymbolLiteral(g) }
 
-  final override string getStartDelimiter() { result = ":" }
-
   // Tree-sitter gives us value text including the colon, which we skip.
   final override string getValueText() { result = g.getValue().suffix(1) }
 
   final override string toString() { result = g.getValue() }
 }
 
-private class ComplexSymbolLiteral extends SymbolLiteral, TComplexSymbolLiteral {
-  final override string getStartDelimiter() { result = ":\"" }
-
-  final override string getEndDelimiter() { result = "\"" }
-}
+private class ComplexSymbolLiteral extends SymbolLiteral, TComplexSymbolLiteral { }
 
 private class DelimitedSymbolLiteral extends ComplexSymbolLiteral, TDelimitedSymbolLiteral {
   private Generated::DelimitedSymbol g;
@@ -475,10 +501,6 @@ class SubshellLiteral extends StringlikeLiteral, TSubshellLiteral {
   final override string getAPrimaryQlClass() { result = "SubshellLiteral" }
 
   final override StringComponent getComponent(int i) { toTreeSitter(result) = g.getChild(i) }
-
-  final override string getStartDelimiter() { result = "`" }
-
-  final override string getEndDelimiter() { result = "`" }
 }
 
 /**
@@ -604,8 +626,8 @@ class ArrayLiteral extends Literal, TArrayLiteral {
   /** Gets the number of elements in this array literal. */
   final int getNumberOfElements() { result = count(this.getAnElement()) }
 
-  final override predicate child(string label, AstNode child) {
-    label = "getElement" and child = this.getElement(_)
+  final override AstNode getAChild(string pred) {
+    pred = "getElement" and result = this.getElement(_)
   }
 }
 
@@ -676,8 +698,8 @@ class HashLiteral extends Literal, THashLiteral {
 
   final override string toString() { result = "{...}" }
 
-  final override predicate child(string label, AstNode child) {
-    label = "getElement" and child = this.getElement(_)
+  final override AstNode getAChild(string pred) {
+    pred = "getElement" and result = this.getElement(_)
   }
 }
 
@@ -716,10 +738,10 @@ class RangeLiteral extends Literal, TRangeLiteral {
 
   final override string toString() { result = "_ " + g.getOperator() + " _" }
 
-  final override predicate child(string label, AstNode child) {
-    label = "getBegin" and child = this.getBegin()
+  final override AstNode getAChild(string pred) {
+    pred = "getBegin" and result = this.getBegin()
     or
-    label = "getEnd" and child = this.getEnd()
+    pred = "getEnd" and result = this.getEnd()
   }
 }
 
diff --git a/ql/src/codeql_ruby/ast/Method.qll b/ql/src/codeql_ruby/ast/Method.qll
index 3d50bf7e49c..3e0ad6c2b87 100644
--- a/ql/src/codeql_ruby/ast/Method.qll
+++ b/ql/src/codeql_ruby/ast/Method.qll
@@ -14,8 +14,8 @@ class Callable extends Expr, CfgScope, TCallable {
   /** Gets the `n`th parameter of this callable. */
   Parameter getParameter(int n) { none() }
 
-  override predicate child(string label, AstNode child) {
-    label = "getParameter" and child = this.getParameter(_)
+  override AstNode getAChild(string pred) {
+    pred = "getParameter" and result = this.getParameter(_)
   }
 }
 
@@ -24,10 +24,10 @@ class MethodBase extends Callable, BodyStmt, Scope, TMethodBase {
   /** Gets the name of this method. */
   string getName() { none() }
 
-  override predicate child(string label, AstNode child) {
-    Callable.super.child(label, child)
+  override AstNode getAChild(string pred) {
+    result = Callable.super.getAChild(pred)
     or
-    BodyStmt.super.child(label, child)
+    result = BodyStmt.super.getAChild(pred)
   }
 }
 
@@ -77,7 +77,6 @@ class SingletonMethod extends MethodBase, TSingletonMethod {
   final override string getName() {
     result = g.getName().(Generated::Token).getValue()
     or
-    // result = g.getName().(SymbolLiteral).getValueText() or
     result = g.getName().(Generated::Setter).getName().getValue() + "="
   }
 
@@ -87,10 +86,10 @@ class SingletonMethod extends MethodBase, TSingletonMethod {
 
   final override string toString() { result = this.getName() }
 
-  final override predicate child(string label, AstNode child) {
-    MethodBase.super.child(label, child)
+  final override AstNode getAChild(string pred) {
+    result = MethodBase.super.getAChild(pred)
     or
-    label = "getObject" and child = this.getObject()
+    pred = "getObject" and result = this.getObject()
   }
 }
 
@@ -113,19 +112,19 @@ class Lambda extends Callable, BodyStmt, TLambda {
 
   final override string toString() { result = "-> { ... }" }
 
-  final override predicate child(string label, AstNode child) {
-    Callable.super.child(label, child)
+  final override AstNode getAChild(string pred) {
+    result = Callable.super.getAChild(pred)
     or
-    BodyStmt.super.child(label, child)
+    result = BodyStmt.super.getAChild(pred)
   }
 }
 
 /** A block. */
 class Block extends Callable, StmtSequence, Scope, TBlock {
-  override predicate child(string label, AstNode child) {
-    Callable.super.child(label, child)
+  override AstNode getAChild(string pred) {
+    result = Callable.super.getAChild(pred)
     or
-    StmtSequence.super.child(label, child)
+    result = StmtSequence.super.getAChild(pred)
   }
 }
 
@@ -141,10 +140,10 @@ class DoBlock extends Block, BodyStmt, TDoBlock {
 
   final override string toString() { result = "do ... end" }
 
-  final override predicate child(string label, AstNode child) {
-    Block.super.child(label, child)
+  final override AstNode getAChild(string pred) {
+    result = Block.super.getAChild(pred)
     or
-    BodyStmt.super.child(label, child)
+    result = BodyStmt.super.getAChild(pred)
   }
 
   final override string getAPrimaryQlClass() { result = "DoBlock" }
diff --git a/ql/src/codeql_ruby/ast/Module.qll b/ql/src/codeql_ruby/ast/Module.qll
index 31bb892cd05..ba23d8327a5 100644
--- a/ql/src/codeql_ruby/ast/Module.qll
+++ b/ql/src/codeql_ruby/ast/Module.qll
@@ -56,10 +56,10 @@ class Toplevel extends ModuleBase, TToplevel {
    */
   final BeginBlock getABeginBlock() { result = getBeginBlock(_) }
 
-  final override predicate child(string label, AstNode child) {
-    ModuleBase.super.child(label, child)
+  final override AstNode getAChild(string pred) {
+    result = ModuleBase.super.getAChild(pred)
     or
-    label = "getBeginBlock" and child = this.getBeginBlock(_)
+    pred = "getBeginBlock" and result = this.getBeginBlock(_)
   }
 
   final override string toString() { result = g.getLocation().getFile().getBaseName() }
@@ -132,9 +132,9 @@ class Namespace extends ModuleBase, ConstantWriteAccess, TNamespace {
    */
   override predicate hasGlobalScope() { none() }
 
-  override predicate child(string label, AstNode child) {
-    ModuleBase.super.child(label, child) or
-    ConstantWriteAccess.super.child(label, child)
+  override AstNode getAChild(string pred) {
+    result = ModuleBase.super.getAChild(pred) or
+    result = ConstantWriteAccess.super.getAChild(pred)
   }
 
   final override string toString() { result = ConstantWriteAccess.super.toString() }
@@ -191,10 +191,10 @@ class Class extends Namespace, TClass {
     )
   }
 
-  final override predicate child(string label, AstNode child) {
-    Namespace.super.child(label, child)
+  final override AstNode getAChild(string pred) {
+    result = Namespace.super.getAChild(pred)
     or
-    label = "getSuperclassExpr" and child = this.getSuperclassExpr()
+    pred = "getSuperclassExpr" and result = this.getSuperclassExpr()
   }
 }
 
@@ -229,10 +229,10 @@ class SingletonClass extends ModuleBase, TSingletonClass {
 
   final override string toString() { result = "class << ..." }
 
-  final override predicate child(string label, AstNode child) {
-    ModuleBase.super.child(label, child)
+  final override AstNode getAChild(string pred) {
+    result = ModuleBase.super.getAChild(pred)
     or
-    label = "getValue" and child = this.getValue()
+    pred = "getValue" and result = this.getValue()
   }
 }
 
diff --git a/ql/src/codeql_ruby/ast/Operation.qll b/ql/src/codeql_ruby/ast/Operation.qll
index 19e757689c1..8d13367f2c3 100644
--- a/ql/src/codeql_ruby/ast/Operation.qll
+++ b/ql/src/codeql_ruby/ast/Operation.qll
@@ -14,9 +14,7 @@ class Operation extends Expr, TOperation {
   /** Gets an operand of this operation. */
   Expr getAnOperand() { none() }
 
-  override predicate child(string label, AstNode child) {
-    label = "getAnOperand" and child = this.getAnOperand()
-  }
+  override AstNode getAChild(string pred) { pred = "getAnOperand" and result = this.getAnOperand() }
 }
 
 /** A unary operation. */
@@ -32,10 +30,10 @@ class UnaryOperation extends Operation, TUnaryOperation {
 
   final override Expr getAnOperand() { result = this.getOperand() }
 
-  final override predicate child(string label, AstNode child) {
-    Operation.super.child(label, child)
+  final override AstNode getAChild(string pred) {
+    result = Operation.super.getAChild(pred)
     or
-    label = "getOperand" and child = this.getOperand()
+    pred = "getOperand" and result = this.getOperand()
   }
 
   final override string toString() { result = this.getOperator() + " ..." }
@@ -115,19 +113,19 @@ class BinaryOperation extends Operation, TBinaryOperation {
 
   final override string toString() { result = "... " + this.getOperator() + " ..." }
 
-  override predicate child(string label, AstNode child) {
-    Operation.super.child(label, child)
+  override AstNode getAChild(string pred) {
+    result = Operation.super.getAChild(pred)
     or
-    label = "getLeftOperand" and child = this.getLeftOperand()
+    pred = "getLeftOperand" and result = this.getLeftOperand()
     or
-    label = "getRightOperand" and child = this.getRightOperand()
+    pred = "getRightOperand" and result = this.getRightOperand()
   }
 
   /** Gets the left operand of this binary operation. */
-  final Expr getLeftOperand() { toTreeSitter(result) = g.getLeft() }
+  final Stmt getLeftOperand() { toTreeSitter(result) = g.getLeft() }
 
   /** Gets the right operand of this binary operation. */
-  final Expr getRightOperand() { toTreeSitter(result) = g.getRight() }
+  final Stmt getRightOperand() { toTreeSitter(result) = g.getRight() }
 }
 
 /**
@@ -328,12 +326,12 @@ class RelationalOperation extends ComparisonOperation, TRelationalOperation {
   /** Gets the lesser operand. */
   Expr getLesserOperand() { none() }
 
-  final override predicate child(string label, AstNode child) {
-    ComparisonOperation.super.child(label, child)
+  final override AstNode getAChild(string pred) {
+    result = ComparisonOperation.super.getAChild(pred)
     or
-    label = "getGreaterOperand" and child = this.getGreaterOperand()
+    pred = "getGreaterOperand" and result = this.getGreaterOperand()
     or
-    label = "getLesserOperand" and child = this.getLesserOperand()
+    pred = "getLesserOperand" and result = this.getLesserOperand()
   }
 }
 
@@ -441,12 +439,12 @@ class Assignment extends Operation, TAssignment {
 
   final override string toString() { result = "... " + this.getOperator() + " ..." }
 
-  override predicate child(string label, AstNode child) {
-    Operation.super.child(label, child)
+  override AstNode getAChild(string pred) {
+    result = Operation.super.getAChild(pred)
     or
-    label = "getLeftOperand" and child = getLeftOperand()
+    pred = "getLeftOperand" and result = getLeftOperand()
     or
-    label = "getRightOperand" and child = getRightOperand()
+    pred = "getRightOperand" and result = getRightOperand()
   }
 }
 
diff --git a/ql/src/codeql_ruby/ast/Parameter.qll b/ql/src/codeql_ruby/ast/Parameter.qll
index 022637dcf6c..1d207d1eb74 100644
--- a/ql/src/codeql_ruby/ast/Parameter.qll
+++ b/ql/src/codeql_ruby/ast/Parameter.qll
@@ -37,9 +37,9 @@ class TuplePatternParameter extends PatternParameter, TuplePattern, TTuplePatter
 
   final override string getAPrimaryQlClass() { result = "TuplePatternParameter" }
 
-  override predicate child(string label, AstNode child) {
-    PatternParameter.super.child(label, child) or
-    TuplePattern.super.child(label, child)
+  override AstNode getAChild(string pred) {
+    result = PatternParameter.super.getAChild(pred) or
+    result = TuplePattern.super.getAChild(pred)
   }
 }
 
@@ -59,11 +59,11 @@ class NamedParameter extends Parameter, TNamedParameter {
   /** Gets the access that defines the underlying local variable. */
   final VariableAccess getDefiningAccess() { result = this.getVariable().getDefiningAccess() }
 
-  override predicate child(string label, AstNode child) {
-    Parameter.super.child(label, child)
+  override AstNode getAChild(string pred) {
+    result = Parameter.super.getAChild(pred)
     or
-    label = "getDefiningAccess" and
-    child = this.getDefiningAccess()
+    pred = "getDefiningAccess" and
+    result = this.getDefiningAccess()
   }
 }
 
@@ -77,7 +77,7 @@ class SimpleParameter extends NamedParameter, PatternParameter, VariablePattern,
 
   final override LocalVariable getVariable() { result = TLocalVariable(_, _, g) }
 
-  override LocalVariable getAVariable() { result = this.getVariable() }
+  final override LocalVariable getAVariable() { result = this.getVariable() }
 
   final override string getAPrimaryQlClass() { result = "SimpleParameter" }
 
@@ -168,10 +168,10 @@ class KeywordParameter extends NamedParameter, TKeywordParameter {
 
   final override Location getLocation() { result = g.getName().getLocation() }
 
-  final override predicate child(string label, AstNode child) {
-    NamedParameter.super.child(label, child)
+  final override AstNode getAChild(string pred) {
+    result = NamedParameter.super.getAChild(pred)
     or
-    label = "getDefaultValue" and child = this.getDefaultValue()
+    pred = "getDefaultValue" and result = this.getDefaultValue()
   }
 }
 
@@ -205,10 +205,10 @@ class OptionalParameter extends NamedParameter, TOptionalParameter {
 
   final override Location getLocation() { result = g.getName().getLocation() }
 
-  final override predicate child(string label, AstNode child) {
-    NamedParameter.super.child(label, child)
+  final override AstNode getAChild(string pred) {
+    result = NamedParameter.super.getAChild(pred)
     or
-    label = "getDefaultValue" and child = this.getDefaultValue()
+    pred = "getDefaultValue" and result = this.getDefaultValue()
   }
 }
 
diff --git a/ql/src/codeql_ruby/ast/Pattern.qll b/ql/src/codeql_ruby/ast/Pattern.qll
index 2ec168d66c2..4018fbbe68f 100644
--- a/ql/src/codeql_ruby/ast/Pattern.qll
+++ b/ql/src/codeql_ruby/ast/Pattern.qll
@@ -91,7 +91,5 @@ class TuplePattern extends Pattern, TTuplePattern {
 
   override string toString() { result = "(..., ...)" }
 
-  override predicate child(string label, AstNode child) {
-    label = "getElement" and child = getElement(_)
-  }
+  override AstNode getAChild(string pred) { pred = "getElement" and result = getElement(_) }
 }
diff --git a/ql/src/codeql_ruby/ast/Statement.qll b/ql/src/codeql_ruby/ast/Statement.qll
index fad6855f5aa..94f1b49ec47 100644
--- a/ql/src/codeql_ruby/ast/Statement.qll
+++ b/ql/src/codeql_ruby/ast/Statement.qll
@@ -103,8 +103,8 @@ class UndefStmt extends Stmt, TUndefStmt {
 
   final override string toString() { result = "undef ..." }
 
-  final override predicate child(string label, AstNode child) {
-    label = "getMethodName" and child = this.getMethodName(_)
+  final override AstNode getAChild(string pred) {
+    pred = "getMethodName" and result = this.getMethodName(_)
   }
 }
 
@@ -131,10 +131,10 @@ class AliasStmt extends Stmt, TAliasStmt {
 
   final override string toString() { result = "alias ..." }
 
-  final override predicate child(string label, AstNode child) {
-    label = "getNewName" and child = this.getNewName()
+  final override AstNode getAChild(string pred) {
+    pred = "getNewName" and result = this.getNewName()
     or
-    label = "getOldName" and child = this.getOldName()
+    pred = "getOldName" and result = this.getOldName()
   }
 }
 
@@ -173,9 +173,7 @@ class ReturningStmt extends Stmt, TReturningStmt {
       )
   }
 
-  final override predicate child(string label, AstNode child) {
-    label = "getValue" and child = this.getValue()
-  }
+  final override AstNode getAChild(string pred) { pred = "getValue" and result = this.getValue() }
 }
 
 /**
diff --git a/ql/src/codeql_ruby/ast/Variable.qll b/ql/src/codeql_ruby/ast/Variable.qll
index 38701154ab1..de1b0c73941 100644
--- a/ql/src/codeql_ruby/ast/Variable.qll
+++ b/ql/src/codeql_ruby/ast/Variable.qll
@@ -161,7 +161,7 @@ class LocalVariableAccess extends VariableAccess, TLocalVariableAccess {
    */
   final predicate isCapturedAccess() { isCapturedAccess(this) }
 
-  override string toString() { result = g.getValue() }
+  final override string toString() { result = g.getValue() }
 }
 
 /** An access to a local variable where the value is updated. */
@@ -181,7 +181,7 @@ class GlobalVariableAccess extends VariableAccess, TGlobalVariableAccess {
 
   final override string getAPrimaryQlClass() { result = "GlobalVariableAccess" }
 
-  override string toString() { result = g.getValue() }
+  final override string toString() { result = g.getValue() }
 }
 
 /** An access to a global variable where the value is updated. */
@@ -201,7 +201,7 @@ class InstanceVariableAccess extends VariableAccess, TInstanceVariableAccess {
 
   final override string getAPrimaryQlClass() { result = "InstanceVariableAccess" }
 
-  override string toString() { result = g.getValue() }
+  final override string toString() { result = g.getValue() }
 }
 
 /** An access to a class variable. */
@@ -215,5 +215,5 @@ class ClassVariableAccess extends VariableAccess, TClassVariableAccess {
 
   final override string getAPrimaryQlClass() { result = "ClassVariableAccess" }
 
-  override string toString() { result = g.getValue() }
+  final override string toString() { result = g.getValue() }
 }
diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
index 2fbc47970c7..ea51daf0b06 100644
--- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
@@ -297,11 +297,11 @@ private class ForIn extends AST::AstNode, ASTInternal::TForIn {
 
 // TODO: remove this class; it should be replaced with an implicit non AST node
 private class ForRange extends AST::ForExpr {
-  override predicate child(string label, AST::AstNode child) {
-    AST::ForExpr.super.child(label, child)
+  override AST::AstNode getAChild(string pred) {
+    result = AST::ForExpr.super.getAChild(pred)
     or
-    label = "<in>" and
-    child = ASTInternal::TForIn(ASTInternal::toTreeSitter(this).(For).getValue())
+    pred = "<in>" and
+    result = ASTInternal::TForIn(ASTInternal::toTreeSitter(this).(For).getValue())
   }
 }
 
diff --git a/ql/src/codeql_ruby/printAst.qll b/ql/src/codeql_ruby/printAst.qll
index 45544f91101..2737e456251 100644
--- a/ql/src/codeql_ruby/printAst.qll
+++ b/ql/src/codeql_ruby/printAst.qll
@@ -52,7 +52,7 @@ class PrintAstNode extends AstNode {
   /**
    * Gets the child node that is accessed using the predicate `edgeName`.
    */
-  PrintAstNode getChild(string edgeName) { this.child(edgeName, result) }
+  PrintAstNode getChild(string edgeName) { result = this.getAChild(edgeName) }
 }
 
 private predicate shouldPrintNode(AstNode n) {