I think I've previously had the same issue as the one you are solving here.

Could there be an idea in changing DataFlow::Node::asExpr() to include reflective calls, or lift this method to DataFlow::Node?

In any case getExpr might not be the most descriptive name.

asExprInclReflectiveCalls? (But that name feels too long, feel free to ignore).

Could there be an idea in changing DataFlow::Node::asExpr()

I think that leads to a recursion mess, and also a mess of contepts. I would prefer the second option:

lift this method to DataFlow::Node

But a good name escapes me. asUnderlyingExpr is my best suggestion.

My main gripe with using asExpr() is that two data flow nodes can suddenly map to the same expression, which brings along its own set of new pitfalls. But maybe we should do it anyway? It's essentially the same issue as parameter nodes not having AST nodes.

The general solution is to take the thing you know is an expression (getTest()), map that to a data-flow node, and follow up with getImmediatePredecessor, getAPredecessor, or getALocalSource. The third version I mentioned in the PR description used getImmediatePredecessor but I backed it out again, because it required more work for that generalization to actually work beyond reflective calls.

I think I've previously had the same issue as the one you are solving here.

@erik-krogh Can you find out what it was? It might help us figure out which solution works best.

@erik-krogh Can you find out what it was? It might help us figure out which solution works best.

From looking around I can't immediately find an instance of me solving the issue, but I think that is because I ended up not solving it.
The issue definitely exists in UseOfReturnlessFunction, which has no explicit handling of reflective calls, and DataFlow::CallNode::asExpr() is therefore none().

As a consequence, UseOfReturnlessFunction has an FP on the last line in this example:

function onlySideEffects() {
  console.log("Boo!")
}
var dead1 = onlySideEffects(); // Correctly not flagged
var dead2 = onlySideEffects.call(null); // FP! flagged by UseOfReturnlessFunction

How about DataFlow::Node::getEnclosingExpr() or getEnclosingAstNode()? This would map reflective calls to the CallExpr, parameter nodes to the Parameter, and ValueNode to the AST node.

(To elaborate on my previous point, extending asExpr() and flow() can lead to unfortunate cases where code like expr.flow().(CallNode).getArgument(0) returns the first two arguments.)

Anyways, I think it's a bit out of scope for this PR, but I'm happy to continue the discussion here. Ping @max-schaefer.

I'm currently doing a dist-compare on an implementation of DataFlow::Node::getEnclosingExpr().

Expr getEnclosingExpr() {
  result = asExpr() or
  this = DataFlow::reflectiveCallNode(result) or
  result = this.(ParameterNode).getParameter()
}

The evaluation is on UseOfReturnlessFunction, where i changed some asExpr() to getEnclosingExpr(), which definitely removes some false positives.
If the results are good I'll land it in a PR.