Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why are CodeUnitsShould have..Type() and notHave..Type() methods not contravariant? #262

Closed
daniel-shuy opened this issue Nov 12, 2019 · 2 comments · Fixed by #283
Closed

Comments

@daniel-shuy
Copy link
Contributor

For example, FieldsShould#haveRawType(DescribedPredicate) is contravariant (takes in a DescribedPredicate<? super JavaClass>), whereas CodeUnitsShould#haveRawReturnType(DescribedPredicate) is invariant (takes in a DescribedPredicate<JavaClass>).

This makes it possible to do:

ArchRuleDefinition.fields()
    .should().haveRawType(CanBeAnnotated.Predicates.annotatedWith(Annotation.class))

(because CanBeAnnotated.Predicates#annotatedWith(Class) returns a DescribedPredicate<CanBeAnnotated>, and JavaClass implements CanBeAnnotated)

but not possible to do:

ArchRuleDefinition.methods()
    .should().haveRawReturnType(CanBeAnnotated.Predicates.annotatedWith(Annotation.class))

Is this intended?

@codecholeric
Copy link
Collaborator

No, sorry, this is not intended ☹️ I think the second one should have a contravariant predicate type as well. Must have slipped through. Maybe I should write some sort of ArchRule for that 😉
There is some "convenience" method annotatedWith(Foo.class).<JavaClass>forSubtype() to mitigate such pain points (even though in this place it should just work out of the box).

@codecholeric
Copy link
Collaborator

I've adjusted the API, there were a couple of places where this slipped through.
Also added an ArchTest to make sure future methods will always be written correctly with respect to DescribedPredicate 😉
Thanks for reporting this issue and helping to make ArchUnit better 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants