RFR 8054213: Class name repeated in output of Type.toString()

[joe darcy]

Hi Svetlana,

Looks okay.

I approve pushing the fix with the following change to the test: have 
the expected output align with the given output. The webrev shows

   63             "FirstInnerClassGeneric<Dummy>$SecondInnerClassGeneric<Dummy>")
   64     public FirstInnerClassGeneric<Dummy>.SecondInnerClassGeneric<Dummy> foo1()

and I think this is clearer as

   63           "FirstInnerClassGeneric<Dummy>$SecondInnerClassGeneric<Dummy>")
   64     public FirstInnerClassGeneric<Dummy>.SecondInnerClassGeneric<Dummy> foo1()

even if the spacing is less conventional.

No need to generate another webrev.

Cheers,

-Joe


On 7/13/2016 8:35 AM, Svetlana Nikandrova wrote:
> Kindly reminder.
>
> On 07.07.2016 18:49, Svetlana Nikandrova wrote:
>> Hello all,
>>
>> seems like Joe is busy right now, so meanwhile I'll be happy to hear 
>> other opinions on this topic.
>>
>> Thank you,
>> Svetlana
>>
>> On 30.06.2016 19:44, Svetlana Nikandrova wrote:
>>> Hello Joe,
>>>
>>> thank you for your advice! GenericStringTest looks really good with 
>>> annotations. I refactored my test, please see updated webrev:
>>> http://cr.openjdk.java.net/~snikandrova/8054213/webrev.01/ 
>>> <http://cr.openjdk.java.net/%7Esnikandrova/8054213/webrev.01/>
>>>
>>> As for "." vs "$" let me provide an example:
>>> E.g. we have class
>>> public class Test { public Nested1<Test>.Nested2<Test> foo() { 
>>> return null; } public class Nested1<T> { public class Nested2<T>{} } }
>>> The output of the
>>> System.out.println(Test.class.getMethod("foo", 
>>> null).getReturnType()); Output: class Test$Nested1$Nested2 (nested 
>>> classes divided by "$")
>>> while
>>> System.out.println(Test.class.getMethod("foo", 
>>> null).getGenericReturnType()); Output:
>>> Test$Nested1<Test>.Nested2<Test> (nested classes divided by "." if 
>>> enclosed by parametrized type and "$" in other cases).
>>> I think it's a little bit strange to have different separators for 
>>> inner classes depended on is it nested by parametrized or raw type.
>>>
>>> Thank you,
>>> Svetlana
>>>
>>> On 28.06.2016 22:02, joe darcy wrote:
>>>> Hello Svetlana,
>>>>
>>>> I'm not convinced '$' should be replaced with '.' in this context 
>>>> as '.' is what the separator used in source code.
>>>>
>>>> In any case, the test should be restructured. I recommend declaring 
>>>> an annotation type to hold the expected to string output. You can 
>>>> them loop over the methods from the class object and for the 
>>>> methods with the annotation verifying the toString output is as 
>>>> expected. For a guide see
>>>>
>>>> test/java/lang/reflect/Constructor/GenericStringTest.java
>>>>
>>>> Thanks,
>>>>
>>>> -Joe
>>>>
>>>>
>>>> On 6/28/2016 11:25 AM, Svetlana Nikandrova wrote:
>>>>> May be someone can find a minute?
>>>>>
>>>>> On 27.06.2016 21:25, Svetlana Nikandrova wrote:
>>>>>> Kindly reminder
>>>>>>
>>>>>> On 24.06.2016 14:58, Svetlana Nikandrova wrote:
>>>>>>> Hello,
>>>>>>>
>>>>>>> let me try again with updated version of webrev:
>>>>>>> Webrev:
>>>>>>> http://cr.openjdk.java.net/~snikandrova/8054213/webrev.01/ 
>>>>>>> <http://cr.openjdk.java.net/%7Esnikandrova/8054213/webrev.01/>
>>>>>>> Issue:
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8054213
>>>>>>>
>>>>>>> Many thanks goes to Sergey Ustimenko for his valuable advises. I 
>>>>>>> believe fix looks nicer now.
>>>>>>> JPRT tested.
>>>>>>>
>>>>>>> Thank you,
>>>>>>> Svetlana
>>>>>>>
>>>>>>> On 17.06.2016 21:45, Svetlana Nikandrova wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> could you please review this fix for toString() method of 
>>>>>>>> ParameterizedTypeImpl.
>>>>>>>> The problem is that when we obtain simple name of nested type 
>>>>>>>> shared prefix is only removed for ParameterizedType owner. We 
>>>>>>>> need to remove it for other cases too.
>>>>>>>>
>>>>>>>> Please note that I also changed delimiter between outer and 
>>>>>>>> inner class from "." to "$". I believe as "$" is usually used 
>>>>>>>> to separate nested class name "." looks inconsistent and 
>>>>>>>> confusing.
>>>>>>>> Let me know if you have any objections.
>>>>>>>>
>>>>>>>> Webrev:
>>>>>>>> http://cr.openjdk.java.net/~msolovie/8054213/webrev.00/ 
>>>>>>>> <http://cr.openjdk.java.net/%7Emsolovie/8054213/webrev.00/>
>>>>>>>> Issue:
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8054213
>>>>>>>>
>>>>>>>> Thank you,
>>>>>>>> Svetlana
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>