surprising discriminator behavior

[masonedmison]

I am using circe-generic-extras to add a discriminator to a simple ADT but I notice that when I manually provide a Encoder for one of the ADT members, the discriminator does not encode as expected however the discriminator does encode as expected if I derive an Encoder instance.

Here is a simple example to illustrate the discrepancy:

import io.circe.Codec
import io.circe.generic.semiauto.deriveCodec
import io.circe.generic.extras.semiauto.deriveConfiguredCodec
import io.circe.Codec
import io.circe.generic.semiauto.deriveEncoder
import io.circe.generic.extras.semiauto.deriveConfiguredEncoder
import io.circe.generic.JsonCodec
import io.circe.generic.extras.Configuration
import io.circe._
import io.circe.syntax._

object sumADTFun extends App {

  @JsonCodec
  sealed trait InnerA

  sealed trait Wrapped

  object Wrapped {
    implicit val config: Configuration =
      Configuration.default
        .withDiscriminator("operation")

    implicit val forCodecs: Encoder[Wrapped] = deriveConfiguredEncoder
  }

  @JsonCodec
  case class B(value: String, num: Int) extends InnerA

  case class WrappedB(innerA: InnerA) extends Wrapped
  object WrappedB {
    // ** different values depending on semi-derived vs manual codecs **
    //implicit val semiDerivedEncoder: Encoder[WrappedB] = deriveEncoder

    // when using semi-derived:
    // {
    //   "innerA" : {
    //     "B" : {
    //       "value" : "foo",
    //       "num" : 42
    //     }
    //   },
    //   "operation" : "WrappedB"
    // }

    // implicit val manualCodecs: Codec[WrappedB] = Codec.from(
    //   Decoder[B].map(WrappedB.apply),
    //   Encoder[B].contramap((wb: WrappedB) => wb.innerA.asInstanceOf[B])
    // )
    implicit val forEncoder: Encoder[WrappedB] = Encoder[InnerA].contramap((wb: WrappedB) => wb.innerA)

    // when using manually specified codecs:
    // {
    //   "WrappedB" : {
    //     "value" : "foo",
    //     "num" : 42
    //  }
    // }
  }

  val b = WrappedB(B("foo", 42))

  println {
    (b: Wrapped).asJson
  }
}

Note the different behavior depending on which Encoder[WrappedB] is in scope.

Also, here is a scastie with the same code as above with the versions I am currently using. Is this a bug or am I just happening to do things in an unexpected way?

Thanks in advance.

[masonedmison]

I see now the issue is is that addDiscriminator has a signature of

  final protected[this] def addDiscriminator[B](
    encode: Encoder[B],
    value: B,
    name: String,
    discriminator: Option[String]
  ): JsonObject

Encoder is pattern matched and the discriminator is only added if Encoder is an Encoder.AsObject. While I understand this and it makes sense, I think it could be nice to have a boolean flag on withDiscriminator to optionally "ignore" the discrimnators forEncoders that are not able to actually add the discriminator (essentially what occurs now) or the option to be to generate a compiler error if all Encoders summoned are not Encoder.AsObjects. If this is something you think would be nice to have, let me know and I'd be more than happy to submit a PR :)

[zarthross]

From my perspective everything is working as expected, and your manual decoder is wrong.

In circe-generic and circe-generic-extras I think it assumes if you make a case class the subclass of a sealed trait, there will 1 day be more case classes for that trait (thats usually why you do that). So in order to decode those later, you are going to need a discriminator.

Now, where the discriminators are were completely determined by where you had your configuration.

For the semi-auto Wrapped trait, you defined operation as the discriminator key, so you got a field, within the Wrapped json called operation with the value of the type of case class.

For the semi-auto InnerA you didn't set a config, so it nested the body of InnerA in an object with a field called B, which is the constructor name for the type..

    {
      "innerA" : { // because the field in `WrappedB` was called `innerA`
        "B" : { //  Because the `InnerA` in `innerA` was of type `B` this is the discriminator.  (Default circe generic behavior)
          "value" : "foo",
           "num" : 42
         }
      },
      "operation" : "WrappedB" // Because Wrapped is a sealed trait it MUST have a discriminator, and `operation` was set in the configuration, so we discriminate there.
   }

The manual codec you list is ignoring the fact that WrappedB contains a field called innerA. Thats why it looks so different.

We do have a Codec for 'wrappers' if you want to use that.
https://github.com/circe/circe-generic-extras/blob/v0.14.3/generic-extras/src/main/scala/io/circe/generic/extras/semiauto.scala#L98

[zarthross]

Close, but feel free to reply or reopen if you see something wrong.