diff --git a/moshi/src/main/java/com/squareup/moshi/ClassJsonAdapter.java b/moshi/src/main/java/com/squareup/moshi/ClassJsonAdapter.java index aee6d55..71d4534 100644 --- a/moshi/src/main/java/com/squareup/moshi/ClassJsonAdapter.java +++ b/moshi/src/main/java/com/squareup/moshi/ClassJsonAdapter.java @@ -95,14 +95,15 @@ final class ClassJsonAdapter extends JsonAdapter { // Look up a type adapter for this type. Type fieldType = resolve(type, rawType, field.getGenericType()); Set annotations = Util.jsonAnnotations(field); - JsonAdapter adapter = moshi.adapter(fieldType, annotations); + String fieldName = field.getName(); + JsonAdapter adapter = moshi.adapter(fieldType, annotations, fieldName); // Create the binding between field and JSON. field.setAccessible(true); // Store it using the field's name. If there was already a field with this name, fail! Json jsonAnnotation = field.getAnnotation(Json.class); - String name = jsonAnnotation != null ? jsonAnnotation.name() : field.getName(); + String name = jsonAnnotation != null ? jsonAnnotation.name() : fieldName; FieldBinding fieldBinding = new FieldBinding<>(name, field, adapter); FieldBinding replaced = fieldBindings.put(name, fieldBinding); if (replaced != null) { diff --git a/moshi/src/main/java/com/squareup/moshi/Moshi.java b/moshi/src/main/java/com/squareup/moshi/Moshi.java index 8d39c47..26db316 100644 --- a/moshi/src/main/java/com/squareup/moshi/Moshi.java +++ b/moshi/src/main/java/com/squareup/moshi/Moshi.java @@ -90,8 +90,18 @@ public final class Moshi { } @CheckReturnValue - @SuppressWarnings("unchecked") // Factories are required to return only matching JsonAdapters. public JsonAdapter adapter(Type type, Set annotations) { + return adapter(type, annotations, null); + } + + /** + * @param fieldName An optional field name associated with this type. The field name is used as a + * hint for better adapter lookup error messages for nested structures. + */ + @CheckReturnValue + @SuppressWarnings("unchecked") // Factories are required to return only matching JsonAdapters. + public JsonAdapter adapter(Type type, Set annotations, + @Nullable String fieldName) { if (type == null) { throw new NullPointerException("type == null"); } @@ -123,8 +133,10 @@ public final class Moshi { } // Prepare for re-entrant calls, then ask each factory to create a type adapter. - DeferredAdapter deferredAdapter = new DeferredAdapter<>(cacheKey); + DeferredAdapter deferredAdapter = new DeferredAdapter<>(type, fieldName, cacheKey); + deferredAdapters.add(deferredAdapter); + int lastIndex = deferredAdapters.size() - 1; try { for (int i = 0, size = factories.size(); i < size; i++) { JsonAdapter result = (JsonAdapter) factories.get(i).create(type, annotations, this); @@ -133,12 +145,19 @@ public final class Moshi { synchronized (adapterCache) { adapterCache.put(cacheKey, result); } + // Remove the type or field name only when we succeed in creating the adapter, + // so we have the full stack at the top level. + deferredAdapters.remove(lastIndex); return result; } } + } catch (IllegalArgumentException e) { + if (lastIndex == 0) { // Rewrite the exception at the top level. + e = errorWithFields(deferredAdapters, e); + } + throw e; } finally { - deferredAdapters.remove(deferredAdapters.size() - 1); - if (deferredAdapters.isEmpty()) { + if (lastIndex == 0) { reentrantCalls.remove(); } } @@ -181,6 +200,27 @@ public final class Moshi { return Arrays.asList(type, annotations); } + static IllegalArgumentException errorWithFields(List> typesAndFieldNames, + IllegalArgumentException e) { + int size = typesAndFieldNames.size(); + if (size == 1 && typesAndFieldNames.get(0).fieldName == null) { + return e; + } + StringBuilder errorMessageBuilder = new StringBuilder(e.getMessage()); + for (int i = size - 1; i >= 0; i--) { + DeferredAdapter deferredAdapter = typesAndFieldNames.get(i); + errorMessageBuilder + .append("\nfor ") + .append(deferredAdapter.type); + if (deferredAdapter.fieldName != null) { + errorMessageBuilder + .append(' ') + .append(deferredAdapter.fieldName); + } + } + return new IllegalArgumentException(errorMessageBuilder.toString(), e); + } + public static final class Builder { final List factories = new ArrayList<>(); @@ -250,11 +290,15 @@ public final class Moshi { *

Typically this is necessary in self-referential object models, such as an {@code Employee} * class that has a {@code List} field for an organization's management hierarchy. */ - private static class DeferredAdapter extends JsonAdapter { + private static final class DeferredAdapter extends JsonAdapter { + final Type type; + final @Nullable String fieldName; @Nullable Object cacheKey; private @Nullable JsonAdapter delegate; - DeferredAdapter(Object cacheKey) { + DeferredAdapter(Type type, @Nullable String fieldName, Object cacheKey) { + this.type = type; + this.fieldName = fieldName; this.cacheKey = cacheKey; } diff --git a/moshi/src/test/java/com/squareup/moshi/JsonQualifiersTest.java b/moshi/src/test/java/com/squareup/moshi/JsonQualifiersTest.java index 647df10..0c46e3a 100644 --- a/moshi/src/test/java/com/squareup/moshi/JsonQualifiersTest.java +++ b/moshi/src/test/java/com/squareup/moshi/JsonQualifiersTest.java @@ -332,7 +332,12 @@ public final class JsonQualifiersTest { moshi.adapter(StringAndFooString.class); fail(); } catch (IllegalArgumentException expected) { - assertThat(expected).hasMessage("No @FromJson adapter for class java.lang.String " + assertThat(expected).hasMessage("No @FromJson adapter for class java.lang.String annotated " + + "[@com.squareup.moshi.JsonQualifiersTest$FooPrefix()]" + + "\nfor class java.lang.String b" + + "\nfor class com.squareup.moshi.JsonQualifiersTest$StringAndFooString"); + assertThat(expected).hasCauseExactlyInstanceOf(IllegalArgumentException.class); + assertThat(expected.getCause()).hasMessage("No @FromJson adapter for class java.lang.String " + "annotated [@com.squareup.moshi.JsonQualifiersTest$FooPrefix()]"); assertThat(expected).hasCauseExactlyInstanceOf(IllegalArgumentException.class); assertThat(expected.getCause()).hasMessage("No next JsonAdapter for class " @@ -356,7 +361,12 @@ public final class JsonQualifiersTest { moshi.adapter(StringAndFooString.class); fail(); } catch (IllegalArgumentException expected) { - assertThat(expected).hasMessage("No @ToJson adapter for class java.lang.String " + assertThat(expected).hasMessage("No @ToJson adapter for class java.lang.String annotated " + + "[@com.squareup.moshi.JsonQualifiersTest$FooPrefix()]" + + "\nfor class java.lang.String b" + + "\nfor class com.squareup.moshi.JsonQualifiersTest$StringAndFooString"); + assertThat(expected).hasCauseExactlyInstanceOf(IllegalArgumentException.class); + assertThat(expected.getCause()).hasMessage("No @ToJson adapter for class java.lang.String " + "annotated [@com.squareup.moshi.JsonQualifiersTest$FooPrefix()]"); assertThat(expected).hasCauseExactlyInstanceOf(IllegalArgumentException.class); assertThat(expected.getCause()).hasMessage("No next JsonAdapter for class " diff --git a/moshi/src/test/java/com/squareup/moshi/MoshiTest.java b/moshi/src/test/java/com/squareup/moshi/MoshiTest.java index cefd67a..1303b63 100644 --- a/moshi/src/test/java/com/squareup/moshi/MoshiTest.java +++ b/moshi/src/test/java/com/squareup/moshi/MoshiTest.java @@ -45,6 +45,7 @@ import static java.lang.annotation.RetentionPolicy.RUNTIME; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.fail; +@SuppressWarnings("CheckReturnValue") public final class MoshiTest { @Test public void booleanAdapter() throws Exception { Moshi moshi = new Moshi.Builder().build(); @@ -955,6 +956,74 @@ public final class MoshiTest { } } + static final class HasPlatformType { + ArrayList strings; + + static final class Wrapper { + HasPlatformType hasPlatformType; + } + + static final class ListWrapper { + List platformTypes; + } + } + + @Test public void reentrantFieldErrorMessagesTopLevelMap() { + Moshi moshi = new Moshi.Builder().build(); + try { + moshi.adapter(Types.newParameterizedType(Map.class, String.class, HasPlatformType.class)); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e).hasMessage( + "Platform java.util.ArrayList (with no annotations) requires explicit " + + "JsonAdapter to be registered" + + "\nfor java.util.ArrayList strings" + + "\nfor class com.squareup.moshi.MoshiTest$HasPlatformType" + + "\nfor java.util.Map"); + assertThat(e).hasCauseExactlyInstanceOf(IllegalArgumentException.class); + assertThat(e.getCause()).hasMessage("Platform java.util.ArrayList " + + "(with no annotations) requires explicit JsonAdapter to be registered"); + } + } + + @Test public void reentrantFieldErrorMessagesWrapper() { + Moshi moshi = new Moshi.Builder().build(); + try { + moshi.adapter(HasPlatformType.Wrapper.class); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e).hasMessage( + "Platform java.util.ArrayList (with no annotations) requires explicit " + + "JsonAdapter to be registered" + + "\nfor java.util.ArrayList strings" + + "\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 java.util.ArrayList " + + "(with no annotations) requires explicit JsonAdapter to be registered"); + } + } + + @Test public void reentrantFieldErrorMessagesListWrapper() { + Moshi moshi = new Moshi.Builder().build(); + try { + moshi.adapter(HasPlatformType.ListWrapper.class); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e).hasMessage( + "Platform java.util.ArrayList (with no annotations) requires explicit " + + "JsonAdapter to be registered" + + "\nfor java.util.ArrayList strings" + + "\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 java.util.ArrayList " + + "(with no annotations) requires explicit JsonAdapter to be registered"); + } + } + @Test public void qualifierWithElementsMayNotBeDirectlyRegistered() throws IOException { try { new Moshi.Builder()