Moshi: Moshi Codegen with Nothing and Retrofit

[TimothyEarley]

Not quite sure where this issue belongs (here or retrofit).

Anyway, I ran into an interesting problem while using retrofit (2.4.0) and moshi kotlin codegen (1.6.0). I have a service which takes a parametric class as a @Body argument. My use case is this.

@JsonClass(generateAdapter = true)
data class Bar<T>(val t: T)

interface Service {
    @POST("...") fun send(@Body Bar<String>): Call<Bar<String>>
}

The generated JsonAdapter captures this generic parameter no problem and retrofit also uses the correct generic type gathered here (and on the master branch here).

The problem arises when I replace String with Nothing. If my analysis was correct this compiles to a raw type. This means that while the codegen still correctly sees the generic, retrofit fails to see the generic parameter at runtime (using Method.getGenericParameterTypes()).

This results in an exception while attempting to call the send method from above. The error is

java.lang.IllegalArgumentException: Unable to create @Body converter for class Bar (parameter #1) for method Service.send
                     at retrofit2.ServiceMethod$Builder.methodError(ServiceMethod.java:755)
                      at retrofit2.ServiceMethod$Builder.parameterError(ServiceMethod.java:760)
                      at retrofit2.ServiceMethod$Builder.parseParameterAnnotation(ServiceMethod.java:716)
                      at retrofit2.ServiceMethod$Builder.parseParameter(ServiceMethod.java:339)
                      at retrofit2.ServiceMethod$Builder.build(ServiceMethod.java:207)
                      at retrofit2.Retrofit.loadServiceMethod(Retrofit.java:170)
                      at retrofit2.Retrofit$1.invoke(Retrofit.java:147)
                      at java.lang.reflect.Proxy.invoke(Proxy.java:913)

         [...]

Caused by: java.lang.RuntimeException: Failed to find the generated JsonAdapter constructor for class Bar
                     at com.squareup.moshi.StandardJsonAdapters.generatedAdapter(StandardJsonAdapters.java:252)
                      at com.squareup.moshi.StandardJsonAdapters$1.create(StandardJsonAdapters.java:62)
                      at com.squareup.moshi.Moshi.adapter(Moshi.java:130)
                      at retrofit2.converter.moshi.MoshiConverterFactory.requestBodyConverter(MoshiConverterFactory.java:106)
                      at retrofit2.Retrofit.nextRequestBodyConverter(Retrofit.java:282)
                      at retrofit2.Retrofit.requestBodyConverter(Retrofit.java:262)
                      at retrofit2.ServiceMethod$Builder.parseParameterAnnotation(ServiceMethod.java:713)
Caused by: java.lang.NoSuchMethodException: <init> [class com.squareup.moshi.Moshi]
...

The relevant function is generatedAdapter which tries to call the wrong constructor, i.e. the one without a type parameter.

I don't know if this issue can be fixed or where it should be fixed, but I thought documenting this behaviour would at least be a start. I have circumvented the issue at the moment by simply not using Nothing.

Anyway, thanks a lot for the otherwise great library (especially the kotlin code gen)!

[JakeWharton]

I don't think there's much Retrofit can do, so you're probably in the right place.

[NightlyNexus]

What should Moshi do here? My preference would be to fail with a clear error message about using raw types. Another option would be to look for an adapter for Void or Nothing or something else, though I doubt many people install adapters for those types.

[swankjesse]

I couldn’t reproduce this problem in a test case.

[swankjesse]

@TimothyEarley can you confirm that this is still broken on the latest snapshot? I believe the bug either doesn’t exist or has since been fixed.

[NightlyNexus]

I can still reproduce this on the current master branch.

@JsonClass(generateAdapter = true)
data class Box<T>(val value: T)

@JsonClass(generateAdapter = true)
data class HasRawTypeWithGeneratedAdapter(val value: List<Box<Nothing>>)

@Test fun foo() {
  val moshi = Moshi.Builder().build()
  val adapter = moshi.adapter(HasRawTypeWithGeneratedAdapter::class.java)
}

java.lang.RuntimeException: Failed to construct the generated JsonAdapter for class com.squareup.moshi.kotlin.codgen.GeneratedAdaptersTest$HasRawTypeWithGeneratedAdapter
    at com.squareup.moshi.StandardJsonAdapters.generatedAdapter(StandardJsonAdapters.java:258)

(it's got a raw type.)

[swankjesse]

@NightlyNexus the underlying problem is we don’t have a built-in adapter for the platform types of Nothing and Void. The fix is surprising!

    val moshi = Moshi.Builder()
        .add(object : Any() {
          @ToJson fun toJson(writer: JsonWriter, o: Nothing?) {
            writer.nullValue()
          }

          @FromJson fun fromJson(reader: JsonReader): Nothing? {
            reader.skipValue()
            return null
          }
        })
        .build()

[NightlyNexus]

oops, yeah! 👍

[TimothyEarley]

Sorry for the late response. I have created a test case here: https://github.com/TimothyEarley/nothing-moshi-test/blob/master/src/main/java/Main.kt

The problem is not a lack of adapters for Nothing and Void, but rather the two different types for Box<Nothing>: retrofit uses getGenericParameterTypes() on a method and gets the raw type Box while moshi codegen uses the class definition Box<T> and creates an adapter that needs a generic type.

[egek92]

any progress on this? It makes impossible to use https://github.com/slackhq/EitherNet without ErrorResponse

[TimothyEarley]

I just looked into the issue again and found the following:

• The error message was improved to indicate the mismatch between generated code and what is beeing looked for (see https://github.com/square/moshi/commit/f891c8187b5d6f151870cb184c7f93f654910b3f)
• Retrofit can really do nothing since the type is erased by the compiler (see https://github.com/JetBrains/kotlin/blob/master/compiler/testData/writeSignature/nothing/nullableNothing.kt ) and so it is fair that it uses the raw type when asking Moshi for the adapter.
• There is a fix to get Moshi to load the adapter, but then the issue becomes that the adapter does not work for nullable T (I think the generator assumes the T in Box is not nullable, whether that is right or not I don't know)

The "fix":

diff --git a/moshi/src/main/java/com/squareup/moshi/internal/Util.java b/moshi/src/main/java/com/squareup/moshi/internal/Util.java
index b517c98..2ef151f 100644
--- a/moshi/src/main/java/com/squareup/moshi/internal/Util.java
+++ b/moshi/src/main/java/com/squareup/moshi/internal/Util.java
@@ -579,6 +579,15 @@ public final class Util {
       throw new RuntimeException("Failed to find the generated JsonAdapter class for " + type, e);
     } catch (NoSuchMethodException e) {
       if (!(type instanceof ParameterizedType) && adapterClass.getTypeParameters().length != 0) {
+        // maybe the type got erased because it was nothing
+        try {
+          Constructor<? extends JsonAdapter<?>> constructor = adapterClass.getDeclaredConstructor(Moshi.class, Type[].class);
+          return constructor.newInstance(moshi, new Type[] { Void.class });
+        } catch (NoSuchMethodException | InstantiationException | IllegalAccessException | InvocationTargetException noSuchMethodException) {
+          // TODO handle exception
+        }
+
+
         throw new RuntimeException(
             "Failed to find the generated JsonAdapter constructor for '"
                 + type

In my original use case the workaround was pretty simple (Use a singleton object instead of Nothing?). I don't know EitherNet, but maybe that could work there too.