From e086cb95e13b995123c6f0807a1cce40c3582816 Mon Sep 17 00:00:00 2001
From: kyri-petrou <67301607+kyri-petrou@users.noreply.github.com>
Date: Mon, 6 Nov 2023 13:17:03 +1100
Subject: [PATCH] Improve interface field description (#1976)

* Improve interface field description

* Fix Scala 2.12 and make ordering of interface fields deterministic
---
 .../caliban/schema/SchemaDerivation.scala     |  5 +-
 .../caliban/schema/SchemaDerivation.scala     |  5 +-
 .../src/main/scala/caliban/schema/Types.scala |  6 ++
 .../schema/SchemaDerivationIssuesSpec.scala   | 59 +++++++++++++++++--
 .../scala/caliban/schema/SchemaSpec.scala     |  2 +-
 5 files changed, 68 insertions(+), 9 deletions(-)

diff --git a/core/src/main/scala-2/caliban/schema/SchemaDerivation.scala b/core/src/main/scala-2/caliban/schema/SchemaDerivation.scala
index fd5e6a565a..84a43813a2 100644
--- a/core/src/main/scala-2/caliban/schema/SchemaDerivation.scala
+++ b/core/src/main/scala-2/caliban/schema/SchemaDerivation.scala
@@ -171,10 +171,13 @@ trait CommonSchemaDerivation[R] {
             .collect { case (_, list) =>
               Types
                 .unify(list)
-                .flatMap(t => list.headOption.map(_.copy(`type` = () => t)))
+                .flatMap(t =>
+                  list.headOption.map(_.copy(description = Types.extractCommonDescription(list), `type` = () => t))
+                )
             }
             .flatten
             .toList
+            .sortBy(_.name)
 
         makeInterface(
           Some(getName(ctx)),
diff --git a/core/src/main/scala-3/caliban/schema/SchemaDerivation.scala b/core/src/main/scala-3/caliban/schema/SchemaDerivation.scala
index 6faefb4d6c..d21f578ca5 100644
--- a/core/src/main/scala-3/caliban/schema/SchemaDerivation.scala
+++ b/core/src/main/scala-3/caliban/schema/SchemaDerivation.scala
@@ -283,10 +283,13 @@ trait CommonSchemaDerivation {
           case (_, list) if list.lengthCompare(impl.size) == 0 =>
             Types
               .unify(list)
-              .flatMap(t => list.headOption.map(_.copy(`type` = () => t)))
+              .flatMap(t =>
+                list.headOption.map(_.copy(description = Types.extractCommonDescription(list), `type` = () => t))
+              )
         }
         .flatten
         .toList
+        .sortBy(_.name)
 
     makeInterface(
       Some(getName(annotations, info)),
diff --git a/core/src/main/scala/caliban/schema/Types.scala b/core/src/main/scala/caliban/schema/Types.scala
index 5e4419d902..72b72da762 100644
--- a/core/src/main/scala/caliban/schema/Types.scala
+++ b/core/src/main/scala/caliban/schema/Types.scala
@@ -212,6 +212,12 @@ object Types {
         case _                        => None
       }
 
+  def extractCommonDescription(l: List[__Field]): Option[String] =
+    l.map(_.description).distinct match {
+      case desc :: Nil => desc
+      case _           => None
+    }
+
   @tailrec
   def same(t1: __Type, t2: __Type): Boolean =
     if (t1.kind == t2.kind && t1.ofType.nonEmpty)
diff --git a/core/src/test/scala/caliban/schema/SchemaDerivationIssuesSpec.scala b/core/src/test/scala/caliban/schema/SchemaDerivationIssuesSpec.scala
index f44acb2d7d..07d7cc3356 100644
--- a/core/src/test/scala/caliban/schema/SchemaDerivationIssuesSpec.scala
+++ b/core/src/test/scala/caliban/schema/SchemaDerivationIssuesSpec.scala
@@ -1,6 +1,6 @@
 package caliban.schema
 
-import caliban.schema.Annotations.{ GQLInterface, GQLUnion }
+import caliban.schema.Annotations.{ GQLDescription, GQLInterface, GQLUnion }
 import zio.Chunk
 import zio.test.ZIOSpecDefault
 import zio.test._
@@ -11,11 +11,6 @@ object SchemaDerivationIssuesSpec extends ZIOSpecDefault {
     test("i1972 & i1973") {
       import i1972_i1973._
 
-      val schema = {
-        val queries = Queries(param = Variable("temp"))
-        graphQL(RootResolver(queries))
-      }.render
-
       assertTrue(
         schema ==
           """schema {
@@ -72,6 +67,40 @@ object SchemaDerivationIssuesSpec extends ZIOSpecDefault {
             |  param: ASTParameter!
             |}""".stripMargin
       )
+    },
+    test("i1951") {
+      import i1951._
+
+      assertTrue(
+        schema ==
+          """schema {
+            |  query: Queries
+            |}
+            |
+            |interface MyInterface {
+            |  "a"
+            |  first: String!
+            |  second: String!
+            |}
+            |
+            |type Bar implements MyInterface {
+            |  "a"
+            |  first: String!
+            |  "c"
+            |  second: String!
+            |}
+            |
+            |type Foo implements MyInterface {
+            |  "a"
+            |  first: String!
+            |  "b"
+            |  second: String!
+            |}
+            |
+            |type Queries {
+            |  param: MyInterface!
+            |}""".stripMargin
+      )
     }
   )
 }
@@ -113,7 +142,25 @@ private object i1972_i1973 {
   val schema = {
     val queries = Queries(param = Variable("temp"))
     graphQL(RootResolver(queries))
+  }.render
+}
+
+private object i1951 {
+  import Schema.auto._
+
+  @GQLInterface
+  sealed trait MyInterface
+  object MyInterface {
+    case class Foo(@GQLDescription("a") first: String, @GQLDescription("b") second: String) extends MyInterface
+    case class Bar(@GQLDescription("a") first: String, @GQLDescription("c") second: String) extends MyInterface
   }
+
+  case class Queries(param: MyInterface)
+
+  val schema = {
+    val queries = Queries(param = MyInterface.Foo("a", "b"))
+    graphQL(RootResolver(queries))
+  }.render
 }
 
 private object i1977 {
diff --git a/core/src/test/scala/caliban/schema/SchemaSpec.scala b/core/src/test/scala/caliban/schema/SchemaSpec.scala
index 1b162282d0..abe73abd91 100644
--- a/core/src/test/scala/caliban/schema/SchemaSpec.scala
+++ b/core/src/test/scala/caliban/schema/SchemaSpec.scala
@@ -87,7 +87,7 @@ object SchemaSpec extends ZIOSpecDefault {
         )
       },
       test("interface only take fields that return the same type") {
-        assertTrue(introspect[MyInterface].fields(__DeprecatedArgs()).toList.flatten.map(_.name) == List("c2", "c1"))
+        assertTrue(introspect[MyInterface].fields(__DeprecatedArgs()).toList.flatten.map(_.name) == List("c1", "c2"))
       },
       test("enum-like sealed traits annotated with GQLUnion") {
         assert(introspect[EnumLikeUnion])(