Java: Exclude qualifier argument for existing models

[tausbn]

Excludes candidates for Argument[this] where we already have a model that covers a different argument of the containing call.

[kaeluka]

Thank you :) <3 ✨

[jhelie]

Should we merge this or do we need input from @github/codeql-java ?

[atorralba]

Does this exclusion require that the models are manual? Otherwise this could increasingly become a problem, as more and more AI-generated models come into play, because those don't guarantee any completeness regarding the rest of the parameters of the related callable.

[kaeluka]

This comment is 10/10, @atorralba — really well spotted ❤️! I'm taking this PR over, as Taus is 🌴, and just pushed a fix to this. Could you take a look?

[atorralba]

I added a small suggestion to further improve OtherArgumentToModeledMethodCharacteristic::appliesToEndpoint, but otherwise LGTM 👍

[atorralba]

This isn't going to always work if otherSink is an instance argument (this), or if either e or otherSink are variadic arguments. I think we could take advantage of ArgumentNode here:

Suggested change

	c = otherSink.asExpr().(Argument).getCall() and
	e.asExpr() in [c.getQualifier(), c.getAnArgument()] and
	e.getCall() = otherSink.(ArgumentNode).getCall()

This implies that Call c isn't needed at the exists, and that we add a getCall predicate to ArgumentNode. If we also considered ImplicitVarargsArray as part of the definition of ArgumentNode it would be the cherry on top:

private class ArgumentNode extends DataFlow::Node {
  Call c;

  ArgumentNode() {
    exists(Argument arg | this.asExpr() = arg and not arg.isVararg() and c = arg.getCall())
    or
    this.(DataFlow::ImplicitVarargsArray).getCall() = c
    or
    this = DataFlow::getInstanceArgument(c)
  }

  Call getCall() { result = c }
}

[kaeluka]

13027a1 I think this commit implements your suggestion :) thank you :) :)