Copilot commented on code in PR #1820:
URL: https://github.com/apache/commons-lang/pull/1820#discussion_r4287263972
##########
src/main/java/org/apache/commons/lang3/reflect/TypeUtils.java:
##########
@@ -1110,6 +1115,43 @@ private static boolean isAssignable(final Type type,
final ParameterizedType toP
}
// get the target type's raw type
final Class<?> toClass = getRawType(toParameterizedType);
+ if (!allowUnchecked) {
+ // Type argument bounds require subtyping, so a raw supertype
cannot supply missing type arguments.
+ if (type instanceof TypeVariable<?>) {
+ for (final Type bound : getImplicitBounds((TypeVariable<?>)
type)) {
+ if (isAssignable(bound, toParameterizedType,
typeVarAssigns, false)) {
+ return true;
+ }
+ }
+ return false;
+ }
+ if (type instanceof Class<?>) {
+ final Class<?> fromClass = (Class<?>) type;
+ if (isRawGenericClass(fromClass) || !isAssignable(fromClass,
toClass)) {
+ return false;
+ }
+ return toClass.equals(fromClass)
+ || isAssignable(getClosestParentType(fromClass,
toClass), toParameterizedType, typeVarAssigns, false);
+ }
+ if (type instanceof ParameterizedType) {
+ final ParameterizedType fromType = (ParameterizedType) type;
+ final Class<?> fromClass = getRawType(fromType);
+ if (!toClass.equals(fromClass)) {
+ if (!isAssignable(fromClass, toClass)) {
+ return false;
+ }
+ final Map<TypeVariable<?>, Type> fromAssignments =
getTypeArguments(fromType);
+ // Preserve externally declared variables when
substituting the parent type.
+ for (final Type argument : new
HashMap<>(fromAssignments).values()) {
+ if (argument instanceof TypeVariable<?>) {
+ fromAssignments.putIfAbsent((TypeVariable<?>)
argument, argument);
+ }
+ }
+ final Type parent = unrollVariables(fromAssignments,
getClosestParentType(fromClass, toClass));
Review Comment:
Copying the entire map just to iterate values (`new
HashMap<>(fromAssignments).values()`) is a bit indirect and easy to misread as
meaningful duplication. Consider iterating over a snapshot of the values (e.g.,
`new ArrayList<>(fromAssignments.values())`) or cloning once up-front into a
fresh `HashMap` that you freely mutate, to make the intent clearer and avoid an
extra map allocation.
##########
src/test/java/org/apache/commons/lang3/reflect/TypeUtilsTest.java:
##########
@@ -405,6 +472,49 @@ public static <T extends Comparable<? extends T>> T
stub3() {
return null;
}
+ static Stream<Arguments> testIsAssignable_ClassWildcardBounds() {
Review Comment:
The `@MethodSource` factory method has the same name as the parameterized
test method (`testIsAssignable_ClassWildcardBounds(...)`) via overloading,
which makes navigation and stack traces harder to interpret. Consider renaming
the arguments provider to something distinct (e.g.,
`classWildcardBoundsArguments`) and referencing it explicitly in
`@MethodSource(\"classWildcardBoundsArguments\")`.
--
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]
