Copilot commented on code in PR #1812:
URL: https://github.com/apache/commons-lang/pull/1812#discussion_r4207696244
##########
src/main/java/org/apache/commons/lang3/reflect/TypeUtils.java:
##########
@@ -384,12 +384,15 @@ public static boolean containsTypeVariables(final Type
type) {
return ((Class<?>) type).getTypeParameters().length > 0;
}
if (type instanceof ParameterizedType) {
- for (final Type arg : ((ParameterizedType)
type).getActualTypeArguments()) {
+ final ParameterizedType parameterizedType = (ParameterizedType)
type;
+ for (final Type arg : parameterizedType.getActualTypeArguments()) {
if (containsTypeVariables(arg)) {
return true;
}
}
- return false;
+ // A raw Class owner (for example Map.class for Map.Entry<String,
Integer>) binds no variables.
+ final Type ownerType = parameterizedType.getOwnerType();
+ return ownerType != null && !(ownerType instanceof Class<?>) &&
containsTypeVariables(ownerType);
Review Comment:
A custom `ParameterizedType` with no arguments whose owner is itself now
causes `containsTypeVariables()` to throw `StackOverflowError`.
`unrollVariables()` fails in the same check before reaching its variable-cycle
guard. This is the shape already exercised by
`testCyclicOwnerParameterizedTypeToString`; mutually cyclic owners have the
same problem. Track visited type identities during owner traversal, and add
regression tests for these two APIs rather than guarding only against an owner
equal to the current type.
##########
src/main/java/org/apache/commons/lang3/reflect/TypeUtils.java:
##########
@@ -1728,25 +1731,24 @@ private static Type unrollVariables(final
Map<TypeVariable<?>, Type> typeArgumen
if (!visited.add(var)) {
return var;
}
- return unrollVariables(typeArguments, typeArguments.get(type),
visited);
+ final Type unrolled = unrollVariables(typeArguments,
typeArguments.get(type), visited);
+ // Only guard against cycles: the same variable may
legitimately occur more than once in a type.
+ visited.remove(var);
+ return unrolled;
}
if (type instanceof ParameterizedType) {
final ParameterizedType p = (ParameterizedType) type;
- final Map<TypeVariable<?>, Type> parameterizedTypeArguments;
- if (p.getOwnerType() == null) {
- parameterizedTypeArguments = typeArguments;
- } else {
- parameterizedTypeArguments = new HashMap<>(typeArguments);
- parameterizedTypeArguments.putAll(getTypeArguments(p));
- }
final Type[] args = p.getActualTypeArguments().clone();
for (int i = 0; i < args.length; i++) {
- final Type unrolled =
unrollVariables(parameterizedTypeArguments, args[i], visited);
+ final Type unrolled = unrollVariables(typeArguments,
args[i], visited);
if (unrolled != null) {
args[i] = unrolled;
}
}
- return parameterizeWithOwner(p.getOwnerType(), (Class<?>)
p.getRawType(), args);
+ // A raw Class owner (for example Map.class for Map.Entry<T,
Integer>) has nothing to unroll.
+ final Type owner = p.getOwnerType();
+ final Type unrolledOwner = owner != null && !(owner instanceof
Class<?>) ? unrollVariables(typeArguments, owner, visited) : owner;
Review Comment:
With an assignment `{T -> Outer<? extends T>.Inner}`, unrolling `T` now ends
in `StackOverflowError`. The new owner traversal reaches the wildcard, but
`unrollBounds()` calls the public `unrollVariables()` overload, which creates a
fresh `visited` set. The active `T` is forgotten and its assignment expands
indefinitely; previously the owner-only type was returned unchanged. Pass the
current `visited` set through `unrollBounds()` for both upper and lower bounds,
and add an owner/wildcard-cycle regression test.
--
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]
