RFR 7183985: Class.getAnnotation() throws an ArrayStoreException when the annotation class not present

[Liam Miller-Cushon]

Hi Joe,

The updated webrev is at:
http://cr.openjdk.java.net/~cushon/7183985/webrev.03/

I added some more test coverage for arrays that contain both missing and
non-missing classes, and for arrays that contain multiple different missing
classes. In the latter case the exception proxy for the first missing class
is returned. I also consolidate the 'typeError' flag and exceptionProxy,
and just check if the latter is set.

On Tue, Feb 27, 2018 at 5:18 PM, Joseph D. Darcy <joe.darcy at oracle.com>
wrote:

> Sorry, I misremembered the situation in javac. For annotation processing
> in javac, AnnotatedElement is not a factor, but the class
>
>     src/jdk.compiler/share/classes/com/sun/tools/javac/
> model/AnnotationProxyMaker.java
>
> in javac does create annotation proxies, which is the same general
> technique used to create the annotation objects at runtime for core
> reflection.  These annotation objects in javac for javax.lang.model and
> annotation processing, do not follow the full contract of java.lang.reflect.AnnotatedElement.
> In particular, the annotations returned are not necessarily serializable.
> The intersection of methods on AnnotatedConstruct and AnnotatedElement is
> only a proper subset of the methods on AnnotatedElement; however,
> getAnnotation​(Class<T> annotationClass) is in the intersection.
>

Understood, thanks. I did some manual testing. There are currently some
issues related to JDK-8187950, but with Jan's pending fix for that bug
everything looks good to me. Here's a demo:
https://gist.github.com/cushon/804021a520cec1832594c6e1138cebdd. The API
correctly models an array of Class values where some classes are missing by
using ErrorTypes for the elements whose classes are missing.


> I believe the new tests could reuse some of the existing types in
> test/jdk/java/lang/annotation/Missing. For example, the new
> MissingAnnotation.java is an alpha-rename of the existing Missing.java. If
> such sharing is not practical, then I'd recommend putting the new files
> into a subdirectory under underneath test/.../annotation/Missing (otherwise
> it will be confusing to edit these tests in the future since too many file
> names will start with "Missing".)
>

I moved the new tests to a subdirectory. I think the rename of Missing to
MissingAnnotation is helpful to distinguish it from the inputs for the
class and enum tests, and I wanted to avoid refactoring the existing test
case, but if you have additional suggestions about the best way to
structure this I'm happy to reorganize things.


> Something like
>
>     assertEquals(Arrays.equals(outer[1].value(), {String.class})
>
> would be more robust against any future changes in Class.toString.
> Likewise for the analogous comparisons.
>

Done.

Thanks,
Liam