nulls in generated Json break backwards compatiblity checks

[TimWSpence]

Opening this to see if anyone has found a better solution to this.

Consider for example:

case class Person(name: String, address: Option[String])

object Person {

  implicit val decoderForPerson: Decoder[Person] = deriveDecodder[Person

  implicit val encoderForPerson: Encoder[Person] = deriveEncoder[Person]

  implicit val eqForPerson: Eq[Person] = Eq.fromUniversalEquals[Person]
}

The address field is optional and if we are unlucky then scalacheck will generate Person("some name", None)
which circe golden will serialize to

{
  "name": "some name",
  "address": null
}

Unfortunately this means that the address subtree is effectively untested for backwards compatiblity. For example,
the following breaking change to the codec for address still passes the golden tests

case class Person(name: String, address: Option[String])


object Person {

  implicit val decoderForPerson: Decoder[Person] = new Decoder[Person] {
    def apply(c: HCursor): Decoder.Result[Person] =
      for {
        name <- c.downField("name").as[String]
        address <- c.downField("address").as[Option[Value]]
      } yield Person(name, address.map(_.value))
  }

  implicit val encoderForPerson: Encoder[Person] = new Encoder[Person] {

    def apply(a: Person): Json = Json.obj(
      "name" -> a.name.asJson,
      "address" -> a.address.fold(Json.Null)(addr => Json.obj("value" -> addr.asJson))
    )

  }

  implicit val eqForPerson: Eq[Person] = Eq.fromUniversalEquals[Person]
}

case class Value(value: String)

object Value{
  implicit val decoderForValue: Decoder[Value] = deriveDecoder[Value]
}

I've hacked around it using circe-droste:

  val containsNulls: Algebra[JsonF, Boolean] = Algebra {
    case JsonF.NullF       => true
    case JsonF.ArrayF(xs)  => xs.exists(identity)
    case JsonF.ObjectF(xs) => xs.map(_._2).exists(identity)
    case _                 => false
  }

generator.filter(person => !cata(containsNulls).apply(person))

but was wondering if anyone had a better solution? Or if things would go terribly wrong if circe-golden did this by default?

[kubukoz]

I think this exhibits a larger problem, in which you can't be sure that the generated cases will cover all branches of an ADT (in this case, Option). Personally I try to ensure the examples I get contain at least one of each branch, otherwise I regenerate the example (albeit inefficient, it seems to work). Still, it'd be great if there was something more automated to do this.

[TimWSpence]

I think this exhibits a larger problem, in which you can't be sure that the generated cases will cover all branches of an ADT (in this case, Option). Personally I try to ensure the examples I get contain at least one of each branch, otherwise I regenerate the example (albeit inefficient, it seems to work). Still, it'd be great if there was something more automated to do this.

Yeah good point 👍