LANG-1818: Fix ClassUtils.getShortClassName(Class) to correctly handle $ in valid class names#1591
Merged
garydgregory merged 3 commits intoapache:masterfrom Jan 28, 2026
Merged
Conversation
garydgregory
requested changes
Jan 27, 2026
Member
garydgregory
left a comment
There was a problem hiding this comment.
Hello @inponomarev
Thank you for the PR. I have a few comments here and there.
|
|
||
| @Test | ||
| void testDollarSignImmediatelyAfterPackage() { | ||
| String result = ClassUtils.getShortClassName($trange.class); |
Contributor
Author
There was a problem hiding this comment.
fixed -- inlined result everywhere
| dim++; | ||
| c = c.getComponentType(); | ||
| } | ||
|
|
Member
There was a problem hiding this comment.
Remove the extra blank line. If you want to "phrase" a method, you can use a // comment to explain what's happening.
| } | ||
|
|
||
| final StringBuilder sb = new StringBuilder(base); | ||
| for (int i = 0; i < dim; i++) { |
Member
There was a problem hiding this comment.
You could reuse StringUtils.repeat() or AppendableJoiner.join(StringBuilder, T...) or at least pre-allocate the StringBuilder since its final size is known.
Contributor
Author
There was a problem hiding this comment.
done, indeed, why not StringUtils.repeat()?
52f3210 to
ada66f9
Compare
garydgregory
added a commit
that referenced
this pull request
Jan 28, 2026
$ in valid class names #1591 - Javadoc - Sort class name - Sort new members
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This issue is tracked in https://issues.apache.org/jira/browse/LANG-1818
Problem
ClassUtils.getShortClassName(Class)currently delegates to the string-basedgetShortClassName(String)usingcls.getName().The string-based method is inherently heuristic and, as documented, cannot reliably distinguish package names, outer classes, and inner classes when given only a JVM binary name. As a result, any
$character is treated as an inner-class separator, even when$is part of a legitimate Java identifier.While this limitation is unavoidable for
getShortClassName(String), it should not apply togetShortClassName(Class), where full reflective metadata is available.Changes
getShortClassName(Class)(and by extensiongetShortClassName(Object))to useClassmetadata instead of parsingClass.getName().$characters that are part of actual class identifiers (top-level classes, member classes, and nested member classes).Examples (fixed behaviour)
Rationale
The ambiguity of $ in JVM binary names is explicitly documented for the string-based APIs. However, when a Class<?> instance is available, that ambiguity disappears. This change improves correctness for the Class-based overloads without altering or reinterpreting the behavior of the string-based methods.