From 9747c217d79b6abeec1043b9fa5bee9140fb4bf5 Mon Sep 17 00:00:00 2001 From: Kyri Petrou Date: Tue, 25 Jun 2024 16:39:11 +1000 Subject: [PATCH 1/3] Fix derivation of Scala 3 case objects with `@GQLField` methods --- .../caliban/schema/SchemaDerivation.scala | 6 +- .../caliban/schema/macros/Macros.scala | 41 ++++++---- .../caliban/schema/Scala3DerivesSpec.scala | 80 ++++++++++++++----- 3 files changed, 89 insertions(+), 38 deletions(-) diff --git a/core/src/main/scala-3/caliban/schema/SchemaDerivation.scala b/core/src/main/scala-3/caliban/schema/SchemaDerivation.scala index 9d5fa5587..325953767 100644 --- a/core/src/main/scala-3/caliban/schema/SchemaDerivation.scala +++ b/core/src/main/scala-3/caliban/schema/SchemaDerivation.scala @@ -106,20 +106,20 @@ trait CommonSchemaDerivation { case m: Mirror.ProductOf[A] => inline erasedValue[m.MirroredElemLabels] match { - case _: EmptyTuple => + case _: EmptyTuple if !Macros.hasFieldsFromMethods[A] => new EnumValueSchema[R, A]( MagnoliaMacro.typeInfo[A], // Workaround until we figure out why the macro uses the parent's annotations when the leaf is a Scala 3 enum inline if (!MagnoliaMacro.isEnum[A]) MagnoliaMacro.anns[A] else Nil, config.enableSemanticNonNull ) - case _ if Macros.hasAnnotation[A, GQLValueType] => + case _ if Macros.hasAnnotation[A, GQLValueType] => new ValueTypeSchema[R, A]( valueTypeSchema[R, m.MirroredElemLabels, m.MirroredElemTypes], MagnoliaMacro.typeInfo[A], MagnoliaMacro.anns[A] ) - case _ => + case _ => new ObjectSchema[R, A]( recurseProduct[R, A, m.MirroredElemLabels, m.MirroredElemTypes]()(), Macros.fieldsFromMethods[R, A], diff --git a/core/src/main/scala-3/caliban/schema/macros/Macros.scala b/core/src/main/scala-3/caliban/schema/macros/Macros.scala index 853b10099..3161832cb 100644 --- a/core/src/main/scala-3/caliban/schema/macros/Macros.scala +++ b/core/src/main/scala-3/caliban/schema/macros/Macros.scala @@ -5,7 +5,7 @@ import caliban.schema.Schema import scala.quoted.* -export magnolia1.TypeInfo +export magnolia1.{ Macro as MMacro, TypeInfo } object Macros { inline def isFieldExcluded[P, T]: Boolean = ${ isFieldExcludedImpl[P, T] } @@ -13,8 +13,12 @@ object Macros { inline def implicitExists[T]: Boolean = ${ implicitExistsImpl[T] } inline def hasAnnotation[T, Ann]: Boolean = ${ hasAnnotationImpl[T, Ann] } - transparent inline def fieldsFromMethods[R, T]: List[(String, List[Any], Schema[R, ?])] = - ${ fieldsFromMethodsImpl[R, T] } + transparent inline def hasFieldsFromMethods[T]: Boolean = + ${ hasFieldsFromMethodsImpl[T] } + + transparent inline def fieldsFromMethods[R, T]: List[(String, List[Any], Schema[R, ?])] = ${ + fieldsFromMethodsImpl[R, T] + } /** * Tests whether type argument [[FieldT]] in [[Parent]] is annotated with [[GQLExcluded]] @@ -76,18 +80,8 @@ object Macros { if (methodSym.signature.paramSigs.size > 0) report.errorAndAbort(s"Method '${methodSym.name}' annotated with @GQLField must be parameterless") - // Unfortunately we can't reuse Magnolias filtering so we copy the implementation - def filterAnnotation(ann: Term): Boolean = { - val tpe = ann.tpe - - tpe != annType && // No need to include the GQLField annotation - (tpe.typeSymbol.maybeOwner.isNoSymbol || - (tpe.typeSymbol.owner.fullName != "scala.annotation.internal" && - tpe.typeSymbol.owner.fullName != "jdk.internal")) - } - def extractAnnotations(methodSym: Symbol): List[Expr[Any]] = - methodSym.annotations.filter(filterAnnotation).map(_.asExpr.asInstanceOf[Expr[Any]]) + methodSym.annotations.filter(filterAnnotation(_, annType)).map(_.asExpr.asInstanceOf[Expr[Any]]) Expr.ofList { targetSym.declaredMethods @@ -105,6 +99,25 @@ object Macros { } } + private def hasFieldsFromMethodsImpl[T: Type](using q: Quotes): Expr[Boolean] = { + import q.reflect.* + val targetSym = TypeTree.of[T].symbol + val annType = TypeRepr.of[GQLField] + val annSym = annType.typeSymbol + + Expr(targetSym.declaredMethods.exists(_.getAnnotation(annSym).isDefined)) + } + + // Unfortunately we can't reuse Magnolias filtering so we copy the implementation + private def filterAnnotation(using q: Quotes)(ann: q.reflect.Term, annType: q.reflect.TypeRepr): Boolean = { + val tpe = ann.tpe + + tpe != annType && // No need to include the GQLField annotation + (tpe.typeSymbol.maybeOwner.isNoSymbol || + (tpe.typeSymbol.owner.fullName != "scala.annotation.internal" && + tpe.typeSymbol.owner.fullName != "jdk.internal")) + } + // Copied from Schema so that we have the same compiler error message private inline def schemaNotFound(tpe: String) = s"""Cannot find a Schema for type $tpe. diff --git a/core/src/test/scala-3/caliban/schema/Scala3DerivesSpec.scala b/core/src/test/scala-3/caliban/schema/Scala3DerivesSpec.scala index 2758c1fab..5f9cf90d5 100644 --- a/core/src/test/scala-3/caliban/schema/Scala3DerivesSpec.scala +++ b/core/src/test/scala-3/caliban/schema/Scala3DerivesSpec.scala @@ -292,27 +292,27 @@ object Scala3DerivesSpec extends ZIOSpecDefault { val expectedSchema = """schema { - query: Query -} - -"Union type Payload" -union Payload2 @mydirective(arg: "value") = Foo | Bar | Baz - -type Bar { - foo: Int! -} - -type Baz { - bar: Int! -} - -type Foo { - value: String! -} - -type Query { - testQuery(isFoo: Boolean!): Payload2! -}""".stripMargin + | query: Query + |} + | + |"Union type Payload" + |union Payload2 @mydirective(arg: "value") = Foo | Bar | Baz + | + |type Bar { + | foo: Int! + |} + | + |type Baz { + | bar: Int! + |} + | + |type Foo { + | value: String! + |} + | + |type Query { + | testQuery(isFoo: Boolean!): Payload2! + |}""".stripMargin val interpreter = gql.interpreterUnsafe for { @@ -325,6 +325,44 @@ type Query { data2 == """{"testQuery":{"foo":1}}""", gql.render == expectedSchema ) + }, + test("case object with @GQLField methods") { + case object Foo derives Schema.SemiAuto { + @GQLField def fooValue: Option[String] = None + @GQLField def barValue: Int = 42 + } + val rendered = graphQL(RootResolver(Foo)).render + + val expected = + """schema { + | query: Foo + |} + | + |type Foo { + | fooValue: String + | barValue: Int! + |}""".stripMargin + + assertTrue(rendered == expected) + }, + test("parameterless case class with @GQLField methods") { + case class Foo() derives Schema.SemiAuto { + @GQLField def fooValue: Option[String] = None + @GQLField def barValue: Int = 42 + } + val rendered = graphQL(RootResolver(Foo())).render + + val expected = + """schema { + | query: Foo + |} + | + |type Foo { + | fooValue: String + | barValue: Int! + |}""".stripMargin + + assertTrue(rendered == expected) } ) } From e9be7d16faa70e4368c5e06a6575a41d8f2ec4f4 Mon Sep 17 00:00:00 2001 From: Kyri Petrou Date: Tue, 25 Jun 2024 16:42:55 +1000 Subject: [PATCH 2/3] Cleanup --- .../caliban/schema/macros/Macros.scala | 47 +++++++++---------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/core/src/main/scala-3/caliban/schema/macros/Macros.scala b/core/src/main/scala-3/caliban/schema/macros/Macros.scala index 3161832cb..6b1b25b7d 100644 --- a/core/src/main/scala-3/caliban/schema/macros/Macros.scala +++ b/core/src/main/scala-3/caliban/schema/macros/Macros.scala @@ -5,7 +5,7 @@ import caliban.schema.Schema import scala.quoted.* -export magnolia1.{ Macro as MMacro, TypeInfo } +export magnolia1.TypeInfo object Macros { inline def isFieldExcluded[P, T]: Boolean = ${ isFieldExcludedImpl[P, T] } @@ -16,9 +16,8 @@ object Macros { transparent inline def hasFieldsFromMethods[T]: Boolean = ${ hasFieldsFromMethodsImpl[T] } - transparent inline def fieldsFromMethods[R, T]: List[(String, List[Any], Schema[R, ?])] = ${ - fieldsFromMethodsImpl[R, T] - } + transparent inline def fieldsFromMethods[R, T]: List[(String, List[Any], Schema[R, ?])] = + ${ fieldsFromMethodsImpl[R, T] } /** * Tests whether type argument [[FieldT]] in [[Parent]] is annotated with [[GQLExcluded]] @@ -52,6 +51,15 @@ object Macros { Expr(TypeRepr.of[P].typeSymbol.flags.is(Flags.Enum) && TypeRepr.of[T].typeSymbol.flags.is(Flags.Enum)) } + private def hasFieldsFromMethodsImpl[T: Type](using q: Quotes): Expr[Boolean] = { + import q.reflect.* + val targetSym = TypeTree.of[T].symbol + val annType = TypeRepr.of[GQLField] + val annSym = annType.typeSymbol + + Expr(targetSym.declaredMethods.exists(_.getAnnotation(annSym).isDefined)) + } + private def fieldsFromMethodsImpl[R: Type, T: Type](using q: Quotes ): Expr[List[(String, List[Any], Schema[R, ?])]] = { @@ -80,8 +88,18 @@ object Macros { if (methodSym.signature.paramSigs.size > 0) report.errorAndAbort(s"Method '${methodSym.name}' annotated with @GQLField must be parameterless") + // Unfortunately we can't reuse Magnolias filtering so we copy the implementation + def filterAnnotation(ann: Term): Boolean = { + val tpe = ann.tpe + + tpe != annType && // No need to include the GQLField annotation + (tpe.typeSymbol.maybeOwner.isNoSymbol || + (tpe.typeSymbol.owner.fullName != "scala.annotation.internal" && + tpe.typeSymbol.owner.fullName != "jdk.internal")) + } + def extractAnnotations(methodSym: Symbol): List[Expr[Any]] = - methodSym.annotations.filter(filterAnnotation(_, annType)).map(_.asExpr.asInstanceOf[Expr[Any]]) + methodSym.annotations.filter(filterAnnotation).map(_.asExpr.asInstanceOf[Expr[Any]]) Expr.ofList { targetSym.declaredMethods @@ -99,25 +117,6 @@ object Macros { } } - private def hasFieldsFromMethodsImpl[T: Type](using q: Quotes): Expr[Boolean] = { - import q.reflect.* - val targetSym = TypeTree.of[T].symbol - val annType = TypeRepr.of[GQLField] - val annSym = annType.typeSymbol - - Expr(targetSym.declaredMethods.exists(_.getAnnotation(annSym).isDefined)) - } - - // Unfortunately we can't reuse Magnolias filtering so we copy the implementation - private def filterAnnotation(using q: Quotes)(ann: q.reflect.Term, annType: q.reflect.TypeRepr): Boolean = { - val tpe = ann.tpe - - tpe != annType && // No need to include the GQLField annotation - (tpe.typeSymbol.maybeOwner.isNoSymbol || - (tpe.typeSymbol.owner.fullName != "scala.annotation.internal" && - tpe.typeSymbol.owner.fullName != "jdk.internal")) - } - // Copied from Schema so that we have the same compiler error message private inline def schemaNotFound(tpe: String) = s"""Cannot find a Schema for type $tpe. From d1d51ad90badd0983aac54148718a0961b9283c5 Mon Sep 17 00:00:00 2001 From: Kyri Petrou Date: Tue, 25 Jun 2024 17:15:00 +1000 Subject: [PATCH 3/3] Put tests under the correct suite --- .../caliban/schema/Scala3DerivesSpec.scala | 78 +++++++++---------- 1 file changed, 39 insertions(+), 39 deletions(-) diff --git a/core/src/test/scala-3/caliban/schema/Scala3DerivesSpec.scala b/core/src/test/scala-3/caliban/schema/Scala3DerivesSpec.scala index 5f9cf90d5..cfe3da054 100644 --- a/core/src/test/scala-3/caliban/schema/Scala3DerivesSpec.scala +++ b/core/src/test/scala-3/caliban/schema/Scala3DerivesSpec.scala @@ -176,7 +176,7 @@ object Scala3DerivesSpec extends ZIOSpecDefault { ) } ), - suite("methods as fields") { + suite("@GQLField annotation") { val expectedSchema = """schema { | query: Bar @@ -244,6 +244,44 @@ object Scala3DerivesSpec extends ZIOSpecDefault { assertTrue(s == """{"foo":{"value":"foo","value2":"foo2"}}""") } } + }, + test("case object") { + case object Foo derives Schema.SemiAuto { + @GQLField def fooValue: Option[String] = None + @GQLField def barValue: Int = 42 + } + val rendered = graphQL(RootResolver(Foo)).render + + val expected = + """schema { + | query: Foo + |} + | + |type Foo { + | fooValue: String + | barValue: Int! + |}""".stripMargin + + assertTrue(rendered == expected) + }, + test("parameterless case class") { + case class Foo() derives Schema.SemiAuto { + @GQLField def fooValue: Option[String] = None + @GQLField def barValue: Int = 42 + } + val rendered = graphQL(RootResolver(Foo())).render + + val expected = + """schema { + | query: Foo + |} + | + |type Foo { + | fooValue: String + | barValue: Int! + |}""".stripMargin + + assertTrue(rendered == expected) } ) }, @@ -325,44 +363,6 @@ object Scala3DerivesSpec extends ZIOSpecDefault { data2 == """{"testQuery":{"foo":1}}""", gql.render == expectedSchema ) - }, - test("case object with @GQLField methods") { - case object Foo derives Schema.SemiAuto { - @GQLField def fooValue: Option[String] = None - @GQLField def barValue: Int = 42 - } - val rendered = graphQL(RootResolver(Foo)).render - - val expected = - """schema { - | query: Foo - |} - | - |type Foo { - | fooValue: String - | barValue: Int! - |}""".stripMargin - - assertTrue(rendered == expected) - }, - test("parameterless case class with @GQLField methods") { - case class Foo() derives Schema.SemiAuto { - @GQLField def fooValue: Option[String] = None - @GQLField def barValue: Int = 42 - } - val rendered = graphQL(RootResolver(Foo())).render - - val expected = - """schema { - | query: Foo - |} - | - |type Foo { - | fooValue: String - | barValue: Int! - |}""".stripMargin - - assertTrue(rendered == expected) } ) }