From 02570d04c3807d0a5548455555bcdc3dadb4a29c Mon Sep 17 00:00:00 2001 From: cchantep Date: Wed, 9 May 2018 15:44:50 +0200 Subject: [PATCH] WIP --- .../play/api/libs/json/JsMacroImpl.scala | 45 ++++++------ .../scala/play/api/libs/json/Writes.scala | 20 ++++-- .../libs/json/JsonExtensionScala2Spec.scala | 4 +- .../scala/play/api/libs/json/MacroSpec.scala | 70 ++++--------------- 4 files changed, 55 insertions(+), 84 deletions(-) diff --git a/play-json/shared/src/main/scala-2/play/api/libs/json/JsMacroImpl.scala b/play-json/shared/src/main/scala-2/play/api/libs/json/JsMacroImpl.scala index 5f5ae0ef7..6b8aa93dc 100644 --- a/play-json/shared/src/main/scala-2/play/api/libs/json/JsMacroImpl.scala +++ b/play-json/shared/src/main/scala-2/play/api/libs/json/JsMacroImpl.scala @@ -170,9 +170,9 @@ class JsMacroImpl(val c: blackbox.Context) { selfRef: Boolean ) - val optTpeCtor = typeOf[Option[_]].typeConstructor + val optTpeCtor = typeOf[Option[_]].typeConstructor val forwardPrefix = "play_jsmacro" - val forwardName = TermName(c.freshName(forwardPrefix)) + val forwardName = TermName(c.freshName(forwardPrefix)) // MacroOptions val options = config.actualType.member(TypeName("Opts")).asType.toTypeIn(config.actualType) @@ -691,10 +691,11 @@ class JsMacroImpl(val c: blackbox.Context) { val (applyFunction, tparams, params, defaultValues) = utility.applyFunction match { case Some(info) => info - case _ => c.abort( - c.enclosingPosition, - "No apply function found matching unapply parameters" - ) + case _ => + c.abort( + c.enclosingPosition, + "No apply function found matching unapply parameters" + ) } // --- @@ -731,17 +732,18 @@ 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) + val resolvedImpl = { + val implTpeName = Option(impl.tpe).fold("null")(_.toString) -if (implTpeName.startsWith(forwardPrefix) || -(implTpeName.startsWith("play.api.libs.json") && -!implTpeName.contains("MacroSpec"))) { -impl // Avoid extra check for builtin formats -} else { -q"""_root_.java.util.Objects.requireNonNull($impl, "Invalid implicit resolution (forward reference?) for '" + $cn + "': " + ${implTpeName})""" -} -} + 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, @@ -749,18 +751,19 @@ q"""_root_.java.util.Objects.requireNonNull($impl, "Invalid implicit resolution (isOption, defaultValue) match { case (true, Some(v)) => val c = TermName(s"${methodName}HandlerWithDefault") - q"$config.optionHandlers.$c($jspathTree, $v)($resolveImpl)" + q"$config.optionHandlers.$c($jspathTree, $v)($resolvedImpl)" - case (true, _) => + case (true, _) => { val c = TermName(s"${methodName}Handler") - q"$config.optionHandlers.$c($jspathTree)($resolveImpl)" + q"$config.optionHandlers.$c($jspathTree)($impl)" + } case (false, Some(v)) => val c = TermName(s"${methodName}WithDefault") - q"$jspathTree.$c($v)($resolveImpl)" + q"$jspathTree.$c($v)($resolvedImpl)" case _ => - q"$jspathTree.${TermName(methodName)}($resolveImpl)" + q"$jspathTree.${TermName(methodName)}($resolvedImpl)" } } .reduceLeft[Tree] { (acc, r) => diff --git a/play-json/shared/src/main/scala/play/api/libs/json/Writes.scala b/play-json/shared/src/main/scala/play/api/libs/json/Writes.scala index 362892212..70be3dc74 100644 --- a/play-json/shared/src/main/scala/play/api/libs/json/Writes.scala +++ b/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) + } } /** diff --git a/play-json/shared/src/test/scala-2/play/api/libs/json/JsonExtensionScala2Spec.scala b/play-json/shared/src/test/scala-2/play/api/libs/json/JsonExtensionScala2Spec.scala index 70ecf20f8..5deee2335 100644 --- a/play-json/shared/src/test/scala-2/play/api/libs/json/JsonExtensionScala2Spec.scala +++ b/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())) diff --git a/play-json/shared/src/test/scala/play/api/libs/json/MacroSpec.scala b/play-json/shared/src/test/scala/play/api/libs/json/MacroSpec.scala index abd9560e3..b9325ff04 100644 --- a/play-json/shared/src/test/scala/play/api/libs/json/MacroSpec.scala +++ b/play-json/shared/src/test/scala/play/api/libs/json/MacroSpec.scala @@ -151,39 +151,29 @@ class MacroSpec extends AnyWordSpec with Matchers with org.scalatestplus.scalach val jsLorem = Json.obj("age" -> 11, "ipsum" -> Json.obj("bar" -> "foo")) try { - jsLorem.validate[Lorem[Simple]] - } catch { - case NonFatal(npe: NullPointerException) => { - val expected = "Invalid implicit resolution" - npe.getMessage.take(expected.size) mustEqual expected + try { + jsLorem.validate[Lorem[Simple]] + } catch { + case e: Throwable if e.getCause != null => + // scalatest ExceptionInInitializerError + throw e.getCause } - - case NonFatal(cause) => throw cause - } - } - - "fails due to forward reference to Format" in { - implicit def format: Format[Lorem[Simple]] = - InvalidForwardResolution.simpleLoremFormat - - val jsLorem = Json.obj("age" -> 11, "ipsum" -> Json.obj("bar" -> "foo")) - - try { - jsLorem.validate[Lorem[Simple]] } catch { case NonFatal(npe: NullPointerException) => { - val expected = "Invalid implicit resolution" - npe.getMessage.take(expected.size) mustEqual expected + val expected = "Implicit value for 'ipsum'" + npe.getMessage.take(expected.size).mustEqual(expected) } - case NonFatal(cause) => throw cause + case cause: Throwable => + cause.printStackTrace() + throw cause } } } "Writes" should { "be generated for simple case class" in { - Json.writes[Simple].writes(Simple("lorem")) mustEqual Json.obj("bar" -> "lorem") + Json.writes[Simple].writes(Simple("lorem")).mustEqual(Json.obj("bar" -> "lorem")) } "as Format for a generic case class" in { @@ -521,38 +511,6 @@ class MacroSpec extends AnyWordSpec with Matchers with org.scalatestplus.scalach formatter.writes(Obj).mustEqual(jsObj) formatter.reads(jsObj).mustEqual(JsSuccess(Obj)) } - - "fails due to forward reference to Writes" in { - implicit def writes: Writes[Lorem[Simple]] = - InvalidForwardResolution.simpleLoremWrites - - try { - Json.toJson(Lorem(age = 11, ipsum = Simple(bar = "foo"))) - } catch { - case NonFatal(npe: NullPointerException) => { - val expected = "Invalid implicit resolution" - npe.getMessage.take(expected.size) mustEqual expected - } - - case NonFatal(cause) => throw cause - } - } - - "fails due to forward reference to Format" in { - implicit def format: Format[Lorem[Simple]] = - InvalidForwardResolution.simpleLoremFormat - - try { - Json.toJson(Lorem(age = 11, ipsum = Simple(bar = "foo"))) - } catch { - case NonFatal(npe: NullPointerException) => { - val expected = "Invalid implicit resolution" - npe.getMessage.take(expected.size) mustEqual expected - } - - case NonFatal(cause) => throw cause - } - } } } @@ -584,11 +542,11 @@ object MacroSpec { object InvalidForwardResolution { // Invalids as forward references to `simpleX` - val simpleLoremReads = Json.reads[Lorem[Simple]] + 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 simpleReads: Reads[Simple] = Json.reads implicit val simpleWrites: OWrites[Simple] = Json.writes[Simple] }