Java: Should `FunctionalExpr` be a non-extending subtype of `ClassInstanceExpr`?

[Marcono1234]

Description of the issue
Currently the Java CodeQL class FunctionalExpr extends ClassInstanceExpr. This makes it inherit a lot of redundant predicates, and also some misleading ones such as getQualifier.

For example for the Java method reference this::toString, using the CodeQL MemberRefExpr::getQualifier has no result because it is not actually the qualifier (or to match the name of the other predicate, "receiver") of the method reference, that is this, but the qualifier of the creation of the synthetic anonymous class.

Since the fact that functional expressions are modelled as ClassInstanceExpr is kind of an implementation detail of the CodeQL Java extractor, would it make sense and be possible to change FunctionalExpr to not extend ClassInstanceExpr but instead let is be a "non-extending subtype" (... instanceof ClassInstanceExpr)?

In the CodeQL Java library code there are a few usages of ClassInstanceExpr predicates on a FunctionalExpr (getAnonymousClass() and getConstructedType()), but they seem to handle certain implementation details of how functional expressions are modelled as ClassInstanceExpr. Maybe if necessary a getPreciseType() / getInternalType() predicate could be added to FunctionalExpr to still allow retrieving this information.

What do you think? Maybe I am overlooking something though, and switching to a non-extending subtype has undesired side effects.

[aschackmull]

That's quite possible, yes. The non-extending subtype feature is relatively new (compared to the definition of these classes), so it was never a consideration when the class hierarchy was originally written.
However, to make such a change we'd have to carefully consider to what degree it would be a breaking change and how to best mitigate that.

[Marcono1234]

Thanks for the answer! Should I give this change a try and create a pull request, or is this rather something you as maintainers would want to do at some point (if you want to do this at all) due to the potential negative effects this could have?

[aschackmull]

I'd lean towards not doing this at all unless there were some very compelling additional reasons.

[Marcono1234]

Ok, feel free to close this issue then if you want.