Merge pull request #7394 from aibaars/ruby-cfg-expr-post

Ruby: CFG: make all expressions "post-order" nodes

Author: aibaars

ruby/ql/consistency-queries/CfgConsistency.ql

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

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

@@ -361,22 +361,22 @@ class CaseExpr extends ControlExpr instanceof CaseExprImpl {
   final Expr getValue() { result = super.getValue() }
 
   /**
-   * Gets the `n`th branch of this case expression, either a `WhenExpr`, an
+   * Gets the `n`th branch of this case expression, either a `WhenClause`, an
    * `InClause`, or a `StmtSequence`.
    */
-  final Expr getBranch(int n) { result = super.getBranch(n) }
+  final AstNode getBranch(int n) { result = super.getBranch(n) }
 
   /**
-   * Gets a branch of this case expression, either a `WhenExpr`, an
+   * Gets a branch of this case expression, either a `WhenClause`, an
    * `InClause`, or a `StmtSequence`.
    */
-  final Expr getABranch() { result = this.getBranch(_) }
+  final AstNode getABranch() { result = this.getBranch(_) }
 
   /** Gets the `n`th `when` branch of this case expression. */
-  deprecated final WhenExpr getWhenBranch(int n) { result = this.getBranch(n) }
+  deprecated final WhenClause getWhenBranch(int n) { result = this.getBranch(n) }
 
   /** Gets a `when` branch of this case expression. */
-  deprecated final WhenExpr getAWhenBranch() { result = this.getABranch() }
+  deprecated final WhenClause getAWhenBranch() { result = this.getABranch() }
 
   /** Gets the `else` branch of this case expression, if any. */
   final StmtSequence getElseBranch() { result = this.getABranch() }
@@ -401,6 +401,11 @@ class CaseExpr extends ControlExpr instanceof CaseExprImpl {
   }
 }
 
+/**
+ * DEPRECATED: Use `WhenClause` instead.
+ */
+deprecated class WhenExpr = WhenClause;
+
 /**
  * A `when` branch of a `case` expression.
  * ```rb
@@ -409,12 +414,12 @@ class CaseExpr extends ControlExpr instanceof CaseExprImpl {
  * end
  * ```
  */
