Hello,
A few initial comments, not my final review.
Objects implementing the AnnotatedElement interface are also created in
javac during annotation processing. As a secondary concern, it would be
good to be have behavior between both javac and runtime annotations
consistent when possible. (My own to-do list includes at least once such
alignment JDK-8164819: "Make javac's toString() on annotation objects
consistent with core reflection".)
Even in javac we've moved away from test and directory names based on
bugid. I'd recommend incorporating these regression tests into the
existing tests in
test/jdk/java/lang/annotation/Missing
or creating a subdirectory for conditions, etc..
It would be worth verifying whether or not this change also covers
java.lang.reflect.Executable.getParameterAnnotations(), more
specifically the implementation of that method in Method and
Constructor. The method getParameterAnnotations() is much less used than
getAnnotations and the other methods on the AnnotatedElement interface,
but still part of the annotations feature.
(As a follow-up refactoring, it might be worthwhile to replace the
interior of the three parseFooArray methods to a shared method that is
passed a lambda for the "Object value = parseFooValue(/*args to get
foo*/...);" logic.)
Thanks,
-Joe
On 2/23/2018 10:06 AM, joe darcy wrote:
On 2/23/2018 9:39 AM, Alan Bateman wrote:
On 22/02/2018 23:20, Liam Miller-Cushon wrote:
Hello,
Please consider this fix for JDK-7183985.
bug: https://bugs.openjdk.java.net/browse/JDK-7183985
webrev: http://cr.openjdk.java.net/~cushon/7183985/webrev.01/
I started a CSR for the change:
https://bugs.openjdk.java.net/browse/JDK-8198584
We have been using the fix at Google for about two years, and there has
been no compatibility impact. I found very few places
ArrayStoreException
was being explicitly handled, and none that were depending on the
current
behaviour of getAnnotation().
There was some previous discussion of the bug on core-libs-dev:
Yes, this one comes up every few years. I'm hoping Joe Darcy will
reply to your review with any background or issues from when this
came up in the past. From a distance then retrofitting
AnnotatedElement getXXX methods to throw TypeNotPresentException
seems reasonable, I'm less sure about the isAnnotationPresent method
as it might be surprising for that to fail.
On my list!
Cheers,
-Joe
