JDK-8005294 : Consider default methods for additions to AnnotatedElement
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.lang:reflect
  • Affected Version: 8
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2012-12-20
  • Updated: 2017-05-17
  • Resolved: 2013-10-31
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8
8 b115Fixed
Related Reports
Relates :  
JDK-8008279 - Remove InvalidContainerAnnotationError.java
Relates :  
JDK-7154390 - Add support for repeating annotations in j.l.r.AnnotatedElement
Relates :  
JDK-8027361 - Some uses of assert statements in default methods in bootclasspath cause VM crash
Description
The changes to the AnnotatedElement interface made under JDK-7154390 introduced some new methods to the interface.  As such, it was a source incompatible change to any implementations of the interface that were not updated at the same time.

While implementations outside of the JDK are expected to be rare, it would be more source compatible to provide default implementations in the interface declaration, even if the declarations just throw exceptions.
Comments
Copying discussion points that helped in reaching the agreement from email here: - Suggestion to have an annotation that has Class value member as one of the members on TestClass1 or TestClass1Super: Explanation from dev: Not very helpful since the behavior of the new code is not dependent on the return types of the annotation members. - Request to include test cases to have annotations on packages apart from classes and methods as they have different code paths: Explanation from dev: the core distinction among annotations is whether or annotation is inherited or not and that is covered by the Class and non-Class cases.
31-10-2013
Changes look fine and the given test covers the neccessary test cases. Agree to push these changes.
30-10-2013
The @implSpec of getDeclaredAnnotationsByType allows for two styles of implementation, one based on getDeclaredAnnotation(Class) and another on getDeclaredAnnotations(). In the end, the fix as pushed uses a getDeclaredAnnotations()-based fix. For posterity, a getDeclaredAnnotation(Class)-based implementation is: - <T extends Annotation> T[] getDeclaredAnnotationsByType(Class<T> annotationClass); + default <T extends Annotation> T[] getDeclaredAnnotationsByType(Class<T> annotationClass) { + Objects.requireNonNull(annotationClass); + int resultSize = 0; + T directlyPresent = getDeclaredAnnotation(annotationClass); + + if (directlyPresent != null) { + resultSize++; + } + + Repeatable repeatable = annotationClass.getAnnotation(Repeatable.class); + if (repeatable == null) { + @SuppressWarnings("unchecked") + T[] returnValue = (T[]) Array.newInstance(annotationClass, resultSize); + if (directlyPresent != null) + returnValue[0] = directlyPresent; + return returnValue; + } else { + // directlyPresent may or may not be null + + // Look through a container to see if an annotation is + // indirectly present + Class<? extends Annotation> containerType = repeatable.value(); + Annotation container = getDeclaredAnnotation(containerType); + T[] indirectlyPresent = null; + + if (container != null) { + indirectlyPresent = AnnotationSupport.getValueArray(container); + resultSize += indirectlyPresent.length; + } + + // Final result size is known + + // todo: review comment for accuracy... + /* + * If resultSize is 0, indirectlyPresent is either + * assigned to the result of the initial Array.newInstance + * call or indirectlyPresent is assigned to a zero-length + * value array from an empty container annotation. In + * either case, a zero-length array is immutable and does + * not need to reallocated before being returned. + * + * If resultSize is nonzero, then indirectlyPresent points + * to the result calling a method on an annotation and + * annotations are required to implement a no sharing + * policy. Therefore, it is not required to copy the + * elements of indirectlyPresent into a new array of the + * same size. + */ + if (indirectlyPresent == null || indirectlyPresent.length == 0) { + @SuppressWarnings("unchecked") + T[] returnValue = (T[]) Array.newInstance(annotationClass, resultSize); + if (resultSize == 1) + returnValue[0] = directlyPresent; + return returnValue; + } else { + // assert indirectlyPresent != null && indirectlyPresent.length > 0; + + if (resultSize == indirectlyPresent.length){ + // assert directlyPresent == null; + return indirectlyPresent; + } else { + // assert directlyPresent != null && indirectlyPresent.length > 0; + @SuppressWarnings("unchecked") + T[] returnValue = (T[]) Array.newInstance(annotationClass, resultSize); + + // Determine whether the directly present annotation + // comes before or after the indirectly present ones. + + int indirectOffset = 0; + + for (Annotation a : getDeclaredAnnotations()) { + Class<? extends Annotation> annotationType = a.annotationType(); + if (annotationType.equals(annotationClass)) { + indirectOffset = 1; + break; + } else if (annotationType.equals(containerType)) { + break; + } + } + + for (int i = 0; i < indirectlyPresent.length; i++) { + returnValue[i + indirectOffset] = indirectlyPresent[i]; + } + + returnValue[(indirectOffset == 1) ? 0 : resultSize - 1] = directlyPresent; + return returnValue; + } + } + } + } +
30-10-2013
Spec changes out for review: http://mail.openjdk.java.net/pipermail/enhanced-metadata-spec-discuss/2013-October/000279.html Review of implementation changes: http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-October/022584.html http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-October/022652.html
28-10-2013
Additional suggestion to improve AnnotatedElement from the enhanced meta data discuss list: http://mail.openjdk.java.net/pipermail/enhanced-metadata-spec-discuss/2013-January/000140.html "If an annotation of type T is present on an element, and T is made repeatable, then adding more annotations of type T to the element is source compatible and binary compatible. It is not behaviorally compatible for the get[Declared]Annotation(Class<T>) methods and get[Declared]Annotations() methods, because they will now see only a container annotation on the element rather than any annotation of type T. It is not behaviorally compatible for the get[Declared]AnnotationsByType(Class<T>) methods, because their results will expose the additional annotations of type T whereas previously they exposed only a single annotation of type T. If an annotation of type TC is present on an element, then making some other annotation type T repeatable (with TC as its containing annotation type) is source compatible and binary compatible. It is behaviorally compatible for the get[Declared]Annotation(Class<T>) methods and get[Declared]Annotations() methods, in that their results will not change just because TC is a containing annotation type. However, it is not behaviorally compatible for the get[Declared]AnnotationsByType(Class<T>) methods, because they will now recognize an annotation of type TC as a container annotation and "look through" it to expose annotations of type T. "
27-02-2013
From code review comments in JDK-8008279 involving AnnotatedElement: "1. AnnotatedElement's javadoc mentions "the containing annotation type of T", but since @Repeatable is only a one-way link from T to TContainer, it is possible for TContainer1, TContainer2, etc to exist that meet the requirements to be T's containing annotation type. Therefore, the javadoc should say: "Attempting to read annotations of a repeatable annotation type T that are contained in an annotation whose type is not, in fact, _a_ containing annotation type of T, will result in an {@link AnnotationFormatError}." "
27-02-2013