Failing to use Override annotaion as a Condition for ArchRule

[gaurabdg]

The goal is to ensure all classes of a specific name pattern have non-protected methods, except for those which are annotated with Override.
But this archrule

      final ArchRule checkMethodsShouldNotBeProtectedRule =
            methods().that()
            .areNotAnnotatedWith(Override.class).and()
            .areDeclaredInClassesThat()
            .haveSimpleNameEndingWith("Check").and()
            .doNotHaveModifier(JavaModifier.ABSTRACT)
            .should().notBeProtected();

gives the following violation
com.tngtech.archunit.base.ArchUnitException$InvalidSyntaxUsageException: Annotation type java.lang.Override has @Retention(SOURCE), thus the information is gone after compile. So checking this with ArchUnit is useless.
Is there any way to fix this/some workarund for static analysis?

[codecholeric]

Unfortunately I think no ☹️ As the message says: In the bytecode there is no trace anymore if this method was annotated with @Override or not, thus it cannot be validated.
I don't think ArchUnit is the right tool for this, because this is a codestyle / source code issue.
IntelliJ for example has an inspection though, that can warn you, if you forgot to add @Override (and offers a nice quick fix to fix all violations in the current file).
Other than that there are also tools like Checkstyle if you're looking for something to include in your build process.

[gaurabdg]

Okay, thanks. Actually I am developing for Checkstyle only, was trying to integrate ArchUnit into the codebase.

[codecholeric]

Ah, okay 😄 Yeah, I think Checkstyle and ArchUnit just have a different approach and use case. Some things might overlap, but there also is a good part that's different and pure source assertions are one of those, they are out of scope for ArchUnit...

[codecholeric]

I'll close this for now, since I don't think ArchUnit can do anything there with respect to @Retention(SOURCE). Feel free to reopen if you have further questions about this issue.

[nbrugger-tgm]

Hi,
I have a question very closely related to this one.
While i do not want to check for the @Override annotation i want to ensure that the method overrides a method from a specific interface.

What i tried

methods()
	.that().areDeclaredInClassesThat(areServiceImpl)
	.and().arePublic()
	.should().beDeclaredInClassesThat(serviceInterface)
	.because("No service should add public methods");

but it seems that if a method is overwriten its signature changes from Interface#method() to Impl#method() which makes the beDeclaredInClassesThat(...) fail

[codecholeric]

Yes, because the overridden method is actually declared in the implementation. No matter if there is a second declaration in an interface that has been overridden. I think what you want might be

methods().that().areDeclaredInClassesThat().haveSimpleNameEndingWith("Impl")
                .should(beOverridden())

...


private ArchCondition<JavaMethod> beOverridden() {
    return new ArchCondition<JavaMethod>("be overridden") {
        @Override
        public void check(JavaMethod method, ConditionEvents events) {
            boolean isOverridden = method.getOwner().getAllRawInterfaces().stream()
                    .anyMatch(c -> c.tryGetMethod(method.getName(), getTypeNames(method.getParameters())).isPresent());

            if (!isOverridden) {
                events.add(SimpleConditionEvent.violated(method, method.getDescription() + " is not overridden from an interface"));
            }
        }

        private String[] getTypeNames(List<JavaParameter> parameters) {
            return parameters.stream().map(p -> p.getRawType().getName()).toArray(String[]::new);
        }
    };
}

I.e. we check out all the interfaces the class implements and see if we can find a method that matches the signature. (Note that there might be an edge-case here for covariantly overridden methods, where a bridge method has been added! So this is not sophisticated enough for all cases, but I hope you can build on that?)

[nbrugger-tgm]

Thanks this is actually very helpful.

Maybe depending on demand of such overwriting/overloading/inheritance rules it makes sense to integrate a few common checks into the core DSL.

If this is considerable i would also be down to implement this and contribute

[codecholeric]

Thanks for the offer 😃 Principally yes, I'm just not completely sure how exactly these methods should look like to be useful to a wide user base. In the end it's always a guess, if something is too specific and should just be handled on an individual basis with a custom predicate/condition, or if it should be part of the fluent API 🤔
But maybe methods().that().areOverridden() and methods().should().beOverridden() would be a good additions 🤔 At least it sounds like these are core attributes of "methods" one might talk about 🤷‍♂️ So in the end it might even make sense to integrate this into the domain object JavaMethod.isOverridden() for the logic how to determine this and simply refer to it from the fluent API predicate/condition.