Moshi: Kotlin Codegen Release Blockers

[swankjesse]

The code’s checked in but there’s some follow-ups to do before we can invite users to use it:

• [x] kotlin-metadata dependency released in mavencentral (https://github.com/Takhion/kotlin-metadata/issues/5)
• [x] constructor signature
• [x] @JsonQualifier annotations
• [x] @MoshiSerializable annotation and factory potentially in core
• [x] ”Generated code; do not edit“ header

[Shakeskeyboarde]

Add jcenter to your pom, and there’s no need for kotlin-metadata to be in maven central.

<repositories>
    <repository>
      <id>jcenter</id>
      <url>https://jcenter.bintray.com/</url>
    </repository>
</repositories>

[Shakeskeyboarde]

https://bintray.com/takhion/maven/kotlin-metadata

[JakeWharton]

Sonatype OSSRH prevents release of artifacts to Maven central which point to arbitrary repositories.

[ZacSweers]

We should probably generated @Generated annotations onto the adapters via https://github.com/google/auto/blob/master/common/src/main/java/com/google/auto/common/GeneratedAnnotations.java#L46-L61

This could possibly just replace the "generated code, do not edit" header bit in the list.

This would also allow for more fine-grained proguard keepnames retentions, which otherwise would have to keep all of moshi's built-in JsonAdapter-suffixed classes as well due to **JsonAdapter

-keepnames @com.squareup.moshi.<wherever this ends up>.MoshiSerializable class *

# Java < 9
-keepnames @javax.annotation.Generated class **JsonAdapter

#  Java 9+
-keepnames @javax.annotation.processing.Generated class **JsonAdapter

[ZacSweers]

Another one for the list: proguard will make seemingly unused constructors private, so we should defensively check in MoshiSerializableFactory if the Constructor is accessible and make it so if it's not

[swankjesse]

I’m looking into this. We need something like compile-testing for Kotlin. Otherwise we can’t test that unsupported inputs like abstract classes are handled appropriately. Right now we’re only testing inputs that can successfully generate an adapter.

[ZacSweers]

@takhion has actually mentioned working on something like that recently,
not sure where it stands.

On Mar 28, 2018 at 7:39 PM, Jesse Wilson [e-mail] wrote:

I’m looking into this. We need something like compile-testing for Kotlin.
Otherwise we can’t test that unsupported inputs like abstract classes are
handled appropriately. Right now we’re only testing inputs that can
successfully generate an adapter.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/square/moshi/issues/461#issuecomment-377102128, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABTEvtXLrys349waEfpsT7HDLLX8pK-pks5tjElRgaJpZM4Sl80x
.

[Takhion]

@swankjesse @hzsweers

• kotlin-metadata in Maven Central
  everything is ready, just waiting on OSSRH-38743

• constructor signature in kotlin-metadata
  will be in the next major version, aaalmost there

• compile-testing for Kotlin
  I'm developing a prototype for kotlin-metadata based on Gradle TestKit, but it's gonna take some more time!

[bishiboosh]

Any global timeline on when this feature could be available to users ? I currently have major problems with the reflection adapter factory because of proguard, and this could save my life.

[LouisCAD]

@bishiboosh Kotshi can already save your life: https://github.com/ansman/kotshi

[bishiboosh]

I know about kotshi, but I would rather use an officially sanctionned implementation (plus this one handles default values better, and will support qualifiers properly)

[LouisCAD]

@bishiboosh FYI, Jake Wharton committed on Kotshi repo. Also, this is open source stuff, so no need for an "official" thing approved by a president 😆. Yes, default values are better handled there, but if you don't have too much of them, using Kotshi until this gets released might be a good solution for you.

[LouisCAD]

@Takhion It seems that they are waiting for a comment response from you, there, in order to host your project on mavenCentral.

[Takhion]

@LouisCAD DOH! Thank you, I had already preemptively replied to that question in the description, and I assumed it was just an automatic reply (which it is), but didn't see the Waiting for Response status 🤦‍♂️! Just replied, let's see...

[LouisCAD]

@Takhion Your comment response definitely changed the status! Back to open now 😃

[bishiboosh]

Kind-of stupid question, but as this is a bug in Kotshi, it may be useful here too : are @Transient field handled properly when creating the adapter ? (as in, ignored when serializing and set to default value when deserializing)

[LouisCAD]

@swankjesse The checkboxes are out of date, kotlin-metadata is now on mavenCentral

[swankjesse]

I’d like to drop support for types that are subtypes of concrete types. The problem is that when we codegen a class we will need to observe the properties of its superclass. But the superclass could change (maybe it’s in a different project!) and we won’t regenerate the subclass’ adapter.

This is easy with reflection (it’s late binding) but nearly impossible with codegen.

Thoughts?

[LouisCAD]

@swankjesse Could you illustrate what you're saying with example code snippets?

[Takhion]

@swankjesse the annotation processor always runs (unless incremental processing is explicitly enabled) so changes to any class should always be tracked, or am I missing something?

[NightlyNexus]

@Takhion that seems reliant on a build system. also, somebody might distribute generated sources and not the generator.

drop support for types that are subtypes of concrete types.
...
we will need to observe the properties of its superclass. But the superclass could change (maybe it’s in a different project!) and we won’t regenerate the subclass’ adapter.

Doesn't that argument apply to types that are subtypes of abstract types, too?

I'm a fan of killing off inheritance to avoid accidents. On one hand, it'd be nice to have parity with the reflective adapters. On the other, I think the annotation processor can be be more opinionated. It's a power user feature.

[Takhion]

@NightlyNexus but that's true of every annotation processor and pretty much expected behaviour: in general you should only use it at the "final" stage of compilation (like when producing the consumer executable/app), unless you know _exactly_ what you're doing :)

[JakeWharton]

Dagger and Butter Knife are two counterpoints to that.

[NightlyNexus]

yeah, i immediately thought of Dagger.

[swankjesse]

It’s done. Yay!