Moshi: kotlin codgen: multiple constructors with default lead to crash

[Wasabi375]

@JsonClass(generateAdapter = true)
class Foo(val f: Int)

@JsonClass(generateAdapter = true)
class Bar(val f: Foo = Foo(5), val b: Int) {
    constructor(f: Int, b: Int = 6): this(Foo(f), b)
}
@ExperimentalStdlibApi
fun main() {
    val moshi = Moshi.Builder().build()
    moshi.adapter<Bar>().fromJson("""{"b":6}""")
}

fails with

Exception in thread "main" java.lang.IllegalArgumentException
    at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
    at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
    at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
    at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
    at moshiTest.BarJsonAdapter.fromJson(BarJsonAdapter.kt:55)
    at moshiTest.BarJsonAdapter.fromJson(BarJsonAdapter.kt:17)
    at com.squareup.moshi.internal.NullSafeJsonAdapter.fromJson(NullSafeJsonAdapter.java:40)
    at com.squareup.moshi.JsonAdapter.fromJson(JsonAdapter.java:43)
    at moshiTest.MainKt.main(main.kt:46)
    at moshiTest.MainKt.main(main.kt)

The problem is that kotlin generates multiple methods with the DefaultConstructorMarker argument. That leads to a chance of the fromJson code choosing the wrong constructor in Util.lookupDefaultsConstructor.

[ZacSweers]

oof. So we either need to check the exact desired signature or pass in arguments for all the desired classes for the lookup (requires caching a lookup of the DefaultConstructorMarker class. I'm thinking the latter, and we can just remove the runtime util requirement for the constructor entirely

[Wasabi375]

I found this will investigating a problem with inline classes.

@JsonClass(generateAdapter = true)
class Consumer(val inline: Inline = Inline(5))
@JsonClass(generateAdapter = true)
inline class Inline(val i: Int)

this fails with a similar problem. I'm still investigating it, but it seems related to how inline classes work as generics. The defaults constructor of Consumer expects an argument of type Int? (the inlined version of Inline) but instead gets Inline. I'm still investigating this, but it seems like inline classes are still very problematic ☹️ . I'll create an issue for that once I know what's going on.

Just mention this so you know that this issue here, isn't really a priority for me at least.

[ZacSweers]

Thanks for the heads up. We're not officially supporting inline yet

[Wasabi375]

Yeah, I know, but I'm still using it and I guess you won't mind the feedback so it can at least be tracked. Also I have no problem with fixing it myself (once I know what's going on).

[ZacSweers]

Opened a PR to fix this here: https://github.com/square/moshi/pull/976

Thanks again @Wasabi375 for all your contributions and bug finding so far!

[Wasabi375]

I wish I wouldn't find them, because I could get on with my project :wink: But then I find them by using moshi in a hobby project of mine and I enjoy the bug hunting, so no hard feelings. It's not liekly that I will ever finish this game engine of mine anyways 🤷‍♂

Moshi is an interesting library. AFAIK it's the only json library so far that supports kotlin default values so there is no other option for me, but then me using all the experimental kotlin featurs plus the complex asset structur I have seems to be the perfect bug hunting ground. So I guess I'll keep reporting them.