[
https://issues.apache.org/jira/browse/LANG-1836?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
William Degrange updated LANG-1836:
-----------------------------------
Description:
*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)
was:
*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.
*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}
*Actual:* {{{}false{}}}, {{{}false{}}}, {{Outer<T>.Inner}}
*Expected:* {{{}true{}}}, {{{}true{}}}, {{Outer<String>.Inner}}
*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}
Suggested regression tests:
* {{Outer<T>.Inner}} → {{true}}
* {{Outer<String>.Inner}} → {{false}}
* {{Map.Entry<String, Integer>}} → {{false}} (the owner is the raw
{{{}Map.class{}}})
> 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)
