Moshi: Document Kotlin types shouldn’t subclass Java types and vice-versa

[christxph]

I'm annotating a field in a Kotlin class (using moshi-kotlin) with @Json(name = "xyz") and serializing produces the expected result, but when I try to serialize a subclass the annotation from the parent class is not applied.

Example:

class Container(@Json(name = "parent") var abc1: Parent = Child(),
                @Json(name = "child") var abc2: Child = Child())

open class Parent(@Json(name = "xyz") var abc: String = "")

class Child : Parent()

...results in:

{
  "parent": {
    "xyz": ""
  },
  "child": {
    "abc": ""
  }
}

expected result:

{
  "parent": {
    "xyz": ""
  },
  "child": {
    "xyz": ""
  }
}

Is this the intended behavior?

[swankjesse]

Yikes. Nope, that looks broken!

[NightlyNexus]

~I think it's a more general problem of not using the parent's properties?~ no.

[Egorand]

Actually declaring the use-site target explicitly solves the problem:

open class Parent(@property:Json(name = "xyz") var abc: String = "")

but it's verbose. Maybe a Kotlin-specific version of @Json which only attaches to properties is a good idea - it'll greatly simplify the logic.

[NightlyNexus]

The Kotlin adapter looks for the annotation on all properties (that's why @property is a workaround), and it also looks for the annotation on the primary constructor parameters that have properties of the same name.

The problem in this issue is that it doesn't look at the superclass's constructor parameters.
I think constructor parameters should continue to be supported instead of forcing users to attach a special annotation only to properties (would muddy data classes).

[Egorand]

That's correct, but it complicates the implementation, as you have to search in a lot of places to collect the annotations, while it really only makes sense to have properties annotated with @Json. I guess if there was something like the following (the class name sucks I know):

@Target(AnnotationTarget.PROPERTY)
annotation class KJson(val name: String)

then there would be no need to look into constructor params (haven't verified if it indeed works this way). Right now you can annotate params that don't have corresponding properties:

class Foo(@Json(name = "bar") foo: String)

which has no effect (or does it?), so would be great to disallow it completely to avoid confusion.

[NightlyNexus]

(aside: If the parameter doesn't have a corresponding property, the factory will throw an IllegalArgumentException when trying to create an adapter.)

Ah, I see what you mean.
With KJson, this data class Foo(@KJson(name = "bar") foo: String) would have the annotation applied to the property (instead of the parameter) automatically. Interesting.

That's cool, even if more API surface area (and a bigger transition for folks migrating off Java).
Dropping @Json support would be a breaking change, though.

_sorry, I went off the original issue topic._

[swankjesse]

Good discussion! Can Moshi simultaneously be the best JSON library for Java _and_ for Kotlin? Or does it have to pick one?

[NightlyNexus]

I've thought about writing a separate toy object mapper for Kotlin. Since Kotlin's type system is like a superset of Java's, it's tough for a Java API for types to understand Kotlin callers' types. :/

[marcosalis]

I'm experiencing a similar issue on a specific Java/Kotlin edge case (1.6.0-SNAPSHOT).

I have a Java model:

public abstract class JavaModel  {
    private final long id;

    public JavaModel(long id) {
        this.id = id;
    }

    public long getId() { return id; }
}

And an extending Kotlin model:

class KotlinModel(id: Long, val name: String?) : JavaModel(id)

I'm trying to deserialize a very simple JSON object:

{ "id" : 1, "name" : "name" }

But it fails with the following exception:

No property for required constructor parameter #0 id of fun (kotlin.Long, kotlin.String?): KotlinModel at com.squareup.moshi.kotlin.KotlinJsonAdapterFactory.create(KotlinJsonAdapter.kt:240)

I can't use val id: Long because the model is already used by another library, which deems that a duplication of the parent field.
Any suggestions?

[swankjesse]

@marcosalis having a Kotlin type extending a Java type or vice-versa is something we’re not expecting. Your best bet is to manually write @ToJson/@FromJson methods for those. Sorry about that.

[marcosalis]

Fair enough @swankjesse, thank you for your reply!

[swankjesse]

We should document that we don’t support Kotlin subtypes of Java types and vice-versa.

[NightlyNexus]

Yes.

And, the original post also shows a broken behavior for Kotlin subclasses of Kotlin types.

[swankjesse]

Added to the README.