-class WhenExpr extends Expr, TWhenExpr {
+class WhenClause extends AstNode, TWhenClause {
   private Ruby::When g;
 
-  WhenExpr() { this = TWhenExpr(g) }
+  WhenClause() { this = TWhenClause(g) }
 
-  final override string getAPrimaryQlClass() { result = "WhenExpr" }
+  final override string getAPrimaryQlClass() { result = "WhenClause" }
 
   /** Gets the body of this case-when expression. */
   final Stmt getBody() { toGenerated(result) = g.getBody() }
@@ -461,7 +466,7 @@ class WhenExpr extends Expr, TWhenExpr {
  * end
  * ```
  */
-class InClause extends Expr, TInClause {
+class InClause extends AstNode, TInClause {
   private Ruby::InClause g;
 
   InClause() { this = TInClause(g) }

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

@@ -305,7 +305,7 @@ private module Cached {
     TUntilExpr(Ruby::Until g) or
     TUntilModifierExpr(Ruby::UntilModifier g) or
     TVariableReferencePattern(Ruby::VariableReferencePattern g) or
-    TWhenExpr(Ruby::When g) or
+    TWhenClause(Ruby::When g) or
     TWhileExpr(Ruby::While g) or
     TWhileModifierExpr(Ruby::WhileModifier g) or
     TYieldCall(Ruby::Yield g)
@@ -344,7 +344,7 @@ private module Cached {
         TTernaryIfExpr or TThen or TTokenConstantAccess or TTokenMethodName or TTokenSuperCall or
         TToplevel or TTrueLiteral or TUnaryMinusExpr or TUnaryPlusExpr or TUndefStmt or
         TUnlessExpr or TUnlessModifierExpr or TUntilExpr or TUntilModifierExpr or
-        TVariableReferencePattern or TWhenExpr or TWhileExpr or TWhileModifierExpr or TYieldCall;
+        TVariableReferencePattern or TWhenClause or TWhileExpr or TWhileModifierExpr or TYieldCall;
 
   class TAstNodeSynth =
     TAddExprSynth or TAssignExprSynth or TBitwiseAndExprSynth or TBitwiseOrExprSynth or
@@ -511,7 +511,7 @@ private module Cached {
     n = TUntilExpr(result) or
     n = TUntilModifierExpr(result) or
     n = TVariableReferencePattern(result) or
-    n = TWhenExpr(result) or
+    n = TWhenClause(result) or
     n = TWhileExpr(result) or
     n = TWhileModifierExpr(result) or
     n = TYieldCall(result)
@@ -657,10 +657,9 @@ class TSelf = TSelfReal or TSelfSynth;
 class TDestructuredLhsExpr = TDestructuredLeftAssignment or TLeftAssignmentList;
 
 class TExpr =
-  TSelf or TArgumentList or TInClause or TRescueClause or TRescueModifierExpr or TPair or
-      TStringConcatenation or TCall or TBlockArgument or TConstantAccess or TControlExpr or
-      TWhenExpr or TLiteral or TCallable or TVariableAccess or TStmtSequence or TOperation or
-      TForwardArgument or TDestructuredLhsExpr;
+  TSelf or TArgumentList or TRescueClause or TRescueModifierExpr or TPair or TStringConcatenation or
+      TCall or TBlockArgument or TConstantAccess or TControlExpr or TLiteral or TCallable or
+      TVariableAccess or TStmtSequence or TOperation or TForwardArgument or TDestructuredLhsExpr;
 
 class TSplatExpr = TSplatExprReal or TSplatExprSynth;
 

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

@@ -5,17 +5,17 @@ private import codeql.ruby.ast.internal.AST
 abstract class CaseExprImpl extends ControlExpr, TCase {
   abstract Expr getValue();
 
-  abstract Expr getBranch(int n);
+  abstract AstNode getBranch(int n);
 }
 
-class CaseWhenExpr extends CaseExprImpl, TCaseExpr {
+class CaseWhenClause extends CaseExprImpl, TCaseExpr {
   private Ruby::Case g;
 
-  CaseWhenExpr() { this = TCaseExpr(g) }
+  CaseWhenClause() { this = TCaseExpr(g) }
 
   final override Expr getValue() { toGenerated(result) = g.getValue() }
 
-  final override Expr getBranch(int n) {
+  final override AstNode getBranch(int n) {
     toGenerated(result) = g.getChild(n) or
     toGenerated(result) = g.getChild(n)
   }
@@ -28,7 +28,7 @@ class CaseMatch extends CaseExprImpl, TCaseMatch {
 
   final override Expr getValue() { toGenerated(result) = g.getValue() }
 
-  final override Expr getBranch(int n) {
+  final override AstNode getBranch(int n) {
     toGenerated(result) = g.getClauses(n)
     or
     n = count(g.getClauses(_)) and toGenerated(result) = g.getElse()

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

@@ -338,7 +338,7 @@ module ExprNodes {
   }
 
   private class CaseExprChildMapping extends ExprChildMapping, CaseExpr {
-    override predicate relevantChild(Expr e) { e = this.getValue() or e = this.getBranch(_) }
+    override predicate relevantChild(Expr e) { e = this.getValue() }
   }
 
   /** A control-flow node that wraps a `MethodCall` AST expression. */
@@ -356,11 +356,6 @@ module ExprNodes {
 
     /** Gets the expression being compared, if any. */
     final ExprCfgNode getValue() { e.hasCfgChild(e.getValue(), this, result) }
-
-    /**
-     * Gets the `n`th branch of this case expression.
-     */
-    final ExprCfgNode getBranch(int n) { e.hasCfgChild(e.getBranch(n), this, result) }
   }
 
   private class ConditionalExprChildMapping extends ExprChildMapping, ConditionalExpr {

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

@@ -107,11 +107,7 @@ abstract class Completion extends TCompletion {
     n = any(RescueModifierExpr parent).getBody() and
     this = [TSimpleCompletion().(TCompletion), TRaiseCompletion()]
     or
-    (
-      mayRaise(n)
-      or
-      n instanceof CaseMatch and not exists(n.(CaseExpr).getElseBranch())
-    ) and
+    mayRaise(n) and
     (
       this = TRaiseCompletion()
       or
@@ -209,7 +205,7 @@ private predicate inBooleanContext(AstNode n) {
   or
   n = any(StmtSequence parent | inBooleanContext(parent)).getLastStmt()
   or
-  exists(CaseExpr c, WhenExpr w |
+  exists(CaseExpr c, WhenClause w |
     not exists(c.getValue()) and
     c.getABranch() = w and
     w.getPattern(_) = n
@@ -231,7 +227,7 @@ private predicate mustHaveMatchingCompletion(AstNode n) {
 private predicate inMatchingContext(AstNode n) {
   n = any(RescueClause r).getException(_)
   or
-  exists(CaseExpr c, WhenExpr w |
+  exists(CaseExpr c, WhenClause w |
     exists(c.getValue()) and
     c.getABranch() = w and
     w.getPattern(_) = n