From eea26c94227f8e1365e545ec407705778e4e6abf Mon Sep 17 00:00:00 2001 From: Hugo van Rijswijk Date: Tue, 8 Sep 2026 22:03:30 +0200 Subject: [PATCH] Add `location` to error messages Adds a `location` field `Ast.Selection.Field` to hold the line and column of the selection. That can then be used to add a `locations` field to error messages. Most changes are in tests with added `location` information to the error messages. The other option would be to strip location information from the expected error messages, but that would make it harder to verify that the location information is actually present. Fixes 2 conformance tests. --- docs/tutorial/in-memory-model.md | 5 +- .../scala/CirceEffectHandlerErrorSuite.scala | 12 +- modules/core/src/main/scala/ast.scala | 3 +- modules/core/src/main/scala/compiler.scala | 37 ++- modules/core/src/main/scala/parser.scala | 22 +- modules/core/src/main/scala/problem.scala | 16 +- modules/core/src/main/scala/query.scala | 37 ++- .../src/main/scala/queryinterpreter.scala | 46 +-- modules/core/src/main/scala/result.scala | 35 +- modules/core/src/main/scala/schema.scala | 4 +- .../test/scala/compiler/CascadeSuite.scala | 17 +- .../test/scala/compiler/CompilerSuite.scala | 90 +++-- .../scala/compiler/EnvironmentSuite.scala | 1 + .../test/scala/compiler/FieldMergeSuite.scala | 311 +++++++++--------- .../test/scala/compiler/FragmentSuite.scala | 207 +++++++----- .../scala/compiler/InputValuesSuite.scala | 5 +- .../compiler/PreserveArgsElaborator.scala | 4 +- .../test/scala/compiler/ProblemSuite.scala | 4 +- .../scala/compiler/SkipIncludeSuite.scala | 74 ++--- .../test/scala/composed/ComposedData.scala | 2 +- .../scala/composed/ComposedListSuite.scala | 2 +- .../scala/conformance/ResponseSuite.scala | 25 +- .../directives/QueryDirectivesSuite.scala | 2 +- .../test/scala/errors/FieldErrorSuite.scala | 23 ++ .../test/scala/minimizer/MinimizerSuite.scala | 4 +- .../src/test/scala/parser/ParserSuite.scala | 196 ++++++----- .../src/test/scala/utils/QueryLocations.scala | 54 +++ .../sql-core/src/main/scala/SqlMapping.scala | 22 +- .../test/scala/SqlComposedWorldMapping.scala | 4 +- .../test/scala/SqlNestedEffectsMapping.scala | 2 +- .../test/scala/SqlWorldCompilerSuite.scala | 12 +- 31 files changed, 762 insertions(+), 516 deletions(-) create mode 100644 modules/core/src/test/scala/utils/QueryLocations.scala diff --git a/docs/tutorial/in-memory-model.md b/docs/tutorial/in-memory-model.md index 22c0b1f6..9dd873b1 100644 --- a/docs/tutorial/in-memory-model.md +++ b/docs/tutorial/in-memory-model.md @@ -209,9 +209,10 @@ Grackle's query algebra consists of the following elements, case class UntypedSelect( name: String, alias: Option[String], args: List[Binding], directives: List[Directive], - child: Query + child: Query, + location: Option[(Int, Int)] = None ) -case class Select(name: String, alias: Option[String], child: Query) +case class Select(name: String, alias: Option[String], child: Query, location: Option[(Int, Int)] = None) case class Group(queries: List[Query]) case class Unique(child: Query) case class Filter(pred: Predicate, child: Query) diff --git a/modules/circe/src/test/scala/CirceEffectHandlerErrorSuite.scala b/modules/circe/src/test/scala/CirceEffectHandlerErrorSuite.scala index 57374e6b..0ef1cd9f 100644 --- a/modules/circe/src/test/scala/CirceEffectHandlerErrorSuite.scala +++ b/modules/circe/src/test/scala/CirceEffectHandlerErrorSuite.scala @@ -48,8 +48,8 @@ final class CirceEffectHandlerErrorSuite extends CatsEffectSuite { val expected = json""" { "errors" : [ - { "message": "value: hi", "path": ["s"] }, - { "message": "value: 42", "path": ["n"] } + { "message": "value: hi", "locations": [{ "line": 3, "column": 9 }], "path": ["s"] }, + { "message": "value: 42", "locations": [{ "line": 4, "column": 9 }], "path": ["n"] } ], "data" : null } @@ -62,7 +62,7 @@ final class CirceEffectHandlerErrorSuite extends CatsEffectSuite { val expected = json""" { "errors" : [ - { "message": "value: hi", "path": ["s"] } + { "message": "value: hi", "locations": [{ "line": 3, "column": 9 }], "path": ["s"] } ], "data" : null } @@ -111,7 +111,7 @@ final class CirceEffectHandlerErrorSuite extends CatsEffectSuite { val expected = json""" { "errors" : [ - { "message": "boom", "path": ["viaEffect"] } + { "message": "boom", "locations": [{ "line": 4, "column": 9 }], "path": ["viaEffect"] } ], "data" : { "ping" : "pong", @@ -167,7 +167,7 @@ final class CirceEffectHandlerErrorSuite extends CatsEffectSuite { val expected = json""" { "errors" : [ - { "message": "boom", "path": ["viaEffect", "name"] } + { "message": "boom", "locations": [{ "line": 5, "column": 11 }], "path": ["viaEffect", "name"] } ], "data" : null } @@ -190,7 +190,7 @@ final class CirceEffectHandlerErrorSuite extends CatsEffectSuite { val expected = json""" { "errors" : [ - { "message": "boom", "path": ["child", "viaEffect"] } + { "message": "boom", "locations": [{ "line": 5, "column": 11 }], "path": ["child", "viaEffect"] } ], "data" : { "ping" : "pong", diff --git a/modules/core/src/main/scala/ast.scala b/modules/core/src/main/scala/ast.scala index ac3d7c71..6a3b3254 100644 --- a/modules/core/src/main/scala/ast.scala +++ b/modules/core/src/main/scala/ast.scala @@ -66,7 +66,8 @@ object Ast { name: Name, arguments: List[(Name, Value)], directives: List[Directive], - selectionSet: List[Selection] + selectionSet: List[Selection], + location: Option[(Int, Int)] = None ) extends Selection case class FragmentSpread( diff --git a/modules/core/src/main/scala/compiler.scala b/modules/core/src/main/scala/compiler.scala index 4f1d563b..cc46ce44 100644 --- a/modules/core/src/main/scala/compiler.scala +++ b/modules/core/src/main/scala/compiler.scala @@ -156,7 +156,7 @@ object QueryParser { * GraphQL errors and warnings are accumulated in the result. */ def parseSelection(sel: Selection): Result[Query] = sel match { - case Field(alias, name, args, directives, sels) => + case Field(alias, name, args, directives, sels, location) => for { args0 <- parseArgs(args) sels0 <- parseSelections(sels) @@ -164,8 +164,8 @@ object QueryParser { } yield { val nme = name.value val alias0 = alias.map(_.value).flatMap(n => if (n == nme) None else Some(n)) - if (sels.isEmpty) UntypedSelect(nme, alias0, args0, dirs, Empty) - else UntypedSelect(nme, alias0, args0, dirs, sels0) + val child = if (sels.isEmpty) Empty else sels0 + UntypedSelect(nme, alias0, args0, dirs, child, location) } case FragmentSpread(Name(name), directives) => @@ -357,7 +357,7 @@ object VariableUsage { def loop(query: Query, tpe: Type): List[Problem] = query match { - case UntypedSelect(nme, _, args, dirs, child) => + case UntypedSelect(nme, _, args, dirs, child, _) => val dirProblems = checkDirectives(dirs) val named = tpe.underlyingNamed // An unknown field is reported by `SelectElaborator`. @@ -881,14 +881,14 @@ object QueryCompiler { dirs.foldMap(dir => argRefs(dir.args)) def loop(q: Query): (Set[String], Set[String]) = q match { - case UntypedSelect(_, _, args, dirs, child) => + case UntypedSelect(_, _, args, dirs, child, _) => (argRefs(args) ++ dirRefs(dirs), Set.empty[String]) |+| loop(child) case UntypedFragmentSpread(nme, dirs) => (dirRefs(dirs), Set(nme)) case UntypedInlineFragment(_, dirs, child) => (dirRefs(dirs), Set.empty[String]) |+| loop(child) case Group(children) => children.foldMap(loop) - case Select(_, _, child) => loop(child) + case Select(_, _, child, _) => loop(child) case Narrow(_, child) => loop(child) case Unique(child) => loop(child) case Filter(_, child) => loop(child) @@ -1195,10 +1195,10 @@ object QueryCompiler { */ def transform(query: Query): Elab[Query] = query match { - case s @ UntypedSelect(fieldName, alias, _, _, child) => + case s @ UntypedSelect(fieldName, alias, _, _, child, _) => transformSelect(fieldName, alias, child).map(ec => s.copy(child = ec)) - case s @ Select(fieldName, alias, child) => + case s @ Select(fieldName, alias, child, _) => transformSelect(fieldName, alias, child).map(ec => s.copy(child = ec)) case n @ Narrow(subtpe, child) => @@ -1350,6 +1350,7 @@ object QueryCompiler { _, _, _, + _, _) => (fieldName, level) match { case ("__typename", Disabled) => @@ -1387,7 +1388,7 @@ object QueryCompiler { object VariablesSkipAndFragmentElaborator extends Phase { override def transform(query: Query): Elab[Query] = query match { - case sel @ UntypedSelect(fieldName, alias, args, dirs, child) => + case sel @ UntypedSelect(fieldName, alias, args, dirs, child, _) => isSkipped(dirs).ifM( Elab.pure(Empty), for { @@ -1534,7 +1535,7 @@ object QueryCompiler { trait SelectElaborator extends Phase { override def transform(query: Query): Elab[Query] = query match { - case sel @ UntypedSelect(fieldName, resultName, args, dirs, child) => + case sel @ UntypedSelect(fieldName, resultName, args, dirs, child, _) => for { c <- Elab.context s <- Elab.schema @@ -1561,7 +1562,7 @@ object QueryCompiler { _ <- Elab.pop e2 <- elab(ec) } yield { - val e1 = Select(sel.name, sel.alias, e2) + val e1 = Select(sel.name, sel.alias, e2, sel.location) val e0 = if (attrs.isEmpty) e1 else mergeQueries(e1 :: attrs.map { case (nme, child) => Select(nme, child) }) @@ -1700,7 +1701,7 @@ object QueryCompiler { extends Phase { override def transform(query: Query): Elab[Query] = query match { - case s @ Select(fieldName, resultName, child) => + case s @ Select(fieldName, resultName, child, _) => for { c <- Elab.context obj <- Elab.liftR( @@ -1758,7 +1759,7 @@ object QueryCompiler { extends Phase { override def transform(query: Query): Elab[Query] = query match { - case s @ Select(fieldName, resultName, child) => + case s @ Select(fieldName, resultName, child, location) => for { c <- Elab.context childCtx = c.forFieldOrAttribute(fieldName, resultName) @@ -1767,7 +1768,7 @@ object QueryCompiler { _ <- Elab.pop } yield effects(c, fieldName) match { case Some(handler) => - Select(fieldName, resultName, Effect(handler, s.copy(child = ec))) + Select(fieldName, resultName, Effect(handler, s.copy(child = ec)), location) case None => s.copy(child = ec) } @@ -1827,11 +1828,11 @@ object QueryCompiler { @tailrec def loop(q: Query, depth: Int, width: Int): (Int, Int) = q match { - case UntypedSelect(_, _, _, _, Empty) => (depth + 1, width + 1) - case Select(_, _, Empty) => (depth + 1, width + 1) + case UntypedSelect(_, _, _, _, Empty, _) => (depth + 1, width + 1) + case Select(_, _, Empty, _) => (depth + 1, width + 1) case Count(_) => (depth + 1, width + 1) - case UntypedSelect(_, _, _, _, child) => loop(child, depth + 1, width) - case Select(_, _, child) => loop(child, depth + 1, width) + case UntypedSelect(_, _, _, _, child, _) => loop(child, depth + 1, width) + case Select(_, _, child, _) => loop(child, depth + 1, width) case g: Group => handleGroup(g, depth, width) case Component(_, _, child) => loop(child, depth, width) case Effect(_, child) => loop(child, depth, width) diff --git a/modules/core/src/main/scala/parser.scala b/modules/core/src/main/scala/parser.scala index 69ea4c09..1bd80ecc 100644 --- a/modules/core/src/main/scala/parser.scala +++ b/modules/core/src/main/scala/parser.scala @@ -17,10 +17,10 @@ package grackle import scala.util.matching.Regex -import cats.implicits._ +import cats.implicits.* import cats.parse.{Parser, Parser0} -import cats.parse.Numbers._ -import cats.parse.Parser._ +import cats.parse.Numbers.* +import cats.parse.Parser.* import cats.parse.Rfc5234.{cr, crlf, digit, hexdig, lf} trait GraphQLParser { @@ -332,9 +332,19 @@ object GraphQLParser { } def Field(n: Int): Parser[Ast.Selection.Field] = - (Alias.backtrack.?.with1 ~ Name ~ Arguments.? ~ Directives ~ SelectionSetN(n).?).map { - case ((((alias, name), args), dirs), sel) => - Ast.Selection.Field(alias, name, args.orEmpty, dirs, sel.orEmpty) + (caret.with1 ~ + (Alias.backtrack.?.with1 ~ Name ~ Arguments.? ~ Directives ~ SelectionSetN(n).?)).map { + case (pos, ((((alias, name), args), dirs), sel)) => + Ast + .Selection + .Field( + alias, + name, + args.orEmpty, + dirs, + sel.orEmpty, + // The caret counts lines and columns from zero, the specification from one. + Some((pos.line + 1, pos.col + 1))) } def InlineFragment(n: Int): Parser[Ast.Selection.InlineFragment] = diff --git a/modules/core/src/main/scala/problem.scala b/modules/core/src/main/scala/problem.scala index d2450979..7fb26daf 100644 --- a/modules/core/src/main/scala/problem.scala +++ b/modules/core/src/main/scala/problem.scala @@ -36,18 +36,22 @@ final case class Problem( def atPath(path: List[Problem.PathSegment]): Problem = if (this.path.isEmpty) copy(path = path) else this + /** + * Yields this problem with `locations` as its source locations, if it carries none. + * + * A problem which a deeper position raised already holds the location of that position, so an + * enclosing position leaves it in place. + */ + def atLocations(locations: List[(Int, Int)]): Problem = + if (this.locations.isEmpty) copy(locations = locations) else this + override def toString = { lazy val pathText: String = path.mkString("/") lazy val locationsText: String = - locations - .map { - case (a, b) => - if (a == b) a.toString else s"$a..$b" - } - .mkString(", ") + locations.map { case (line, column) => s"$line:$column" }.mkString(", ") val s = (path.nonEmpty, locations.nonEmpty) match { case (true, true) => s"$message (at $pathText: $locationsText)" diff --git a/modules/core/src/main/scala/query.scala b/modules/core/src/main/scala/query.scala index 126ae9d6..a6e5ada0 100644 --- a/modules/core/src/main/scala/query.scala +++ b/modules/core/src/main/scala/query.scala @@ -48,8 +48,16 @@ object Query { /** * Select field `name` possibly aliased, and continue with `child` + * + * `location` holds the line and the column of the selection in the text of the request, if + * the parser recorded them. Both count from one. */ - case class Select(name: String, alias: Option[String], child: Query) extends Query { + case class Select( + name: String, + alias: Option[String], + child: Query, + location: Option[(Int, Int)] = None) + extends Query { def resultName: String = alias.getOrElse(name) def render = { @@ -78,7 +86,8 @@ object Query { alias: Option[String], args: List[Binding], directives: List[Directive], - child: Query) + child: Query, + location: Option[(Int, Int)] = None) extends Query { def resultName: String = alias.getOrElse(name) @@ -311,8 +320,8 @@ object Query { def rootName(q: Query): Option[(String, Option[String])] = { def loop(q: Query): Option[(String, Option[String])] = q match { - case UntypedSelect(name, alias, _, _, _) => Some((name, alias)) - case Select(name, alias, _) => Some((name, alias)) + case UntypedSelect(name, alias, _, _, _, _) => Some((name, alias)) + case Select(name, alias, _, _) => Some((name, alias)) case Environment(_, child) => loop(child) case TransformCursor(_, child) => loop(child) case _ => None @@ -327,8 +336,8 @@ object Query { def resultName(q: Query): Option[String] = { def loop(q: Query): Option[String] = q match { - case UntypedSelect(name, alias, _, _, _) => Some(alias.getOrElse(name)) - case Select(name, alias, _) => Some(alias.getOrElse(name)) + case UntypedSelect(name, alias, _, _, _, _) => Some(alias.getOrElse(name)) + case Select(name, alias, _, _) => Some(alias.getOrElse(name)) case Environment(_, child) => loop(child) case TransformCursor(_, child) => loop(child) case _ => None @@ -379,8 +388,8 @@ object Query { def children(q: Query): List[Query] = { def loop(q: Query): List[Query] = q match { - case UntypedSelect(_, _, _, _, child) => ungroup(child) - case Select(_, _, child) => ungroup(child) + case UntypedSelect(_, _, _, _, child, _) => ungroup(child) + case Select(_, _, child, _) => ungroup(child) case Environment(_, child) => loop(child) case TransformCursor(_, child) => loop(child) case _ => Nil @@ -395,8 +404,8 @@ object Query { def extractChild(query: Query): Option[Query] = { def loop(q: Query): Option[Query] = q match { - case UntypedSelect(_, _, _, _, child) => Some(child) - case Select(_, _, child) => Some(child) + case UntypedSelect(_, _, _, _, child, _) => Some(child) + case Select(_, _, child, _) => Some(child) case Environment(_, child) => loop(child) case TransformCursor(_, child) => loop(child) case _ => None @@ -426,8 +435,8 @@ object Query { def hasField(query: Query, fieldName: String): Boolean = { def loop(q: Query): Boolean = ungroup(q).exists { - case UntypedSelect(`fieldName`, _, _, _, _) => true - case Select(`fieldName`, _, _) => true + case UntypedSelect(`fieldName`, _, _, _, _, _) => true + case Select(`fieldName`, _, _, _) => true case Environment(_, child) => loop(child) case TransformCursor(_, child) => loop(child) case _ => false @@ -441,8 +450,8 @@ object Query { def fieldAlias(query: Query, fieldName: String): Option[String] = { def loop(q: Query): Option[String] = ungroup(q).collectFirstSome { - case UntypedSelect(`fieldName`, alias, _, _, _) => alias - case Select(`fieldName`, alias, _) => alias + case UntypedSelect(`fieldName`, alias, _, _, _, _) => alias + case Select(`fieldName`, alias, _, _) => alias case Environment(_, child) => loop(child) case TransformCursor(_, child) => loop(child) case _ => None diff --git a/modules/core/src/main/scala/queryinterpreter.scala b/modules/core/src/main/scala/queryinterpreter.scala index 510d457e..61abaf77 100644 --- a/modules/core/src/main/scala/queryinterpreter.scala +++ b/modules/core/src/main/scala/queryinterpreter.scala @@ -199,17 +199,19 @@ class QueryInterpreter[F[_]](mapping: Mapping[F]) { if (tpe.isNullable) value else ProtoJson.nonNull(value) /** - * Handles a failure of the field `name` at the position `pos` as a field error: the problems - * carry the path of `pos`, and a nullable field completes as null. + * Handles a failure of the selection `sel` at the position `pos` as a field error. + * + * A nullable position completes as null and keeps the rest of the response. A non-null + * position propagates the null to the nearest enclosing nullable position. * * @see * https://spec.graphql.org/September2025/#sec-Handling-Field-Errors */ - private def fieldError(tpe: Type, pos: ResponsePosition, name: String)( + private def fieldError(tpe: Type, pos: ResponsePosition, sel: Select)( res: Result[List[(String, ProtoJson)]]): Result[List[(String, ProtoJson)]] = - res.atPath(pos.path) match { + res.at(pos.path, sel.location) match { case Result.Failure(ps) if tpe.isNullable => - Result.Warning(ps, List((name, ProtoJson.fromJson(Json.Null)))) + Result.Warning(ps, List((sel.resultName, ProtoJson.fromJson(Json.Null)))) case other => other } @@ -239,7 +241,7 @@ class QueryInterpreter[F[_]](mapping: Mapping[F]) { case Group(siblings) => siblings.flatTraverse(query => runFields(query, tpe, cursor, path)) - case Introspect(schema, s @ Select("__typename", _, Empty)) if tpe.isNamed => + case Introspect(schema, s @ Select("__typename", _, Empty, _)) if tpe.isNamed => val fail = Result.failure(s"'__typename' cannot be applied to non-selectable type '$tpe'") def mkTypeNameFields(name: String) = @@ -278,14 +280,14 @@ class QueryInterpreter[F[_]](mapping: Mapping[F]) { } .getOrElse(List((sel.resultName, ProtoJson.fromJson(Json.Null))).success) - case sel @ Select(fieldName, _, Count(Select(countName, _, _))) => + case sel @ Select(fieldName, _, Count(Select(countName, _, _, _)), _) => def size(c: Cursor): Result[Int] = if (c.isList) c.asList(Iterator).map(_.size) else 1.success val fieldTpe = tpe.field(fieldName).getOrElse(ScalarType.AttributeType) val fieldPos = path.field(sel.resultName) - fieldError(fieldTpe, fieldPos, sel.resultName) { + fieldError(fieldTpe, fieldPos, sel) { for { c0 <- cursor.field(countName, None) count <- @@ -294,7 +296,7 @@ class QueryInterpreter[F[_]](mapping: Mapping[F]) { } yield List((sel.resultName, ProtoJson.fromJson(Json.fromInt(count)))) } - case sel @ Select(fieldName, _, Effect(handler, cont)) => + case sel @ Select(fieldName, _, Effect(handler, cont), _) => val fieldTpe = tpe.field(fieldName).getOrElse(ScalarType.AttributeType) val fieldPos = path.field(sel.resultName) val value = @@ -303,13 +305,14 @@ class QueryInterpreter[F[_]](mapping: Mapping[F]) { handler.asInstanceOf[EffectHandler[F]], cont, cursor, - fieldPos) + fieldPos, + sel.location) List((sel.resultName, atPosition(value, fieldTpe))).success - case sel @ Select(fieldName, resultName, child) => + case sel @ Select(fieldName, resultName, child, _) => val fieldTpe = tpe.field(fieldName).getOrElse(ScalarType.AttributeType) val fieldPos = path.field(sel.resultName) - fieldError(fieldTpe, fieldPos, sel.resultName) { + fieldError(fieldTpe, fieldPos, sel) { for { c <- cursor.field(fieldName, resultName) value <- runValue(child, fieldTpe, c, fieldPos) @@ -662,7 +665,8 @@ object QueryInterpreter { handler: Option[EffectHandler[F]], query: Query, cursor: Cursor, - position: ResponsePosition) + position: ResponsePosition, + location: Option[(Int, Int)]) extends DeferredJson // A partially constructed object which has at least one deferred subtree. private[QueryInterpreter] case class ProtoObject(fields: Seq[(String, ProtoJson)]) @@ -692,15 +696,16 @@ object QueryInterpreter { query: Query, cursor: Cursor, position: ResponsePosition = ResponsePosition.root): ProtoJson = - wrap(EffectJson(mapping, None, query, cursor, position)) + wrap(EffectJson(mapping, None, query, cursor, position, None)) def effect[F[_]]( mapping: Mapping[F], handler: EffectHandler[F], query: Query, cursor: Cursor, - position: ResponsePosition = ResponsePosition.root): ProtoJson = - wrap(EffectJson(mapping, Some(handler), query, cursor, position)) + position: ResponsePosition = ResponsePosition.root, + location: Option[(Int, Int)] = None): ProtoJson = + wrap(EffectJson(mapping, Some(handler), query, cursor, position, location)) def fromJson(value: Json): ProtoJson = wrap(value) @@ -1016,7 +1021,7 @@ object QueryInterpreter { mapping .interpreter .runValue(query, cursor.tpe, cursor, e.position) - .atPath(e.position.path) + .at(e.position.path, e.location) })) } yield res } @@ -1035,7 +1040,9 @@ object QueryInterpreter { batch.tupleRight(None) // Handles a failed batch as a field error: its problems become warnings and its positions - // complete as null. A batch which covers exactly one position carries the path of that position. + // complete as null. A batch covers one or more response positions. When it covers exactly + // one, the failure belongs to that position and the problems carry its response path and its + // source location def batchFieldError( batch: List[EffectJson[F]], ps: NonEmptyChain[Problem]): Result[Completed] = { @@ -1043,7 +1050,8 @@ object QueryInterpreter { batch match { case List(e) => val path = e.position.path - ps.map(_.atPath(path)) + val locations = e.location.toList + ps.map(_.atPath(path).atLocations(locations)) case _ => ps } Result.Warning(ps0, nullBatch(batch)) diff --git a/modules/core/src/main/scala/result.scala b/modules/core/src/main/scala/result.scala index 7ad0ede3..833924d3 100644 --- a/modules/core/src/main/scala/result.scala +++ b/modules/core/src/main/scala/result.scala @@ -112,12 +112,35 @@ sealed trait Result[+T] { */ def atPath(path: List[Problem.PathSegment]): Result[T] = if (path.isEmpty) this - else - this match { - case Result.Failure(ps) => Result.Failure(ps.map(_.atPath(path))) - case Result.Warning(ps, value) => Result.Warning(ps.map(_.atPath(path)), value) - case other => other - } + else mapProblems(_.atPath(path)) + + /** + * Yields this result with `locations` as the source locations of each problem which has none. + */ + def atLocations(locations: List[(Int, Int)]): Result[T] = + if (locations.isEmpty) this + else mapProblems(_.atLocations(locations)) + + /** + * Yields this result with `path` as the response path and `location` as the source location + * of each problem which has neither. + */ + def at(path: List[Problem.PathSegment], location: Option[(Int, Int)]): Result[T] = + if (path.isEmpty && location.isEmpty) this + else { + val locations = location.toList + mapProblems(_.atPath(path).atLocations(locations)) + } + + /** + * Yields this result with `f` applied to each of its problems. + */ + private def mapProblems(f: Problem => Problem): Result[T] = + this match { + case Result.Failure(ps) => Result.Failure(ps.map(f)) + case Result.Warning(ps, value) => Result.Warning(ps.map(f), value) + case other => other + } def withProblems(problems: NonEmptyChain[Problem]): Result[T] = this match { diff --git a/modules/core/src/main/scala/schema.scala b/modules/core/src/main/scala/schema.scala index 3235d0cf..6d75989d 100644 --- a/modules/core/src/main/scala/schema.scala +++ b/modules/core/src/main/scala/schema.scala @@ -1542,7 +1542,7 @@ object Directive { def queryWarnings(query: Query): List[Problem] = { def loop(query: Query): List[Problem] = query match { - case UntypedSelect(_, _, _, dirs, child) => + case UntypedSelect(_, _, _, dirs, child, _) => validateDirectives(schema, Ast.DirectiveLocation.FIELD, dirs, vars) ++ loop(child) case UntypedFragmentSpread(_, dirs) => validateDirectives(schema, Ast.DirectiveLocation.FRAGMENT_SPREAD, dirs, vars) @@ -1552,7 +1552,7 @@ object Directive { Ast.DirectiveLocation.INLINE_FRAGMENT, dirs, vars) ++ loop(child) - case Select(_, _, child) => loop(child) + case Select(_, _, child, _) => loop(child) case Group(children) => children.flatMap(loop) case Narrow(_, child) => loop(child) case Unique(child) => loop(child) diff --git a/modules/core/src/test/scala/compiler/CascadeSuite.scala b/modules/core/src/test/scala/compiler/CascadeSuite.scala index db01ae78..7a6fa627 100644 --- a/modules/core/src/test/scala/compiler/CascadeSuite.scala +++ b/modules/core/src/test/scala/compiler/CascadeSuite.scala @@ -20,6 +20,7 @@ import scala.PartialFunction.condOpt import cats.effect.IO import io.circe.literal._ import munit.CatsEffectSuite +import utils.QueryLocations._ import grackle._ import grackle.Query._ @@ -49,17 +50,21 @@ final class CascadeSuite extends CatsEffectSuite { "filter" -> CascadeMapping.CascadedFilter(Some("foo"), None, Some(23), Some(10)))), Select( "foo", + None, Select( "cascaded", + None, Group( List( - Select("foo"), - Select("bar"), - Select("fooBar"), - Select("limit") + Select("foo", None, Empty, loc(5, 13)), + Select("bar", None, Empty, loc(6, 13)), + Select("fooBar", None, Empty, loc(7, 13)), + Select("limit", None, Empty, loc(8, 13)) ) - ) - ) + ), + loc(4, 11) + ), + loc(3, 9) ) ) diff --git a/modules/core/src/test/scala/compiler/CompilerSuite.scala b/modules/core/src/test/scala/compiler/CompilerSuite.scala index b3c4c732..2882f56f 100644 --- a/modules/core/src/test/scala/compiler/CompilerSuite.scala +++ b/modules/core/src/test/scala/compiler/CompilerSuite.scala @@ -19,6 +19,7 @@ import cats.data.NonEmptyChain import cats.effect.IO import cats.implicits._ import munit.CatsEffectSuite +import utils.QueryLocations._ import grackle._ import grackle.Predicate._ @@ -48,8 +49,8 @@ final class CompilerSuite extends CatsEffectSuite { None, List(Binding("id", StringValue("1000"))), Nil, - UntypedSelect("name", None, Nil, Nil, Empty) - ) + UntypedSelect("name", None, Nil, Nil, Empty, loc(4, 11)), + loc(3, 9)) val res = queryParser.parseText(query).map(_._1) assertEquals(res, Result.Success(List(UntypedQuery(None, expected, Nil, Nil)))) @@ -77,8 +78,9 @@ final class CompilerSuite extends CatsEffectSuite { None, Nil, Nil, - UntypedSelect("name", None, Nil, Nil, Empty) - ) + UntypedSelect("name", None, Nil, Nil, Empty, loc(5, 13)), + loc(4, 11)), + loc(3, 9) ) val res = queryParser.parseText(query).map(_._1) @@ -100,8 +102,8 @@ final class CompilerSuite extends CatsEffectSuite { None, List(Binding("id", StringValue("1000"))), Nil, - UntypedSelect("name", None, Nil, Nil, Empty) - ) + UntypedSelect("name", None, Nil, Nil, Empty, loc(4, 11)), + loc(3, 9)) val res = queryParser.parseText(query).map(_._1) assertEquals(res, Result.Success(List(UntypedSubscription(None, expected, Nil, Nil)))) @@ -125,14 +127,15 @@ final class CompilerSuite extends CatsEffectSuite { None, List(Binding("id", StringValue("1000"))), Nil, - UntypedSelect("name", None, Nil, Nil, Empty) ~ + UntypedSelect("name", None, Nil, Nil, Empty, loc(4, 11)) ~ UntypedSelect( "friends", None, Nil, Nil, - UntypedSelect("name", None, Nil, Nil, Empty) - ) + UntypedSelect("name", None, Nil, Nil, Empty, loc(6, 13)), + loc(5, 11)), + loc(3, 9) ) val res = queryParser.parseText(query).map(_._1) @@ -160,21 +163,23 @@ final class CompilerSuite extends CatsEffectSuite { None, List(Binding("episode", EnumValue("NEWHOPE"))), Nil, - UntypedSelect("name", None, Nil, Nil, Empty) ~ + UntypedSelect("name", None, Nil, Nil, Empty, loc(4, 11)) ~ UntypedSelect( "friends", None, Nil, Nil, - UntypedSelect("name", None, Nil, Nil, Empty) ~ + UntypedSelect("name", None, Nil, Nil, Empty, loc(6, 13)) ~ UntypedSelect( "friends", None, Nil, Nil, - UntypedSelect("name", None, Nil, Nil, Empty) - ) - ) + UntypedSelect("name", None, Nil, Nil, Empty, loc(8, 15)), + loc(7, 13)), + loc(5, 11) + ), + loc(3, 9) ) val res = queryParser.parseText(query).map(_._1) @@ -201,21 +206,24 @@ final class CompilerSuite extends CatsEffectSuite { Nil, Group( List( - UntypedSelect("id", None, Nil, Nil, Empty), - UntypedSelect("name", None, Nil, Nil, Empty), + UntypedSelect("id", None, Nil, Nil, Empty, loc(4, 11)), + UntypedSelect("name", None, Nil, Nil, Empty, loc(5, 11)), UntypedSelect( "profilePic", Some("smallPic"), List(Binding("size", IntValue(64))), Nil, - Empty), + Empty, + loc(6, 11)), UntypedSelect( "profilePic", Some("bigPic"), List(Binding("size", IntValue(1024))), Nil, - Empty) - )) + Empty, + loc(7, 11)) + )), + loc(3, 9) ) val res = queryParser.parseText(query).map(_._1) @@ -250,19 +258,23 @@ final class CompilerSuite extends CatsEffectSuite { None, Nil, Nil, - UntypedSelect("name", None, Nil, Nil, Empty)) ~ + UntypedSelect("name", None, Nil, Nil, Empty, loc(5, 13)), + loc(4, 11)) ~ UntypedSelect( "mutationType", None, Nil, Nil, - UntypedSelect("name", None, Nil, Nil, Empty)) ~ + UntypedSelect("name", None, Nil, Nil, Empty, loc(8, 13)), + loc(7, 11)) ~ UntypedSelect( "subscriptionType", None, Nil, Nil, - UntypedSelect("name", None, Nil, Nil, Empty)) + UntypedSelect("name", None, Nil, Nil, Empty, loc(11, 13)), + loc(10, 11)), + loc(3, 9) ) val res = queryParser.parseText(query).map(_._1) @@ -290,13 +302,11 @@ final class CompilerSuite extends CatsEffectSuite { Unique( Filter( Eql(AtomicMapping.CharacterType / "id", Const("1000")), - Select("name") ~ - Select( - "friends", - Select("name") - ) + Select("name", None, Empty, loc(4, 11)) ~ + Select("friends", None, Select("name", None, Empty, loc(6, 13)), loc(5, 11)) ) - ) + ), + loc(3, 9) ) val res = AtomicMapping.compiler.compile(query) @@ -412,29 +422,37 @@ final class CompilerSuite extends CatsEffectSuite { TrivialJoin, Select( "componenta", - Select("fielda1") ~ + None, + Select("fielda1", None, Empty, loc(4, 11)) ~ Select( "fielda2", + None, Component( ComponentB, TrivialJoin, Select( "componentb", - Select("fieldb1") ~ + None, + Select("fieldb1", None, Empty, loc(7, 15)) ~ Select( "fieldb2", + None, Component( ComponentC, TrivialJoin, Select( "componentc", - Select("fieldc1") - ) - ) - ) + None, + Select("fieldc1", None, Empty, loc(10, 19)), + loc(9, 17)) + ), + loc(8, 15)), + loc(6, 13) ) - ) - ) + ), + loc(5, 11) + ), + loc(3, 9) ) ) diff --git a/modules/core/src/test/scala/compiler/EnvironmentSuite.scala b/modules/core/src/test/scala/compiler/EnvironmentSuite.scala index 64737701..1c88cb83 100644 --- a/modules/core/src/test/scala/compiler/EnvironmentSuite.scala +++ b/modules/core/src/test/scala/compiler/EnvironmentSuite.scala @@ -223,6 +223,7 @@ final class EnvironmentSuite extends CatsEffectSuite { "errors" : [ { "message" : "Missing argument", + "locations" : [ { "line" : 4, "column" : 11 } ], "path" : [ "nested", "url" ] } ], diff --git a/modules/core/src/test/scala/compiler/FieldMergeSuite.scala b/modules/core/src/test/scala/compiler/FieldMergeSuite.scala index c5823c94..9424dfa6 100644 --- a/modules/core/src/test/scala/compiler/FieldMergeSuite.scala +++ b/modules/core/src/test/scala/compiler/FieldMergeSuite.scala @@ -20,6 +20,7 @@ import cats.implicits._ import io.circe.Json import io.circe.literal._ import munit.CatsEffectSuite +import utils.QueryLocations._ import grackle._ import grackle.PathTerm.UniquePath @@ -90,35 +91,37 @@ final class FieldMergeSuite extends CatsEffectSuite { Eql(UniquePath(List("id")), Const("1")), Group( List( - Select("name", None, Empty), - Select("profilePic", None, Empty), - Select("id", None, Empty), - Select("name", Some("foo"), Empty), - Select("name", Some("bar"), Empty), + Select("name", None, Empty, loc(4, 11)), + Select("profilePic", None, Empty, loc(5, 11)), + Select("id", None, Empty, loc(7, 11)), + Select("name", Some("foo"), Empty, loc(8, 11)), + Select("name", Some("bar"), Empty, loc(10, 11)), Select( "friends", None, Group( List( - Select("name", None, Empty), - Select("profilePic", None, Empty), - Select("id", None, Empty) - )) + Select("name", None, Empty, loc(13, 13)), + Select("profilePic", None, Empty, loc(17, 13)), + Select("id", None, Empty, loc(21, 13)) + )), + loc(12, 11) ), Select( "friends", Some("baz"), - Group( - List( - Select("name", None, Empty), - Select("name", Some("quux"), Empty), - Select("profilePic", None, Empty), - Select("id", None, Empty) - )) + Group(List( + Select("name", None, Empty, loc(25, 13)), + Select("name", Some("quux"), Empty, loc(26, 13)), + Select("profilePic", None, Empty, loc(31, 13)), + Select("id", None, Empty, loc(35, 13)) + )), + loc(24, 11) ) )) ) - ) + ), + loc(3, 9) ) val expectedResult = json""" @@ -191,11 +194,12 @@ final class FieldMergeSuite extends CatsEffectSuite { Eql(UniquePath(List("id")), Const("1")), Group( List( - Select("name", Some("profilePic"), Empty), - Select("profilePic", Some("name"), Empty) + Select("name", Some("profilePic"), Empty, loc(4, 11)), + Select("profilePic", Some("name"), Empty, loc(5, 11)) )) ) - ) + ), + loc(3, 9) ) val expectedResult = json""" @@ -233,7 +237,7 @@ final class FieldMergeSuite extends CatsEffectSuite { val compiled = FieldMergeMapping.compiler.compile(query) - assertEquals(compiled.map(_.query), Result.failure(expected)) + assertEquals(compiled, Result.failure(expected)) } test("unmergeable alias (2)") { @@ -251,7 +255,7 @@ final class FieldMergeSuite extends CatsEffectSuite { val compiled = FieldMergeMapping.compiler.compile(query) - assertEquals(compiled.map(_.query), Result.failure(expected)) + assertEquals(compiled, Result.failure(expected)) } test("fields with arguments (1)") { @@ -275,11 +279,12 @@ final class FieldMergeSuite extends CatsEffectSuite { Eql(UniquePath(List("id")), Const("1")), Group( List( - Select("name", None, Empty), - Select("profilePic", None, Empty) + Select("name", None, Empty, loc(4, 11)), + Select("profilePic", None, Empty, loc(7, 11)) )) ) - ) + ), + loc(3, 9) ) val expectedResult = json""" @@ -319,7 +324,7 @@ final class FieldMergeSuite extends CatsEffectSuite { val compiled = FieldMergeMapping.compiler.compile(query) - assertEquals(compiled.map(_.query), Result.failure(expected)) + assertEquals(compiled, Result.failure(expected)) } test("fields with arguments (3)") { @@ -343,20 +348,20 @@ final class FieldMergeSuite extends CatsEffectSuite { Unique( Filter( Eql(UniquePath(List("id")), Const("1")), - Select("name", None, Empty) + Select("name", None, Empty, loc(4, 11)) ) - ) - ), + ), + loc(3, 9)), Select( "user", Some("foo"), Unique( Filter( Eql(UniquePath(List("id")), Const("1")), - Select("profilePic", None, Empty) + Select("profilePic", None, Empty, loc(7, 11)) ) - ) - ) + ), + loc(6, 9)) )) val expectedResult = json""" @@ -402,20 +407,20 @@ final class FieldMergeSuite extends CatsEffectSuite { Unique( Filter( Eql(UniquePath(List("id")), Const("1")), - Select("name", None, Empty) + Select("name", None, Empty, loc(4, 11)) ) - ) - ), + ), + loc(3, 9)), Select( "user", Some("foo"), Unique( Filter( Eql(UniquePath(List("id")), Const("2")), - Select("profilePic", None, Empty) + Select("profilePic", None, Empty, loc(7, 11)) ) - ) - ) + ), + loc(6, 9)) )) val expectedResult = json""" @@ -461,11 +466,12 @@ final class FieldMergeSuite extends CatsEffectSuite { Eql(UniquePath(List("id")), Const("1")), Group( List( - Select("name", None, Empty), - Select("profilePic", None, Empty) + Select("name", None, Empty, loc(4, 11)), + Select("profilePic", None, Empty, loc(7, 11)) )) ) - ) + ), + loc(3, 9) ) val expectedResult = json""" @@ -508,7 +514,7 @@ final class FieldMergeSuite extends CatsEffectSuite { .compiler .compile(query, untypedVars = Some(json"""{ "id1": "1", "id2": "2" }""")) - assertEquals(compiled.map(_.query), Result.failure(expected)) + assertEquals(compiled, Result.failure(expected)) } test("fields with arguments (7)") { @@ -529,7 +535,7 @@ final class FieldMergeSuite extends CatsEffectSuite { val compiled = FieldMergeMapping.compiler.compile(query, untypedVars = Some(json"""{ "id1": "1" }""")) - assertEquals(compiled.map(_.query), Result.failure(expected)) + assertEquals(compiled, Result.failure(expected)) } test("fields with skip (1)") { @@ -553,14 +559,10 @@ final class FieldMergeSuite extends CatsEffectSuite { Unique( Filter( Eql(UniquePath(List("id")), Const("1")), - Select( - "friends", - None, - Select("name", None, Empty) - ) + Select("friends", None, Select("name", None, Empty, loc(5, 13)), loc(4, 11)) ) - ) - ) + ), + loc(3, 9)) val expectedResult = json""" { @@ -609,14 +611,10 @@ final class FieldMergeSuite extends CatsEffectSuite { Unique( Filter( Eql(UniquePath(List("id")), Const("1")), - Select( - "friends", - None, - Select("name", None, Empty) - ) + Select("friends", None, Select("name", None, Empty, loc(5, 13)), loc(4, 11)) ) - ) - ) + ), + loc(3, 9)) val expectedResult = json""" { @@ -665,14 +663,10 @@ final class FieldMergeSuite extends CatsEffectSuite { Unique( Filter( Eql(UniquePath(List("id")), Const("1")), - Select( - "friends", - None, - Select("name", None, Empty) - ) + Select("friends", None, Select("name", None, Empty, loc(5, 13)), loc(4, 11)) ) - ) - ) + ), + loc(3, 9)) val expectedResult = json""" { @@ -721,7 +715,7 @@ final class FieldMergeSuite extends CatsEffectSuite { val compiled = FieldMergeMapping.compiler.compile(query) - assertEquals(compiled.map(_.query), Result.failure(expected)) + assertEquals(compiled, Result.failure(expected)) } test("fields with skip (5)") { @@ -750,12 +744,13 @@ final class FieldMergeSuite extends CatsEffectSuite { None, Group( List( - Select("name", None, Empty), - Select("profilePic", None, Empty) - )) - ) + Select("name", None, Empty, loc(5, 13)), + Select("profilePic", None, Empty, loc(8, 13)) + )), + loc(4, 11)) ) - ) + ), + loc(3, 9) ) val expectedResult = json""" @@ -814,12 +809,18 @@ final class FieldMergeSuite extends CatsEffectSuite { None, Group( List( - Narrow(FieldMergeMapping.UserType, Select("name", Some("label"), Empty)), - Narrow(FieldMergeMapping.PageType, Select("title", Some("label"), Empty)) - )) + Narrow( + FieldMergeMapping.UserType, + Select("name", Some("label"), Empty, loc(6, 15))), + Narrow( + FieldMergeMapping.PageType, + Select("title", Some("label"), Empty, loc(9, 15))) + )), + loc(4, 11) ) ) - ) + ), + loc(3, 9) ) val expectedResult = json""" @@ -864,7 +865,7 @@ final class FieldMergeSuite extends CatsEffectSuite { val compiled = FieldMergeMapping.compiler.compile(query) - assertEquals(compiled.map(_.query), Result.failure(expected)) + assertEquals(compiled, Result.failure(expected)) } test("fields with variants (3)") { @@ -904,21 +905,23 @@ final class FieldMergeSuite extends CatsEffectSuite { Select( "friends", Some("likers"), - Select("name", None, Empty) - ) + Select("name", None, Empty, loc(7, 17)), + loc(6, 15)) ), Narrow( FieldMergeMapping.PageType, Select( "likers", None, - Select("name", None, Empty) - ) + Select("name", None, Empty, loc(12, 17)), + loc(11, 15)) ) - )) + )), + loc(4, 11) ) ) - ) + ), + loc(3, 9) ) val expectedResult = json""" @@ -971,7 +974,7 @@ final class FieldMergeSuite extends CatsEffectSuite { val compiled = FieldMergeMapping.compiler.compile(query) - assertEquals(compiled.map(_.query), Result.failure(expected)) + assertEquals(compiled, Result.failure(expected)) } test("fields with variants (5)") { @@ -1001,7 +1004,7 @@ final class FieldMergeSuite extends CatsEffectSuite { val compiled = FieldMergeMapping.compiler.compile(query) - assertEquals(compiled.map(_.query), Result.failure(expected)) + assertEquals(compiled, Result.failure(expected)) } test("fields with variants (6)") { @@ -1027,7 +1030,7 @@ final class FieldMergeSuite extends CatsEffectSuite { val compiled = FieldMergeMapping.compiler.compile(query) - assertEquals(compiled.map(_.query), Result.failure(expected)) + assertEquals(compiled, Result.failure(expected)) } test("fields with variants (7)") { @@ -1050,12 +1053,12 @@ final class FieldMergeSuite extends CatsEffectSuite { List( Narrow( FieldMergeMapping.UserType, - Select("name", None, Empty) + Select("name", None, Empty, loc(5, 13)) ), - Select("id", None, Empty) + Select("id", None, Empty, loc(7, 11)) ) - ) - ) + ), + loc(3, 9)) val expectedResult = json""" { @@ -1110,10 +1113,10 @@ final class FieldMergeSuite extends CatsEffectSuite { Unique( Filter( Eql(UniquePath(List("id")), Const("1")), - Select("name", None, Empty) + Select("name", None, Empty, loc(4, 11)) ) - ) - ) + ), + loc(3, 9)) val expectedResult = json""" { @@ -1151,10 +1154,10 @@ final class FieldMergeSuite extends CatsEffectSuite { Unique( Filter( Eql(UniquePath(List("id")), Const("1")), - Select("name", None, Empty) + Select("name", None, Empty, loc(4, 11)) ) - ) - ) + ), + loc(3, 9)) val expectedResult = json""" { @@ -1192,10 +1195,10 @@ final class FieldMergeSuite extends CatsEffectSuite { Unique( Filter( Eql(UniquePath(List("id")), Const("1")), - Select("name", None, Empty) + Select("name", None, Empty, loc(4, 11)) ) - ) - ) + ), + loc(3, 9)) val expectedResult = json""" { @@ -1241,11 +1244,12 @@ final class FieldMergeSuite extends CatsEffectSuite { Eql(UniquePath(List("id")), Const("1")), Group( List( - Select("name", None, Empty), - Select("profilePic", None, Empty) + Select("name", None, Empty, loc(5, 13)), + Select("profilePic", None, Empty, loc(10, 13)) )) ) - ) + ), + loc(4, 11) ) val expectedResult = json""" @@ -1293,11 +1297,12 @@ final class FieldMergeSuite extends CatsEffectSuite { Eql(UniquePath(List("id")), Const("1")), Group( List( - Select("name", None, Empty), - Select("profilePic", None, Empty) + Select("name", None, Empty, loc(5, 13)), + Select("profilePic", None, Empty, loc(10, 13)) )) ) - ) + ), + loc(3, 9) ) val expectedResult = json""" @@ -1354,13 +1359,15 @@ final class FieldMergeSuite extends CatsEffectSuite { FieldMergeMapping.UserType, Group( List( - Select("name", None, Empty), - Select("profilePic", None, Empty) + Select("name", None, Empty, loc(6, 15)), + Select("profilePic", None, Empty, loc(13, 15)) )) - ) + ), + loc(4, 11) ) ) - ) + ), + loc(3, 9) ) val expectedResult = json""" @@ -1410,7 +1417,7 @@ final class FieldMergeSuite extends CatsEffectSuite { val compiled = FieldMergeMapping.compiler.compile(query) - assertEquals(compiled.map(_.query), Result.failure(expected)) + assertEquals(compiled, Result.failure(expected)) } test("inline fragments (5)") { @@ -1453,20 +1460,22 @@ final class FieldMergeSuite extends CatsEffectSuite { None, Group( List( - Narrow(FieldMergeMapping.UserType, Select("name", None, Empty)), - Narrow(FieldMergeMapping.ProfileType, Select("id", None, Empty)), + Narrow(FieldMergeMapping.UserType, Select("name", None, Empty, loc(7, 17))), + Narrow(FieldMergeMapping.ProfileType, Select("id", None, Empty, loc(10, 17))), Narrow( FieldMergeMapping.UserType, Group( List( - Select("id", None, Empty), - Select("name", None, Empty) + Select("id", None, Empty, loc(19, 17)), + Select("name", None, Empty, loc(20, 17)) )) ) - )) + )), + loc(5, 13) ) ) - ) + ), + loc(3, 9) ) val expectedResult = json""" @@ -1535,15 +1544,17 @@ final class FieldMergeSuite extends CatsEffectSuite { FieldMergeMapping.UserType, Group( List( - Select("id", None, Empty), - Select("name", None, Empty) + Select("id", None, Empty, loc(7, 17)), + Select("name", None, Empty, loc(8, 17)) )) ), - Narrow(FieldMergeMapping.ProfileType, Select("id", None, Empty)) - )) + Narrow(FieldMergeMapping.ProfileType, Select("id", None, Empty, loc(20, 17))) + )), + loc(5, 13) ) ) - ) + ), + loc(3, 9) ) val expectedResult = json""" @@ -1589,7 +1600,7 @@ final class FieldMergeSuite extends CatsEffectSuite { val compiled = FieldMergeMapping.compiler.compile(query) - assertEquals(compiled.map(_.query), Result.failure(expected)) + assertEquals(compiled, Result.failure(expected)) } test("inline fragments (8)") { @@ -1626,23 +1637,17 @@ final class FieldMergeSuite extends CatsEffectSuite { None, Group( List( - Select( - "friends", - None, - Select("name", None, Empty) - ), + Select("friends", None, Select("name", None, Empty, loc(7, 17)), loc(6, 15)), Narrow( FieldMergeMapping.UserType, - Select( - "friends", - None, - Select("id", None, Empty) - ) + Select("friends", None, Select("id", None, Empty, loc(13, 19)), loc(12, 17)) ) - )) + )), + loc(4, 11) ) ) - ) + ), + loc(3, 9) ) val expectedResult = json""" @@ -1715,19 +1720,17 @@ final class FieldMergeSuite extends CatsEffectSuite { None, Group( List( - Select("id", None, Empty), + Select("id", None, Empty, loc(6, 15)), Narrow( FieldMergeMapping.UserType, - Select( - "friends", - None, - Select("name", None, Empty) - ) + Select("friends", None, Select("name", None, Empty, loc(9, 19)), loc(8, 17)) ) - )) + )), + loc(4, 11) ) ) - ) + ), + loc(3, 9) ) val expectedResult = json""" @@ -1790,7 +1793,7 @@ final class FieldMergeSuite extends CatsEffectSuite { val compiled = FieldMergeMapping.compiler.compile(query) - assertEquals(compiled.map(_.query), Result.failure(expected)) + assertEquals(compiled, Result.failure(expected)) } test("merge across fragments") { @@ -1826,16 +1829,17 @@ final class FieldMergeSuite extends CatsEffectSuite { None, Group( List( - Select("name", None, Empty), - Select("profilePic", None, Empty) + Select("name", None, Empty, loc(5, 13)), + Select("profilePic", None, Empty, loc(13, 11)) ) - ) - ), - Select("id", None, Empty) + ), + loc(4, 11)), + Select("id", None, Empty, loc(15, 9)) ) ) ) - ) + ), + loc(3, 9) ) val expectedResult = json""" @@ -1899,18 +1903,15 @@ final class FieldMergeSuite extends CatsEffectSuite { Eql(UniquePath(List("id")), Const("1")), Group( List( - Select("name", None, Empty), - Select("profilePic", None, Empty) + Select("name", None, Empty, loc(4, 11)), + Select("profilePic", None, Empty, loc(12, 11)) ) ) ) - ) + ), + loc(3, 9) ), - Select( - "profiles", - None, - Select("id", None, Empty) - ) + Select("profiles", None, Select("id", None, Empty, loc(16, 11)), loc(15, 9)) ) ) diff --git a/modules/core/src/test/scala/compiler/FragmentSuite.scala b/modules/core/src/test/scala/compiler/FragmentSuite.scala index 1e139ab5..413e753f 100644 --- a/modules/core/src/test/scala/compiler/FragmentSuite.scala +++ b/modules/core/src/test/scala/compiler/FragmentSuite.scala @@ -20,6 +20,7 @@ import cats.implicits._ import io.circe.Json import io.circe.literal._ import munit.CatsEffectSuite +import utils.QueryLocations._ import grackle._ import grackle.Predicate._ @@ -62,6 +63,7 @@ final class FragmentSuite extends CatsEffectSuite { val expected = Select( "user", + None, Unique( Filter( Eql(FragmentMapping.UserType / "id", Const("1")), @@ -69,25 +71,30 @@ final class FragmentSuite extends CatsEffectSuite { List( Select( "friends", + None, Group( List( - Select("id"), - Select("name"), - Select("profilePic") - )) + Select("id", None, Empty, loc(14, 9)), + Select("name", None, Empty, loc(15, 9)), + Select("profilePic", None, Empty, loc(16, 9)) + )), + loc(4, 11) ), Select( "mutualFriends", + None, Group( List( - Select("id"), - Select("name"), - Select("profilePic") - )) + Select("id", None, Empty, loc(14, 9)), + Select("name", None, Empty, loc(15, 9)), + Select("profilePic", None, Empty, loc(16, 9)) + )), + loc(7, 11) ) )) ) - ) + ), + loc(3, 9) ) val expectedResult = json""" @@ -159,6 +166,7 @@ final class FragmentSuite extends CatsEffectSuite { val expected = Select( "user", + None, Unique( Filter( Eql(FragmentMapping.UserType / "id", Const("1")), @@ -166,25 +174,30 @@ final class FragmentSuite extends CatsEffectSuite { List( Select( "friends", + None, Group( List( - Select("id"), - Select("name"), - Select("profilePic") - )) + Select("id", None, Empty, loc(14, 9)), + Select("name", None, Empty, loc(15, 9)), + Select("profilePic", None, Empty, loc(20, 9)) + )), + loc(4, 11) ), Select( "mutualFriends", + None, Group( List( - Select("id"), - Select("name"), - Select("profilePic") - )) + Select("id", None, Empty, loc(14, 9)), + Select("name", None, Empty, loc(15, 9)), + Select("profilePic", None, Empty, loc(20, 9)) + )), + loc(7, 11) ) )) ) - ) + ), + loc(3, 9) ) val expectedResult = json""" @@ -260,6 +273,7 @@ final class FragmentSuite extends CatsEffectSuite { val expected = Select( "user", + None, Unique( Filter( Eql(FragmentMapping.UserType / "id", Const("1")), @@ -267,25 +281,30 @@ final class FragmentSuite extends CatsEffectSuite { List( Select( "friends", + None, Group( List( - Select("id"), - Select("name"), - Select("profilePic") - )) + Select("id", None, Empty, loc(14, 9)), + Select("name", None, Empty, loc(19, 9)), + Select("profilePic", None, Empty, loc(24, 9)) + )), + loc(4, 11) ), Select( "mutualFriends", + None, Group( List( - Select("id"), - Select("name"), - Select("profilePic") - )) + Select("id", None, Empty, loc(14, 9)), + Select("name", None, Empty, loc(19, 9)), + Select("profilePic", None, Empty, loc(24, 9)) + )), + loc(7, 11) ) )) ) - ) + ), + loc(3, 9) ) val expectedResult = json""" @@ -356,13 +375,15 @@ final class FragmentSuite extends CatsEffectSuite { val expected = Select( "profiles", + None, Group( List( - Select("id"), - Introspect(FragmentMapping.schema, Select("__typename")), - Narrow(User, Select("name")), - Narrow(Page, Select("title")) - )) + Select("id", None, Empty, loc(4, 11)), + Introspect(FragmentMapping.schema, Select("__typename", None, Empty, loc(5, 11))), + Narrow(User, Select("name", None, Empty, loc(12, 9))), + Narrow(Page, Select("title", None, Empty, loc(16, 9))) + )), + loc(3, 9) ) val expectedResult = json""" @@ -429,12 +450,14 @@ final class FragmentSuite extends CatsEffectSuite { val expected = Select( "profiles", + None, Group( List( - Select("id"), - Narrow(User, Select("name")), - Narrow(Page, Select("title")) - )) + Select("id", None, Empty, loc(4, 11)), + Narrow(User, Select("name", None, Empty, loc(6, 13))), + Narrow(Page, Select("title", None, Empty, loc(9, 13))) + )), + loc(3, 9) ) val expectedResult = json""" @@ -513,31 +536,38 @@ final class FragmentSuite extends CatsEffectSuite { List( Select( "user", + None, Unique( Filter( Eql(FragmentMapping.UserType / "id", Const("1")), Select( "favourite", - Group(List( - Introspect(FragmentMapping.schema, Select("__typename")), - Narrow( - User, - Group(List( - Select("id"), - Select("name") - )) - ), - Narrow( - Page, - Group(List( - Select("id"), - Select("title") - )) - ) - )) + None, + Group( + List( + Introspect( + FragmentMapping.schema, + Select("__typename", None, Empty, loc(5, 13))), + Narrow( + User, + Group(List( + Select("id", None, Empty, loc(20, 9)), + Select("name", None, Empty, loc(21, 9)) + )) + ), + Narrow( + Page, + Group(List( + Select("id", None, Empty, loc(25, 9)), + Select("title", None, Empty, loc(26, 9)) + )) + ) + )), + loc(4, 11) ) ) - ) + ), + loc(3, 9) ), Select( "user", @@ -547,26 +577,32 @@ final class FragmentSuite extends CatsEffectSuite { Eql(FragmentMapping.PageType / "id", Const("2")), Select( "favourite", - Group(List( - Introspect(FragmentMapping.schema, Select("__typename")), - Narrow( - User, - Group(List( - Select("id"), - Select("name") - )) - ), - Narrow( - Page, - Group(List( - Select("id"), - Select("title") - )) - ) - )) + None, + Group( + List( + Introspect( + FragmentMapping.schema, + Select("__typename", None, Empty, loc(12, 13))), + Narrow( + User, + Group(List( + Select("id", None, Empty, loc(20, 9)), + Select("name", None, Empty, loc(21, 9)) + )) + ), + Narrow( + Page, + Group(List( + Select("id", None, Empty, loc(25, 9)), + Select("title", None, Empty, loc(26, 9)) + )) + ) + )), + loc(11, 11) ) ) - ) + ), + loc(10, 9) ) )) @@ -642,27 +678,31 @@ final class FragmentSuite extends CatsEffectSuite { None, Group( List( - Introspect(FragmentMapping.schema, Select("__typename", None, Empty)), + Introspect( + FragmentMapping.schema, + Select("__typename", None, Empty, loc(5, 13))), Narrow( User, Group( List( - Select("id", None, Empty), - Select("name", None, Empty) + Select("id", None, Empty, loc(12, 9)), + Select("name", None, Empty, loc(13, 9)) )) ), Narrow( Page, Group( List( - Select("id", None, Empty), - Select("title", None, Empty) + Select("id", None, Empty, loc(17, 9)), + Select("title", None, Empty, loc(18, 9)) )) ) - )) + )), + loc(4, 11) ) ) - ) + ), + loc(3, 9) ) val expectedResult = json""" @@ -706,15 +746,14 @@ final class FragmentSuite extends CatsEffectSuite { val expected = Select( "user", + None, Unique( Filter( Eql(FragmentMapping.UserType / "id", Const("1")), - Select( - "friends", - Select("id") - ) + Select("friends", None, Select("id", None, Empty, loc(11, 9)), loc(4, 11)) ) - ) + ), + loc(3, 9) ) val expectedResult = json""" diff --git a/modules/core/src/test/scala/compiler/InputValuesSuite.scala b/modules/core/src/test/scala/compiler/InputValuesSuite.scala index 0130bc67..e2d4c872 100644 --- a/modules/core/src/test/scala/compiler/InputValuesSuite.scala +++ b/modules/core/src/test/scala/compiler/InputValuesSuite.scala @@ -575,8 +575,9 @@ final class InputValuesSuite extends CatsEffectSuite { """ assertEquals( - InputValuesMapping.compiler.compile(supplied, None).map(_.query), - InputValuesMapping.compiler.compile(defaulted, None).map(_.query)) + InputValuesMapping.compiler.compile(supplied, None), + InputValuesMapping.compiler.compile(defaulted, None) + ) } test("single value default of an input object field coerces to a list") { diff --git a/modules/core/src/test/scala/compiler/PreserveArgsElaborator.scala b/modules/core/src/test/scala/compiler/PreserveArgsElaborator.scala index 695d08de..38580698 100644 --- a/modules/core/src/test/scala/compiler/PreserveArgsElaborator.scala +++ b/modules/core/src/test/scala/compiler/PreserveArgsElaborator.scala @@ -25,7 +25,7 @@ object PreserveArgsElaborator extends SelectElaborator { def subst(query: Query, fieldName: String, preserved: Preserved): Query = { def loop(query: Query): Query = query match { - case Select(`fieldName`, alias, child) => + case Select(`fieldName`, alias, child, _) => UntypedSelect( fieldName, alias, @@ -44,7 +44,7 @@ object PreserveArgsElaborator extends SelectElaborator { override def transform(query: Query): Elab[Query] = { query match { - case UntypedSelect(fieldName, _, _, _, _) => + case UntypedSelect(fieldName, _, _, _, _, _) => for { t <- super.transform(query) preserved <- Elab.envE[Preserved]("preserved") diff --git a/modules/core/src/test/scala/compiler/ProblemSuite.scala b/modules/core/src/test/scala/compiler/ProblemSuite.scala index 89b45646..65e08625 100644 --- a/modules/core/src/test/scala/compiler/ProblemSuite.scala +++ b/modules/core/src/test/scala/compiler/ProblemSuite.scala @@ -123,14 +123,14 @@ final class ProblemSuite extends CatsEffectSuite { test("toString (full)") { assertEquals( Problem("foo", List(1 -> 2, 5 -> 6), List(Name("bar"), Name("baz"))).toString, - "foo (at bar/baz: 1..2, 5..6)" + "foo (at bar/baz: 1:2, 5:6)" ) } test("toString (no path)") { assertEquals( Problem("foo", List(1 -> 2, 5 -> 6), Nil).toString, - "foo (at 1..2, 5..6)" + "foo (at 1:2, 5:6)" ) } diff --git a/modules/core/src/test/scala/compiler/SkipIncludeSuite.scala b/modules/core/src/test/scala/compiler/SkipIncludeSuite.scala index 3ad700f5..a4c56d1f 100644 --- a/modules/core/src/test/scala/compiler/SkipIncludeSuite.scala +++ b/modules/core/src/test/scala/compiler/SkipIncludeSuite.scala @@ -17,6 +17,7 @@ package compiler import io.circe.literal._ import munit.CatsEffectSuite +import utils.QueryLocations._ import grackle._ import grackle.Query._ @@ -51,8 +52,8 @@ final class SkipIncludeSuite extends CatsEffectSuite { val expected = Group( List( - Select("field", Some("b"), Select("subfieldB")), - Select("field", Some("c"), Select("subfieldA")) + Select("field", Some("b"), Select("subfieldB", None, Empty, loc(7, 11)), loc(6, 9)), + Select("field", Some("c"), Select("subfieldA", None, Empty, loc(10, 11)), loc(9, 9)) )) val compiled = SkipIncludeMapping.compiler.compile(query, untypedVars = Some(variables)) @@ -93,26 +94,26 @@ final class SkipIncludeSuite extends CatsEffectSuite { val expected = Group( List( - Select("field", Some("a")), + Select("field", Some("a"), Empty, loc(3, 9)), Select( "field", Some("b"), Group( List( - Select("subfieldA"), - Select("subfieldB") - )) - ), + Select("subfieldA", None, Empty, loc(18, 9)), + Select("subfieldB", None, Empty, loc(19, 9)) + )), + loc(6, 9)), Select( "field", Some("c"), Group( List( - Select("subfieldA"), - Select("subfieldB") - )) - ), - Select("field", Some("d")) + Select("subfieldA", None, Empty, loc(18, 9)), + Select("subfieldB", None, Empty, loc(19, 9)) + )), + loc(9, 9)), + Select("field", Some("d"), Empty, loc(12, 9)) )) val compiled = SkipIncludeMapping.compiler.compile(query, untypedVars = Some(variables)) @@ -146,12 +147,13 @@ final class SkipIncludeSuite extends CatsEffectSuite { val expected = Select( "field", + None, Group( List( - Select("subfieldB", Some("b")), - Select("subfieldA", Some("c")) - )) - ) + Select("subfieldB", Some("b"), Empty, loc(10, 9)), + Select("subfieldA", Some("c"), Empty, loc(11, 9)) + )), + loc(3, 9)) val compiled = SkipIncludeMapping.compiler.compile(query, untypedVars = Some(variables)) @@ -198,26 +200,26 @@ final class SkipIncludeSuite extends CatsEffectSuite { val expected = Group( List( - Select("field", Some("a")), + Select("field", Some("a"), Empty, loc(3, 9)), Select( "field", Some("b"), Group( List( - Select("subfieldA"), - Select("subfieldB") - )) - ), + Select("subfieldA", None, Empty, loc(11, 13)), + Select("subfieldB", None, Empty, loc(12, 13)) + )), + loc(9, 9)), Select( "field", Some("c"), Group( List( - Select("subfieldA"), - Select("subfieldB") - )) - ), - Select("field", Some("d")) + Select("subfieldA", None, Empty, loc(17, 13)), + Select("subfieldB", None, Empty, loc(18, 13)) + )), + loc(15, 9)), + Select("field", Some("d"), Empty, loc(21, 9)) )) val compiled = SkipIncludeMapping.compiler.compile(query, untypedVars = Some(variables)) @@ -249,12 +251,13 @@ final class SkipIncludeSuite extends CatsEffectSuite { val expected = Select( "field", + None, Group( List( - Select("subfieldB", Some("b")), - Select("subfieldA", Some("c")) - )) - ) + Select("subfieldB", Some("b"), Empty, loc(6, 13)), + Select("subfieldA", Some("c"), Empty, loc(7, 13)) + )), + loc(3, 9)) val compiled = SkipIncludeMapping.compiler.compile(query, untypedVars = Some(variables)) @@ -271,7 +274,8 @@ final class SkipIncludeSuite extends CatsEffectSuite { } """ - val expected = Select("field", Some("a"), Select("subfieldA")) + val expected = + Select("field", Some("a"), Select("subfieldA", None, Empty, loc(3, 56)), loc(3, 9)) val compiled = SkipIncludeMapping.compiler.compile(query) @@ -287,9 +291,7 @@ final class SkipIncludeSuite extends CatsEffectSuite { val compiled = SkipIncludeMapping.compiler.compile(query) - assertEquals( - compiled.map(_.query), - Result.failure("Directive 'skip' may not occur more than once")) + assertEquals(compiled, Result.failure("Directive 'skip' may not occur more than once")) } test("repeated include on the same selection is rejected") { @@ -301,9 +303,7 @@ final class SkipIncludeSuite extends CatsEffectSuite { val compiled = SkipIncludeMapping.compiler.compile(query) - assertEquals( - compiled.map(_.query), - Result.failure("Directive 'include' may not occur more than once")) + assertEquals(compiled, Result.failure("Directive 'include' may not occur more than once")) } } diff --git a/modules/core/src/test/scala/composed/ComposedData.scala b/modules/core/src/test/scala/composed/ComposedData.scala index 0c777d7a..2b5c0f18 100644 --- a/modules/core/src/test/scala/composed/ComposedData.scala +++ b/modules/core/src/test/scala/composed/ComposedData.scala @@ -193,7 +193,7 @@ object ComposedMapping extends ComposedMapping[IO] { def countryCurrencyJoin(q: Query, c: Cursor): Result[Query] = (c.focus, q) match { - case (c: CountryData.Country, Select("currency", _, child)) => + case (c: CountryData.Country, Select("currency", _, child, _)) => Select( "fx", Unique(Filter(Eql(CurrencyType / "code", Const(c.currencyCode)), child))).success diff --git a/modules/core/src/test/scala/composed/ComposedListSuite.scala b/modules/core/src/test/scala/composed/ComposedListSuite.scala index 0a109a04..72893a6a 100644 --- a/modules/core/src/test/scala/composed/ComposedListSuite.scala +++ b/modules/core/src/test/scala/composed/ComposedListSuite.scala @@ -179,7 +179,7 @@ object ComposedListMapping extends ComposedMapping[IO] { def collectionItemJoin(q: Query, c: Cursor): Result[Query] = (c.focus, q) match { - case (c: CollectionData.Collection, Select("items", _, child)) => + case (c: CollectionData.Collection, Select("items", _, child, _)) => Group(c .itemIds .map(id => diff --git a/modules/core/src/test/scala/conformance/ResponseSuite.scala b/modules/core/src/test/scala/conformance/ResponseSuite.scala index 765997ab..9e72a851 100644 --- a/modules/core/src/test/scala/conformance/ResponseSuite.scala +++ b/modules/core/src/test/scala/conformance/ResponseSuite.scala @@ -77,26 +77,21 @@ final class ResponseSuite extends ConformanceSuite { // 7.1.6 Errors // https://spec.graphql.org/September2025/#sec-Request-Error-Result - // Grackle discards the whole `data` entry when a field raises an error, and it attaches - // neither `path` nor `locations` to the error. The response is - // `{"errors": [{"message": "..."}], "data": null}`. /** * The request of section 7.1.6, which the specification runs against two schemas. */ - private val heroFriendsDoc = """ - query ($episode: Episode!) { - hero(episode: $episode) { - name - heroFriends: friends { - id - name - } - } + private val heroFriendsDoc = """query ($episode: Episode!) { + hero(episode: $episode) { + name + heroFriends: friends { + id + name } - """ + } +}""" yields( - "an error carries the response path of the position which raised it".fail, + "an error carries the response path of the position which raised it", ResponseMappings.NullableName, json"""{"episode": "NEWHOPE"}""")(heroFriendsDoc)(json""" { @@ -132,7 +127,7 @@ final class ResponseSuite extends ConformanceSuite { // The same request against a schema whose `name` field is non-null. The null bubbles up to the // nearest nullable position, which is the entry of the `heroFriends` list. yields( - "a null from an error bubbles up to the nearest nullable position".fail, + "a null from an error bubbles up to the nearest nullable position", ResponseMappings.NonNullName, json"""{"episode": "NEWHOPE"}""")(heroFriendsDoc)(json""" { diff --git a/modules/core/src/test/scala/directives/QueryDirectivesSuite.scala b/modules/core/src/test/scala/directives/QueryDirectivesSuite.scala index 8db4783e..3b2858e9 100644 --- a/modules/core/src/test/scala/directives/QueryDirectivesSuite.scala +++ b/modules/core/src/test/scala/directives/QueryDirectivesSuite.scala @@ -186,7 +186,7 @@ object QueryDirectivesMapping extends ValueMapping[IO] { object upperCaseElaborator extends Phase { override def transform(query: Query): Elab[Query] = query match { - case UntypedSelect(nme, alias, _, directives, _) + case UntypedSelect(nme, alias, _, directives, _, _) if directives.exists(_.name == "upperCase") => for { c <- Elab.context diff --git a/modules/core/src/test/scala/errors/FieldErrorSuite.scala b/modules/core/src/test/scala/errors/FieldErrorSuite.scala index 5a8a685e..de87181f 100644 --- a/modules/core/src/test/scala/errors/FieldErrorSuite.scala +++ b/modules/core/src/test/scala/errors/FieldErrorSuite.scala @@ -42,6 +42,8 @@ final class FieldErrorSuite extends CatsEffectSuite { } """ + private val nameLocation = json"""[{ "line": 6, "column": 9 }]""" + /** * The response for `query`, with `data` as its data entry. * @@ -54,6 +56,7 @@ final class FieldErrorSuite extends CatsEffectSuite { "errors": [ { "message": $message, + "locations": $nameLocation, "path": ["items", 1, "name"] } ], @@ -109,6 +112,7 @@ final class FieldErrorSuite extends CatsEffectSuite { "errors": [ { "message": $message, + "locations": [{ "line": 4, "column": 11 }], "path": ["items", 1, "name"] } ], @@ -138,6 +142,7 @@ final class FieldErrorSuite extends CatsEffectSuite { "errors": [ { "message": $message, + "locations": [{ "line": 4, "column": 9 }], "path": ["tagCount"] } ], @@ -169,6 +174,7 @@ final class FieldErrorSuite extends CatsEffectSuite { "errors": [ { "message": $message, + "locations": [{ "line": 5, "column": 9 }], "path": ["delegated", "name"] } ], @@ -191,6 +197,23 @@ final class FieldErrorSuite extends CatsEffectSuite { assertIO(NonNullDelegate.compileAndRun(delegateQuery), delegateExpected(Json.Null)) } + test("an error carries the source location of the field which raised it") { + val located = """query { + items { + name + } +}""" + + val expected = json"""[{ "line": 3, "column": 5 }]""" + + assertIO( + NullableName + .compileAndRun(located) + .map(_.hcursor.downField("errors").downN(0).downField("locations").focus), + Some(expected) + ) + } + test("a response path uses the alias of the position") { val aliased = """ query { diff --git a/modules/core/src/test/scala/minimizer/MinimizerSuite.scala b/modules/core/src/test/scala/minimizer/MinimizerSuite.scala index 327072be..a04e1f0d 100644 --- a/modules/core/src/test/scala/minimizer/MinimizerSuite.scala +++ b/modules/core/src/test/scala/minimizer/MinimizerSuite.scala @@ -16,6 +16,7 @@ package minimizer import munit.CatsEffectSuite +import utils.QueryLocations._ import grackle.{GraphQLParser, QueryMinimizer, Result} @@ -35,7 +36,8 @@ final class MinimizerSuite extends CatsEffectSuite { val Some(parsed0) = parser.parseText(query).toOption: @unchecked val Some(parsed1) = parser.parseText(minimized).toOption: @unchecked - assertEquals(parsed0, parsed1) + // Minimize changes text, so compare without locations + assertEquals(stripLocations(parsed0), stripLocations(parsed1)) } test("minimize simple query") { diff --git a/modules/core/src/test/scala/parser/ParserSuite.scala b/modules/core/src/test/scala/parser/ParserSuite.scala index a0233dc1..07515f00 100644 --- a/modules/core/src/test/scala/parser/ParserSuite.scala +++ b/modules/core/src/test/scala/parser/ParserSuite.scala @@ -51,9 +51,9 @@ final class ParserSuite extends CatsEffectSuite { List((Name("id"), IntValue(1000))), Nil, List( - Field(None, Name("name"), Nil, Nil, Nil) - ) - ) + Field(None, Name("name"), Nil, Nil, Nil, Some((4, 11))) + ), + Some((3, 9))) ) ) @@ -86,8 +86,9 @@ final class ParserSuite extends CatsEffectSuite { ), Nil, List( - Field(None, Name("quux"), Nil, Nil, Nil) - ) + Field(None, Name("quux"), Nil, Nil, Nil, Some((4, 11))) + ), + Some((3, 9)) ) ) ) @@ -124,8 +125,9 @@ final class ParserSuite extends CatsEffectSuite { ), Nil, List( - Field(None, Name("quux"), Nil, Nil, Nil) - ) + Field(None, Name("quux"), Nil, Nil, Nil, Some((4, 11))) + ), + Some((3, 9)) ) ) ) @@ -172,28 +174,29 @@ final class ParserSuite extends CatsEffectSuite { Nil, Nil, List( - Field(None, Name("name"), Nil, Nil, Nil) - ) - ), + Field(None, Name("name"), Nil, Nil, Nil, Some((5, 13))) + ), + Some((4, 11))), Field( None, Name("mutationType"), Nil, Nil, List( - Field(None, Name("name"), Nil, Nil, Nil) - ) - ), + Field(None, Name("name"), Nil, Nil, Nil, Some((8, 13))) + ), + Some((7, 11))), Field( None, Name("subscriptionType"), Nil, Nil, List( - Field(None, Name("name"), Nil, Nil, Nil) - ) - ) - ) + Field(None, Name("name"), Nil, Nil, Nil, Some((11, 13))) + ), + Some((10, 11))) + ), + Some((3, 9)) ) ) ) @@ -228,26 +231,28 @@ final class ParserSuite extends CatsEffectSuite { List((Name("episode"), EnumValue(Name("NEWHOPE")))), Nil, List( - Field(None, Name("name"), Nil, Nil, Nil), + Field(None, Name("name"), Nil, Nil, Nil, Some((4, 11))), Field( None, Name("friends"), Nil, Nil, List( - Field(None, Name("name"), Nil, Nil, Nil), + Field(None, Name("name"), Nil, Nil, Nil, Some((6, 13))), Field( None, Name("friends"), Nil, Nil, List( - Field(None, Name("name"), Nil, Nil, Nil) - ) - ) - ) + Field(None, Name("name"), Nil, Nil, Nil, Some((8, 15))) + ), + Some((7, 13))) + ), + Some((5, 11)) ) - ) + ), + Some((3, 9)) ) ) ) @@ -279,21 +284,24 @@ final class ParserSuite extends CatsEffectSuite { List((Name("id"), IntValue(4))), Nil, List( - Field(None, Name("id"), Nil, Nil, Nil), - Field(None, Name("name"), Nil, Nil, Nil), + Field(None, Name("id"), Nil, Nil, Nil, Some((4, 11))), + Field(None, Name("name"), Nil, Nil, Nil, Some((5, 11))), Field( Some(Name("smallPic")), Name("profilePic"), List((Name("size"), IntValue(64))), Nil, - Nil), + Nil, + Some((6, 11))), Field( Some(Name("bigPic")), Name("profilePic"), List((Name("size"), IntValue(1024))), Nil, - Nil) - ) + Nil, + Some((7, 11))) + ), + Some((3, 9)) ) ) ) @@ -324,15 +332,17 @@ final class ParserSuite extends CatsEffectSuite { Name("character"), List((Name("id"), StringValue("1000"))), Nil, - List(Field(None, Name("name"), Nil, Nil, Nil))), + List(Field(None, Name("name"), Nil, Nil, Nil, Some((4, 11)))), + Some((3, 9))), Field( Some(Name("darth")), Name("character"), List((Name("id"), StringValue("1001"))), Nil, List( - Field(None, Name("name"), Nil, Nil, Nil) - ) + Field(None, Name("name"), Nil, Nil, Nil, Some((7, 11))) + ), + Some((6, 9)) ) ) ) @@ -367,15 +377,17 @@ final class ParserSuite extends CatsEffectSuite { List((Name("id"), IntValue(4))), Nil, List( - Field(None, Name("id"), Nil, Nil, Nil), - Field(None, Name("name"), Nil, Nil, Nil), + Field(None, Name("id"), Nil, Nil, Nil, Some((4, 11))), + Field(None, Name("name"), Nil, Nil, Nil, Some((5, 11))), Field( None, Name("profilePic"), List((Name("size"), Variable(Name("devicePicSize")))), Nil, - Nil) - ) + Nil, + Some((6, 11))) + ), + Some((3, 9)) ) ) ) @@ -415,15 +427,17 @@ final class ParserSuite extends CatsEffectSuite { List((Name("id"), IntValue(4))), Nil, List( - Field(None, Name("id"), Nil, Nil, Nil), - Field(None, Name("name"), Nil, Nil, Nil), + Field(None, Name("id"), Nil, Nil, Nil, Some((4, 11))), + Field(None, Name("name"), Nil, Nil, Nil, Some((5, 11))), Field( None, Name("profilePic"), List((Name("size"), Variable(Name("devicePicSize")))), Nil, - Nil) - ) + Nil, + Some((6, 11))) + ), + Some((3, 9)) ) ) ) @@ -463,15 +477,17 @@ final class ParserSuite extends CatsEffectSuite { List((Name("id"), IntValue(4))), Nil, List( - Field(None, Name("id"), Nil, Nil, Nil), - Field(None, Name("name"), Nil, Nil, Nil), + Field(None, Name("id"), Nil, Nil, Nil, Some((4, 11))), + Field(None, Name("name"), Nil, Nil, Nil, Some((5, 11))), Field( None, Name("profilePic"), List((Name("size"), Variable(Name("devicePicSize")))), Nil, - Nil) - ) + Nil, + Some((6, 11))) + ), + Some((3, 9)) ) ) ) @@ -522,28 +538,29 @@ final class ParserSuite extends CatsEffectSuite { Nil, Nil, List( - Field(None, Name("name"), Nil, Nil, Nil) - ) - ), + Field(None, Name("name"), Nil, Nil, Nil, Some((6, 13))) + ), + Some((5, 11))), Field( None, Name("mutationType"), Nil, Nil, List( - Field(None, Name("name"), Nil, Nil, Nil) - ) - ), + Field(None, Name("name"), Nil, Nil, Nil, Some((9, 13))) + ), + Some((8, 11))), Field( None, Name("subscriptionType"), Nil, Nil, List( - Field(None, Name("name"), Nil, Nil, Nil) - ) - ) - ) + Field(None, Name("name"), Nil, Nil, Nil, Some((14, 13))) + ), + Some((13, 11))) + ), + Some((4, 9)) ) ) ) @@ -596,10 +613,11 @@ final class ParserSuite extends CatsEffectSuite { Some(Named(Name("Character"))), Nil, List( - Field(None, Name("age"), Nil, Nil, Nil) + Field(None, Name("age"), Nil, Nil, Nil, Some((6, 13))) ) ) - ) + ), + Some((3, 9)) ) ) ), @@ -608,7 +626,7 @@ final class ParserSuite extends CatsEffectSuite { Named(Name("Character")), Nil, List( - Field(None, Name("name"), Nil, Nil, Nil) + Field(None, Name("name"), Nil, Nil, Nil, Some((12, 9))) ) ) ) @@ -645,14 +663,15 @@ final class ParserSuite extends CatsEffectSuite { List((Name("id"), IntValue(1000))), Nil, List( - Field(None, Name("name"), Nil, Nil, Nil), + Field(None, Name("name"), Nil, Nil, Nil, Some((4, 11))), InlineFragment( None, List( Directive(Name("include"), List((Name("if"), Variable(Name("expanded")))))), - List(Field(None, Name("age"), List(), List(), List())) + List(Field(None, Name("age"), List(), List(), List(), Some((6, 13)))) ) - ) + ), + Some((3, 9)) ) ) ) @@ -688,13 +707,14 @@ final class ParserSuite extends CatsEffectSuite { List((Name("id"), IntValue(1000))), Nil, List( - Field(None, Name("name"), Nil, Nil, Nil), + Field(None, Name("name"), Nil, Nil, Nil, Some((4, 11))), InlineFragment( None, List(Directive(Name("dir"), Nil)), - List(Field(None, Name("age"), List(), List(), List())) + List(Field(None, Name("age"), List(), List(), List(), Some((6, 13)))) ) - ) + ), + Some((3, 9)) ) ) ) @@ -709,7 +729,7 @@ final class ParserSuite extends CatsEffectSuite { def assertParse(input: String, expected: Value) = parser.parseText(s"query { foo(bar: $input) }").toOption match { - case Some(List(Operation(_, _, _, _, List(Field(_, _, List((_, v)), _, _)), _))) => + case Some(List(Operation(_, _, _, _, List(Field(_, _, List((_, v)), _, _, _)), _))) => assertEquals(v, expected) case _ => assert(false) } @@ -972,12 +992,18 @@ final class ParserSuite extends CatsEffectSuite { } test("comment at end of file without a line terminator") { - val expected = - List(Operation(Query, None, Nil, Nil, List(Field(None, Name("x"), Nil, Nil, Nil)))) + def expected(location: (Int, Int)) = + List( + Operation( + Query, + None, + Nil, + Nil, + List(Field(None, Name("x"), Nil, Nil, Nil, Some(location))))) - assertEquals(parser.parseText("query { x } # done"), Result(expected)) - assertEquals(parser.parseText("query { x } #"), Result(expected)) - assertEquals(parser.parseText("query { # inner\n x } # a\n# b"), Result(expected)) + assertEquals(parser.parseText("query { x } # done"), Result(expected((1, 9)))) + assertEquals(parser.parseText("query { x } #"), Result(expected((1, 9)))) + assertEquals(parser.parseText("query { # inner\n x } # a\n# b"), Result(expected((2, 2)))) } test("fragment name that starts with 'on'") { @@ -994,12 +1020,18 @@ final class ParserSuite extends CatsEffectSuite { Nil, Nil, List( - Field(None, Name("x"), Nil, Nil, List(FragmentSpread(Name("onlyFriends"), Nil))))), + Field( + None, + Name("x"), + Nil, + Nil, + List(FragmentSpread(Name("onlyFriends"), Nil)), + Some((2, 15))))), FragmentDefinition( Name("onlyFriends"), Named(Name("X")), Nil, - List(Field(None, Name("name"), Nil, Nil, Nil))) + List(Field(None, Name("name"), Nil, Nil, Nil, Some((3, 35))))) ) assertEquals(parser.parseText(query), Result(expected)) @@ -1028,7 +1060,8 @@ final class ParserSuite extends CatsEffectSuite { (Name("c"), EnumValue(Name("nullable"))) ), Nil, - Nil + Nil, + Some((1, 9)) )) ) @@ -1054,7 +1087,9 @@ final class ParserSuite extends CatsEffectSuite { (Name("c"), NullValue) ), Nil, - Nil))) + Nil, + Some((1, 9)))) + ) assertEquals(parser.parseText(query), Result(List(expected))) } @@ -1068,7 +1103,7 @@ final class ParserSuite extends CatsEffectSuite { None, Nil, Nil, - List(Field(None, Name("likeStory"), Nil, Nil, Nil)), + List(Field(None, Name("likeStory"), Nil, Nil, Nil, Some((1, 31)))), Some("Like a story")) assertEquals(parser.parseText(query), Result(List(expected))) @@ -1097,7 +1132,7 @@ final class ParserSuite extends CatsEffectSuite { Nil, Some("the id to fetch"))), Nil, - List(Field(None, Name("x"), Nil, Nil, Nil)), + List(Field(None, Name("x"), Nil, Nil, Nil, Some((7, 9)))), Some("Fetch x") ) @@ -1118,12 +1153,19 @@ final class ParserSuite extends CatsEffectSuite { None, Nil, Nil, - List(Field(None, Name("x"), Nil, Nil, List(FragmentSpread(Name("frag"), Nil))))), + List( + Field( + None, + Name("x"), + Nil, + Nil, + List(FragmentSpread(Name("frag"), Nil)), + Some((2, 15))))), FragmentDefinition( Name("frag"), Named(Name("X")), Nil, - List(Field(None, Name("name"), Nil, Nil, Nil)), + List(Field(None, Name("name"), Nil, Nil, Nil, Some((4, 28)))), Some("shared fields")) ) diff --git a/modules/core/src/test/scala/utils/QueryLocations.scala b/modules/core/src/test/scala/utils/QueryLocations.scala new file mode 100644 index 00000000..6a470b4d --- /dev/null +++ b/modules/core/src/test/scala/utils/QueryLocations.scala @@ -0,0 +1,54 @@ +// Copyright (c) 2016-2025 Association of Universities for Research in Astronomy, Inc. (AURA) +// Copyright (c) 2016-2025 Grackle Contributors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package utils + +import grackle.Ast + +/** + * Helpers for the source locations of field selections. + */ +object QueryLocations { + + /** + * Helper to create a location for a field selection + */ + def loc(line: Int, column: Int): Option[(Int, Int)] = + Some((line, column)) + + /** + * Clears the source location of every field selection of the document `doc`. + */ + def stripLocations(doc: Ast.Document): Ast.Document = + doc.map { + case d: Ast.OperationDefinition.QueryShorthand => + d.copy(selectionSet = stripSelectionLocations(d.selectionSet)) + case d: Ast.OperationDefinition.Operation => + d.copy(selectionSet = stripSelectionLocations(d.selectionSet)) + case d: Ast.FragmentDefinition => + d.copy(selectionSet = stripSelectionLocations(d.selectionSet)) + case other => other + } + + private def stripSelectionLocations(sels: List[Ast.Selection]): List[Ast.Selection] = + sels.map { + case s: Ast.Selection.Field => + s.copy(selectionSet = stripSelectionLocations(s.selectionSet), location = None) + case s: Ast.Selection.InlineFragment => + s.copy(selectionSet = stripSelectionLocations(s.selectionSet)) + case other => other + } + +} diff --git a/modules/sql-core/src/main/scala/SqlMapping.scala b/modules/sql-core/src/main/scala/SqlMapping.scala index b93c0782..1c34b44f 100644 --- a/modules/sql-core/src/main/scala/SqlMapping.scala +++ b/modules/sql-core/src/main/scala/SqlMapping.scala @@ -898,18 +898,18 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self // Preserve OrderBy case o: OrderBy => o.copy(child = loop(o.child, context)) - case s @ Select(fieldName, _, Count(_)) => + case s @ Select(fieldName, _, Count(_), _) => if (context.tpe.underlying.hasField(fieldName)) s.copy(child = Empty) else Empty - case s @ Select(fieldName, resultName, _) => + case s @ Select(fieldName, resultName, _, _) => val fieldContext = context .forField(fieldName, resultName) .getOrElse( throw new SqlMappingException(s"No field '$fieldName' of type ${context.tpe}")) s.copy(child = loop(s.child, fieldContext)) - case s @ UntypedSelect(fieldName, resultName, _, _, _) => + case s @ UntypedSelect(fieldName, resultName, _, _, _, _) => val fieldContext = context .forField(fieldName, resultName) .getOrElse( @@ -3837,7 +3837,7 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self def unapply(q: Query): Option[(Query, List[Narrow])] = { def isPolySelect(q: Query): Boolean = q match { - case Select(fieldName, _, _) => + case Select(fieldName, _, _, _) => typeMappings.fieldIsPolymorphic(context, fieldName) case _ => false } @@ -3849,7 +3849,7 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self } val ungrouped = ungroup(q).flatMap { - case sel @ Select(fieldName, _, _) if isPolySelect(sel) => + case sel @ Select(fieldName, _, _, _) if isPolySelect(sel) => typeMappings.rawFieldMapping(context, fieldName) match { case Some(TypeMappings.PolymorphicFieldMapping(cands)) => cands.map { case (pred, _) => Narrow(schema.uncheckedRef(pred.tpe), sel) } @@ -3974,12 +3974,12 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self object NonSubobjectSelect { def unapply(q: Query): Option[String] = q match { - case Select(fieldName, _, child) + case Select(fieldName, _, child, _) if child == Empty || isJsonb(context, fieldName) || !isLocallyMapped( context, q) => Some(fieldName) - case Select(fieldName, _, Effect(_, _)) => + case Select(fieldName, _, Effect(_, _), _) => Some(fieldName) case _ => None @@ -3987,10 +3987,10 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self } q match { - case Select(fieldName, _, Count(child)) => + case Select(fieldName, _, Count(child), _) => def childContext(q: Query): Result[Context] = q match { - case Select(fieldName, resultName, _) => + case Select(fieldName, resultName, _, _) => context.forField(fieldName, resultName) case FilterOrderByOffsetLimit(_, _, _, _, child) => childContext(child) @@ -4088,7 +4088,7 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self } // Non-leaf non-Json element: compile subobject queries - case s @ Select(fieldName, resultName, child) => + case s @ Select(fieldName, resultName, child, _) => context.forField(fieldName, resultName).flatMap { fieldContext => if (schema.isRootType(context.tpe)) loop(child, fieldContext, Nil, false) else { @@ -4127,7 +4127,7 @@ trait SqlMappingLike[F[_]] extends CirceMappingLike[F] with SqlModule[F] { self def loop(query: Query): Boolean = query match { case Empty => true - case Select(_, _, Empty) => true + case Select(_, _, Empty, _) => true case Group(children) => children.forall(loop) case _ => false } diff --git a/modules/sql-core/src/test/scala/SqlComposedWorldMapping.scala b/modules/sql-core/src/test/scala/SqlComposedWorldMapping.scala index cbcc8c66..f14c1c2f 100644 --- a/modules/sql-core/src/test/scala/SqlComposedWorldMapping.scala +++ b/modules/sql-core/src/test/scala/SqlComposedWorldMapping.scala @@ -116,7 +116,7 @@ class CurrencyMapping[F[_]: Sync](dataRef: Ref[F, CurrencyData], countRef: Ref[F val expandedQueries = queries.map { - case (Select("currencies", _, child), c @ Code(code)) => + case (Select("currencies", _, child, _), c @ Code(code)) => (SimpleCurrencyQuery(List(code), child), c) case other => other } @@ -179,7 +179,7 @@ class CurrencyMapping[F[_]: Sync](dataRef: Ref[F, CurrencyData], countRef: Ref[F def unapply(sel: Query): Option[(String, Query)] = sel match { - case Environment(env, Select("currencies", None, child)) => + case Environment(env, Select("currencies", None, child, _)) => env.get[List[String]]("countryCodes").flatMap(_.headOption).map((_, child)) case _ => None } diff --git a/modules/sql-core/src/test/scala/SqlNestedEffectsMapping.scala b/modules/sql-core/src/test/scala/SqlNestedEffectsMapping.scala index 95de34e6..3354008e 100644 --- a/modules/sql-core/src/test/scala/SqlNestedEffectsMapping.scala +++ b/modules/sql-core/src/test/scala/SqlNestedEffectsMapping.scala @@ -277,7 +277,7 @@ trait SqlNestedEffectsMapping[F[_]] extends SqlTestMapping[F] { } runGrouped(queries) { - case (Select(_, _, child), cursors, indices) => + case (Select(_, _, child, _), cursors, indices) => val codes = cursors .flatMap(_.fieldAs[Json]("countryCode").toOption.flatMap(_.asString).toList) .map(toCode) diff --git a/modules/sql-core/src/test/scala/SqlWorldCompilerSuite.scala b/modules/sql-core/src/test/scala/SqlWorldCompilerSuite.scala index 6a316a61..4b8c4ea5 100644 --- a/modules/sql-core/src/test/scala/SqlWorldCompilerSuite.scala +++ b/modules/sql-core/src/test/scala/SqlWorldCompilerSuite.scala @@ -88,8 +88,12 @@ trait SqlWorldCompilerSuite extends CatsEffectSuite { SqlStatsMonitor.SqlStats( Select( "country", + None, Unique( - Filter(Eql(schema.ref("Country") / "code", Const("GBR")), Select("name")))), + Filter( + Eql(schema.ref("Country") / "code", Const("GBR")), + Select("name", None, Empty, Some((4, 13))))), + Some((3, 11))), simpleRestrictedQuerySql, List(encodeArg("GBR")), 1, @@ -148,7 +152,11 @@ trait SqlWorldCompilerSuite extends CatsEffectSuite { SqlStatsMonitor.SqlStats( Select( "cities", - Filter(Like(schema.ref("City") / "name", "Linh%", true), Select("name"))), + None, + Filter( + Like(schema.ref("City") / "name", "Linh%", true), + Select("name", None, Empty, Some((4, 13)))), + Some((3, 11))), simpleFilteredQuerySql, List(encodeArg(filterArg)), 3,