Add override def allowUnknownKeys/@allowUnknownKeys(b: Boolean) t…

…o allow treatment of unknown keys to be configurable (#548) Previously, they were always allowed. Now you can opt-in to turn them into an error on a per-case-class basis via an annotation, or globally via an override (which you can then disable on a per-case-class basis using the same annotation) Fixes #537
com-lihaoyi · Jan 11, 2024 · 91261bd · 91261bd
1 parent 04b34f7
commit 91261bd
Show file tree

Hide file tree

Showing 7 changed files with 108 additions and 7 deletions.
diff --git a/upickle/implicits/src-2/upickle/implicits/internal/Macros.scala b/upickle/implicits/src-2/upickle/implicits/internal/Macros.scala
@@ -242,6 +242,13 @@ object Macros {
                   hasDefaults: Seq[Boolean],
                   targetType: c.Type,
                   varargs: Boolean) = {
+      val allowUnknownKeysAnnotation = targetType.typeSymbol
+        .annotations
+        .find(_.tpe == typeOf[upickle.implicits.allowUnknownKeys])
+        .flatMap(_.scalaArgs.headOption)
+        .map { case Literal(Constant(b)) => b.asInstanceOf[Boolean] }
+        .headOption
+
       val defaults = deriveDefaults(companion, hasDefaults)
 
       val localReaders = for (i <- rawArgs.indices) yield TermName("localReader" + i)
@@ -271,7 +278,18 @@ object Macros {
                   for(i <- mappedArgs.indices)
                   yield cq"${mappedArgs(i)} => $i"
                 }
-                case _ => -1
+                case _ =>
+                  ${
+                    allowUnknownKeysAnnotation match {
+                      case None =>
+                        q"""
+                           if (${c.prefix}.allowUnknownKeys) -1
+                           else throw new _root_.upickle.core.Abort("Unknown Key: " + s.toString)
+                         """
+                      case Some(false) => q"""throw new _root_.upickle.core.Abort("Unknown Key: " + s.toString)"""
+                      case Some(true) => q"-1"
+                    }
+                  }
               }
             }
 

diff --git a/upickle/implicits/src-3/upickle/implicits/Readers.scala b/upickle/implicits/src-3/upickle/implicits/Readers.scala
@@ -12,7 +12,12 @@ trait ReadersVersionSpecific
     with Annotator
     with CaseClassReadWriters:
 
