Error in Base Query right after Data Flow Analysis Integration —— Empty Relation

[oriana19993926782]

Hello CodeQL Community,

Background:
The base version of my query, which I refer to as the "Basic Query", runs successfully and yields expected results. This query identifies unit tests, sifts out specific assertions and finds out expected output of specific assertions. However, when I extend this query to include data flow analysis (referred to as the "Advanced Query"), I encounter an error.

Error Message: The Advanced Query fails with the following error: “Failed to run query: ERROR: empty relation: 'argument2' in type test”. Interestingly, this error occurs in a part of the query that is common to both the Basic and Advanced versions.

Basic Query:

import java

class AssertMethodCall extends MethodAccess {
  AssertMethodCall() {
    this.getMethod().getDeclaringType().getQualifiedName().matches("%Assert%") and
    this.getEnclosingCallable() instanceof TestMethod
  }
}

from TestMethod tm, AssertMethodCall assertMethodCall, Callable focalM, Expr argument
, Expr expected, Expr argument2, MethodAccess focalMethodCall
where 
  assertMethodCall.getEnclosingCallable() = tm and
  argument = assertMethodCall.getAnArgument() and
  focalMethodCall = argument.(MethodAccess) and
  focalM = focalMethodCall.getCallee() and
  focalM.fromSource() and
  not focalM instanceof GetterMethod and
  not focalM.hasNoParameters() 
  and argument2 = assertMethodCall.getAnArgument() 
  and not argument = argument2 
  and
  ((
    not argument2 instanceof MethodAccess
    and expected =  argument2 
    ) 
    or
    (
    argument2 instanceof MethodAccess and
    assertMethodCall.toString().matches("assertThat%") and
    (argument2.(MethodAccess).getMethod().getName().toString().matches("is")
    or argument2.(MethodAccess).getMethod().getName().toString().matches("instanceOf"))
    and expected = argument2.(MethodAccess).getArgument(_)
    )
  )

  select 
    tm,
    tm.getBody().getLocation(),
    tm.getBody(),
    assertMethodCall, 
    assertMethodCall.getLocation() as assertLoc, 
    expected.getLocation()

Advanced Query(additional variables and logic for data flow analysis on the expected output of the assertion in unit tests, if being a variable).

import java
import semmle.code.java.dataflow.DataFlow

class AssertMethodCall extends MethodAccess {
  AssertMethodCall() {
    this.getMethod().getDeclaringType().getQualifiedName().matches("%Assert%") and
    this.getEnclosingCallable() instanceof TestMethod
  }
}

from TestMethod tm, AssertMethodCall assertMethodCall, Callable focalM, Expr argument
, Expr expected, Expr argument2, MethodAccess focalMethodCall
, VarAccess expectedVariable, Variable var, Expr varInit, DataFlow::Node declaredNode, DataFlow::Node endNode

where
  tm = assertMethodCall.getEnclosingCallable() and
  argument = assertMethodCall.getAnArgument() and
  focalMethodCall = argument.(MethodAccess) and
  focalM = focalMethodCall.getCallee() and
  focalM.fromSource() and
  not focalM instanceof GetterMethod and
  not focalM.hasNoParameters() and
  argument2 = assertMethodCall.getAnArgument() and
  not argument2 = argument 
  and
  ((
    not argument2 instanceof MethodAccess
    and expected =  argument2 
    ) 
    or
    (
    argument2 instanceof MethodAccess and
    assertMethodCall.toString().matches("assertThat%") and
    (argument2.(MethodAccess).getMethod().getName().toString().matches("is")
    or argument2.(MethodAccess).getMethod().getName().toString().matches("instanceOf"))
    and expected = argument2.(MethodAccess).getArgument(_)
    )
  )

  and expectedVariable = expected.(VarAccess) and
  var = expectedVariable.getVariable() and
  varInit = var.getAnAssignedValue() and
  declaredNode = DataFlow::exprNode(varInit) 

  and DataFlow::localFlow(declaredNode, endNode) 
  and endNode.getLocation().getStartLine() < assertMethodCall.getLocation().getStartLine()
  and endNode.getEnclosingCallable().getFile() = assertMethodCall.getEnclosingCallable().getFile()

select
  tm,
  assertMethodCall.getLocation(),
  expected, 
  var,
  endNode.(DataFlow::ExprNode).asExpr().getEnclosingStmt()

