You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2022/09/15 14:35:25 UTC
[GitHub] [commons-lang] thiyagu-7 opened a new pull request, #949: LANG-1691: ClassUtils.getShortCanonicalName - using canonicalName rat…
thiyagu-7 opened a new pull request, #949:
URL: https://github.com/apache/commons-lang/pull/949
JIRA issue - https://issues.apache.org/jira/browse/LANG-1691
The `getShortCanonicalName` is using class name rather than the canonical name. Fixing it by using the canonicalName.
Also updated the unit tests because of this change. For some of the tests, the canonical name will be null and hence changed the expected value to be an empty string. For inner class, the output will not have the outer class name.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] garydgregory merged pull request #949: LANG-1691: ClassUtils.getShortCanonicalName - using canonicalName rat…
Posted by GitBox <gi...@apache.org>.
garydgregory merged PR #949:
URL: https://github.com/apache/commons-lang/pull/949
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] thiyagu-7 commented on a diff in pull request #949: LANG-1691: ClassUtils.getShortCanonicalName - using canonicalName rat…
Posted by GitBox <gi...@apache.org>.
thiyagu-7 commented on code in PR #949:
URL: https://github.com/apache/commons-lang/pull/949#discussion_r972177609
##########
src/test/java/org/apache/commons/lang3/ClassUtilsTest.java:
##########
@@ -540,12 +539,11 @@ public void test_getShortCanonicalName_Object() {
class Named {
// empty
}
- // WARNING: this is fragile, implementation may change, naming is not guaranteed
- assertEquals("ClassUtilsTest.9", ClassUtils.getShortCanonicalName(new Object() {
+ assertEquals("", ClassUtils.getShortCanonicalName(new Object() {
// empty
}, "<null>"));
- assertEquals("ClassUtilsTest.9Named", ClassUtils.getShortCanonicalName(new Named(), "<null>"));
- assertEquals("ClassUtilsTest.Inner", ClassUtils.getShortCanonicalName(new Inner(), "<null>"));
+ assertEquals("", ClassUtils.getShortCanonicalName(new Named(), "<null>"));
Review Comment:
I felt the same. It is because it is a _local class_. Local classes don't have canonical name
https://docs.oracle.com/javase/8/docs/api/java/lang/Class.html#getCanonicalName--
> Returns null if the underlying class does not have a canonical name (i.e., if it is a local or anonymous class or an array whose component type does not have a canonical name).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] garydgregory commented on a diff in pull request #949: LANG-1691: ClassUtils.getShortCanonicalName - using canonicalName rat…
Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #949:
URL: https://github.com/apache/commons-lang/pull/949#discussion_r972116172
##########
src/test/java/org/apache/commons/lang3/ClassUtilsTest.java:
##########
@@ -540,12 +539,11 @@ public void test_getShortCanonicalName_Object() {
class Named {
// empty
}
- // WARNING: this is fragile, implementation may change, naming is not guaranteed
- assertEquals("ClassUtilsTest.9", ClassUtils.getShortCanonicalName(new Object() {
+ assertEquals("", ClassUtils.getShortCanonicalName(new Object() {
// empty
}, "<null>"));
- assertEquals("ClassUtilsTest.9Named", ClassUtils.getShortCanonicalName(new Named(), "<null>"));
- assertEquals("ClassUtilsTest.Inner", ClassUtils.getShortCanonicalName(new Inner(), "<null>"));
+ assertEquals("", ClassUtils.getShortCanonicalName(new Named(), "<null>"));
Review Comment:
This seems obviously no good to me because "Named" is a real concrete class defined in this very file, it's name is not "". What am I missing?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] thiyagu-7 commented on a diff in pull request #949: LANG-1691: ClassUtils.getShortCanonicalName - using canonicalName rat…
Posted by GitBox <gi...@apache.org>.
thiyagu-7 commented on code in PR #949:
URL: https://github.com/apache/commons-lang/pull/949#discussion_r972177609
##########
src/test/java/org/apache/commons/lang3/ClassUtilsTest.java:
##########
@@ -540,12 +539,11 @@ public void test_getShortCanonicalName_Object() {
class Named {
// empty
}
- // WARNING: this is fragile, implementation may change, naming is not guaranteed
- assertEquals("ClassUtilsTest.9", ClassUtils.getShortCanonicalName(new Object() {
+ assertEquals("", ClassUtils.getShortCanonicalName(new Object() {
// empty
}, "<null>"));
- assertEquals("ClassUtilsTest.9Named", ClassUtils.getShortCanonicalName(new Named(), "<null>"));
- assertEquals("ClassUtilsTest.Inner", ClassUtils.getShortCanonicalName(new Inner(), "<null>"));
+ assertEquals("", ClassUtils.getShortCanonicalName(new Named(), "<null>"));
Review Comment:
I felt the same. It is because it is a _local class_. Local classes don't have canonical name
https://docs.oracle.com/javase/8/docs/api/java/lang/Class.html#getCanonicalName--
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] garydgregory commented on a diff in pull request #949: LANG-1691: ClassUtils.getShortCanonicalName - using canonicalName rat…
Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #949:
URL: https://github.com/apache/commons-lang/pull/949#discussion_r972116172
##########
src/test/java/org/apache/commons/lang3/ClassUtilsTest.java:
##########
@@ -540,12 +539,11 @@ public void test_getShortCanonicalName_Object() {
class Named {
// empty
}
- // WARNING: this is fragile, implementation may change, naming is not guaranteed
- assertEquals("ClassUtilsTest.9", ClassUtils.getShortCanonicalName(new Object() {
+ assertEquals("", ClassUtils.getShortCanonicalName(new Object() {
// empty
}, "<null>"));
- assertEquals("ClassUtilsTest.9Named", ClassUtils.getShortCanonicalName(new Named(), "<null>"));
- assertEquals("ClassUtilsTest.Inner", ClassUtils.getShortCanonicalName(new Inner(), "<null>"));
+ assertEquals("", ClassUtils.getShortCanonicalName(new Named(), "<null>"));
Review Comment:
This seems obviously no good to me because "Named" is a real concrete class defined in this very file, its name is not "". What am I missing?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] garydgregory commented on a diff in pull request #949: LANG-1691: ClassUtils.getShortCanonicalName - using canonicalName rat…
Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #949:
URL: https://github.com/apache/commons-lang/pull/949#discussion_r972342964
##########
src/test/java/org/apache/commons/lang3/ClassUtilsTest.java:
##########
@@ -540,12 +539,11 @@ public void test_getShortCanonicalName_Object() {
class Named {
// empty
}
- // WARNING: this is fragile, implementation may change, naming is not guaranteed
- assertEquals("ClassUtilsTest.9", ClassUtils.getShortCanonicalName(new Object() {
+ assertEquals("", ClassUtils.getShortCanonicalName(new Object() {
// empty
}, "<null>"));
- assertEquals("ClassUtilsTest.9Named", ClassUtils.getShortCanonicalName(new Named(), "<null>"));
- assertEquals("ClassUtilsTest.Inner", ClassUtils.getShortCanonicalName(new Inner(), "<null>"));
+ assertEquals("", ClassUtils.getShortCanonicalName(new Named(), "<null>"));
Review Comment:
@thiyagu-7
Thank you for the clarification!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org