RFR 8054213: Class name repeated in output of Type.toString()
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
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
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
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
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
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 classTest { publicNested1<Test>.Nested2<Test> foo() { return null; } public classNested1<T> { public classNested2<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
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 classTest {
publicNested1<Test>.Nested2<Test> foo() { return null; }
public classNested1<T> { public classNested2<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
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 classTest {
publicNested1<Test>.Nested2<Test> foo() { return null; }
public classNested1<T> { public classNested2<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
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 >
Thank you, Joe! Sure, I'll change alignment in the test. Thanks, Svetlana On 15.07.2016 8:56, joe darcy wrote:
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 classTest {
publicNested1<Test>.Nested2<Test> foo() { return null; }
public classNested1<T> { public classNested2<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 >> >
participants (2)
-
joe darcy
-
Svetlana Nikandrova