Make animal sniffer annotations optional by elharo · Pull Request #3649 · google/guava (original) (raw)

I think the case for this is a little better than the case for AutoValue's annotations, but I still have my doubts.

I think the overall consensus that annotation-only packages should be optional or provided is wrong, as discussed in the AutoValue thread. There are two main cases:

• Omitting an annotation-only dependency can cause failures with -Xlint:all -Werror. The good news is that I haven't been able to produce this failure unless the annotation has elements. And the single Animal Sniffer annotation has no elements -- yet, anyway :) That doesn't mean that success is guaranteed, though, only that it happens to work now. I don't think that javac makes guarantees about incomplete classpaths in general, though I could be wrong.
• Omitting an annotation-only dependency can cause problems with plugins, annotation processors, etc. For example, I could imagine static analysis that checks, if any annotation is declared on both a class and its superclass, whether that annotation is @Inherited (so that it can suggest removing it from the subclass). This static analysis would need to load all annotations. Now, it can just skip checking an annotation if the annotation class isn't available, but I'm not sure we can say that we're doing the right thing if we force that. And maybe the static analysis would be more important than my example, involving security or something. For example, there are systems that let you define that code annotated with one annotation "implies" one or more other annotations ("meta-annotations"). So if security-critical static analysis sees @MyFoo on a method, then it might need to look at @MyFoo to make sure that it doesn't imply something like @CompileTimeConstant. (Possible example, albeit non-security-critical: @IncompatibleModifiers.)

So I think that including annotations is still the right thing to do by default. The typical guidance I've seen to make them provided or optional doesn't even consider these issues, so I don't think we should put much stock in it.

I assume that the goal here is to avoid version conflicts. Is this a case in which the Linkage Checker can detect that it's harmless to have different but identical versions of the class file present?

edit: an additional concern:

• If users look up annotations with reflection at runtime, missing annotations can lead to exceptions in some cases, even though the JDK has gone to some effort to avoid that in some cases.