Add haveReturnTypeThat() to MethodsThat?

[jwcarman]

I am trying to write a rule that says "no rest endpoint method should return a type that depends upon a type from the domain package." It feels like we might need something similar to areDeclaredInClassesThat but applicable to the (raw?) return type of the method.

[hankem]

MembersThat.areDeclaredInClassesThat is used to select for which members the rule should apply.

Using GivenMembers.that(DescribedPredicate predicate), you could build a custom rule that applies to methods based on their raw¹ return type, e.g. methods().that(returnTypesDependingOnDomainObjects())… with

    static DescribedPredicate<JavaMethod> returnTypesDependingOnDomainObjects() {
        return new DescribedPredicate<JavaMethod>("return types depending on domain objects") {
            @Override
            public boolean apply(JavaMethod method) {
                JavaClass returnType = method.getRawReturnType();  // TODO #115 consider generic type parameters
                Set<Dependency> returnTypesDirectDependencies = returnType.getDirectDependenciesFromSelf();
                // ...
            }
        };
    }

However, your problem description sounds as if you actually wanted to refer to the raw¹ return type in the assertion part of your rule, and there is already CodeUnitsShould.havaRawReturnType(type) (or not), where type can be specified as java.lang.Class<?> type, java.lang.String typeName, or DescribedPredicate<? super JavaClass> predicate. In your case, you'd probably have to use the predicate version in a similar way to the example above, just that you can directly start with the JavaMethod's rawReturnType.

¹ The current version ArchUnit 0.14.1 does not yet support generic types.

---

FWIW: I'm using the following rules in my own projects to test rest endpoint methods:

    @ArchTest
    static final ArchRule rest_resource_methods_should_explicitly_be_Throttled_or_Unthrottled =
            restResourceMethods()
                    .should().beAnnotatedWith(Throttled.class)  // Throttled is a custom annotation in my project.
                    .orShould().beAnnotatedWith(Unthrottled.class)  // And so is this Unthrottled.
                    .because("throttling should not be missed accidentially");

    @ArchTest
    static final ArchRule rest_resource_methods_should_have_Operation_annotation =
            restResourceMethods()
                    .should().beAnnotatedWith(io.swagger.v3.oas.annotations.Operation.class)
                    .because("the API should automatically be documented via OpenAPI");

    static GivenMethodsConjunction restResourceMethods() {
        return methods().that().areAnnotatedWith(javax.ws.rs.Path.class);
    }

Maybe this helps to select your rest endpoint methods?

[jwcarman]

I guess the simplest way to describe what I'm looking to use a methods (the rest endpoints) expression as a starting point to define the classes for a classes (the raw return types) expression:

noMethods().that(areRestEndpoints())
  .should()
  .haveRawReturnTypeThat().dependOnClassesThat().resideInAPackage("..domain.entity..");

[codecholeric]

Yes, your rule looks reasonable. Note that there are predefined methods for methods().that().areDeclaredInClassesThat().areAnnotatedWith(..) and methods().that().areAnnotatedWith(..) which maybe could replace your areRestEndpoints() predicate. But other than that I wouldn't know how to define your rule more concisely either :wink: Do you have any further question or does this solve your issue?

[jwcarman]

I don't think the return type selector pattern exists, though. That's what I'm looking to add.

[codecholeric]

I don't completely get what you mean by "the return type selector pattern"? That sounds to me like methods().that().haveRawReturnType(..)? But there you might have to supply a predicate if it exceeds an exact type match, i.e. methods().that().haveRawReturnType(predicate).

[codecholeric]

Maybe if you paste areRestEndpoints() here, I understand better :wink:

[jwcarman]

I'm selecting the methods of interest (are meta annotated with RequestMapping), but I want to apply ArchConditions to their return types. So, it might be nice to have a DSL element that bridges that gap, hence the haveRawReturnTypeThat() method. I'm somewhat new to ArchUnit, so maybe I'm just thinking about this all wrong. If so I apologize.

[jwcarman]

The areRestEndpoints() method was just "syntactic sugar" for:

methods()
  .that()
  .areMetaAnnotatedWith(RequestMapping.class)

In hindsight, I probably should have just used that so as to not distract from the subject at hand. Sorry for the noise.

[codecholeric]

Ah, okay, now I understand :smiley: I don't know, if you want to make it more explicit it might sense to extract it and give it a custom description :+1: So is your problem solved then or do you have any further questions? But yes, it would be possible to extend the API to offer haveRawReturnTypesThat().predicateOnJavaClass(). If anyone wants to supply a PR, I'm open, otherwise my pipeline for the near future is unfortunately filled up :wink:

[jwcarman]

I'll see what I can do on a PR. I've looked through the codebase and am starting to find my way around. If you had some pointers on where to start digging and noodling on ideas, that'd be great.

I don't have any further questions. Thank you for your time. I was more trying to understand if this is something that would be inline with the vision of the library. Seems like it might be, so hopefully we can get it working!

[codecholeric]

Hmm, I can try, but it's not completely trivial. So don't get frustrated :wink: (a lot of Generics juggling :wink:) There is a public interface layer defining the API and the possible transitions. The starting point for the should-assertions would e.g. be

CodeUnitsShould.haveRawReturnType(..)

So you would have to add a

ClassesThat<CONJUNCTION> CodeUnitsShould.haveRawReturnTypesThat()

About the implementation you can look at

OnlyBeAccessedSpecificationInternal.byClassesThat()

The advantage is that be returning the predefined ClassesThat / ClassesThatInternal you get all the methods out of the box, i.e. as soon as you add it, the API will be able to join all the predicates available to classes, e.g.

methods().should().haveRawReturnTypesThat().areAssignableTo(Serializable.class)

[hankem]

FYI, my initially suggested workaround that does not need the new CodeUnitsShould.haveRawReturnTypesThat() could look like this:

noMethods()
    .that(areRestEndpoints())
    .should().haveRawReturnType(dependingOnClassesInAPackage("..domain.entity.."));

with

static DescribedPredicate<JavaClass> dependingOnClassesInAPackage(String packageIdentifier) {
    PackageMatcher packageMatcher = PackageMatcher.of(packageIdentifier);
    return DescribedPredicate.describe("depending on classes in a package " + packageIdentifier, javaClass -> {
        if (packageMatcher.matches(javaClass.getPackageName())) {
            return true;
        }
        for (Dependency dependency : javaClass.getDirectDependenciesFromSelf()) {  // TODO consider transitive dependencies
            if (packageMatcher.matches(dependency.getTargetClass().getPackageName())) {
                return true;
            }
        }
        return false;
    });
}

(I've included a check to prevent that the return type itself is already in the specifed package. Considering transitive dependencies is left as an exercise... 😉)

[JKLedzion]

Hi, could you please let me know if this issue is still open and adding haveRawReturnTypesThat() is still needed?

[codecholeric]

Since the last update here was end of 2020, I would say yes (any objections @jwcarman? I'm just guessing by now that you probably didn't find the time :wink:)

[jwcarman]

Indeed I didn't find the time, unfortunately. I think it could be useful, but if there are other ways already supported by the DSL and they're adequately expressive, I'd say it's fine to close this.

[Mohinish007]

Do we have any solution to this problem as I am stuck with the same issue as @jwcarman