square / moshi

A modern JSON library for Kotlin and Java.
https://square.github.io/moshi/1.x/
Apache License 2.0
9.74k stars 758 forks source link

Upgrade Moshi to Kotlin and call it Moshi 2 #1252

Open swankjesse opened 4 years ago

swankjesse commented 4 years ago

As with Okio and OkHttp let’s get Moshi implemented in Kotlin.

Goals

Implemented in Kotlin with a dependency on the Kotlin standard library. We want the option to support multiplatform including Kotlin/Native .

100% binary compatibility with Moshi 1.x.

100% Java source compatibility with Moshi 1.x.

Easy, automated upgrade for Kotlin source

No performance regressions. We must write careful Kotlin, paying attention to abstractions that have a runtime cost.

Non-Goals

I’d like to do the Kotlin conversion first, and then attack the problem of deeply supporting Kotlin’s richer type system. I prefer to save that for a follow-up release, though I can be convinced otherwise!

Tasks

ZacSweers commented 4 years ago

Can you expand on what you mean by the type system bit? For instance, I think we may want to make Moshi run on KType under the hood rather than Type, not sure if that has implications on what you were thinking of

swankjesse commented 4 years ago

I’d like to do that, but I think we can hold it until 2.1.

ZacSweers commented 4 years ago

I'll take a pass at adding the japicmp tooling today or tomorrow. Should be good just pulling from what Okio/OkHttp do?

For benchmarks, we could reuse my benchmarking project with one of 1.x or 2.x shaded for comparisons. Have any other benchmarks in mind?

swankjesse commented 4 years ago

Borrowing liberally from OkHttp and Okio is good.

For benchmarks we just want to compare old vs. new with some repeatability.

ZacSweers commented 3 years ago

Should we integrate kotlin' ABI validator on top of japicmp? This way we can prevent surprise ABI changes due to kotlin bytecode changes. Noticed leakcanary uses it too and seems fairly easy to integrate

https://github.com/Kotlin/binary-compatibility-validator

JakeWharton commented 3 years ago

Why would you use it on top of japicmp? What does it provide that japicmp does not? Last I checked it was just a signature validator exactly like japicmp.

ZacSweers commented 3 years ago

japicmp doesn't take some kotlin constructs into consideration, like internal, $WhenMappings, multifile facades, etc.

JakeWharton commented 3 years ago

When mappings should be private and synthetic and not show up. I'm not sure what multifile facade means but if you mean @JvmMultifileClass that should work just fine since all contents show up as a single class in bytecode. Internal is indeed unfortunate (that Kotlin implemented them in a way that they're actually public).

It would be nice to do better than Kotlin's binary compatibility validator. It has no multiplatform support for the various binary interfaces produced and no source compatibility validation and when I spoke to them (a year ago) they had no plans to add those.

JakeWharton commented 3 years ago

Do we lose anything from switching? I would assume not. I'm not opposed to switching, but I usually like to gain something significant. Does it run on our Java codebase today and emit signature files that we can accomplish the stated goals of compatibility across major versions?

ZacSweers commented 3 years ago

Had a draft sitting around and forgot to followup, but the other goal of binary validator that should sort of be a separate issue is ensuring compatibility between 2.x releases too, vs not just breaking with 1.x. I.e. don't add something in 2.1 and accidentally break it in 2.2 (both of which would be "compatible" with 1.x).

spencergriffin commented 3 years ago

Is this going to be prioritized soon? I am evaluating KMP and wondering if it would be better to switch to using kotlinx serialization or if moshi will support kotlin/native soon.

ZacSweers commented 2 years ago

Is this going to be prioritized soon? I am evaluating KMP and wondering if it would be better to switch to using kotlinx serialization or if moshi will support kotlin/native soon.

The only plans currently are to migrate to Kotlin. KMP support is not covered by this issue. Maybe some day, but that's not the immediate goal and it's best not to conflate the two.

yogurtearl commented 2 years ago

Looks like that .java files that are left are tests and example code.

Is converting tests and examples from Java to Kotlin in scope for this issue (and 2.x)?

ZacSweers commented 2 years ago

Examples maybe, tests no (intentionally leaving them unchanged to cover for any possibly regressions, both functionally and for java interop). @swankjesse for okio/okhttp, were recipes converted?