alhudz commented on code in PR #1807:
URL: https://github.com/apache/commons-lang/pull/1807#discussion_r4177989677
##########
src/changes/changes.xml:
##########
@@ -46,6 +46,7 @@ The <action> type attribute can be add,update,fix,remove.
<body>
<release version="3.21.0" date="2026-09-25" description="This is a feature
and maintenance release. Java 8 or later is required.">
<!-- FIX -->
+ <action type="fix" dev="ggregory" due-to="Alhuda
Khan">TypeUtils.wildcardType().build() now reports Object as the implicit upper
bound, fixing WildcardType.getUpperBounds() and equals() symmetry with a JDK
wildcard.</action>
Review Comment:
Sorry for the slow reply. Dropped in 83c9bc5: `src/changes/changes.xml` is
back to the base version and out of the diff. I'll leave that file alone on
future PRs.
##########
src/main/java/org/apache/commons/lang3/reflect/TypeUtils.java:
##########
@@ -236,7 +236,9 @@ private static final class WildcardTypeImpl implements
WildcardType {
* @param lowerBounds of this type.
*/
private WildcardTypeImpl(final Type[] upperBounds, final Type[]
lowerBounds) {
- this.upperBounds = upperBounds != null ? upperBounds.clone() :
ArrayUtils.EMPTY_TYPE_ARRAY;
+ // A wildcard with no explicit upper bound has an implicit upper
bound of Object, per
+ // WildcardType.getUpperBounds(); returning an empty array breaks
equals() with a JDK wildcard.
+ this.upperBounds = ArrayUtils.isNotEmpty(upperBounds) ?
upperBounds.clone() : new Type[] {Object.class};
Review Comment:
@garydgregory Reviewed, it's valid. With the upper bound defaulted the two
wildcards are equal both ways but still hashed differently.
`repro:` `jdk.hashCode() == TypeUtils.wildcardType().build().hashCode()`,
where `jdk` is the `?` of a `Comparable<?>` field
`expected:` `true`
`actual (e90b50c):` `false`, the added assertion fails with `expected:
<640070710> but was: <654128897>`
`fix (2a9ef8e):` `WildcardTypeImpl.hashCode()` now returns
`Arrays.hashCode(lowerBounds) ^ Arrays.hashCode(upperBounds)`, and
`testWildcardTypeImplicitUpperBound` asserts `jdk.hashCode() ==
built.hashCode()`
The JDK doesn't specify that algorithm, this mirrors what its own
`WildcardTypeImpl` does, so the new assertion will flag it if a JDK ever
changes it. I also checked `? extends String` and `? super String` against
their JDK counterparts: equal both ways, same hash code. Default `mvn` goal
locally on Java 21: 89364 tests, 0 failures, checkstyle/spotbugs/pmd clean.
Not in this PR: `ParameterizedTypeImpl` and `GenericArrayTypeImpl` have the
same mismatch on `master` already (`TypeUtils.parameterize(Map.class,
String.class, Integer.class)` equals the JDK's `Map<String, Integer>` both ways
with a different hash code, same for `genericArrayType`). That isn't
wildcard-related, so I've kept it out of here and it'd be a separate PR.
Commons Text has no `TypeUtils` counterpart, so there's nothing to port on
that side.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
