Dependencies should consider annotations

[codecholeric]

At the moment JavaClass.getDirectDependenciesFromSelf() and JavaClass.getDirectDependenciesToSelf() don't consider annotations.
Annotations (and their parameters) should be considered dependencies as well.
This includes

• all annotations on the JavaClass or any JavaMember of that class
• all JavaClass parameters of those annotations
• all JavaClass parameters of annotations within those annotations (and transitively their annotations and so on)

E.g.

@OnClass
@InAnnotation(
    type = AnotherDependency.class,
    nested = @InNestedAnnotation(type = YetAnotherDependency.class)
)
class SomeClass {
    @OnField
    Object field;

    @OnConstructor
    SomeClass() {}

    @OnMethod
    void method() {
    }
}

Then OnClass, InAnnotation, AnotherDependency, InNestedAnnotation, YetAnotherDependency, OnField, OnConstructor and OnMethod should all be returned as direct dependencies of SomeClass.

[slq]

Hi @codecholeric, have you started working on this issue? If not I can take it.

[codecholeric]

Hi @slq, first of all thanks a lot for the offer!! I've blocked this issue for someone who offered to contribute as well, so I assume it is in progress. Maybe you could look into #115 if you feel like it. I haven't marked it as "good start for contributing", but I think it would be the next best thing 😉
Would that be okay for you? I can of course support you, if any questions should arise!

[kosani]

The differences between JavaClass and JavaMemeber made me duplicate parts of the code in order to make proper Dependency descriptions. I think they might be solved by introducing / upgrading some new concepts.

Firstly, it seems to me that there might be a need for a HasDescription implementation on JavaClass.
It has already been implicitly used when creating a Dependency from origin and target:

    String originType = origin.isInterface() ? "Interface" : "Class";
    String originDescription = originType + " " + bracketFormat(origin.getName());

might as well make it explicit, if there's not a better reason not to?

Secondly, the last part of the description has to do with the location that's derived differently:
JavaMemeber -> getOwner()
JavaClass -> this()

Should a JavaClass also implement HasOwner and return itself or is there a need for a new concept?

[codecholeric]

Yeah, HasDescription on JavaClass sounds reasonable, feel free to add that! But HasOwner I'm not convinced, I don't see any meaning in a class owning itself. I wouldn't add this just cause it's convenient at that one place...

[kosani]

I've managed to implement most dependencies from and that way seems to be pretty straight-forward. I've tried to fetch dependencies to class through MemberDependenciesOnClass (I might consider a different approach, since annotation is not a 'member'), but the problem is that the annotations are provided lazily through a supplier and the dependencies to the annotation do not get registered when getDirectDependenciesToSelf is invoked. I've tried using Map<String, JavaAnnotation> instead of Supplier<Map<String, JavaAnnotation>>, which works.. But I believe that the supplier was there for a reason, so I would like to know what's the reason is.
Why are some JavaClass fields supplied (allFields, allMethods, allConstrucotrs), while others are used directly (fields, methods, constructors)?

[codecholeric]

The Supplier in general is used in places where the creation of certain objects

• has a noticeable performance impact
• is only used for specific tests

I don't know if this really holds for the simple building of annotations. You could compare an import of 50K classes though with and without the change of Supplier to direct reference and see, how a test that just tests package accesses compares in performance.

The Supplier is used for example, if a rule checks for dependencies to self, since then the graph must be completely resolved to find back references.

[kosani]

I've tried testing the performance with and without the supplier on a set of 27441 classes and the duration of both tests seem to converge to around 30s.

ArchRule rule = classes().should().resideInAPackage("..");
JavaClasses classes = new ClassFileImporter().importPackages("java", "javax", "org", "com", "net", "junit", "sun", "jdk");
rule.check(classes);

So, if my testing and reasoning is correct, the supplier can be omitted in the case of annotations?

[codecholeric]

Yeah, I've checked it on the full JDK as well, and didn't notice any difference. Maybe originally I did something more complex there, but for now I think it's save to replace the Supplier with a simple map. The simpler the better 😃.
Would be cool, if you could throw it out of JavaMember then as well 😉

[kosani]

