RFR: 8022343: j.l.Class.getAnnotatedSuperclass() doesn't return null in some cases
Joe Darcy
joe.darcy at oracle.com
Fri Aug 23 16:57:14 UTC 2013
Looks good to go back; thanks,
-Joe
On 08/23/2013 09:54 AM, Joel Borggrén-Franck wrote:
> Fixed,
>
> http://cr.openjdk.java.net/~jfranck/8022343/webrev.02/
>
> cheers
> /Joel
>
> On Aug 23, 2013, at 2:34 AM, Joseph Darcy <joe.darcy at oracle.com> wrote:
>
>> Hi Joel,
>>
>> The new version is better, but for the testing in question I would prefer to see something even simpler like:
>>
>> public static void main(String[] args) throws Exception {
>> int failed = 0;
>> Class<?>[] testData = {/* list of class literals*/}
>> for (Class<?> toTest: testData) {
>> Object res = toTest.getAnnotatedSuperclass();
>>
>> if (res != null) {
>> failed++;
>> System.out.println(toTest + ".getAnnotatedSuperclass() returns: "
>> + res + ", was non-null");
>> }
>> }
>>
>> if (failed != 0)
>> throw new RuntimeException("Test failed, check log for details");
>> }
>>
>>
>> -Joe
>>
>> On 8/21/2013 5:04 AM, Joel Borggrén-Franck wrote:
>>> Hi Joe, Paul
>>>
>>> I rewrote the test in Paul's style without using testNG.
>>>
>>> http://cr.openjdk.java.net/~jfranck/8022343/webrev.01/
>>>
>>> Please review.
>>>
>>> cheers
>>> /Joel
>>>
>>> On 2013-08-19, Joe Darcy wrote:
>>>> Hi Joel,
>>>>
>>>> I agree the code looks fine.
>>>>
>>>> However, I concur with the general sentiment of Paul test advice
>>>> without advocating using testng for this task.
>>>>
>>>> A loop over a Class<?>[] initialized with the kinds of values of
>>>> interest would seem to be better structured to me and allow for
>>>> better exception handing, etc.
>>>>
>>>> -Joe
>>>>
>>>> On 08/19/2013 01:34 AM, Paul Sandoz wrote:
>>>>> Hi Joel,
>>>>>
>>>>> The fix looks OK.
>>>>>
>>>>> Not suggesting you do the following, unless you really want to, but the test is an example of where TestNG data providers are useful, since all cases will be tested and reported for pass or failure, rather than in this case the first failure will cause other checks (if any) not to be tested and in addition will not report which check failed (one needs to look at the stack trace).
>>>>>
>>>>> Not tested:
>>>>>
>>>>> @DataProvider(name = "class")
>>>>> private static Object[][] getClasses() {
>>>>> // Using the stream API because i can :-)
>>>>> // Arguably simpler in this case to use new Object[][] { {} }
>>>>> return Stream.of(
>>>>> Object.class,
>>>>> If.class,
>>>>> Object[].class,
>>>>> void.class,
>>>>> int.class).
>>>>> map(e -> new Object[] { e }).
>>>>> toArray(Object[][]::new);
>>>>> }
>>>>>
>>>>> @Test(dataProvider = "class")
>>>>> public void testAnnotatedSuperClassIsNull(Class c) {
>>>>> assertNull(c.getAnnotatedSuperclass())
>>>>> }
>>>>>
>>>>> Paul.
>>>>>
>>>>> On Aug 16, 2013, at 2:17 PM, Joel Borggren-Franck <joel.franck at oracle.com> wrote:
>>>>>
>>>>>> Hi
>>>>>>
>>>>>> Please review this small fix for a type annotation reflection issue.
>>>>>>
>>>>>> The javadoc spec for Class.getAnnotatedSuperclass says:
>>>>>>
>>>>>> * If this Class represents either the Object class, an interface type, an
>>>>>> * array type, a primitive type, or void, the return value is null.
>>>>>>
>>>>>> The patch fixes this.
>>>>>>
>>>>>> Webrev at: http://cr.openjdk.java.net/~jfranck/8022343/webrew.00/
>>>>>>
>>>>>> Patch also included it at the end of this mail.
>>>>>>
>>>>>> cheers
>>>>>> /Joel
>>>>>>
>>>>>>
>>>>>>
>>>>>> diff -r b07b19182e40 src/share/classes/java/lang/Class.java
>>>>>> --- a/src/share/classes/java/lang/Class.java Thu Aug 15 15:04:59 2013 +0100
>>>>>> +++ b/src/share/classes/java/lang/Class.java Fri Aug 16 13:20:31 2013 +0200
>>>>>> @@ -3338,8 +3338,16 @@
>>>>>> * @since 1.8
>>>>>> */
>>>>>> public AnnotatedType getAnnotatedSuperclass() {
>>>>>> - return TypeAnnotationParser.buildAnnotatedSuperclass(getRawTypeAnnotations(), getConstantPool(), this);
>>>>>> -}
>>>>>> + if(this == Object.class ||
>>>>>> + isInterface() ||
>>>>>> + isArray() ||
>>>>>> + isPrimitive() ||
>>>>>> + this == Void.TYPE) {
>>>>>> + return null;
>>>>>> + }
>>>>>> +
>>>>>> + return TypeAnnotationParser.buildAnnotatedSuperclass(getRawTypeAnnotations(), getConstantPool(), this);
>>>>>> + }
>>>>>>
>>>>>> /**
>>>>>> * Returns an array of AnnotatedType objects that represent the use of types to
>>>>>> diff -r b07b19182e40 test/java/lang/annotation/TypeAnnotationReflection.java
>>>>>> --- a/test/java/lang/annotation/TypeAnnotationReflection.java Thu Aug 15 15:04:59 2013 +0100
>>>>>> +++ b/test/java/lang/annotation/TypeAnnotationReflection.java Fri Aug 16 13:20:31 2013 +0200
>>>>>> @@ -23,7 +23,7 @@
>>>>>>
>>>>>> /*
>>>>>> * @test
>>>>>> - * @bug 8004698 8007073
>>>>>> + * @bug 8004698 8007073 8022343
>>>>>> * @summary Unit test for type annotations
>>>>>> */
>>>>>>
>>>>>> @@ -58,7 +58,7 @@
>>>>>> }
>>>>>>
>>>>>> private static void testSuper() throws Exception {
>>>>>> - check(Object.class.getAnnotatedSuperclass().getAnnotations().length == 0);
>>>>>> + check(Object.class.getAnnotatedSuperclass() == null);
>>>>>> check(Class.class.getAnnotatedSuperclass().getAnnotations().length == 0);
>>>>>>
>>>>>> AnnotatedType a;
>>>>>> diff -r b07b19182e40 test/java/lang/annotation/typeAnnotations/GetAnnotatedSuperclass.java
>>>>>> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
>>>>>> +++ b/test/java/lang/annotation/typeAnnotations/GetAnnotatedSuperclass.java Fri Aug 16 13:20:31 2013 +0200
>>>>>> @@ -0,0 +1,50 @@
>>>>>> +/*
>>>>>> + * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
>>>>>> + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>>>>> + *
>>>>>> + * This code is free software; you can redistribute it and/or modify it
>>>>>> + * under the terms of the GNU General Public License version 2 only, as
>>>>>> + * published by the Free Software Foundation.
>>>>>> + *
>>>>>> + * This code is distributed in the hope that it will be useful, but WITHOUT
>>>>>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>>>>>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
>>>>>> + * version 2 for more details (a copy is included in the LICENSE file that
>>>>>> + * accompanied this code).
>>>>>> + *
>>>>>> + * You should have received a copy of the GNU General Public License version
>>>>>> + * 2 along with this work; if not, write to the Free Software Foundation,
>>>>>> + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
>>>>>> + *
>>>>>> + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
>>>>>> + * or visit www.oracle.com if you need additional information or have any
>>>>>> + * questions.
>>>>>> + */
>>>>>> +
>>>>>> +/*
>>>>>> + * @test
>>>>>> + * @bug 8022343
>>>>>> + * @summary make sure Class.getAnnotatedSuperclass() returns null when specified to do so
>>>>>> + */
>>>>>> +
>>>>>> +import java.util.*;
>>>>>> +import java.lang.annotation.*;
>>>>>> +import java.lang.reflect.*;
>>>>>> +import java.io.Serializable;
>>>>>> +
>>>>>> +public class GetAnnotatedSuperclass {
>>>>>> + public static void main(String[] args) throws Exception {
>>>>>> + check(Object.class.getAnnotatedSuperclass() == null);
>>>>>> + check(If.class.getAnnotatedSuperclass() == null);
>>>>>> + check(Object[].class.getAnnotatedSuperclass() == null);
>>>>>> + check(void.class.getAnnotatedSuperclass() == null);
>>>>>> + check(int.class.getAnnotatedSuperclass() == null);
>>>>>> + }
>>>>>> +
>>>>>> + private static void check(boolean b) {
>>>>>> + if (!b)
>>>>>> + throw new RuntimeException();
>>>>>> + }
>>>>>> + interface If {}
>>>>>> +}
>>>>>> +
More information about the core-libs-dev
mailing list