ArchUnit icon indicating copy to clipboard operation
ArchUnit copied to clipboard

Consider inheritence hierarchy while resolving the annotations of a class or a member

Open OLibutzki opened this issue 3 years ago • 2 comments

I would like to ensure that methods of certain classes are annotated with a certain annotation. The annotation has not to be placed at the implementation class' metho ditself, but can be put into a base class or the declaring interface. Unfortunately, ArchUnit does not detect annotations of base classes/implented interfaces or overriden methods.

Example:

interface MyInterface {
  @MyAnnotation
  void myMethod() {
  }
}

@MyClassMarker
class MyClass implements MyInterface{
  @Override
  void myMethod() {
  }
}

The following checks fails as myMethod itself.

methods( )
  .that( ).areDeclaredInClassesThat( ).areAnnotatedWith( MyClassMarker.class) 
  .should( ).beAnnotatedWith( MyAnnotation.class);

OLibutzki avatar Mar 24 '21 06:03 OLibutzki

The current behavior seems fair to me, given that MyClass.myMethod is not directly annotated (which could be relevant in some use cases), but I understand your use case and agree that this could be a nice extension.

Do you have a suggestion how the new API should look like? I could imagine something like this:

methods()
    .that().areDeclaredInClassesThat().areAnnotatedWith(MyClassMarker.class)
    .should().beAnnotatedDirectlyOrViaSupertypeWith(MyAnnotation.class);

(The name is certainly not great, but I thought starting with .beAnnotated might be useful to discover it via autocompletion...)

In any case, note that you can already implement a custom ArchCondition to achieve your goal:

ArchRule rule = methods()
        .that().areDeclaredInClassesThat().areAnnotatedWith(MyClassMarker.class)
        .should(beAnnotatedDirectlyOrViaSupertypeWith(MyAnnotation.class));

static ArchCondition<JavaMethod> beAnnotatedDirectlyOrViaSupertypeWith(Class<? extends Annotation> annotation) {
    return new ArchCondition<JavaMethod>("be annotated (directly or via a supertype) with @" + annotation.getSimpleName()) {
        @Override
        public void check(JavaMethod method, ConditionEvents events) {
            boolean satisfied = method.isAnnotatedWith(annotation) || isAnnotatedViaSupertype(method);
            String message = method.getFullName() + " is " + (satisfied ? "" : "not ") + " annotated with @" + annotation.getSimpleName();
            events.add(new SimpleConditionEvent(method, satisfied, message));
        }

        boolean isAnnotatedViaSupertype(JavaMethod method) {
            String[] parameters = method.getRawParameterTypes().stream().map(JavaClass::getName).toArray(String[]::new);
            return Stream.concat(method.getOwner().getAllRawSuperclasses().stream(), method.getOwner().getAllInterfaces().stream())  // #551 requires changing getAllInterfaces to getAllRawInterfaces
                    .map(supertype -> supertype.tryGetMethod(method.getName(), parameters))
                    .anyMatch(overriddenMethod -> overriddenMethod.isPresent() && overriddenMethod.get().isAnnotatedWith(annotation));
        }
    };
}

hankem avatar Mar 24 '21 08:03 hankem

Hi @hankem,

thanks a lot for answering that quickly!

I agree that the current behaviour is reasonable and this issue is not about changing the API but about extending the API.

Your suggestion sounds a bit bulky, but it says what it does. Honestly, I have no better idea at the moment.

I highly appreciate your support by implementing the ArchCondition. Your support is extraordinary and one reason (beside the API itself) why I like ArchUnit so much.

OLibutzki avatar Mar 24 '21 12:03 OLibutzki