eclipse-jdt / eclipse-jdt/eclipse.jdt.core
[Annotations] Order-dependent inherited method annotations affect `@CheckReturnValue` consumers
- Dominant language
- Java
- Stars
- 237
- Forks
- 195
- Avg merge
- 1d 12h
- Merged PRs (30d)
- 47
Description
## Summary
When a method invocation is resolved against several override-equivalent inherited methods, the effective annotation information visible to downstream tools depends on declaration order (`a, b` vs `b, a` / `a & b` vs `b & a`).
This is understandable for ecj's own null analysis, which has dedicated semantics for nullness annotations. But for third-party annotations, the current behavior is difficult for tools and users because equivalent-looking source declarations can expose different annotation information depending only on order.
This issue is a follow-up to #4297, as suggested here: https://github.com/eclipse-jdt/eclipse.jdt.core/issues/4297#issuecomment-4519871318
## Practical use case
This visitor interprets an annotation `@CheckReturnValue` (I used the official one from ` javax.annotation.CheckReturnValue` but that is actually irrelevant since the comparison is done by "simple name" i.e. `CheckReturnValue`) and wants to decide whether ignoring the result of a method invocation should be reported.
```java
public class CheckReturnValueVisitor extends ASTVisitor {
private List _problems;
private CompilationUnit _cu;
public CheckReturnValueVisitor(List problems, CompilationUnit cu) {
_problems = problems;
_cu = cu;
}
@Override
public boolean visit(ExpressionStatement node) {
Expression expression = node.getExpression();
if (!(expression instanceof MethodInvocation)) {
return true;
}
MethodInvocation invocation = (MethodInvocation) expression;
if (hasCheckReturnValue(invocation)) {
_problems.add(new CheckReturnValueIgnored(_cu, node, invocation));
return false;
}
return true;
}
/**
* @return true if the method is annotated with @CheckReturnValue (no matter which package it comes from).
*/
private static boolean hasCheckReturnValue(MethodInvocation invocation) {
IMethodBinding binding = invocation.resolveMethodBinding();
if (binding != null) {
for (IAnnotationBinding a : binding.getAnnotations()) {
if ("CheckReturnValue".equals(a.getAnnotationType().getName())) {
return true;
}
}
}
return false;
}
}
```
That tool needs annotation information for the invoked method. In the presence of inherited override-equivalent methods, order dependence means that the invocation may appear to have different annotation information depending on whether the source declares `a, b` or `b, a`, even though the programmer intent looks equivalent.
From a user perspective this is surprising and hard to reason about.
## Expected / desired behavior
Consider this snippet:
```java
import javax.annotation.CheckReturnValue;
interface A {
@CheckReturnValue int method();
}
interface B {
int method();
}
interface I1 extends A, B {
default void test() {
method(); // order A,B --> ERROR
}
}
interface I2 extends B, A {
default void test() {
method(); // order B,A --> OK
}
}
```
I understand from the discussion in #4297 that JLS/JSR 308 does not define one merged annotated signature for arbitrary third-party annotations, so I am not assuming ecj must invent semantics for `@CheckReturnValue`.
Still, I would like to understand whether JDT/ecj could expose this situation in a way that is more stable and tool-friendly, for example by making it possible for downstream tools to reliably discover all relevant candidate methods/annotations rather than one order-dependent view.
In other words, if ecj/JDT is not meant to assign semantics to `@CheckReturnValue`, can it at least expose enough information so that tools which do assign semantics can do so consistently? In the code presented above (see `CheckReturnValueVisitor`), I see that the issue is that I am using `invocation.resolveMethodBinding()` to get **the** method binding (either coming from `A` or from `B`), but this means that the decision about _which method to honor_ has already been made by `org.eclipse.jdt.core.dom.MethodInvocation.resolveMethodBinding()`.
## Questions
1. Which JDT/ecj API is the intended one for a downstream tool to use here?
2. Is there something better than `org.eclipse.jdt.core.dom.MethodInvocation.resolveMethodBinding()`?
If I had a way to know that `method()` is declared in several classes/interfaces (`A` and `B`) then I would be able to "merge" them and I could decide whether or not to (return `true` from `CheckReturnValueVisitor ::hasCheckReturnValue(...)` and ergo whether or not to) report a problem in `CheckReturnValueVisitor::visit(...)`.
Contributor guide
Assessment
This issue has not been assessed yet.