Scala 3.3/Play 2.9 compatibility (#121)

* play-json, upickle, sourcecode, Scala

* Scala 3.3 and play-json 2.10 new RCs, bump sbt

* Scala 3.3.1, play-json 2.10.1, sbt/plugin updates, address some deprecations

* Comment on stack as List

* Target 1.9.0

* fix CONTRIBUTING link in PR template

* upgrade dependencies (where bin compat)

.github/PULL_REQUEST_TEMPLATE.md

@@ -15,7 +15,7 @@ _Put an `x` in the boxes that apply_
 
 _Put an `x` in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code._
 
-- [ ] I have read the [CONTRIBUTING](https://github.com/Optum/oss-template/blob/main/CONTRIBUTING.md) doc
+- [ ] I have read the [CONTRIBUTING](https://github.com/rallyhealth/weePickle/blob/v1/CONTRIBUTING.md) doc
 - [ ] I have added tests that prove my fix is effective or that my feature works
 - [ ] I have added necessary documentation (if appropriate)
 - [ ] Any dependent changes have been merged and published in downstream modules

bench/src/main/scala/bench/BlackholeUVisitor.scala

@@ -16,7 +16,7 @@ class BlackholeUVisitor(bh: Blackhole) extends Visitor[Any, Null] {
     }
   }
 
-  override def visitObject(length: Int, index: Int): ObjVisitor[Any, Null] = new ObjVisitor[Any, Null] {
+  override def visitObject(length: Int, jsonableKeys: Boolean, index: Int): ObjVisitor[Any, Null] = new ObjVisitor[Any, Null] {
     override def visitKey(index: Int): Visitor[_, _] = {
       BlackholeUVisitor.this
     }

bench/src/main/scala/bench/ScalaVersionBench.scala

@@ -335,7 +335,7 @@ object HandrolledFlatPrimitives {
   implicit val writer: default.Reader[HandrolledFlatPrimitives] = upickle.default.macroR[HandrolledFlatPrimitives]
   implicit val reader: default.Writer[HandrolledFlatPrimitives] = new upickle.default.Writer[HandrolledFlatPrimitives] {
     override def write0[V](out: core.Visitor[_, V], v: HandrolledFlatPrimitives): V = {
-      val obj = out.visitObject(6, -1).narrow
+      val obj = out.visitObject(6, true, -1).narrow
 
       obj.visitKeyValue(obj.visitKey(-1).visitString("i", -1))
       obj.visitValue(obj.subVisitor.visitInt32(Int.MinValue, -1), -1)

build.sbt

@@ -9,27 +9,27 @@ crossScalaVersions := Nil // crossScalaVersions must be set to Nil on the aggreg
 lazy val bench = project
   .dependsOn(
     `weepickle-tests` % "compile;test",
-    `weejson-upickle`,
   )
   .enablePlugins(JmhPlugin)
   .settings(
     noPublish,
     crossScalaVersions := Seq(scala213, scala3),
     libraryDependencies ++= Seq(
-      "com.fasterxml.jackson.core" % "jackson-databind" % "2.13.0",
-      "com.fasterxml.jackson.dataformat" % "jackson-dataformat-smile" % "2.13.0",
-      "com.fasterxml.jackson.module" %% "jackson-module-scala" % "2.13.0",
-      "io.circe" %% "circe-generic" % "0.14.1",
-      "io.circe" %% "circe-parser" % "0.14.1",
-      "org.msgpack" % "jackson-dataformat-msgpack" % "0.9.0",
-      "com.lihaoyi" %% "sourcecode" % "0.2.7",
+      "com.lihaoyi" %% "upickle" % "3.1.3", // need the latest Scala 3 support
+      "com.fasterxml.jackson.core" % "jackson-databind" % "2.15.2",
+      "com.fasterxml.jackson.dataformat" % "jackson-dataformat-smile" % "2.15.2",
+      "com.fasterxml.jackson.module" %% "jackson-module-scala" % "2.15.2",
+      "io.circe" %% "circe-generic" % "0.14.6",
+      "io.circe" %% "circe-parser" % "0.14.6",
+      "org.msgpack" % "jackson-dataformat-msgpack" % "0.9.6",
+      "com.lihaoyi" %% "sourcecode" % "0.3.1",
     )
   )
 
 lazy val `weepickle-core` = project
   .settings(
     libraryDependencies ++= Seq(
-      "org.scala-lang.modules" %% "scala-collection-compat" % (if (scalaBinaryVersion.value == "3") "2.5.0" else "2.4.3")
+      "org.scala-lang.modules" %% "scala-collection-compat" % "2.11.0"
     )
   )
 
@@ -130,7 +130,7 @@ lazy val `weejson-jackson` = project
   .dependsOn(`weepickle-core`)
   .settings(
     libraryDependencies ++= Seq(
-      "com.fasterxml.jackson.core" % "jackson-core" % "2.13.0"
+      "com.fasterxml.jackson.core" % "jackson-core" % "2.15.2"
     ),
     mimaBinaryIssueFilters ++= Seq(
       ProblemFilters.exclude[DirectMissingMethodProblem]("com.rallyhealth.weejson.v1.jackson.TextBufferCharSequence.isEmpty")
@@ -141,32 +141,36 @@ lazy val `weejson-circe` = project
   .dependsOn(weejson)
   .settings(
     libraryDependencies ++= Seq(
-      "io.circe" %% "circe-parser" % (if (scalaBinaryVersion.value == "2.11") "0.11.2" else "0.14.1")
+      "io.circe" %% "circe-parser" % (if (scalaBinaryVersion.value == "2.11") "0.11.2" else "0.14.6")
     )
   )
 
 lazy val `weejson-json4s` = project
   .dependsOn(weejson)
   .settings(
     libraryDependencies ++= Seq(
-      "org.json4s" %% "json4s-ast" % (if (scalaBinaryVersion.value == "3") "4.0.3" else "3.6.10"),
+      // 4.x.x breaks binary compatibility, consider for weePickle v2
+      "org.json4s" %% "json4s-ast" % (if (scalaBinaryVersion.value == "3") "4.0.6" else "3.6.12"),
     )
   )
 lazy val `weejson-argonaut` = project
   .dependsOn(weejson)
   .settings(
+    Compile / unmanagedSourceDirectories ++= (
+      if (scalaBinaryVersion.value == "2.11") Nil else Seq((Compile / sourceDirectory).value / "scala-2.12+")
+    ),
     libraryDependencies ++= Seq(
-      "io.argonaut" %% "argonaut" % (if (scalaBinaryVersion.value == "3") "6.3.7" else "6.2.5") ,
+      "io.argonaut" %% "argonaut" % (if (scalaBinaryVersion.value == "2.11") "6.2.6" else "6.3.9"),
     )
   )
 
 // We need one project for 'weepickle-testing' to dependOn that is available on all Scala versions.
-// This "base" project uses Play 2.7 when on any version of Scala 2 and Play 2.10 when on Scala 3.
+// This "base" project uses play-json 2.7 when on Scala 2.11 and play-json 2.10 when on later Scala versions.
 lazy val `weejson-play-base` = (project in file("weejson-play"))
   .dependsOn(weepickle)
   .settings(
     libraryDependencies ++= Seq(
-      "com.typesafe.play" %% "play-json" % (if (scalaBinaryVersion.value == "3") "2.10.0-RC5" else "2.7.4"),
+      "com.typesafe.play" %% "play-json" % (if (scalaBinaryVersion.value == "2.11") "2.7.4" else "2.10.1"),
     ),
     noPublish
   )
@@ -191,17 +195,27 @@ lazy val `weejson-play25` = playProject("2.5.19", Seq(scala211))
 
 lazy val `weejson-play27` = playProject("2.7.4", supportedScala2Versions)
 
-lazy val `weejson-play28` = playProject("2.8.1", Seq(scala213))
+lazy val `weejson-play28` = playProject("2.8.2", Seq(scala213))
 
-lazy val `weejson-play29` = playProject("2.9.2", Seq(scala213))
+// Using play-json version numbering, which is slightly confusing. Play 2.9 uses play-json 2.10 for some reason,
+// and no version of Play actually uses play-json 2.9
+lazy val `weejson-play29` = playProject("2.9.4", Seq(scala213))
 
-lazy val `weejson-play210` = playProject("2.10.0-RC5", Seq(scala3))
+lazy val `weejson-play210` = playProject("2.10.1", Seq(scala213, scala3)).settings(
+  mimaPreviousArtifacts := {
+    if (VersionNumber(version.value).matchesSemVer(SemanticSelector("<1.9.0")) && scalaBinaryVersion.value == "2.13")
+      Set.empty // TODO: remove once there's a previous Scala 2.13 artifact
+    else
+      mimaPreviousArtifacts.value
+  }
+)
 
 lazy val `weejson-upickle` = project
   .dependsOn(weepickle)
   .settings(
     libraryDependencies ++= Seq(
-      "com.lihaoyi" %% "upickle" % "1.4.2",
+      // 2.x.x/3.x.x break binary compatibility, consider for weePickle v2
+      "com.lihaoyi" %% "upickle" % "1.6.0",
     ),
     mimaPreviousArtifacts := {
       if (VersionNumber(version.value).matchesSemVer(SemanticSelector("<1.6.0")))
@@ -215,7 +229,7 @@ lazy val weeyaml = project
   .dependsOn(`weejson-jackson`)
   .settings(
     libraryDependencies ++= Seq(
-      "com.fasterxml.jackson.dataformat" % "jackson-dataformat-yaml" % "2.13.0",
+      "com.fasterxml.jackson.dataformat" % "jackson-dataformat-yaml" % "2.15.2",
     )
   )
 
@@ -226,7 +240,7 @@ lazy val weexml = project
   )
   .settings(
     libraryDependencies ++= Seq(
-      "com.fasterxml.jackson.dataformat" % "jackson-dataformat-xml" % "2.13.0",
+      "com.fasterxml.jackson.dataformat" % "jackson-dataformat-xml" % "2.15.2",
     )
   )
 

project/WeePicklePlugin.scala

@@ -13,9 +13,9 @@ object WeePicklePlugin extends AutoPlugin {
   object autoImport {
 
     val scala211 = "2.11.12"
-    val scala212 = "2.12.12"
-    val scala213 = "2.13.7"
-    val scala3 = "3.1.0"
+    val scala212 = "2.12.18"
+    val scala213 = "2.13.12"
+    val scala3 = "3.3.1"
     val supportedScala2Versions = Seq(scala211, scala212, scala213)
     val supportedScalaVersions = supportedScala2Versions :+ scala3
 
@@ -67,13 +67,10 @@ object WeePicklePlugin extends AutoPlugin {
       compilerPlugin(acyclic.value),
       acyclic.value % "provided"
     ),
-    mimaPreviousArtifacts := {
-      if (scalaBinaryVersion.value == "3") Set.empty // TODO: remove once there's a previous Scala 3 artifact
-      else
-        previousStableVersion.value
-          .map(organization.value %% moduleName.value % _)
-          .toSet
-    },
+    mimaPreviousArtifacts := previousStableVersion.value
+      .map(organization.value %% moduleName.value % _)
+      .toSet
+    ,
     (Test / test) := {
       mimaReportBinaryIssues.value
       (Test / test).value
@@ -96,7 +93,7 @@ object WeePicklePlugin extends AutoPlugin {
           builder ++= Seq(
             "-Xmax-inlines",
             "128" // MacroTests exceeds default of 32 inlines, 64 is still too small
-            //TODO: what new options should we add? See: https://docs.scala-lang.org/scala3/guides/migration/options-lookup.html
+            // See: https://docs.scala-lang.org/scala3/guides/migration/options-lookup.html
           )
           ()
 

project/build.properties

@@ -1 +1 @@
-sbt.version=1.5.5
+sbt.version=1.9.6

project/plugins.sbt

@@ -1,5 +1,5 @@
-addSbtPlugin("com.typesafe" % "sbt-mima-plugin" % "0.9.2")
+addSbtPlugin("com.typesafe" % "sbt-mima-plugin" % "1.1.2")
 addSbtPlugin("com.dwijnand" % "sbt-dynver" % "4.1.1")
-addSbtPlugin("pl.project13.scala" % "sbt-jmh" % "0.3.7")
-addSbtPlugin("org.xerial.sbt" % "sbt-sonatype" % "3.9.7")
-addSbtPlugin("com.github.sbt" % "sbt-pgp" % "2.1.2")
+addSbtPlugin("pl.project13.scala" % "sbt-jmh" % "0.4.4")
+addSbtPlugin("org.xerial.sbt" % "sbt-sonatype" % "3.9.21")
+addSbtPlugin("com.github.sbt" % "sbt-pgp" % "2.2.1")

weejson-jackson/src/main/scala/com/rallyhealth/weejson/v1/jackson/FromJson.scala

@@ -7,6 +7,7 @@ import com.fasterxml.jackson.core.{JsonFactory, JsonParser, JsonToken, JsonToken
 import com.rallyhealth.weepickle.v1.core.{Abort, CallbackVisitor, FromInput, TransformException, Visitor}
 
 import java.io.{File, InputStream, Reader}
+import java.nio.CharBuffer
 import java.nio.file.Path
 import scala.annotation.switch
 import scala.util.Try
@@ -109,8 +110,8 @@ class JsonFromInput(createParser: () => JsonParser) extends FromInput {
           if (p.getTextLength == 0) ""
           else {
             val start = p.getTextOffset
-            val end = start + p.getTextLength
-            new runtime.ArrayCharSequence(p.getTextCharacters, start, end)
+            val length = p.getTextLength
+            CharBuffer.wrap(p.getTextCharacters, start, length)
           }
         v.visitString(cs)
       case ID_FIELD_NAME =>

weejson-jackson/src/main/scala/com/rallyhealth/weejson/v1/jackson/VisitorJsonGenerator.scala

@@ -6,8 +6,6 @@ import java.math.BigInteger
 import com.fasterxml.jackson.core._
 import com.rallyhealth.weepickle.v1.core._
 
-import scala.collection.mutable
-
 /**
   * Implements most of the `JsonGenerator` interface.
   *
@@ -28,9 +26,19 @@ class VisitorJsonGenerator[J](
 
   private val cs = new TextBufferCharSequence(Array.emptyCharArray, 0, 0)
 
-  private val stack: mutable.ArrayStack[ObjArrVisitor[Any, _]] = new mutable.ArrayStack[ObjArrVisitor[Any, _]]()
-
-  // Manually managing this reference is faster than calling stack.top every time.
+  /*
+   * This stack used to be implemented as an ArrayStack. Scala 2.13 deprecated ArrayStack with the message "Use Stack
+   * instead of ArrayStack; it now uses an array-based implementation". However, Scala 2.12 deprecated Stack with the
+   * message "Stack is an inelegant and potentially poorly-performing wrapper around List. Use a List assigned to a var
+   * instead." To avoid these dueling deprecations in the cross-version build, we are using a List now (i.e., a mutable
+   * reference to an immutable data structure instead of an immutable reference to a mutable data structure), which may
+   * slightly increase the memory allocation rate for all but the smallest messages. This is unlikely to cause any
+   * noticeable performance regression, but at least it is private, so if such a regression is identified later, it can
+   * be addressed without breaking binary compatibility.
+   */
+  private var stack: List[ObjArrVisitor[Any, _]] = Nil
+
+  // Manually managing this reference is faster than calling stack.head every time.
   private var ctxt: ObjArrVisitor[Any, _] = {
     // Create a dummy root stack element that returns the rootVisitor.
     // This is simpler than special-casing the rootVisitor.
@@ -50,12 +58,13 @@ class VisitorJsonGenerator[J](
   protected def top: ObjArrVisitor[Any, _] = ctxt
 
   protected def push(e: ObjArrVisitor[Any, _]): Unit = {
-    stack.push(top)
+    stack = top :: stack
     ctxt = e
   }
   protected def pop(): ObjArrVisitor[Any, _] = {
     val ret = top
-    ctxt = stack.pop()
+    ctxt = stack.head
+    stack = stack.tail
     ret
   }
 
@@ -209,7 +218,7 @@ class VisitorJsonGenerator[J](
 
   override def close(): Unit = {
     if (!isClosed) {
-      stack.clear()
+      stack = Nil
       ctxt = null
     }
   }

weejson/src/main/scala-2/com/rallyhealth/weejson/v1/package.scala

@@ -0,0 +1,6 @@
+package com.rallyhealth.weejson
+
+package object v1 {
+  private[v1] val CollectionConverters = scala.collection.JavaConverters
+
+}

weejson/src/main/scala-3/com/rallyhealth/weejson/v1/package.scala

@@ -0,0 +1,6 @@
+package com.rallyhealth.weejson
+
+package object v1 {
+  private val CollectionConverters = scala.jdk.CollectionConverters
+
+}

weejson/src/main/scala/com/rallyhealth/weejson/v1/Value.scala

@@ -3,7 +3,8 @@ package com.rallyhealth.weejson.v1
 import com.rallyhealth.weejson.v1.jackson.ToJson
 import com.rallyhealth.weepickle.v1.core.{ArrVisitor, FromInput, ObjVisitor, Visitor}
 
-import scala.collection.JavaConverters._
+import CollectionConverters._ // from package object
+import scala.annotation.nowarn
 import scala.collection.compat._
 import scala.collection.mutable
 import scala.collection.mutable.ArrayBuffer
@@ -144,7 +145,9 @@ object Value extends AstTransformer[Value] {
     }
   }
 
+  @nowarn("cat=deprecation") // change to IterableOnce in a future version
   implicit def JsonableSeq[T](items: TraversableOnce[T])(implicit f: T => Value): Arr = Arr.from(items.iterator.map(f))
+  @nowarn("cat=deprecation") // change to IterableOnce in a future version
   implicit def JsonableDict[T](items: TraversableOnce[(String, T)])(implicit f: T => Value): Obj =
     Obj.from(items.iterator.map(x => (x._1, f(x._2))))
   implicit def JsonableBoolean(i: Boolean): Bool = if (i) True else False
@@ -212,6 +215,7 @@ case class Str(value: String) extends Value
 case class Obj(value: mutable.Map[String, Value]) extends Value
 
 object Obj {
+  @nowarn("cat=deprecation") // change to IterableOnce in a future version
   implicit def from(items: TraversableOnce[(String, Value)]): Obj = {
     val initialCapacity = items match {
       case is: mutable.IndexedSeq[_] => is.size
@@ -235,6 +239,7 @@ object Obj {
 case class Arr(value: ArrayBuffer[Value]) extends Value
 
 object Arr {
+  @nowarn("cat=deprecation") // change to IterableOnce in a future version
   implicit def from[T](items: TraversableOnce[T])(implicit viewBound: T => Value): Arr =
     Arr(items.iterator.map(x => viewBound(x)).to(mutable.ArrayBuffer))
 

weexml/src/test/scala/com/rallyhealth/weejson/v1/xml/XmlTests.scala

@@ -1,6 +1,6 @@
 package com.rallyhealth.weejson.v1.xml
 
-import com.rallyhealth.weejson.v1.{Str, Value}
+import com.rallyhealth.weejson.v1.{Obj, Str, Value}
 import org.scalactic.TypeCheckedTripleEquals
 import org.scalatest.Inside
 import org.scalatest.freespec.AnyFreeSpec
@@ -109,19 +109,19 @@ class XmlTests extends AnyFreeSpec with Matchers with Inside with TypeCheckedTri
       }
       "int" in {
         val x = readXml("1")
-        assert(x === Str("1"))
-        val y = x.transform(ToXml.string)
+        assert(x === Obj("" -> Str("1"))) // different than weexml 1.8 (jackson-dataformat-xml 2.13 -> 2.15)
+        val y = Str("1").transform(ToXml.string)
         assert(y == "<root>1</root>")
         val v = FromXml(y).transform(Value)
-        assert(v === Str("1"))
+        assert(v === Obj("" -> Str("1"))) // different than weexml 1.8 (jackson-dataformat-xml 2.13 -> 2.15)
       }
       "string" in {
         val x = readXml("\"1\"") // parses as object with nameless attribute in {"":"\"1\""}
-        assert(x === Str("\"1\""))
-        val y = x.transform(ToXml.string)
+        assert(x === Obj("" -> Str("\"1\""))) // different than weexml 1.8 (jackson-dataformat-xml 2.13 -> 2.15)
+        val y = Str("\"1\"").transform(ToXml.string)
         assert(y === "<root>\"1\"</root>")
         val v2 = FromXml(y).transform(Value)
-        assert(v2 === Str("\"1\""))
+        assert(v2 === Obj("" -> Str("\"1\""))) // different than weexml 1.8 (jackson-dataformat-xml 2.13 -> 2.15)
       }
     }
   }