JS: Make ValueNode the ParameterNode with a step to the SSA node

Author: asgerf

javascript/ql/src/semmle/javascript/dataflow/Configuration.qll

@@ -1601,6 +1601,9 @@ class MidPathNode extends PathNode, MkMidNode {
     nd.(DataFlow::SsaDefinitionNode).getSsaVariable().getDefinition() instanceof
       SsaImplicitDefinition
     or
+    // Skip SSA definition of parameter as its location coincides with the parameter node
+    nd = DataFlow::ssaDefinitionNode(SSA::definition(any(SimpleParameter p)))
+    or
     // Skip to the top of big left-leaning string concatenation trees.
     nd = any(AddExpr add).flow() and
     nd = any(AddExpr add).getAnOperand().flow()

javascript/ql/src/semmle/javascript/dataflow/DataFlow.qll

@@ -713,10 +713,6 @@ module DataFlow {
         parameterNode(paramNode, param)
       |
         result = paramNode
-        or
-        // special case: there is no SSA flow step for unused parameters
-        paramNode instanceof UnusedParameterNode and
-        result = param.getDefault().flow()
       )
     }
 
@@ -877,32 +873,6 @@ module DataFlow {
     override string getPropertyName() { none() }
   }
 
-  /**
-   * A data flow node representing an unused parameter.
-   *
-   * This case exists to ensure all parameters have a corresponding data-flow node.
-   * In most cases, parameters are represented by SSA definitions or destructuring pattern nodes.
-   */
-  private class UnusedParameterNode extends DataFlow::Node, TUnusedParameterNode {
-    SimpleParameter p;
-
-    UnusedParameterNode() { this = TUnusedParameterNode(p) }
-
-    override string toString() { result = p.toString() }
-
-    override ASTNode getAstNode() { result = p }
-
-    override BasicBlock getBasicBlock() { result = p.getBasicBlock() }
-
-    override predicate hasLocationInfo(
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      p.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
-
-    override File getFile() { result = p.getFile() }
-  }
-
   /**
    * A data flow node representing an HTML attribute.
    */
@@ -1256,7 +1226,9 @@ module DataFlow {
   /**
    * INTERNAL: Use `parameterNode(Parameter)` instead.
    */
-  predicate parameterNode(DataFlow::Node nd, Parameter p) { nd = lvalueNode(p) }
+  predicate parameterNode(DataFlow::Node nd, Parameter p) {
+    nd = valueNode(p)
+  }
 
   /**
    * INTERNAL: Use `thisNode(StmtContainer container)` instead.
@@ -1309,8 +1281,6 @@ module DataFlow {
     )
     or
     result = TValueNode(lvalue.(DestructuringPattern))
-    or
-    result = TUnusedParameterNode(lvalue)
   }
 
   /**
@@ -1365,6 +1335,11 @@ module DataFlow {
       succ = lvalueNode(def.getTarget())
     )
     or
+    exists(SimpleParameter param |
+      pred = valueNode(param) and // The value node represents the incoming argument
+      succ = lvalueNode(param) // The SSA node represents the parameters's local variable
+    )
+    or
     exists(PropertyPattern pattern |
       pred = TPropNode(pattern) and
       succ = lvalueNode(pattern.getValuePattern())
@@ -1576,8 +1551,6 @@ module DataFlow {
     exists(PropertyPattern p | nd = TPropNode(p)) and cause = "heap"
     or
     nd instanceof TElementPatternNode and cause = "heap"
-    or
-    nd instanceof UnusedParameterNode and cause = "call"
   }
 
   /**

javascript/ql/src/semmle/javascript/dataflow/internal/BasicExprTypeInference.qll

@@ -416,3 +416,23 @@ private class AnalyzedOptionalChainExpr extends DataFlow::AnalyzedValueNode {
     result = TAbstractUndefined()
   }
 }
+
+/**
+ * Flow analysis for parameter AST nodes.
+ *
+ * For legacy reasons this node takes its value from the SSA variable node,
+ * even though the SSA variable node is a successor of this node.
+ */
+private class AnalyzedParameterValueNode extends AnalyzedNode {
+  Parameter p;
+
+  AnalyzedParameterValueNode() {
+    DataFlow::parameterNode(this, p)
+  }
+
+  override AbstractValue getAValue() {
+    result = p.(AnalyzedVarDef).getAnRhsValue()
+    or
+    result = TIndefiniteAbstractValue("call")
+  }
+}

javascript/ql/src/semmle/javascript/dataflow/internal/DataFlowNode.qll

@@ -22,7 +22,6 @@ newtype TNode =
     (kind = "call" or kind = "apply")
   } or
   TThisNode(StmtContainer f) { f.(Function).getThisBinder() = f or f instanceof TopLevel } or
-  TUnusedParameterNode(SimpleParameter p) { not exists(SSA::definition(p)) } or
   TDestructuredModuleImportNode(ImportDeclaration decl) {
     exists(decl.getASpecifier().getImportedName())
   } or