code review

build.sbt

@@ -58,6 +58,13 @@ lazy val ast = project
     name := "sangria-ast",
     description := "Scala GraphQL AST representation",
     mimaPreviousArtifacts := Set("org.sangria-graphql" %% "sangria-ast" % "4.0.0"),
+    mimaBinaryIssueFilters ++= Seq(
+      ProblemFilters.exclude[IncompatibleMethTypeProblem]("sangria.ast.Document.merge"),
+      ProblemFilters.exclude[IncompatibleMethTypeProblem](
+        "sangria.ast.AggregateSourceMapper.merge"),
+      ProblemFilters.exclude[DirectMissingMethodProblem](
+        "sangria.ast.AggregateSourceMapper.delegateById")
+    ),
     apiURL := {
       val ver = CrossVersion.binaryScalaVersion(scalaVersion.value)
       Some(url(s"https://www.javadoc.io/doc/org.sangria-graphql/sangria-ast_$ver/latest/"))

modules/ast/src/main/scala/sangria/ast/QueryAst.scala

@@ -85,8 +85,8 @@ object Document {
     *
     * The result `Document` will retain correlation to the original `sourceMapper`s.
     */
-  def merge(documents: Traversable[Document]): Document = {
-    val originalSourceMappers = documents.flatMap(_.sourceMapper).toVector
+  def merge(documents: Iterable[Document]): Document = {
+    val originalSourceMappers = documents.flatMap(_.sourceMapper)
     val sourceMapper =
       if (originalSourceMappers.nonEmpty) Some(AggregateSourceMapper.merge(originalSourceMappers))
       else None

modules/ast/src/main/scala/sangria/ast/SourceMapper.scala

@@ -58,7 +58,8 @@ class DefaultSourceMapper(val id: String, val sourceMapperInput: SourceMapperInp
   */
 class AggregateSourceMapper(val id: String, val delegates: Vector[SourceMapper])
     extends SourceMapper {
-  lazy val delegateById: Map[String, SourceMapper] = delegates.iterator.map(d => d.id -> d).toMap
+  private lazy val delegateById: Map[String, SourceMapper] =
+    delegates.iterator.map(d => d.id -> d).toMap
 
   override lazy val source: String = delegates.map(_.source.trim).mkString("\n\n")
 
@@ -75,6 +76,6 @@ object AggregateSourceMapper {
     case m => Vector(m)
   }
 
-  def merge(mappers: Vector[SourceMapper]): AggregateSourceMapper =
-    new AggregateSourceMapper("merged", mappers.flatMap(expand))
+  def merge(mappers: Iterable[SourceMapper]): AggregateSourceMapper =
+    new AggregateSourceMapper("merged", mappers.flatMap(expand).toVector)
 }

modules/core/src/main/scala/sangria/execution/batch/BatchExecutor.scala

@@ -32,6 +32,8 @@ import scala.util.{Failure, Success, Try}
 import scala.util.control.Breaks.{break, breakable}
 import BatchExecutionPlan._
 
+import scala.annotation.tailrec
+
 /** __EXPERIMENTAL__
   *
   * Batch query executor which provides following features:
@@ -340,12 +342,14 @@ object BatchExecutor {
     }
   }
 
+  @tailrec
   private def isInputList(tpe: Type): Boolean = tpe match {
     case _: ListInputType[_] => true
     case OptionInputType(ofType) => isInputList(ofType)
     case _ => false
   }
 
+  @tailrec
   private def isInputList(tpe: ast.Type): Boolean = tpe match {
     case _: ast.ListType => true
     case ast.NotNullType(ofType, _) => isInputList(ofType)

modules/core/src/main/scala/sangria/execution/middleware.scala

@@ -1,6 +1,6 @@
 package sangria.execution
 
-import language.{higherKinds, implicitConversions}
+import language.implicitConversions
 import sangria.marshalling.InputUnmarshaller
 
 import sangria.ast

modules/core/src/main/scala/sangria/introspection/package.scala

@@ -2,10 +2,11 @@ package sangria
 
 import sangria.parser.QueryParser
 import sangria.schema._
-
-import sangria.util.tag.@@ // Scala 3 issue workaround
+import sangria.util.tag.@@
 import sangria.marshalling.FromInput.CoercedScalaResult
 
+import scala.annotation.tailrec
+
 package object introspection {
   object TypeKind extends Enumeration {
     val Scalar, Object, Interface, Union, Enum, InputObject, List, NonNull = Value
@@ -194,6 +195,7 @@ package object introspection {
     false)
 
   private def getKind(value: (Boolean, Type)) = {
+    @tailrec
     def identifyKind(t: Type, optional: Boolean): TypeKind.Value = t match {
       case OptionType(ofType) => identifyKind(ofType, true)
       case OptionInputType(ofType) => identifyKind(ofType, true)
@@ -213,6 +215,7 @@ package object introspection {
     identifyKind(tpe, fromTypeList)
   }
 
+  @tailrec
   private def findNamed(tpe: Type): Option[Type with Named] = tpe match {
     case o: OptionType[_] => findNamed(o.ofType)
     case o: OptionInputType[_] => findNamed(o.ofType)
@@ -222,6 +225,7 @@ package object introspection {
     case _ => None
   }
 
+  @tailrec
   private def findListType(tpe: Type): Option[Type] = tpe match {
     case o: OptionType[_] => findListType(o.ofType)
     case o: OptionInputType[_] => findListType(o.ofType)

modules/core/src/main/scala/sangria/marshalling/queryAst.scala

@@ -91,7 +91,7 @@ class QueryAstResultMarshaller extends ResultMarshaller {
 
   def enumNode(value: String, typeName: String) = ast.EnumValue(value)
 
-  def arrayNode(values: Vector[Node]) = ast.ListValue(values.toVector)
+  def arrayNode(values: Vector[Node]) = ast.ListValue(values)
   def optionalArrayNodeValue(value: Option[Node]) = value match {
     case Some(v) => v
     case None => nullNode

modules/core/src/main/scala/sangria/schema/Context.scala

@@ -7,7 +7,6 @@ import sangria.marshalling._
 import sangria.util.Cache
 import sangria.{ast, introspection}
 
-import scala.language.implicitConversions
 import scala.reflect.ClassTag
 
 case class MappingDeferred[A, +B](deferred: Deferred[A], mapFn: A => (B, Vector[Throwable]))

modules/core/src/main/scala/sangria/schema/Schema.scala

@@ -1522,7 +1522,7 @@ case class Schema[Ctx, Val](
       .toMap
 
   lazy val directivesByName: Map[String, Directive] =
-    directives.groupBy(_.name).mapValues(_.head).toMap
+    directives.groupBy(_.name).iterator.map { case (k, v) => (k, v.head) }.toMap
 
   def getInputType(tpe: ast.Type): Option[InputType[_]] = tpe match {
     case ast.NamedType(name, _) =>

modules/core/src/main/scala/sangria/validation/TypeComparators.scala

@@ -2,7 +2,10 @@ package sangria.validation
 
 import sangria.schema._
 
+import scala.annotation.tailrec
+
 object TypeComparators {
+  @tailrec
   def isEqualType(type1: Type, type2: Type): Boolean =
     (type1, type2) match {
       case (OptionType(t1), OptionType(t2)) => isEqualType(t1, t2)
@@ -13,6 +16,7 @@ object TypeComparators {
       case _ => false
     }
 
+  @tailrec
   def isSubType(schema: Schema[_, _], subType: Type, superType: Type): Boolean =
     (subType, superType) match {
       case (OptionType(ofType1), OptionType(ofType2)) => isSubType(schema, ofType1, ofType2)

modules/core/src/main/scala/sangria/validation/Violation.scala

@@ -1110,7 +1110,7 @@ case class NotExactlyOneOfField(
     sourceMapper: Option[SourceMapper],
     locations: List[AstLocation]
 ) extends AstNodeViolation {
-  lazy val simpleErrorMessage = s"Exactly one key must be specified for oneOf type '${typeName}'."
+  lazy val simpleErrorMessage = s"Exactly one key must be specified for oneOf type '$typeName'."
 }
 
 case class OneOfMandatoryField(

modules/derivation/src/main/scala-2/sangria/macros/derive/DeriveMacroSupport.scala

@@ -20,43 +20,37 @@ trait DeriveMacroSupport {
   }
 
   protected def symbolName(annotations: List[Annotation]): Option[Tree] =
-    annotations
+    annotations.iterator
       .map(_.tree)
-      .collect { case q"new $name($arg)" if name.tpe =:= typeOf[GraphQLName] => arg }
-      .headOption
+      .collectFirst { case q"new $name($arg)" if name.tpe =:= typeOf[GraphQLName] => arg }
 
   protected def symbolOutputType(annotations: List[Annotation]): Option[Tree] =
-    annotations
+    annotations.iterator
       .map(_.tree)
-      .collect { case q"new $name($arg)" if name.tpe =:= typeOf[GraphQLOutputType] => arg }
-      .headOption
+      .collectFirst { case q"new $name($arg)" if name.tpe =:= typeOf[GraphQLOutputType] => arg }
 
   protected def symbolInputType(annotations: List[Annotation]): Option[Tree] =
-    annotations
+    annotations.iterator
       .map(_.tree)
-      .collect { case q"new $name($arg)" if name.tpe =:= typeOf[GraphQLInputType] => arg }
-      .headOption
+      .collectFirst { case q"new $name($arg)" if name.tpe =:= typeOf[GraphQLInputType] => arg }
 
   protected def symbolDescription(annotations: List[Annotation]): Option[Tree] =
-    annotations
+    annotations.iterator
       .map(_.tree)
-      .collect { case q"new $name($arg)" if name.tpe =:= typeOf[GraphQLDescription] => arg }
-      .headOption
+      .collectFirst { case q"new $name($arg)" if name.tpe =:= typeOf[GraphQLDescription] => arg }
 
   protected def symbolDefault(annotations: List[Annotation]): Option[Tree] =
-    annotations
+    annotations.iterator
       .map(_.tree)
-      .collect { case q"new $name($arg)" if name.tpe =:= typeOf[GraphQLDefault] => arg }
-      .headOption
+      .collectFirst { case q"new $name($arg)" if name.tpe =:= typeOf[GraphQLDefault] => arg }
 
   protected def symbolDeprecation(annotations: List[Annotation]): Option[Tree] =
-    annotations
+    annotations.iterator
       .map(_.tree)
-      .collect { case q"new $name($arg)" if name.tpe =:= typeOf[GraphQLDeprecated] => arg }
-      .headOption
+      .collectFirst { case q"new $name($arg)" if name.tpe =:= typeOf[GraphQLDeprecated] => arg }
 
   protected def symbolFieldTags(annotations: List[Annotation]): Tree =
-    annotations
+    annotations.iterator
       .map(_.tree)
       .foldLeft(q"List[sangria.execution.FieldTag]()") {
         case (acc, q"new $name(..$fieldTags)") if name.tpe =:= typeOf[GraphQLFieldTags] =>
@@ -65,10 +59,10 @@ trait DeriveMacroSupport {
       }
 
   protected def memberExcluded(annotations: List[Annotation]): Boolean =
-    annotations.find(_.tree.tpe =:= typeOf[GraphQLExclude]).fold(false)(_ => true)
+    annotations.exists(_.tree.tpe =:= typeOf[GraphQLExclude])
 
   protected def memberField(annotations: List[Annotation]): Boolean =
-    annotations.find(_.tree.tpe =:= typeOf[GraphQLField]).fold(false)(_ => true)
+    annotations.exists(_.tree.tpe =:= typeOf[GraphQLField])
 
   // TODO: most probably not needed, so should be removed in future
   protected def defaultMethodArgValue(method: String, pos: Int) = {