Looking up annotations (also) reports annotations of annotations

[haumacher]

Situation as follows: I have an annotation Foo that can annotate all kind of types (including annotations) and it annotates itself with the string "bar":

@Foo("bar")
@Retention(RUNTIME)
@Target(ElementType.TYPE)
public @interface Foo {

	String value();
	
}

There is an interface hierarchy with interfaces A and B, where B is annotated with Foo:

public interface A {

}

@Foo("baz")
public interface B extends A {

}

Now, I'm looking for all specializations of A that are annotated with Foo:

	public static void main(String[] args) {
		ClassGraph classGraph = new ClassGraph();
		classGraph.verbose();
		ScanResult scanResult = classGraph.enableClassInfo().enableAnnotationInfo().scan();
		
		ClassInfoList classes = scanResult.getClassesImplementing(A.class);
		
		classes.forEach(info -> System.out.println(info.getName() + ": " + info.getAnnotationInfo()));
	}

This code reports

test.classgraph.annotation.B: [@test.classgraph.annotation.Foo("bar"), @test.classgraph.annotation.Foo("baz")]

Which tells me that B is annotated twice with Foo - one time with the value bar and one time with the value baz. Even worse, if I'm asking info.getAnnotationInfo(Foo.class) I get @test.classgraph.annotation.Foo("bar"), which is the meta annotation of Foo instead the requested annotation of B.

Are my expectations wrong?

[haumacher]

test-classgraph.zip

A stand-alone Eclipse test project with the code shown above.

[haumacher]

The complete output of the main method: test-classgraph.log

[lukehutch]

ClassGraph currently returns meta-annotations and inherited annotations when you call ClassInfo.getAnnotationInfo(), correct. I agree this probably doesn't match users' expectations, and I don't remember why I set it up this way!

I just added the method ClassInfo.getAnnotationInfoDirectOnly(). Can you please test the latest version in git, and let me know if this matches your expectations?

PS thanks for the test project! But you were right in your interpretation of the current behavior, and it's a quick fix to add a method that returns just the direct annotations.

[haumacher]

Hi @lukehutch,

I tried your update. Yes, the getAnnotationInfoDirectOnly() method now only reports the "expected" annotations. But looking up some concrete annotation info.getAnnotationInfo(Foo.class) still reports the wrong one.

I cannot image a situation, where one wants to lookup annotations of a class including all meta-annotations of all annotations of this class. If one wants that functionality, he could easily implement it himself by walking the dependency tree. I think, the default implementation should only report annotations of the class.

However, for some annotations it might be desirable to look up the first one defined on the class or one of its super classes (if the requested annotation is marked java.lang.annotation.Inherited). But "inheriting" annotations from other annotations of a class does not look reasonable to me.

[lukehutch]

@haumacher Thank you for this perspective, and sorry for the delayed response. I think you're right that a user's default assumption would be that only direct annotations should be returned by ClassInfo.getAnnotationInfo().

I remember why I made it work this way originally: because methods like ClassInfo.getSubclasses() and ClassInfo.getSuperclasses() returned the full transitive closure of classes. It seemed reasonable at the time to make ClassInfo.getAnnotationInfo have the same behavior.

It has been so long since I wrote this code that I forgot that not only did I add a method directOnly() to ClassInfoList to get only directly related classes (e.g. ClassInfo.getSubClasses().directOnly()), but I also added this method to AnnotationInfoList. So I didn't actually need to add ClassInfo.getAnnotationInfoDirectOnly(), because you can already call ClassInfo.getAnnotationInfo().directOnly(). I'll remove ClassInfo.getAnnotationInfoDirectOnly(), since it duplicates this capability. For your purposes, ClassInfo.getAnnotationInfo().directOnly() should work.

However, this brings up a bigger discussion: there is a chance to break the API for a version 5.0.0 release of ClassGraph, and I am willing to change the behavior of the API at that point. I have two questions, if you're able to weigh in on this:

• If ClassInfo.getAnnotationInfo() is changed to return only direct annotations, should ClassInfo.getSubclasses() also only return direct subclasses, and should ClassInfo.getSuperclasses() return only the single direct superclass? (Particularly for getSuperclasses(), the name implies all superclasses, not just the one...)
• If only direct relationships are returned, then what is the best API for getting the entire transitive closure of related classes?
  1. ClassInfo.getAnnotationInfo(boolean directOnly), adding a required boolean parameter (I don't like this as much, because Java doesn't have named parameters, so it's not obvious to someone who doesn't know the API what a boolean parameter does).
  2. Have two separate methods: ClassInfo.getAnnotationInfoDirect() and ClassInfo.getAnnotationInfoAllReachable() or similar
  3. Add a method AnnotationInfoList.getTransitiveClosure() to fetch the entire transitive closure (the inverse of the current AnnotationInfoList.directOnly() method), i.e. ClassInfo.getAnnotationInfo().getTransitiveClosure()

