RFR: 8004912: Repeating annotations - getAnnotationsByType is not working as expected

Tue Oct 22 12:57:48 UTC 2013

Hi,

The spec says:

When the new get[Declared]AnnotationsByType(Class<T>) methods are
called for a repeatable annotation type T, the question is how to extend 
the policy
to handle multiple annotations of type T on the superclass and/or 
subclass. Oracle
proposes the following policy for Java SE 8:

. If a class declaration does not have either a "directly present" 
annotation of
type T or an "indirectly present by containment" annotation of type T, 
the class
declaration may be "associated" with an annotation of type T due to 
inheritance.

. If a class declaration does have either a "directly present" 
annotation of type T
or an "indirectly present by containment" annotation of type T, the 
annotation
is deemed to "override" every annotation of type T which is "associated" 
with
the superclass.

This policy for Java SE 8 is reified in the following definitions:

. An annotation A is directly present on an element E if E has a
RuntimeVisibleAnnotations or RuntimeVisibleParameterAnnotations or
RuntimeVisibleTypeAnnotations attribute, and the attribute contains 
A.1.2 Core Reflection API REPEATING ANNOTATIONS
12

. An annotation A is indirectly present on an element E if E has a
RuntimeVisibleAnnotations or RuntimeVisibleParameterAnnotations or
RuntimeVisibleTypeAnnotations attribute, and A's type is repeatable, and 
the
attribute contains exactly one annotation whose value element contains A 
and
whose type is the containing annotation type of A's type (§9.6).

. An annotation A is present on an element E if either:
  - A is directly present on E; or
  - No annotation of A's type is directly present on E, and E is a 
class, and A's type
is inheritable (§9.6.3.3), and A is present on the superclass of E.

. An annotation A is associated with an element E if either:
  - A is directly or indirectly present on E; or
  - No annotation of A's type is directly or indirectly present on E, 
and E is a class,
and A's type is inheritable (§9.6.3.3), and A is associated with the 
superclass
of E.

So I guess E.class.getAnnotations() returns the annotations "present" on 
the E
and E.class.getAnnotationsByType(A.class) returns the annotations of 
type A "associated" with E

Am I right?

Let's just focus on "associated" part now - the getAnnotationsByType 
method. The following test:

http://cr.openjdk.java.net/~plevart/jdk8-tl/RepearingAnnotations/InheritedRepeatingAnnotationsTest.java 
<http://cr.openjdk.java.net/%7Eplevart/jdk8-tl/RepearingAnnotations/InheritedRepeatingAnnotationsTest.java>

dumps @Repeatable @Inherited annotations "associated" with the following 
classes:

@Ann(10)
class A1 {}

@Ann(20)
class A2 extends A1 {}

class A3 extends A2 {}

@Ann(10) @Ann(11)
class B1 {}

@Ann(20)
class B2 extends B1 {}

class B3 extends B2 {}

@Ann(10)
class C1 {}

@Ann(20) @Ann(21)
class C2 extends C1 {}

class C3 extends C2 {}

@Ann(10) @Ann(11)
class D1 {}

@Ann(20) @Ann(21)
class D2 extends D1 {}

class D3 extends D2 {}

Here's the output:

class A2: [@Ann(value=20)]
class B2: [@Ann(value=20)]
class C2: [@Ann(value=20), @Ann(value=21)]
class D2: [@Ann(value=20), @Ann(value=21)]

class A3: [@Ann(value=20)]
class B3: [@Ann(value=10), @Ann(value=11), @Ann(value=20)]
class C3: [@Ann(value=10), @Ann(value=20), @Ann(value=21)]
class D3: [@Ann(value=20), @Ann(value=21)]

Class B3 is showing @Ann(10) and @Ann(11), while class C3 is showing 
@Ann(10) as being "associated" with them, but they should not, right?

Class B3 should show the same annotations as B2 and class C3 should show 
the same annotations as class C2, I think.

Regards, Peter

  On 10/22/2013 01:03 PM, Joel Borggrén-Franck wrote:
