Ruby: Adopt shared ConditionalCompletionSplitting implementation

Author: hvitved

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

@@ -22,7 +22,6 @@ class AstNode extends Ast::AstNode {
 private module CfgInput implements CfgShared::InputSig<Location> {
   private import ControlFlowGraphImpl as Impl
   private import Completion as Comp
-  private import Splitting as Splitting
   private import codeql.ruby.CFG as Cfg
 
   class AstNode = Impl::AstNode;
@@ -45,10 +44,6 @@ private module CfgInput implements CfgShared::InputSig<Location> {
     scope.(Impl::CfgScopeImpl).exit(last, c)
   }
 
-  class SplitKindBase = Splitting::TSplitKind;
-
-  class Split = Splitting::Split;
-
   class SuccessorType = Cfg::SuccessorType;
 
   SuccessorType getAMatchingSuccessorType(Completion c) { result = c.getAMatchingSuccessorType() }
@@ -67,7 +62,21 @@ private module CfgInput implements CfgShared::InputSig<Location> {
   }
 }
 
-import CfgShared::Make<Location, CfgInput>
+private module CfgSplittingInput implements CfgShared::SplittingInputSig<Location, CfgInput> {
+  private import Splitting as S
+
+  class SplitKindBase = S::TSplitKind;
+
+  class Split = S::Split;
+}
+
+private module ConditionalCompletionSplittingInput implements
+  CfgShared::ConditionalCompletionSplittingInputSig<Location, CfgInput, CfgSplittingInput>
+{
+  import Splitting::ConditionalCompletionSplitting::ConditionalCompletionSplittingInput
+}
+
+import CfgShared::MakeWithSplitting<Location, CfgInput, CfgSplittingInput, ConditionalCompletionSplittingInput>
 
 abstract class CfgScopeImpl extends AstNode {
   abstract predicate entry(AstNode first);

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

@@ -3,7 +3,8 @@
  */
 
 private import codeql.ruby.AST as Ast
-private import Completion
+private import Completion as Comp
+private import Comp
 private import ControlFlowGraphImpl
 private import SuccessorTypes
 private import codeql.ruby.controlflow.ControlFlowGraph
@@ -31,7 +32,7 @@ class Split extends TSplit {
   string toString() { none() }
 }
 
-private module ConditionalCompletionSplitting {
+module ConditionalCompletionSplitting {
   /**
    * A split for conditional completions. For example, in
    *
@@ -51,67 +52,61 @@ private module ConditionalCompletionSplitting {
 
     ConditionalCompletionSplit() { this = TConditionalCompletionSplit(completion) }
 
+    ConditionalCompletion getCompletion() { result = completion }
+
     override string toString() { result = completion.toString() }
   }
 
-  private class ConditionalCompletionSplitKind extends SplitKind, TConditionalCompletionSplitKind {
+  private class ConditionalCompletionSplitKind_ extends SplitKind, TConditionalCompletionSplitKind {
     override int getListOrder() { result = 0 }
 
     override predicate isEnabled(AstNode n) { this.appliesTo(n) }
 
     override string toString() { result = "ConditionalCompletion" }
   }
 
-  int getNextListOrder() { result = 1 }
+  module ConditionalCompletionSplittingInput {
+    private import Completion as Comp
 
-  private class ConditionalCompletionSplitImpl extends SplitImpl instanceof ConditionalCompletionSplit
-  {
-    ConditionalCompletion completion;
+    class ConditionalCompletion = Comp::ConditionalCompletion;
 
-    ConditionalCompletionSplitImpl() { this = TConditionalCompletionSplit(completion) }
+    class ConditionalCompletionSplitKind extends ConditionalCompletionSplitKind_, TSplitKind { }
 
-    override ConditionalCompletionSplitKind getKind() { any() }
+    class ConditionalCompletionSplit = ConditionalCompletionSplitting::ConditionalCompletionSplit;
 
-    override predicate hasEntry(AstNode pred, AstNode succ, Completion c) {
-      succ(pred, succ, c) and
-      last(succ, _, completion) and
+    bindingset[parent, parentCompletion]
+    predicate condPropagateExpr(
+      AstNode parent, ConditionalCompletion parentCompletion, AstNode child,
+      ConditionalCompletion childCompletion
+    ) {
+      child = parent.(Ast::NotExpr).getOperand() and
+      childCompletion.(BooleanCompletion).getDual() = parentCompletion
+      or
+      childCompletion = parentCompletion and
       (
-        last(succ.(Ast::NotExpr).getOperand(), pred, c) and
-        completion.(BooleanCompletion).getDual() = c
-        or
-        last(succ.(Ast::LogicalAndExpr).getAnOperand(), pred, c) and
-        completion = c
+        child = parent.(Ast::LogicalAndExpr).getAnOperand()
         or
-        last(succ.(Ast::LogicalOrExpr).getAnOperand(), pred, c) and
-        completion = c
+        child = parent.(Ast::LogicalOrExpr).getAnOperand()
         or
-        last(succ.(Ast::StmtSequence).getLastStmt(), pred, c) and
-        completion = c
+        child = parent.(Ast::StmtSequence).getLastStmt()
         or
-        last(succ.(Ast::ConditionalExpr).getBranch(_), pred, c) and
-        completion = c
+        child = parent.(Ast::ConditionalExpr).getBranch(_)
       )
-      or
-      succ(pred, succ, c) and
-      succ instanceof Ast::WhenClause and
-      completion = c
     }
+  }
 
-    override predicate hasEntryScope(CfgScope scope, AstNode succ) { none() }
+  int getNextListOrder() { result = 1 }
 
-    override predicate hasExit(AstNode pred, AstNode succ, Completion c) {
-      this.appliesTo(pred) and
+  private class ConditionalCompletionSplitImpl extends SplitImplementations::ConditionalCompletionSplitting::ConditionalCompletionSplitImpl
+  {
+    override predicate hasEntry(AstNode pred, AstNode succ, Completion c) {
+      super.hasEntry(pred, succ, c)
+      or
+      // a non-standard case is needed for `when` clauses
       succ(pred, succ, c) and
-      if c instanceof ConditionalCompletion then completion = c else any()
-    }
-
-    override predicate hasExitScope(CfgScope scope, AstNode last, Completion c) {
-      this.appliesTo(last) and
-      succExit(scope, last, c) and
-      if c instanceof ConditionalCompletion then completion = c else any()
+      succ instanceof Ast::WhenClause and
+      c = this.getCompletion()
     }
-
-    override predicate hasSuccessor(AstNode pred, AstNode succ, Completion c) { none() }
   }
 }