Java: LambdaExpr.getExprBody() parent is always ReturnStmt

[Marcono1234]

Description of the issue
Currently QL always pretends that the parent of a LambdaExpr.getExprBody() is a ReturnStmt, however that is incorrect.
If the lambda implements an interface method with void as return type, the parent must not be ReturnStmt, but should be ExprStmt instead.

Example query:

import java

class RunMethod extends Method {
  RunMethod() {
    getDeclaringType().hasQualifiedName("java.lang", "Runnable")
    and hasStringSignature("run()")
  }
}

from LambdaExpr lambda, Expr expr
where
  expr = lambda.getExprBody()
  and expr.getParent() instanceof ReturnStmt
  and lambda.asMethod().getAnOverride*() instanceof RunMethod
select expr

The current behavior likely causes false negatives for multiple queries in this repository because they check expr.getParent() instanceof ExprStmt to see if the return value of the call is discarded.

Edit: This apparently also affects the implicit method which MemberRefExpr.asMethod() has as result. And there the implicit method also has the wrong return type, e.g. run(): String for a Runnable r = this::toString();.

So there are three bugs:

• LambdaExpr.asMethod() has ReturnStmt instead of ExprStmt for target interface methods with void as return type
• MemberRefExpr.asMethod() has ReturnStmt instead of ExprStmt for target interface methods with void as return type
• MemberRefExpr.asMethod() has wrong Method.getReturnType() (i.e. non-void) for target interface methods with void as return type

[aschackmull]

You are right that this is technically wrong. However, looking into this I found that our extractor matches the QL implementation and always emits a ReturnStmt containing the expression body - even if the body has type void. So as it currently stands this means that the getExprBody() predicate actually works as it should, as long as one doesn't peek into the body of the implicit method defined by the lambda. We should probably fix this in both the extractor and the QL code, but since it likely doesn't cause problems at the moment, it'll take a while for us to get to.

[Marcono1234]

as long as one doesn't peek into the body of the implicit method defined by the lambda
[...]
but since it likely doesn't cause problems at the moment, it'll take a while for us to get to.

The case mentioned above where a expr.getParent() instanceof ExprStmt check is used to find out if the result of a method call is discarded (see also QL built-in query) does not explicitly peek into the lambda body, but is still affected because it would not find something like this:

Runnable r = () -> getClass().getResourceAsStream​("test");

Though such cases are probably not that common so this might not be that critical.

[Marcono1234]

Actually the expr.getParent() instanceof ExprStmt check also misses update expressions in a for statement, and probably switch expressions whose result is unused.