-  abstract class CaseClassReadereader[T](paramCount: Int, missingKeyCount: Long) extends CaseClassReader[T] {
+  abstract class CaseClassReadereader[T](paramCount: Int,
+                                         missingKeyCount: Long,
+                                         allowUnknownKeys: Boolean) extends CaseClassReader[T] {
+    // Bincompat stub
+    def this(paramCount: Int, missingKeyCount: Long) = this(paramCount, missingKeyCount, true)
+
     def visitors0: Product
     lazy val visitors = visitors0
     def fromProduct(p: Product): T
@@ -31,6 +36,9 @@ trait ReadersVersionSpecific
       def visitKeyValue(v: Any): Unit =
         val k = objectAttributeKeyReadMap(v.toString).toString
         currentIndex = keyToIndex(k)
+        if (currentIndex == -1 && !allowUnknownKeys) {
+          throw new upickle.core.Abort("Unknown Key: " + k.toString)
+        }
 
       def visitEnd(index: Int): T =
         storeDefaults(this)
@@ -55,7 +63,11 @@ trait ReadersVersionSpecific
 
   inline def macroR[T](using m: Mirror.Of[T]): Reader[T] = inline m match {
     case m: Mirror.ProductOf[T] =>
-      val reader = new CaseClassReadereader[T](macros.paramsCount[T], macros.checkErrorMissingKeysCount[T]()){
+      val reader = new CaseClassReadereader[T](
+        macros.paramsCount[T],
+        macros.checkErrorMissingKeysCount[T](),
+        macros.extractIgnoreUnknownKeys[T]().headOption.getOrElse(this.allowUnknownKeys)
+      ){
         override def visitors0 = compiletime.summonAll[Tuple.Map[m.MirroredElemTypes, Reader]]
         override def fromProduct(p: Product): T = m.fromProduct(p)
         override def keyToIndex(x: String): Int = macros.keyToIndex[T](x)

diff --git a/upickle/implicits/src-3/upickle/implicits/macros.scala b/upickle/implicits/src-3/upickle/implicits/macros.scala
@@ -41,6 +41,18 @@ def extractKey[A](using Quotes)(sym: quotes.reflect.Symbol): Option[String] =
     .find(_.tpe =:= TypeRepr.of[upickle.implicits.key])
     .map{case Apply(_, Literal(StringConstant(s)) :: Nil) => s}
 
+inline def extractIgnoreUnknownKeys[T](): List[Boolean] = ${extractIgnoreUnknownKeysImpl[T]}
+def extractIgnoreUnknownKeysImpl[T](using Quotes, Type[T]): Expr[List[Boolean]] =
+  import quotes.reflect._
+  Expr.ofList(
+    TypeRepr.of[T].typeSymbol
+      .annotations
+      .find(_.tpe =:= TypeRepr.of[upickle.implicits.allowUnknownKeys])
+      .map{case Apply(_, Literal(BooleanConstant(b)) :: Nil) => b}
+      .map(Expr(_))
+      .toList
+    )
+
 inline def paramsCount[T]: Int = ${paramsCountImpl[T]}
 def paramsCountImpl[T](using Quotes, Type[T]) = {
   Expr(fieldLabelsImpl0[T].size)

diff --git a/upickle/implicits/src/upickle/implicits/CaseClassReadWriters.scala b/upickle/implicits/src/upickle/implicits/CaseClassReadWriters.scala
@@ -11,7 +11,7 @@ import upickle.core.{Abort, AbortException, ArrVisitor, NoOpVisitor, ObjVisitor,
 * package to form the public API1
 */
 trait CaseClassReadWriters extends upickle.core.Types{
-
+  def allowUnknownKeys: Boolean = true
   abstract class CaseClassReader[V] extends SimpleReader[V] {
     override def expectedMsg = "expected dictionary"
 

diff --git a/upickle/implicits/src/upickle/implicits/key.scala b/upickle/implicits/src/upickle/implicits/key.scala
@@ -2,4 +2,5 @@ package upickle.implicits
 
 import scala.annotation.StaticAnnotation
 
-case class key(s: String) extends StaticAnnotation
+case class key(s: String) extends StaticAnnotation
+case class allowUnknownKeys(b: Boolean) extends StaticAnnotation
diff --git a/upickle/src/upickle/Api.scala b/upickle/src/upickle/Api.scala
@@ -149,6 +149,14 @@ trait Api
     override def isJsonDictKey = true
     def write0[R](out: Visitor[_, R], v: T): R = readwriter.write0(out, v)
   }
+
+  /**
+   * Configure whether you want upickle to skip unknown keys during de-serialization
+   * of `case class`es. Can be overriden for the entire serializer via `override def`, and
+   * further overriden for individual `case class`es via the annotation
+   * `@upickle.implicits.allowUnknownKeys(b: Boolean)`
+   */
+  override def allowUnknownKeys: Boolean = true
   // End Api
 }
 

diff --git a/upickle/test/src/upickle/MacroTests.scala b/upickle/test/src/upickle/MacroTests.scala
@@ -78,6 +78,28 @@ object GenericIssue545{
   implicit def apiResultRw[T: upickle.default.ReadWriter]: upickle.default.ReadWriter[ApiResult[T]] = upickle.default.macroRW[ApiResult[T]]
 }
 
+object UnknownKeys{
+  case class Default(id: Int, name: String)
+
+  implicit val defaultRw: upickle.default.ReadWriter[Default] = upickle.default.macroRW[Default]
+  implicit val defaultRw2: DisallowPickler.ReadWriter[Default] = DisallowPickler.macroRW[Default]
+
+  @upickle.implicits.allowUnknownKeys(false)
+  case class DisAllow(id: Int, name: String)
+
+  implicit val disAllowRw: upickle.default.ReadWriter[DisAllow] = upickle.default.macroRW[DisAllow]
+  implicit val disAllowRw2: DisallowPickler.ReadWriter[DisAllow] = DisallowPickler.macroRW[DisAllow]
+
+  @upickle.implicits.allowUnknownKeys(true)
+  case class Allow(id: Int, name: String)
+
+  implicit val allowRw: upickle.default.ReadWriter[Allow] = upickle.default.macroRW[Allow]
+  implicit val allowRw2: DisallowPickler.ReadWriter[Allow] = DisallowPickler.macroRW[Allow]
+
+  object DisallowPickler extends upickle.AttributeTagged {
+    override def allowUnknownKeys = false
+  }
+}
 object MacroTests extends TestSuite {
 
   // Doesn't work :(
@@ -578,13 +600,41 @@ object MacroTests extends TestSuite {
     test("genericIssue545"){
       // Make sure case class default values are properly picked up for
       // generic case classes in Scala 3
-      import upickle.implicits.key
-
       upickle.default.read[GenericIssue545.Person]("{\"id\":1}") ==>
         GenericIssue545.Person(1)
 
       upickle.default.read[GenericIssue545.ApiResult[GenericIssue545.Person]]("{\"total_count\": 10}") ==>
         GenericIssue545.ApiResult[GenericIssue545.Person](None, 10)
     }
+
+    test("unknownKeys"){
+      // For upickle default, we defualt to allowing unknown keys, and explicitly annotating
+      // `@allowUnknownKeys(true)` does nothing, but `@allowUnknownKeys(false)` makes unknown
+      // keys an error (just for the annotated class)
+      upickle.default.read[UnknownKeys.Default]("""{"id":1, "name":"x", "omg": "wtf"}""") ==>
+        UnknownKeys.Default(1, "x")
+
+      upickle.default.read[UnknownKeys.Allow]("""{"id":1, "name":"x", "omg": "wtf"}""") ==>
+        UnknownKeys.Allow(1, "x")
+
+      intercept[upickle.core.AbortException]{
+        upickle.default.read[UnknownKeys.DisAllow]("""{"id":1, "name":"x", "omg": "wtf"}""")
+      }
+
+      // If the upickle API sets `override def allowUnknownKeys = false`, we default to treating unknown keys
+      // as an error, `@allowUnknownKeys(false)` does nothing, but `@allowUnknownKeys(true)` makes unknown
+      // keys get ignored (just for the annotated class)
+      intercept[upickle.core.AbortException] {
+        UnknownKeys.DisallowPickler.read[UnknownKeys.Default]("""{"id":1, "name":"x", "omg": "wtf"}""") ==>
+          UnknownKeys.Default(1, "x")
+      }
+
+      UnknownKeys.DisallowPickler.read[UnknownKeys.Allow]("""{"id":1, "name":"x", "omg": "wtf"}""") ==>
+        UnknownKeys.Allow(1, "x")
+
+      intercept[upickle.core.AbortException]{
+        UnknownKeys.DisallowPickler.read[UnknownKeys.DisAllow]("""{"id":1, "name":"x", "omg": "wtf"}""")
+      }
+    }
   }
 }