Make Writes typeclass invariant

.jvmopts

@@ -0,0 +1,3 @@
+-Xss2M
+-XX:+CMSClassUnloadingEnabled
+-XX:ReservedCodeCacheSize=192m

play-json/jvm/src/main/scala/play/api/libs/json/EnvWrites.scala

@@ -15,12 +15,16 @@ import play.api.libs.json.jackson.JacksonJson
 trait EnvWrites {
   import scala.language.implicitConversions
 
+  @deprecated("Use `jsonNodeWrites`", "2.8.0")
+  object JsonNodeWrites extends Writes[JsonNode] {
+    def writes(o: JsonNode): JsValue = JacksonJson.jsonNodeToJsValue(o)
+  }
+
   /**
    * Serializer for Jackson JsonNode
    */
-  implicit object JsonNodeWrites extends Writes[JsonNode] {
-    def writes(o: JsonNode): JsValue = JacksonJson.jsonNodeToJsValue(o)
-  }
+  implicit def jsonNodeWrites[T <: JsonNode]: Writes[T] =
+    Writes[T](JacksonJson.jsonNodeToJsValue)
 
   /** Typeclass to implement way of formatting of Java8 temporal types. */
   trait TemporalFormatter[T <: Temporal] {

play-json/shared/src/main/scala/play/api/libs/json/JsConstraints.scala

@@ -4,6 +4,8 @@
 
 package play.api.libs.json
 
+import scala.collection.immutable.Map
+
 trait ConstraintFormat {
   @inline def of[A](implicit fmt: Format[A]): Format[A] = fmt
 
@@ -206,13 +208,13 @@ trait ConstraintWrites {
     def writes(a: A): JsValue = JsNull
   }
 
-  def list[A](implicit writes: Writes[A]): Writes[List[A]] = Writes.traversableWrites[A]
+  def list[A](implicit writes: Writes[A]): Writes[List[A]] = Writes.iterableWrites[A, List]
 
-  def set[A](implicit writes: Writes[A]): Writes[Set[A]] = Writes.traversableWrites[A]
+  def set[A](implicit writes: Writes[A]): Writes[Set[A]] = Writes.iterableWrites[A, Set]
 
-  def seq[A](implicit writes: Writes[A]): Writes[Seq[A]] = Writes.traversableWrites[A]
+  def seq[A](implicit writes: Writes[A]): Writes[Seq[A]] = Writes.iterableWrites[A, Seq]
 
-  def map[A](implicit writes: Writes[A]): OWrites[collection.immutable.Map[String, A]] = Writes.mapWrites[A]
+  def map[A](implicit writes: Writes[A]): OWrites[Map[String, A]] = Writes.genericMapWrites[A, Map]
 
   /**
    * Pure Option Writer[T] which writes "null" when None which is different

play-json/shared/src/main/scala/play/api/libs/json/JsMacroImpl.scala

@@ -566,6 +566,7 @@ import scala.reflect.macros.blackbox
     // --- Sub implementations
 
     val readsType = c.typeOf[Reads[_]]
+    val writesType = c.typeOf[Writes[_]]
 
     def macroSealedFamilyImpl(subTypes: List[Type]): c.Expr[M[A]] = {
       if (subTypes.isEmpty) {
@@ -574,24 +575,21 @@ import scala.reflect.macros.blackbox
 
       def readLambda: Tree = {
         val resolver = new ImplicitResolver({ orig: Type => orig })
-        val cases = Match(q"dis", (
-          cq"""_ => $json.JsError("error.invalid")""" :: (
-            subTypes.foldLeft(List.empty[CaseDef]) { (out, t) =>
-              val rtpe = appliedType(readsType, List(t))
-              val reader = resolver.createImplicit(
-                atpe, rtpe
-              )(t).neededImplicit
-
-              if (reader.isEmpty) {
-                c.abort(
-                  c.enclosingPosition,
-                  s"No instance of Reads is available for ${t.typeSymbol.fullName} in the implicit scope (Hint: if declared in the same file, make sure it's declared before)"
-                )
-              }
-              cq"name if name == $config.typeNaming(${t.typeSymbol.fullName}) => $reader.reads(vjs)" :: out
-            }
-          )
-        ).reverse)
+        val cases = Match(q"dis", (subTypes.map { t =>
+          val rtpe = appliedType(readsType, List(t))
+          val reader = resolver.createImplicit(
+            atpe, rtpe
+          )(t).neededImplicit
+
+          if (reader.isEmpty) {
+            c.abort(
+              c.enclosingPosition,
+              s"No instance of Reads is available for ${t.typeSymbol.fullName} in the implicit scope (Hint: if declared in the same file, make sure it's declared before)"
+            )
+          }
+
+          cq"name if name == $config.typeNaming(${t.typeSymbol.fullName}) => $reader.reads(vjs)"
+        }) :+ cq"""_ => $json.JsError("error.invalid")""")
 
         q"""(_: $json.JsValue) match {
           case obj @ $json.JsObject(_) => obj.value.get($config.discriminator) match {
@@ -608,23 +606,24 @@ import scala.reflect.macros.blackbox
       }
 
       def writeLambda: Tree = {
-        val owner = c.internal.enclosingOwner
-
-        if (!owner.isTerm) {
-          c.abort(
-            c.enclosingPosition,
-            "the macro must be used to generate the body of a term"
-          )
-        }
+        val resolver = new ImplicitResolver({ orig: Type => orig })
+        val cases = Match(q"v", subTypes.map { t =>
+          val wtpe = appliedType(writesType, List(t))
+          val writer = resolver.createImplicit(
+            atpe, wtpe
+          )(t).neededImplicit
+
+          if (writer.isEmpty) {
+            c.abort(
+              c.enclosingPosition,
+              s"No instance of Writes is available for ${t.typeSymbol.fullName} in the implicit scope (Hint: if declared in the same file, make sure it's declared before)"
+            )
+          }
 
-        // ---
+          // ---
 
-        val term = owner.asTerm
-        val cases = Match(q"v", subTypes.map { t =>
-          // Use `implicitly` rather than `ImplicitResolver.createImplicit`,
-          // due to the implicit/contravariance workaround
           cq"""x: $t => {
-            val xjs = implicitly[$json.Writes[$t]].writes(x)
+            val xjs = ${writer}.writes(x)
             @inline def jso = xjs match {
               case xo @ $json.JsObject(_) => xo
               case jsv => $json.JsObject(Seq("_value" -> jsv))
@@ -634,20 +633,7 @@ import scala.reflect.macros.blackbox
           }"""
         })
 
-        // Use shadowing to eliminate the generated term itself
-        // from the implicit scope, due to the contravariant/implicit issue:
-        // https://groups.google.com/forum/#!topic/scala-language/ZE83TvSWpT4
-
-        val shadowName = TermName(term.name.decodedName.toString.trim)
-        // DO NOT directly use `term.name` as for some reason,
-        // the TermName is provided within Scala.JS is appended with a final ' '
-
-        q"""{ v: ${atpe} =>
-          def ${shadowName}: $json.OWrites[${atpe}] =
-            sys.error("Invalid implicit resolution")
-
-          $cases
-        }"""
+        q"{ v: ${atpe} => $cases }"
       }
 
       def tree = methodName match {

play-json/shared/src/main/scala/play/api/libs/json/Writes.scala

@@ -4,19 +4,23 @@
 
 package play.api.libs.json
 
-import play.api.libs.functional.ContravariantFunctor
+import java.util.Date
 
 import scala.annotation.implicitNotFound
+import scala.language.higherKinds
+
 import scala.collection._
 import scala.reflect.ClassTag
 
+import play.api.libs.functional.ContravariantFunctor
+
 /**
  * Json serializer: write an implicit to define a serializer for any type
  */
 @implicitNotFound(
   "No Json serializer found for type ${A}. Try to implement an implicit Writes or Format for this type."
 )
-trait Writes[-A] { self =>
+trait Writes[A] { self =>
   /**
    * Converts the `A` value into a [[JsValue]].
    */
@@ -46,7 +50,7 @@ trait Writes[-A] { self =>
 @implicitNotFound(
   "No Json serializer as JsObject found for type ${A}. Try to implement an implicit OWrites or OFormat for this type."
 )
-trait OWrites[-A] extends Writes[A] {
+trait OWrites[A] extends Writes[A] {
   def writes(o: A): JsObject
 
   /**
@@ -265,15 +269,48 @@ trait DefaultWrites extends LowPriorityWrites {
   /**
    * Serializer for Map[String,V] types.
    */
+  @deprecated("Use `genericMapWrites`", "2.8.0")
   implicit def mapWrites[V: Writes]: OWrites[MapWrites.Map[String, V]] = MapWrites.mapWrites
 
   /**
-   * Serializer for JsValues.
+   * Serializer for Map[String,V] types.
    */
-  implicit object JsValueWrites extends Writes[JsValue] {
+  implicit def genericMapWrites[V, M[A, B] <: MapWrites.Map[A, B]](implicit w: Writes[V]): OWrites[M[String, V]] = OWrites[M[String, V]] { ts =>
+    JsObject(ts.mapValues(w.writes(_)).toSeq)
+  }
+
+  @deprecated("Use `jsValueWrites`", "2.8.0")
+  object JsValueWrites extends Writes[JsValue] {
     def writes(o: JsValue) = o
   }
 
+  /**
+   * Serializer for JsValues.
+   */
+  implicit def jsValueWrites[T <: JsValue]: Writes[T] = Writes[T] { js => js }
+
+  /**
+   * Serializer for JsNull.
+   *
+   * {{{
+   * Json.obj("foo" -> None)
+   * // equivalent to Json.obj("foo" -> JsNull)
+   * }}}
+   */
+  implicit val NoneWrites: Writes[None.type] =
+    Writes[None.type] { _ => JsNull }
+
+  /**
+   * If `Some` is directly used (not as `Option`).
+   *
+   * {{{
+   * Json.obj("foo" -> Some(writeableValue))
+   * // equivalent to Json.obj("foo" -> writeableValue)
+   * }}}
+   */
+  implicit def someWrites[T](implicit w: Writes[T]): Writes[Some[T]] =
+    Writes[Some[T]] { some => w.writes(some.get) }
+
   /**
    * Serializer for Option.
    */
@@ -292,17 +329,22 @@ trait DefaultWrites extends LowPriorityWrites {
     def writes(d: java.util.Date): JsValue = JsString(new java.text.SimpleDateFormat(pattern).format(d))
   }
 
+  @deprecated("Use `defaultDateWrites`", "2.8.0")
+  object DefaultDateWrites extends Writes[Date] {
+    def writes(d: Date): JsValue = JsNumber(d.getTime)
+  }
+
   /**
    * Default Serializer java.util.Date -> JsNumber(d.getTime (nb of ms))
    */
-  implicit object DefaultDateWrites extends Writes[java.util.Date] {
-    def writes(d: java.util.Date): JsValue = JsNumber(d.getTime)
-  }
+  implicit def defaultDateWrites[T <: Date]: Writes[T] =
+    Writes[T] { d => JsNumber(d.getTime) }
 
   /**
    * Serializer for java.sql.Date
    * @param pattern the pattern used by SimpleDateFormat
    */
+  @deprecated("Use `dateWrites`", "2.8.0")
   def sqlDateWrites(pattern: String): Writes[java.sql.Date] = new Writes[java.sql.Date] {
     def writes(d: java.sql.Date): JsValue = JsString(new java.text.SimpleDateFormat(pattern).format(d))
   }
@@ -320,13 +362,21 @@ trait DefaultWrites extends LowPriorityWrites {
   implicit def enumNameWrites[E <: Enumeration]: Writes[E#Value] =
     Writes[E#Value] { value: E#Value => JsString(value.toString) }
 
+  /**
+   * Serializer for [[scala.collection.immutable.Range]]
+   * (aka specialized `Seq` of `Int`).
+   */
+  implicit def rangeWrites[T <: Range]: Writes[T] = Writes[T] { range =>
+    // `iterableWrites` cannot be resolved for as,
+    // even if `Range <: Traversable[_]`, it's not a parametrized one
+    // and so doesn't accept a type parameter (doesn't match `M[_]` constraint).
+    JsArray(range.map(JsNumber(_)))
+  }
 }
 
 sealed trait LowPriorityWrites extends EnvWrites {
-  /**
-   * Serializer for Traversables types.
-   */
-  implicit def traversableWrites[A: Writes]: Writes[Traversable[A]] = {
+  @deprecated("Use `iterableWrites`", "2.8.0")
+  def traversableWrites[A: Writes]: Writes[Traversable[A]] = {
     val w = implicitly[Writes[A]]
 
     Writes[Traversable[A]] { as =>
@@ -339,4 +389,30 @@ sealed trait LowPriorityWrites extends EnvWrites {
     // Avoid resolution ambiguity with more specific Traversable Writes,
     // such as OWrites.map
   }
+
+  /**
+   * Serializer for Iterable types.
+   */
+  implicit def iterableWrites[A, M[T] <: Iterable[T]](implicit w: Writes[A]): Writes[M[A]] = {
+    // Use Iterable rather than Traversable, for 2.13 compat
+
+    Writes[M[A]] { as =>
+      val builder = mutable.ArrayBuilder.make[JsValue]
+
+      as.foreach { a: A =>
+        builder += w.writes(a)
+      }
+
+      JsArray(builder.result())
+    }
+
+    // Avoid resolution ambiguity with more specific Iterable Writes,
+    // such as OWrites.map
+  }
+
+  /**
+   * Serializer for any type that is provided an implicit conversion to String
+   * (e.g. tagged types).
+   */
+  implicit def stringableWrites[T](implicit conv: T => String): Writes[T] = Writes.StringWrites.contramap[T](conv)
 }

play-json/shared/src/test/scala/play/api/libs/json/MacroSpec.scala

@@ -227,6 +227,12 @@ class MacroSpec extends WordSpec with MustMatchers
       wsimple.validate(Json.reads[Simple]).get mustEqual simple
       wopt mustEqual jsOptional
       wopt.validate(Json.reads[Optional]).get mustEqual optional
+
+      // was StackOverFlow exception
+      Json.toJson[Family1](Family1Member("bar")) mustEqual Json.obj(
+        "_type" -> "play.api.libs.json.MacroSpec.Family1Member",
+        "foo" -> "bar"
+      )
     }
 
     "be generated for a ValueClass" in {
@@ -655,7 +661,6 @@ class MacroSpec extends WordSpec with MustMatchers
     def id(value: String): Id = value.asInstanceOf[Id]
 
     implicit val idReads: Reads[Id] = implicitly[Reads[String]].asInstanceOf[Reads[Id]]
-    implicit val idWrites: Writes[Id] = implicitly[Writes[String]]
   }
   case class Foo(id: Foo.Id, value: Option[Either[String, Foo]])
 
@@ -683,4 +688,32 @@ class MacroSpec extends WordSpec with MustMatchers
   object WithDefaultInCompanion {
     implicit val format: OFormat[WithDefaultInCompanion] = Json.using[Json.WithDefaultValues].format[WithDefaultInCompanion]
   }
+
+  // ---
+
+  sealed trait Family1
+  object Family1 {
+    def w: OWrites[Family1] = Json.writes[Family1]
+    implicit lazy val writes: OWrites[Family1] = w
+  }
+
+  case class Family1Member(foo: String) extends Family1
+  object Family1Member {
+    implicit def writer: OWrites[Family1Member] = Json.writes[Family1Member]
+  }
+
+  // ---
+
+  sealed trait Family2
+  case class Family2Member(p: Int) extends Family2
+  object Family2 {
+    implicit def w: OWrites[Family2] = {
+      shapeless.test.illTyped("Json.writes[Family2]")
+      ???
+    }
+    /* Should fail, as there is no implicit for Family2Member,
+     for now due to the contravariance `w` being defined is self resolved
+     as Writes instance this subtype Family2Member
+     */
+  }
 }