Fix unwanted skipping of required fields that has values of product t…

…ypes + yet more efficient encoding of product types (#1343)
zio · Feb 26, 2025 · 0dd96c4 · 0dd96c4
1 parent f6ace96
commit 0dd96c4
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 159 deletions.
diff --git a/zio-json/shared/src/main/scala-2.x/zio/json/macros.scala b/zio-json/shared/src/main/scala-2.x/zio/json/macros.scala
@@ -507,80 +507,67 @@ object DeriveJsonEncoder {
     else {
       val nameTransform =
         ctx.annotations.collectFirst { case jsonMemberNames(format) => format }.getOrElse(config.fieldNameMapping)
-      val params        = ctx.parameters.filter(p => p.annotations.collectFirst { case _: jsonExclude => () }.isEmpty).toArray
       val explicitNulls = config.explicitNulls || ctx.annotations.exists(_.isInstanceOf[jsonExplicitNull])
       val explicitEmptyCollections = ctx.annotations.collectFirst { case a: jsonExplicitEmptyCollections => a.encoding }
         .getOrElse(config.explicitEmptyCollections.encoding)
+      val params = ctx.parameters.filter(p => p.annotations.collectFirst { case _: jsonExclude => () }.isEmpty).toArray
       new JsonEncoder[A] {
-        private[this] lazy val fields: Array[FieldEncoder[Any, Param[JsonEncoder, A]]] = params.map { p =>
-          val name              = p.annotations.collectFirst { case jsonField(name) => name }.getOrElse(nameTransform(p.label))
-          val withExplicitNulls = explicitNulls || p.annotations.exists(_.isInstanceOf[jsonExplicitNull])
-          val withExplicitEmptyCollections = p.annotations.collectFirst { case a: jsonExplicitEmptyCollections =>
-            a.encoding
-          }.getOrElse(explicitEmptyCollections)
+        private[this] lazy val fields = params.map { p =>
           FieldEncoder(
-            p,
-            name,
-            p.typeclass.asInstanceOf[JsonEncoder[Any]],
-            withExplicitNulls = withExplicitNulls,
-            withExplicitEmptyCollections = withExplicitEmptyCollections
+            p = p,
+            name = p.annotations.collectFirst { case jsonField(name) => name }.getOrElse(nameTransform(p.label)),
+            encoder = p.typeclass.asInstanceOf[JsonEncoder[Any]],
+            withExplicitNulls = explicitNulls || p.annotations.exists(_.isInstanceOf[jsonExplicitNull]),
+            withExplicitEmptyCollections = p.annotations.collectFirst { case a: jsonExplicitEmptyCollections =>
+              a.encoding
+            }.getOrElse(explicitEmptyCollections)
           )
         }
 
-        override def isEmpty(a: A): Boolean = fields.forall { field =>
-          val paramValue = field.p.dereference(a)
-          field.encoder.isEmpty(paramValue) || field.encoder.isNothing(paramValue)
-        }
-
         def unsafeEncode(a: A, indent: Option[Int], out: Write): Unit = {
           out.write('{')
           val indent_ = JsonEncoder.bump(indent)
           JsonEncoder.pad(indent_, out)
           val fields     = this.fields
           var idx        = 0
-          var prevFields = false // whether any fields have been written
+          var prevFields = false
           while (idx < fields.length) {
             val field = fields(idx)
             idx += 1
-            val encoder = field.encoder
-            val p       = field.p.dereference(a)
-            if ({
-              (field.flags: @switch) match {
-                case 0 => encoder.isEmpty(p) || encoder.isNothing(p)
-                case 1 => encoder.isNothing(p)
-                case 2 => encoder.isEmpty(p)
-                case _ => false
-              }
-            }) ()
+            val p = field.p.dereference(a)
+            if (field.skip(p)) ()
             else {
               if (prevFields) {
                 out.write(',')
                 JsonEncoder.pad(indent_, out)
               } else prevFields = true
-              out.write(field.encodedName)
-              if (indent.isEmpty) out.write(':')
-              else out.write(" : ")
-              encoder.unsafeEncode(p, indent_, out)
+              if (indent.isEmpty) out.write(field.encodedName)
+              else out.write(field.prettyEncodedName)
+              field.encoder.unsafeEncode(p, indent_, out)
             }
           }
           JsonEncoder.pad(indent, out)
           out.write('}')
         }
 
-        override final def toJsonAST(a: A): Either[String, Json] =
-          fields
-            .foldLeft[Either[String, Chunk[(String, Json)]]](Right(Chunk.empty)) { case (c, field) =>
-              val param      = field.p
-              val paramValue = param.dereference(a).asInstanceOf[param.PType]
-              field.encodeOrDefault(paramValue)(
-                () =>
-                  c.flatMap { chunk =>
-                    param.typeclass.toJsonAST(paramValue).map(value => chunk :+ field.name -> value)
-                  },
-                c
-              )
+        override final def toJsonAST(a: A): Either[String, Json] = {
+          val buf    = Array.newBuilder[(String, Json)]
+          val fields = this.fields
+          var idx    = 0
+          while (idx < fields.length) {
+            val field = fields(idx)
+            idx += 1
+            val p = field.p.dereference(a)
+            if (field.skip(p)) ()
+            else {
+              field.encoder.toJsonAST(p) match {
+                case Right(value) => buf += ((field.name, value))
+                case _            =>
+              }
             }
-            .map(Json.Obj.apply)
+          }
+          new Right(Json.Obj(Chunk.fromArray(buf.result())))
+        }
       }
     }
 

diff --git a/zio-json/shared/src/main/scala-3/zio/json/macros.scala b/zio-json/shared/src/main/scala-3/zio/json/macros.scala
@@ -519,39 +519,31 @@ object DeriveJsonDecoder extends JsonDecoderDerivation(JsonCodecConfiguration.de
 }
 
 sealed class JsonEncoderDerivation(config: JsonCodecConfiguration) extends Derivation[JsonEncoder] { self =>
-   def join[A](ctx: CaseClass[Typeclass, A]): JsonEncoder[A] =
+  def join[A](ctx: CaseClass[Typeclass, A]): JsonEncoder[A] =
     if (ctx.params.isEmpty) caseObjectEncoder.narrow[A]
     else {
+      val nameTransform =
+        ctx.annotations.collectFirst { case jsonMemberNames(format) => format }.getOrElse(config.fieldNameMapping)
+      val explicitNulls = config.explicitNulls || ctx.annotations.exists(_.isInstanceOf[jsonExplicitNull])
+      val explicitEmptyCollections = ctx.annotations.collectFirst { case a: jsonExplicitEmptyCollections =>
+        a.encoding
+      }.getOrElse(config.explicitEmptyCollections.encoding)
+      val params = IArray.genericWrapArray(ctx.params.filterNot { param =>
+        param.annotations.collectFirst { case _: jsonExclude => () }.isDefined
+      }).toArray
       new JsonEncoder[A] {
-        private val nameTransform =
-          ctx.annotations.collectFirst { case jsonMemberNames(format) => format }.getOrElse(config.fieldNameMapping)
-        private val params = IArray.genericWrapArray(ctx.params.filterNot { param =>
-          param.annotations.collectFirst { case _: jsonExclude => () }.isDefined
-        }).toArray
-        private val explicitNulls = config.explicitNulls || ctx.annotations.exists(_.isInstanceOf[jsonExplicitNull])
-        private val explicitEmptyCollections = ctx.annotations.collectFirst { case a: jsonExplicitEmptyCollections =>
-          a.encoding
-        }.getOrElse(config.explicitEmptyCollections.encoding)
-        private lazy val fields: Array[FieldEncoder[Any, CaseClass.Param[JsonEncoder, A]]] = params.map { p =>
-          val name = p.annotations.collectFirst { case jsonField(name) => name }.getOrElse(nameTransform(p.label))
-          val withExplicitNulls = explicitNulls || p.annotations.exists(_.isInstanceOf[jsonExplicitNull])
-          val withExplicitEmptyCollections = p.annotations.collectFirst { case a: jsonExplicitEmptyCollections =>
-            a.encoding
-          }.getOrElse(explicitEmptyCollections)
+        private lazy val fields = params.map { p =>
           FieldEncoder(
-            p,
-            name,
-            p.typeclass.asInstanceOf[JsonEncoder[Any]],
-            withExplicitNulls = withExplicitNulls,
-            withExplicitEmptyCollections = withExplicitEmptyCollections
+            p = p,
+            name = p.annotations.collectFirst { case jsonField(name) => name }.getOrElse(nameTransform(p.label)),
+            encoder = p.typeclass.asInstanceOf[JsonEncoder[Any]],
+            withExplicitNulls = explicitNulls || p.annotations.exists(_.isInstanceOf[jsonExplicitNull]),
+            withExplicitEmptyCollections = p.annotations.collectFirst { case a: jsonExplicitEmptyCollections =>
+              a.encoding
+            }.getOrElse(explicitEmptyCollections)
           )
         }
 
-        override def isEmpty(a: A): Boolean = fields.forall { field =>
-          val paramValue = field.p.deref(a)
-          field.encoder.isEmpty(paramValue) || field.encoder.isNothing(paramValue)
-        }
-
         def unsafeEncode(a: A, indent: Option[Int], out: Write): Unit = {
           out.write('{')
           val indent_ = JsonEncoder.bump(indent)
@@ -562,45 +554,40 @@ sealed class JsonEncoderDerivation(config: JsonCodecConfiguration) extends Deriv
           while (idx < fields.length) {
             val field = fields(idx)
             idx += 1
-            val encoder = field.encoder
-            val p     = field.p.deref(a)
-            if ({
-              (field.flags: @switch) match {
-                case 0 => encoder.isEmpty(p) || encoder.isNothing(p)
-                case 1 => encoder.isNothing(p)
-                case 2 => encoder.isEmpty(p)
-                case _ => false
-              }
-            }) ()
+            val p = field.p.deref(a)
+            if (field.skip(p)) ()
             else {
               if (prevFields) {
                 out.write(',')
                 JsonEncoder.pad(indent_, out)
               } else prevFields = true
-              out.write(field.encodedName)
-              if (indent.isEmpty) out.write(':')
-              else out.write(" : ")
-              encoder.unsafeEncode(p, indent_, out)
+              if (indent.isEmpty) out.write(field.encodedName)
+              else out.write(field.prettyEncodedName)
+              field.encoder.unsafeEncode(p, indent_, out)
             }
           }
           JsonEncoder.pad(indent, out)
           out.write('}')
         }
 
-        override final def toJsonAST(a: A): Either[String, Json] =
-          fields
-            .foldLeft[Either[String, Chunk[(String, Json)]]](Right(Chunk.empty)) { case (c, field) =>
-              val param = field.p
-              val paramValue = param.deref(a)
-              field.encodeOrDefault(paramValue)(
-                () =>
-                  c.flatMap { chunk =>
-                    param.typeclass.toJsonAST(paramValue).map(value => chunk :+ field.name -> value)
-                  },
-                c
-              )
+        override final def toJsonAST(a: A): Either[String, Json] = {
+          val buf = Array.newBuilder[(String, Json)]
+          val fields = this.fields
+          var idx = 0
+          while (idx < fields.length) {
+            val field = fields(idx)
+            idx += 1
+            val p = field.p.deref(a)
+            if (field.skip(p)) ()
+            else {
+              field.encoder.toJsonAST(p) match {
+                case Right(value) => buf += ((field.name, value))
+                case _ =>
+              }
             }
-            .map(Json.Obj.apply)
+          }
+          new Right(Json.Obj(Chunk.fromArray(buf.result())))
+        }
       }
     }
 

diff --git a/zio-json/shared/src/main/scala/zio/json/internal/FieldEncoder.scala b/zio-json/shared/src/main/scala/zio/json/internal/FieldEncoder.scala
@@ -1,28 +1,22 @@
 package zio.json.internal
 
-import zio.Chunk
 import zio.json._
-import zio.json.ast.Json
 import scala.annotation.switch
 
 private[json] class FieldEncoder[T, P](
   val p: P,
-  val name: String,
   val encoder: JsonEncoder[T],
-  val flags: Int
+  val encodedName: String,
+  val prettyEncodedName: String,
+  val name: String,
+  private[this] val flags: Int
 ) {
-  val encodedName: String = JsonEncoder.string.encodeJson(name, None).toString
-
-  def encodeOrDefault(t: T)(
-    encode: () => Either[String, Chunk[(String, Json)]],
-    default: Either[String, Chunk[(String, Json)]]
-  ): Either[String, Chunk[(String, Json)]] =
-    (flags: @switch) match {
-      case 0 => if (encoder.isEmpty(t) || encoder.isNothing(t)) default else encode()
-      case 1 => if (encoder.isNothing(t)) default else encode()
-      case 2 => if (encoder.isEmpty(t)) default else encode()
-      case _ => encode()
-    }
+  def skip(t: T): Boolean = (flags: @switch) match {
+    case 0 => encoder.isEmpty(t) || encoder.isNothing(t)
+    case 1 => encoder.isNothing(t)
+    case 2 => encoder.isEmpty(t)
+    case _ => false
+  }
 }
 
 private[json] object FieldEncoder {
@@ -32,16 +26,20 @@ private[json] object FieldEncoder {
     encoder: JsonEncoder[T],
     withExplicitNulls: Boolean,
     withExplicitEmptyCollections: Boolean
-  ): FieldEncoder[T, P] =
+  ): FieldEncoder[T, P] = {
+    val encodedName = JsonEncoder.string.encodeJson(name, None).toString
     new FieldEncoder(
       p,
-      name,
-      encoder, {
+      encoder,
+      encodedName + ':',
+      encodedName + " : ",
+      name, {
         if (withExplicitNulls) {
           if (withExplicitEmptyCollections) 3 else 2
         } else {
           if (withExplicitEmptyCollections) 1 else 0
         }
       }
     )
+  }
 }
diff --git a/zio-json/shared/src/test/scala/zio/json/ConfigurableDeriveCodecSpec.scala b/zio-json/shared/src/test/scala/zio/json/ConfigurableDeriveCodecSpec.scala
@@ -258,7 +258,7 @@ object ConfigurableDeriveCodecSpec extends ZIOSpecDefault {
         case class EmptyObj(a: Empty)
         case class EmptySeq(b: Seq[Int])
 
-        val expectedStr      = """{}"""
+        val expectedStr      = """{"a":{}}"""
         val expectedEmptyObj = EmptyObj(Empty(None))
         val expectedEmptySeq = EmptySeq(Seq.empty)
 
@@ -271,7 +271,7 @@ object ConfigurableDeriveCodecSpec extends ZIOSpecDefault {
 
         assertTrue(
           expectedEmptyObj.toJson == expectedStr,
-          expectedEmptySeq.toJson == expectedStr,
+          expectedEmptySeq.toJson == "{}",
           expectedStr.fromJson[EmptyObj] == Right(expectedEmptyObj),
           expectedStr.fromJson[EmptySeq] == Right(expectedEmptySeq)
         )
@@ -357,7 +357,7 @@ object ConfigurableDeriveCodecSpec extends ZIOSpecDefault {
         case class EmptyObj(a: Empty)
         case class EmptySeq(b: Seq[Int])
 
-        val expectedJson     = Json.Obj()
+        val expectedJson     = Json.Obj(Chunk("a" -> Json.Obj.empty))
         val expectedEmptyObj = EmptyObj(Empty(None))
         val expectedEmptySeq = EmptySeq(Seq.empty)
 
@@ -370,7 +370,7 @@ object ConfigurableDeriveCodecSpec extends ZIOSpecDefault {
 
         assertTrue(
           expectedEmptyObj.toJsonAST == Right(expectedJson),
-          expectedEmptySeq.toJsonAST == Right(expectedJson),
+          expectedEmptySeq.toJsonAST == Right(Json.Obj()),
           expectedJson.as[EmptyObj] == Right(expectedEmptyObj),
           expectedJson.as[EmptySeq] == Right(expectedEmptySeq)
         )