Reintroduce hidden then/else/do in AST; include all in CFG

Author: hvitved

ql/src/codeql_ruby/ast/Expr.qll

@@ -57,16 +57,6 @@ class ArgumentList extends Expr, TArgumentList {
 class StmtSequence extends Expr, TStmtSequence {
   override string getAPrimaryQlClass() { result = "StmtSequence" }
 
-  override string toString() {
-    exists(int c | c = this.getNumberOfStatements() |
-      c = 0 and result = ";"
-      or
-      c = 1 and result = this.getStmt(0).toString()
-      or
-      c > 1 and result = "...; ..."
-    )
-  }
-
   /** Gets the `n`th statement in this sequence. */
   Stmt getStmt(int n) { none() }
 
@@ -91,6 +81,8 @@ private class Then extends StmtSequence, TThen {
   Then() { this = TThen(g) }
 
   override Stmt getStmt(int n) { toGenerated(result) = g.getChild(n) }
+
+  final override string toString() { result = "then ..." }
 }
 
 private class Else extends StmtSequence, TElse {
@@ -99,6 +91,8 @@ private class Else extends StmtSequence, TElse {
   Else() { this = TElse(g) }
 
   override Stmt getStmt(int n) { toGenerated(result) = g.getChild(n) }
+
+  final override string toString() { result = "else ..." }
 }
 
 private class Do extends StmtSequence, TDo {
@@ -107,6 +101,8 @@ private class Do extends StmtSequence, TDo {
   Do() { this = TDo(g) }
 
   override Stmt getStmt(int n) { toGenerated(result) = g.getChild(n) }
+
+  final override string toString() { result = "do ..." }
 }
 
 private class Ensure extends StmtSequence, TEnsure {
@@ -212,13 +208,7 @@ class ParenthesizedExpr extends StmtSequence, TParenthesizedExpr {
 
   final override string getAPrimaryQlClass() { result = "ParenthesizedExpr" }
 
-  final override string toString() {
-    exists(int c | c = this.getNumberOfStatements() |
-      c = 0 and result = "()"
-      or
-      c > 0 and result = "(" + StmtSequence.super.toString() + ")"
-    )
-  }
+  final override string toString() { result = "( ... )" }
 
   final override Stmt getStmt(int n) { toGenerated(result) = g.getChild(n) }
 }

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

@@ -94,10 +94,10 @@ private module Cached {
       not strictcount(int i | exists(g.getParent().(Generated::LeftAssignmentList).getChild(i))) = 1
     } or
     TDivExpr(Generated::Binary g) { g instanceof @binary_slash } or
-    TDo(Generated::Do g) { exists(g.getChild(_)) } or
+    TDo(Generated::Do g) or
     TDoBlock(Generated::DoBlock g) { not g.getParent() instanceof Generated::Lambda } or
     TElementReference(Generated::ElementReference g) or
-    TElse(Generated::Else g) { exists(g.getChild(_)) } or
+    TElse(Generated::Else g) or
     TElsif(Generated::Elsif g) or
     TEmptyStmt(Generated::EmptyStatement g) or
     TEndBlock(Generated::EndBlock g) or
@@ -198,7 +198,7 @@ private module Cached {
     TSubshellLiteral(Generated::Subshell g) or
     TSymbolArrayLiteral(Generated::SymbolArray g) or
     TTernaryIfExpr(Generated::Conditional g) or
-    TThen(Generated::Then g) { exists(g.getChild(_)) } or
+    TThen(Generated::Then g) or
     TTokenConstantAccess(Generated::Constant g) {
       // A tree-sitter `constant` token is a read of that constant in any context
       // where an identifier would be a vcall.

ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll

@@ -643,17 +643,10 @@ module Trees {
 
     final override predicate first(AstNode first) { first(this.getCondition(), first) }
 
-    private AstNode getConditionSucc() {
-      result = this.getBody()
-      or
-      not exists(this.getBody()) and
-      result = this.getCondition()
-    }
-
     final override predicate succ(AstNode pred, AstNode succ, Completion c) {
       last(this.getCondition(), pred, c) and
       this.entersLoopWhenConditionIs(c.(BooleanCompletion).getValue()) and
-      first(this.getConditionSucc(), succ)
+      first(this.getBody(), succ)
       or
       last(this.getBody(), pred, c) and
       first(this.getCondition(), succ) and
@@ -1117,21 +1110,6 @@ module Trees {
     }
   }
 
-  private class TSimpleHiddenStmtSequenceTree =
-    ASTInternal::TElse or ASTInternal::TThen or ASTInternal::TDo;
-
-  private class SimpleHiddenStmtSequenceTree extends StmtSequenceTree, TSimpleHiddenStmtSequenceTree {
-    final override predicate propagatesAbnormal(AstNode child) { child = this.getAStmt() }
-
-    final override predicate first(AstNode first) { first(this.getStmt(0), first) }
-
-    final override predicate last(AstNode last, Completion c) { last(this.getLastStmt(), last, c) }
-
-    final override predicate succ(AstNode pred, AstNode succ, Completion c) {
-      StmtSequenceTree.super.succ(pred, succ, c)
-    }
-  }
-
   /**
    * Control-flow tree for any pre-order StmtSequence that doesn't have a more
    * specific implementation.
@@ -1144,8 +1122,7 @@ module Trees {
       not this instanceof EndBlock and
       not this instanceof StringInterpolationComponent and
       not this instanceof Block and
-      not this instanceof ParenthesizedExpr and
-      not this instanceof TSimpleHiddenStmtSequenceTree
+      not this instanceof ParenthesizedExpr
     }
 
     final override predicate propagatesAbnormal(AstNode child) { child = this.getAStmt() }