Ruby: Split callable and its body into two AST nodes.

Author: aschackmull

ruby/ql/consistency-queries/CfgConsistency.ql

@@ -27,7 +27,7 @@ query predicate scopeNoFirst(CfgScope scope) {
   not scope =
     any(Callable c |
       not exists(c.getAParameter()) and
-      not c.(BodyStmt).hasEnsure() and
-      not exists(c.(BodyStmt).getARescue())
+      not c.getBody().hasEnsure() and
+      not exists(c.getBody().getARescue())
     )
 }

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

@@ -167,6 +167,8 @@ class StmtSequence extends Expr, TStmtSequence {
  */
 class BodyStmt extends StmtSequence, TBodyStmt {
   final override Stmt getStmt(int n) {
+    synthChild(this, n, result)
+    or
     toGenerated(result) =
       rank[n + 1](Ruby::AstNode node, int i |
         node = getBodyStmtChild(this, i) and

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

@@ -8,7 +8,7 @@ private import internal.TreeSitter
 private import internal.Method
 
 /** A callable. */
-class Callable extends StmtSequence, Expr, Scope, TCallable {
+class Callable extends Expr, Scope, TCallable {
   /** Gets the number of parameters of this callable. */
   final int getNumberOfParameters() { result = count(this.getAParameter()) }
 
@@ -18,27 +18,26 @@ class Callable extends StmtSequence, Expr, Scope, TCallable {
   /** Gets the `n`th parameter of this callable. */
   Parameter getParameter(int n) { none() }
 
+  /** Gets the body of this callable. */
+  BodyStmt getBody() { none() }
+
   override AstNode getAChild(string pred) {
     result = super.getAChild(pred)
     or
+    pred = "getBody" and result = this.getBody()
+    or
     pred = "getParameter" and result = this.getParameter(_)
   }
 }
 
 /** A method. */
-class MethodBase extends Callable, BodyStmt, Scope, TMethodBase {
+class MethodBase extends Callable, Scope, TMethodBase {
   /** Gets the name of this method. */
   string getName() { none() }
 
   /** Holds if the name of this method is `name`. */
   final predicate hasName(string name) { this.getName() = name }
 
-  override AstNode getAChild(string pred) {
-    result = Callable.super.getAChild(pred)
-    or
-    result = BodyStmt.super.getAChild(pred)
-  }
-
   /**
    * Holds if this method is public.
    * Methods are public by default.
@@ -218,6 +217,10 @@ class Method extends MethodBase, TMethod {
     toGenerated(result) = g.getParameters().getChild(n)
   }
 
+  final override BodyStmt getBody() {
+    toGenerated(result) = g.getBody() or synthChild(this, _, result)
+  }
+
   final override string toString() { result = this.getName() }
 
   overlay[global]
@@ -280,6 +283,10 @@ class SingletonMethod extends MethodBase, TSingletonMethod {
     toGenerated(result) = g.getParameters().getChild(n)
   }
 
+  final override BodyStmt getBody() {
+    toGenerated(result) = g.getBody() or synthChild(this, _, result)
+  }
+
   final override string toString() { result = this.getName() }
 
   final override AstNode getAChild(string pred) {
@@ -321,7 +328,7 @@ class SingletonMethod extends MethodBase, TSingletonMethod {
  * -> (x) { x + 1 }
  * ```
  */
-class Lambda extends Callable, BodyStmt, TLambda {
+class Lambda extends Callable, TLambda {
   private Ruby::Lambda g;
 
   Lambda() { this = TLambda(g) }
@@ -332,13 +339,12 @@ class Lambda extends Callable, BodyStmt, TLambda {
     toGenerated(result) = g.getParameters().getChild(n)
   }
 
-  final override string toString() { result = "-> { ... }" }
-
-  final override AstNode getAChild(string pred) {
-    result = Callable.super.getAChild(pred)
-    or
-    result = BodyStmt.super.getAChild(pred)
+  final override BodyStmt getBody() {
+    toGenerated(result) = g.getBody().(Ruby::DoBlock).getBody() or
+    toGenerated(result) = g.getBody().(Ruby::Block).getBody()
   }
+
+  final override string toString() { result = "-> { ... }" }
 }
 
 /** A block. */
@@ -355,15 +361,15 @@ class Block extends Callable, Scope, TBlock {
    */
   LocalVariableWriteAccess getLocalVariable(int n) { none() }
 
-  override AstNode getAChild(string pred) {
+  final override AstNode getAChild(string pred) {
     result = Callable.super.getAChild(pred)
     or
     pred = "getLocalVariable" and result = this.getLocalVariable(_)
   }
 }
 
 /** A block enclosed within `do` and `end`. */
-class DoBlock extends Block, BodyStmt, TDoBlock {
+class DoBlock extends Block, TDoBlock {
   private Ruby::DoBlock g;
 
   DoBlock() { this = TDoBlock(g) }
@@ -376,13 +382,9 @@ class DoBlock extends Block, BodyStmt, TDoBlock {
     toGenerated(result) = g.getParameters().getChild(n)
   }
 
-  final override string toString() { result = "do ... end" }
+  final override BodyStmt getBody() { toGenerated(result) = g.getBody() }
 
-  final override AstNode getAChild(string pred) {
-    result = Block.super.getAChild(pred)
-    or
-    result = BodyStmt.super.getAChild(pred)
-  }
+  final override string toString() { result = "do ... end" }
 
   final override string getAPrimaryQlClass() { result = "DoBlock" }
 }

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

@@ -100,9 +100,16 @@ private module Cached {
     } or
     TBlockArgument(Ruby::BlockArgument g) or
     TBlockParameter(Ruby::BlockParameter g) or
+    TBodyStatement(Ruby::BodyStatement g) {
+      any(Ruby::Method m).getBody() = g or
+      any(Ruby::SingletonMethod m).getBody() = g or
+      any(Ruby::DoBlock b).getBody() = g
+    } or
+    TBodyStmtSynth(Ast::AstNode parent, int i) { mkSynthChild(BodyStmtKind(), parent, i) } or
     TBooleanLiteralSynth(Ast::AstNode parent, int i, boolean value) {
       mkSynthChild(BooleanLiteralKind(value), parent, i)
     } or
+    TBraceBlockBody(Ruby::BlockBody g) or
     TBraceBlockSynth(Ast::AstNode parent, int i) { mkSynthChild(BraceBlockKind(), parent, i) } or
     TBraceBlockReal(Ruby::Block g) { not g.getParent() instanceof Ruby::Lambda } or
     TBreakStmt(Ruby::Break g) or
@@ -362,23 +369,24 @@ private module Cached {
         TAssignMulExpr or TAssignRShiftExpr or TAssignSubExpr or TBareStringLiteral or
         TBareSymbolLiteral or TBeginBlock or TBeginExpr or TBitwiseAndExprReal or
         TBitwiseOrExprReal or TBitwiseXorExprReal or TBlockArgument or TBlockParameter or
-        TBraceBlockReal or TBreakStmt or TCaseEqExpr or TCaseExpr or TCaseMatchReal or
-        TCharacterLiteral or TClassDeclaration or TClassVariableAccessReal or TComplementExpr or
-        TComplexLiteral or TDefinedExprReal or TDelimitedSymbolLiteral or
-        TDestructuredLeftAssignment or TDestructuredParameter or TDivExprReal or TDo or TDoBlock or
-        TElementReference or TElseReal or TElsif or TEmptyStmt or TEncoding or TEndBlock or
-        TEnsure or TEqExpr or TExponentExprReal or TFalseLiteral or TFile or TFindPattern or
-        TFloatLiteral or TForExpr or TForwardParameter or TForwardArgument or TGEExpr or TGTExpr or
-        TGlobalVariableAccessReal or THashKeySymbolLiteral or THashLiteral or THashPattern or
-        THashSplatExprReal or THashSplatNilParameter or THashSplatParameter or THereDoc or
-        TIdentifierMethodCall or TIfReal or TIfModifierExpr or TInClauseReal or
-        TInstanceVariableAccessReal or TIntegerLiteralReal or TKeywordParameter or TLEExpr or
-        TLShiftExprReal or TLTExpr or TLambda or TLeftAssignmentList or TLine or
-        TLocalVariableAccessReal or TLogicalAndExprReal or TLogicalOrExprReal or TMethod or
-        TMatchPattern or TModuleDeclaration or TModuloExprReal or TMulExprReal or TNEExpr or
-        TNextStmt or TNilLiteralReal or TNoRegExpMatchExpr or TNotExprReal or TOptionalParameter or
-        TPairReal or TParenthesizedExpr or TParenthesizedPattern or TRShiftExprReal or
-        TRangeLiteralReal or TRationalLiteral or TRedoStmt or TRegExpLiteral or TRegExpMatchExpr or
+        TBodyStatement or TBraceBlockBody or TBraceBlockReal or TBreakStmt or TCaseEqExpr or
+        TCaseExpr or TCaseMatchReal or TCharacterLiteral or TClassDeclaration or
+        TClassVariableAccessReal or TComplementExpr or TComplexLiteral or TDefinedExprReal or
+        TDelimitedSymbolLiteral or TDestructuredLeftAssignment or TDestructuredParameter or
+        TDivExprReal or TDo or TDoBlock or TElementReference or TElseReal or TElsif or TEmptyStmt or
+        TEncoding or TEndBlock or TEnsure or TEqExpr or TExponentExprReal or TFalseLiteral or
+        TFile or TFindPattern or TFloatLiteral or TForExpr or TForwardParameter or
+        TForwardArgument or TGEExpr or TGTExpr or TGlobalVariableAccessReal or
+        THashKeySymbolLiteral or THashLiteral or THashPattern or THashSplatExprReal or
+        THashSplatNilParameter or THashSplatParameter or THereDoc or TIdentifierMethodCall or
+        TIfReal or TIfModifierExpr or TInClauseReal or TInstanceVariableAccessReal or
+        TIntegerLiteralReal or TKeywordParameter or TLEExpr or TLShiftExprReal or TLTExpr or
+        TLambda or TLeftAssignmentList or TLine or TLocalVariableAccessReal or
+        TLogicalAndExprReal or TLogicalOrExprReal or TMethod or TMatchPattern or
+        TModuleDeclaration or TModuloExprReal or TMulExprReal or TNEExpr or TNextStmt or
+        TNilLiteralReal or TNoRegExpMatchExpr or TNotExprReal or TOptionalParameter or TPairReal or
+        TParenthesizedExpr or TParenthesizedPattern or TRShiftExprReal or TRangeLiteralReal or
+        TRationalLiteral or TRedoStmt or TRegExpLiteral or TRegExpMatchExpr or
         TRegularArrayLiteral or TRegularMethodCall or TRegularStringLiteral or TRegularSuperCall or
         TRescueClause or TRescueModifierExpr or TRetryStmt or TReturnStmt or
         TScopeResolutionConstantAccess or TSelfReal or TSimpleParameterReal or
@@ -393,13 +401,13 @@ private module Cached {
 
   class TAstNodeSynth =
     TAddExprSynth or TAssignExprSynth or TBitwiseAndExprSynth or TBitwiseOrExprSynth or
-        TBitwiseXorExprSynth or TBraceBlockSynth or TBooleanLiteralSynth or TCaseMatchSynth or
-        TClassVariableAccessSynth or TConstantReadAccessSynth or TConstantWriteAccessSynth or
-        TDivExprSynth or TElseSynth or TExponentExprSynth or TGlobalVariableAccessSynth or
-        TIfSynth or TInClauseSynth or TInstanceVariableAccessSynth or TIntegerLiteralSynth or
-        TLShiftExprSynth or TLocalVariableAccessSynth or TLogicalAndExprSynth or
-        TLogicalOrExprSynth or TMethodCallSynth or TModuloExprSynth or TMulExprSynth or
-        TNilLiteralSynth or TRShiftExprSynth or TRangeLiteralSynth or TSelfSynth or
+        TBitwiseXorExprSynth or TBraceBlockSynth or TBodyStmtSynth or TBooleanLiteralSynth or
+        TCaseMatchSynth or TClassVariableAccessSynth or TConstantReadAccessSynth or
+        TConstantWriteAccessSynth or TDivExprSynth or TElseSynth or TExponentExprSynth or
+        TGlobalVariableAccessSynth or TIfSynth or TInClauseSynth or TInstanceVariableAccessSynth or
+        TIntegerLiteralSynth or TLShiftExprSynth or TLocalVariableAccessSynth or
+        TLogicalAndExprSynth or TLogicalOrExprSynth or TMethodCallSynth or TModuloExprSynth or
+        TMulExprSynth or TNilLiteralSynth or TRShiftExprSynth or TRangeLiteralSynth or TSelfSynth or
         TSimpleParameterSynth or TSplatExprSynth or THashSplatExprSynth or TStmtSequenceSynth or
         TSubExprSynth or TPairSynth or TSimpleSymbolLiteralSynth;
 
@@ -439,6 +447,8 @@ private module Cached {
     n = TBitwiseXorExprReal(result) or
     n = TBlockArgument(result) or
     n = TBlockParameter(result) or
+    n = TBodyStatement(result) or
+    n = TBraceBlockBody(result) or
     n = TBraceBlockReal(result) or
     n = TBreakStmt(result) or
     n = TCaseEqExpr(result) or
@@ -584,6 +594,8 @@ private module Cached {
     or
     result = TBitwiseXorExprSynth(parent, i)
     or
+    result = TBodyStmtSynth(parent, i)
+    or
     result = TBooleanLiteralSynth(parent, i, _)
     or
     result = TBraceBlockSynth(parent, i)
@@ -757,9 +769,9 @@ class TElse = TElseReal or TElseSynth;
 
 class TStmtSequence =
   TBeginBlock or TEndBlock or TThen or TElse or TDo or TEnsure or TStringInterpolationComponent or
-      TBlock or TBodyStmt or TParenthesizedExpr or TStmtSequenceSynth;
+      TBodyStmt or TParenthesizedExpr or TStmtSequenceSynth;
 
-class TBodyStmt = TBeginExpr or TModuleBase or TMethod or TLambda or TDoBlock or TSingletonMethod;
+class TBodyStmt = TBeginExpr or TModuleBase or TBraceBlockBody or TBodyStatement or TBodyStmtSynth;
 
 class TNilLiteral = TNilLiteralReal or TNilLiteralSynth;
 

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

@@ -14,6 +14,18 @@ class StmtSequenceSynth extends StmtSequence, TStmtSequenceSynth {
   final override string toString() { result = "..." }
 }
 
+class BodyStatement extends BodyStmt, TBodyStatement {
+  final override string toString() { result = "..." }
+}
+
+class BraceBlockBody extends BodyStmt, TBraceBlockBody {
+  final override string toString() { result = "..." }
+}
+
+class BodyStmtSynth extends BodyStmt, TBodyStmtSynth {
+  final override string toString() { result = "..." }
+}
+
 class Then extends StmtSequence, TThen {
   private Ruby::Then g;
 
@@ -64,27 +76,31 @@ class Ensure extends StmtSequence, TEnsure {
 
 // Not defined by dispatch, as it should not be exposed
 Ruby::AstNode getBodyStmtChild(TBodyStmt b, int i) {
-  exists(Ruby::Method g, Ruby::AstNode body | b = TMethod(g) and body = g.getBody() |
-    result = body.(Ruby::BodyStatement).getChild(i)
-    or
-    i = 0 and result = body and not body instanceof Ruby::BodyStatement
-  )
-  or
-  exists(Ruby::SingletonMethod g, Ruby::AstNode body |
-    b = TSingletonMethod(g) and body = g.getBody()
-  |
-    result = body.(Ruby::BodyStatement).getChild(i)
-    or
-    i = 0 and result = body and not body instanceof Ruby::BodyStatement
-  )
-  or
-  exists(Ruby::Lambda g | b = TLambda(g) |
-    result = g.getBody().(Ruby::DoBlock).getBody().getChild(i) or
-    result = g.getBody().(Ruby::Block).getBody().getChild(i)
-  )
+  result = any(Ruby::BlockBody g | b = TBraceBlockBody(g)).getChild(i)
   or
-  result = any(Ruby::DoBlock g | b = TDoBlock(g)).getBody().getChild(i)
+  result = any(Ruby::BodyStatement g | b = TBodyStatement(g)).getChild(i)
   or
+  // exists(Ruby::Method g, Ruby::AstNode body | b = TMethod(g) and body = g.getBody() |
+  //   result = body.(Ruby::BodyStatement).getChild(i)
+  //   or
+  //   i = 0 and result = body and not body instanceof Ruby::BodyStatement
+  // )
+  // or
+  // exists(Ruby::SingletonMethod g, Ruby::AstNode body |
+  //   b = TSingletonMethod(g) and body = g.getBody()
+  // |
+  //   result = body.(Ruby::BodyStatement).getChild(i)
+  //   or
+  //   i = 0 and result = body and not body instanceof Ruby::BodyStatement
+  // )
+  // or
+  // exists(Ruby::Lambda g | b = TLambda(g) |
+  //   result = g.getBody().(Ruby::DoBlock).getBody().getChild(i) or
+  //   result = g.getBody().(Ruby::Block).getBody().getChild(i)
+  // )
+  // or
+  // result = any(Ruby::DoBlock g | b = TDoBlock(g)).getBody().getChild(i)
+  // or
   result = any(Ruby::Program g | b = TToplevel(g)).getChild(i) and
   not result instanceof Ruby::BeginBlock
   or

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

@@ -18,7 +18,7 @@ class BraceBlockReal extends BraceBlock, TBraceBlockReal {
     toGenerated(result) = g.getParameters().getChild(n)
   }
 
-  final override Stmt getStmt(int i) { toGenerated(result) = g.getBody().getChild(i) }
+  final override BodyStmt getBody() { toGenerated(result) = g.getBody() }
 }
 
 /**
@@ -28,8 +28,5 @@ class BraceBlockReal extends BraceBlock, TBraceBlockReal {
 class BraceBlockSynth extends BraceBlock, TBraceBlockSynth {
   final override Parameter getParameter(int n) { synthChild(this, n, result) }
 
-  final override Stmt getStmt(int i) {
-    i >= 0 and
-    synthChild(this, i + this.getNumberOfParameters(), result)
-  }
+  final override BodyStmt getBody() { synthChild(this, _, result) }
 }