From 0463c695db09b968f7b398c464f041aab8ca3ad0 Mon Sep 17 00:00:00 2001 From: Jan Chyb Date: Tue, 14 Jan 2025 12:54:35 +0100 Subject: [PATCH] Fix issue with wrong Action[Ctx, _] conversion being summoned --- build.sbt | 5 ++ .../macros/derive/DeriveMacroSupport.scala | 11 +-- .../macros/derive/DeriveObjectTypeMacro.scala | 79 ++++++------------- .../derive/DeriveObjectTypeMacroSpec.scala | 27 +++++++ .../macros/derive/Issue906Context.scala | 64 +++++++++++++++ 5 files changed, 128 insertions(+), 58 deletions(-) create mode 100644 modules/derivation/src/test/scala/sangria/macros/derive/Issue906Context.scala diff --git a/build.sbt b/build.sbt index 604277d0..ec6922bf 100644 --- a/build.sbt +++ b/build.sbt @@ -254,6 +254,11 @@ lazy val derivation = project name := "sangria-derivation", Test / testOptions += Tests.Argument(TestFrameworks.ScalaTest, "-oF"), mimaPreviousArtifacts := Set("org.sangria-graphql" %% "sangria-derivation" % "4.0.0"), + mimaBinaryIssueFilters ++= Seq( + // internal method + ProblemFilters.exclude[DirectMissingMethodProblem]( + "sangria.macros.derive.DeriveMacroSupport.unsafeSelectByName") + ), // Macros libraryDependencies ++= (if (isScala3.value) Seq.empty else Seq("org.scala-lang" % "scala-reflect" % scalaVersion.value)), diff --git a/modules/derivation/src/main/scala-3/sangria/macros/derive/DeriveMacroSupport.scala b/modules/derivation/src/main/scala-3/sangria/macros/derive/DeriveMacroSupport.scala index 43618446..1b8a8e30 100644 --- a/modules/derivation/src/main/scala-3/sangria/macros/derive/DeriveMacroSupport.scala +++ b/modules/derivation/src/main/scala-3/sangria/macros/derive/DeriveMacroSupport.scala @@ -5,7 +5,7 @@ import sangria.schema.{InputType, OutputType} import scala.quoted._ import scala.reflect.ClassTag -trait DeriveMacroSupport { +private[derive] trait DeriveMacroSupport { def reportErrors(using Quotes)(errors: Seq[(PositionPointer, String)]) = { import quotes.reflect.report @@ -109,11 +109,12 @@ trait DeriveMacroSupport { } } - protected def unsafeSelectByName[S](using quotes: Quotes)(using - Type[S])(memberExpr: Expr[_], name: String): Expr[S] = { + protected def unsafeSelectByName[T: Type](using quotes: Quotes)( + memberExpr: Expr[_], + name: String + ): Expr[T] = { import quotes.reflect._ - val a = Select.unique(memberExpr.asTerm, name) - a.etaExpand(Symbol.spliceOwner).asExprOf[S] + Select.unique(memberExpr.asTerm, name).asExprOf[T] } protected def getClassTag[T](using Type[T], Quotes): Expr[ClassTag[T]] = { diff --git a/modules/derivation/src/main/scala-3/sangria/macros/derive/DeriveObjectTypeMacro.scala b/modules/derivation/src/main/scala-3/sangria/macros/derive/DeriveObjectTypeMacro.scala index 10b32290..dfda0a24 100644 --- a/modules/derivation/src/main/scala-3/sangria/macros/derive/DeriveObjectTypeMacro.scala +++ b/modules/derivation/src/main/scala-3/sangria/macros/derive/DeriveObjectTypeMacro.scala @@ -129,34 +129,13 @@ class DeriveObjectTypeMacro(using globalQuotes: Quotes) extends DeriveMacroSuppo ctxValType match { case Some((tpe, fn)) => given Type[CtxVal] = tpe - Expr.summon[t => Action[Ctx, t]] match - case Some(conversion) => - '{ - $conversion(${ - unsafeSelectByName[t]('{ $fn(c.ctx) }, field.method.name) - }) - } - case None => - reportSummoningErrors( - Seq( - s"Implicit conversion not found: ${TypeRepr.of[t => Action[Ctx, t]].show}"), - Seq(None) - ) + wrappedInActionConversion[Ctx, t]( + unsafeSelectByName[t]('{ $fn(c.ctx) }, field.method.name) + ) case None => - Expr.summon[t => Action[Ctx, t]] match - case Some(conversion) => - '{ - $conversion(${ - unsafeSelectByName[t]('{ c.value }, field.method.name) - .asExprOf[t] - }) - } - case None => - reportSummoningErrors( - Seq( - s"Implicit conversion not found: ${TypeRepr.of[t => Action[Ctx, t]].show}"), - Seq(None) - ) + wrappedInActionConversion[Ctx, t]( + unsafeSelectByName[t]('{ c.value }, field.method.name) + ) } } } @@ -181,7 +160,7 @@ class DeriveObjectTypeMacro(using globalQuotes: Quotes) extends DeriveMacroSuppo case Some(lookup) => '{ $lookup.graphqlType } case None => reportSummoningErrors( - Seq(s"GraphQlOutputType not found: ${TypeRepr.of[t => Action[Ctx, t]].show}"), + Seq(s"GraphQlOutputType not found: ${TypeRepr.of[t => Action[Ctx, _]].show}"), Seq(None) ) } @@ -319,40 +298,34 @@ class DeriveObjectTypeMacro(using globalQuotes: Quotes) extends DeriveMacroSuppo case d: DefDef => d.returnTpt.tpe.asType match case '[t] => - Expr.summon[t => Action[Ctx, t]] match - case Some(conversion) => - '{ - $conversion(${ - args - .foldLeft[Select | Apply](method) { (m, argList) => - Apply(m, argList) - } - .asExprOf[t] - }) + wrappedInActionConversion[Ctx, t]( + args + .foldLeft[Select | Apply](method) { (m, argList) => + Apply(m, argList) } - case None => - reportSummoningErrors( - Seq( - s"Implicit conversion not found: ${TypeRepr.of[t => Action[Ctx, t]].show}"), - Seq(None) - ) + .asExprOf[t] + ) else globalQuotes.reflect.Ref(member.method).tpe.widen.asType match { case '[t] => - Expr.summon[t => Action[Ctx, t]] match - case Some(conversion) => - '{ $conversion(${ method.asExprOf[t] }) } - case None => - reportSummoningErrors( - Seq( - s"Implicit conversion not found: ${TypeRepr.of[t => Action[Ctx, t]].show}"), - Seq(None) - ) + wrappedInActionConversion[Ctx, t](method.asExprOf[t]) } } } } + private def wrappedInActionConversion[Ctx: Type, T: Type](value: Expr[T]) = { + import globalQuotes.reflect._ + Expr.summon[T => Action[Ctx, _]] match + case Some(conversion) => + '{ $conversion(${ value }) } + case None => + reportSummoningErrors( + Seq(s"Implicit conversion not found: ${TypeRepr.of[T => Action[Ctx, _]].show}"), + Seq(None) + ) + } + private def createArg(config: Seq[MacroDeriveObjectSetting], member: KnownMember)( arg: globalQuotes.reflect.Symbol) = import globalQuotes.reflect._ diff --git a/modules/derivation/src/test/scala/sangria/macros/derive/DeriveObjectTypeMacroSpec.scala b/modules/derivation/src/test/scala/sangria/macros/derive/DeriveObjectTypeMacroSpec.scala index 38beb9c2..c0116294 100644 --- a/modules/derivation/src/test/scala/sangria/macros/derive/DeriveObjectTypeMacroSpec.scala +++ b/modules/derivation/src/test/scala/sangria/macros/derive/DeriveObjectTypeMacroSpec.scala @@ -836,5 +836,32 @@ class DeriveObjectTypeMacroSpec extends AnyWordSpec with Matchers with FutureRes "deriveObjectType[Unit, TestContainer[TestSubject]]()" should compile } + + "handle Future result values correctly (issue #906)" in { + import Issue906Context._ + import sangria.parser.QueryParser + + val query = QueryParser + .parse( + s""" + query SomeTest { + mySample { + myFutureString + } + } + """ + ) + .fold(throw _, identity) + + Executor.execute(MySample.schema, query, MyRepository.sample).await should be( + Map( + "data" -> Map( + "mySample" -> Map( + "myFutureString" -> "Hello World" + ) + ) + ) + ) + } } } diff --git a/modules/derivation/src/test/scala/sangria/macros/derive/Issue906Context.scala b/modules/derivation/src/test/scala/sangria/macros/derive/Issue906Context.scala new file mode 100644 index 00000000..1331e1e6 --- /dev/null +++ b/modules/derivation/src/test/scala/sangria/macros/derive/Issue906Context.scala @@ -0,0 +1,64 @@ +package sangria.macros.derive + +import sangria.schema._ + +import scala.concurrent.Future + +object Issue906Context { + + @GraphQLName("SampleCaseClass") + @GraphQLDescription("A sample case class") + final case class SampleCaseClass( + name: String + ) { + @GraphQLField + @GraphQLName("myString") + def graphqlMyString: String = "Hello World" + + @GraphQLField + @GraphQLName("myFutureString") + def graphqlMyFutureString: Future[String] = Future.successful("Hello World") + + } + + object SampleCaseClass { + val sample: SampleCaseClass = SampleCaseClass("My test") + } + + trait MyRepository { + def getSampleCaseClass: SampleCaseClass + } + + object MyRepository { + def sample: MyRepository = new MyRepository { + override def getSampleCaseClass: SampleCaseClass = SampleCaseClass.sample + } + } + + object MySample { + val schema: Schema[MyRepository, Unit] = { + + implicit val graphqlSampleCaseClass: ObjectType[MyRepository, SampleCaseClass] = + deriveObjectType[MyRepository, SampleCaseClass]() + + val queries: ObjectType[MyRepository, Unit] = ObjectType( + "Query", + fields( + Field( + "mySample", + graphqlSampleCaseClass, + Some("test"), + arguments = Nil, + resolve = (ctx: Context[MyRepository, Unit]) => SampleCaseClass.sample + ) + ) + ) + + Schema( + query = queries, + mutation = None, + additionalTypes = Nil + ) + } + } +}