-
Notifications
You must be signed in to change notification settings - Fork 137
com.strobel.reflection.TypeCache can store incorrect association between Object[] and array of generic parameters #41
Description
The TypeCache class generates the same descriptor for both Object[] and T[], causing incorrect association of a descriptor to a type, depending on which type populates the cache first.
Below code provides a test example
package com.strobel.reflection;
import org.junit.Test;
import java.util.Arrays;
import static org.junit.Assert.assertEquals;
public class CustomTests {
@Test
public void genericParameterArrayTypesDoNotCollideInTypeCache() {
final Type<MyClass> c1 = Type.of(MyClass.class); // Load class with method having T[] parameter
c1.getMethods(); // Load methods of class with method having T[] parameter
final Type<MyOtherClass> c2 = Type.of(MyOtherClass.class); // Load class with method having Object[] parameter
final Type<Object[]> objectArrayType = Types.Object.makeArrayType(); // Prepare Object[] type for comparison
final MethodInfo method = c2.getMethod("getFirstOrNull", objectArrayType); // Load specific method having Object[] parameter
final Type<?> methodParameterType = method.getParameters().getParameterTypes().get(0); // Retrieve method parameter type (expected to be Object[])
assertEquals(objectArrayType, methodParameterType); // The method parameter type should match the expected Object[], but will be T[]
}
private static class MyClass {
public static <T> Iterable<T> enumerate(final T[] items) {
return Arrays.asList(items);
}
}
private static class MyOtherClass {
public static Object getFirstOrNull(final Object... values) {
return values.length > 0 ? values[0] : null;
}
}
}Above code fails, unsless c2 method is loaded before c1 methods.
The cause is that
| this.descriptor = type.getInternalName(); |
calls the
getInternalName of an ArrayType, which returns| return "[" + _elementType.getErasedSignature(); |
which is
[Ljava/lang/Object; for both Object[] and T[].
If the T[] is loaded first, it will then populate the cache _definitionMap, as per
| _definitionMap.put(descriptor, type); |
storing
T[] as the equivalent type of [Ljava/lang/Object;. Once updated, it will not change anymore.
Due to this, the getFirstOrNull method above will incorrectly return that the array type used as parameter is T[], instead of Object[]
If I did not misunderstood the purpose of the TypeCache._map, it wants to store the most recent instance of a type, matching the descriptor and generic type arguments.
If so, in my opinion the descriptor could be changed like this:
this.descriptor =
type.isArray() && type.getElementType().isGenericParameter() ? "[L" + type.getElementType().getInternalName() + ";" : type.getInternalName();But I am not really sure this is the expected behavior... could you explain a bit the _map purpose, expecially regarding generic types (both bounded and unbounded)?