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

Fix invalid annotation literals #443

Merged
merged 1 commit into from
Sep 21, 2023

Conversation

Ladicek
Copy link
Contributor

@Ladicek Ladicek commented Mar 7, 2023

The Annotation.equals() contract directly states that for two annotation objects to be equal, they must (among others) both implement the [same] annotation interface.

The CDI TCK used to create instances of memberless annotations simply by instantiating an anonymous subclass of AnnotationLiteral, which doesn't implement the annotation interface and hence is invalid.

It has never been a problem, because AnnotationLiteral implements equality in a slightly more permissive manner: instead of checking that both objects implement the same annotation interface, it checks that both objects are annotations that have an equal annotationType(). It becomes problematic when an implementation uses a different mechanism to instantiate annotations than extending AnnotationLiteral, one that closely follows the Annotation contract.

This commit replaces all usages of such anonymous subclasses by proper AnnotationLiteral subclasses that implement the annotation interface as well.

@Ladicek
Copy link
Contributor Author

Ladicek commented Mar 7, 2023

Related to jakartaee/cdi#656

I'm not submitting this as a challenge, because we decided to compensate for this on our side, but I believe this should be fixed in the next major/minor CDI TCK version (4.1/5.0 I believe).

The `Annotation.equals()` contract directly states that for two
annotation objects to be equal, they must (among others) both
implement the [same] annotation interface.

The CDI TCK used to create instances of memberless annotations
simply by instantiating an anonymous subclass of `AnnotationLiteral`,
which doesn't implement the annotation interface and hence is invalid.

It has never been a problem, because `AnnotationLiteral` implements
equality in a slightly more permissive manner: instead of checking
that both objects implement the same annotation interface, it checks
that both objects are annotations that have an equal `annotationType()`.
It becomes problematic when an implementation uses a different mechanism
to instantiate annotations than extending `AnnotationLiteral`, one that
closely follows the `Annotation` contract.

This commit replaces all usages of such anonymous subclasses by
proper `AnnotationLiteral` subclasses that implement the annotation
interface as well.
Copy link
Contributor

@manovotn manovotn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into this!

@Ladicek Ladicek added this to the CDI 4.1 milestone May 23, 2023
@manovotn
Copy link
Contributor

Moving into ready for review state as 4.1 changes can go into main branch now.

@manovotn manovotn marked this pull request as ready for review September 21, 2023 08:19
@Ladicek
Copy link
Contributor Author

Ladicek commented Sep 21, 2023

This has been approved before, so I'll just push the button :-)

@Ladicek Ladicek merged commit dcdbe5a into jakartaee:master Sep 21, 2023
2 checks passed
@Ladicek Ladicek deleted the fix-annotation-literals branch September 21, 2023 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants