move variable position validation for OneOf to VariablesInAllowedPositionRule

src/validation/ValidationContext.ts

@@ -35,6 +35,7 @@ type NodeWithSelectionSet = OperationDefinitionNode | FragmentDefinitionNode;
 interface VariableUsage {
  readonly node: VariableNode;
  readonly type: Maybe<GraphQLInputType>;
+ readonly parentType: Maybe<GraphQLInputType>;
  readonly defaultValue: GraphQLDefaultValueUsage | undefined;
  readonly fragmentVariableDefinition: Maybe<VariableDefinitionNode>;
 }
@@ -226,13 +227,15 @@ export class ValidationContext extends ASTValidationContext {
  newUsages.push({
  node: variable,
  type: typeInfo.getInputType(),
+ parentType: typeInfo.getParentInputType(),
  defaultValue: undefined, // fragment variables have a variable default but no location default, which is what this default value represents
  fragmentVariableDefinition,
  });
  } else {
  newUsages.push({
  node: variable,
  type: typeInfo.getInputType(),
+ parentType: typeInfo.getParentInputType(),
  defaultValue: typeInfo.getDefaultValue(),
  fragmentVariableDefinition: undefined,
  });

src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts

@@ -1094,22 +1094,6 @@ describe('Validate: Values of correct type', () => {
  ]);
  });
 
- it('Exactly one nullable variable', () => {
- expectErrors(`
- query ($string: String) {
- complicatedArgs {
- oneOfArgField(oneOfArg: { stringField: $string })
- }
- }
- `).toDeepEqual([
- {
- message:
- 'Variable "$string" must be non-nullable to be used for OneOf Input Object "OneOfInput".',
- locations: [{ line: 4, column: 37 }],
- },
- ]);
- });
-
  it('More than one field', () => {
  expectErrors(`
  {

src/validation/__tests__/VariablesInAllowedPositionRule-test.ts

@@ -365,6 +365,37 @@ describe('Validate: Variables are in allowed positions', () => {
  });
  });
 
+ describe('Validates OneOf Input Objects', () => {
+ it('Allows exactly one non-nullable variable', () => {
+ expectValid(`
+ query ($string: String!) {
+ complicatedArgs {
+ oneOfArgField(oneOfArg: { stringField: $string })
+ }
+ }
+ `);
+ });
+
+ it('Forbids one nullable variable', () => {
+ expectErrors(`
+ query ($string: String) {
+ complicatedArgs {
+ oneOfArgField(oneOfArg: { stringField: $string })
+ }
+ }
+ `).toDeepEqual([
+ {
+ message:
+ 'Variable "$string" is of type "String" but must be non-nullable to be used for OneOf Input Object "OneOfInput".',
+ locations: [
+ { line: 2, column: 16 },
+ { line: 4, column: 52 },
+ ],
+ },
+ ]);
+ });
+ });
+
  describe('Fragment arguments are validated', () => {
  it('Boolean => Boolean', () => {
  expectValid(`

src/validation/rules/ValuesOfCorrectTypeRule.ts

@@ -8,7 +8,6 @@ import type {
  ObjectFieldNode,
  ObjectValueNode,
  ValueNode,
- VariableDefinitionNode,
 } from '../../language/ast.js';
 import { Kind } from '../../language/kinds.js';
 import { print } from '../../language/printer.js';
@@ -40,17 +39,7 @@ import type { ValidationContext } from '../ValidationContext.js';
 export function ValuesOfCorrectTypeRule(
  context: ValidationContext,
 ): ASTVisitor {
- let variableDefinitions: { [key: string]: VariableDefinitionNode } = {};
-
  return {
- OperationDefinition: {
- enter() {
- variableDefinitions = {};
- },
- },
- VariableDefinition(definition) {
- variableDefinitions[definition.variable.name.value] = definition;
- },
  ListValue(node) {
  // Note: TypeInfo will traverse into a list's item type, so look to the
  // parent input type to check if it is a list.
@@ -84,13 +73,7 @@ export function ValuesOfCorrectTypeRule(
  }
 
  if (type.isOneOf) {
- validateOneOfInputObject(
- context,
- node,
- type,
- fieldNodeMap,
- variableDefinitions,
- );
+ validateOneOfInputObject(context, node, type, fieldNodeMap);
  }
  },
  ObjectField(node) {
@@ -189,7 +172,6 @@ function validateOneOfInputObject(
  node: ObjectValueNode,
  type: GraphQLInputObjectType,
  fieldNodeMap: Map<string, ObjectFieldNode>,
- variableDefinitions: { [key: string]: VariableDefinitionNode },
 ): void {
  const keys = Array.from(fieldNodeMap.keys());
  const isNotExactlyOneField = keys.length !== 1;
@@ -206,29 +188,12 @@ function validateOneOfInputObject(
 
  const value = fieldNodeMap.get(keys[0])?.value;
  const isNullLiteral = !value || value.kind === Kind.NULL;
- const isVariable = value?.kind === Kind.VARIABLE;
 
  if (isNullLiteral) {
  context.reportError(
  new GraphQLError(`Field "${type}.${keys[0]}" must be non-null.`, {
  nodes: [node],
  }),
  );
- return;
- }
-
- if (isVariable) {
- const variableName = value.name.value;
- const definition = variableDefinitions[variableName];
- const isNullableVariable = definition.type.kind !== Kind.NON_NULL_TYPE;
-
- if (isNullableVariable) {
- context.reportError(
- new GraphQLError(
- `Variable "$${variableName}" must be non-nullable to be used for OneOf Input Object "${type}".`,
- { nodes: [node] },
- ),
- );
- }
  }
 }

src/validation/rules/VariablesInAllowedPositionRule.ts

@@ -11,7 +11,11 @@ import type {
  GraphQLDefaultValueUsage,
  GraphQLType,
 } from '../../type/definition.js';
-import { isNonNullType } from '../../type/definition.js';
+import {
+ isInputObjectType,
+ isNonNullType,
+ isNullableType,
+} from '../../type/definition.js';
 import type { GraphQLSchema } from '../../type/schema.js';
 
 import { isTypeSubTypeOf } from '../../utilities/typeComparators.js';
@@ -42,6 +46,7 @@ export function VariablesInAllowedPositionRule(
  for (const {
  node,
  type,
+ parentType,
  defaultValue,
  fragmentVariableDefinition,
  } of usages) {
@@ -78,6 +83,21 @@ export function VariablesInAllowedPositionRule(
  ),
  );
  }
+
+ if (
+ isInputObjectType(parentType) &&
+ parentType.isOneOf &&
+ isNullableType(varType)
+ ) {
+ const varTypeStr = inspect(varType);
+ const parentTypeStr = inspect(parentType);
+ context.reportError(
+ new GraphQLError(
+ `Variable "$${varName}" is of type "${varTypeStr}" but must be non-nullable to be used for OneOf Input Object "${parentTypeStr}".`,
+ { nodes: [varDef, node] },
+ ),
+ );
+ }
  }
  }
  },
@@ -90,8 +110,11 @@ export function VariablesInAllowedPositionRule(
 
 /**
  * Returns true if the variable is allowed in the location it was found,
- * which includes considering if default values exist for either the variable
+ * including considering if default values exist for either the variable
  * or the location at which it is located.
+ *
+ * OneOf Input Object Type fields are considered separately above to
+ * provide a more descriptive error message.
  */
 function allowedVariableUsage(
  schema: GraphQLSchema,