Issue Observed:
The error seems to be related to the argument2 variable, which is used in both versions of the query.
Despite the error occurring in a shared portion of both queries, the Basic Query runs without issues, suggesting that argument2 is correctly initialized and used in that context.

I am puzzled as to why this error is occurring in the "basic query part" of Advanced Query, especially since the Basic Query, which contains same logic for argument2, runs correctly. I have reviewed the queries for any potential issues but have not been able to identify the cause of the problem.

The java project under question is https://github.com/apache/shardingsphere-elasticjob.

I am reaching out to the community for insights or suggestions on what might be causing this issue and how to resolve it. Any assistance or guidance would be greatly appreciated.

Thank you in advance for your help!

[smowton]

The full error says "...in type test" -- this is telling you that the type test not argument2 instanceof MethodAccess is pointless. We can see why by noting we also have constraints expectedVariable = expected.(VarAccess) and expected = argument2 -- hence we know that for any result to be returned, expected and argument2 must be a VarAccess, and VarAccess is disjoint with MethodAccess, therefore the test not argument2 instanceof MethodAccess is redundant and can be deleted.

A few other little hints while I'm here:

• argument is always cast to MethodAccess, and is always equal to focalMethodCall, so we can drop focalMethodCall = argument.(MethodAccess) and just use focalMethodCall.
• not argument2 = focalMethodCall is better written argument2 != focalMethodCall
• getName() returns a string, so toString is redundant
• matches without using the _ or % wildcards is just equality, so you can replace it with =
• You can use a set literal [a, b] to replace a disjunction of the same expression with each of a and b
• expected is always equal to expectedVariable, so we can delete expected and substitute expectedVariable everywhere.
• Using both an inline cast argument2.(MethodAccess) and argument2 instanceof MethodAccess is redundant -- the inline cast already requires that argument2 is a MethodAccess.
• Using getMethod().getName() is preferred to using toString, because toString's format is for human consumption and could change.
• getArgument(_) is better written getAnArgument()
• asExpr is declared on DataFlow::Node -- no need to cast to an ExprNode.

Here's the cleaned up version:

import java
import semmle.code.java.dataflow.DataFlow

class AssertMethodCall extends MethodAccess {
  AssertMethodCall() {
    this.getMethod().getDeclaringType().getQualifiedName().matches("%Assert%") and
    this.getEnclosingCallable() instanceof TestMethod
  }
}

from
  TestMethod tm, AssertMethodCall assertMethodCall, Callable focalM, Expr argument2,
  MethodAccess focalMethodCall, VarAccess expectedVariable, Variable var, Expr varInit,
  DataFlow::Node declaredNode, DataFlow::Node endNode
where
  tm = assertMethodCall.getEnclosingCallable() and
  focalMethodCall = assertMethodCall.getAnArgument() and
  focalM = focalMethodCall.getCallee() and
  focalM.fromSource() and
  not focalM instanceof GetterMethod and
  not focalM.hasNoParameters() and
  argument2 = assertMethodCall.getAnArgument() and
  argument2 != focalMethodCall and
  (
    expectedVariable = argument2
    or
    assertMethodCall.getMethod().getName().matches("assertThat%") and
    argument2.(MethodAccess).getMethod().getName() = ["is", "instanceOf"] and
    expectedVariable = argument2.(MethodAccess).getAnArgument()
  ) and
  var = expectedVariable.getVariable() and
  varInit = var.getAnAssignedValue() and
  declaredNode = DataFlow::exprNode(varInit) and
  DataFlow::localFlow(declaredNode, endNode) and
  endNode.getLocation().getStartLine() < assertMethodCall.getLocation().getStartLine() and
  endNode.getEnclosingCallable().getFile() = assertMethodCall.getEnclosingCallable().getFile()
select tm, assertMethodCall.getLocation(), expectedVariable, var,
  endNode.asExpr().getEnclosingStmt()

[oriana19993926782]

Thank you so so so much! Now I understand where I went wrong. My previous approach was to identify all potential expected outputs, regardless of whether they were variables, and then narrow down to those that were variables for the data flow analysis. It's like I was using two chained queries, silly of me. I realize now that if the expected output I'm interested in should be a variable, I should approach it as such right from the start. Thank you for solving this problem and for taking the extra time to tidy up my query. It's much cleaner now, and I've learned a lot from your help! 💗💗💗