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

[joe darcy]

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 ", "")));