feat: validate duplicate typename between output and input

ValdemarGr · Sep 1, 2023 · b83b336 · b83b336
1 parent d76021e
commit b83b336
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 15 deletions.
diff --git a/modules/core/src/main/scala/gql/SchemaShape.scala b/modules/core/src/main/scala/gql/SchemaShape.scala
@@ -59,8 +59,8 @@ final case class SchemaShape[F[_], Q, M, S](
 
   lazy val ast = SchemaUtil.toAst[F](this)
 
-  // This is safe by construction
-  lazy val stub = SchemaUtil.stubSchema(ast).toOption.get
+  // This is safe by construction if your schema is valid
+  lazy val stub = SchemaUtil.stubSchema(ast).fold(xs => throw new RuntimeException(xs.toList.mkString("\n")), identity)
 
   lazy val stubInputs = SchemaShape.discover(stub).inputs
 }

diff --git a/modules/core/src/main/scala/gql/Validation.scala b/modules/core/src/main/scala/gql/Validation.scala
@@ -31,6 +31,10 @@ object Validation {
     def message: String
   }
   object Error {
+    final case class DuplicateTypenameInInputAndOutput(typename: String) extends Error {
+      def message: String =
+        s"Typename `$typename` appears both as input and output type. You must rename one of them such as `Input$typename`."
+    }
     final case class DivergingTypeReference(typename: String) extends Error {
       def message: String =
         s"`$typename` is not reference equal. Use lazy val or `cats.Eval` to declare this type."
@@ -205,6 +209,9 @@ object Validation {
         S.modify(_.copy(currentPath = s.currentPath))
     }
 
+  def getDiscovery[F[_]](name: String, discovery: SchemaShape.DiscoveryState[F]) =
+    (discovery.inputs.get(name), discovery.outputs.get(name))
+
   def useOutputEdge[F[_], G[_]](sel: Selectable[F, ?], discovery: SchemaShape.DiscoveryState[F])(
       fa: G[Unit]
   )(implicit G: Monad[G], S: Stateful[G, ValidationState[F]]): G[Unit] =
@@ -213,12 +220,13 @@ object Validation {
         case Some(o) if o eq sel => G.unit
         case Some(_)             => raise(CyclicDivergingTypeReference(sel.name))
         case None =>
-          discovery.outputs
-            .get(sel.name)
-            .traverse_ {
-              case o if o eq sel => G.unit
-              case _             => raise(DivergingTypeReference(sel.name))
-            } >>
+          val checkF = getDiscovery(sel.name, discovery) match {
+            case (Some(_), _)                   => raise(DuplicateTypenameInInputAndOutput(sel.name))
+            case (None, Some(o)) if !(o eq sel) => raise(DivergingTypeReference(sel.name))
+            case _                              => G.unit
+          }
+
+          checkF >>
             S.modify(s => s.copy(seenOutputs = s.seenOutputs + (sel.name -> sel))) *>
             fa <*
             S.modify(s => s.copy(seenOutputs = s.seenOutputs - sel.name))
@@ -233,12 +241,13 @@ object Validation {
         case Some(i) if i eq it => G.unit
         case Some(_)            => raise(CyclicDivergingTypeReference(it.name))
         case None =>
-          discovery.inputs
-            .get(it.name)
-            .traverse_ {
-              case i if i eq it => G.unit
-              case _            => raise(DivergingTypeReference(it.name))
-            } >>
+          val checkF = getDiscovery(it.name, discovery) match {
+            case (_, Some(_))                  => raise(DuplicateTypenameInInputAndOutput(it.name))
+            case (Some(i), None) if !(i eq it) => raise(DivergingTypeReference(it.name))
+            case _                             => G.unit
+          }
+
+          checkF >>
             S.modify(s => s.copy(seenInputs = s.seenInputs + (it.name -> it))) *>
             fa <*
             S.modify(s => s.copy(seenInputs = s.seenInputs - it.name))

diff --git a/modules/core/src/main/scala/gql/ast.scala b/modules/core/src/main/scala/gql/ast.scala
@@ -43,7 +43,7 @@ object ast extends AstImplicits.Implicits {
     def anyFields: List[(String, AnyField[F, ?, ?])]
 
     lazy val abstractFields: List[(String, AbstractField[F, ?])] =
-      anyFields.map{ case (k, v) => (k, v.asAbstract) }
+      anyFields.map { case (k, v) => (k, v.asAbstract) }
 
     lazy val abstractFieldMap: Map[String, AbstractField[F, ?]] =
       abstractFields.toMap

diff --git a/modules/server/src/test/scala/gql/ValidationTest.scala b/modules/server/src/test/scala/gql/ValidationTest.scala
@@ -103,8 +103,20 @@ class ValidationTest extends CatsEffectSuite {
     "four" -> lift(arg[Int]("x")) { case _ => "heya" }
   ).subtypeOf[Catch]
 
+  case object InOutSameName
+  implicit lazy val inOutSameName: Type[IO, InOutSameName.type] = tpe[IO, InOutSameName.type](
+    "InOutSameName",
+    "blah" -> lift(_ => "blah")
+  )
+
+  implicit lazy val inOutSameNameInput: Input[InOutSameName.type] = input[InOutSameName.type](
+    "InOutSameName",
+    arg[String]("blah").map(_ => InOutSameName)
+  )
+
   lazy val schemaShape: SchemaShape[IO, Unit, Unit, Unit] = SchemaShape.unit[IO](
     fields(
+      "inOutSameName" -> lift(arg[InOutSameName.type]("blah"))((_, _) => InOutSameName),
       "badStructure" -> lift(_ => BadStructure()),
       "duplicateUnion" -> lift(_ => (MutuallyRecursive1(42): MutRecUnion)),
       "duplicateInterface" -> lift(_ => (MutuallyRecursive1(42): MutRecInterface)),
@@ -154,6 +166,14 @@ class ValidationTest extends CatsEffectSuite {
     //     Nil
     // })
   }
+
+  test("should catch input and output types with same name") {
+    val err = errors.collect { case (Validation.Error.DuplicateTypenameInInputAndOutput("InOutSameName"), path) =>
+      path.path.toList
+    }
+
+    assert(clue(err).size == 1)
+  }
 }
 
 object ValidationTest {