I'm tending towards the third option for an API change; however, it would not be backwards compatible to change ClassInfo.getSubclasses() and ClassInfo.getSuperclasses() to return only a single step's worth of related classes -- so I would probably need to change the name of those methods somehow, in order to intentionally break existing code, and force users of the library to update their code to manually fetch the transitive closure, if that's what they intended. It would be a bit of a drag for all users of getSubclasses() (a very common usage of ClassGraph) to have to call an additional method to fetch the transitive closure.

Please let me know your thoughts. Thanks!

[haumacher]

I forgot that not only did I add a method directOnly() to ClassInfoList to get only directly related classes

This is something that I would never have expected that a list result of a getter provides specialized "filtering" utilities. In my use-case, I evaluated ClassGraph to replace a Java annotation processor that retrieves information at compile-time with a run-time solution that is aware of all classes that could potentially be loaded. Therefore I was biased for what to expect from a Java model API by javax.lang.model.AnnotatedConstruct and related interfaces.

However, this brings up a bigger discussion

I personally prefer an API that makes easy things simple and complex ones possible - not the other way around. I'm happy, if the names on an API reflect this principle - short names for simple functions and composed names for complex functions. Here, the direct (simple) relations of a class/interface is the direct super class/interfaces, its directly implemented/extended interfaces and the reverse of this relations - direct sub classes/interfaces. I would expect that those relation are cached and fast to retrieve using a simple name getXyz(). Complex relations as "all-transitive-generalizations" or "all-transitive-specializations" of a type should have names that remind me that retrieving this relation might involve computational overhead that I do not want to invest, if it is not really necessary, e.g. computeTransitiveXyz() or getTransitiveXyz().

In this concrete example I would prefer an API like this:

• ClassInfo.getAnnotationInfo() - direct
• ClassInfo.getAnnotationInfo(Class/String) - direct
• ClassInfo.getAnnotationInfoRepeatable(Class/String) - direct
• ClassInfo.getSuperclass() - direct
• ClassInfo.getInterfaces() - direct
• ClassInfo.getSubclasses() - directly extending or implementing
  The complex queries can feel like queries, not getters.
• ClassInfo.getFirstAnnotationInfo(Class/String) - first annotation found in the class or one of its generalizations
• ClassInfo.getFirstAnnotationInfoRepeatable(Class/String)
• ClassInfo.getTransitiveSuperclasses(Filter<ClassInfo>) - walks the forest, gives the chance to stop the traversal
• ClassInfo.getTransitiveInterfaces(Filter<ClassInfo>)
• ClassInfo.getTransitiveSubclasses(Filter<ClassInfo>)

But in the ClassGraph API, it seems to be common that getters with list result are not declared List<X> but ListOfX with enhanced functionality in the result type. Accepting this, I would also prefer AnnotationInfoList.getTransitiveClosure() to retrieve the complex result from the simple one. However I would not expect to find an annotation of an annotation in AnnotationInfoList.getTransitiveClosure() - only annotations of generalizations of the type in question.

The drawback of the ListOfX approach is that this does not work for the one result APIs. From ClassInfo.getAnnotationInfo(Class/String) I only expect direct anntations. But I possibly cannot write ClassInfo.getAnnotationInfo(Class/String).inAnySuperClass() or ClassInfo.getAnnotationInfo(Class/String).ofAnyRelatedType(). For that, a fluent API would be ClassInfo.annotations().includingSuperClassAnnotations().includingMetaAnnotations().findSingle() or ...findRepeatable(). But for most use cases, this is inefficient overkill.

For the compatibility issue - yes this is a good idea not to change the semantics of an existing method. In such a case, I prefer a change that helps the customer to easily find the correct fix. If you change ClassInfo.getSubclasses() to ClassInfo.getDirectSubclasses() and ClassInfo.getTransitiveSubclasses(), then one can press Ctrl-Space in Eclipes at the line of the error and get the two possible options. If you change ClassInfo.getSubclasses() to ClassInfo.getDirectSubclasses() where the result has the additional API .getTransitiveClosure() or something like that, the customer will not be able to guess what he has to do without reading migration notes - and nobody likes to do so.

In short - no there is no optimal solution - choose carefully :-)

[lukehutch]

@haumacher all great thoughts -- thanks for taking the time to think about this, and to provide a detailed writeup!

I think I will move to .getDirect...() vs. .getAll...() for superclasses/subclasses and interfaces, and .getDirectAnnotations() vs. .getMetaAnnotations() for annotations. (There is a further issue with repeated annotations, as you point out, but also @Inherited annotations, and I don't want the API to multiply into a cross product of all possible things a user might want to query for, but I'll try to find a clean way to contain the complexity of the options!)

At least for now you know how to get just the direct annotations, however I'll keep this bug open, since I'm starting to assemble a list of things to do for ClassGraph 5.0, which will break the API for the first time in over 4 years. It might be a few months before I get this done, however.