Merge pull request #201 from square/jwilson.1015.fix_enclosed_types

Fix enclosed types with adapter methods.

moshi/src/main/java/com/squareup/moshi/Types.java

@@ -18,7 +18,6 @@ package com.squareup.moshi;
 import java.lang.reflect.Array;
 import java.lang.reflect.GenericArrayType;
 import java.lang.reflect.GenericDeclaration;
-import java.lang.reflect.Modifier;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.lang.reflect.TypeVariable;
@@ -37,12 +36,22 @@ public final class Types {
   }
 
   /**
-   * Returns a new parameterized type, applying {@code typeArguments} to {@code rawType}.
+   * Returns a new parameterized type, applying {@code typeArguments} to {@code rawType}. Use this
+   * method if {@code rawType} is not enclosed in another type.
    */
   public static ParameterizedType newParameterizedType(Type rawType, Type... typeArguments) {
     return new ParameterizedTypeImpl(null, rawType, typeArguments);
   }
 
+  /**
+   * Returns a new parameterized type, applying {@code typeArguments} to {@code rawType}. Use this
+   * method if {@code rawType} is enclosed in {@code ownerType}.
+   */
+  public static ParameterizedType newParameterizedTypeWithOwner(
+      Type ownerType, Type rawType, Type... typeArguments) {
+    return new ParameterizedTypeImpl(ownerType, rawType, typeArguments);
+  }
+
   /** Returns an array type whose elements are all instances of {@code componentType}. */
   public static GenericArrayType arrayOf(Type componentType) {
     return new GenericArrayTypeImpl(componentType);
@@ -405,12 +414,11 @@ public final class Types {
     final Type[] typeArguments;
 
     ParameterizedTypeImpl(Type ownerType, Type rawType, Type... typeArguments) {
-      // 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;
-        if (ownerType == null && !isStaticOrTopLevelClass) throw new IllegalArgumentException();
+      // Require an owner type if the raw type needs it.
+      if (rawType instanceof Class<?>
+          && (ownerType == null) != (((Class<?>) rawType).getEnclosingClass() == null)) {
+        throw new IllegalArgumentException(
+            "unexpected owner type for " + rawType + ": " + ownerType);
       }
 
       this.ownerType = ownerType == null ? null : canonicalize(ownerType);

moshi/src/test/java/com/squareup/moshi/AdapterMethodsTest.java

@@ -22,6 +22,7 @@ import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -547,6 +548,44 @@ public final class AdapterMethodsTest {
     }
   }
 
+  @Test public void adaptedTypeIsEnclosedParameterizedType() throws Exception {
+    Moshi moshi = new Moshi.Builder()
+        .add(new EnclosedParameterizedTypeJsonAdapter())
+        .build();
+    JsonAdapter<Box<Point>> boxAdapter = moshi.adapter(Types.newParameterizedTypeWithOwner(
+        AdapterMethodsTest.class, Box.class, Point.class));
+    Box<Point> box = new Box<>(new Point(5, 8));
+    String json = "[{\"x\":5,\"y\":8}]";
+    assertThat(boxAdapter.toJson(box)).isEqualTo(json);
+    assertThat(boxAdapter.fromJson(json)).isEqualTo(box);
+  }
+
+  static class EnclosedParameterizedTypeJsonAdapter {
+    @FromJson Box<Point> boxFromJson(List<Point> points) {
+      return new Box<>(points.get(0));
+    }
+
+    @ToJson List<Point> boxToJson(Box<Point> box) throws Exception {
+      return Collections.singletonList(box.data);
+    }
+  }
+
+  static class Box<T> {
+    final T data;
+
+    public Box(T data) {
+      this.data = data;
+    }
+
+    @Override public boolean equals(Object o) {
+      return o instanceof Box && ((Box) o).data.equals(data);
+    }
+
+    @Override public int hashCode() {
+      return data.hashCode();
+    }
+  }
+
   static class Point {
     final int x;
     final int y;

moshi/src/test/java/com/squareup/moshi/TypesTest.java

@@ -34,27 +34,32 @@ public final class TypesTest {
     assertThat(getFirstTypeArgument(type)).isEqualTo(A.class);
 
     // A<B>. A is a static inner class.
-    type = Types.newParameterizedType(A.class, B.class);
+    type = Types.newParameterizedTypeWithOwner(TypesTest.class, A.class, B.class);
     assertThat(getFirstTypeArgument(type)).isEqualTo(B.class);
+  }
 
-    final class D {
-    }
+  @Test public void parameterizedTypeWithRequiredOwnerMissing() throws Exception {
     try {
-      // D<A> is not allowed since D is not a static inner class.
-      Types.newParameterizedType(D.class, A.class);
+      Types.newParameterizedType(A.class, B.class);
       fail();
     } catch (IllegalArgumentException expected) {
+      assertThat(expected).hasMessage("unexpected owner type for " + A.class + ": null");
     }
+  }
 
-    // A<D> is allowed.
-    type = Types.newParameterizedType(A.class, D.class);
-    assertThat(getFirstTypeArgument(type)).isEqualTo(D.class);
+  @Test public void parameterizedTypeWithUnnecessaryOwnerProvided() throws Exception {
+    try {
+      Types.newParameterizedTypeWithOwner(A.class, List.class, B.class);
+      fail();
+    } catch (IllegalArgumentException expected) {
+      assertThat(expected).hasMessage("unexpected owner type for " + List.class + ": " + A.class);
+    }
   }
 
   @Test public void getFirstTypeArgument() throws Exception {
     assertThat(getFirstTypeArgument(A.class)).isNull();
 
-    Type type = Types.newParameterizedType(A.class, B.class, C.class);
+    Type type = Types.newParameterizedTypeWithOwner(TypesTest.class, A.class, B.class, C.class);
     assertThat(getFirstTypeArgument(type)).isEqualTo(B.class);
   }