diff --git a/moshi/src/main/java/com/squareup/moshi/ClassJsonAdapter.java b/moshi/src/main/java/com/squareup/moshi/ClassJsonAdapter.java index a149116..9d33553 100644 --- a/moshi/src/main/java/com/squareup/moshi/ClassJsonAdapter.java +++ b/moshi/src/main/java/com/squareup/moshi/ClassJsonAdapter.java @@ -23,6 +23,8 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Modifier; import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; +import java.util.Collection; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.TreeMap; @@ -58,6 +60,11 @@ final class ClassJsonAdapter extends JsonAdapter { if (rawType.isInterface() || rawType.isEnum()) return null; if (!annotations.isEmpty()) return null; if (Util.isPlatformType(rawType)) { + throwIfIsCollectionClass(type, List.class); + throwIfIsCollectionClass(type, Set.class); + throwIfIsCollectionClass(type, Map.class); + throwIfIsCollectionClass(type, Collection.class); + String messagePrefix = "Platform " + rawType; if (type instanceof ParameterizedType) { messagePrefix += " in " + type; @@ -94,6 +101,21 @@ final class ClassJsonAdapter extends JsonAdapter { return new ClassJsonAdapter<>(classFactory, fields).nullSafe(); } + /** + * Throw clear error messages for the common beginner mistake of using the concrete + * collection classes instead of the collection interfaces, eg: ArrayList instead of List. + */ + private void throwIfIsCollectionClass(Type type, Class collectionInterface) { + Class rawClass = Types.getRawType(type); + if (collectionInterface.isAssignableFrom(rawClass)) { + throw new IllegalArgumentException( + "No JsonAdapter for " + type + ", you should probably use " + + collectionInterface.getSimpleName() + " instead of " + rawClass.getSimpleName() + + " (Moshi only supports the collection interfaces by default)" + + " or else register a custom JsonAdapter."); + } + } + /** Creates a field binding for each of declared field of {@code type}. */ private void createFieldBindings( Moshi moshi, Type type, Map> fieldBindings) { diff --git a/moshi/src/test/java/com/squareup/moshi/MoshiTest.java b/moshi/src/test/java/com/squareup/moshi/MoshiTest.java index 1d51689..e6f2013 100644 --- a/moshi/src/test/java/com/squareup/moshi/MoshiTest.java +++ b/moshi/src/test/java/com/squareup/moshi/MoshiTest.java @@ -28,12 +28,14 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; +import java.util.UUID; import javax.annotation.Nullable; import javax.crypto.KeyGenerator; import okio.Buffer; @@ -45,7 +47,7 @@ import static java.lang.annotation.RetentionPolicy.RUNTIME; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.fail; -@SuppressWarnings("CheckReturnValue") +@SuppressWarnings({"CheckReturnValue", "ResultOfMethodCallIgnored"}) public final class MoshiTest { @Test public void booleanAdapter() throws Exception { Moshi moshi = new Moshi.Builder().build(); @@ -954,8 +956,48 @@ public final class MoshiTest { } } + @Test public void collectionClassesHaveClearErrorMessage() { + Moshi moshi = new Moshi.Builder().build(); + try { + moshi.adapter(Types.newParameterizedType(ArrayList.class, String.class)); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e).hasMessage("No JsonAdapter for " + + "java.util.ArrayList, " + + "you should probably use List instead of ArrayList " + + "(Moshi only supports the collection interfaces by default) " + + "or else register a custom JsonAdapter."); + } + + try { + moshi.adapter(Types.newParameterizedType(HashMap.class, String.class, String.class)); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e).hasMessage("No JsonAdapter for " + + "java.util.HashMap, " + + "you should probably use Map instead of HashMap " + + "(Moshi only supports the collection interfaces by default) " + + "or else register a custom JsonAdapter."); + } + } + + @Test public void noCollectionErrorIfAdapterExplicitlyProvided() { + Moshi moshi = new Moshi.Builder() + .add(new JsonAdapter.Factory() { + @Override public JsonAdapter create(Type type, Set annotations, + Moshi moshi) { + return new MapJsonAdapter(moshi, String.class, String.class); + } + }) + .build(); + + JsonAdapter> adapter = moshi.adapter( + Types.newParameterizedType(HashMap.class, String.class, String.class)); + assertThat(adapter).isInstanceOf(MapJsonAdapter.class); + } + static final class HasPlatformType { - ArrayList strings; + UUID uuid; static final class Wrapper { HasPlatformType hasPlatformType; @@ -973,14 +1015,14 @@ public final class MoshiTest { fail(); } catch (IllegalArgumentException e) { assertThat(e).hasMessage( - "Platform class java.util.ArrayList in java.util.ArrayList requires explicit " + "Platform class java.util.UUID requires explicit " + "JsonAdapter to be registered" - + "\nfor java.util.ArrayList strings" + + "\nfor class java.util.UUID uuid" + "\nfor class com.squareup.moshi.MoshiTest$HasPlatformType" + "\nfor java.util.Map"); assertThat(e).hasCauseExactlyInstanceOf(IllegalArgumentException.class); - assertThat(e.getCause()).hasMessage("Platform class java.util.ArrayList in java.util.ArrayList " + assertThat(e.getCause()).hasMessage("Platform class java.util.UUID " + "requires explicit JsonAdapter to be registered"); } } @@ -992,13 +1034,13 @@ public final class MoshiTest { fail(); } catch (IllegalArgumentException e) { assertThat(e).hasMessage( - "Platform class java.util.ArrayList in java.util.ArrayList requires explicit " + "Platform class java.util.UUID requires explicit " + "JsonAdapter to be registered" - + "\nfor java.util.ArrayList strings" + + "\nfor class java.util.UUID uuid" + "\nfor class com.squareup.moshi.MoshiTest$HasPlatformType hasPlatformType" + "\nfor class com.squareup.moshi.MoshiTest$HasPlatformType$Wrapper"); assertThat(e).hasCauseExactlyInstanceOf(IllegalArgumentException.class); - assertThat(e.getCause()).hasMessage("Platform class java.util.ArrayList in java.util.ArrayList " + assertThat(e.getCause()).hasMessage("Platform class java.util.UUID " + "requires explicit JsonAdapter to be registered"); } } @@ -1010,14 +1052,14 @@ public final class MoshiTest { fail(); } catch (IllegalArgumentException e) { assertThat(e).hasMessage( - "Platform class java.util.ArrayList in java.util.ArrayList requires explicit " + "Platform class java.util.UUID requires explicit " + "JsonAdapter to be registered" - + "\nfor java.util.ArrayList strings" + + "\nfor class java.util.UUID uuid" + "\nfor class com.squareup.moshi.MoshiTest$HasPlatformType" + "\nfor java.util.List platformTypes" + "\nfor class com.squareup.moshi.MoshiTest$HasPlatformType$ListWrapper"); assertThat(e).hasCauseExactlyInstanceOf(IllegalArgumentException.class); - assertThat(e.getCause()).hasMessage("Platform class java.util.ArrayList in java.util.ArrayList " + assertThat(e.getCause()).hasMessage("Platform class java.util.UUID " + "requires explicit JsonAdapter to be registered"); } }