Ruby: Use shared SuccessorType.

Author: aschackmull

ruby/ql/consistency-queries/DataFlowConsistency.ql

@@ -28,7 +28,7 @@ private module Input implements InputSig<Location, RubyDataFlow> {
     exists(CfgNodes::ExprCfgNode n |
       arg.argumentOf(call, _) and
       n = call.asCall() and
-      arg.asExpr().getASuccessor(any(SuccessorTypes::ConditionalSuccessor c)).getASuccessor*() = n and
+      arg.asExpr().getASuccessor(any(ConditionalSuccessor c)).getASuccessor*() = n and
       n.getASplit() instanceof Split::ConditionalCompletionSplit
     )
   }

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

@@ -8,7 +8,6 @@ private import codeql.ruby.ast.internal.TreeSitter
 private import codeql.ruby.controlflow.ControlFlowGraph
 private import internal.ControlFlowGraphImpl as CfgImpl
 private import CfgNodes
-private import SuccessorTypes
 private import CfgImpl::BasicBlocks as BasicBlocksImpl
 
 /**

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

@@ -2,9 +2,9 @@
 overlay[local]
 module;
 
+import codeql.controlflow.SuccessorType
 private import codeql.ruby.AST
 private import codeql.ruby.controlflow.BasicBlocks
-private import SuccessorTypes
 private import internal.ControlFlowGraphImpl as CfgImpl
 private import internal.Splitting as Splitting
 private import internal.Completion
@@ -59,241 +59,6 @@ class CfgNode extends CfgImpl::Node {
   BasicBlock getBasicBlock() { result.getANode() = this }
 }
 
-/** The type of a control flow successor. */
-class SuccessorType extends CfgImpl::TSuccessorType {
-  /** Gets a textual representation of successor type. */
-  string toString() { none() }
-}
-
-/** Provides different types of control flow successor types. */
-module SuccessorTypes {
-  /** A normal control flow successor. */
-  class NormalSuccessor extends SuccessorType, CfgImpl::TSuccessorSuccessor {
-    final override string toString() { result = "successor" }
-  }
-
-  /**
-   * A conditional control flow successor. Either a Boolean successor (`BooleanSuccessor`)
-   * or a matching successor (`MatchingSuccessor`)
-   */
-  class ConditionalSuccessor extends SuccessorType {
-    boolean value;
-
-    ConditionalSuccessor() {
-      this = CfgImpl::TBooleanSuccessor(value) or
-      this = CfgImpl::TMatchingSuccessor(value)
-    }
-
-    /** Gets the Boolean value of this successor. */
-    final boolean getValue() { result = value }
-
-    override string toString() { result = this.getValue().toString() }
-  }
-
-  /**
-   * A Boolean control flow successor.
-   *
-   * For example, in
-   *
-   * ```rb
-   * if x >= 0
-   *   puts "positive"
-   * else
-   *   puts "negative"
-   * end
-   * ```
-   *
-   * `x >= 0` has both a `true` successor and a `false` successor.
-   */
-  class BooleanSuccessor extends ConditionalSuccessor, CfgImpl::TBooleanSuccessor { }
-
-  /**
-   * A matching control flow successor.
-   *
-   * For example, this program fragment:
-   *
-   * ```rb
-   * case x
-   *   when 1 then puts "one"
-   *   else puts "not one"
-   * end
-   * ```
-   *
-   * has a control flow graph containing matching successors:
-   *
-   * ```
-   *            x
-   *            |
-   *            1
-   *           / \
-   *          /   \
-   *         /     \
-   *        /       \
-   *     match    non-match
-   *       |          |
-   *  puts "one"  puts "not one"
-   * ```
-   */
-  class MatchingSuccessor extends ConditionalSuccessor, CfgImpl::TMatchingSuccessor {
-    override string toString() { if value = true then result = "match" else result = "no-match" }
-  }
-
-  /**
-   * A `return` control flow successor.
-   *
-   * Example:
-   *
-   * ```rb
-   * def sum(x,y)
-   *   return x + y
-   * end
-   * ```
-   *
-   * The exit node of `sum` is a `return` successor of the `return x + y`
-   * statement.
-   */
-  class ReturnSuccessor extends SuccessorType, CfgImpl::TReturnSuccessor {
-    final override string toString() { result = "return" }
-  }
-
-  /**
-   * A `break` control flow successor.
-   *
-   * Example:
-   *
-   * ```rb
-   * def m
-   *   while x >= 0
-   *     x -= 1
-   *     if num > 100
-   *       break
-   *     end
-   *   end
-   *   puts "done"
-   * end
-   * ```
-   *
-   * The node `puts "done"` is `break` successor of the node `break`.
-   */
-  class BreakSuccessor extends SuccessorType, CfgImpl::TBreakSuccessor {
-    final override string toString() { result = "break" }
-  }
-
-  /**
-   * A `next` control flow successor.
-   *
-   * Example:
-   *
-   * ```rb
-   * def m
-   *   while x >= 0
-   *     x -= 1
-   *     if num > 100
-   *       next
-   *     end
-   *   end
-   *   puts "done"
-   * end
-   * ```
-   *
-   * The node `x >= 0` is `next` successor of the node `next`.
-   */
-  class NextSuccessor extends SuccessorType, CfgImpl::TNextSuccessor {
-    final override string toString() { result = "next" }
-  }
-
-  /**
-   * A `redo` control flow successor.
-   *
-   * Example:
-   *
-   * Example:
-   *
-   * ```rb
-   * def m
-   *   while x >= 0
-   *     x -= 1
-   *     if num > 100
-   *       redo
-   *     end
-   *   end
-   *   puts "done"
-   * end
-   * ```
-   *
-   * The node `x -= 1` is `redo` successor of the node `redo`.
-   */
-  class RedoSuccessor extends SuccessorType, CfgImpl::TRedoSuccessor {
-    final override string toString() { result = "redo" }
-  }
-
-  /**
-   * A `retry` control flow successor.
-   *
-   * Example:
-   *
-   * Example:
-   *
-   * ```rb
-   * def m
-   *   begin
-   *     puts "Retry"
-   *     raise
-   *   rescue
-   *     retry
-   *   end
-   * end
-   * ```
-   *
-   * The node `puts "Retry"` is `retry` successor of the node `retry`.
-   */
-  class RetrySuccessor extends SuccessorType, CfgImpl::TRetrySuccessor {
-    final override string toString() { result = "retry" }
-  }
-
-  /**
-   * An exceptional control flow successor.
-   *
-   * Example:
-   *
-   * ```rb
-   * def m x
-   *   if x > 2
-   *     raise "x > 2"
-   *   end
-   *   puts "x <= 2"
-   * end
-   * ```
-   *
-   * The exit node of `m` is an exceptional successor of the node
-   * `raise "x > 2"`.
-   */
-  class RaiseSuccessor extends SuccessorType, CfgImpl::TRaiseSuccessor {
-    final override string toString() { result = "raise" }
-  }
-
-  /**
-   * An exit control flow successor.
-   *
-   * Example:
-   *
-   * ```rb
-   * def m x
-   *   if x > 2
-   *     exit 1
-   *   end
-   *   puts "x <= 2"
-   * end
-   * ```
-   *
-   * The exit node of `m` is an exit successor of the node
-   * `exit 1`.
-   */
-  class ExitSuccessor extends SuccessorType, CfgImpl::TExitSuccessor {
-    final override string toString() { result = "exit" }
-  }
-}
-
 class Split = Splitting::Split;
 
 /** Provides different kinds of control flow graph splittings. */

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

