square / moshi

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

Code gen should error if there are no properties #1044

Open ber4444 opened 4 years ago

ber4444 commented 4 years ago

Classes that are generated look like this:

@Suppress("DEPRECATION", "unused", "ClassName", "REDUNDANT_PROJECTION")
class FooJsonAdapter(
  moshi: Moshi
) : JsonAdapter<Foo>() {

where Parameter 'moshi' is never used is the warning. Which then becomes an error in projects that has

    kotlinOptions {
        allWarningsAsErrors = true
ZacSweers commented 4 years ago

That shouldn't ever happen, where do you have a generated class that doesn't use the Moshi parameter?

ber4444 commented 4 years ago

It's happening for classes like this, for example. Maybe I'm using incorrect syntax?

@JsonClass(generateAdapter = true)
class Foo {

    @JsonClass(generateAdapter = true)
    data class FooData(
        val data: Data?,
        val status: Status?
    )
    @JsonClass(generateAdapter = true)
    data class Bar(
        val success: Boolean?,
        val responseCode: Int?
    )
ZacSweers commented 4 years ago

Can you paste the generated adapters for those? Those should be using the Moshi parameter

ber4444 commented 4 years ago
import com.squareup.moshi.JsonAdapter
import com.squareup.moshi.JsonReader
import com.squareup.moshi.JsonWriter
import com.squareup.moshi.Moshi
import java.lang.NullPointerException
import kotlin.String
import kotlin.Suppress
import kotlin.text.buildString

@Suppress("DEPRECATION", "unused", "ClassName", "REDUNDANT_PROJECTION")
class FooJsonAdapter(
  moshi: Moshi
) : JsonAdapter<Foo>() {
  private val options: JsonReader.Options = JsonReader.Options.of()

  override fun toString(): String = buildString(39) {
      append("GeneratedJsonAdapter(").append("Foo").append(')') }

  override fun fromJson(reader: JsonReader): Foo {
    reader.beginObject()
    while (reader.hasNext()) {
      when (reader.selectName(options)) {
        -1 -> {
          // Unknown name, skip it.
          reader.skipName()
          reader.skipValue()
        }
      }
    }
    reader.endObject()
    return Foo(
    )
  }

  override fun toJson(writer: JsonWriter, value: Foo?) {
    if (value == null) {
      throw NullPointerException("value was null! Wrap in .nullSafe() to write nullable values.")
    }
    writer.beginObject()
    writer.endObject()
  }
}
ZacSweers commented 4 years ago

Oh I see now, just the empty Foo class. What's the use case for deserializing a property-less class there?

ber4444 commented 4 years ago

I think it's just left-over from Moshi 1.8 times when

@JsonClass(generateAdapter = true)
class Foo {
    data class FooData(

used to work. Someone added the annotation to FooData without removing it on the outer class.

ZacSweers commented 4 years ago

Gotcha, I think this probably isn't a big we want to "fix" since it's hiding a real issue under the hood. Maybe we should just error on no properties

gpeal commented 2 years ago

@ZacSweers I just ran into this for a polymorphic type in which some of the types represent a state but have no need for nested properties. This is probably also tangentially related to some object conversations on other issues here. Thoughts?

ZacSweers commented 2 years ago

does this cover your use case? https://github.com/ZacSweers/MoshiX/tree/main/moshi-sealed#object-subtypes