> Hi Peter,
>
> Spec is here: http://cr.openjdk.java.net/~abuckley/8misc.pdf
>
> FYI I pushed this before I saw your mail but I do think the code is correct. An extra pair of eyes would be great though!
>
> FWIW I suspect we can be better with regards to allocation especially if we optimize for the common case where there is either a single annotation or a single container, but that will have to be a follow up.
>
> Btw there are a lot of extra test cases in the langtools repository (!?!) look in langtools/test/tools/javac/annotations/repeatingAnnotations/combo/ReflectionTest.java
>
> cheers
> /Joel
>
> On 22 okt 2013, at 18:50, Peter Levart <peter.levart at gmail.com> wrote:
>
>> Hi, I would just like to ask for a pointer to some specification document that describes inheritance of repeating annotations. I couldn't find one on the net.
>>
>> I have a feeling there's something wrong with the logic of AnnotationsSuport.getAssociatedAnnotations. Why? Because it is based on two maps: declaredAnnotations map and allAnnotations map. The later is a map of inherited and/or declared annotations which is aggregated without the knowledge of repeating annotations (the containers). I doubt this map keeps enough information to reconstruct a sound set of inherited and/or declared repeating annotations in all situations.
>>
>> But I'd like to 1st see the specification before showing you some examples where problems arise.
>>
>> Regards, Peter
>>
>> On 10/22/2013 12:21 PM, Joel Borggrén-Franck wrote:
>>> Hi Andreas,
>>>
>>> A few nits:
>>>
>>> Class.java:
>>>
>>>   import java.util.Collection;
>>> +import java.util.Collections;
>>>   import java.util.HashSet;
>>>
>>> unused import.
>>>
>>> AnnotationSupport.java:
>>>
>>> +    /**
>>> +     * Equivalent to calling {@code getDirectlyAndIndirectlyPresentAnnotations(
>>> +     * annotations, annoClass, false)}.
>>> +     */
>>>
>>> I think it is equivalent to annotations, annoClass, true
>>>
>>> Otherwise looks good. I can sponsor this fix.
>>>
>>> cheers
>>> /Joel
>>>
>>> On 21 okt 2013, at 21:01, Andreas Lundblad <andreas.lundblad at oracle.com> wrote:
>>>
>>>> Hi,
>>>>
>>>> New revision up for review:
>>>>
>>>> http://aoeu.se/webrevs/8019420-and-8004912/webrev.01
>>>>
>>>> The following has been addressed since webrev.00:
>>>>
>>>> - Order of directly / indirectly present annotations now respects the order of the keys in the given map of annotations.
>>>>
>>>> - A new test has been added to test the above behavior.
>>>>
>>>> best regards,
>>>> Andreas
>>>>
>>>>
>>>> ----- Original Message -----
>>>> From: andreas.lundblad at oracle.com
>>>> To: core-libs-dev at openjdk.java.net
>>>> Sent: Wednesday, October 16, 2013 4:00:08 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna
>>>> Subject: RFR: 8004912: Repeating annotations - getAnnotationsByType is not working as expected
>>>>
>>>> Hi,
>>>>
>>>> Please review the fix for JDK-8004912 and JDK-8019420 below.
>>>>
>>>> Description:
>>>>
>>>> The behavior of Class.get[Declared]AnnotationsByType was wrong. These methods delegate to sun.reflect.annotation.AnnotationSupport which has been rewritten.
>>>>
>>>> NonInheritableContainee.java is added and contains the test referred to in JDK-8019420.
>>>>
>>>> RepeatedUnitTest.java have been updated to include the test cases in JDK-8004912.
>>>>
>>>> There are more tests available in tl/langtools/test/tools/javac/annotations/repeatingAnnotations/combo/ReflectionTest.java (NB. this file is in the langtools repo)
>>>>
>>>>
>>>> Link to web review:
>>>> http://cr.openjdk.java.net/~alundblad/8019420-and-8004912/
>>>>
>>>> Link to bug reports:
>>>> https://bugs.openjdk.java.net/browse/JDK-8004912
>>>> https://bugs.openjdk.java.net/browse/JDK-8019420
>>>>
>>>>
>>>> -- Andreas Lundblad