JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

[Mike Duigou]

I reviewed the BlenderRev fairly closely and did not find any errors. The only weirdness I saw was several cases where multiple "Specified by:" declarations were present for a method and one of the instances referenced a class to which it didn't appear to be able to link to. 

Example: Method.getTypeParameters():

Specified by:
getTypeParameters in interface java.lang.reflect.GenericDeclaration

It wasn't clear to me why it needed two "Specified by:" entries and only one of them was hot linked to the specifying class.

Just javadoc weirdness?

Mike

On Jul 19 2011, at 12:49 , Joe Darcy wrote:

> Agreed; I've posted a BlenderRev corresponding to the current patch at:
> 
>   http://cr.openjdk.java.net/~darcy/7007535.4/BR-7007535.html
> 
> Thanks,
> 
> -Joe
> 
> Mike Duigou wrote:
>> This looks good to me but I agree with David that it's probably important to look at the generated javadoc and specdiff output before finalizing.
>> 
>> Mike
>> 
>> On Jul 18 2011, at 00:51 , Joe Darcy wrote:
>> 
>>  
>>> Peter and David.
>>> 
>>> Thanks for the careful review; the @throws information still needs its own {@inheritDoc}; I've uploaded a webrev with this and other corrections:
>>> 
>>> http://cr.openjdk.java.net/~darcy/7007535.4
>>> 
>>> More comments interspersed below.
>>> 
>>> Thanks,
>>> 
>>> -Joe
>>> 
>>> Peter Jones wrote:
>>>    
>>>> Hi Joe,
>>>> 
>>>> On Jul 15, 2011, at 1:49 AM, David Holmes wrote:
>>>>       
>>>>> On 14/07/2011 12:21 PM, joe.darcy at oracle.com wrote:
>>>>>           
>>>>>> Please code review my JDK 8 changes for
>>>>>> 
>>>>>> 7007535: (reflect) Please generalize Constructor and Method
>>>>>> http://cr.openjdk.java.net/~darcy/7007535.3
>>>>>> 
>>>>>> To summarize the changes, a new superclass is defined to capture the common
>>>>>> functionality of java.lang.reflect.Method and java.lang.reflect.Constructor.
>>>>>> That superclass is named "Executable" along the lines of
>>>>>> javax.lang.model.ExecutableElement, which models constructors and methods in
>>>>>> the JSR 269 language model.
>>>>>> 
>>>>>> Both specification and implementation code are shared. To preserve the right
>>>>>> @since behavior, it is common that in Method/Constructor the javadoc for a
>>>>>> method will now look like:
>>>>>> 
>>>>>> /**
>>>>>> * {@inheritDoc}
>>>>>> * @since 1.5
>>>>>> */
>>>>>>               
>>>>> Unless they have fixed/changed javadoc (entirely possible) it used to be that the above would not cause @throws declarations for unchecked exceptions to be inherited - you have/had to explicitly repeat them as:
>>>>> 
>>>>> @throws <exception-type> {@inheritDoc}
>>>>>           
>>>> Yes, that would seem to be needed for some of the inherited getters of generics info, which specify unchecked exception types.
>>>> 
>>>>       
>>>>>> Since Executable is being created in JDK 8, it would be incorrect for
>>>>>> methods in that class to have an @since of 1.5; adding the @since in
>>>>>> Method/Constructor preserves the right information.
>>>>>>               
>>>> In Executable.java, getAnnotation and getDeclaredAnnotations do have "@since 1.5"-- oversight?
>>>>       
>>> Yes; that was incorrect.
>>> 
>>>    
>>>> In Constructor.java and Method.java, getExceptionTypes has "@since 1.5", but that method has existed in those classes since 1.1.
>>>> 
>>>>       
>>> Fixed.
>>> 
>>>    
>>>> In Executable.java:
>>>> 
>>>> 216     /**
>>>> 217      * Returns an array of {@code Class} objects that represent the formal
>>>> 218      * parameter types, in declaration order, of the method
>>>> 219      * represented by this {@code Method} object.  Returns an array of length
>>>> 220      * 0 if the underlying method takes no parameters.
>>>> 221      *
>>>> 222      * @return the parameter types for the method this object
>>>> 223      * represents
>>>> 
>>>> At least "{@code Method}" needs to be generalized, and perhaps all occurrences of "method"?
>>>>       
>>> Corrected.
>>>    
>> 
>>  
>