Shorten $type tag used for discriminating sealed trait cases (#596)

Fixes #527

This PR shortens the `"$type": "foo.bar.Qux"` tag that uPickle uses to
distinguish between cases of a `sealed trait`, down to `"$type": "Qux"`
or `"$type": "bar.Qux"`. Because the `trait` is `sealed`, upickle is
able to look at all the other cases, and picks the shortest
partially-qualified name that is enough to distinguish all the different
cases. As mentioned in the original issue, this makes the serialization
format more compact, more robust against code changes (e.g. changes in
`package` name), and it homogenizes the serialization format of `sealed
trait` hierarchies and Scala 3 `enum`s (which already use a short
partially-qualified name as the `$type` tag)

Despite being binary compatible at the JVM level, this is a backwards
incompatible change that will need to go into uPickle 4.x. To ease the
rollout, the implementation is able to read both old-style fully
qualified `$type` tags as well as new-style partially-qualified
`$type`-tags, and whether new-style output or old-style output is
generated during serialization is controlled by a
`objectTypeKeyWriteFullyQualified` flag.

Someone migrating to uPickle 4.x from upickle 3.x can preserve the
existing read/write behavior by replacing `upickle.default` with a
`CustomPickler` with `objectTypeKeyWriteFullyQualified = true`, roll it
out across their system so their entire system can read both old-style
and new-style `$type` tags, and then flip individual components to
`objectTypeKeyWriteFullyQualified = false` (or back to
`upickle.default`). The system will work correctly with a mix of
old-style and new-style `$type` tags being generated, until everything
is on new-style `$type` tags.

build.sc

@@ -56,7 +56,9 @@ trait CommonPublishModule
     "3.1.2",
     "3.1.3",
     "3.1.4",
-//    "3.1.5", Not sure why this fails, maybe some artifacts were not properly published?
+    "3.2.0",
+    "3.3.0",
+    "3.3.1",
   )
   def isDotty = crossScalaVersion.startsWith("0") || crossScalaVersion.startsWith("3")
   def pomSettings = PomSettings(

upickle/core/src/upickle/core/Types.scala

@@ -169,11 +169,15 @@ trait Types{ types =>
     }
   }
   object TaggedReader{
-    class Leaf[T](private[upickle] override val tagKey: String, tagValue: String, r: Reader[T]) extends TaggedReader[T]{
+    class Leaf[T](private[upickle] override val tagKey: String,
+                  tagValue: String,
+                  tagShortValue: String,
+                  r: Reader[T]) extends TaggedReader[T]{
       @deprecated("Not used, left for binary compatibility")
-      def this(tag: String, r: Reader[T]) = this(Annotator.defaultTagKey, tag, r)
+      def this(tag: String, r: Reader[T]) = this(Annotator.defaultTagKey, tag, tag, r)
+      def this(tagKey: String, tagValue: String, r: Reader[T]) = this(tagKey, tagValue, tagValue, r)
 
-      def findReader(s: String) = if (s == tagValue) r else null
+      def findReader(s: String) = if (s == tagValue || s == tagShortValue) r else null
     }
     class Node[T](private[upickle] override val tagKey: String, rs: TaggedReader[_ <: T]*) extends TaggedReader[T]{
       @deprecated("Not used, left for binary compatibility")
@@ -201,7 +205,10 @@ trait Types{ types =>
     }
   }
   object TaggedWriter{
-    class Leaf[T](checker: Annotator.Checker, tagKey: String, tagValue: String, r: ObjectWriter[T]) extends TaggedWriter[T]{
+    class Leaf[T](checker: Annotator.Checker,
+                  tagKey: String,
+                  tagValue: String,
+                  r: ObjectWriter[T]) extends TaggedWriter[T]{
       @deprecated("Not used, left for binary compatibility")
       def this(checker: Annotator.Checker, tag: String, r: ObjectWriter[T]) =
         this(checker, Annotator.defaultTagKey, tag, r)
@@ -234,10 +241,12 @@ trait Types{ types =>
   trait TaggedReadWriter[T] extends ReadWriter[T] with TaggedReader[T] with TaggedWriter[T] with SimpleReader[T]{
     override def visitArray(length: Int, index: Int) = taggedArrayContext(this, index)
     override def visitObject(length: Int, jsonableKeys: Boolean, index: Int) = taggedObjectContext(this, index)
-
   }
+
   object TaggedReadWriter{
-    class Leaf[T](c: ClassTag[_], private[upickle] override val tagKey: String, tagValue: String, r: ObjectWriter[T] with Reader[T]) extends TaggedReadWriter[T]{
+    class Leaf[T](c: ClassTag[_],
+                  private[upickle] override val tagKey: String,
+                  tagValue: String, r: ObjectWriter[T] with Reader[T]) extends TaggedReadWriter[T]{
       @deprecated("Not used, left for binary compatibility")
       def this(c: ClassTag[_], tag: String, r: ObjectWriter[T] with Reader[T]) = this(c, Annotator.defaultTagKey, tag, r)
 
@@ -308,28 +317,37 @@ class CurrentlyDeriving[T]
  * like Scala 2 `case object`s do, so we instead use a `Checker.Val` to check
  * for `.equals` equality during writes to determine which tag to use.
  */
+@scala.annotation.nowarn("msg=deprecated")
 trait Annotator { this: Types =>
+
   @deprecated("Not used, left for binary compatibility")
   def annotate[V](rw: Reader[V], n: String): TaggedReader[V]
 
-  // Calling deprecated method to maintain binary compatibility
-  @annotation.nowarn("msg=deprecated")
-  def annotate[V](rw: Reader[V], key: String, value: String): TaggedReader[V] = annotate(rw, value)
+  @deprecated("Not used, left for binary compatibility")
+  def annotate[V](rw: Reader[V], key: String, value: String): TaggedReader[V] = annotate(rw, value, value)
+
+  def annotate[V](rw: Reader[V], key: String, value: String, shortValue: String): TaggedReader[V] = annotate(rw, value)
 
   @deprecated("Not used, left for binary compatibility")
   def annotate[V](rw: ObjectWriter[V], n: String, checker: Annotator.Checker): TaggedWriter[V]
 
-  // Calling deprecated method to maintain binary compatibility
-  @annotation.nowarn("msg=deprecated")
+  @deprecated("Not used, left for binary compatibility")
   def annotate[V](rw: ObjectWriter[V], key: String, value: String, checker: Annotator.Checker): TaggedWriter[V] =
+    annotate(rw, key, value, value, checker)
+
+  def annotate[V](rw: ObjectWriter[V], key: String, value: String, shortValue: String, checker: Annotator.Checker): TaggedWriter[V] =
     annotate(rw, value, checker)
 
+  @deprecated("Not used, left for binary compatibility")
   def annotate[V](rw: ObjectWriter[V], key: String, value: String)(implicit ct: ClassTag[V]): TaggedWriter[V] =
-    annotate(rw, key, value, Annotator.Checker.Cls(ct.runtimeClass))
+    annotate(rw, key, value, value)(ct)
+
+  def annotate[V](rw: ObjectWriter[V], key: String, value: String, shortValue: String)(implicit ct: ClassTag[V]): TaggedWriter[V] =
+    annotate(rw, key, value, shortValue, Annotator.Checker.Cls(ct.runtimeClass))
 
   @deprecated("Not used, left for binary compatibility")
   final def annotate[V](rw: ObjectWriter[V], n: String)(implicit ct: ClassTag[V]): TaggedWriter[V] =
-    annotate(rw, Annotator.defaultTagKey, n, Annotator.Checker.Cls(ct.runtimeClass))
+    annotate(rw, Annotator.defaultTagKey, n, n, Annotator.Checker.Cls(ct.runtimeClass))
 }
 object Annotator{
   def defaultTagKey = "$type"

upickle/implicits/src-2/upickle/implicits/internal/Macros.scala

@@ -223,9 +223,31 @@ object Macros {
           (_: c.Symbol).name.toString,
           fail(tpe, _),
         )
-        val tagValue = customKey(tpe.typeSymbol).getOrElse(TypeName(tpe.typeSymbol.fullName).decodedName.toString)
 
-        q"${c.prefix}.annotate($derived, $tagKey, $tagValue)"
+
+        val segments =
+          sealedParents
+            .flatMap(_.asClass.knownDirectSubclasses)
+            .map(_.fullName.split('.'))
+
+        // -1 because even if there is only one subclass, and so no name segments
+        // are needed to differentiate between them, we want to keep at least
+        // the rightmost name segment
+        val identicalSegmentCount = Range(0, segments.map(_.length).max - 1)
+          .takeWhile(i => segments.map(_.lift(i)).distinct.size == 1)
+          .length
+
+        val tagValue = customKey(tpe.typeSymbol)
+          .getOrElse(TypeName(tpe.typeSymbol.fullName).decodedName.toString)
+
+        val shortTagValue = customKey(tpe.typeSymbol)
+          .getOrElse(
+            TypeName(
+              tpe.typeSymbol.fullName.split('.').drop(identicalSegmentCount).mkString(".")
+            ).decodedName.toString
+          )
+
+        q"${c.prefix}.annotate($derived, $tagKey, $tagValue, $shortTagValue)"
       }
     }
 

upickle/implicits/src-3/upickle/implicits/Readers.scala

@@ -76,9 +76,19 @@ trait ReadersVersionSpecific
       }
 
       inline if macros.isSingleton[T] then
-        annotate[T](SingletonReader[T](macros.getSingleton[T]), macros.tagKey[T], macros.tagName[T])
+        annotate[T](
+          SingletonReader[T](macros.getSingleton[T]),
+          macros.tagKey[T],
+          macros.tagName[T],
+          macros.shortTagName[T]
+        )
       else if macros.isMemberOfSealedHierarchy[T] then
-        annotate[T](reader, macros.tagKey[T], macros.tagName[T])
+        annotate[T](
+          reader,
+          macros.tagKey[T],
+          macros.tagName[T],
+          macros.shortTagName[T],
+        )
       else reader
 
     case m: Mirror.SumOf[T] =>
@@ -101,7 +111,8 @@ trait ReadersVersionSpecific
     val actual = implicitly[Reader[V]].asInstanceOf[TaggedReader[T]]
     val tagKey = macros.tagKey[T]
     val tagName = macros.tagName[T]
-    new TaggedReader.Leaf(tagKey, tagName, actual.findReader(tagName))
+    val shortTagName = macros.shortTagName[T]
+    new TaggedReader.Leaf(tagKey, tagName, shortTagName, actual.findReader(tagName))
   }
 
   // see comment in MacroImplicits as to why Dotty's extension methods aren't used here

upickle/implicits/src-3/upickle/implicits/Writers.scala

@@ -48,13 +48,15 @@ trait WritersVersionSpecific
           SingletonWriter[T](null.asInstanceOf[T]),
           macros.tagKey[T],
           macros.tagName[T],
+          macros.shortTagName[T],
           Annotator.Checker.Val(macros.getSingleton[T]),
         )
       else if macros.isMemberOfSealedHierarchy[T] then
         annotate[T](
           writer,
           macros.tagKey[T],
           macros.tagName[T],
+          macros.shortTagName[T],
           Annotator.Checker.Cls(implicitly[ClassTag[T]].runtimeClass),
         )
       else writer

upickle/implicits/src-3/upickle/implicits/macros.scala

@@ -199,6 +199,9 @@ def tagKeyImpl[T](using Quotes, Type[T]): Expr[String] =
 
 inline def tagName[T]: String = ${ tagNameImpl[T] }
 def tagNameImpl[T](using Quotes, Type[T]): Expr[String] =
+  tagNameImpl0(identity)
+
+def tagNameImpl0[T](transform: String => String)(using Quotes, Type[T]): Expr[String] =
   import quotes.reflect._
 
   val sym = TypeTree.of[T].symbol
@@ -228,9 +231,24 @@ def tagNameImpl[T](using Quotes, Type[T]): Expr[String] =
           case AppliedType(TypeRef(prefix, value), _) => value
         }
       } else {
-        TypeTree.of[T].tpe.typeSymbol.fullName.filter(_ != '$')
+        transform(TypeTree.of[T].tpe.typeSymbol.fullName.filter(_ != '$'))
       }
   )
+inline def shortTagName[T]: String = ${ shortTagNameImpl[T] }
+def shortTagNameImpl[T](using Quotes, Type[T]): Expr[String] =
+  import quotes.reflect._
+  val sym = TypeTree.of[T].symbol
+  val segments = TypeRepr.of[T].baseClasses
+    .filter(_.flags.is(Flags.Sealed))
+    .flatMap(_.children)
+    .filter(_.flags.is(Flags.Case))
+    .map(_.fullName.split('.'))
+
+  val identicalSegmentCount = Range(0, segments.map(_.length).max - 1)
+    .takeWhile(i => segments.map(_.lift(i)).distinct.size == 1)
+    .length
+
+  tagNameImpl0(_.split('.').drop(identicalSegmentCount).mkString("."))
 
 inline def isSingleton[T]: Boolean = ${ isSingletonImpl[T] }
 def isSingletonImpl[T](using Quotes, Type[T]): Expr[Boolean] =

upickle/implicits/src/upickle/implicits/MacrosCommon.scala

@@ -3,11 +3,42 @@ package upickle.implicits
 // Common things for derivation
 trait MacrosCommon {
 
+  /**
+   * Whether to use the fully-qualified name of `case class`es and `case object`s which
+   * are part of `sealed trait` hierarchies when serializing them and writing their `$type`
+   * key. Defaults to `false`, so `$type` key uses the shortest partially-qualified name.
+   * Can be set to `true` to use their fully-qualified name.
+   */
+  def objectTypeKeyWriteFullyQualified: Boolean = false
+
+  /**
+   * Whether or not to write `case class` keys which match their default values.
+   * Defaults to `false`, allowing those keys to be omitted. Can be set to `true`
+   * to always write field values even if they are equal to the default
+   */
   def serializeDefaults: Boolean = false
 
+  /**
+   * Transform dictionary keys when writing `case class`es when reading. Can
+   * be overriden to provide custom mappings between Scala field names and JSON
+   * field names. Needs to be kept in sync with [[objectAttributeKeyWriteMap]]
+   *
+   * This customizes the mapping across all `case class`es fields handled by this
+   * upickle instance. This can be customized on a field-by-field basis using the
+   * [[upickle.implicits.key]] annotation on the `case class` field
+   */
   def objectAttributeKeyReadMap(s: CharSequence): CharSequence = s
   def objectAttributeKeyWriteMap(s: CharSequence): CharSequence = s
 
+  /**
+   * Transforms the value of the `$type` field when writing `sealed trait`s,
+   * to allow custom mapping between the `case class` name and the `$type` field
+   * in the generated JSON. Must be kept in sync with [[objectTypeKeyWriteMap]].
+   *
+   * * This customizes the mapping across all `case class`es fields handled by this
+   * * upickle instance. This can be customized on a per-`sealed trait` basis using the
+   * * [[upickle.implicits.key]] annotation on the `case class`
+   */
   def objectTypeKeyReadMap(s: CharSequence): CharSequence = s
   def objectTypeKeyWriteMap(s: CharSequence): CharSequence = s
 

upickle/src/upickle/Api.scala

@@ -245,20 +245,27 @@ object default extends AttributeTagged{
  */
 object legacy extends LegacyApi
 trait LegacyApi extends Api with Annotator{
-  override def annotate[V](rw: Reader[V], key: String, value: String) = {
-    new TaggedReader.Leaf[V](key, value, rw)
+  @deprecated("Not used, left for binary compatibility")
+  override def annotate[V](rw: Reader[V], key: String, value: String) =  annotate(rw, key, value, value)
+
+  override def annotate[V](rw: Reader[V], key: String, value: String, shortValue: String) = {
+    new TaggedReader.Leaf[V](key, value, shortValue, rw)
   }
 
   @deprecated("Not used, left for binary compatibility")
   override final def annotate[V](rw: Reader[V], n: String) =
-    annotate(rw, Annotator.defaultTagKey, n)
+    annotate(rw, Annotator.defaultTagKey, n, n)
 
+  @deprecated("Not used, left for binary compatibility")
   override def annotate[V](rw: ObjectWriter[V], key: String, value: String, checker: Annotator.Checker): TaggedWriter[V] =
-    new TaggedWriter.Leaf[V](checker, key, value, rw)
+    annotate(rw, key, value, value, checker)
+
+  override def annotate[V](rw: ObjectWriter[V], key: String, value: String, shortValue: String, checker: Annotator.Checker): TaggedWriter[V] =
+    new TaggedWriter.Leaf[V](checker, key, if (objectTypeKeyWriteFullyQualified) value else shortValue, rw)
 
   @deprecated("Not used, left for binary compatibility")
   override final def annotate[V](rw: ObjectWriter[V], n: String, checker: Annotator.Checker): TaggedWriter[V] =
-    annotate(rw, Annotator.defaultTagKey, n, checker)
+    annotate(rw, Annotator.defaultTagKey, n, n, checker)
 
   def taggedExpectedMsg = "expected sequence"
   sealed trait TaggedReaderState
@@ -318,19 +325,28 @@ trait AttributeTagged extends Api with Annotator{
   @deprecated("Not used, left for binary compatibility")
   def tagName = Annotator.defaultTagKey
 
-  override def annotate[V](rw: Reader[V], key: String, value: String) = {
-    new TaggedReader.Leaf[V](key, value, rw)
+  @deprecated("Not used, left for binary compatibility")
+  override def annotate[V](rw: Reader[V], key: String, value: String) = annotate(rw, key, value, value)
+
+  override def annotate[V](rw: Reader[V], key: String, value: String, shortValue: String) = {
+    new TaggedReader.Leaf[V](key, value, shortValue, rw)
   }
   @deprecated("Not used, left for binary compatibility")
   override final def annotate[V](rw: Reader[V], n: String) =
-    annotate(rw, Annotator.defaultTagKey, n)
+    annotate(rw, Annotator.defaultTagKey, n, n)
 
+  @deprecated("Not used, left for binary compatibility")
   override def annotate[V](rw: ObjectWriter[V], key: String, value: String, checker: Annotator.Checker): TaggedWriter[V] = {
-    new TaggedWriter.Leaf[V](checker, key, value, rw)
+    annotate(rw, key, value, value, checker)
   }
+
+  override def annotate[V](rw: ObjectWriter[V], key: String, value: String, shortValue: String, checker: Annotator.Checker): TaggedWriter[V] = {
+    new TaggedWriter.Leaf[V](checker, key, if (objectTypeKeyWriteFullyQualified) value else shortValue, rw)
+  }
+
   @deprecated("Not used, left for binary compatibility")
   override final def annotate[V](rw: ObjectWriter[V], n: String, checker: Annotator.Checker): TaggedWriter[V] =
-    annotate(rw, Annotator.defaultTagKey, n, checker)
+    annotate(rw, Annotator.defaultTagKey, n, n, checker)
 
   def taggedExpectedMsg = "expected dictionary"
   private def isTagName(tagKey: String, i: Any) = i match{

upickle/test/src-2/upickle/AdvancedTestsScala2Only.scala

@@ -20,16 +20,16 @@ object AdvancedTestsScala2Only extends TestSuite {
 
     test("gadt"){
       test("simple"){
-        test - rw(Gadt.Exists("hello"): Gadt[_], """{"$type":"upickle.Gadt.Exists","path":"hello"}""")
-        test - rw(Gadt.IsDir(" "): Gadt[_], """{"$type":"upickle.Gadt.IsDir","path":" "}""")
-        test - rw(Gadt.ReadBytes("\""): Gadt[_], """{"$type":"upickle.Gadt.ReadBytes","path":"\""}""")
-        test - rw(Gadt.CopyOver(Seq(1, 2, 3), ""): Gadt[_], """{"$type":"upickle.Gadt.CopyOver","src":[1,2,3],"path":""}""")
+        test - rw(Gadt.Exists("hello"): Gadt[_], """{"$type":"Exists","path":"hello"}""")
+        test - rw(Gadt.IsDir(" "): Gadt[_], """{"$type":"IsDir","path":" "}""")
+        test - rw(Gadt.ReadBytes("\""): Gadt[_], """{"$type":"ReadBytes","path":"\""}""")
+        test - rw(Gadt.CopyOver(Seq(1, 2, 3), ""): Gadt[_], """{"$type":"CopyOver","src":[1,2,3],"path":""}""")
       }
       test("partial"){
-        test - rw(Gadt2.Exists("hello"): Gadt2[_, String], """{"$type":"upickle.Gadt2.Exists","v":"hello"}""")
-        test - rw(Gadt2.IsDir(123): Gadt2[_, Int], """{"$type":"upickle.Gadt2.IsDir","v":123}""")
-        test - rw(Gadt2.ReadBytes('h'): Gadt2[_, Char], """{"$type":"upickle.Gadt2.ReadBytes","v":"h"}""")
-        test - rw(Gadt2.CopyOver(Seq(1, 2, 3), ""): Gadt2[_, Unit], """{"$type":"upickle.Gadt2.CopyOver","src":[1,2,3],"v":""}""")
+        test - rw(Gadt2.Exists("hello"): Gadt2[_, String], """{"$type":"Exists","v":"hello"}""")
+        test - rw(Gadt2.IsDir(123): Gadt2[_, Int], """{"$type":"IsDir","v":123}""")
+        test - rw(Gadt2.ReadBytes('h'): Gadt2[_, Char], """{"$type":"ReadBytes","v":"h"}""")
+        test - rw(Gadt2.CopyOver(Seq(1, 2, 3), ""): Gadt2[_, Unit], """{"$type":"CopyOver","src":[1,2,3],"v":""}""")
       }
     }