Proposal to make new-in-Java-SE-8 methods on AnnotatedElement default methods

[Alex Buckley]

On 10/24/2013 8:34 AM, Joel Borggrén-Franck wrote:
> On 24 okt 2013, at 15:16, Joe Darcy <joe.darcy at oracle.com> wrote:
>>
>>
>> +     * @implSpec The default implementation first calls {@link
>> +     * #getAnnotation(Class)} on the argument type. If the annotation
>> +     * is directly or indirectly present, it is returned in an
>> +     * one-element array. Otherwise, if the argument annotation type
>> +     * is repeatable and an annotation of the container type is
>> +     * directly or indirectly present, then the returned result is
>> +     * equal to the result of calling the {@code value} method on the
>> +     * container annotation.
...
>> -    <T extends Annotation> T[] getAnnotationsByType(Class<T> annotationClass);
>> +    default <T extends Annotation> T[] getAnnotationsByType(Class<T> annotationClass) {
>>
>
> This is the wrong order. The precise lookup of an associated annotation is declared then inherited, quoting the javadoc for AnnotatedElement:

Yes, and the root cause is an inconsistency in the @implSpec. The first 
call to getAnnotation(Class) will tell you if an annotation is 
_present_, which is not the same thing as "directly or indirectly 
present". In fact, as a legacy method, getAnnotation(Class) will never 
reveal an indirectly present annotation. The @implSpec should say "If an 
annotation of type T is present, it is included in the returned array. 
[If multiple annotations of type T are present because someone made a 
Runtime*Annotations attribute with @Foo @Foo by hand, then the first is 
chosen.]"

There's a bigger problem: the word "Otherwise". The intent of 
getAnnotationsByType(Class) is to expose the directly present annotation 
(if any) _and_ indirectly present annotations as may be found in a 
container annotation. This is Example 1.2-3 in 8misc.pdf, where an @Foo 
and an @FooContainer are side by side on an element. Returning a 
one-element array can't be right.

>> +     * @implSpec The default implementation first calls {@link
>> +     * #getDeclaredAnnotation(Class)} on the argument type. If the
>> +     * annotation is directly present, it is returned in an
>> +     * one-element array. Otherwise, if the argument annotation type
>> +     * is repeatable and an annotation of the container type is
>> +     * directly present, then the returned result is equal to the
>> +     * result of calling the {@code value} method on the container
>> +     * annotation.
>> +     *
>>       * @param <T> the type of the annotation to query for and return
>>       * if directly or indirectly present
>>       * @param annotationClass the Class object corresponding to the
>> @@ -277,7 +315,9 @@
>>       * @throws NullPointerException if the given annotation class is null
>>       * @since 1.8
>>       */
>> -    <T extends Annotation> T[] getDeclaredAnnotationsByType(Class<T> annotationClass);
>> +    default <T extends Annotation> T[] getDeclaredAnnotationsByType(Class<T> annotationClass) {
>>
>
> This should work.

In a similar fashion to above, the intent of 
getDeclaredAnnotationsByType(Class) is to reveal all directly and 
indirectly present annotations, not just one directly present annotation.

>> +     * @implSpec The default implementation first performs a null check
>> +     * and then loops over the results of {@link
>> +     * getDeclaredAnnotations} returning the first annotation whose
>> +     * annotation type matches the argument type.
>> +     *
>>       * @param <T> the type of the annotation to query for and return if directly present
>>       * @param annotationClass the Class object corresponding to the
>>       *        annotation type
>> @@ -247,7 +265,18 @@
>>       * @throws NullPointerException if the given annotation class is null
>>       * @since 1.8
>>       */
>> -    <T extends Annotation> T getDeclaredAnnotation(Class<T> annotationClass);
>> +    default <T extends Annotation> T getDeclaredAnnotation(Class<T> annotationClass) {
>>
>
> Wouldn't this risk the same issues as when we turned isAnnotationPresent() into a default?

The spec is correct though :-)

Alex