Improved type handling

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

@@ -37,7 +37,7 @@
  * @author Jesse Wilson
  */
 public final class $Gson$Types {
-  static final Type[] EMPTY_TYPE_ARRAY = new Type[] {};
+  private static final Type[] EMPTY_TYPE_ARRAY = new Type[] {};
 
   private $Gson$Types() {
     throw new UnsupportedOperationException();
@@ -149,7 +149,10 @@ public static Class<?> getRawType(Type type) {
       return Object.class;
 
     } else if (type instanceof WildcardType) {
-      return getRawType(((WildcardType) type).getUpperBounds()[0]);
+      Type[] bounds = ((WildcardType) type).getUpperBounds();
+      // Currently the JLS only permits one bound for wildcards so using first bound is safe
+      assert bounds.length == 1;
+      return getRawType(bounds[0]);
 
     } else {
       String className = type == null ? "null" : type.getClass().getName();
@@ -158,7 +161,7 @@ public static Class<?> getRawType(Type type) {
     }
   }
 
-  static boolean equal(Object a, Object b) {
+  private static boolean equal(Object a, Object b) {
     return a == b || (a != null && a.equals(b));
   }
 
@@ -220,7 +223,7 @@ public static boolean equals(Type a, Type b) {
     }
   }
 
-  static int hashCodeOrZero(Object o) {
+  private static int hashCodeOrZero(Object o) {
     return o != null ? o.hashCode() : 0;
   }
 
@@ -233,19 +236,19 @@ public static String typeToString(Type type) {
    * IntegerSet}, the result for when supertype is {@code Set.class} is {@code Set<Integer>} and the
    * result when the supertype is {@code Collection.class} is {@code Collection<Integer>}.
    */
-  static Type getGenericSupertype(Type context, Class<?> rawType, Class<?> toResolve) {
-    if (toResolve == rawType) {
+  private static Type getGenericSupertype(Type context, Class<?> rawType, Class<?> supertype) {
+    if (supertype == rawType) {
       return context;
     }
 
     // we skip searching through interfaces if unknown is an interface
-    if (toResolve.isInterface()) {
+    if (supertype.isInterface()) {
       Class<?>[] interfaces = rawType.getInterfaces();
       for (int i = 0, length = interfaces.length; i < length; i++) {
-        if (interfaces[i] == toResolve) {
+        if (interfaces[i] == supertype) {
           return rawType.getGenericInterfaces()[i];
-        } else if (toResolve.isAssignableFrom(interfaces[i])) {
-          return getGenericSupertype(rawType.getGenericInterfaces()[i], interfaces[i], toResolve);
+        } else if (supertype.isAssignableFrom(interfaces[i])) {
+          return getGenericSupertype(rawType.getGenericInterfaces()[i], interfaces[i], supertype);
         }
       }
     }
@@ -254,17 +257,17 @@ static Type getGenericSupertype(Type context, Class<?> rawType, Class<?> toResol
     if (!rawType.isInterface()) {
       while (rawType != Object.class) {
         Class<?> rawSupertype = rawType.getSuperclass();
-        if (rawSupertype == toResolve) {
+        if (rawSupertype == supertype) {
           return rawType.getGenericSuperclass();
-        } else if (toResolve.isAssignableFrom(rawSupertype)) {
-          return getGenericSupertype(rawType.getGenericSuperclass(), rawSupertype, toResolve);
+        } else if (supertype.isAssignableFrom(rawSupertype)) {
+          return getGenericSupertype(rawType.getGenericSuperclass(), rawSupertype, supertype);
         }
         rawType = rawSupertype;
       }
     }
 
     // we can't resolve this further
-    return toResolve;
+    return supertype;
   }
 
   /**
@@ -274,10 +277,13 @@ static Type getGenericSupertype(Type context, Class<?> rawType, Class<?> toResol
    *
    * @param supertype a superclass of, or interface implemented by, this.
    */
-  static Type getSupertype(Type context, Class<?> contextRawType, Class<?> supertype) {
+  private static Type getSupertype(Type context, Class<?> contextRawType, Class<?> supertype) {
     if (context instanceof WildcardType) {
       // wildcards are useless for resolving supertypes. As the upper bound has the same raw type, use it instead
-      context = ((WildcardType)context).getUpperBounds()[0];
+      Type[] bounds = ((WildcardType)context).getUpperBounds();
+      // Currently the JLS only permits one bound for wildcards so using first bound is safe
+      assert bounds.length == 1;
+      context = bounds[0];
     }
     checkArgument(supertype.isAssignableFrom(contextRawType));
     return resolve(context, contextRawType,
@@ -301,9 +307,6 @@ public static Type getArrayComponentType(Type array) {
   public static Type getCollectionElementType(Type context, Class<?> contextRawType) {
     Type collectionType = getSupertype(context, contextRawType, Collection.class);
 
-    if (collectionType instanceof WildcardType) {
-      collectionType = ((WildcardType)collectionType).getUpperBounds()[0];
-    }
     if (collectionType instanceof ParameterizedType) {
       return ((ParameterizedType) collectionType).getActualTypeArguments()[0];
     }
@@ -334,11 +337,11 @@ public static Type[] getMapKeyAndValueTypes(Type context, Class<?> contextRawTyp
   }
 
   public static Type resolve(Type context, Class<?> contextRawType, Type toResolve) {
-    return resolve(context, contextRawType, toResolve, new HashSet<TypeVariable>());
+    return resolve(context, contextRawType, toResolve, new HashSet<TypeVariable<?>>());
   }
 
   private static Type resolve(Type context, Class<?> contextRawType, Type toResolve,
-                              Collection<TypeVariable> visitedTypeVariables) {
+                              Collection<TypeVariable<?>> visitedTypeVariables) {
     // this implementation is made a little more complicated in an attempt to avoid object-creation
     while (true) {
       if (toResolve instanceof TypeVariable) {
@@ -416,7 +419,7 @@ private static Type resolve(Type context, Class<?> contextRawType, Type toResolv
     }
   }
 
-  static Type resolveTypeVariable(Type context, Class<?> contextRawType, TypeVariable<?> unknown) {
+  private static Type resolveTypeVariable(Type context, Class<?> contextRawType, TypeVariable<?> unknown) {
     Class<?> declaredByRaw = declaringClassOf(unknown);
 
     // we can't reduce this further
@@ -453,7 +456,7 @@ private static Class<?> declaringClassOf(TypeVariable<?> typeVariable) {
         : null;
   }
 
-  static void checkNotPrimitive(Type type) {
+  private static void checkNotPrimitive(Type type) {
     checkArgument(!(type instanceof Class<?>) || !((Class<?>) type).isPrimitive());
   }
 
@@ -550,8 +553,9 @@ public Type getGenericComponentType() {
 
   /**
    * The WildcardType interface supports multiple upper bounds and multiple
-   * lower bounds. We only support what the Java 6 language needs - at most one
-   * bound. If a lower bound is set, the upper bound must be Object.class.
+   * lower bounds. However, the Java Language Specification only permits at most
+   * one bounds so we are limiting it to that.
+   * If a lower bound is set, the upper bound must be Object.class.
    */
   private static final class WildcardTypeImpl implements WildcardType, Serializable {
     private final Type upperBound;

gson/src/main/java/com/google/gson/internal/bind/MapTypeAdapterFactory.java

@@ -120,8 +120,7 @@ public MapTypeAdapterFactory(ConstructorConstructor constructorConstructor,
       return null;
     }
 
-    Class<?> rawTypeOfSrc = $Gson$Types.getRawType(type);
-    Type[] keyAndValueTypes = $Gson$Types.getMapKeyAndValueTypes(type, rawTypeOfSrc);
+    Type[] keyAndValueTypes = $Gson$Types.getMapKeyAndValueTypes(type, rawType);
     TypeAdapter<?> keyAdapter = getKeyAdapter(gson, keyAndValueTypes[0]);
     TypeAdapter<?> valueAdapter = gson.getAdapter(TypeToken.get(keyAndValueTypes[1]));
     ObjectConstructor<T> constructor = constructorConstructor.get(typeToken);

gson/src/main/java/com/google/gson/reflect/TypeToken.java

@@ -22,6 +22,7 @@
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.lang.reflect.TypeVariable;
+import java.lang.reflect.WildcardType;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -37,17 +38,14 @@
  * <p>
  * {@code TypeToken<List<String>> list = new TypeToken<List<String>>() {};}
  *
- * <p>This syntax cannot be used to create type literals that have wildcard
- * parameters, such as {@code Class<?>} or {@code List<? extends CharSequence>}.
- *
  * @author Bob Lee
  * @author Sven Mawson
  * @author Jesse Wilson
  */
 public class TypeToken<T> {
-  final Class<? super T> rawType;
-  final Type type;
-  final int hashCode;
+  private final Class<? super T> rawType;
+  private final Type type;
+  private final int hashCode;
 
   /**
    * Constructs a new type literal. Derives represented class from type
@@ -56,10 +54,17 @@ public class TypeToken<T> {
    * <p>Clients create an empty anonymous subclass. Doing so embeds the type
    * parameter in the anonymous class's type hierarchy so we can reconstitute it
    * at runtime despite erasure.
+   *
+   * <p>Because {@code TypeToken} is mainly intended for usage with Gson
+   * (and not other libraries) using a type variable as part of the type
+   * argument for {@code TypeToken} is not allowed. Due to type erasure the
+   * runtime type of a type variable is not available to Gson and therefore
+   * it cannot provide the functionality the user might expect, which would
+   * give a false sense of type-safety.
    */
   @SuppressWarnings("unchecked")
   protected TypeToken() {
-    this.type = getSuperclassTypeParameter(getClass());
+    this.type = getTypeTokenTypeArgument();
     this.rawType = (Class<? super T>) $Gson$Types.getRawType(type);
     this.hashCode = type.hashCode();
   }
@@ -68,23 +73,50 @@ protected TypeToken() {
    * Unsafe. Constructs a type literal manually.
    */
   @SuppressWarnings("unchecked")
-  TypeToken(Type type) {
+  private TypeToken(Type type) {
     this.type = $Gson$Types.canonicalize($Gson$Preconditions.checkNotNull(type));
     this.rawType = (Class<? super T>) $Gson$Types.getRawType(this.type);
     this.hashCode = this.type.hashCode();
   }
 
   /**
-   * Returns the type from super class's type parameter in {@link $Gson$Types#canonicalize
+   * Verifies that {@code this} is an instance of a direct subclass of TypeToken and
+   * returns the type argument for {@code T} in {@link $Gson$Types#canonicalize
    * canonical form}.
    */
-  static Type getSuperclassTypeParameter(Class<?> subclass) {
-    Type superclass = subclass.getGenericSuperclass();
-    if (superclass instanceof Class) {
-      throw new RuntimeException("Missing type parameter.");
+  private Type getTypeTokenTypeArgument() {
+    Type superclass = getClass().getGenericSuperclass();
+    if (superclass instanceof ParameterizedType) {
+      ParameterizedType parameterized = (ParameterizedType) superclass;
+      if (parameterized.getRawType() == TypeToken.class) {
+        Type typeArgument = $Gson$Types.canonicalize(parameterized.getActualTypeArguments()[0]);
+        verifyNoTypeVariable(typeArgument);
+        return typeArgument;
+      }
+    }
+
+    // User created subclass of subclass of TypeToken
+    throw new IllegalStateException("Must only create direct subclasses of TypeToken");
+  }
+
+  private static void verifyNoTypeVariable(Type type) {
+    if (type instanceof TypeVariable) {
+      throw new IllegalArgumentException("TypeToken type argument must not contain a type variable");
+    } else if (type instanceof GenericArrayType) {
+      verifyNoTypeVariable(((GenericArrayType) type).getGenericComponentType());
+    } else if (type instanceof ParameterizedType) {
+      for (Type typeArgument : ((ParameterizedType) type).getActualTypeArguments()) {
+        verifyNoTypeVariable(typeArgument);
+      }
+    } else if (type instanceof WildcardType) {
+      WildcardType wildcardType = (WildcardType) type;
+      for (Type bound : wildcardType.getLowerBounds()) {
+        verifyNoTypeVariable(bound);
+      }
+      for (Type bound : wildcardType.getUpperBounds()) {
+        verifyNoTypeVariable(bound);
+      }
     }
-    ParameterizedType parameterized = (ParameterizedType) superclass;
-    return $Gson$Types.canonicalize(parameterized.getActualTypeArguments()[0]);
   }
 
   /**

gson/src/test/java/com/google/gson/reflect/TypeTokenTest.java

@@ -27,23 +27,24 @@
 /**
  * @author Jesse Wilson
  */
-@SuppressWarnings({"deprecation"})
 public final class TypeTokenTest extends TestCase {
-
+  // These fields are accessed using reflection by the tests below
   List<Integer> listOfInteger = null;
   List<Number> listOfNumber = null;
   List<String> listOfString = null;
   List<?> listOfUnknown = null;
   List<Set<String>> listOfSetOfString = null;
   List<Set<?>> listOfSetOfUnknown = null;
 
+  @SuppressWarnings({"deprecation"})
   public void testIsAssignableFromRawTypes() {
     assertTrue(TypeToken.get(Object.class).isAssignableFrom(String.class));
     assertFalse(TypeToken.get(String.class).isAssignableFrom(Object.class));
     assertTrue(TypeToken.get(RandomAccess.class).isAssignableFrom(ArrayList.class));
     assertFalse(TypeToken.get(ArrayList.class).isAssignableFrom(RandomAccess.class));
   }
 
+  @SuppressWarnings({"deprecation"})
   public void testIsAssignableFromWithTypeParameters() throws Exception {
     Type a = getClass().getDeclaredField("listOfInteger").getGenericType();
     Type b = getClass().getDeclaredField("listOfNumber").getGenericType();
@@ -56,6 +57,7 @@ public void testIsAssignableFromWithTypeParameters() throws Exception {
     assertFalse(TypeToken.get(b).isAssignableFrom(a));
   }
 
+  @SuppressWarnings({"deprecation"})
   public void testIsAssignableFromWithBasicWildcards() throws Exception {
     Type a = getClass().getDeclaredField("listOfString").getGenericType();
     Type b = getClass().getDeclaredField("listOfUnknown").getGenericType();
@@ -69,6 +71,7 @@ public void testIsAssignableFromWithBasicWildcards() throws Exception {
     // assertTrue(TypeToken.get(b).isAssignableFrom(a));
   }
 
+  @SuppressWarnings({"deprecation"})
   public void testIsAssignableFromWithNestedWildcards() throws Exception {
     Type a = getClass().getDeclaredField("listOfSetOfString").getGenericType();
     Type b = getClass().getDeclaredField("listOfSetOfUnknown").getGenericType();
@@ -102,4 +105,73 @@ public void testParameterizedFactory() {
     Type listOfListOfString = TypeToken.getParameterized(List.class, listOfString).getType();
     assertEquals(expectedListOfListOfListOfString, TypeToken.getParameterized(List.class, listOfListOfString));
   }
+
+  /**
+   * User must only create direct subclasses of TypeToken, but not subclasses
+   * of subclasses (...) of TypeToken.
+   */
+  public void testTypeTokenSubSubClass() {
+    class SubTypeToken<T> extends TypeToken<String> {}
+    class SubSubTypeToken1<T> extends SubTypeToken<T> {}
+    class SubSubTypeToken2 extends SubTypeToken<Integer> {}
+
+    try {
+      new SubTypeToken<Integer>() {};
+      fail();
+    } catch (IllegalStateException expected) {
+    }
+
+    try {
+      new SubSubTypeToken1<Integer>();
+      fail();
+    } catch (IllegalStateException expected) {
+    }
+
+    try {
+      new SubSubTypeToken2();
+      fail();
+    } catch (IllegalStateException expected) {
+    }
+  }
+
+  /**
+   * TypeToken type argument must not contain a type variable because, due to
+   * type erasure, at runtime only the bound of the type variable is available
+   * which is likely not what the user wanted.
+   *
+   * <p>Note that type variables are allowed for the methods calling {@code TypeToken(Type)}
+   * because there the return type is {@code TypeToken<?>} which does not give
+   * a false sense of type-safety.
+   */
+  public <T> void testTypeTokenTypeVariable() {
+    try {
+      new TypeToken<T>() {};
+      fail();
+    } catch (IllegalArgumentException expected) {
+    }
+
+    try {
+      new TypeToken<List<T>>() {};
+      fail();
+    } catch (IllegalArgumentException expected) {
+    }
+
+    try {
+      new TypeToken<List<? extends T>>() {};
+      fail();
+    } catch (IllegalArgumentException expected) {
+    }
+
+    try {
+      new TypeToken<List<? super T>>() {};
+      fail();
+    } catch (IllegalArgumentException expected) {
+    }
+
+    try {
+      new TypeToken<List<T[]>>() {};
+      fail();
+    } catch (IllegalArgumentException expected) {
+    }
+  }
 }