Merge branch 'main' into constants_scopes

ql/src/codeql_ruby/ast/Expr.qll

@@ -145,6 +145,14 @@ class ExprSequence extends Expr {
  */
 class BodyStatement extends ExprSequence {
   override BodyStatement::Range range;
+
+  /** Gets the `else` block in this block, if any. */
+  final ExprSequence getElse() { result = range.getElse() }
+
+  /** Gets the `ensure` block in this block, if any. */
+  final ExprSequence getEnsure() { result = range.getEnsure() }
+
+  final predicate hasEnsure() { exists(this.getEnsure()) }
 }
 
 /**

ql/src/codeql_ruby/ast/Method.qll

@@ -56,14 +56,14 @@ class SingletonMethod extends Callable, BodyStatement, @singleton_method {
  * -> (x) { x + 1 }
  * ```
  */
-class Lambda extends Callable, @lambda {
+class Lambda extends Callable, BodyStatement, @lambda {
   final override Lambda::Range range;
 
   final override string getAPrimaryQlClass() { result = "Lambda" }
 }
 
 /** A block. */
-class Block extends Callable {
+class Block extends Callable, ExprSequence {
   override Block::Range range;
 }
 

ql/src/codeql_ruby/ast/internal/AST.qll

@@ -33,8 +33,6 @@ module AstNode {
       this = any(Generated::Call c).getMethod() and
       not this instanceof Generated::ScopeResolution
       or
-      this instanceof Generated::Ensure
-      or
       this instanceof Generated::Rescue
       or
       this instanceof Generated::RestAssignment

ql/src/codeql_ruby/ast/internal/Expr.qll

@@ -185,7 +185,25 @@ module ExprSequence {
 }
 
 module BodyStatement {
-  abstract class Range extends ExprSequence::Range { }
+  abstract class Range extends ExprSequence::Range {
+    final override Expr getExpr(int n) {
+      result =
+        rank[n + 1](Generated::AstNode node, int i |
+          node = getChild(i) and
+          not node instanceof Generated::Else and
+          not node instanceof Generated::Rescue and
+          not node instanceof Generated::Ensure
+        |
+          node order by i
+        )
+    }
+
+    final ExprSequence getElse() { result = unique(Generated::Else s | s = getChild(_)) }
+
+    final ExprSequence getEnsure() { result = unique(Generated::Ensure s | s = getChild(_)) }
+
+    abstract Generated::AstNode getChild(int i);
+  }
 }
 
 module ParenthesizedExpr {
@@ -228,6 +246,16 @@ module DoExpr {
   }
 }
 
+module Ensure {
+  class Range extends ExprSequence::Range, @ensure {
+    final override Generated::Ensure generated;
+
+    final override Expr getExpr(int n) { result = generated.getChild(n) }
+
+    final override string toString() { result = "ensure ... end" }
+  }
+}
+
 module Pair {
   class Range extends Expr::Range, @pair {
     final override Generated::Pair generated;

ql/src/codeql_ruby/ast/internal/Method.qll

@@ -23,7 +23,7 @@ module Method {
 
     final predicate isSetter() { generated.getName() instanceof Generated::Setter }
 
-    final override Expr getExpr(int i) { result = generated.getChild(i) }
+    final override Generated::AstNode getChild(int i) { result = generated.getChild(i) }
 
     final override string toString() { result = this.getName() }
   }
@@ -41,33 +41,40 @@ module SingletonMethod {
       result = generated.getName().(Generated::Setter).getName().getValue() + "="
     }
 
-    final override Expr getExpr(int i) { result = generated.getChild(i) }
+    final override Generated::AstNode getChild(int i) { result = generated.getChild(i) }
 
     final override string toString() { result = this.getName() }
   }
 }
 
 module Lambda {
-  class Range extends Callable::Range, @lambda {
+  class Range extends Callable::Range, BodyStatement::Range, @lambda {
     final override Generated::Lambda generated;
 
     final override Parameter::Range getParameter(int n) {
       result = generated.getParameters().getChild(n)
     }
 
+    final override Generated::AstNode getChild(int i) {
+      result = generated.getBody().(Generated::DoBlock).getChild(i) or
+      result = generated.getBody().(Generated::Block).getChild(i)
+    }
+
     final override string toString() { result = "-> { ... }" }
   }
 }
 
 module Block {
-  abstract class Range extends Callable::Range { }
+  abstract class Range extends Callable::Range, ExprSequence::Range {
+    Range() { not generated.getParent() instanceof Generated::Lambda }
+  }
 }
 
 module DoBlock {
   class Range extends Block::Range, BodyStatement::Range, @do_block {
     final override Generated::DoBlock generated;
 
-    final override Expr getExpr(int i) { result = generated.getChild(i) }
+    final override Generated::AstNode getChild(int i) { result = generated.getChild(i) }
 
     final override Parameter::Range getParameter(int n) {
       result = generated.getParameters().getChild(n)
@@ -85,6 +92,8 @@ module BraceBlock {
       result = generated.getParameters().getChild(n)
     }
 
+    final override Expr getExpr(int i) { result = generated.getChild(i) }
+
     final override string toString() { result = "{ ... }" }
   }
 }

ql/src/codeql_ruby/ast/internal/Module.qll

@@ -11,7 +11,7 @@ module Class {
   class Range extends ModuleBase::Range, ConstantWriteAccess::Range, @class {
     final override Generated::Class generated;
 
-    final override Expr getExpr(int i) { result = generated.getChild(i) }
+    final override Generated::AstNode getChild(int i) { result = generated.getChild(i) }
 
     final override string getName() {
       result = generated.getName().(Generated::Token).getValue() or
@@ -40,7 +40,7 @@ module SingletonClass {
   class Range extends ModuleBase::Range, @singleton_class {
     final override Generated::SingletonClass generated;
 
-    final override Expr getExpr(int i) { result = generated.getChild(i) }
+    final override Generated::AstNode getChild(int i) { result = generated.getChild(i) }
 
     final Expr getValue() { result = generated.getValue() }
 
@@ -52,7 +52,7 @@ module Module {
   class Range extends ModuleBase::Range, ConstantWriteAccess::Range, @module {
     final override Generated::Module generated;
 
-    final override Expr getExpr(int n) { result = generated.getChild(n) }
+    final override Generated::AstNode getChild(int i) { result = generated.getChild(i) }
 
     final override string getName() {
       result = generated.getName().(Generated::Token).getValue() or

ql/src/codeql_ruby/dataflow/internal/DataFlowPrivate.qll

@@ -132,15 +132,15 @@ private module Cached {
     or
     nodeFrom.asExpr() = nodeTo.asExpr().(CfgNodes::ExprNodes::CaseExprCfgNode).getBranch(_)
     or
-    exists(CfgNodes::ExprCfgNode exprTo, ExprReturnNode n |
+    exists(CfgNodes::ExprCfgNode exprTo, ReturningStatementNode n |
       nodeFrom = n and
       exprTo = nodeTo.asExpr() and
-      n.getKind() instanceof BreakReturnKind and
+      n.getReturningNode().getNode() instanceof BreakStmt and
       exprTo.getNode() instanceof Loop and
-      nodeTo.asExpr().getAPredecessor(any(SuccessorTypes::BreakSuccessor s)) = n.getExprNode()
+      nodeTo.asExpr().getAPredecessor(any(SuccessorTypes::BreakSuccessor s)) = n.getReturningNode()
     )
     or
-    nodeFrom.asExpr() = nodeTo.(ExprReturnNode).getExprNode().getReturnedValueNode()
+    nodeFrom.asExpr() = nodeTo.(ReturningStatementNode).getReturningNode().getReturnedValueNode()
     or
     nodeTo.asExpr() =
       any(CfgNodes::ExprNodes::ForExprCfgNode for |
@@ -182,6 +182,28 @@ class SsaDefinitionNode extends NodeImpl, TSsaDefinitionNode {
   override string toStringImpl() { result = def.toString() }
 }
 
+/**
+ * A value returning statement, viewed as a node in a data flow graph.
+ *
+ * Note that because of control-flow splitting, one `ReturningStmt` may correspond
+ * to multiple `ReturningStatementNode`s, just like it may correspond to multiple
+ * `ControlFlow::Node`s.
+ */
+class ReturningStatementNode extends NodeImpl, TReturningNode {
+  private CfgNodes::ReturningCfgNode n;
+
+  ReturningStatementNode() { this = TReturningNode(n) }
+
+  /** Gets the expression corresponding to this node. */
+  CfgNodes::ReturningCfgNode getReturningNode() { result = n }
+
+  override CfgScope getCfgScope() { result = n.getScope() }
+
+  override Location getLocationImpl() { result = n.getLocation() }
+
+  override string toStringImpl() { result = n.toString() }
+}
+
 private module ParameterNodes {
   abstract private class ParameterNodeImpl extends ParameterNode, NodeImpl { }
 
@@ -241,29 +263,49 @@ abstract class ReturnNode extends Node {
 }
 
 private module ReturnNodes {
+  private predicate isValid(CfgNodes::ReturningCfgNode node) {
+    exists(ReturningStmt stmt, Callable scope |
+      stmt = node.getNode() and
+      scope = node.getScope()
+    |
+      stmt instanceof ReturnStmt and
+      (scope instanceof Method or scope instanceof SingletonMethod or scope instanceof Lambda)
+      or
+      stmt instanceof NextStmt and
+      (scope instanceof Block or scope instanceof Lambda)
+      or
+      stmt instanceof BreakStmt and
+      (scope instanceof Block or scope instanceof Lambda)
+    )
+  }
+
   /**
    * A data-flow node that represents an expression returned by a callable,
    * either using an explict `return` statement or as the expression of a method body.
    */
-  class ExprReturnNode extends ReturnNode, NodeImpl, TReturningNode {
+  class ExplicitReturnNode extends ReturnNode, ReturningStatementNode {
     private CfgNodes::ReturningCfgNode n;
 
-    ExprReturnNode() { this = TReturningNode(n) }
-
-    /** Gets the statement corresponding to this node. */
-    CfgNodes::ReturningCfgNode getExprNode() { result = n }
+    ExplicitReturnNode() {
+      isValid(this.getReturningNode()) and
+      n.getASuccessor().(CfgNodes::AnnotatedExitNode).isNormal() and
+      n.getScope() instanceof Callable
+    }
 
     override ReturnKind getKind() {
       if n.getNode() instanceof BreakStmt
       then result instanceof BreakReturnKind
       else result instanceof NormalReturnKind
     }
+  }
 
-    override CfgScope getCfgScope() { result = n.getScope() }
+  class ExprReturnNode extends ReturnNode, ExprNode {
+    ExprReturnNode() {
+      this.getExprNode().getASuccessor().(CfgNodes::AnnotatedExitNode).isNormal() and
+      this.getEnclosingCallable() instanceof Callable
+    }
 
-    override Location getLocationImpl() { result = n.getLocation() }
-
-    override string toStringImpl() { result = n.toString() }
+    override ReturnKind getKind() { result instanceof NormalReturnKind }
   }
 }