Johannes
/
gson-comments
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Improve creation of `ParameterizedType` (#2375) 

- Reject non-generic raw types for TypeToken.getParameterized
- Fix ParameterizedTypeImpl erroneously requiring owner type for types
  without owner
This commit is contained in:
Marcono1234 2023-06-24 00:24:23 +02:00 committed by GitHub
parent 9cf0f8d302
commit a589ef2008
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 139 additions and 126 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
gson/src/main/java/com/google/gson/internal/$Gson$Types.java
View File

@ -33,8 +33,8 @@ import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Properties;
import java.util.Objects;
import java.util.Properties;
/**
* Static methods for working with types.
@ -138,9 +138,8 @@ public final class $Gson$Types {
} else if (type instanceof ParameterizedType) {
ParameterizedType parameterizedType = (ParameterizedType) type;
// I'm not exactly sure why getRawType() returns Type instead of Class.
// Neal isn't either but suspects some pathological case related
// to nested classes exists.
// getRawType() returns Type instead of Class; that seems to be an API mistake,
// see https://bugs.openjdk.org/browse/JDK-8250659
Type rawType = parameterizedType.getRawType();
checkArgument(rawType instanceof Class);
return (Class<?>) rawType;
@ -481,19 +480,33 @@ public final class $Gson$Types {
checkArgument(!(type instanceof Class<?>) || !((Class<?>) type).isPrimitive());
}
/**
* Whether an {@linkplain ParameterizedType#getOwnerType() owner type} must be specified when
* constructing a {@link ParameterizedType} for {@code rawType}.
*
* <p>Note that this method might not require an owner type for all cases where Java reflection
* would create parameterized types with owner type.
*/
public static boolean requiresOwnerType(Type rawType) {
if (rawType instanceof Class<?>) {
Class<?> rawTypeAsClass = (Class<?>) rawType;
return !Modifier.isStatic(rawTypeAsClass.getModifiers())
&& rawTypeAsClass.getDeclaringClass() != null;
}
return false;
}
private static final class ParameterizedTypeImpl implements ParameterizedType, Serializable {
private final Type ownerType;
private final Type rawType;
private final Type[] typeArguments;
public ParameterizedTypeImpl(Type ownerType, Type rawType, Type... typeArguments) {
// TODO: Should this enforce that rawType is a Class? See JDK implementation of
// the ParameterizedType interface and https://bugs.openjdk.org/browse/JDK-8250659
requireNonNull(rawType);
// require an owner type if the raw type needs it
if (rawType instanceof Class<?>) {
Class<?> rawTypeAsClass = (Class<?>) rawType;
boolean isStaticOrTopLevelClass = Modifier.isStatic(rawTypeAsClass.getModifiers())
|| rawTypeAsClass.getEnclosingClass() == null;
checkArgument(ownerType != null || isStaticOrTopLevelClass);
if (ownerType == null && requiresOwnerType(rawType)) {
throw new IllegalArgumentException("Must specify owner type for " + rawType);
}
this.ownerType = ownerType == null ? null : canonicalize(ownerType);

22
gson/src/main/java/com/google/gson/reflect/TypeToken.java
View File

@ -338,8 +338,8 @@ public class TypeToken<T> {
* and care must be taken to pass in the correct number of type arguments.
*
* @throws IllegalArgumentException
* If {@code rawType} is not of type {@code Class}, or if the type arguments are invalid for
* the raw type
* If {@code rawType} is not of type {@code Class}, if it is not a generic type, or if the
* type arguments are invalid for the raw type
*/
public static TypeToken<?> getParameterized(Type rawType, Type... typeArguments) {
Objects.requireNonNull(rawType);
@ -354,6 +354,18 @@ public class TypeToken<T> {
Class<?> rawClass = (Class<?>) rawType;
TypeVariable<?>[] typeVariables = rawClass.getTypeParameters();
// Note: Does not check if owner type of rawType is generic because this factory method
// does not support specifying owner type
if (typeVariables.length == 0) {
throw new IllegalArgumentException(rawClass.getName() + " is not a generic type");
}
// Check for this here to avoid misleading exception thrown by ParameterizedTypeImpl
if ($Gson$Types.requiresOwnerType(rawType)) {
throw new IllegalArgumentException("Raw type " + rawClass.getName() + " is not supported because"
+ " it requires specifying an owner type");
}
int expectedArgsCount = typeVariables.length;
int actualArgsCount = typeArguments.length;
if (actualArgsCount != expectedArgsCount) {
@ -362,7 +374,7 @@ public class TypeToken<T> {
}
for (int i = 0; i < expectedArgsCount; i++) {
Type typeArgument = typeArguments[i];
Type typeArgument = Objects.requireNonNull(typeArguments[i], "Type argument must not be null");
Class<?> rawTypeArgument = $Gson$Types.getRawType(typeArgument);
TypeVariable<?> typeVariable = typeVariables[i];
@ -370,8 +382,8 @@ public class TypeToken<T> {
Class<?> rawBound = $Gson$Types.getRawType(bound);
if (!rawBound.isAssignableFrom(rawTypeArgument)) {
throw new IllegalArgumentException("Type argument " + typeArgument + " does not satisfy bounds "
+ "for type variable " + typeVariable + " declared by " + rawType);
throw new IllegalArgumentException("Type argument " + typeArgument + " does not satisfy bounds"
+ " for type variable " + typeVariable + " declared by " + rawType);
}
}
}

32
gson/src/test/java/com/google/gson/internal/GsonTypesTest.java
View File

@ -17,7 +17,7 @@
package com.google.gson.internal;
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.fail;
import static org.junit.Assert.assertThrows;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
@ -29,20 +29,33 @@ public final class GsonTypesTest {
@Test
public void testNewParameterizedTypeWithoutOwner() throws Exception {
// List<A>. List is a top-level class
Type type = $Gson$Types.newParameterizedTypeWithOwner(null, List.class, A.class);
assertThat(getFirstTypeArgument(type)).isEqualTo(A.class);
ParameterizedType type = $Gson$Types.newParameterizedTypeWithOwner(null, List.class, A.class);
assertThat(type.getOwnerType()).isNull();
assertThat(type.getRawType()).isEqualTo(List.class);
assertThat(type.getActualTypeArguments()).asList().containsExactly(A.class);
// A<B>. A is a static inner class.
type = $Gson$Types.newParameterizedTypeWithOwner(null, A.class, B.class);
assertThat(getFirstTypeArgument(type)).isEqualTo(B.class);
IllegalArgumentException e = assertThrows(IllegalArgumentException.class,
// NonStaticInner<A> is not allowed without owner
() -> $Gson$Types.newParameterizedTypeWithOwner(null, NonStaticInner.class, A.class));
assertThat(e).hasMessageThat().isEqualTo("Must specify owner type for " + NonStaticInner.class);
type = $Gson$Types.newParameterizedTypeWithOwner(GsonTypesTest.class, NonStaticInner.class, A.class);
assertThat(type.getOwnerType()).isEqualTo(GsonTypesTest.class);
assertThat(type.getRawType()).isEqualTo(NonStaticInner.class);
assertThat(type.getActualTypeArguments()).asList().containsExactly(A.class);
final class D {
}
try {
// D<A> is not allowed since D is not a static inner class
$Gson$Types.newParameterizedTypeWithOwner(null, D.class, A.class);
fail();
} catch (IllegalArgumentException expected) {}
// D<A> is allowed since D has no owner type
type = $Gson$Types.newParameterizedTypeWithOwner(null, D.class, A.class);
assertThat(type.getOwnerType()).isNull();
assertThat(type.getRawType()).isEqualTo(D.class);
assertThat(type.getActualTypeArguments()).asList().containsExactly(A.class);
// A<D> is allowed.
type = $Gson$Types.newParameterizedTypeWithOwner(null, A.class, D.class);
@ -63,6 +76,9 @@ public final class GsonTypesTest {
}
private static final class C {
}
@SuppressWarnings({"ClassCanBeStatic", "UnusedTypeParameter"})
private final class NonStaticInner<T> {
}
/**
* Given a parameterized type A&lt;B,C&gt;, returns B. If the specified type is not

178
gson/src/test/java/com/google/gson/reflect/TypeTokenTest.java
View File

@ -17,9 +17,10 @@
package com.google.gson.reflect;
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.fail;
import static org.junit.Assert.assertThrows;
import java.lang.reflect.GenericArrayType;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.util.ArrayList;
import java.util.List;
@ -103,11 +104,13 @@ public final class TypeTokenTest {
Type listOfString = new TypeToken<List<String>>() {}.getType();
assertThat(TypeToken.getArray(listOfString)).isEqualTo(expectedListOfStringArray);
try {
TypeToken.getArray(null);
fail();
} catch (NullPointerException e) {
}
TypeToken<?> expectedIntArray = new TypeToken<int[]>() {};
assertThat(TypeToken.getArray(int.class)).isEqualTo(expectedIntArray);
assertThrows(NullPointerException.class, () -> TypeToken.getArray(null));
}
static class NestedGeneric<T> {
}
@Test
@ -131,84 +134,70 @@ public final class TypeTokenTest {
TypeToken<?> expectedSatisfyingTwoBounds = new TypeToken<GenericWithMultiBound<ClassSatisfyingBounds>>() {};
assertThat(TypeToken.getParameterized(GenericWithMultiBound.class, ClassSatisfyingBounds.class)).isEqualTo(expectedSatisfyingTwoBounds);
TypeToken<?> nestedTypeToken = TypeToken.getParameterized(NestedGeneric.class, Integer.class);
ParameterizedType nestedParameterizedType = (ParameterizedType) nestedTypeToken.getType();
// TODO: This seems to differ from how Java reflection behaves; when using TypeToken<NestedGeneric<Integer>>,
// then NestedGeneric<Integer> does have an owner type
assertThat(nestedParameterizedType.getOwnerType()).isNull();
assertThat(nestedParameterizedType.getRawType()).isEqualTo(NestedGeneric.class);
assertThat(nestedParameterizedType.getActualTypeArguments()).asList().containsExactly(Integer.class);
class LocalGenericClass<T> {}
TypeToken<?> expectedLocalType = new TypeToken<LocalGenericClass<Integer>>() {};
assertThat(TypeToken.getParameterized(LocalGenericClass.class, Integer.class)).isEqualTo(expectedLocalType);
}
@Test
public void testParameterizedFactory_Invalid() {
try {
TypeToken.getParameterized(null, new Type[0]);
fail();
} catch (NullPointerException e) {
}
assertThrows(NullPointerException.class, () -> TypeToken.getParameterized(null, new Type[0]));
assertThrows(NullPointerException.class, () -> TypeToken.getParameterized(List.class, new Type[] { null }));
GenericArrayType arrayType = (GenericArrayType) TypeToken.getArray(String.class).getType();
try {
TypeToken.getParameterized(arrayType, new Type[0]);
fail();
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().isEqualTo("rawType must be of type Class, but was java.lang.String[]");
IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> TypeToken.getParameterized(arrayType, new Type[0]));
assertThat(e).hasMessageThat().isEqualTo("rawType must be of type Class, but was java.lang.String[]");
e = assertThrows(IllegalArgumentException.class, () -> TypeToken.getParameterized(String.class, Number.class));
assertThat(e).hasMessageThat().isEqualTo("java.lang.String is not a generic type");
e = assertThrows(IllegalArgumentException.class, () -> TypeToken.getParameterized(List.class, new Type[0]));
assertThat(e).hasMessageThat().isEqualTo("java.util.List requires 1 type arguments, but got 0");
e = assertThrows(IllegalArgumentException.class, () -> TypeToken.getParameterized(List.class, String.class, String.class));
assertThat(e).hasMessageThat().isEqualTo("java.util.List requires 1 type arguments, but got 2");
// Primitive types must not be used as type argument
e = assertThrows(IllegalArgumentException.class, () -> TypeToken.getParameterized(List.class, int.class));
assertThat(e).hasMessageThat().isEqualTo("Type argument int does not satisfy bounds"
+ " for type variable E declared by " + List.class);
e = assertThrows(IllegalArgumentException.class, () -> TypeToken.getParameterized(GenericWithBound.class, String.class));
assertThat(e).hasMessageThat().isEqualTo("Type argument class java.lang.String does not satisfy bounds"
+ " for type variable T declared by " + GenericWithBound.class);
e = assertThrows(IllegalArgumentException.class, () -> TypeToken.getParameterized(GenericWithBound.class, Object.class));
assertThat(e).hasMessageThat().isEqualTo("Type argument class java.lang.Object does not satisfy bounds"
+ " for type variable T declared by " + GenericWithBound.class);
e = assertThrows(IllegalArgumentException.class, () -> TypeToken.getParameterized(GenericWithMultiBound.class, Number.class));
assertThat(e).hasMessageThat().isEqualTo("Type argument class java.lang.Number does not satisfy bounds"
+ " for type variable T declared by " + GenericWithMultiBound.class);
e = assertThrows(IllegalArgumentException.class, () -> TypeToken.getParameterized(GenericWithMultiBound.class, CharSequence.class));
assertThat(e).hasMessageThat().isEqualTo("Type argument interface java.lang.CharSequence does not satisfy bounds"
+ " for type variable T declared by " + GenericWithMultiBound.class);
e = assertThrows(IllegalArgumentException.class, () -> TypeToken.getParameterized(GenericWithMultiBound.class, Object.class));
assertThat(e).hasMessageThat().isEqualTo("Type argument class java.lang.Object does not satisfy bounds"
+ " for type variable T declared by " + GenericWithMultiBound.class);
class Outer {
class NonStaticInner<T> {}
}
try {
TypeToken.getParameterized(String.class, String.class);
fail();
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().isEqualTo("java.lang.String requires 0 type arguments, but got 1");
}
try {
TypeToken.getParameterized(List.class, new Type[0]);
fail();
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().isEqualTo("java.util.List requires 1 type arguments, but got 0");
}
try {
TypeToken.getParameterized(List.class, String.class, String.class);
fail();
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().isEqualTo("java.util.List requires 1 type arguments, but got 2");
}
try {
TypeToken.getParameterized(GenericWithBound.class, String.class);
fail();
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().isEqualTo("Type argument class java.lang.String does not satisfy bounds "
+ "for type variable T declared by " + GenericWithBound.class);
}
try {
TypeToken.getParameterized(GenericWithBound.class, Object.class);
fail();
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().isEqualTo("Type argument class java.lang.Object does not satisfy bounds "
+ "for type variable T declared by " + GenericWithBound.class);
}
try {
TypeToken.getParameterized(GenericWithMultiBound.class, Number.class);
fail();
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().isEqualTo("Type argument class java.lang.Number does not satisfy bounds "
+ "for type variable T declared by " + GenericWithMultiBound.class);
}
try {
TypeToken.getParameterized(GenericWithMultiBound.class, CharSequence.class);
fail();
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().isEqualTo("Type argument interface java.lang.CharSequence does not satisfy bounds "
+ "for type variable T declared by " + GenericWithMultiBound.class);
}
try {
TypeToken.getParameterized(GenericWithMultiBound.class, Object.class);
fail();
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().isEqualTo("Type argument class java.lang.Object does not satisfy bounds "
+ "for type variable T declared by " + GenericWithMultiBound.class);
}
e = assertThrows(IllegalArgumentException.class, () -> TypeToken.getParameterized(Outer.NonStaticInner.class, Object.class));
assertThat(e).hasMessageThat().isEqualTo("Raw type " + Outer.NonStaticInner.class.getName()
+ " is not supported because it requires specifying an owner type");
}
private static class CustomTypeToken extends TypeToken<String> {
@ -231,40 +220,23 @@ public final class TypeTokenTest {
class SubSubTypeToken1<T> extends SubTypeToken<T> {}
class SubSubTypeToken2 extends SubTypeToken<Integer> {}
try {
new SubTypeToken<Integer>() {};
fail();
} catch (IllegalStateException expected) {
assertThat(expected).hasMessageThat().isEqualTo("Must only create direct subclasses of TypeToken");
}
IllegalStateException e = assertThrows(IllegalStateException.class, () -> new SubTypeToken<Integer>() {});
assertThat(e).hasMessageThat().isEqualTo("Must only create direct subclasses of TypeToken");
try {
new SubSubTypeToken1<Integer>();
fail();
} catch (IllegalStateException expected) {
assertThat(expected).hasMessageThat().isEqualTo("Must only create direct subclasses of TypeToken");
}
e = assertThrows(IllegalStateException.class, () -> new SubSubTypeToken1<Integer>());
assertThat(e).hasMessageThat().isEqualTo("Must only create direct subclasses of TypeToken");
try {
new SubSubTypeToken2();
fail();
} catch (IllegalStateException expected) {
assertThat(expected).hasMessageThat().isEqualTo("Must only create direct subclasses of TypeToken");
}
e = assertThrows(IllegalStateException.class, () -> new SubSubTypeToken2());
assertThat(e).hasMessageThat().isEqualTo("Must only create direct subclasses of TypeToken");
}
@SuppressWarnings("rawtypes")
@Test
public void testTypeTokenRaw() {
try {
new TypeToken() {};
fail();
} catch (IllegalStateException expected) {
assertThat(expected).hasMessageThat().isEqualTo("TypeToken must be created with a type argument: new TypeToken<...>() {};"
+ " When using code shrinkers (ProGuard, R8, ...) make sure that generic signatures are preserved."
+ "\nSee https://github.com/google/gson/blob/main/Troubleshooting.md#type-token-raw"
);
}
IllegalStateException e = assertThrows(IllegalStateException.class, () -> new TypeToken() {});
assertThat(e).hasMessageThat().isEqualTo("TypeToken must be created with a type argument: new TypeToken<...>() {};"
+ " When using code shrinkers (ProGuard, R8, ...) make sure that generic signatures are preserved."
+ "\nSee https://github.com/google/gson/blob/main/Troubleshooting.md#type-token-raw");
}
}