From 1b1764325af454fb35957951c9de4adfdaa52f1b Mon Sep 17 00:00:00 2001 From: Hugo van Rijswijk Date: Wed, 19 Aug 2026 16:35:11 +0200 Subject: [PATCH] Fix list coercion, variable usage validation and argument defaults A non-list value is a valid input for a list type. Only literal lists were accepted, so `arg: [String]` rejected `"foo"` where the specification accepts it as `["foo"]`. A single value is now wrapped, recursively for nested lists, and `null` is left unwrapped. Default values pass through the same coercion, so a supplied value and an equal default no longer produce different results. A variable with no supplied value must fall back to the default of the location it fills. The default was applied to an absent argument only, not to an absent variable. A variable was also never compared with the type of the location where it is used. Value coercion rejected some incompatible usages as a side effect, and single value coercion removes that side effect. [Rule 5.8.5](https://spec.graphql.org/September2025/#sec-All-Variable-Usages-Are-Allowed) is now implemented directly. It covers field arguments, directive arguments, list entries and input object fields, in the operation and in every fragment it can reach. The rule also depends on the default value fix above, so the two cases which allow a nullable variable at a non-null location are now observable end to end. Five tests in `SkipIncludeSuite` and six in `VariablesSuite` declared a nullable variable at a non-null location. Those documents are invalid under rule 5.8.5, so the declarations are now non-null. --- modules/core/src/main/scala/compiler.scala | 343 ++++++++++++++---- modules/core/src/main/scala/schema.scala | 27 +- .../scala/compiler/InputValuesSuite.scala | 262 +++++++++++++ .../scala/compiler/SkipIncludeSuite.scala | 10 +- .../scala/compiler/VariableUsageSuite.scala | 331 +++++++++++++++++ .../test/scala/compiler/VariablesSuite.scala | 14 +- 6 files changed, 900 insertions(+), 87 deletions(-) create mode 100644 modules/core/src/test/scala/compiler/VariableUsageSuite.scala diff --git a/modules/core/src/main/scala/compiler.scala b/modules/core/src/main/scala/compiler.scala index fbb0f05c..73f8799e 100644 --- a/modules/core/src/main/scala/compiler.scala +++ b/modules/core/src/main/scala/compiler.scala @@ -206,6 +206,223 @@ object QueryParser { } } +/** + * Validation of variable usages against the locations where they appear. + * + * @see + * https://spec.graphql.org/September2025/#sec-All-Variable-Usages-Are-Allowed + */ +object VariableUsage { + + /** + * Is the use of `varDef` allowed at a location of type `locationType`? + * + * A nullable variable is allowed at a non-null location if the variable has a non-null + * default value, or if the location itself has a default value. In that case the variable is + * compared against the nullable form of the location type. + * + * @see + * https://spec.graphql.org/September2025/#IsVariableUsageAllowed() + */ + def isVariableUsageAllowed( + varDef: InputValue, + locationType: Type, + locationHasDefault: Boolean): Boolean = { + val variableType = varDef.tpe + if (locationType.isNullable || !variableType.isNullable) + variableType <:< locationType + else { + val hasNonNullVariableDefaultValue = + varDef.defaultValue.exists(v => v != NullValue && v != AbsentValue) + (hasNonNullVariableDefaultValue || locationHasDefault) && + variableType <:< locationType.nullable + } + } + + /** + * Validate every variable usage of an operation and of the fragments it can reach. + * + * Each fragment is validated once against its own type condition, so a fragment which is + * spread more than once yields at most one problem per usage. + */ + def validateVariableUsages( + schema: Schema, + rootTpe: Type, + op: UntypedOperation, + frags: List[UntypedFragment], + varDefs: VarDefs): Result[Unit] = { + + // A problem is reported for a variable usage only, so an operation without variable + // definitions has nothing to check. + if (varDefs.isEmpty) Result.unit + else { + val varDefsByName = varDefs.map(varDef => (varDef.name, varDef)).toMap + + /* + * Check the variable usages of `value` against a location of type `locationType`. + * + * A variable can appear directly, as an entry of a list value, or as a field of an input + * object value. The item type of a list carries no default value. + */ + def checkValueUsages( + value: Value, + locationType: Type, + locationHasDefault: Boolean, + what: String, + where: String): List[Problem] = + value match { + case VariableRef(varName) => + varDefsByName.get(varName) match { + // An undefined variable is reported by `Value.elaborateValue`. + case None => Nil + case Some(varDef) => + if (isVariableUsageAllowed(varDef, locationType, locationHasDefault)) Nil + else + List(Problem( + s"Variable '$$$varName' of type '${SchemaRenderer.renderType(varDef.tpe)}' is not compatible with $what of type '${SchemaRenderer.renderType(locationType)}' in $where")) + } + case ListValue(elems) => + locationType.item match { + case None => Nil + case Some(itemType) => + val itemWhat = s"an item of $what" + elems.flatMap(checkValueUsages(_, itemType, false, itemWhat, where)) + } + case ObjectValue(fields) => + locationType.underlyingNamed.dealias match { + case io: InputObjectType => + fields.flatMap { + case (nme, fieldValue) => + checkLocation( + io.inputFieldInfo(nme), + fieldValue, + s"input field '$nme'", + where) + } + case _ => Nil + } + case _ => Nil + } + + /* + * Check the variable usages of `value` against the definition of the location it fills. + * + * An unknown argument or input field is reported by `Value.checkValue`. + */ + def checkLocation( + iv: Option[InputValue], + value: Value, + what: String, + where: String): List[Problem] = + iv.toList + .flatMap(iv0 => + checkValueUsages(value, iv0.tpe, iv0.defaultValue.isDefined, what, where)) + + /* + * Check the variable usages of a set of arguments against their definitions. + */ + def checkArgs( + args: List[Binding], + infos: List[InputValue], + where: String): List[Problem] = + args.flatMap { + case Binding(nme, value) => + checkLocation(infos.find(_.name == nme), value, s"argument '$nme'", where) + } + + /* + * Check the variable usages of the arguments of a set of directives. + */ + def checkDirectives(dirs: List[Directive]): List[Problem] = + dirs.flatMap { dir => + // An undefined directive is reported by `Directive.validateDirectivesForQuery`. + schema + .directives + .find(_.name == dir.name) + .toList + .flatMap(defn => checkArgs(dir.args, defn.args, s"directive '${dir.name}'")) + } + + /* + * The definition of field `nme` of type `tpe`. + * + * The introspection meta-fields are not defined in the target schema, so they are looked + * up in the introspection schema. They are available at the root of a query only. + */ + def fieldInfo(tpe: NamedType, nme: String): Option[Field] = + tpe.fieldInfo(nme).orElse { + if (tpe =:= schema.queryType) Introspection.schema.queryType.fieldInfo(nme) + else None + } + + def loop(query: Query, tpe: Type): List[Problem] = + query match { + case UntypedSelect(nme, _, args, dirs, child) => + val dirProblems = checkDirectives(dirs) + val named = tpe.underlyingNamed + // An unknown field is reported by `SelectElaborator`. + fieldInfo(named.dealias, nme) match { + case None => dirProblems + case Some(field) => + val where = s"field '$nme' of type '${named.name}'" + dirProblems ++ checkArgs(args, field.args, where) ++ loop(child, field.tpe) + } + case UntypedFragmentSpread(_, dirs) => + checkDirectives(dirs) + case UntypedInlineFragment(tpnme, dirs, child) => + val childTpe = tpnme.flatMap(schema.definition).getOrElse(tpe) + checkDirectives(dirs) ++ loop(child, childTpe) + case Group(children) => children.flatMap(loop(_, tpe)) + // Other query algebra nodes appear only after elaboration. + case _ => Nil + } + + /* + * The fragments which `op` can reach, directly or through another fragment. + */ + val reachableFrags: List[UntypedFragment] = + if (frags.isEmpty) Nil + else { + val fragsByName = frags.map(frag => (frag.name, frag)).toMap + + @tailrec + def closeSpreads( + pending: List[String], + seen: Set[String], + acc: List[UntypedFragment]): List[UntypedFragment] = + pending match { + case Nil => acc.reverse + case hd :: tl if seen.contains(hd) => closeSpreads(tl, seen, acc) + case hd :: tl => + fragsByName.get(hd) match { + // An undefined fragment is reported by `validateVariablesAndFragments`. + case None => closeSpreads(tl, seen + hd, acc) + case Some(frag) => + closeSpreads( + QueryCompiler.fragmentSpreads(frag.child).toList ::: tl, + seen + hd, + frag :: acc) + } + } + + closeSpreads(QueryCompiler.fragmentSpreads(op.query).toList, Set.empty, Nil) + } + + val varDefnProblems = op.variables.flatMap(varDef => checkDirectives(varDef.directives)) + val opProblems = checkDirectives(op.directives) ++ loop(op.query, rootTpe) + val fragProblems = + reachableFrags.flatMap { frag => + schema + .definition(frag.tpnme) + .toList + .flatMap(fragTpe => checkDirectives(frag.directives) ++ loop(frag.child, fragTpe)) + } + + Result.fromProblems(varDefnProblems ++ opProblems ++ fragProblems) + } + } +} + /** * GraphQL query compiler. * @@ -278,6 +495,7 @@ class QueryCompiler(parser: QueryParser, schema: Schema, phases: List[Phase]) { vars <- compileVars(varDefs, untypedVars) _ <- Directive.validateDirectivesForQuery(schema, op, frags, vars) rootTpe <- op.rootTpe(schema) + _ <- VariableUsage.validateVariableUsages(schema, rootTpe, op, frags, varDefs) res <- ( for { query <- allPhases.foldLeftM(op.query) { (acc, phase) => @@ -373,75 +591,6 @@ class QueryCompiler(parser: QueryParser, schema: Schema, phases: List[Phase]) { if (duplicateFrags.nonEmpty) duplicateFrags.toList.map(nme => Problem(s"Fragment '$nme' is defined more than once")) else { - def collectQueryRefs(query: Query): (Set[String], Set[String]) = { - @tailrec - def loop( - queries: Iterator[Query], - vars: Set[String], - frags: Set[String]): (Set[String], Set[String]) = - if (!queries.hasNext) (vars, frags) - else - queries.next() match { - case UntypedSelect(_, _, args, dirs, child) => - val v0 = args.iterator.flatMap(arg => collectValueRefs(arg.value)).toSet - val v1 = dirs - .iterator - .flatMap(dir => dir.args.iterator.flatMap(arg => collectValueRefs(arg.value))) - .toSet - loop(Iterator.single(child) ++ queries, vars ++ v0 ++ v1, frags) - case UntypedFragmentSpread(nme, dirs) => - val v0 = dirs - .iterator - .flatMap(dir => dir.args.iterator.flatMap(arg => collectValueRefs(arg.value))) - .toSet - loop(queries, vars ++ v0, frags + nme) - case UntypedInlineFragment(_, dirs, child) => - val v0 = dirs - .iterator - .flatMap(dir => dir.args.iterator.flatMap(arg => collectValueRefs(arg.value))) - .toSet - loop(Iterator.single(child) ++ queries, vars ++ v0, frags) - case Group(children) => - loop(children.iterator ++ queries, vars, frags) - case Select(_, _, child) => loop(Iterator.single(child) ++ queries, vars, frags) - case Narrow(_, child) => loop(Iterator.single(child) ++ queries, vars, frags) - case Unique(child) => loop(Iterator.single(child) ++ queries, vars, frags) - case Filter(_, child) => loop(Iterator.single(child) ++ queries, vars, frags) - case Limit(_, child) => loop(Iterator.single(child) ++ queries, vars, frags) - case Offset(_, child) => loop(Iterator.single(child) ++ queries, vars, frags) - case OrderBy(_, child) => loop(Iterator.single(child) ++ queries, vars, frags) - case Introspect(_, child) => loop(Iterator.single(child) ++ queries, vars, frags) - case Environment(_, child) => loop(Iterator.single(child) ++ queries, vars, frags) - case Component(_, _, child) => - loop(Iterator.single(child) ++ queries, vars, frags) - case Effect(_, child) => loop(Iterator.single(child) ++ queries, vars, frags) - case TransformCursor(_, child) => - loop(Iterator.single(child) ++ queries, vars, frags) - case Count(_) => loop(queries, vars, frags) - case Empty => loop(queries, vars, frags) - } - - loop(Iterator.single(query), Set.empty[String], Set.empty[String]) - } - - def collectValueRefs(value: Value): Set[String] = { - @tailrec - def loop(values: Iterator[Value], vars: Set[String]): Set[String] = - if (!values.hasNext) vars - else - values.next() match { - case VariableRef(nme) => - loop(values, vars + nme) - case ObjectValue(fields) => - loop(fields.iterator.map(_._2) ++ values, vars) - case ListValue(elems) => - loop(elems.iterator ++ values, vars) - case _ => loop(values, vars) - } - - loop(Iterator.single(value), Set.empty[String]) - } - val fragRefs: Map[String, (Set[String], Set[String])] = frags.map { frag => (frag.name, collectQueryRefs(frag.child)) }.toMap @@ -718,6 +867,62 @@ class QueryCompiler(parser: QueryParser, schema: Schema, phases: List[Phase]) { } object QueryCompiler { + + /** + * The names of the variables and of the fragments which `query` refers to directly. + */ + private[grackle] def collectQueryRefs(query: Query): (Set[String], Set[String]) = { + val noRefs = (Set.empty[String], Set.empty[String]) + + def argRefs(args: List[Binding]): Set[String] = + args.foldMap(arg => collectValueRefs(arg.value)) + + def dirRefs(dirs: List[Directive]): Set[String] = + dirs.foldMap(dir => argRefs(dir.args)) + + def loop(q: Query): (Set[String], Set[String]) = q match { + 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 Narrow(_, child) => loop(child) + case Unique(child) => loop(child) + case Filter(_, child) => loop(child) + case Limit(_, child) => loop(child) + case Offset(_, child) => loop(child) + case OrderBy(_, child) => loop(child) + case Introspect(_, child) => loop(child) + case Environment(_, child) => loop(child) + case Component(_, _, child) => loop(child) + case Effect(_, child) => loop(child) + case TransformCursor(_, child) => loop(child) + case Count(_) => noRefs + case Empty => noRefs + } + + loop(query) + } + + /** + * The names of the variables which `value` refers to. + */ + private[grackle] def collectValueRefs(value: Value): Set[String] = + value match { + case VariableRef(nme) => Set(nme) + case ObjectValue(fields) => fields.foldMap { case (_, v) => collectValueRefs(v) } + case ListValue(elems) => elems.foldMap(collectValueRefs) + case _ => Set.empty + } + + /** + * The names of the fragments which `query` spreads directly. + */ + private[grackle] def fragmentSpreads(query: Query): Set[String] = collectQueryRefs(query)._2 + sealed trait IntrospectionLevel object IntrospectionLevel { case object Full extends IntrospectionLevel diff --git a/modules/core/src/main/scala/schema.scala b/modules/core/src/main/scala/schema.scala index 033269ac..674ee137 100644 --- a/modules/core/src/main/scala/schema.scala +++ b/modules/core/src/main/scala/schema.scala @@ -357,6 +357,9 @@ sealed trait Type extends Product { /** * `true` if this type is a subtype of `other`. + * + * @see + * https://spec.graphql.org/September2025/#AreTypesCompatible() */ def <:<(other: Type): Boolean = (this.dealias, other.dealias) match { @@ -1167,11 +1170,14 @@ object Value { */ def checkValue(iv: InputValue, value: Option[Value], location: String): Result[Value] = (iv.tpe.dealias, value) match { - case (_, None) if iv.defaultValue.isDefined => - iv.defaultValue.get.success - case (_: NullableType, None) => - AbsentValue.success - case (_: NullableType, Some(AbsentValue)) => + // A default value is coerced in the same way as a supplied value. The default is + // cleared first, so that an absent default cannot fall back to itself. + // + // An absent variable yields `AbsentValue`, which counts as no value. The default value + // of the location applies to it, as it applies to an argument that is not present. + case (_, None | Some(AbsentValue)) if iv.defaultValue.isDefined => + checkValue(iv.copy(defaultValue = None), iv.defaultValue, location) + case (_: NullableType, None | Some(AbsentValue)) => AbsentValue.success case (_: NullableType, Some(NullValue)) => NullValue.success @@ -1212,6 +1218,10 @@ object Value { checkValue(iv.copy(tpe = tpe, defaultValue = None), Some(elem), location) } .map(ListValue.apply) + // A single value coerces to a list of size one. `null` is not wrapped. + case (ListType(tpe), Some(value)) if value != NullValue && value != AbsentValue => + checkValue(iv.copy(tpe = tpe, defaultValue = None), Some(value), location).map(v => + ListValue(List(v))) case (i @ InputObjectType(nme, _, ivs, _), Some(ObjectValue(fs))) => val obj = fs.toMap val unknownFields = fs.map(_._1).filterNot(f => ivs.exists(_.name == f)) @@ -1256,8 +1266,9 @@ object Value { import JsonExtractor._ (iv.tpe.dealias, value) match { + // A default value is a query algebra value, not JSON, so it is coerced by `checkValue`. case (_, None) if iv.defaultValue.isDefined => - iv.defaultValue.get.success + checkValue(iv.copy(defaultValue = None), iv.defaultValue, location) case (_: NullableType, None) => AbsentValue.success case (_: NullableType, Some(jsonNull(_))) => @@ -1295,6 +1306,10 @@ object Value { checkVarValue(iv.copy(tpe = tpe, defaultValue = None), Some(elem), location) } .map(vs => ListValue(vs.toList)) + // A single value coerces to a list of size one. `null` is not wrapped. + case (ListType(tpe), Some(value)) if !value.isNull => + checkVarValue(iv.copy(tpe = tpe, defaultValue = None), Some(value), location).map(v => + ListValue(List(v))) case (InputObjectType(nme, _, ivs, _), Some(jsonObject(obj))) => val unknownFields = obj.keys.filterNot(f => ivs.exists(_.name == f)) if (unknownFields.nonEmpty) diff --git a/modules/core/src/test/scala/compiler/InputValuesSuite.scala b/modules/core/src/test/scala/compiler/InputValuesSuite.scala index b0f27f39..5c5c5815 100644 --- a/modules/core/src/test/scala/compiler/InputValuesSuite.scala +++ b/modules/core/src/test/scala/compiler/InputValuesSuite.scala @@ -16,6 +16,7 @@ package compiler import cats.data.NonEmptyChain +import io.circe.literal._ import munit.CatsEffectSuite import grackle._ @@ -103,6 +104,193 @@ final class InputValuesSuite extends CatsEffectSuite { assertEquals(compiled.map(_.query), Result.Success(expected)) } + test("single value coerces to a list of size one") { + val query = """ + query { + listField(arg: "foo") { + subfield + } + } + """ + + val expected = + UntypedSelect( + "listField", + None, + List(Binding("arg", ListValue(List(StringValue("foo"))))), + Nil, + UntypedSelect("subfield", None, Nil, Nil, Empty) + ) + + val compiled = InputValuesMapping.compiler.compile(query, None) + assertEquals(compiled.map(_.query), Result.Success(expected)) + } + + test("single value coerces to a nested list") { + val query = """ + query { + nestedListField(arg: 1) { + subfield + } + } + """ + + val expected = + UntypedSelect( + "nestedListField", + None, + List(Binding("arg", ListValue(List(ListValue(List(IntValue(1))))))), + Nil, + UntypedSelect("subfield", None, Nil, Nil, Empty) + ) + + val compiled = InputValuesMapping.compiler.compile(query, None) + assertEquals(compiled.map(_.query), Result.Success(expected)) + } + + test("single input object coerces to a list of size one") { + val query = """ + query { + objectListField(arg: { foo: 23, bar: true, baz: "quux" }) { + subfield + } + } + """ + + val expected = + UntypedSelect( + "objectListField", + None, + List( + Binding( + "arg", + ListValue( + List( + ObjectValue( + List( + ("foo", IntValue(23)), + ("bar", BooleanValue(true)), + ("baz", StringValue("quux")), + ("defaulted", StringValue("quux")), + ("nullable", AbsentValue) + )))) + )), + Nil, + UntypedSelect("subfield", None, Nil, Nil, Empty) + ) + + val compiled = InputValuesMapping.compiler.compile(query, None) + assertEquals(compiled.map(_.query), Result.Success(expected)) + } + + test("null is not wrapped in a list") { + val query = """ + query { + nullableListField(arg: null) { + subfield + } + } + """ + + val expected = + UntypedSelect( + "nullableListField", + None, + List(Binding("arg", NullValue)), + Nil, + UntypedSelect("subfield", None, Nil, Nil, Empty) + ) + + val compiled = InputValuesMapping.compiler.compile(query, None) + assertEquals(compiled.map(_.query), Result.Success(expected)) + } + + test("single value of the wrong type is still rejected for a list") { + val query = """ + query { + listField(arg: 23) { + subfield + } + } + """ + + val expected = + Problem("Expected String found '23' for 'arg' in field 'listField' of type 'Query'") + + val compiled = InputValuesMapping.compiler.compile(query, None) + assertEquals(compiled.map(_.query), Result.Failure(NonEmptyChain.one(expected))) + } + + test("single variable value coerces to a list of size one") { + val query = """ + query ($arg: [String]!) { + nullableListField(arg: $arg) { + subfield + } + } + """ + + val variables = json"""{ "arg": "foo" }""" + + val expected = + UntypedSelect( + "nullableListField", + None, + List(Binding("arg", ListValue(List(StringValue("foo"))))), + Nil, + UntypedSelect("subfield", None, Nil, Nil, Empty) + ) + + val compiled = InputValuesMapping.compiler.compile(query, untypedVars = Some(variables)) + assertEquals(compiled.map(_.query), Result.Success(expected)) + } + + test("an argument default applies to an absent variable") { + val query = """ + query ($arg: [String]) { + defaultedListField(arg: $arg) { + subfield + } + } + """ + + val expected = + UntypedSelect( + "defaultedListField", + None, + List(Binding("arg", ListValue(List(StringValue("foo"))))), + Nil, + UntypedSelect("subfield", None, Nil, Nil, Empty) + ) + + val compiled = InputValuesMapping.compiler.compile(query, untypedVars = Some(json"""{}""")) + assertEquals(compiled.map(_.query), Result.Success(expected)) + } + + test("null variable value is not wrapped in a list") { + val query = """ + query ($arg: [String]) { + nullableListField(arg: $arg) { + subfield + } + } + """ + + val variables = json"""{ "arg": null }""" + + val expected = + UntypedSelect( + "nullableListField", + None, + List(Binding("arg", NullValue)), + Nil, + UntypedSelect("subfield", None, Nil, Nil, Empty) + ) + + val compiled = InputValuesMapping.compiler.compile(query, untypedVars = Some(variables)) + assertEquals(compiled.map(_.query), Result.Success(expected)) + } + test("input object value") { val query = """ query { @@ -248,6 +436,72 @@ final class InputValuesSuite extends CatsEffectSuite { val compiled = OneOfInputValuesMapping.compiler.compile(query, None) assertEquals(compiled.map(_.query), Result.Failure(NonEmptyChain.one(expected))) } + + test("single value default coerces to a list of size one") { + val query = """ + query { + defaultedListField { + subfield + } + } + """ + + val expected = + UntypedSelect( + "defaultedListField", + None, + List(Binding("arg", ListValue(List(StringValue("foo"))))), + Nil, + UntypedSelect("subfield", None, Nil, Nil, Empty) + ) + + val compiled = InputValuesMapping.compiler.compile(query, None) + assertEquals(compiled.map(_.query), Result.Success(expected)) + } + + test("a supplied value and a default of the same shape agree") { + val supplied = """ + query { + defaultedListField(arg: "foo") { + subfield + } + } + """ + + val defaulted = """ + query { + defaultedListField { + subfield + } + } + """ + + assertEquals( + InputValuesMapping.compiler.compile(supplied, None).map(_.query), + InputValuesMapping.compiler.compile(defaulted, None).map(_.query)) + } + + test("single value default of an input object field coerces to a list") { + val query = """ + query { + defaultedObjectField(arg: {}) { + subfield + } + } + """ + + val expected = + UntypedSelect( + "defaultedObjectField", + None, + List(Binding("arg", ObjectValue(List(("xs", ListValue(List(IntValue(1)))))))), + Nil, + UntypedSelect("subfield", None, Nil, Nil, Empty) + ) + + val compiled = InputValuesMapping.compiler.compile(query, None) + assertEquals(compiled.map(_.query), Result.Success(expected)) + } } object InputValuesMapping extends TestMapping { @@ -256,11 +510,19 @@ object InputValuesMapping extends TestMapping { type Query { field(arg: Int): Result! listField(arg: [String!]!): Result! + nullableListField(arg: [String]): Result! + defaultedListField(arg: [String] = "foo"): Result! + nestedListField(arg: [[Int]]): Result! + objectListField(arg: [InObj!]!): Result! objectField(arg: InObj!): Result! + defaultedObjectField(arg: DefObj!): Result! } type Result { subfield: String! } + input DefObj { + xs: [Int] = 1 + } input InObj { foo: Int! bar: Boolean! diff --git a/modules/core/src/test/scala/compiler/SkipIncludeSuite.scala b/modules/core/src/test/scala/compiler/SkipIncludeSuite.scala index 9fab3189..3ad700f5 100644 --- a/modules/core/src/test/scala/compiler/SkipIncludeSuite.scala +++ b/modules/core/src/test/scala/compiler/SkipIncludeSuite.scala @@ -25,7 +25,7 @@ import grackle.syntax._ final class SkipIncludeSuite extends CatsEffectSuite { test("skip/include field") { val query = """ - query ($yup: Boolean, $nope: Boolean) { + query ($yup: Boolean!, $nope: Boolean!) { a: field @skip(if: $yup) { subfieldA } @@ -62,7 +62,7 @@ final class SkipIncludeSuite extends CatsEffectSuite { test("skip/include fragment spread") { val query = """ - query ($yup: Boolean, $nope: Boolean) { + query ($yup: Boolean!, $nope: Boolean!) { a: field { ...frag @skip(if: $yup) } @@ -122,7 +122,7 @@ final class SkipIncludeSuite extends CatsEffectSuite { test("fragment spread with nested skip/include") { val query = """ - query ($yup: Boolean, $nope: Boolean) { + query ($yup: Boolean!, $nope: Boolean!) { field { ...frag } @@ -160,7 +160,7 @@ final class SkipIncludeSuite extends CatsEffectSuite { test("skip/include inline fragment") { val query = """ - query ($yup: Boolean, $nope: Boolean) { + query ($yup: Boolean!, $nope: Boolean!) { a: field { ... on Value @skip(if: $yup) { subfieldA @@ -227,7 +227,7 @@ final class SkipIncludeSuite extends CatsEffectSuite { test("inline fragment with nested skip/include") { val query = """ - query ($yup: Boolean, $nope: Boolean) { + query ($yup: Boolean!, $nope: Boolean!) { field { ... on Value { a: subfieldA @skip(if: $yup) diff --git a/modules/core/src/test/scala/compiler/VariableUsageSuite.scala b/modules/core/src/test/scala/compiler/VariableUsageSuite.scala new file mode 100644 index 00000000..013022b7 --- /dev/null +++ b/modules/core/src/test/scala/compiler/VariableUsageSuite.scala @@ -0,0 +1,331 @@ +// 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 compiler + +import io.circe.Json +import io.circe.literal._ +import munit.{CatsEffectSuite, Location} + +import grackle._ +import grackle.ScalarType._ +import grackle.Value._ +import grackle.syntax._ + +/** + * Tests for rule 5.8.5, All Variable Usages Are Allowed. + * + * @see + * https://spec.graphql.org/September2025/#sec-All-Variable-Usages-Are-Allowed + */ +final class VariableUsageSuite extends CatsEffectSuite { + + test("a variable of the same type is allowed") { + assertAllowed( + """query ($v: String!) { scalarField(arg: $v) { subfield } }""", + json"""{"v": "a"}""") + } + + test("a non-null variable is allowed at a nullable location") { + assertAllowed( + """query ($v: String!) { nullableScalarField(arg: $v) { subfield } }""", + json"""{"v": "a"}""") + } + + test("a nullable variable is rejected at a non-null location") { + assertRejected( + """query ($v: String) { scalarField(arg: $v) { subfield } }""", + "Variable '$v' of type 'String' is not compatible with argument 'arg' of type 'String!' in field 'scalarField' of type 'Query'" + ) + } + + test("a nullable variable with a non-null default is allowed at a non-null location") { + assertAllowed("""query ($v: String = "d") { scalarField(arg: $v) { subfield } }""") + } + + test("a nullable variable with a null default is rejected at a non-null location") { + assertRejected( + """query ($v: String = null) { scalarField(arg: $v) { subfield } }""", + "Variable '$v' of type 'String' is not compatible with argument 'arg' of type 'String!' in field 'scalarField' of type 'Query'" + ) + } + + test("a variable of a different named type is rejected") { + assertRejected( + """query ($v: Int!) { scalarField(arg: $v) { subfield } }""", + "Variable '$v' of type 'Int!' is not compatible with argument 'arg' of type 'String!' in field 'scalarField' of type 'Query'", + json"""{"v": 1}""" + ) + } + + test("a non-list variable is rejected at a list location") { + assertRejected( + """query ($v: String!) { listField(arg: $v) { subfield } }""", + "Variable '$v' of type 'String!' is not compatible with argument 'arg' of type '[String!]!' in field 'listField' of type 'Query'", + json"""{"v": "a"}""" + ) + } + + test("a list variable is rejected at a non-list location") { + assertRejected( + """query ($v: [String!]!) { scalarField(arg: $v) { subfield } }""", + "Variable '$v' of type '[String!]!' is not compatible with argument 'arg' of type 'String!' in field 'scalarField' of type 'Query'", + json"""{"v": ["a"]}""" + ) + } + + test("a nullable item is rejected where a non-null item is expected") { + assertRejected( + """query ($v: [String]!) { listField(arg: $v) { subfield } }""", + "Variable '$v' of type '[String]!' is not compatible with argument 'arg' of type '[String!]!' in field 'listField' of type 'Query'", + json"""{"v": ["a"]}""" + ) + } + + test("a non-null item is allowed where a nullable item is expected") { + assertAllowed( + """query ($v: [String!]!) { nullableItemListField(arg: $v) { subfield } }""", + json"""{"v": ["a"]}""") + } + + test("nested list types are compared item by item") { + assertAllowed( + """query ($v: [[Int]]!) { nestedListField(arg: $v) { subfield } }""", + json"""{"v": [[1]]}""") + + assertRejected( + """query ($v: [Int]!) { nestedListField(arg: $v) { subfield } }""", + "Variable '$v' of type '[Int]!' is not compatible with argument 'arg' of type '[[Int]]!' in field 'nestedListField' of type 'Query'", + json"""{"v": [1]}""" + ) + } + + test("a variable inside a list value is checked against the item type") { + assertAllowed( + """query ($v: String!) { listField(arg: [$v]) { subfield } }""", + json"""{"v": "a"}""") + + assertRejected( + """query ($v: String) { listField(arg: [$v]) { subfield } }""", + "Variable '$v' of type 'String' is not compatible with an item of argument 'arg' of type 'String!' in field 'listField' of type 'Query'" + ) + } + + test("a variable inside an input object is checked against the field type") { + assertAllowed( + """query ($v: String!) { objectField(arg: { required: $v }) { subfield } }""", + json"""{"v": "a"}""") + + assertRejected( + """query ($v: String) { objectField(arg: { required: $v }) { subfield } }""", + "Variable '$v' of type 'String' is not compatible with input field 'required' of type 'String!' in field 'objectField' of type 'Query'" + ) + } + + test("a variable in a directive argument is checked") { + assertAllowed( + """query ($v: Boolean!) { scalarField(arg: "a") @skip(if: $v) { subfield } }""", + json"""{"v": true}""") + + // A value is supplied so that argument coercion succeeds and the usage rule is reached. + assertRejected( + """query ($v: Boolean) { scalarField(arg: "a") @skip(if: $v) { subfield } }""", + "Variable '$v' of type 'Boolean' is not compatible with argument 'if' of type 'Boolean!' in directive 'skip'", + json"""{"v": true}""" + ) + } + + test("a variable usage inside a fragment is checked") { + assertRejected( + """ + query ($v: String) { ...frag } + fragment frag on Query { scalarField(arg: $v) { subfield } } + """, + "Variable '$v' of type 'String' is not compatible with argument 'arg' of type 'String!' in field 'scalarField' of type 'Query'" + ) + } + + test("a variable usage inside an inline fragment is checked") { + assertRejected( + """query ($v: String) { ... on Query { scalarField(arg: $v) { subfield } } }""", + "Variable '$v' of type 'String' is not compatible with argument 'arg' of type 'String!' in field 'scalarField' of type 'Query'" + ) + } + + test("a fragment is validated only against the operations which can reach it") { + val compiled = + VariableUsageMapping + .compiler + .compile( + """ + query A($v: String) { ...frag } + query B($v: [String]) { nullableListField(arg: $v) { subfield } } + fragment frag on Query { nullableScalarField(arg: $v) { subfield } } + """, + name = Some("A"), + untypedVars = Some(json"""{}""") + ) + + assert(compiled.hasValue, compiled.toString) + } + + test("a variable in an introspection meta-field argument is checked") { + val allowed = + IntrospectionUsageMapping + .compiler + .compile( + """query ($v: String!) { __type(name: $v) { name } }""", + untypedVars = Some(json"""{"v": "Query"}""")) + + assert(allowed.hasValue, allowed.toString) + + assertRejected( + """query ($v: String) { __type(name: $v) { name } }""", + "Variable '$v' of type 'String' is not compatible with argument 'name' of type 'String!' in field '__type' of type 'Query'" + ) + } + + test("a variable below an introspection meta-field is checked") { + val allowed = + IntrospectionUsageMapping + .compiler + .compile( + """query ($v: Boolean!) { __schema { types { fields(includeDeprecated: $v) { name } } } }""", + untypedVars = Some(json"""{"v": true}""")) + + assert(allowed.hasValue, allowed.toString) + + assertRejected( + """query ($v: [Boolean!]!) { __schema { types { fields(includeDeprecated: $v) { name } } } }""", + "Variable '$v' of type '[Boolean!]!' is not compatible with argument 'includeDeprecated' of type 'Boolean!' in field 'fields' of type '__Type'", + json"""{"v": [true]}""" + ) + } + + test("a fragment spread twice yields one problem per usage") { + val compiled = + compile( + """ + query ($v: String) { a: scalarField(arg: "a") { ...frag } b: scalarField(arg: "b") { ...frag } } + fragment frag on Result { subfieldWithArg(arg: $v) } + """, + json"""{}""" + ) + + assertEquals(compiled.toProblems.size, 1L) + } + + test("an input type is a subtype of the same type, and of its nullable form only") { + assert(StringType <:< StringType) + assert(StringType <:< NullableType(StringType)) + assert(!(NullableType(StringType) <:< StringType)) + assert(!(IntType <:< StringType)) + } + + test("list types are subtypes item by item") { + assert(ListType(StringType) <:< ListType(NullableType(StringType))) + assert(!(ListType(NullableType(StringType)) <:< ListType(StringType))) + assert(!(StringType <:< ListType(StringType))) + assert(!(ListType(StringType) <:< StringType)) + assert(ListType(ListType(StringType)) <:< ListType(ListType(StringType))) + assert(!(ListType(StringType) <:< ListType(ListType(StringType)))) + } + + test("a nullable variable is allowed where the argument has a default") { + assertAllowed("""query ($v: String) { defaultedField(arg: $v) { subfield } }""") + } + + test("a nullable variable is allowed where the input field has a default") { + assertAllowed( + """query ($v: String) { objectField(arg: { required: "a", defaulted: $v }) { subfield } }""") + } + + test("a nullable variable needs a default at a non-null location") { + val nullable = NullableType(StringType) + + assert(!VariableUsage.isVariableUsageAllowed(varDef(nullable, None), StringType, false)) + assert(VariableUsage.isVariableUsageAllowed(varDef(nullable, None), StringType, true)) + assert( + VariableUsage + .isVariableUsageAllowed(varDef(nullable, Some(StringValue("x"))), StringType, false)) + assert( + !VariableUsage + .isVariableUsageAllowed(varDef(nullable, Some(NullValue)), StringType, false)) + } + + test("a default does not make incompatible types compatible") { + val nullable = NullableType(IntType) + + assert(!VariableUsage.isVariableUsageAllowed(varDef(nullable, None), StringType, true)) + } + + def varDef(tpe: Type, default: Option[Value]): InputValue = + InputValue("v", None, tpe, default, Nil) + + def compile(query: String, vars: Json = json"""{}"""): Result[Query] = + VariableUsageMapping.compiler.compile(query, untypedVars = Some(vars)).map(_.query) + + def assertAllowed( + query: String, + vars: Json = json"""{}""" + )(implicit loc: Location): Unit = { + val compiled = compile(query, vars) + assert(compiled.hasValue, compiled.toString) + } + + def assertRejected( + query: String, + message: String, + vars: Json = json"""{}""" + )(implicit loc: Location): Unit = + assertEquals(compile(query, vars), Result.failure(message)) +} + +object VariableUsageMapping extends TestMapping { + val schema = + schema""" + type Query { + scalarField(arg: String!): Result! + nullableScalarField(arg: String): Result! + defaultedField(arg: String! = "x"): Result! + listField(arg: [String!]!): Result! + nullableListField(arg: [String]): Result! + nullableItemListField(arg: [String]!): Result! + nestedListField(arg: [[Int]]!): Result! + objectField(arg: InObj!): Result! + } + type Result { + subfield: String! + subfieldWithArg(arg: String!): String! + } + input InObj { + required: String! + optional: String + defaulted: String! = "x" + } + """ + + override val selectElaborator = PreserveArgsElaborator +} + +/** + * The same schema with the default elaborator. + * + * `PreserveArgsElaborator` cannot elaborate an introspection selection, so the introspection + * tests use this mapping for the queries which must compile. + */ +object IntrospectionUsageMapping extends TestMapping { + val schema = VariableUsageMapping.schema +} diff --git a/modules/core/src/test/scala/compiler/VariablesSuite.scala b/modules/core/src/test/scala/compiler/VariablesSuite.scala index fa965c36..9bd7b309 100644 --- a/modules/core/src/test/scala/compiler/VariablesSuite.scala +++ b/modules/core/src/test/scala/compiler/VariablesSuite.scala @@ -63,7 +63,7 @@ final class VariablesSuite extends CatsEffectSuite { test("list variable query") { val query = """ - query getProfile($ids: [ID!]) { + query getProfile($ids: [ID!]!) { users(ids: $ids) { name } @@ -92,7 +92,7 @@ final class VariablesSuite extends CatsEffectSuite { test("enum variable query") { val query = """ - query getUserType($userType: UserType) { + query getUserType($userType: UserType!) { usersByType(userType: $userType) { name } @@ -121,7 +121,7 @@ final class VariablesSuite extends CatsEffectSuite { test("scalar variable query") { val query = """ - query getLoggedInByDate($date: Date) { + query getLoggedInByDate($date: Date!) { usersLoggedInByDate(date: $date) { name } @@ -150,7 +150,7 @@ final class VariablesSuite extends CatsEffectSuite { test("scalar variable query bigdecimal") { val query = """ - query queryWithBigDecimal($input: BigDecimal) { + query queryWithBigDecimal($input: BigDecimal!) { queryWithBigDecimal(input: $input) { name } @@ -179,7 +179,7 @@ final class VariablesSuite extends CatsEffectSuite { test("object variable query") { val query = """ - query doSearch($pattern: Pattern) { + query doSearch($pattern: Pattern!) { search(pattern: $pattern) { name id @@ -228,7 +228,7 @@ final class VariablesSuite extends CatsEffectSuite { test("invalid: bogus input object field") { val query = """ - query doSearch($pattern: Pattern) { + query doSearch($pattern: Pattern!) { search(pattern: $pattern) { name id @@ -421,7 +421,7 @@ final class VariablesSuite extends CatsEffectSuite { test("variables in directive argument") { val query = """ - query getZuckProfile($skipName: Boolean) { + query getZuckProfile($skipName: Boolean!) { user(id: 4) { id name @skip(if: $skipName)