While analyzing some failed tests I think I've figure out why annotations were supplied in a JavaClass and not instantiated eagerly in completeMembers method. This was just a mechanism to ensure that all JavaClass.completeMembers were called before any JavaAnnotation was created. For the same reason, some tests from ClassFileImporterTest failed, for example: imports_class_with_annotation_with_empty_array.

The test results in a NullPointerException when annotation.get("primitives").get is invoked because the JavaAnnotation of type ClassAnnotationWithArrays is built before JavaClass.completeMembers of ClassAnnotationWithArrays is invoked. Therefore, primitives[] was never built as a JavaAnnotation parameter.

I'm trying to find a third way of providing annotation dependencies to class that is not coupled with the completeMembers method. Is it possible to do it somewhere in ClassGraphCreator.complete() method? For instance, after completeMembers() is invoked?

Other tests fail mostly due to the new features added. For example, Object() is annotated with jdk.internal.HotSpotIntrinsicCandidate. I'll list these tests after these more critical ones are addressed.

[codecholeric]

Ay, now that you mention it, I do remember 😉 I think that might have been one of those places where a comment would have been a good idea.
I probably don't understand your problem completely though. Seems to me that memberDependenciesOnClass is created after completeMembers anyway, isn't it? Shouldn't you have all the info necessary by that time?

[kosani]

Only the info that is registered into the context. Since JavaAnnotations are instantiated via a Supplier, it's context.createAnnotations(JavaClass.this) doesn't get invoked by the time I start looking into memberDependenciesOnClass. Since registration into the context is done during context.createAnnotations(JavaClass.this), the info about annotation dependencies to class is missing.

It looks like annotations need to be registered after completeMembers has been invoked on all classes, but also before memberDependenciesOnClass has been built on any class.

I've managed to do this in my latest commit, which fixed the issue, but there are still a couple of tests that are bugging me.

[codecholeric]

Ah I see! As far as I remember I wanted to avoid a second run over the classes there (since you could iterate all classes again and complete the annotations after the run to complete all members). So there are two things coming to my mind

a) check if that really costs so much performance (compare 50K classes or sth. like that again 😉) If not you can iterate twice and still throw out the supplier
b) make the memberDependenciesOnClass reference a Supplier as well, so that the annotations are created when they are actually needed within the dependencies (similar to accessesToSelf as far as I remember, I think I avoided resolving the whole graph there eagerly as well)

As soon as I find some time I'll look more closely at your commits! Thanks a lot for your hard work!!

[kosani]

Okay, so it seems there is another issue with my current implementation of registering annotation dependencies to self. This time with annotations on fields and methods. I've tried to register annotations on members during registering fields, methods and constructors in ClassGraphCreator

void registerFields(Set<JavaField> fields) {
    for (JavaField field : fields) {
        fieldTypeDependencies.put(field.getRawType(), field);
        registerMemberWithAnnotations(field);
    }
}

which is also faulty because I am accessing the supplier of annotations (eagerly) before all methods have been registered. Thus, the following tests are broken

ClassFileImporterTest.imports_fields_with_complex_annotations_correctly
ClassFileImporterTest.imports_fields_with_two_annotations_correctly
ClassFileImporterTest.imports_method_with_annotation_with_empty_array

for the same reason imports_class_with_annotation_with_empty_array was broken.

It seems that I also need to access all JavaMembers with annotations, but only after each classes members have been completed.

[nbrugger-tgm]

For me this still doesn't work:

@Override
@RequiresPermission(Permission.VIEW_COLUMN)
public List<FormulaDto> getFormulas() {
	....
}

with this rules:

classes().should(not(new ArchCondition<JavaClass>("access @NoUse stuff") {
	@Override
	public void check(JavaClass item, ConditionEvents events) {
		... item.getAllAccessesFromSelf() ...
	}
}));

and item.getAccessesFromSelf() does not return any access to RequiresPermission or Permission.
RequiresPermission is a RUNTIME scoped annotation.

The other way around also does not work:

noMembers().that().areAnnotatedWith(Permission.NoUse.class).should(new ArchCondition<JavaMember>("be accessed") {
	@Override
	public void check(JavaMember item, ConditionEvents events) {
		... item.getAccessesToSelf() ...
	}
});

when item is Permission.VIEW_COLUMN , item.getAccessesToSelf() does not return any access with getFormulas() (or the class its contained in) as origin.