Address PR reviews

core/src/main/scala/caliban/CalibanError.scala

@@ -1,5 +1,6 @@
 package caliban
 
+import caliban.interop.circe.IsCirceEncoder
 import caliban.parsing.adt.LocationInfo
 
 /**
@@ -14,14 +15,19 @@ object CalibanError {
   /**
    * Describes an error that happened while parsing a query.
    */
-  case class ParsingError(msg: String, innerThrowable: Option[Throwable] = None) extends CalibanError {
+  case class ParsingError(
+    msg: String,
+    locationInfo: Option[LocationInfo] = None,
+    innerThrowable: Option[Throwable] = None
+  ) extends CalibanError {
     override def toString: String = s"""Parsing error: $msg ${innerThrowable.fold("")(_.toString)}"""
   }
 
   /**
    * Describes an error that happened while validating a query.
    */
-  case class ValidationError(msg: String, explanatoryText: String) extends CalibanError {
+  case class ValidationError(msg: String, explanatoryText: String, locationInfo: Option[LocationInfo] = None)
+      extends CalibanError {
     override def toString: String = s"""Validation error: $msg $explanatoryText"""
   }
 
@@ -44,4 +50,53 @@ object CalibanError {
       s"Execution error$field: $msg$inner"
     }
   }
+
+  implicit def circeEncoder[F[_]](implicit ev: IsCirceEncoder[F]): F[CalibanError] =
+    ErrorCirce.errorValueEncoder.asInstanceOf[F[CalibanError]]
+}
+
+private object ErrorCirce {
+  import io.circe._
+  import io.circe.syntax._
+
+  private def locationToJson(li: LocationInfo): Json =
+    Json.obj("line" -> li.line.asJson, "column" -> li.column.asJson)
+
+  val errorValueEncoder: Encoder[CalibanError] = Encoder.instance[CalibanError] {
+    case CalibanError.ParsingError(msg, locationInfo, _) =>
+      Json
+        .obj(
+          "message" -> msg.asJson,
+          "locations" -> Some(locationInfo).collect {
+            case Some(li) => Json.arr(locationToJson(li))
+          }.asJson
+        )
+        .dropNullValues
+    case CalibanError.ValidationError(msg, _, locationInfo) =>
+      Json
+        .obj(
+          "message" -> msg.asJson,
+          "locations" -> Some(locationInfo).collect {
+            case Some(li) => Json.arr(locationToJson(li))
+          }.asJson
+        )
+        .dropNullValues
+    case CalibanError.ExecutionError(msg, path, locationInfo, _) =>
+      Json
+        .obj(
+          "message" -> msg.asJson,
+          "locations" -> Some(locationInfo).collect {
+            case Some(li) => Json.arr(locationToJson(li))
+          }.asJson,
+          "path" -> Some(path).collect {
+            case p if p.nonEmpty =>
+              Json.fromValues(p.map {
+                case Left(value)  => value.asJson
+                case Right(value) => value.asJson
+              })
+          }.asJson
+        )
+        .dropNullValues
+  }
+
 }

core/src/main/scala/caliban/GraphQLResponse.scala

@@ -1,6 +1,5 @@
 package caliban
 
-import caliban.CalibanError.ExecutionError
 import caliban.ResponseValue.ObjectValue
 import caliban.interop.circe._
 
@@ -34,21 +33,8 @@ private object GraphQLResponseCirce {
 
   private def handleError(err: Any): Json =
     err match {
-      case ExecutionError(_, path, location, _) if path.nonEmpty =>
-        val locationJson =
-          location.fold(Json.obj())(
-            l => Json.obj("location" -> Json.obj("column" -> l.column.asJson, "line" -> l.line.asJson))
-          )
-        Json.obj(
-          "message" -> Json.fromString(err.toString),
-          "path" -> Json
-            .fromValues(path.map {
-              case Left(value)  => Json.fromString(value)
-              case Right(value) => Json.fromInt(value)
-            })
-            .deepMerge(locationJson)
-        )
-      case _ => Json.obj("message" -> Json.fromString(err.toString))
+      case ce: CalibanError => ce.asJson
+      case _                => Json.obj("message" -> Json.fromString(err.toString))
     }
 
 }

