[
https://issues.apache.org/jira/browse/LANG-1836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18123785#comment-18123785
]
Gary D. Gregory commented on LANG-1836:
---------------------------------------
Hello [~dwilliam]
Thank you for your report.
Feel free to propose a PR on GitHub.
> TypeUtils.containsTypeVariables(Type) ignores the owner type of a
> ParameterizedType
> -----------------------------------------------------------------------------------
>
> Key: LANG-1836
> URL: https://issues.apache.org/jira/browse/LANG-1836
> Project: Commons Lang
> Issue Type: Bug
> Affects Versions: 3.21.0
> Reporter: William Degrange
> Priority: Major
>
> *Description*
> {{TypeUtils.containsTypeVariables(Type)}} returns {{false}} for a
> {{ParameterizedType}} whose owner type contains a type variable. The
> {{ParameterizedType}} branch only checks {{{}getActualTypeArguments(){}}};
> {{getOwnerType()}} is never inspected.
> This contradicts the Javadoc ("Tests, recursively, whether any of the type
> parameters associated with {{type}} are bound to variables"): the type
> parameters of the owner are part of the type's parameterization.
> Also, in the {{ParameterizedType}} branch of
> {{{}TypeUtils.unrollVariables(Map, Type){}}}:
> # *The owner type is never unrolled.* Only {{getActualTypeArguments()}} are
> processed; the result is built with
> {{{}parameterizeWithOwner(p.getOwnerType(), ...){}}}, i.e. the original
> owner. Type variables in the owner are left unresolved. (When the owner is
> the only place holding variables, the method returns early because
> {{containsTypeVariables}} ignores the owner, see the related issue. Fixing
> that issue alone is not enough.)
> # *Caller assignments are overridden.* When the owner is not null,
> {{getTypeArguments(p)}} is merged into the caller's map with {{{}putAll{}}}.
> For {{{}Outer<T>.Inner<T>{}}}, {{getTypeArguments(p)}} returns \{T -> T, U ->
> T}, and the identity entry {{T -> T}} replaces the caller's {{{}T ->
> String{}}}.
> This causes infinite recursion.
> *Steps to reproduce*
> {code:java}
> import java.lang.reflect.Type;
> import java.lang.reflect.TypeVariable;
> import java.util.Collections;
> import org.apache.commons.lang3.reflect.TypeUtils;
> public class ContainsTypeVariablesOwnerRepro {
> static class Outer<T> {
> class Inner {}
> Inner inner; // generic type is Outer<T>.Inner
> }
> public static void main(String[] args) throws Exception {
> Type fromReflection =
> Outer.class.getDeclaredField("inner").getGenericType();
> System.out.println(TypeUtils.containsTypeVariables(fromReflection));
> // false, expected true
> TypeVariable<?> t = Outer.class.getTypeParameters()[0];
> Type built =
> TypeUtils.parameterizeWithOwner(TypeUtils.parameterize(Outer.class, t),
> Outer.Inner.class);
> System.out.println(TypeUtils.containsTypeVariables(built)); // false,
> expected true
>
> // Consequence: unrollVariables short-circuits and does not
> substitute T
>
> System.out.println(TypeUtils.unrollVariables(Collections.singletonMap(t,
> String.class), fromReflection));
> // prints Outer<T>.Inner, expected Outer<String>.Inner
> }
> }{code}
> {code:java}
> import java.lang.reflect.Type;
> import java.lang.reflect.TypeVariable;
> import java.util.Collections;
> import java.util.Map;
> import org.apache.commons.lang3.reflect.TypeUtils;
> public class UnrollOwnerRepro {
> static class Outer<T> {
> class Inner<U> {}
> Inner<Integer> a; // Outer<T>.Inner<Integer>
> Inner<T> b; // Outer<T>.Inner<T>
> }
> public static void main(String[] args) throws Exception {
> TypeVariable<?> t = Outer.class.getTypeParameters()[0];
> Map<TypeVariable<?>, Type> map = Collections.singletonMap(t,
> String.class);
>
> Type a = Outer.class.getDeclaredField("a").getGenericType();
> System.out.println(TypeUtils.unrollVariables(map, a));
> // actual: Outer<T>.Inner<java.lang.Integer>
> // expected: Outer<java.lang.String>.Inner<java.lang.Integer>
>
> Type b = Outer.class.getDeclaredField("b").getGenericType();
> System.out.println(TypeUtils.unrollVariables(map, b));
> // actual: java.lang.StackOverflowError
> // expected: Outer<java.lang.String>.Inner<java.lang.String>
> }
>
> } {code}
> *Suggested fix*
> Recurse into the owner type, but only when it is not a raw {{{}Class{}}}. For
> a static nested type such as {{{}Map.Entry<String, Integer>{}}}, the owner is
> the raw {{{}Map.class{}}}, and the {{Class}} branch would return {{true}}
> because {{Map}} declares type parameters:
> {code:java}
> if (type instanceof ParameterizedType) {
> final ParameterizedType parameterizedType = (ParameterizedType) type;
> for (final Type arg : parameterizedType.getActualTypeArguments()) {
> if (containsTypeVariables(arg)) {
> return true;
> }
> }
> final Type ownerType = parameterizedType.getOwnerType();
> return ownerType != null && !(ownerType instanceof Class<?>) &&
> containsTypeVariables(ownerType);
> } {code}
> Unroll the owner like the arguments, with the caller's assignments, and keep
> the owner unchanged when it is a raw {{Class}} (static nested types such as
> {{{}Map.Entry{}}}):
> {code:java}
> if (type instanceof ParameterizedType) {
> final ParameterizedType p = (ParameterizedType) type;
> final Type[] args = p.getActualTypeArguments().clone();
> for (int i = 0; i < args.length; i++) {
> final Type unrolled = unrollVariables(typeArguments, args[i],
> visited);
> if (unrolled != null) {
> args[i] = unrolled;
> }
> }
> Type owner = p.getOwnerType();
> if (owner != null && !(owner instanceof Class<?>)) {
> final Type unrolledOwner = unrollVariables(typeArguments, owner,
> visited);
> if (unrolledOwner != null) {
> owner = unrolledOwner;
> }
> }
> return parameterizeWithOwner(owner, (Class<?>) p.getRawType(), args);
> } {code}
>
> The {{putAll(getTypeArguments(p))}} merge is dropped. The actual type
> arguments of {{p}} are expressed in terms of the enclosing scope's variables,
> not the raw type's own parameters, so merging the raw type's parameter
> mapping is not needed to resolve them. It is also what introduces the {{T ->
> T}} identity entry. If maintainers rely on that merge elsewhere, an
> alternative is to never let it override existing caller entries.
> Suggested regression tests (1):
> * {{Outer<T>.Inner}} → {{true}}
> * {{Outer<String>.Inner}} → {{false}}
> * {{Map.Entry<String, Integer>}} → {{false}} (the owner is the raw
> {{{}Map.class{}}})
> Suggested regression tests (2), with \{T -> String} :
> * {{Outer<T>.Inner<Integer>}} → {{Outer<String>.Inner<Integer>}}
> * {{Outer<T>.Inner<T>}} → {{Outer<String>.Inner<String>}}
> * {{Map.Entry<T, Integer>}} → {{Map.Entry<String, Integer>}} (the raw owner
> {{Map.class}} is left unchanged)
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