@@ -12,7 +12,6 @@ private import codeql.ruby.ast.internal.Control
 private import codeql.ruby.controlflow.ControlFlowGraph
 private import ControlFlowGraphImpl as CfgImpl
 private import NonReturning
-private import SuccessorTypes
 
 private newtype TCompletion =
   TSimpleCompletion() or
@@ -267,7 +266,7 @@ abstract private class NonNestedNormalCompletion extends NormalCompletion { }
 
 /** A simple (normal) completion. */
 class SimpleCompletion extends NonNestedNormalCompletion, TSimpleCompletion {
-  override NormalSuccessor getAMatchingSuccessorType() { any() }
+  override DirectSuccessor getAMatchingSuccessorType() { any() }
 
   override string toString() { result = "simple" }
 }
@@ -359,7 +358,7 @@ class BreakCompletion extends Completion {
     this = TNestedCompletion(_, TBreakCompletion(), _)
   }
 
-  override BreakSuccessor getAMatchingSuccessorType() { any() }
+  override JumpSuccessor getAMatchingSuccessorType() { any() }
 
   override string toString() {
     // `NestedCompletion` defines `toString()` for the other case
@@ -377,7 +376,7 @@ class NextCompletion extends Completion {
     this = TNestedCompletion(_, TNextCompletion(), _)
   }
 
-  override NextSuccessor getAMatchingSuccessorType() { any() }
+  override JumpSuccessor getAMatchingSuccessorType() { any() }
 
   override string toString() {
     // `NestedCompletion` defines `toString()` for the other case
@@ -395,7 +394,7 @@ class RedoCompletion extends Completion {
     this = TNestedCompletion(_, TRedoCompletion(), _)
   }
 
-  override RedoSuccessor getAMatchingSuccessorType() { any() }
+  override JumpSuccessor getAMatchingSuccessorType() { any() }
 
   override string toString() {
     // `NestedCompletion` defines `toString()` for the other case
@@ -413,7 +412,7 @@ class RetryCompletion extends Completion {
     this = TNestedCompletion(_, TRetryCompletion(), _)
   }
 
-  override RetrySuccessor getAMatchingSuccessorType() { any() }
+  override JumpSuccessor getAMatchingSuccessorType() { any() }
 
   override string toString() {
     // `NestedCompletion` defines `toString()` for the other case
@@ -431,7 +430,7 @@ class RaiseCompletion extends Completion {
     this = TNestedCompletion(_, TRaiseCompletion(), _)
   }
 
-  override RaiseSuccessor getAMatchingSuccessorType() { any() }
+  override ExceptionSuccessor getAMatchingSuccessorType() { any() }
 
   override string toString() {
     // `NestedCompletion` defines `toString()` for the other case
@@ -512,7 +511,7 @@ class NestedBreakCompletion extends NormalCompletion, NestedCompletion {
 
   override SuccessorType getAMatchingSuccessorType() {
     outer instanceof SimpleCompletion and
-    result instanceof BreakSuccessor
+    result instanceof JumpSuccessor
     or
     result = outer.(ConditionalCompletion).getAMatchingSuccessorType()
   }

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

@@ -50,17 +50,13 @@ private module CfgInput implements CfgShared::InputSig<Location> {
 
   SuccessorType getAMatchingSuccessorType(Completion c) { result = c.getAMatchingSuccessorType() }
 
-  predicate successorTypeIsSimple(SuccessorType t) {
-    t instanceof Cfg::SuccessorTypes::NormalSuccessor
-  }
+  predicate successorTypeIsSimple(SuccessorType t) { t instanceof Cfg::DirectSuccessor }
 
-  predicate successorTypeIsCondition(SuccessorType t) {
-    t instanceof Cfg::SuccessorTypes::ConditionalSuccessor
-  }
+  predicate successorTypeIsCondition(SuccessorType t) { t instanceof Cfg::ConditionalSuccessor }
 
   predicate isAbnormalExitType(SuccessorType t) {
-    t instanceof Cfg::SuccessorTypes::RaiseSuccessor or
-    t instanceof Cfg::SuccessorTypes::ExitSuccessor
+    t instanceof Cfg::ExceptionSuccessor or
+    t instanceof Cfg::ExitSuccessor
   }
 
   private predicate id(Ruby::AstNode node1, Ruby::AstNode node2) { node1 = node2 }
@@ -1528,21 +1524,3 @@ CfgScope getCfgScope(AstNode n) {
     pragma[only_bind_into](result) = getCfgScopeImpl(n0)
   )
 }
-
-cached
-private module Cached {
-  cached
-  newtype TSuccessorType =
-    TSuccessorSuccessor() or
-    TBooleanSuccessor(boolean b) { b in [false, true] } or
-    TMatchingSuccessor(boolean isMatch) { isMatch in [false, true] } or
-    TReturnSuccessor() or
-    TBreakSuccessor() or
-    TNextSuccessor() or
-    TRedoSuccessor() or
-    TRetrySuccessor() or
-    TRaiseSuccessor() or // TODO: Add exception type?
-    TExitSuccessor()
-}
-
-import Cached

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

@@ -6,7 +6,7 @@ private import codeql.ruby.CFG
 /** Holds if the guard `guard` controls block `bb` upon evaluating to `branch`. */
 pragma[nomagic]
 predicate guardControlsBlock(CfgNodes::AstCfgNode guard, BasicBlock bb, boolean branch) {
-  exists(ConditionBlock conditionBlock, SuccessorTypes::ConditionalSuccessor s |
+  exists(ConditionBlock conditionBlock, ConditionalSuccessor s |
     guard = conditionBlock.getLastNode() and
     s.getValue() = branch and
     conditionBlock.edgeDominates(bb, s)