core/src/main/scala/caliban/execution/Executor.scala

@@ -1,5 +1,7 @@
 package caliban.execution
 
+import caliban.CalibanError.ExecutionError
+
 import scala.annotation.tailrec
 import scala.collection.immutable.ListMap
 import caliban.ResponseValue._
@@ -57,13 +59,11 @@ object Executor {
       step match {
         case s @ PureStep(value) =>
           value match {
-            case EnumValue(v) if mergeFields(currentField, v).collectFirst {
-                  case Field("__typename", _, _, _, _, _, _, _) => true
-                }.nonEmpty =>
+            case EnumValue(v) =>
               // special case of an hybrid union containing case objects, those should return an object instead of a string
               val obj = mergeFields(currentField, v).collectFirst {
-                case Field(name @ "__typename", _, _, alias, _, _, _, _) =>
-                  ObjectValue(List(alias.getOrElse(name) -> StringValue(v)))
+                case f: Field if f.name == "__typename" =>
+                  ObjectValue(List(f.alias.getOrElse(f.name) -> StringValue(v)))
               }
               obj.fold(s)(PureStep(_))
             case _ => s
@@ -92,14 +92,14 @@ object Executor {
         case QueryStep(inner) =>
           ReducedStep.QueryStep(
             inner.bimap(
-              GenericSchema.effectfulExecutionError(path, Some(currentField.locationInfo), _),
+              effectfulExecutionError(path, Some(currentField.locationInfo), _),
               reduceStep(_, currentField, arguments, path)
             )
           )
         case StreamStep(stream) =>
           ReducedStep.StreamStep(
             stream.bimap(
-              GenericSchema.effectfulExecutionError(path, Some(currentField.locationInfo), _),
+              effectfulExecutionError(path, Some(currentField.locationInfo), _),
               reduceStep(_, currentField, arguments, path)
             )
           )
@@ -207,4 +207,14 @@ object Executor {
       }))
     else ReducedStep.ObjectStep(items)
 
+  private def effectfulExecutionError(
+    path: List[Either[String, Int]],
+    locationInfo: Option[LocationInfo],
+    e: Throwable
+  ): ExecutionError =
+    e match {
+      case e: ExecutionError => e
+      case other             => ExecutionError("Effect failure", path.reverse, locationInfo, Some(other))
+    }
+
 }

core/src/main/scala/caliban/parsing/Parser.scala

@@ -191,13 +191,15 @@ object Parser {
   /**
    * Parses the given string into a [[caliban.parsing.adt.Document]] object or fails with a [[caliban.CalibanError.ParsingError]].
    */
-  def parseQuery(query: String): IO[ParsingError, Document] =
+  def parseQuery(query: String): IO[ParsingError, Document] = {
+    val sm = SourceMapper(query)
     Task(parse(query, document(_)))
-      .mapError(ex => ParsingError(s"Internal parsing error", Some(ex)))
+      .mapError(ex => ParsingError(s"Internal parsing error", innerThrowable = Some(ex)))
       .flatMap {
-        case Parsed.Success(value, _) => IO.succeed(Document(value.definitions, SourceMapper(query)))
-        case f: Parsed.Failure        => IO.fail(ParsingError(f.msg))
+        case Parsed.Success(value, _) => IO.succeed(Document(value.definitions, sm))
+        case f: Parsed.Failure        => IO.fail(ParsingError(f.msg, Some(sm.getLocation(f.index))))
       }
+  }
 
   /**
    * Checks if the query is valid, if not returns an error string.

core/src/main/scala/caliban/parsing/SourceMapper.scala

@@ -15,7 +15,10 @@ trait SourceMapper {
 object SourceMapper {
 
   /**
-   * Implementation taken from https://github.com/lihaoyi/fastparse/blob/master/fastparse/src/fastparse/ParserInput.scala
+   * Implementation taken from https://github.com/lihaoyi/fastparse/blob/e334ca88b747fb3b6637ef6d76715ad66e048a6c/fastparse/src/fastparse/ParserInput.scala#L123-L131
+   *
+   * It is used to look up a line/column number pair given a raw index into a source string. The numbers are determined by
+   * computing the number of newlines occurring between 0 and the current index.
    */
   private[parsing] case class DefaultSourceMapper(source: String) extends SourceMapper {
     private[this] lazy val lineNumberLookup = Util.lineNumberLookup(source)

core/src/main/scala/caliban/schema/Schema.scala

@@ -417,16 +417,3 @@ trait DerivationSchema[R] {
   implicit def gen[T]: Typeclass[T] = macro Magnolia.gen[T]
 
 }
-
-object GenericSchema {
-
-  def effectfulExecutionError(
-    path: List[Either[String, Int]],
-    locationInfo: Option[LocationInfo],
-    e: Throwable
-  ): ExecutionError =
-    e match {
-      case e: ExecutionError => e
-      case other             => ExecutionError("Effect failure", path.reverse, locationInfo, Some(other))
-    }
-}

core/src/main/scala/caliban/validation/Validator.scala

@@ -132,9 +132,9 @@ object Validator {
 
   private def collectSelectionSets(selectionSet: List[Selection]): List[Selection] =
     selectionSet ++ selectionSet.flatMap {
-      case _: FragmentSpread                  => Nil
-      case Field(_, _, _, _, selectionSet, _) => collectSelectionSets(selectionSet)
-      case InlineFragment(_, _, selectionSet) => collectSelectionSets(selectionSet)
+      case _: FragmentSpread => Nil
+      case f: Field          => collectSelectionSets(f.selectionSet)
+      case f: InlineFragment => collectSelectionSets(f.selectionSet)
     }
 
   private def collectAllDirectives(context: Context): IO[ValidationError, List[(Directive, __DirectiveLocation)]] =

core/src/test/scala/caliban/GraphQLResponseSpec.scala

@@ -25,7 +25,7 @@ object GraphQLResponseSpec
             equalTo(
               Json.obj(
                 "data"   -> Json.fromString("data"),
-                "errors" -> Json.arr(Json.obj("message" -> Json.fromString("Execution error: Resolution failed")))
+                "errors" -> Json.arr(Json.obj("message" -> Json.fromString("Resolution failed")))
               )
             )
           )

core/src/test/scala/caliban/parsing/ParserSpec.scala

@@ -9,7 +9,7 @@ import caliban.parsing.adt.ExecutableDefinition.{ FragmentDefinition, OperationD
 import caliban.parsing.adt.OperationType.{ Mutation, Query }
 import caliban.parsing.adt.Selection.{ Field, FragmentSpread, InlineFragment }
 import caliban.parsing.adt.Type.{ ListType, NamedType }
-import caliban.parsing.adt.{ Directive, Document, Selection, VariableDefinition }
+import caliban.parsing.adt.{ Directive, Document, LocationInfo, Selection, VariableDefinition }
 import zio.test.Assertion._
 import zio.test._
 
@@ -436,7 +436,10 @@ object ParserSpec
                         |    name(
                         |  }
                         |}""".stripMargin
-          assertM(Parser.parseQuery(query).run, fails(equalTo(ParsingError("Position 4:3, found \"}\\n}\""))))
+          assertM(
+            Parser.parseQuery(query).run,
+            fails(equalTo(ParsingError("Position 4:3, found \"}\\n}\"", locationInfo = Some(LocationInfo(3, 4)))))
+          )
         }
       )
     )