Forward ref check

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

@@ -170,8 +170,9 @@ class JsMacroImpl(val c: blackbox.Context) {
         selfRef: Boolean
     )
 
-    val optTpeCtor  = typeOf[Option[_]].typeConstructor
-    val forwardName = TermName(c.freshName("forward"))
+    val optTpeCtor    = typeOf[Option[_]].typeConstructor
+    val forwardPrefix = "play_jsmacro"
+    val forwardName   = TermName(c.freshName(forwardPrefix))
 
     // MacroOptions
     val options = config.actualType.member(TypeName("Opts")).asType.toTypeIn(config.actualType)
@@ -693,7 +694,7 @@ class JsMacroImpl(val c: blackbox.Context) {
         case _ =>
           c.abort(
             c.enclosingPosition,
-            s"No apply function found matching unapply parameters"
+            "No apply function found matching unapply parameters"
           )
       }
 
@@ -731,24 +732,38 @@ class JsMacroImpl(val c: blackbox.Context) {
             val defaultValue = // not applicable for 'write' only
               defaultValueMap.get(name).filter(_ => methodName != "write")
 
+            val resolvedImpl = {
+              val implTpeName = Option(impl.tpe).fold("null")(_.toString)
+
+              if (implTpeName.startsWith(forwardPrefix) ||
+                  (implTpeName.startsWith("play.api.libs.json") &&
+                  !(implTpeName.startsWith("play.api.libs.json.Functional") ||
+                    implTpeName.contains("MacroSpec")))) {
+                impl // Avoid extra check for builtin formats
+              } else {
+                q"""_root_.java.util.Objects.requireNonNull($impl, "Implicit value for '" + $cn + "' was null (forward reference?): " + ${implTpeName})"""
+              }
+            }
+
             // - If we're an default value, invoke the withDefault version
             // - If we're an option with default value,
             //   invoke the WithDefault version
             (isOption, defaultValue) match {
               case (true, Some(v)) =>
                 val c = TermName(s"${methodName}HandlerWithDefault")
-                q"$config.optionHandlers.$c($jspathTree, $v)($impl)"
+                q"$config.optionHandlers.$c($jspathTree, $v)($resolvedImpl)"
 
-              case (true, _) =>
+              case (true, _) => {
                 val c = TermName(s"${methodName}Handler")
                 q"$config.optionHandlers.$c($jspathTree)($impl)"
+              }
 
               case (false, Some(v)) =>
                 val c = TermName(s"${methodName}WithDefault")
-                q"$jspathTree.$c($v)($impl)"
+                q"$jspathTree.$c($v)($resolvedImpl)"
 
               case _ =>
-                q"$jspathTree.${TermName(methodName)}($impl)"
+                q"$jspathTree.${TermName(methodName)}($resolvedImpl)"
             }
         }
         .reduceLeft[Tree] { (acc, r) =>
@@ -820,6 +835,7 @@ class JsMacroImpl(val c: blackbox.Context) {
             case _ =>
               q"$json.OFormat[${atpe}](instance.reads(_), instance.writes(_))"
           }
+
           val forwardCall =
             q"private val $forwardName = $forward"
 

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

@@ -144,9 +144,7 @@ object OWrites extends PathWrites with ConstraintWrites {
   /**
    * Returns an instance which uses `f` as [[OWrites.writes]] function.
    */
-  def apply[A](f: A => JsObject): OWrites[A] = new OWrites[A] {
-    def writes(a: A): JsObject = f(a)
-  }
+  def apply[A](f: A => JsObject): OWrites[A] = new FunctionalOWrites[A](f)
 
   /**
    * Transforms the resulting [[JsObject]] using the given function,
@@ -159,6 +157,12 @@ object OWrites extends PathWrites with ConstraintWrites {
     OWrites[A] { a =>
       f(a, w.writes(a))
     }
+
+  // ---
+
+  private[json] final class FunctionalOWrites[A](w: A => JsObject) extends OWrites[A] {
+    def writes(a: A): JsObject = w(a)
+  }
 }
 
 /**
@@ -177,9 +181,7 @@ object Writes extends PathWrites with ConstraintWrites with DefaultWrites with G
   /**
    * Returns an instance which uses `f` as [[Writes.writes]] function.
    */
-  def apply[A](f: A => JsValue): Writes[A] = new Writes[A] {
-    def writes(a: A): JsValue = f(a)
-  }
+  def apply[A](f: A => JsValue): Writes[A] = new FunctionalWrites[A](f)
 
   /**
    * Transforms the resulting [[JsValue]] using the given function,
@@ -194,6 +196,12 @@ object Writes extends PathWrites with ConstraintWrites with DefaultWrites with G
     Writes[A] { a =>
       f(a, w.writes(a))
     }
+
+  // ---
+
+  private[json] final class FunctionalWrites[A](w: A => JsValue) extends Writes[A] {
+    def writes(a: A): JsValue = w(a)
+  }
 }
 
 /**

play-json/shared/src/test/scala-2/play/api/libs/json/JsonExtensionScala2Spec.scala

@@ -127,7 +127,9 @@ class JsonExtensionScala2Spec extends AnyWordSpec with Matchers {
       implicit val jsonConfiguration: JsonConfiguration.Aux[Json.WithDefaultValues] =
         JsonConfiguration[Json.WithDefaultValues](optionHandlers = OptionHandlers.WritesNull)
       val formatter = Json.format[OptionalWithDefault]
-      formatter.writes(OptionalWithDefault()).mustEqual(Json.obj("props"            -> JsNull))
+
+      formatter.writes(OptionalWithDefault()).mustEqual(Json.obj("props" -> JsNull))
+
       formatter.writes(OptionalWithDefault(Some("foo"))).mustEqual(Json.obj("props" -> "foo"))
 
       formatter.reads(Json.obj()).mustEqual(JsSuccess(OptionalWithDefault()))

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

@@ -4,6 +4,8 @@
 
 package play.api.libs.json
 
+import scala.util.control.NonFatal
+
 import org.scalatest.matchers.must.Matchers
 import Matchers._
 import org.scalatest.wordspec.AnyWordSpec
@@ -141,6 +143,32 @@ class MacroSpec extends AnyWordSpec with Matchers with org.scalatestplus.scalach
         jsOptional.validate[Family].mustEqual(JsSuccess(optional))
       }
     }
+
+    "fails due to forward reference to Reads" in {
+      implicit def reads: Reads[Lorem[Simple]] =
+        InvalidForwardResolution.simpleLoremReads
+
+      val jsLorem = Json.obj("age" -> 11, "ipsum" -> Json.obj("bar" -> "foo"))
+
+      try {
+        try {
+          jsLorem.validate[Lorem[Simple]]
+        } catch {
+          case e: Throwable if e.getCause != null =>
+            // scalatest ExceptionInInitializerError
+            throw e.getCause
+        }
+      } catch {
+        case NonFatal(npe: NullPointerException) => {
+          val expected = "Implicit value for 'ipsum'"
+          npe.getMessage.take(expected.size).mustEqual(expected)
+        }
+
+        case cause: Throwable =>
+          cause.printStackTrace()
+          throw cause
+      }
+    }
   }
 
   "Writes" should {
@@ -512,6 +540,16 @@ object MacroSpec {
    */
   }
 
+  object InvalidForwardResolution {
+    // Invalids as forward references to `simpleX`
+    val simpleLoremReads  = Json.reads[Lorem[Simple]]
+    val simpleLoremWrites = Json.writes[Lorem[Simple]]
+    val simpleLoremFormat = Json.format[Lorem[Simple]]
+
+    implicit val simpleReads: Reads[Simple]    = Json.reads
+    implicit val simpleWrites: OWrites[Simple] = Json.writes[Simple]
+  }
+
   object Foo {
     // https://github.com/lampepfl/dotty/issues/11054 Type aliasing breaks constValue
     // import shapeless.tag.@@