Python: At most one **kwargs ParameterNode per callable

Similar to the Ruby changes from
github#11461

I feel the change to `DataFlowFunciton.getParameter` where we use
`not exists(func.getArgByName(_))` is not very great, but I was not allowed
to use `not exists(this.getParameter(any(ParameterPosition _).isKeyword(_)))`
because of negative recursion.

Author: RasmusWL

python/ql/lib/semmle/python/dataflow/new/internal/DataFlowDispatch.qll

@@ -321,6 +321,7 @@ abstract class DataFlowFunction extends DataFlowCallable, TFunction {
     or
     exists(string name | ppos.isKeyword(name) | result.getParameter() = func.getArgByName(name))
     or
+    // `*args`
     exists(int index |
       (
         ppos.isStarArgs(index) and
@@ -343,9 +344,22 @@ abstract class DataFlowFunction extends DataFlowCallable, TFunction {
       not exists(func.getArg(_)) and index = 0
     )
     or
-    ppos.isDictSplat() and result.getParameter() = func.getKwarg()
-    or
-    ppos.isDictSplat() and result = TSynthDictSplatParameterNode(this)
+    // `**kwargs`
+    // since dataflow library has restriction that we can only have ONE result per
+    // parameter position, if there is both a synthetic **kwargs and a real **kwargs
+    // parameter, we only give the result for the synthetic, and add local flow from the
+    // synthetic to the real. It might seem more natural to do it in the other
+    // direction, but since we have a clearStep on the real **kwargs parameter, we that
+    // content-clearing would also affect the synthetic parameter, which we don't want.
+    (
+      not exists(func.getArgByName(_)) and
+      ppos.isDictSplat() and
+      result.getParameter() = func.getKwarg()
+      or
+      exists(func.getArgByName(_)) and
+      ppos.isDictSplat() and
+      result = TSynthDictSplatParameterNode(this)
+    )
   }
 }
 
@@ -1400,7 +1414,16 @@ class SummaryParameterNode extends ParameterNodeImpl, TSummaryParameterNode {
   override Parameter getParameter() { none() }
 
   override predicate isParameterOf(DataFlowCallable c, ParameterPosition ppos) {
-    sc = c.asLibraryCallable() and ppos = pos
+    sc = c.asLibraryCallable() and
+    ppos = pos and
+    // avoid overlap with `SynthDictSplatParameterNode`
+    not (
+      pos.isDictSplat() and
+      exists(ParameterPosition keywordPos |
+        FlowSummaryImpl::Private::summaryParameterNodeRange(sc, keywordPos) and
+        keywordPos.isKeyword(_)
+      )
+    )
   }
 
   override DataFlowCallable getEnclosingCallable() { result.asLibraryCallable() = sc }

python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll

@@ -224,8 +224,11 @@ private predicate synthDictSplatArgumentNodeStoreStep(
 private predicate dictSplatParameterNodeClearStep(ParameterNode n, DictionaryElementContent c) {
   exists(DataFlowCallable callable, ParameterPosition dictSplatPos, ParameterPosition keywordPos |
     dictSplatPos.isDictSplat() and
-    n = callable.getParameter(dictSplatPos) and
-    not n instanceof SynthDictSplatParameterNode and
+    (
+      n.getParameter() = callable.(DataFlowFunction).getScope().getKwarg()
+      or
+      n = TSummaryParameterNode(callable.asLibraryCallable(), dictSplatPos)
+    ) and
     exists(callable.getParameter(keywordPos)) and
     keywordPos.isKeyword(c.getKey())
   )
@@ -276,6 +279,31 @@ class SynthDictSplatParameterNode extends ParameterNodeImpl, TSynthDictSplatPara
   override Parameter getParameter() { none() }
 }
 
+/**
+ * Flow step from the synthetic `**kwargs` parameter to the real `**kwargs` parameter.
+ * Due to restriction in dataflow library, we can only give one of them as result for
+ * `DataFlowCallable.getParameter`, so this is a workaround to ensure there is flow to
+ * _both_ of them.
+ */
+private predicate dictSplatParameterNodeFlowStep(
+  ParameterNodeImpl nodeFrom, ParameterNodeImpl nodeTo
+) {
+  exists(DataFlowCallable callable |
+    nodeFrom = TSynthDictSplatParameterNode(callable) and
+    (
+      nodeTo.getParameter() = callable.(DataFlowFunction).getScope().getKwarg()
+      or
+      exists(ParameterPosition pos |
+        nodeTo = TSummaryParameterNode(callable.asLibraryCallable(), pos) and
+        pos.isDictSplat()
+      )
+    )
+  )
+}
+
+/**
+ * Reads from the synthetic **kwargs parameter to each keyword parameter.
+ */
 predicate synthDictSplatParameterNodeReadStep(
   SynthDictSplatParameterNode nodeFrom, DictionaryElementContent c, ParameterNode nodeTo
 ) {
@@ -418,6 +446,8 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
   simpleLocalFlowStepForTypetracking(nodeFrom, nodeTo)
   or
   summaryFlowSteps(nodeFrom, nodeTo)
+  or
+  dictSplatParameterNodeFlowStep(nodeFrom, nodeTo)
 }
 
 /**

python/ql/test/experimental/dataflow/consistency/dataflow-consistency.expected

@@ -20,7 +20,5 @@ argHasPostUpdate
 postWithInFlow
 viableImplInCallContextTooLarge
 uniqueParameterNodeAtPosition
-| test.py:239:1:239:42 | Function overflowCallee | ** | test.py:239:1:239:42 | SynthDictSplatParameterNode | Parameters with overlapping positions. |
-| test.py:239:1:239:42 | Function overflowCallee | ** | test.py:239:35:239:40 | ControlFlowNode for kwargs | Parameters with overlapping positions. |
 uniqueParameterNodePosition
 uniqueContentApprox

python/ql/test/experimental/dataflow/coverage/dataflow-consistency.expected

@@ -20,15 +20,5 @@ argHasPostUpdate
 postWithInFlow
 viableImplInCallContextTooLarge
 uniqueParameterNodeAtPosition
-| argumentPassing.py:50:1:60:2 | Function argument_passing | ** | argumentPassing.py:50:1:60:2 | SynthDictSplatParameterNode | Parameters with overlapping positions. |
-| argumentPassing.py:50:1:60:2 | Function argument_passing | ** | argumentPassing.py:59:7:59:7 | ControlFlowNode for g | Parameters with overlapping positions. |
-| argumentPassing.py:185:1:185:23 | Function mixed | ** | argumentPassing.py:185:1:185:23 | SynthDictSplatParameterNode | Parameters with overlapping positions. |
-| argumentPassing.py:185:1:185:23 | Function mixed | ** | argumentPassing.py:185:16:185:21 | ControlFlowNode for kwargs | Parameters with overlapping positions. |
-| classes.py:441:5:441:41 | Function __prepare__ | ** | classes.py:441:5:441:41 | SynthDictSplatParameterNode | Parameters with overlapping positions. |
-| classes.py:441:5:441:41 | Function __prepare__ | ** | classes.py:441:36:441:39 | ControlFlowNode for kwds | Parameters with overlapping positions. |
-| test.py:407:1:407:28 | Function f_extra_keyword | ** | test.py:407:1:407:28 | SynthDictSplatParameterNode | Parameters with overlapping positions. |
-| test.py:407:1:407:28 | Function f_extra_keyword | ** | test.py:407:26:407:26 | ControlFlowNode for b | Parameters with overlapping positions. |
-| test.py:521:23:521:43 | Function lambda | ** | test.py:521:23:521:43 | SynthDictSplatParameterNode | Parameters with overlapping positions. |
-| test.py:521:23:521:43 | Function lambda | ** | test.py:521:35:521:35 | ControlFlowNode for b | Parameters with overlapping positions. |
 uniqueParameterNodePosition
 uniqueContentApprox