Ruby: Use new parameter position for synthetic hash-splat instead

We wanted to ensure that a callable did not have multiple parameters
with same parameter position. Originally we fixed this with
e0bd210. This commit reverts that and
solves it by introducing a new parameter position instead.

Author: RasmusWL

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll

@@ -441,6 +441,13 @@ private module Cached {
       FlowSummaryImplSpecific::ParsePositions::isParsedKeywordArgumentPosition(_, name)
     } or
     THashSplatParameterPosition() or
+    // To get flow from a hash-splat argument to a keyword parameter, we add a read-step
+    // from a synthetic hash-splat parameter. We need this separate synthetic ParameterNode,
+    // since we clear content of the normal hash-splat parameter for the names that
+    // correspond to normal keyword parameters. Since we cannot re-use the same parameter
+    // position for multiple parameter nodes in the same callable, we introduce this
+    // synthetic parameter position.
+    TSynthHashSplatParameterPosition() or
     TSplatAllParameterPosition() or
     TAnyParameterPosition() or
     TAnyKeywordParameterPosition()
@@ -1238,6 +1245,8 @@ class ParameterPosition extends TParameterPosition {
   /** Holds if this position represents a hash-splat parameter. */
   predicate isHashSplat() { this = THashSplatParameterPosition() }
 
+  predicate isSynthHashSplat() { this = TSynthHashSplatParameterPosition() }
+
   predicate isSplatAll() { this = TSplatAllParameterPosition() }
 
   /**
@@ -1263,6 +1272,8 @@ class ParameterPosition extends TParameterPosition {
     or
     this.isHashSplat() and result = "**"
     or
+    this.isSynthHashSplat() and result = "synthetic **"
+    or
     this.isSplatAll() and result = "*"
     or
     this.isAny() and result = "any"
@@ -1345,6 +1356,8 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
   or
   ppos.isHashSplat() and apos.isHashSplat()
   or
+  ppos.isSynthHashSplat() and apos.isHashSplat()
+  or
   ppos.isSplatAll() and apos.isSplatAll()
   or
   ppos.isAny() and argumentPositionIsNotSelf(apos)

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll

@@ -189,18 +189,6 @@ module LocalFlow {
   }
 
   predicate localFlowStepCommon(Node nodeFrom, Node nodeTo) {
-    exists(DataFlowCallable c | nodeFrom = TSynthHashSplatParameterNode(c) |
-      exists(HashSplatParameter p |
-        p.getCallable() = c.asCallable() and
-        nodeTo = TNormalParameterNode(p)
-      )
-      or
-      exists(ParameterPosition pos |
-        nodeTo = TSummaryParameterNode(c.asLibraryCallable(), pos) and
-        pos.isHashSplat()
-      )
-    )
-    or
     localSsaFlowStep(nodeFrom, nodeTo)
     or
     nodeFrom.asExpr() = nodeTo.asExpr().(CfgNodes::ExprNodes::BlockArgumentCfgNode).getValue()
@@ -648,9 +636,7 @@ private module ParameterNodes {
           )
         or
         parameter = callable.getAParameter().(HashSplatParameter) and
-        pos.isHashSplat() and
-        // avoid overlap with `SynthHashSplatParameterNode`
-        not callable.getAParameter() instanceof KeywordParameter
+        pos.isHashSplat()
         or
         parameter = callable.getParameter(0).(SplatParameter) and
         pos.isSplatAll()
@@ -780,7 +766,7 @@ private module ParameterNodes {
     final override Parameter getParameter() { none() }
 
     final override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
-      c = callable and pos.isHashSplat()
+      c = callable and pos.isSynthHashSplat()
     }
 
     final override CfgScope getCfgScope() { result = callable.asCallable() }
@@ -802,16 +788,7 @@ private module ParameterNodes {
     override Parameter getParameter() { none() }
 
     override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
-      sc = c.asLibraryCallable() and
-      pos = pos_ and
-      // avoid overlap with `SynthHashSplatParameterNode`
-      not (
-        pos.isHashSplat() and
-        exists(ParameterPosition keywordPos |
-          FlowSummaryImpl::Private::summaryParameterNodeRange(sc, keywordPos) and
-          keywordPos.isKeyword(_)
-        )
-      )
+      sc = c.asLibraryCallable() and pos = pos_
     }
 
     override CfgScope getCfgScope() { none() }