JDK 12 RFR of JDK-6304578: (reflect) toGenericString fails to print bounds of type variables on generic methods

[Vicente Romero]

last iteration looks good to me,

Vicente

On 10/25/18 5:47 PM, joe darcy wrote:
> Hi Peter,
>
> Coming back to this review after my Code One activities this year have 
> run their course...
>
>
> On 10/17/2018 3:07 PM, Peter Levart wrote:
>> Hi Joe,
>>
>> On 10/17/2018 09:16 PM, joe darcy wrote:
>>> PS In response to some off-list feedback, an updated webrev uses a 
>>> stream-ier implementation:
>>>
>>> http://cr.openjdk.java.net/~darcy/6304578.1/
>>
>> I suggest further niceing:
>>
>> - if you make typeVarBounds() static, you could use it in 
>> Stream.map() as a method reference.
>> - in Class.java line 285, you could also use Type::getTypeName method 
>> reference
>> - in Class.java line 269, wouldn't the output be nicer if the 
>> delimiter in joining collector was ", " instead of "," (with a space 
>> after comma)?
>>
>> The same for Executable.
>>
>
> Revised webrev generally taking those suggestions up at:
>
>     http://cr.openjdk.java.net/~darcy/6304578.2/
>
> Diff of relevant parts of prior version below (screening out unrelated 
> changes to Class made in the interim).
>
> The behavior with respect to using a comma as a separator is part of 
> the spec of these methods and I don't want to alter that as part of 
> this change.
>
> Thanks,
>
> -Joe
>
> diff -r 6304578.1/raw_files/new/  6304578.2/raw_files/new/
> diff -r 
> 6304578.1/raw_files/new/src/java.base/share/classes/java/lang/Class.java 
> 6304578.2/raw_files/new/src/java.base/share/classes/java/lang/Class.java
> 268c268
> <         sb.append(Stream.of(typeparms).map(t -> typeVarBounds(t)).
> ---
> > sb.append(Stream.of(typeparms).map(Class::typeVarBounds).
> 279c279
> <     String typeVarBounds(TypeVariable<?> typeVar) {
> ---
> >     static String typeVarBounds(TypeVariable<?> typeVar) {
> 285c285
> <         Stream.of(bounds).map(e -> e.getTypeName()).
> ---
> >         Stream.of(bounds).map(Type::getTypeName).
> 3413c3413,3414
> <         StringJoiner sj = new StringJoiner(", ", getName() + "." + 
> name + "(", ")");
> ---
> >         StringBuilder sb = new StringBuilder();
> >     sb.append(getName() + "." + name + "(");
> 3415,3420c3416,3420
> <             for (int i = 0; i < argTypes.length; i++) {
> <                 Class<?> c = argTypes[i];
> <                 sj.add((c == null) ? "null" : c.getName());
> <             }
> <         }
> <         return sj.toString();
> ---
> >         Stream.of(argTypes).map(c -> {return (c == null) ? "null" : 
> c.getName();}).
> >         collect(Collectors.joining(","));
> >     }
> >     sb.append(")");
> >         return sb.toString();
> diff -r 
> 6304578.1/raw_files/new/src/java.base/share/classes/java/lang/reflect/Executable.java 
> 6304578.2/raw_files/new/src/java.base/share/classes/java/lang/reflect/Executable.java 
>
> 116c116
> <         sb.append(Stream.of(parameterTypes).map(p -> p.getTypeName()).
> ---
> > sb.append(Stream.of(parameterTypes).map(Type::getTypeName).
> 122c122
> <         sb.append(Stream.of(exceptionTypes).map(e -> e.getTypeName()).
> ---
> > sb.append(Stream.of(exceptionTypes).map(Type::getTypeName).
> 137c137
> <     String typeVarBounds(TypeVariable<?> typeVar) {
> ---
> >     static String typeVarBounds(TypeVariable<?> typeVar) {
> 143c143
> <         Stream.of(bounds).map(e -> e.getTypeName()).
> ---
> >         Stream.of(bounds).map(Type::getTypeName).
> 156c156
> <         sb.append(Stream.of(typeparms).map(t -> typeVarBounds(t)).
> ---
> > sb.append(Stream.of(typeparms).map(Executable::typeVarBounds).
> 176,180c176,177
> <                 StringJoiner joiner = new StringJoiner(",", " throws 
> ", "");
> <                 for (Type exceptionType : exceptionTypes) {
> <                     joiner.add(exceptionType.getTypeName());
> <                 }
> <                 sb.append(joiner.toString());
> ---
> > sb.append(Stream.of(exceptionTypes).map(Type::getTypeName).
> >               collect(Collectors.joining(",", " throws ", "")));
>
>