Bugzilla – Full Text Bug Listing
| Summary: | No warning if unused private method is annotated with @Nullable | ||
|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Björn Michael <b.michael> |
| Component: | Core | Assignee: | Stephan Herrmann <stephan.herrmann> |
| Status: | CLOSED DUPLICATE | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | register.eclipse, stephan.herrmann |
| Version: | 4.7.1 | ||
| Target Milestone: | 4.8 M3 | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
Unexpected as it may seem, this is by design. Please read bug 438244 for background / rationale. *** This bug has been marked as a duplicate of bug 438244 *** Then it is a bug by design and stays a bug.
As it is now it is not possible to trust to compiler warnings anymore.
Furthermore @Inject can't be compared to @Nullable regarding their usages.
Why does this example work as excpected? "The method unusedvarargmethod(Object...) from the type B is never used locally" is shown.
@SafeVarargs
private static void unusedvarargmethod(Object... objs) {
// ...
}
Mark it as duplicate again if you still think it is ok for you.
(In reply to Björn Michael from comment #2) > Then it is a bug by design and stays a bug. Let me explain more, why in principle I don't consider this a bug: For some annotations the "unused" warning is desired, for some annotations the warning is wrong because the annotation may indicate an actual usage (frameworks or annotation processors may take the annotation as an instruction to actually use the member - which the compiler cannot see). Unfortunately, the number of annotation types in the Java eco system is several magnitudes larger than the number of compiler engineers, so we have no chance to special-case each and every annotation in the compiler. Please see that this issue has a long history, e.g. - bug 365437 - bug 376590 - bug 386692 Now, specifically for @Nullable we actually have a special branch to ensure that those do not suppress the "unused" warning (because the compiler has been enhanced to "know" about null annotations). Please check: did you configure that exact annotation type for annotation based null analysis? - If so, the warning should be shown - otherwise we have a bug. - If not, then the annotation is s.t. the compiler does not know about and so it has to pessimistically assume usage by a framework or annotation processor. Leaving the bug open for clarification about your configuration. If 'Enable annotation-based null analysis' is turned OFF and this is currently the default setting then unused warning is missing for @Nullable annotated method.
If 'Enable annotation-based null analysis' is turned ON and javax.annotation.Nullable is correctly configured then unused warning is shown for the same method.
But let's assume this
@CheckReturnValue
@Nullable
private static Object unusedMethod() {
return null;
}
now it is broken again.
(In reply to Björn Michael from comment #4) All this happens for good reasons. The compiler must be careful to never flag a member as unused where in fact it could be relevant. A user removing a relevant member due to this warning is far worse than an expected warning not being raised. Hence, flagging an annotated member as unused can only happen, when the compiler "understands" that none of the annotations signal a relevant use. The compiler knows the semantics of annotations that are configured for null analysis. Neither a disabled @Nullable nor @CheckReturnValue are "understood" by the compiler so we are not on safe grounds for telling the user to remove the member. Note, that for any change in this area, it doesn't suffice to convince me, but all participants in linked bugs have their opinions, too. The current state is the best we could agree upon after long discussions - which shouldn't imply this is a weak compromise, the solution does make sense. Let me mention a possible direction for improvement: it has been discussed whether users could provide custom lists of annotations that they consider as relevant use / no relevant use. PRO: This seems to be only option to cover all possible annotations. Difficult: We would have to carefully explain the implications of providing a custom (black/white) list, and still face that some users will blame JDT for "wrong" warnings based on their own blacklist / whitelist. CON: While an extension point would be the natural way for opening up the compiler, this is not possible, because ecj cannot depend on equinox. *** This bug has been marked as a duplicate of bug 438244 *** Verified for 4.8 M3 using I20171023-2000 build |
Consider the following code: ---------------------------- import javax.annotation.Nullable; public class UnusedMethodHolder { @Nullable private static Object unusedMethod() { return null; } } Actual behaviour: ----------------- There is NO warning "The method unusedMethod() from the type UnusedMethodHolder is never used locally". Expected behaviour: ------------------- There should be a warning that this method is not used if "Unused private member warning" is enabled in compiler settings.