diff --git a/src/execution/__tests__/variables-test.ts b/src/execution/__tests__/variables-test.ts index 3a859a0bdc..04aaf5971b 100644 --- a/src/execution/__tests__/variables-test.ts +++ b/src/execution/__tests__/variables-test.ts @@ -60,6 +60,13 @@ const TestComplexScalar = new GraphQLScalarType({ }, }); +const NestedType: GraphQLObjectType = new GraphQLObjectType({ + name: 'NestedType', + fields: { + echo: fieldWithInputArg({ type: GraphQLString }), + }, +}); + const TestInputObject = new GraphQLInputObjectType({ name: 'TestInputObject', fields: { @@ -129,6 +136,10 @@ const TestType = new GraphQLObjectType({ type: TestNestedInputObject, defaultValue: 'Hello World', }), + nested: { + type: NestedType, + resolve: () => ({}), + }, list: fieldWithInputArg({ type: new GraphQLList(GraphQLString) }), nnList: fieldWithInputArg({ type: new GraphQLNonNull(new GraphQLList(GraphQLString)), @@ -154,6 +165,14 @@ function executeQuery( return executeSync({ schema, document, variableValues }); } +function executeQueryWithFragmentArguments( + query: string, + variableValues?: { [variable: string]: unknown }, +) { + const document = parse(query, { experimentalFragmentArguments: true }); + return executeSync({ schema, document, variableValues }); +} + describe('Execute: Handles inputs', () => { describe('Handles objects and nullability', () => { describe('using inline structs', () => { @@ -1136,4 +1155,348 @@ describe('Execute: Handles inputs', () => { }); }); }); + + describe('using fragment arguments', () => { + it('when there are no fragment arguments', () => { + const result = executeQueryWithFragmentArguments(` + query { + ...a + } + fragment a on TestType { + fieldWithNonNullableStringInput(input: "A") + } + `); + expect(result).to.deep.equal({ + data: { + fieldWithNonNullableStringInput: '"A"', + }, + }); + }); + + it('when a value is required and provided', () => { + const result = executeQueryWithFragmentArguments(` + query { + ...a(value: "A") + } + fragment a($value: String!) on TestType { + fieldWithNonNullableStringInput(input: $value) + } + `); + expect(result).to.deep.equal({ + data: { + fieldWithNonNullableStringInput: '"A"', + }, + }); + }); + + it('when a value is required and not provided', () => { + const result = executeQueryWithFragmentArguments(` + query { + ...a + } + fragment a($value: String!) on TestType { + fieldWithNullableStringInput(input: $value) + } + `); + + expect(result).to.have.property('errors'); + expect(result.errors).to.have.length(1); + expect(result.errors?.[0].message).to.match( + /Argument "value" of required type "String!"/, + ); + }); + + it('when the definition has a default and is provided', () => { + const result = executeQueryWithFragmentArguments(` + query { + ...a(value: "A") + } + fragment a($value: String! = "B") on TestType { + fieldWithNonNullableStringInput(input: $value) + } + `); + expect(result).to.deep.equal({ + data: { + fieldWithNonNullableStringInput: '"A"', + }, + }); + }); + + it('when the definition has a default and is not provided', () => { + const result = executeQueryWithFragmentArguments(` + query { + ...a + } + fragment a($value: String! = "B") on TestType { + fieldWithNonNullableStringInput(input: $value) + } + `); + expect(result).to.deep.equal({ + data: { + fieldWithNonNullableStringInput: '"B"', + }, + }); + }); + + it('when a definition has a default, is not provided, and spreads another fragment', () => { + const result = executeQueryWithFragmentArguments(` + query { + ...a + } + fragment a($a: String! = "B") on TestType { + ...b(b: $a) + } + fragment b($b: String!) on TestType { + fieldWithNonNullableStringInput(input: $b) + } + `); + expect(result).to.deep.equal({ + data: { + fieldWithNonNullableStringInput: '"B"', + }, + }); + }); + + it('when the definition has a non-nullable default and is provided null', () => { + const result = executeQueryWithFragmentArguments(` + query { + ...a(value: null) + } + fragment a($value: String! = "B") on TestType { + fieldWithNullableStringInput(input: $value) + } + `); + + expect(result).to.have.property('errors'); + expect(result.errors).to.have.length(1); + expect(result.errors?.[0].message).to.match( + /Argument "value" of non-null type "String!"/, + ); + }); + + it('when the definition has no default and is not provided', () => { + const result = executeQueryWithFragmentArguments(` + query { + ...a + } + fragment a($value: String) on TestType { + fieldWithNonNullableStringInputAndDefaultArgumentValue(input: $value) + } + `); + expect(result).to.deep.equal({ + data: { + fieldWithNonNullableStringInputAndDefaultArgumentValue: + '"Hello World"', + }, + }); + }); + + it('when an argument is shadowed by an operation variable', () => { + const result = executeQueryWithFragmentArguments(` + query($x: String! = "A") { + ...a(x: "B") + } + fragment a($x: String) on TestType { + fieldWithNullableStringInput(input: $x) + } + `); + expect(result).to.deep.equal({ + data: { + fieldWithNullableStringInput: '"B"', + }, + }); + }); + + it('when a nullable argument without a field default is not provided and shadowed by an operation variable', () => { + const result = executeQueryWithFragmentArguments(` + query($x: String = "A") { + ...a + } + fragment a($x: String) on TestType { + fieldWithNullableStringInput(input: $x) + } + `); + expect(result).to.deep.equal({ + data: { + fieldWithNullableStringInput: null, + }, + }); + }); + + it('when a nullable argument with a field default is not provided and shadowed by an operation variable', () => { + const result = executeQueryWithFragmentArguments(` + query($x: String = "A") { + ...a + } + fragment a($x: String) on TestType { + fieldWithNonNullableStringInputAndDefaultArgumentValue(input: $x) + } + `); + expect(result).to.deep.equal({ + data: { + fieldWithNonNullableStringInputAndDefaultArgumentValue: + '"Hello World"', + }, + }); + }); + + it('when a fragment-variable is shadowed by an intermediate fragment-spread but defined in the operation-variables', () => { + const result = executeQueryWithFragmentArguments(` + query($x: String = "A") { + ...a + } + fragment a($x: String) on TestType { + ...b + } + fragment b on TestType { + fieldWithNullableStringInput(input: $x) + } + `); + expect(result).to.deep.equal({ + data: { + fieldWithNullableStringInput: '"A"', + }, + }); + }); + + it('when a fragment is used with different args', () => { + const result = executeQueryWithFragmentArguments(` + query($x: String = "Hello") { + a: nested { + ...a(x: "a") + } + b: nested { + ...a(x: "b", b: true) + } + hello: nested { + ...a(x: $x) + } + } + fragment a($x: String, $b: Boolean = false) on NestedType { + a: echo(input: $x) @skip(if: $b) + b: echo(input: $x) @include(if: $b) + } + `); + expect(result).to.deep.equal({ + data: { + a: { + a: '"a"', + }, + b: { + b: '"b"', + }, + hello: { + a: '"Hello"', + }, + }, + }); + }); + + it('when the argument variable is nested in a complex type', () => { + const result = executeQueryWithFragmentArguments(` + query { + ...a(value: "C") + } + fragment a($value: String) on TestType { + list(input: ["A", "B", $value, "D"]) + } + `); + expect(result).to.deep.equal({ + data: { + list: '["A", "B", "C", "D"]', + }, + }); + }); + + it('when argument variables are used recursively', () => { + const result = executeQueryWithFragmentArguments(` + query { + ...a(aValue: "C") + } + fragment a($aValue: String) on TestType { + ...b(bValue: $aValue) + } + fragment b($bValue: String) on TestType { + list(input: ["A", "B", $bValue, "D"]) + } + `); + expect(result).to.deep.equal({ + data: { + list: '["A", "B", "C", "D"]', + }, + }); + }); + + it('when argument variables with the same name are used directly and recursively', () => { + const result = executeQueryWithFragmentArguments(` + query { + ...a(value: "A") + } + fragment a($value: String!) on TestType { + ...b(value: "B") + fieldInFragmentA: fieldWithNonNullableStringInput(input: $value) + } + fragment b($value: String!) on TestType { + fieldInFragmentB: fieldWithNonNullableStringInput(input: $value) + } + `); + expect(result).to.deep.equal({ + data: { + fieldInFragmentA: '"A"', + fieldInFragmentB: '"B"', + }, + }); + }); + + it('when argument passed in as list', () => { + const result = executeQueryWithFragmentArguments(` + query Q($opValue: String = "op") { + ...a(aValue: "A") + } + fragment a($aValue: String, $bValue: String) on TestType { + ...b(aValue: [$aValue, "B"], bValue: [$bValue, $opValue]) + } + fragment b($aValue: [String], $bValue: [String], $cValue: String) on TestType { + aList: list(input: $aValue) + bList: list(input: $bValue) + cList: list(input: [$cValue]) + } + `); + expect(result).to.deep.equal({ + data: { + aList: '["A", "B"]', + bList: '[null, "op"]', + cList: '[null]', + }, + }); + }); + + it('when argument passed to a directive', () => { + const result = executeQueryWithFragmentArguments(` + query { + ...a(value: true) + } + fragment a($value: Boolean!) on TestType { + fieldWithNonNullableStringInput @skip(if: $value) + } + `); + expect(result).to.deep.equal({ + data: {}, + }); + }); + + it('when argument passed to a directive on a nested field', () => { + const result = executeQueryWithFragmentArguments(` + query { + ...a(value: true) + } + fragment a($value: Boolean!) on TestType { + nested { echo(input: "echo") @skip(if: $value) } + } + `); + expect(result).to.deep.equal({ + data: { nested: {} }, + }); + }); + }); }); diff --git a/src/execution/collectFields.ts b/src/execution/collectFields.ts index d0961bfae8..04fd1fa510 100644 --- a/src/execution/collectFields.ts +++ b/src/execution/collectFields.ts @@ -17,9 +17,25 @@ import { } from '../type/directives'; import type { GraphQLSchema } from '../type/schema'; +import type { GraphQLVariableSignature } from '../utilities/getVariableSignature'; import { typeFromAST } from '../utilities/typeFromAST'; -import { getDirectiveValues } from './values'; +import { experimentalGetArgumentValues, getDirectiveValues } from './values'; + +export interface FragmentVariables { + signatures: ObjMap; + values: ObjMap; +} + +export interface FieldDetails { + node: FieldNode; + fragmentVariables?: FragmentVariables | undefined; +} + +export interface FragmentDetails { + definition: FragmentDefinitionNode; + variableSignatures?: ObjMap | undefined; +} /** * Given a selectionSet, collects all of the fields and returns them. @@ -32,11 +48,11 @@ import { getDirectiveValues } from './values'; */ export function collectFields( schema: GraphQLSchema, - fragments: ObjMap, + fragments: ObjMap, variableValues: { [variable: string]: unknown }, runtimeType: GraphQLObjectType, selectionSet: SelectionSetNode, -): Map> { +): Map> { const fields = new Map(); collectFieldsImpl( schema, @@ -46,6 +62,7 @@ export function collectFields( selectionSet, fields, new Set(), + undefined, ); return fields; } @@ -62,56 +79,66 @@ export function collectFields( */ export function collectSubfields( schema: GraphQLSchema, - fragments: ObjMap, + fragments: ObjMap, variableValues: { [variable: string]: unknown }, returnType: GraphQLObjectType, - fieldNodes: ReadonlyArray, -): Map> { - const subFieldNodes = new Map(); + fieldEntries: ReadonlyArray, +): Map> { + const subFieldEntries = new Map(); const visitedFragmentNames = new Set(); - for (const node of fieldNodes) { - if (node.selectionSet) { + for (const entry of fieldEntries) { + if (entry.node.selectionSet) { collectFieldsImpl( schema, fragments, variableValues, returnType, - node.selectionSet, - subFieldNodes, + entry.node.selectionSet, + subFieldEntries, visitedFragmentNames, + entry.fragmentVariables, ); } } - return subFieldNodes; + return subFieldEntries; } function collectFieldsImpl( schema: GraphQLSchema, - fragments: ObjMap, + fragments: ObjMap, variableValues: { [variable: string]: unknown }, runtimeType: GraphQLObjectType, selectionSet: SelectionSetNode, - fields: Map>, + fields: Map>, visitedFragmentNames: Set, + fragmentVariables?: FragmentVariables, ): void { for (const selection of selectionSet.selections) { switch (selection.kind) { case Kind.FIELD: { - if (!shouldIncludeNode(variableValues, selection)) { + if (!shouldIncludeNode(selection, variableValues, fragmentVariables)) { continue; } const name = getFieldEntryKey(selection); const fieldList = fields.get(name); if (fieldList !== undefined) { - fieldList.push(selection); + fieldList.push({ + node: selection, + fragmentVariables, + }); } else { - fields.set(name, [selection]); + fields.set(name, [ + { + node: selection, + fragmentVariables, + }, + ]); } break; } case Kind.INLINE_FRAGMENT: { if ( - !shouldIncludeNode(variableValues, selection) || + !shouldIncludeNode(selection, variableValues, fragmentVariables) || !doesFragmentConditionMatch(schema, selection, runtimeType) ) { continue; @@ -124,6 +151,7 @@ function collectFieldsImpl( selection.selectionSet, fields, visitedFragmentNames, + fragmentVariables, ); break; } @@ -131,7 +159,7 @@ function collectFieldsImpl( const fragName = selection.name.value; if ( visitedFragmentNames.has(fragName) || - !shouldIncludeNode(variableValues, selection) + !shouldIncludeNode(selection, variableValues, fragmentVariables) ) { continue; } @@ -139,18 +167,34 @@ function collectFieldsImpl( const fragment = fragments[fragName]; if ( !fragment || - !doesFragmentConditionMatch(schema, fragment, runtimeType) + !doesFragmentConditionMatch(schema, fragment.definition, runtimeType) ) { continue; } + + const fragmentVariableSignatures = fragment.variableSignatures; + let newFragmentVariables: FragmentVariables | undefined; + if (fragmentVariableSignatures) { + newFragmentVariables = { + signatures: fragmentVariableSignatures, + values: experimentalGetArgumentValues( + selection, + Object.values(fragmentVariableSignatures), + variableValues, + fragmentVariables, + ), + }; + } + collectFieldsImpl( schema, fragments, variableValues, runtimeType, - fragment.selectionSet, + fragment.definition.selectionSet, fields, visitedFragmentNames, + newFragmentVariables, ); break; } @@ -163,10 +207,16 @@ function collectFieldsImpl( * directives, where `@skip` has higher precedence than `@include`. */ function shouldIncludeNode( - variableValues: { [variable: string]: unknown }, node: FragmentSpreadNode | FieldNode | InlineFragmentNode, + variableValues: { [variable: string]: unknown }, + fragmentVariables: FragmentVariables | undefined, ): boolean { - const skip = getDirectiveValues(GraphQLSkipDirective, node, variableValues); + const skip = getDirectiveValues( + GraphQLSkipDirective, + node, + variableValues, + fragmentVariables, + ); if (skip?.if === true) { return false; } @@ -175,6 +225,7 @@ function shouldIncludeNode( GraphQLIncludeDirective, node, variableValues, + fragmentVariables, ); if (include?.if === false) { return false; diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 55c22ea9de..7b5288c388 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -4,6 +4,7 @@ import { invariant } from '../jsutils/invariant'; import { isIterableObject } from '../jsutils/isIterableObject'; import { isObjectLike } from '../jsutils/isObjectLike'; import { isPromise } from '../jsutils/isPromise'; +import { mapValue } from '../jsutils/mapValue'; import type { Maybe } from '../jsutils/Maybe'; import { memoize3 } from '../jsutils/memoize3'; import type { ObjMap } from '../jsutils/ObjMap'; @@ -20,7 +21,6 @@ import { locatedError } from '../error/locatedError'; import type { DocumentNode, FieldNode, - FragmentDefinitionNode, OperationDefinitionNode, } from '../language/ast'; import { OperationTypeNode } from '../language/ast'; @@ -52,11 +52,14 @@ import { import type { GraphQLSchema } from '../type/schema'; import { assertValidSchema } from '../type/validate'; +import { getVariableSignature } from '../utilities/getVariableSignature'; + +import type { FieldDetails, FragmentDetails } from './collectFields'; import { collectFields, collectSubfields as _collectSubfields, } from './collectFields'; -import { getArgumentValues, getVariableValues } from './values'; +import { experimentalGetArgumentValues, getVariableValues } from './values'; /** * A memoized collection of relevant subfields with regard to the return @@ -67,7 +70,7 @@ const collectSubfields = memoize3( ( exeContext: ExecutionContext, returnType: GraphQLObjectType, - fieldNodes: ReadonlyArray, + fieldNodes: ReadonlyArray, ) => _collectSubfields( exeContext.schema, @@ -106,7 +109,7 @@ const collectSubfields = memoize3( */ export interface ExecutionContext { schema: GraphQLSchema; - fragments: ObjMap; + fragments: ObjMap; rootValue: unknown; contextValue: unknown; operation: OperationDefinitionNode; @@ -289,7 +292,7 @@ export function buildExecutionContext( } = args; let operation: OperationDefinitionNode | undefined; - const fragments: ObjMap = Object.create(null); + const fragments: ObjMap = Object.create(null); for (const definition of document.definitions) { switch (definition.kind) { case Kind.OPERATION_DEFINITION: @@ -306,9 +309,18 @@ export function buildExecutionContext( operation = definition; } break; - case Kind.FRAGMENT_DEFINITION: - fragments[definition.name.value] = definition; + case Kind.FRAGMENT_DEFINITION: { + let variableSignatures; + if (definition.variableDefinitions) { + variableSignatures = Object.create(null); + for (const varDef of definition.variableDefinitions) { + const signature = getVariableSignature(schema, varDef); + variableSignatures[signature.name] = signature; + } + } + fragments[definition.name.value] = { definition, variableSignatures }; break; + } default: // ignore non-executable definitions } @@ -402,7 +414,7 @@ function executeFieldsSerially( parentType: GraphQLObjectType, sourceValue: unknown, path: Path | undefined, - fields: Map>, + fields: Map>, ): PromiseOrValue> { return promiseReduce( fields.entries(), @@ -440,7 +452,7 @@ function executeFields( parentType: GraphQLObjectType, sourceValue: unknown, path: Path | undefined, - fields: Map>, + fields: Map>, ): PromiseOrValue> { const results = Object.create(null); let containsPromise = false; @@ -494,10 +506,10 @@ function executeField( exeContext: ExecutionContext, parentType: GraphQLObjectType, source: unknown, - fieldNodes: ReadonlyArray, + fieldEntries: ReadonlyArray, path: Path, ): PromiseOrValue { - const fieldDef = getFieldDef(exeContext.schema, parentType, fieldNodes[0]); + const fieldDef = getFieldDef(exeContext.schema, parentType, fieldEntries[0]); if (!fieldDef) { return; } @@ -508,7 +520,7 @@ function executeField( const info = buildResolveInfo( exeContext, fieldDef, - fieldNodes, + fieldEntries, parentType, path, ); @@ -518,10 +530,11 @@ function executeField( // Build a JS object of arguments from the field.arguments AST, using the // variables scope to fulfill any variable references. // TODO: find a way to memoize, in case this field is within a List type. - const args = getArgumentValues( - fieldDef, - fieldNodes[0], + const args = experimentalGetArgumentValues( + fieldEntries[0].node, + fieldDef.args, exeContext.variableValues, + fieldEntries[0].fragmentVariables, ); // The resolve function's optional third argument is a context value that @@ -534,13 +547,20 @@ function executeField( let completed; if (isPromise(result)) { completed = result.then((resolved) => - completeValue(exeContext, returnType, fieldNodes, info, path, resolved), + completeValue( + exeContext, + returnType, + fieldEntries, + info, + path, + resolved, + ), ); } else { completed = completeValue( exeContext, returnType, - fieldNodes, + fieldEntries, info, path, result, @@ -551,13 +571,21 @@ function executeField( // Note: we don't rely on a `catch` method, but we do expect "thenable" // to take a second callback for the error case. return completed.then(undefined, (rawError) => { - const error = locatedError(rawError, fieldNodes, pathToArray(path)); + const error = locatedError( + rawError, + fieldEntries.map((entry) => entry.node), + pathToArray(path), + ); return handleFieldError(error, returnType, exeContext); }); } return completed; } catch (rawError) { - const error = locatedError(rawError, fieldNodes, pathToArray(path)); + const error = locatedError( + rawError, + fieldEntries.map((entry) => entry.node), + pathToArray(path), + ); return handleFieldError(error, returnType, exeContext); } } @@ -568,7 +596,7 @@ function executeField( export function buildResolveInfo( exeContext: ExecutionContext, fieldDef: GraphQLField, - fieldNodes: ReadonlyArray, + fieldEntries: ReadonlyArray, parentType: GraphQLObjectType, path: Path, ): GraphQLResolveInfo { @@ -576,12 +604,15 @@ export function buildResolveInfo( // information about the current execution state. return { fieldName: fieldDef.name, - fieldNodes, + fieldNodes: fieldEntries.map((entry) => entry.node), returnType: fieldDef.type, parentType, path, schema: exeContext.schema, - fragments: exeContext.fragments, + fragments: mapValue( + exeContext.fragments, + (fragment) => fragment.definition, + ), rootValue: exeContext.rootValue, operation: exeContext.operation, variableValues: exeContext.variableValues, @@ -629,7 +660,7 @@ function handleFieldError( function completeValue( exeContext: ExecutionContext, returnType: GraphQLOutputType, - fieldNodes: ReadonlyArray, + fieldNodes: ReadonlyArray, info: GraphQLResolveInfo, path: Path, result: unknown, @@ -720,7 +751,7 @@ function completeValue( function completeListValue( exeContext: ExecutionContext, returnType: GraphQLList, - fieldNodes: ReadonlyArray, + fieldEntries: ReadonlyArray, info: GraphQLResolveInfo, path: Path, result: unknown, @@ -746,7 +777,7 @@ function completeListValue( completeValue( exeContext, itemType, - fieldNodes, + fieldEntries, info, itemPath, resolved, @@ -756,7 +787,7 @@ function completeListValue( completedItem = completeValue( exeContext, itemType, - fieldNodes, + fieldEntries, info, itemPath, item, @@ -770,7 +801,7 @@ function completeListValue( return completedItem.then(undefined, (rawError) => { const error = locatedError( rawError, - fieldNodes, + fieldEntries.map((entry) => entry.node), pathToArray(itemPath), ); return handleFieldError(error, itemType, exeContext); @@ -778,7 +809,11 @@ function completeListValue( } return completedItem; } catch (rawError) { - const error = locatedError(rawError, fieldNodes, pathToArray(itemPath)); + const error = locatedError( + rawError, + fieldEntries.map((entry) => entry.node), + pathToArray(itemPath), + ); return handleFieldError(error, itemType, exeContext); } }); @@ -811,7 +846,7 @@ function completeLeafValue( function completeAbstractValue( exeContext: ExecutionContext, returnType: GraphQLAbstractType, - fieldNodes: ReadonlyArray, + fieldEntries: ReadonlyArray, info: GraphQLResolveInfo, path: Path, result: unknown, @@ -820,6 +855,7 @@ function completeAbstractValue( const contextValue = exeContext.contextValue; const runtimeType = resolveTypeFn(result, contextValue, info, returnType); + const fieldNodes = fieldEntries.map((entry) => entry.node); if (isPromise(runtimeType)) { return runtimeType.then((resolvedRuntimeType) => completeObjectValue( @@ -832,7 +868,7 @@ function completeAbstractValue( info, result, ), - fieldNodes, + fieldEntries, info, path, result, @@ -850,7 +886,7 @@ function completeAbstractValue( info, result, ), - fieldNodes, + fieldEntries, info, path, result, @@ -918,13 +954,13 @@ function ensureValidRuntimeType( function completeObjectValue( exeContext: ExecutionContext, returnType: GraphQLObjectType, - fieldNodes: ReadonlyArray, + fieldEntries: ReadonlyArray, info: GraphQLResolveInfo, path: Path, result: unknown, ): PromiseOrValue> { // Collect sub-fields to execute to complete this value. - const subFieldNodes = collectSubfields(exeContext, returnType, fieldNodes); + const subFieldNodes = collectSubfields(exeContext, returnType, fieldEntries); // If there is an isTypeOf predicate function, call it with the // current result. If isTypeOf returns false, then raise an error rather @@ -935,7 +971,11 @@ function completeObjectValue( if (isPromise(isTypeOf)) { return isTypeOf.then((resolvedIsTypeOf) => { if (!resolvedIsTypeOf) { - throw invalidReturnTypeError(returnType, result, fieldNodes); + throw invalidReturnTypeError( + returnType, + result, + fieldEntries.map((entry) => entry.node), + ); } return executeFields( exeContext, @@ -948,7 +988,11 @@ function completeObjectValue( } if (!isTypeOf) { - throw invalidReturnTypeError(returnType, result, fieldNodes); + throw invalidReturnTypeError( + returnType, + result, + fieldEntries.map((entry) => entry.node), + ); } } @@ -1044,9 +1088,9 @@ export const defaultFieldResolver: GraphQLFieldResolver = export function getFieldDef( schema: GraphQLSchema, parentType: GraphQLObjectType, - fieldNode: FieldNode, + entry: FieldDetails, ): Maybe> { - const fieldName = fieldNode.name.value; + const fieldName = entry.node.name.value; if ( fieldName === SchemaMetaFieldDef.name && diff --git a/src/execution/subscribe.ts b/src/execution/subscribe.ts index 8b20ec3374..bc799e9585 100644 --- a/src/execution/subscribe.ts +++ b/src/execution/subscribe.ts @@ -214,14 +214,14 @@ async function executeSubscription( rootType, operation.selectionSet, ); - const [responseName, fieldNodes] = [...rootFields.entries()][0]; - const fieldDef = getFieldDef(schema, rootType, fieldNodes[0]); + const [responseName, fieldEntries] = [...rootFields.entries()][0]; + const fieldDef = getFieldDef(schema, rootType, fieldEntries[0]); if (!fieldDef) { - const fieldName = fieldNodes[0].name.value; + const fieldName = fieldEntries[0].node.name.value; throw new GraphQLError( `The subscription field "${fieldName}" is not defined.`, - { nodes: fieldNodes }, + { nodes: fieldEntries.map((entry) => entry.node) }, ); } @@ -229,7 +229,7 @@ async function executeSubscription( const info = buildResolveInfo( exeContext, fieldDef, - fieldNodes, + fieldEntries, rootType, path, ); @@ -240,7 +240,11 @@ async function executeSubscription( // Build a JS object of arguments from the field.arguments AST, using the // variables scope to fulfill any variable references. - const args = getArgumentValues(fieldDef, fieldNodes[0], variableValues); + const args = getArgumentValues( + fieldDef, + fieldEntries[0].node, + variableValues, + ); // The resolve function's optional third argument is a context value that // is provided to every resolve function within an execution. It is commonly @@ -257,6 +261,10 @@ async function executeSubscription( } return eventStream; } catch (error) { - throw locatedError(error, fieldNodes, pathToArray(path)); + throw locatedError( + error, + fieldEntries.map((entry) => entry.node), + pathToArray(path), + ); } } diff --git a/src/execution/values.ts b/src/execution/values.ts index d65ea9cf20..7500619c6c 100644 --- a/src/execution/values.ts +++ b/src/execution/values.ts @@ -1,5 +1,4 @@ import { inspect } from '../jsutils/inspect'; -import { keyMap } from '../jsutils/keyMap'; import type { Maybe } from '../jsutils/Maybe'; import type { ObjMap } from '../jsutils/ObjMap'; import { printPathArray } from '../jsutils/printPathArray'; @@ -9,20 +8,24 @@ import { GraphQLError } from '../error/GraphQLError'; import type { DirectiveNode, FieldNode, + FragmentSpreadNode, VariableDefinitionNode, } from '../language/ast'; import { Kind } from '../language/kinds'; import { print } from '../language/printer'; -import type { GraphQLField } from '../type/definition'; -import { isInputType, isNonNullType } from '../type/definition'; +import type { GraphQLArgument, GraphQLField } from '../type/definition'; +import { isNonNullType } from '../type/definition'; import type { GraphQLDirective } from '../type/directives'; import type { GraphQLSchema } from '../type/schema'; import { coerceInputValue } from '../utilities/coerceInputValue'; -import { typeFromAST } from '../utilities/typeFromAST'; +import type { GraphQLVariableSignature } from '../utilities/getVariableSignature'; +import { getVariableSignature } from '../utilities/getVariableSignature'; import { valueFromAST } from '../utilities/valueFromAST'; +import type { FragmentVariables } from './collectFields'; + type CoercedVariableValues = | { errors: ReadonlyArray; coerced?: never } | { coerced: { [variable: string]: unknown }; errors?: never }; @@ -77,24 +80,16 @@ function coerceVariableValues( ): { [variable: string]: unknown } { const coercedValues: { [variable: string]: unknown } = {}; for (const varDefNode of varDefNodes) { - const varName = varDefNode.variable.name.value; - const varType = typeFromAST(schema, varDefNode.type); - if (!isInputType(varType)) { - // Must use input types for variables. This should be caught during - // validation, however is checked again here for safety. - const varTypeStr = print(varDefNode.type); - onError( - new GraphQLError( - `Variable "$${varName}" expected value of type "${varTypeStr}" which cannot be used as an input type.`, - { nodes: varDefNode.type }, - ), - ); + const varSignature = getVariableSignature(schema, varDefNode); + if (varSignature instanceof GraphQLError) { + onError(varSignature); continue; } + const { name: varName, type: varType } = varSignature; if (!hasOwnProperty(inputs, varName)) { if (varDefNode.defaultValue) { - coercedValues[varName] = valueFromAST(varDefNode.defaultValue, varType); + coercedValues[varName] = varSignature.defaultValue; } else if (isNonNullType(varType)) { const varTypeStr = inspect(varType); onError( @@ -149,24 +144,25 @@ function coerceVariableValues( * exposed to user code. Care should be taken to not pull values from the * Object prototype. */ -export function getArgumentValues( - def: GraphQLField | GraphQLDirective, - node: FieldNode | DirectiveNode, - variableValues?: Maybe>, +export function experimentalGetArgumentValues( + node: FieldNode | DirectiveNode | FragmentSpreadNode, + argDefs: ReadonlyArray, + variableValues: Maybe>, + fragmentVariables?: Maybe, ): { [argument: string]: unknown } { const coercedValues: { [argument: string]: unknown } = {}; // FIXME: https://github.com/graphql/graphql-js/issues/2203 /* c8 ignore next */ const argumentNodes = node.arguments ?? []; - const argNodeMap = keyMap(argumentNodes, (arg) => arg.name.value); + const argNodeMap = new Map(argumentNodes.map((arg) => [arg.name.value, arg])); - for (const argDef of def.args) { + for (const argDef of argDefs) { const name = argDef.name; const argType = argDef.type; - const argumentNode = argNodeMap[name]; + const argumentNode = argNodeMap.get(name); - if (!argumentNode) { + if (argumentNode == null) { if (argDef.defaultValue !== undefined) { coercedValues[name] = argDef.defaultValue; } else if (isNonNullType(argType)) { @@ -184,9 +180,12 @@ export function getArgumentValues( if (valueNode.kind === Kind.VARIABLE) { const variableName = valueNode.name.value; + const scopedVariableValues = fragmentVariables?.signatures[variableName] + ? fragmentVariables.values + : variableValues; if ( - variableValues == null || - !hasOwnProperty(variableValues, variableName) + scopedVariableValues == null || + !hasOwnProperty(scopedVariableValues, variableName) ) { if (argDef.defaultValue !== undefined) { coercedValues[name] = argDef.defaultValue; @@ -199,7 +198,7 @@ export function getArgumentValues( } continue; } - isNull = variableValues[variableName] == null; + isNull = scopedVariableValues[variableName] == null; } if (isNull && isNonNullType(argType)) { @@ -210,7 +209,12 @@ export function getArgumentValues( ); } - const coercedValue = valueFromAST(valueNode, argType, variableValues); + const coercedValue = valueFromAST( + valueNode, + argType, + variableValues, + fragmentVariables?.values, + ); if (coercedValue === undefined) { // Note: ValuesOfCorrectTypeRule validation should catch this before // execution. This is a runtime check to ensure execution does not @@ -236,17 +240,31 @@ export function getArgumentValues( * exposed to user code. Care should be taken to not pull values from the * Object prototype. */ +export function getArgumentValues( + def: GraphQLField | GraphQLDirective, + node: FieldNode | DirectiveNode, + variableValues?: Maybe>, +): { [argument: string]: unknown } { + return experimentalGetArgumentValues(node, def.args, variableValues); +} + export function getDirectiveValues( directiveDef: GraphQLDirective, node: { readonly directives?: ReadonlyArray }, variableValues?: Maybe>, + fragmentVariables?: Maybe, ): undefined | { [argument: string]: unknown } { const directiveNode = node.directives?.find( (directive) => directive.name.value === directiveDef.name, ); if (directiveNode) { - return getArgumentValues(directiveDef, directiveNode, variableValues); + return experimentalGetArgumentValues( + directiveNode, + directiveDef.args, + variableValues, + fragmentVariables, + ); } } diff --git a/src/index.ts b/src/index.ts index 877939d879..ae2f4215ac 100644 --- a/src/index.ts +++ b/src/index.ts @@ -263,6 +263,7 @@ export type { SelectionNode, FieldNode, ArgumentNode, + FragmentArgumentNode, ConstArgumentNode, FragmentSpreadNode, InlineFragmentNode, diff --git a/src/language/__tests__/parser-test.ts b/src/language/__tests__/parser-test.ts index 87e7b92c34..c580aae303 100644 --- a/src/language/__tests__/parser-test.ts +++ b/src/language/__tests__/parser-test.ts @@ -395,15 +395,35 @@ describe('Parser', () => { expect('loc' in result).to.equal(false); }); - it('Legacy: allows parsing fragment defined variables', () => { + it('allows parsing fragment defined variables', () => { const document = 'fragment a($v: Boolean = false) on t { f(v: $v) }'; expect(() => - parse(document, { allowLegacyFragmentVariables: true }), + parse(document, { experimentalFragmentArguments: true }), ).to.not.throw(); expect(() => parse(document)).to.throw('Syntax Error'); }); + it('disallows parsing fragment defined variables without experimental flag', () => { + const document = 'fragment a($v: Boolean = false) on t { f(v: $v) }'; + + expect(() => parse(document)).to.throw(); + }); + + it('allows parsing fragment spread arguments', () => { + const document = 'fragment a on t { ...b(v: $v) }'; + + expect(() => + parse(document, { experimentalFragmentArguments: true }), + ).to.not.throw(); + }); + + it('disallows parsing fragment spread arguments without experimental flag', () => { + const document = 'fragment a on t { ...b(v: $v) }'; + + expect(() => parse(document)).to.throw(); + }); + it('contains location that can be Object.toStringified, JSON.stringified, or jsutils.inspected', () => { const { loc } = parse('{ id }'); diff --git a/src/language/__tests__/printer-test.ts b/src/language/__tests__/printer-test.ts index 227e90dd44..13ad9ce6c6 100644 --- a/src/language/__tests__/printer-test.ts +++ b/src/language/__tests__/printer-test.ts @@ -110,10 +110,10 @@ describe('Printer: Query document', () => { `); }); - it('Legacy: prints fragment with variable directives', () => { + it('prints fragment with variable directives', () => { const queryASTWithVariableDirective = parse( 'fragment Foo($foo: TestType @test) on TestType @testDirective { id }', - { allowLegacyFragmentVariables: true }, + { experimentalFragmentArguments: true }, ); expect(print(queryASTWithVariableDirective)).to.equal(dedent` fragment Foo($foo: TestType @test) on TestType @testDirective { @@ -122,14 +122,14 @@ describe('Printer: Query document', () => { `); }); - it('Legacy: correctly prints fragment defined variables', () => { + it('correctly prints fragment defined variables', () => { const fragmentWithVariable = parse( ` fragment Foo($a: ComplexType, $b: Boolean = false) on TestType { id } `, - { allowLegacyFragmentVariables: true }, + { experimentalFragmentArguments: true }, ); expect(print(fragmentWithVariable)).to.equal(dedent` fragment Foo($a: ComplexType, $b: Boolean = false) on TestType { @@ -213,4 +213,32 @@ describe('Printer: Query document', () => { `), ); }); + + it('prints fragment spread with arguments', () => { + const fragmentSpreadWithArguments = parse( + 'fragment Foo on TestType { ...Bar(a: {x: $x}, b: true) }', + { experimentalFragmentArguments: true }, + ); + expect(print(fragmentSpreadWithArguments)).to.equal(dedent` + fragment Foo on TestType { + ...Bar(a: {x: $x}, b: true) + } + `); + }); + + it('prints fragment spread with multi-line arguments', () => { + const fragmentSpreadWithArguments = parse( + 'fragment Foo on TestType { ...Bar(a: {x: $x, y: $y, z: $z, xy: $xy}, b: true, c: "a long string extending arguments over max length") }', + { experimentalFragmentArguments: true }, + ); + expect(print(fragmentSpreadWithArguments)).to.equal(dedent` + fragment Foo on TestType { + ...Bar( + a: {x: $x, y: $y, z: $z, xy: $xy} + b: true + c: "a long string extending arguments over max length" + ) + } + `); + }); }); diff --git a/src/language/__tests__/visitor-test.ts b/src/language/__tests__/visitor-test.ts index 9149b103e3..8dab3026e7 100644 --- a/src/language/__tests__/visitor-test.ts +++ b/src/language/__tests__/visitor-test.ts @@ -455,10 +455,10 @@ describe('Visitor', () => { ]); }); - it('Legacy: visits variables defined in fragments', () => { + it('visits variables defined in fragments', () => { const ast = parse('fragment a($v: Boolean = false) on t { f }', { noLocation: true, - allowLegacyFragmentVariables: true, + experimentalFragmentArguments: true, }); const visited: Array = []; @@ -1361,4 +1361,48 @@ describe('Visitor', () => { ]); }); }); + + it('visits arguments on fragment spreads', () => { + const ast = parse('fragment a on t { ...s(v: false) }', { + noLocation: true, + experimentalFragmentArguments: true, + }); + const visited: Array = []; + + visit(ast, { + enter(node) { + checkVisitorFnArgs(ast, arguments); + visited.push(['enter', node.kind, getValue(node)]); + }, + leave(node) { + checkVisitorFnArgs(ast, arguments); + visited.push(['leave', node.kind, getValue(node)]); + }, + }); + + expect(visited).to.deep.equal([ + ['enter', 'Document', undefined], + ['enter', 'FragmentDefinition', undefined], + ['enter', 'Name', 'a'], + ['leave', 'Name', 'a'], + ['enter', 'NamedType', undefined], + ['enter', 'Name', 't'], + ['leave', 'Name', 't'], + ['leave', 'NamedType', undefined], + ['enter', 'SelectionSet', undefined], + ['enter', 'FragmentSpread', undefined], + ['enter', 'Name', 's'], + ['leave', 'Name', 's'], + ['enter', 'FragmentArgument', { kind: 'BooleanValue', value: false }], + ['enter', 'Name', 'v'], + ['leave', 'Name', 'v'], + ['enter', 'BooleanValue', false], + ['leave', 'BooleanValue', false], + ['leave', 'FragmentArgument', { kind: 'BooleanValue', value: false }], + ['leave', 'FragmentSpread', undefined], + ['leave', 'SelectionSet', undefined], + ['leave', 'FragmentDefinition', undefined], + ['leave', 'Document', undefined], + ]); + }); }); diff --git a/src/language/ast.ts b/src/language/ast.ts index 29029342a1..f6d1bfbcb3 100644 --- a/src/language/ast.ts +++ b/src/language/ast.ts @@ -145,6 +145,7 @@ export type ASTNode = | SelectionSetNode | FieldNode | ArgumentNode + | FragmentArgumentNode | FragmentSpreadNode | InlineFragmentNode | FragmentDefinitionNode @@ -208,12 +209,12 @@ export const QueryDocumentKeys: { SelectionSet: ['selections'], Field: ['alias', 'name', 'arguments', 'directives', 'selectionSet'], Argument: ['name', 'value'], + FragmentArgument: ['name', 'value'], - FragmentSpread: ['name', 'directives'], + FragmentSpread: ['name', 'arguments', 'directives'], InlineFragment: ['typeCondition', 'directives', 'selectionSet'], FragmentDefinition: [ 'name', - // Note: fragment variable definitions are deprecated and will removed in v17.0.0 'variableDefinitions', 'typeCondition', 'directives', @@ -383,6 +384,7 @@ export interface FragmentSpreadNode { readonly kind: Kind.FRAGMENT_SPREAD; readonly loc?: Location; readonly name: NameNode; + readonly arguments?: ReadonlyArray; readonly directives?: ReadonlyArray; } @@ -398,7 +400,6 @@ export interface FragmentDefinitionNode { readonly kind: Kind.FRAGMENT_DEFINITION; readonly loc?: Location; readonly name: NameNode; - /** @deprecated variableDefinitions will be removed in v17.0.0 */ readonly variableDefinitions?: ReadonlyArray; readonly typeCondition: NamedTypeNode; readonly directives?: ReadonlyArray; @@ -502,6 +503,13 @@ export interface ConstObjectFieldNode { readonly value: ConstValueNode; } +export interface FragmentArgumentNode { + readonly kind: Kind.FRAGMENT_ARGUMENT; + readonly loc?: Location | undefined; + readonly name: NameNode; + readonly value: ValueNode; +} + /** Directives */ export interface DirectiveNode { diff --git a/src/language/directiveLocation.ts b/src/language/directiveLocation.ts index 5c8aeb7240..8cc407b9ab 100644 --- a/src/language/directiveLocation.ts +++ b/src/language/directiveLocation.ts @@ -23,6 +23,7 @@ enum DirectiveLocation { ENUM_VALUE = 'ENUM_VALUE', INPUT_OBJECT = 'INPUT_OBJECT', INPUT_FIELD_DEFINITION = 'INPUT_FIELD_DEFINITION', + FRAGMENT_VARIABLE_DEFINITION = 'FRAGMENT_VARIABLE_DEFINITION', } export { DirectiveLocation }; diff --git a/src/language/index.ts b/src/language/index.ts index ec4d195e1a..79b0dc2f20 100644 --- a/src/language/index.ts +++ b/src/language/index.ts @@ -43,6 +43,7 @@ export type { SelectionNode, FieldNode, ArgumentNode, + FragmentArgumentNode /* for experimental fragment arguments */, ConstArgumentNode, FragmentSpreadNode, InlineFragmentNode, diff --git a/src/language/kinds.ts b/src/language/kinds.ts index cd05f66a3b..8e6e258801 100644 --- a/src/language/kinds.ts +++ b/src/language/kinds.ts @@ -12,6 +12,7 @@ enum Kind { SELECTION_SET = 'SelectionSet', FIELD = 'Field', ARGUMENT = 'Argument', + FRAGMENT_ARGUMENT = 'FragmentArgument', /** Fragments */ FRAGMENT_SPREAD = 'FragmentSpread', diff --git a/src/language/parser.ts b/src/language/parser.ts index eb54a0376b..10dbbb04d3 100644 --- a/src/language/parser.ts +++ b/src/language/parser.ts @@ -23,6 +23,7 @@ import type { FieldDefinitionNode, FieldNode, FloatValueNode, + FragmentArgumentNode, FragmentDefinitionNode, FragmentSpreadNode, InlineFragmentNode, @@ -103,6 +104,27 @@ export interface ParseOptions { * ``` */ allowLegacyFragmentVariables?: boolean; + + /** + * EXPERIMENTAL: + * + * If enabled, the parser will understand and parse fragment variable definitions + * and arguments on fragment spreads. Fragment variable definitions will be represented + * in the `variableDefinitions` field of the FragmentDefinitionNode. + * Fragment spread arguments will be represented in the `arguments` field of FragmentSpreadNode. + * + * For example: + * + * ```graphql + * { + * t { ...A(var: true) } + * } + * fragment A($var: Boolean = false) on T { + * ...B(x: $var) + * } + * ``` + */ + experimentalFragmentArguments?: boolean | undefined; } /** @@ -485,7 +507,7 @@ export class Parser { /** * Corresponds to both FragmentSpread and InlineFragment in the spec. * - * FragmentSpread : ... FragmentName Directives? + * FragmentSpread : ... FragmentName Arguments? Directives? * * InlineFragment : ... TypeCondition? Directives? SelectionSet */ @@ -495,9 +517,21 @@ export class Parser { const hasTypeCondition = this.expectOptionalKeyword('on'); if (!hasTypeCondition && this.peek(TokenKind.NAME)) { + const name = this.parseFragmentName(); + if ( + this.peek(TokenKind.PAREN_L) && + this._options.experimentalFragmentArguments + ) { + return this.node(start, { + kind: Kind.FRAGMENT_SPREAD, + name, + arguments: this.parseFragmentArguments(), + directives: this.parseDirectives(false), + }); + } return this.node(start, { kind: Kind.FRAGMENT_SPREAD, - name: this.parseFragmentName(), + name, directives: this.parseDirectives(false), }); } @@ -509,31 +543,42 @@ export class Parser { }); } + /* experimental */ + parseFragmentArguments(): Array { + const item = this.parseFragmentArgument; + return this.optionalMany(TokenKind.PAREN_L, item, TokenKind.PAREN_R); + } + + /* experimental */ + parseFragmentArgument(): FragmentArgumentNode { + const start = this._lexer.token; + const name = this.parseName(); + + this.expectToken(TokenKind.COLON); + return this.node(start, { + kind: Kind.FRAGMENT_ARGUMENT, + name, + value: this.parseValueLiteral(false), + }); + } + /** * FragmentDefinition : - * - fragment FragmentName on TypeCondition Directives? SelectionSet + * - fragment FragmentName VariableDefinitions? on TypeCondition Directives? SelectionSet * * TypeCondition : NamedType */ parseFragmentDefinition(): FragmentDefinitionNode { const start = this._lexer.token; this.expectKeyword('fragment'); - // Legacy support for defining variables within fragments changes - // the grammar of FragmentDefinition: - // - fragment FragmentName VariableDefinitions? on TypeCondition Directives? SelectionSet - if (this._options.allowLegacyFragmentVariables === true) { - return this.node(start, { - kind: Kind.FRAGMENT_DEFINITION, - name: this.parseFragmentName(), - variableDefinitions: this.parseVariableDefinitions(), - typeCondition: (this.expectKeyword('on'), this.parseNamedType()), - directives: this.parseDirectives(false), - selectionSet: this.parseSelectionSet(), - }); - } return this.node(start, { kind: Kind.FRAGMENT_DEFINITION, name: this.parseFragmentName(), + variableDefinitions: + this._options.experimentalFragmentArguments === true || + this._options.allowLegacyFragmentVariables === true + ? this.parseVariableDefinitions() + : undefined, typeCondition: (this.expectKeyword('on'), this.parseNamedType()), directives: this.parseDirectives(false), selectionSet: this.parseSelectionSet(), diff --git a/src/language/printer.ts b/src/language/printer.ts index e95c118d8b..3c89f004e0 100644 --- a/src/language/printer.ts +++ b/src/language/printer.ts @@ -57,23 +57,26 @@ const printDocASTReducer: ASTReducer = { Field: { leave({ alias, name, arguments: args, directives, selectionSet }) { const prefix = wrap('', alias, ': ') + name; - let argsLine = prefix + wrap('(', join(args, ', '), ')'); - if (argsLine.length > MAX_LINE_LENGTH) { - argsLine = prefix + wrap('(\n', indent(join(args, '\n')), '\n)'); - } - - return join([argsLine, join(directives, ' '), selectionSet], ' '); + return join( + [wrappedLineAndArgs(prefix, args), join(directives, ' '), selectionSet], + ' ', + ); }, }, Argument: { leave: ({ name, value }) => name + ': ' + value }, + FragmentArgument: { leave: ({ name, value }) => name + ': ' + value }, // Fragments FragmentSpread: { - leave: ({ name, directives }) => - '...' + name + wrap(' ', join(directives, ' ')), + leave: ({ name, arguments: args, directives }) => { + const prefix = '...' + name; + return ( + wrappedLineAndArgs(prefix, args) + wrap(' ', join(directives, ' ')) + ); + }, }, InlineFragment: { @@ -345,3 +348,15 @@ function hasMultilineItems(maybeArray: Maybe>): boolean { /* c8 ignore next */ return maybeArray?.some((str) => str.includes('\n')) ?? false; } + +function wrappedLineAndArgs( + prefix: string, + args: ReadonlyArray | undefined, +): string { + let argsLine = prefix + wrap('(', join(args, ', '), ')'); + + if (argsLine.length > MAX_LINE_LENGTH) { + argsLine = prefix + wrap('(\n', indent(join(args, '\n')), '\n)'); + } + return argsLine; +} diff --git a/src/type/__tests__/introspection-test.ts b/src/type/__tests__/introspection-test.ts index 29994c76ed..9e77e342ca 100644 --- a/src/type/__tests__/introspection-test.ts +++ b/src/type/__tests__/introspection-test.ts @@ -859,6 +859,11 @@ describe('Introspection', () => { isDeprecated: false, deprecationReason: null, }, + { + name: 'FRAGMENT_VARIABLE_DEFINITION', + isDeprecated: false, + deprecationReason: null, + }, { name: 'SCHEMA', isDeprecated: false, diff --git a/src/type/definition.ts b/src/type/definition.ts index 7eaac560dc..a4f36e0ce7 100644 --- a/src/type/definition.ts +++ b/src/type/definition.ts @@ -40,6 +40,7 @@ import type { import { Kind } from '../language/kinds'; import { print } from '../language/printer'; +import type { GraphQLVariableSignature } from '../utilities/getVariableSignature'; import { valueFromASTUntyped } from '../utilities/valueFromASTUntyped'; import { assertEnumValueName, assertName } from './assertName'; @@ -1069,7 +1070,9 @@ export interface GraphQLArgument { astNode: Maybe; } -export function isRequiredArgument(arg: GraphQLArgument): boolean { +export function isRequiredArgument( + arg: GraphQLArgument | GraphQLVariableSignature, +): boolean { return isNonNullType(arg.type) && arg.defaultValue === undefined; } diff --git a/src/type/introspection.ts b/src/type/introspection.ts index 2c66ca5098..c30b32e9c0 100644 --- a/src/type/introspection.ts +++ b/src/type/introspection.ts @@ -155,7 +155,11 @@ export const __DirectiveLocation: GraphQLEnumType = new GraphQLEnumType({ }, VARIABLE_DEFINITION: { value: DirectiveLocation.VARIABLE_DEFINITION, - description: 'Location adjacent to a variable definition.', + description: 'Location adjacent to an operation variable definition.', + }, + FRAGMENT_VARIABLE_DEFINITION: { + value: DirectiveLocation.FRAGMENT_VARIABLE_DEFINITION, + description: 'Location adjacent to a fragment variable definition.', }, SCHEMA: { value: DirectiveLocation.SCHEMA, diff --git a/src/utilities/TypeInfo.ts b/src/utilities/TypeInfo.ts index e72dfb01fb..55887a0ddf 100644 --- a/src/utilities/TypeInfo.ts +++ b/src/utilities/TypeInfo.ts @@ -1,6 +1,13 @@ import type { Maybe } from '../jsutils/Maybe'; +import type { ObjMap } from '../jsutils/ObjMap'; -import type { ASTNode, FieldNode } from '../language/ast'; +import type { + ASTNode, + DocumentNode, + FieldNode, + FragmentDefinitionNode, + VariableDefinitionNode, +} from '../language/ast'; import { isNode } from '../language/ast'; import { Kind } from '../language/kinds'; import type { ASTVisitor } from '../language/visitor'; @@ -38,6 +45,10 @@ import type { GraphQLSchema } from '../type/schema'; import { typeFromAST } from './typeFromAST'; +export interface FragmentSignature { + readonly definition: FragmentDefinitionNode; + readonly variableDefinitions: ObjMap; +} /** * TypeInfo is a utility class which, given a GraphQL schema, can keep track * of the current field and type definitions at any point in a GraphQL document @@ -53,6 +64,12 @@ export class TypeInfo { private _directive: Maybe; private _argument: Maybe; private _enumValue: Maybe; + private _fragmentSignaturesByName: ( + fragmentName: string, + ) => Maybe; + + private _fragmentSignature: Maybe; + private _fragmentArgument: Maybe; private _getFieldDef: GetFieldDefFn; constructor( @@ -64,7 +81,10 @@ export class TypeInfo { initialType?: Maybe, /** @deprecated will be removed in 17.0.0 */ - getFieldDefFn?: GetFieldDefFn, + getFieldDefFn?: Maybe, + fragmentSignatures?: Maybe< + (fragmentName: string) => Maybe + >, ) { this._schema = schema; this._typeStack = []; @@ -75,6 +95,9 @@ export class TypeInfo { this._directive = null; this._argument = null; this._enumValue = null; + this._fragmentSignaturesByName = fragmentSignatures ?? (() => null); + this._fragmentSignature = null; + this._fragmentArgument = null; this._getFieldDef = getFieldDefFn ?? getFieldDef; if (initialType) { if (isInputType(initialType)) { @@ -137,6 +160,20 @@ export class TypeInfo { return this._argument; } + getFragmentSignature(): Maybe { + return this._fragmentSignature; + } + + getFragmentSignatureByName(): ( + fragmentName: string, + ) => Maybe { + return this._fragmentSignaturesByName; + } + + getFragmentArgument(): Maybe { + return this._fragmentArgument; + } + getEnumValue(): Maybe { return this._enumValue; } @@ -148,6 +185,12 @@ export class TypeInfo { // checked before continuing since TypeInfo is used as part of validation // which occurs before guarantees of schema and document validity. switch (node.kind) { + case Kind.DOCUMENT: { + const fragmentSignatures = getFragmentSignatures(node); + this._fragmentSignaturesByName = (fragmentName: string) => + fragmentSignatures[fragmentName]; + break; + } case Kind.SELECTION_SET: { const namedType: unknown = getNamedType(this.getType()); this._parentTypeStack.push( @@ -177,6 +220,12 @@ export class TypeInfo { this._typeStack.push(isObjectType(rootType) ? rootType : undefined); break; } + case Kind.FRAGMENT_SPREAD: { + this._fragmentSignature = this.getFragmentSignatureByName()( + node.name.value, + ); + break; + } case Kind.INLINE_FRAGMENT: case Kind.FRAGMENT_DEFINITION: { const typeConditionAST = node.typeCondition; @@ -193,6 +242,17 @@ export class TypeInfo { ); break; } + case Kind.FRAGMENT_ARGUMENT: { + const fragmentSignature = this.getFragmentSignature(); + const argDef = fragmentSignature?.variableDefinitions[node.name.value]; + this._fragmentArgument = argDef; + let argType: unknown; + if (argDef) { + argType = typeFromAST(this._schema, argDef.type); + } + this._inputTypeStack.push(isInputType(argType) ? argType : undefined); + break; + } case Kind.ARGUMENT: { let argDef; let argType: unknown; @@ -254,6 +314,10 @@ export class TypeInfo { leave(node: ASTNode) { switch (node.kind) { + case Kind.DOCUMENT: + this._fragmentSignaturesByName = /* c8 ignore start */ () => + null /* c8 ignore end */; + break; case Kind.SELECTION_SET: this._parentTypeStack.pop(); break; @@ -264,6 +328,9 @@ export class TypeInfo { case Kind.DIRECTIVE: this._directive = null; break; + case Kind.FRAGMENT_SPREAD: + this._fragmentSignature = null; + break; case Kind.OPERATION_DEFINITION: case Kind.INLINE_FRAGMENT: case Kind.FRAGMENT_DEFINITION: @@ -272,6 +339,12 @@ export class TypeInfo { case Kind.VARIABLE_DEFINITION: this._inputTypeStack.pop(); break; + case Kind.FRAGMENT_ARGUMENT: { + this._fragmentArgument = null; + this._defaultValueStack.pop(); + this._inputTypeStack.pop(); + break; + } case Kind.ARGUMENT: this._argument = null; this._defaultValueStack.pop(); @@ -325,6 +398,25 @@ function getFieldDef( } } +function getFragmentSignatures( + document: DocumentNode, +): ObjMap { + const fragmentSignatures = Object.create(null); + for (const definition of document.definitions) { + if (definition.kind === Kind.FRAGMENT_DEFINITION) { + const variableDefinitions = Object.create(null); + if (definition.variableDefinitions) { + for (const varDef of definition.variableDefinitions) { + variableDefinitions[varDef.variable.name.value] = varDef; + } + } + const signature = { definition, variableDefinitions }; + fragmentSignatures[definition.name.value] = signature; + } + } + return fragmentSignatures; +} + /** * Creates a new visitor instance which maintains a provided TypeInfo instance * along with visiting visitor. diff --git a/src/utilities/__tests__/TypeInfo-test.ts b/src/utilities/__tests__/TypeInfo-test.ts index 5c04458c51..e2a7b6110f 100644 --- a/src/utilities/__tests__/TypeInfo-test.ts +++ b/src/utilities/__tests__/TypeInfo-test.ts @@ -68,6 +68,9 @@ describe('TypeInfo', () => { expect(typeInfo.getDirective()).to.equal(null); expect(typeInfo.getArgument()).to.equal(null); expect(typeInfo.getEnumValue()).to.equal(null); + expect(typeInfo.getFragmentSignature()).to.equal(null); + expect(typeInfo.getFragmentSignatureByName()('')).to.equal(null); + expect(typeInfo.getFragmentArgument()).to.equal(null); }); }); @@ -457,4 +460,216 @@ describe('visitWithTypeInfo', () => { ['leave', 'SelectionSet', null, 'Human', 'Human'], ]); }); + + it('supports traversals of fragment arguments', () => { + const typeInfo = new TypeInfo(testSchema); + + const ast = parse( + ` + query { + ...Foo(x: 4) + } + fragment Foo( + $x: ID! + ) on QueryRoot { + human(id: $x) { name } + } + `, + { experimentalFragmentArguments: true }, + ); + + const visited: Array = []; + visit( + ast, + visitWithTypeInfo(typeInfo, { + enter(node) { + const type = typeInfo.getType(); + const inputType = typeInfo.getInputType(); + visited.push([ + 'enter', + node.kind, + node.kind === 'Name' ? node.value : null, + String(type), + String(inputType), + ]); + }, + leave(node) { + const type = typeInfo.getType(); + const inputType = typeInfo.getInputType(); + visited.push([ + 'leave', + node.kind, + node.kind === 'Name' ? node.value : null, + String(type), + String(inputType), + ]); + }, + }), + ); + + expect(visited).to.deep.equal([ + ['enter', 'Document', null, 'undefined', 'undefined'], + ['enter', 'OperationDefinition', null, 'QueryRoot', 'undefined'], + ['enter', 'SelectionSet', null, 'QueryRoot', 'undefined'], + ['enter', 'FragmentSpread', null, 'QueryRoot', 'undefined'], + ['enter', 'Name', 'Foo', 'QueryRoot', 'undefined'], + ['leave', 'Name', 'Foo', 'QueryRoot', 'undefined'], + ['enter', 'FragmentArgument', null, 'QueryRoot', 'ID!'], + ['enter', 'Name', 'x', 'QueryRoot', 'ID!'], + ['leave', 'Name', 'x', 'QueryRoot', 'ID!'], + ['enter', 'IntValue', null, 'QueryRoot', 'ID!'], + ['leave', 'IntValue', null, 'QueryRoot', 'ID!'], + ['leave', 'FragmentArgument', null, 'QueryRoot', 'ID!'], + ['leave', 'FragmentSpread', null, 'QueryRoot', 'undefined'], + ['leave', 'SelectionSet', null, 'QueryRoot', 'undefined'], + ['leave', 'OperationDefinition', null, 'QueryRoot', 'undefined'], + ['enter', 'FragmentDefinition', null, 'QueryRoot', 'undefined'], + ['enter', 'Name', 'Foo', 'QueryRoot', 'undefined'], + ['leave', 'Name', 'Foo', 'QueryRoot', 'undefined'], + ['enter', 'VariableDefinition', null, 'QueryRoot', 'ID!'], + ['enter', 'Variable', null, 'QueryRoot', 'ID!'], + ['enter', 'Name', 'x', 'QueryRoot', 'ID!'], + ['leave', 'Name', 'x', 'QueryRoot', 'ID!'], + ['leave', 'Variable', null, 'QueryRoot', 'ID!'], + ['enter', 'NonNullType', null, 'QueryRoot', 'ID!'], + ['enter', 'NamedType', null, 'QueryRoot', 'ID!'], + ['enter', 'Name', 'ID', 'QueryRoot', 'ID!'], + ['leave', 'Name', 'ID', 'QueryRoot', 'ID!'], + ['leave', 'NamedType', null, 'QueryRoot', 'ID!'], + ['leave', 'NonNullType', null, 'QueryRoot', 'ID!'], + ['leave', 'VariableDefinition', null, 'QueryRoot', 'ID!'], + ['enter', 'NamedType', null, 'QueryRoot', 'undefined'], + ['enter', 'Name', 'QueryRoot', 'QueryRoot', 'undefined'], + ['leave', 'Name', 'QueryRoot', 'QueryRoot', 'undefined'], + ['leave', 'NamedType', null, 'QueryRoot', 'undefined'], + ['enter', 'SelectionSet', null, 'QueryRoot', 'undefined'], + ['enter', 'Field', null, 'Human', 'undefined'], + ['enter', 'Name', 'human', 'Human', 'undefined'], + ['leave', 'Name', 'human', 'Human', 'undefined'], + ['enter', 'Argument', null, 'Human', 'ID'], + ['enter', 'Name', 'id', 'Human', 'ID'], + ['leave', 'Name', 'id', 'Human', 'ID'], + ['enter', 'Variable', null, 'Human', 'ID'], + ['enter', 'Name', 'x', 'Human', 'ID'], + ['leave', 'Name', 'x', 'Human', 'ID'], + ['leave', 'Variable', null, 'Human', 'ID'], + ['leave', 'Argument', null, 'Human', 'ID'], + ['enter', 'SelectionSet', null, 'Human', 'undefined'], + ['enter', 'Field', null, 'String', 'undefined'], + ['enter', 'Name', 'name', 'String', 'undefined'], + ['leave', 'Name', 'name', 'String', 'undefined'], + ['leave', 'Field', null, 'String', 'undefined'], + ['leave', 'SelectionSet', null, 'Human', 'undefined'], + ['leave', 'Field', null, 'Human', 'undefined'], + ['leave', 'SelectionSet', null, 'QueryRoot', 'undefined'], + ['leave', 'FragmentDefinition', null, 'QueryRoot', 'undefined'], + ['leave', 'Document', null, 'undefined', 'undefined'], + ]); + }); + + it('supports traversals of fragment arguments with default-value', () => { + const typeInfo = new TypeInfo(testSchema); + + const ast = parse( + ` + query { + ...Foo(x: null) + } + fragment Foo( + $x: ID = 4 + ) on QueryRoot { + human(id: $x) { name } + } + `, + { experimentalFragmentArguments: true }, + ); + + const visited: Array = []; + visit( + ast, + visitWithTypeInfo(typeInfo, { + enter(node) { + const type = typeInfo.getType(); + const inputType = typeInfo.getInputType(); + visited.push([ + 'enter', + node.kind, + node.kind === 'Name' ? node.value : null, + String(type), + String(inputType), + ]); + }, + leave(node) { + const type = typeInfo.getType(); + const inputType = typeInfo.getInputType(); + visited.push([ + 'leave', + node.kind, + node.kind === 'Name' ? node.value : null, + String(type), + String(inputType), + ]); + }, + }), + ); + + expect(visited).to.deep.equal([ + ['enter', 'Document', null, 'undefined', 'undefined'], + ['enter', 'OperationDefinition', null, 'QueryRoot', 'undefined'], + ['enter', 'SelectionSet', null, 'QueryRoot', 'undefined'], + ['enter', 'FragmentSpread', null, 'QueryRoot', 'undefined'], + ['enter', 'Name', 'Foo', 'QueryRoot', 'undefined'], + ['leave', 'Name', 'Foo', 'QueryRoot', 'undefined'], + ['enter', 'FragmentArgument', null, 'QueryRoot', 'ID'], + ['enter', 'Name', 'x', 'QueryRoot', 'ID'], + ['leave', 'Name', 'x', 'QueryRoot', 'ID'], + ['enter', 'NullValue', null, 'QueryRoot', 'ID'], + ['leave', 'NullValue', null, 'QueryRoot', 'ID'], + ['leave', 'FragmentArgument', null, 'QueryRoot', 'ID'], + ['leave', 'FragmentSpread', null, 'QueryRoot', 'undefined'], + ['leave', 'SelectionSet', null, 'QueryRoot', 'undefined'], + ['leave', 'OperationDefinition', null, 'QueryRoot', 'undefined'], + ['enter', 'FragmentDefinition', null, 'QueryRoot', 'undefined'], + ['enter', 'Name', 'Foo', 'QueryRoot', 'undefined'], + ['leave', 'Name', 'Foo', 'QueryRoot', 'undefined'], + ['enter', 'VariableDefinition', null, 'QueryRoot', 'ID'], + ['enter', 'Variable', null, 'QueryRoot', 'ID'], + ['enter', 'Name', 'x', 'QueryRoot', 'ID'], + ['leave', 'Name', 'x', 'QueryRoot', 'ID'], + ['leave', 'Variable', null, 'QueryRoot', 'ID'], + ['enter', 'NamedType', null, 'QueryRoot', 'ID'], + ['enter', 'Name', 'ID', 'QueryRoot', 'ID'], + ['leave', 'Name', 'ID', 'QueryRoot', 'ID'], + ['leave', 'NamedType', null, 'QueryRoot', 'ID'], + ['enter', 'IntValue', null, 'QueryRoot', 'ID'], + ['leave', 'IntValue', null, 'QueryRoot', 'ID'], + ['leave', 'VariableDefinition', null, 'QueryRoot', 'ID'], + ['enter', 'NamedType', null, 'QueryRoot', 'undefined'], + ['enter', 'Name', 'QueryRoot', 'QueryRoot', 'undefined'], + ['leave', 'Name', 'QueryRoot', 'QueryRoot', 'undefined'], + ['leave', 'NamedType', null, 'QueryRoot', 'undefined'], + ['enter', 'SelectionSet', null, 'QueryRoot', 'undefined'], + ['enter', 'Field', null, 'Human', 'undefined'], + ['enter', 'Name', 'human', 'Human', 'undefined'], + ['leave', 'Name', 'human', 'Human', 'undefined'], + ['enter', 'Argument', null, 'Human', 'ID'], + ['enter', 'Name', 'id', 'Human', 'ID'], + ['leave', 'Name', 'id', 'Human', 'ID'], + ['enter', 'Variable', null, 'Human', 'ID'], + ['enter', 'Name', 'x', 'Human', 'ID'], + ['leave', 'Name', 'x', 'Human', 'ID'], + ['leave', 'Variable', null, 'Human', 'ID'], + ['leave', 'Argument', null, 'Human', 'ID'], + ['enter', 'SelectionSet', null, 'Human', 'undefined'], + ['enter', 'Field', null, 'String', 'undefined'], + ['enter', 'Name', 'name', 'String', 'undefined'], + ['leave', 'Name', 'name', 'String', 'undefined'], + ['leave', 'Field', null, 'String', 'undefined'], + ['leave', 'SelectionSet', null, 'Human', 'undefined'], + ['leave', 'Field', null, 'Human', 'undefined'], + ['leave', 'SelectionSet', null, 'QueryRoot', 'undefined'], + ['leave', 'FragmentDefinition', null, 'QueryRoot', 'undefined'], + ['leave', 'Document', null, 'undefined', 'undefined'], + ]); + }); }); diff --git a/src/utilities/__tests__/printSchema-test.ts b/src/utilities/__tests__/printSchema-test.ts index 37af4a60f7..8ee29d892c 100644 --- a/src/utilities/__tests__/printSchema-test.ts +++ b/src/utilities/__tests__/printSchema-test.ts @@ -848,9 +848,12 @@ describe('Type System Printer', () => { """Location adjacent to an inline fragment.""" INLINE_FRAGMENT - """Location adjacent to a variable definition.""" + """Location adjacent to an operation variable definition.""" VARIABLE_DEFINITION + """Location adjacent to a fragment variable definition.""" + FRAGMENT_VARIABLE_DEFINITION + """Location adjacent to a schema definition.""" SCHEMA diff --git a/src/utilities/buildASTSchema.ts b/src/utilities/buildASTSchema.ts index eeff08e6ed..02602f5b0b 100644 --- a/src/utilities/buildASTSchema.ts +++ b/src/utilities/buildASTSchema.ts @@ -102,6 +102,7 @@ export function buildSchema( const document = parse(source, { noLocation: options?.noLocation, allowLegacyFragmentVariables: options?.allowLegacyFragmentVariables, + experimentalFragmentArguments: options?.experimentalFragmentArguments, }); return buildASTSchema(document, { diff --git a/src/utilities/getVariableSignature.ts b/src/utilities/getVariableSignature.ts new file mode 100644 index 0000000000..362e39314f --- /dev/null +++ b/src/utilities/getVariableSignature.ts @@ -0,0 +1,46 @@ +import { GraphQLError } from '../error/GraphQLError'; + +import type { VariableDefinitionNode } from '../language/ast'; +import { print } from '../language/printer'; + +import { isInputType } from '../type/definition'; +import type { GraphQLInputType, GraphQLSchema } from '../type/index'; + +import { typeFromAST } from './typeFromAST'; +import { valueFromAST } from './valueFromAST'; + +/** + * A GraphQLVariableSignature is required to coerce a variable value. + * + * Designed to have comparable interface to GraphQLArgument so that + * getArgumentValues() can be reused for fragment arguments. + * */ +export interface GraphQLVariableSignature { + name: string; + type: GraphQLInputType; + defaultValue: unknown; +} + +export function getVariableSignature( + schema: GraphQLSchema, + varDefNode: VariableDefinitionNode, +): GraphQLVariableSignature | GraphQLError { + const varName = varDefNode.variable.name.value; + const varType = typeFromAST(schema, varDefNode.type); + + if (!isInputType(varType)) { + // Must use input types for variables. This should be caught during + // validation, however is checked again here for safety. + const varTypeStr = print(varDefNode.type); + return new GraphQLError( + `Variable "$${varName}" expected value of type "${varTypeStr}" which cannot be used as an input type.`, + { nodes: varDefNode.type }, + ); + } + + return { + name: varName, + type: varType, + defaultValue: valueFromAST(varDefNode.defaultValue, varType), + }; +} diff --git a/src/utilities/valueFromAST.ts b/src/utilities/valueFromAST.ts index 2e6cc1c613..bdc964c175 100644 --- a/src/utilities/valueFromAST.ts +++ b/src/utilities/valueFromAST.ts @@ -39,6 +39,7 @@ export function valueFromAST( valueNode: Maybe, type: GraphQLInputType, variables?: Maybe>, + fragmentVariables?: Maybe>, ): unknown { if (!valueNode) { // When there is no node, then there is also no value. @@ -48,11 +49,12 @@ export function valueFromAST( if (valueNode.kind === Kind.VARIABLE) { const variableName = valueNode.name.value; - if (variables == null || variables[variableName] === undefined) { + const variableValue = + fragmentVariables?.[variableName] ?? variables?.[variableName]; + if (variableValue === undefined) { // No valid return value. return; } - const variableValue = variables[variableName]; if (variableValue === null && isNonNullType(type)) { return; // Invalid: intentionally return no value. } @@ -66,7 +68,7 @@ export function valueFromAST( if (valueNode.kind === Kind.NULL) { return; // Invalid: intentionally return no value. } - return valueFromAST(valueNode, type.ofType, variables); + return valueFromAST(valueNode, type.ofType, variables, fragmentVariables); } if (valueNode.kind === Kind.NULL) { @@ -79,7 +81,7 @@ export function valueFromAST( if (valueNode.kind === Kind.LIST) { const coercedValues = []; for (const itemNode of valueNode.values) { - if (isMissingVariable(itemNode, variables)) { + if (isMissingVariable(itemNode, variables, fragmentVariables)) { // If an array contains a missing variable, it is either coerced to // null or if the item type is non-null, it considered invalid. if (isNonNullType(itemType)) { @@ -87,7 +89,12 @@ export function valueFromAST( } coercedValues.push(null); } else { - const itemValue = valueFromAST(itemNode, itemType, variables); + const itemValue = valueFromAST( + itemNode, + itemType, + variables, + fragmentVariables, + ); if (itemValue === undefined) { return; // Invalid: intentionally return no value. } @@ -96,7 +103,12 @@ export function valueFromAST( } return coercedValues; } - const coercedValue = valueFromAST(valueNode, itemType, variables); + const coercedValue = valueFromAST( + valueNode, + itemType, + variables, + fragmentVariables, + ); if (coercedValue === undefined) { return; // Invalid: intentionally return no value. } @@ -111,7 +123,10 @@ export function valueFromAST( const fieldNodes = keyMap(valueNode.fields, (field) => field.name.value); for (const field of Object.values(type.getFields())) { const fieldNode = fieldNodes[field.name]; - if (!fieldNode || isMissingVariable(fieldNode.value, variables)) { + if ( + !fieldNode || + isMissingVariable(fieldNode.value, variables, fragmentVariables) + ) { if (field.defaultValue !== undefined) { coercedObj[field.name] = field.defaultValue; } else if (isNonNullType(field.type)) { @@ -119,7 +134,12 @@ export function valueFromAST( } continue; } - const fieldValue = valueFromAST(fieldNode.value, field.type, variables); + const fieldValue = valueFromAST( + fieldNode.value, + field.type, + variables, + fragmentVariables, + ); if (fieldValue === undefined) { return; // Invalid: intentionally return no value. } @@ -165,9 +185,12 @@ export function valueFromAST( function isMissingVariable( valueNode: ValueNode, variables: Maybe>, + fragmentVariables: Maybe>, ): boolean { return ( valueNode.kind === Kind.VARIABLE && + (fragmentVariables == null || + fragmentVariables[valueNode.name.value] === undefined) && (variables == null || variables[valueNode.name.value] === undefined) ); } diff --git a/src/validation/ValidationContext.ts b/src/validation/ValidationContext.ts index f6944a6ebd..a96fe5fb97 100644 --- a/src/validation/ValidationContext.ts +++ b/src/validation/ValidationContext.ts @@ -9,6 +9,7 @@ import type { FragmentSpreadNode, OperationDefinitionNode, SelectionSetNode, + VariableDefinitionNode, VariableNode, } from '../language/ast'; import { Kind } from '../language/kinds'; @@ -26,6 +27,7 @@ import type { import type { GraphQLDirective } from '../type/directives'; import type { GraphQLSchema } from '../type/schema'; +import type { FragmentSignature } from '../utilities/TypeInfo'; import { TypeInfo, visitWithTypeInfo } from '../utilities/TypeInfo'; type NodeWithSelectionSet = OperationDefinitionNode | FragmentDefinitionNode; @@ -33,6 +35,7 @@ interface VariableUsage { readonly node: VariableNode; readonly type: Maybe; readonly defaultValue: Maybe; + readonly fragmentVariableDefinition: Maybe; } /** @@ -199,17 +202,42 @@ export class ValidationContext extends ASTValidationContext { let usages = this._variableUsages.get(node); if (!usages) { const newUsages: Array = []; - const typeInfo = new TypeInfo(this._schema); + const typeInfo = new TypeInfo( + this._schema, + undefined, + undefined, + this._typeInfo.getFragmentSignatureByName(), + ); + const fragmentDefinition = + node.kind === Kind.FRAGMENT_DEFINITION ? node : undefined; + visit( node, visitWithTypeInfo(typeInfo, { VariableDefinition: () => false, Variable(variable) { - newUsages.push({ - node: variable, - type: typeInfo.getInputType(), - defaultValue: typeInfo.getDefaultValue(), - }); + let fragmentVariableDefinition; + if (fragmentDefinition) { + const fragmentSignature = typeInfo.getFragmentSignatureByName()( + fragmentDefinition.name.value, + ); + + fragmentVariableDefinition = + fragmentSignature?.variableDefinitions[variable.name.value]; + newUsages.push({ + node: variable, + type: typeInfo.getInputType(), + 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(), + defaultValue: typeInfo.getDefaultValue(), + fragmentVariableDefinition: undefined, + }); + } }, }), ); @@ -261,6 +289,16 @@ export class ValidationContext extends ASTValidationContext { return this._typeInfo.getArgument(); } + getFragmentSignature(): Maybe { + return this._typeInfo.getFragmentSignature(); + } + + getFragmentSignatureByName(): ( + fragmentName: string, + ) => Maybe { + return this._typeInfo.getFragmentSignatureByName(); + } + getEnumValue(): Maybe { return this._typeInfo.getEnumValue(); } diff --git a/src/validation/__tests__/KnownArgumentNamesRule-test.ts b/src/validation/__tests__/KnownArgumentNamesRule-test.ts index 4ce5fd190f..854a75ad66 100644 --- a/src/validation/__tests__/KnownArgumentNamesRule-test.ts +++ b/src/validation/__tests__/KnownArgumentNamesRule-test.ts @@ -97,6 +97,19 @@ describe('Validate: Known argument names', () => { `); }); + it('fragment args are known', () => { + expectValid(` + { + dog { + ...withArg(dogCommand: SIT) + } + } + fragment withArg($dogCommand: DogCommand) on Dog { + doesKnowCommand(dogCommand: $dogCommand) + } + `); + }); + it('field args are invalid', () => { expectErrors(` { @@ -145,6 +158,43 @@ describe('Validate: Known argument names', () => { ]); }); + it('arg passed to fragment without arg is reported', () => { + expectErrors(` + { + dog { + ...withoutArg(unknown: true) + } + } + fragment withoutArg on Dog { + doesKnowCommand + } + `).toDeepEqual([ + { + message: 'Unknown argument "unknown" on fragment "withoutArg".', + locations: [{ line: 4, column: 25 }], + }, + ]); + }); + + it('misspelled fragment args are reported', () => { + expectErrors(` + { + dog { + ...withArg(command: SIT) + } + } + fragment withArg($dogCommand: DogCommand) on Dog { + doesKnowCommand(dogCommand: $dogCommand) + } + `).toDeepEqual([ + { + message: + 'Unknown argument "command" on fragment "withArg". Did you mean "dogCommand"?', + locations: [{ line: 4, column: 22 }], + }, + ]); + }); + it('invalid arg name', () => { expectErrors(` fragment invalidArgName on Dog { diff --git a/src/validation/__tests__/KnownDirectivesRule-test.ts b/src/validation/__tests__/KnownDirectivesRule-test.ts index 4cb6e225c1..652689fd63 100644 --- a/src/validation/__tests__/KnownDirectivesRule-test.ts +++ b/src/validation/__tests__/KnownDirectivesRule-test.ts @@ -44,6 +44,7 @@ const schemaWithDirectives = buildSchema(` directive @onFragmentSpread on FRAGMENT_SPREAD directive @onInlineFragment on INLINE_FRAGMENT directive @onVariableDefinition on VARIABLE_DEFINITION + directive @onFragmentVariableDefinition on FRAGMENT_VARIABLE_DEFINITION `); const schemaWithSDLDirectives = buildSchema(` @@ -150,7 +151,9 @@ describe('Validate: Known directives', () => { someField @onField } - fragment Frag on Human @onFragmentDefinition { + fragment Frag( + $arg: Int @onFragmentVariableDefinition + ) on Human @onFragmentDefinition { name @onField } `); @@ -175,7 +178,7 @@ describe('Validate: Known directives', () => { someField @onQuery } - fragment Frag on Human @onQuery { + fragment Frag($arg: Int @onVariableDefinition) on Human @onQuery { name @onQuery } `).toDeepEqual([ @@ -219,9 +222,14 @@ describe('Validate: Known directives', () => { message: 'Directive "@onQuery" may not be used on FIELD.', locations: [{ column: 19, line: 16 }], }, + { + message: + 'Directive "@onVariableDefinition" may not be used on FRAGMENT_VARIABLE_DEFINITION.', + locations: [{ column: 31, line: 19 }], + }, { message: 'Directive "@onQuery" may not be used on FRAGMENT_DEFINITION.', - locations: [{ column: 30, line: 19 }], + locations: [{ column: 63, line: 19 }], }, { message: 'Directive "@onQuery" may not be used on FIELD.', diff --git a/src/validation/__tests__/NoUndefinedVariablesRule-test.ts b/src/validation/__tests__/NoUndefinedVariablesRule-test.ts index e027d4a49b..ca69fe0789 100644 --- a/src/validation/__tests__/NoUndefinedVariablesRule-test.ts +++ b/src/validation/__tests__/NoUndefinedVariablesRule-test.ts @@ -404,4 +404,67 @@ describe('Validate: No undefined variables', () => { }, ]); }); + + it('fragment defined arguments are not undefined variables', () => { + expectValid(` + query Foo { + ...FragA + } + fragment FragA($a: String) on Type { + field1(a: $a) + } + `); + }); + + it('defined variables used as fragment arguments are not undefined variables', () => { + expectValid(` + query Foo($b: String) { + ...FragA(a: $b) + } + fragment FragA($a: String) on Type { + field1 + } + `); + }); + + it('variables used as fragment arguments may be undefined variables', () => { + expectErrors(` + query Foo { + ...FragA(a: $a) + } + fragment FragA($a: String) on Type { + field1 + } + `).toDeepEqual([ + { + message: 'Variable "$a" is not defined by operation "Foo".', + locations: [ + { line: 3, column: 21 }, + { line: 2, column: 7 }, + ], + }, + ]); + }); + + it('variables shadowed by parent fragment arguments are still undefined variables', () => { + expectErrors(` + query Foo { + ...FragA + } + fragment FragA($a: String) on Type { + ...FragB + } + fragment FragB on Type { + field1(a: $a) + } + `).toDeepEqual([ + { + message: 'Variable "$a" is not defined by operation "Foo".', + locations: [ + { line: 9, column: 19 }, + { line: 2, column: 7 }, + ], + }, + ]); + }); }); diff --git a/src/validation/__tests__/NoUnusedVariablesRule-test.ts b/src/validation/__tests__/NoUnusedVariablesRule-test.ts index 6be63cd23d..dcb94add26 100644 --- a/src/validation/__tests__/NoUnusedVariablesRule-test.ts +++ b/src/validation/__tests__/NoUnusedVariablesRule-test.ts @@ -230,4 +230,42 @@ describe('Validate: No unused variables', () => { }, ]); }); + + it('fragment defined arguments are not unused variables', () => { + expectValid(` + query Foo { + ...FragA + } + fragment FragA($a: String) on Type { + field1(a: $a) + } + `); + }); + + it('defined variables used as fragment arguments are not unused variables', () => { + expectValid(` + query Foo($b: String) { + ...FragA(a: $b) + } + fragment FragA($a: String) on Type { + field1(a: $a) + } + `); + }); + + it('unused fragment variables are reported', () => { + expectErrors(` + query Foo { + ...FragA(a: "value") + } + fragment FragA($a: String) on Type { + field1 + } + `).toDeepEqual([ + { + message: 'Variable "$a" is never used in fragment "FragA".', + locations: [{ line: 5, column: 22 }], + }, + ]); + }); }); diff --git a/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts b/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts index 46cf014e46..99580e417e 100644 --- a/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts +++ b/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts @@ -1138,4 +1138,239 @@ describe('Validate: Overlapping fields can be merged', () => { }, ]); }); + + describe('fragment arguments must produce fields that can be merged', () => { + it('allows conflicting spreads at different depths', () => { + expectValid(` + query ValidDifferingFragmentArgs($command1: DogCommand, $command2: DogCommand) { + dog { + ...DoesKnowCommand(command: $command1) + mother { + ...DoesKnowCommand(command: $command2) + } + } + } + fragment DoesKnowCommand($command: DogCommand) on Dog { + doesKnowCommand(dogCommand: $command) + } + `); + }); + + it('encounters conflict in fragments', () => { + expectErrors(` + { + ...WithArgs(x: 3) + ...WithArgs(x: 4) + } + fragment WithArgs($x: Int) on Type { + a(x: $x) + } + `).toDeepEqual([ + { + message: + 'Spreads "WithArgs" conflict because WithArgs(x: 3) and WithArgs(x: 4) have different fragment arguments.', + locations: [ + { line: 3, column: 11 }, + { line: 4, column: 11 }, + ], + }, + ]); + }); + + it('allows operations with overlapping fields with arguments using identical operation variables', () => { + expectValid(` + query ($y: Int = 1) { + a(x: $y) + ...WithArgs(x: 1) + } + fragment WithArgs($x: Int = 1) on Type { + a(x: $y) + } + `); + }); + + it('allows operations with overlapping fields with identical variable arguments passed via fragment arguments', () => { + expectValid(` + query ($y: Int = 1) { + a(x: $y) + ...WithArgs(x: $y) + } + fragment WithArgs($x: Int) on Type { + a(x: $x) + } + `); + }); + + it('allows operations with overlapping fields with identical variable arguments passed via nested fragment arguments', () => { + expectValid(` + query ($z: Int = 1) { + a(x: $z) + ...WithArgs(y: $z) + } + fragment WithArgs($y: Int) on Type { + ...NestedWithArgs(x: $y) + } + fragment NestedWithArgs($x: Int) on Type { + a(x: $x) + } + `); + }); + + it('allows operations with overlapping fields with identical arguments via fragment variable defaults', () => { + expectValid(` + query { + a(x: 1) + ...WithArgs + } + fragment WithArgs($x: Int = 1) on Type { + a(x: $x) + } + `); + }); + + it('raises errors with overlapping fields with arguments that conflict via operation variables even with defaults and fragment variable defaults', () => { + expectErrors(` + query ($y: Int = 1) { + a(x: $y) + ...WithArgs + } + fragment WithArgs($x: Int = 1) on Type { + a(x: $x) + } + `).toDeepEqual([ + { + message: + 'Fields "a" conflict because they have differing arguments. Use different aliases on the fields to fetch both if this was intentional.', + locations: [ + { line: 3, column: 11 }, + { line: 7, column: 11 }, + ], + }, + ]); + }); + + it('allows operations with overlapping list fields with identical variable arguments passed via fragment arguments', () => { + expectValid(` + query Query($stringListVarY: [String]) { + complicatedArgs { + stringListArgField(stringListArg: $stringListVarY) + ...WithArgs(stringListVarX: $stringListVarY) + } + } + fragment WithArgs($stringListVarX: [String]) on Type { + stringListArgField(stringListArg: $stringListVarX) + } + `); + }); + + it('allows operations with overlapping list fields with identical variable arguments in item position passed via fragment arguments', () => { + expectValid(` + query Query($stringListVarY: [String]) { + complicatedArgs { + stringListArgField(stringListArg: [$stringListVarY]) + ...WithArgs(stringListVarX: $stringListVarY) + } + } + fragment WithArgs($stringListVarX: [String]) on Type { + stringListArgField(stringListArg: [$stringListVarX]) + } + `); + }); + + it('allows operations with overlapping input object fields with identical variable arguments passed via fragment arguments', () => { + expectValid(` + query Query($complexVarY: ComplexInput) { + complicatedArgs { + complexArgField(complexArg: $complexVarY) + ...WithArgs(complexVarX: $complexVarY) + } + } + fragment WithArgs($complexVarX: ComplexInput) on Type { + complexArgField(complexArg: $complexVarX) + } + `); + }); + + it('allows operations with overlapping input object fields with identical variable arguments in field position passed via fragment arguments', () => { + expectValid(` + query Query($boolVarY: Boolean) { + complicatedArgs { + complexArgField(complexArg: {requiredArg: $boolVarY}) + ...WithArgs(boolVarX: $boolVarY) + } + } + fragment WithArgs($boolVarX: Boolean) on Type { + complexArgField(complexArg: {requiredArg: $boolVarX}) + } + `); + }); + + it('encounters nested field conflict in fragments that could otherwise merge', () => { + expectErrors(` + query ValidDifferingFragmentArgs($command1: DogCommand, $command2: DogCommand) { + dog { + ...DoesKnowCommandNested(command: $command1) + mother { + ...DoesKnowCommandNested(command: $command2) + } + } + } + fragment DoesKnowCommandNested($command: DogCommand) on Dog { + doesKnowCommand(dogCommand: $command) + mother { + doesKnowCommand(dogCommand: $command) + } + } + `).toDeepEqual([ + { + message: + 'Fields "mother" conflict because subfields "doesKnowCommand" conflict because they have differing arguments. Use different aliases on the fields to fetch both if this was intentional.', + locations: [ + { line: 5, column: 13 }, + { line: 13, column: 13 }, + { line: 12, column: 11 }, + { line: 11, column: 11 }, + ], + }, + ]); + }); + + it('encounters nested conflict in fragments', () => { + expectErrors(` + { + connection { + edges { + ...WithArgs(x: 3) + } + } + ...Connection + } + fragment Connection on Type { + connection { + edges { + ...WithArgs(x: 4) + } + } + } + fragment WithArgs($x: Int) on Type { + a(x: $x) + } + `).toDeepEqual([ + { + message: + 'Spreads "WithArgs" conflict because WithArgs(x: 3) and WithArgs(x: 4) have different fragment arguments.', + locations: [ + { + column: 15, + line: 5, + }, + { + column: 15, + line: 13, + }, + ], + }, + ]); + }); + }); }); diff --git a/src/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts b/src/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts index 23a272572c..a5f1d235f5 100644 --- a/src/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts +++ b/src/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts @@ -353,4 +353,86 @@ describe('Validate: Provided required arguments', () => { ]); }); }); + + describe('Fragment required arguments', () => { + it('ignores unknown arguments', () => { + expectValid(` + { + ...Foo(unknownArgument: true) + } + fragment Foo on Query { + dog + } + `); + }); + + it('Missing nullable argument with default is allowed', () => { + expectValid(` + { + ...F + } + fragment F($x: Int = 3) on Query { + foo + } + `); + }); + + it('Missing nullable argument is allowed', () => { + expectValid(` + { + ...F + } + fragment F($x: Int) on Query { + foo + } + `); + }); + + it('Missing non-nullable argument with default is allowed', () => { + expectValid(` + { + ...F + } + fragment F($x: Int! = 3) on Query { + foo + } + `); + }); + + it('Missing non-nullable argument is not allowed', () => { + expectErrors(` + { + ...F + } + fragment F($x: Int!) on Query { + foo + } + `).toDeepEqual([ + { + message: + 'Fragment "F" argument "x" of type "Int!" is required, but it was not provided.', + locations: [{ line: 3, column: 13 }], + }, + ]); + }); + + it('Supplies required variables', () => { + expectValid(` + { + ...F(x: 3) + } + fragment F($x: Int!) on Query { + foo + } + `); + }); + + it('Skips missing fragments', () => { + expectValid(` + { + ...Missing(x: 3) + } + `); + }); + }); }); diff --git a/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts b/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts index 3610fa648b..ab9da66ca5 100644 --- a/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts +++ b/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts @@ -1284,4 +1284,35 @@ describe('Validate: Values of correct type', () => { ]); }); }); + + describe('Fragment argument values', () => { + it('list variables with invalid item', () => { + expectErrors(` + fragment InvalidItem($a: [String] = ["one", 2]) on Query { + dog { name } + } + `).toDeepEqual([ + { + message: 'String cannot represent a non string value: 2', + locations: [{ line: 2, column: 53 }], + }, + ]); + }); + + it('fragment spread with invalid argument value', () => { + expectErrors(` + fragment GivesString on Query { + ...ExpectsInt(a: "three") + } + fragment ExpectsInt($a: Int) on Query { + dog { name } + } + `).toDeepEqual([ + { + message: 'Int cannot represent non-integer value: "three"', + locations: [{ line: 3, column: 28 }], + }, + ]); + }); + }); }); diff --git a/src/validation/__tests__/VariablesAreInputTypesRule-test.ts b/src/validation/__tests__/VariablesAreInputTypesRule-test.ts index 7b754fd76f..226613720f 100644 --- a/src/validation/__tests__/VariablesAreInputTypesRule-test.ts +++ b/src/validation/__tests__/VariablesAreInputTypesRule-test.ts @@ -18,6 +18,9 @@ describe('Validate: Variables are input types', () => { query Foo($a: Unknown, $b: [[Unknown!]]!) { field(a: $a, b: $b) } + fragment Bar($a: Unknown, $b: [[Unknown!]]!) on Query { + field(a: $a, b: $b) + } `); }); @@ -26,6 +29,9 @@ describe('Validate: Variables are input types', () => { query Foo($a: String, $b: [Boolean!]!, $c: ComplexInput) { field(a: $a, b: $b, c: $c) } + fragment Bar($a: String, $b: [Boolean!]!, $c: ComplexInput) on Query { + field(a: $a, b: $b, c: $c) + } `); }); @@ -49,4 +55,25 @@ describe('Validate: Variables are input types', () => { }, ]); }); + + it('output types on fragment arguments are invalid', () => { + expectErrors(` + fragment Bar($a: Dog, $b: [[CatOrDog!]]!, $c: Pet) on Query { + field(a: $a, b: $b, c: $c) + } + `).toDeepEqual([ + { + locations: [{ line: 2, column: 24 }], + message: 'Variable "$a" cannot be non-input type "Dog".', + }, + { + locations: [{ line: 2, column: 33 }], + message: 'Variable "$b" cannot be non-input type "[[CatOrDog!]]!".', + }, + { + locations: [{ line: 2, column: 53 }], + message: 'Variable "$c" cannot be non-input type "Pet".', + }, + ]); + }); }); diff --git a/src/validation/__tests__/VariablesInAllowedPositionRule-test.ts b/src/validation/__tests__/VariablesInAllowedPositionRule-test.ts index 090f1680c4..3af5ef4e76 100644 --- a/src/validation/__tests__/VariablesInAllowedPositionRule-test.ts +++ b/src/validation/__tests__/VariablesInAllowedPositionRule-test.ts @@ -356,5 +356,109 @@ describe('Validate: Variables are in allowed positions', () => { dog @include(if: $boolVar) }`); }); + + it('undefined in directive with default value with option', () => { + expectValid(` + { + dog @include(if: $x) + }`); + }); + }); + + describe('Fragment arguments are validated', () => { + it('Boolean => Boolean', () => { + expectValid(` + query Query($booleanArg: Boolean) + { + complicatedArgs { + ...A(b: $booleanArg) + } + } + fragment A($b: Boolean) on ComplicatedArgs { + booleanArgField(booleanArg: $b) + } + `); + }); + + it('Boolean => Boolean with default value', () => { + expectValid(` + query Query($booleanArg: Boolean) + { + complicatedArgs { + ...A(b: $booleanArg) + } + } + fragment A($b: Boolean = true) on ComplicatedArgs { + booleanArgField(booleanArg: $b) + } + `); + }); + + it('Boolean => Boolean!', () => { + expectErrors(` + query Query($ab: Boolean) + { + complicatedArgs { + ...A(b: $ab) + } + } + fragment A($b: Boolean!) on ComplicatedArgs { + booleanArgField(booleanArg: $b) + } + `).toDeepEqual([ + { + message: + 'Variable "$ab" of type "Boolean" used in position expecting type "Boolean!".', + locations: [ + { line: 2, column: 21 }, + { line: 5, column: 21 }, + ], + }, + ]); + }); + + it('Int => Int! fails when variable provides null default value', () => { + expectErrors(` + query Query($intVar: Int = null) { + complicatedArgs { + ...A(i: $intVar) + } + } + fragment A($i: Int!) on ComplicatedArgs { + nonNullIntArgField(nonNullIntArg: $i) + } + `).toDeepEqual([ + { + message: + 'Variable "$intVar" of type "Int" used in position expecting type "Int!".', + locations: [ + { line: 2, column: 21 }, + { line: 4, column: 21 }, + ], + }, + ]); + }); + + it('Int fragment arg => Int! field arg fails even when shadowed by Int! variable', () => { + expectErrors(` + query Query($intVar: Int!) { + complicatedArgs { + ...A(i: $intVar) + } + } + fragment A($intVar: Int) on ComplicatedArgs { + nonNullIntArgField(nonNullIntArg: $intVar) + } + `).toDeepEqual([ + { + message: + 'Variable "$intVar" of type "Int" used in position expecting type "Int!".', + locations: [ + { line: 7, column: 20 }, + { line: 8, column: 45 }, + ], + }, + ]); + }); }); }); diff --git a/src/validation/__tests__/harness.ts b/src/validation/__tests__/harness.ts index ea4840341c..de54b482da 100644 --- a/src/validation/__tests__/harness.ts +++ b/src/validation/__tests__/harness.ts @@ -126,7 +126,7 @@ export function expectValidationErrorsWithSchema( rule: ValidationRule, queryStr: string, ): any { - const doc = parse(queryStr); + const doc = parse(queryStr, { experimentalFragmentArguments: true }); const errors = validate(schema, doc, [rule]); return expectJSON(errors); } diff --git a/src/validation/rules/KnownArgumentNamesRule.ts b/src/validation/rules/KnownArgumentNamesRule.ts index 332b21c1ca..373bb6981b 100644 --- a/src/validation/rules/KnownArgumentNamesRule.ts +++ b/src/validation/rules/KnownArgumentNamesRule.ts @@ -26,6 +26,29 @@ export function KnownArgumentNamesRule(context: ValidationContext): ASTVisitor { return { // eslint-disable-next-line new-cap ...KnownArgumentNamesOnDirectivesRule(context), + FragmentArgument(argNode) { + const fragmentSignature = context.getFragmentSignature(); + if (fragmentSignature) { + const varDef = + fragmentSignature.variableDefinitions[argNode.name.value]; + if (!varDef) { + const argName = argNode.name.value; + const suggestions = suggestionList( + argName, + Array.from( + Object.values(fragmentSignature.variableDefinitions), + ).map((varSignature) => varSignature.variable.name.value), + ); + context.reportError( + new GraphQLError( + `Unknown argument "${argName}" on fragment "${fragmentSignature.definition.name.value}".` + + didYouMean(suggestions), + { nodes: argNode }, + ), + ); + } + } + }, Argument(argNode) { const argDef = context.getArgument(); const fieldDef = context.getFieldDef(); @@ -33,8 +56,10 @@ export function KnownArgumentNamesRule(context: ValidationContext): ASTVisitor { if (!argDef && fieldDef && parentType) { const argName = argNode.name.value; - const knownArgsNames = fieldDef.args.map((arg) => arg.name); - const suggestions = suggestionList(argName, knownArgsNames); + const suggestions = suggestionList( + argName, + fieldDef.args.map((arg) => arg.name), + ); context.reportError( new GraphQLError( `Unknown argument "${argName}" on field "${parentType.name}.${fieldDef.name}".` + diff --git a/src/validation/rules/KnownDirectivesRule.ts b/src/validation/rules/KnownDirectivesRule.ts index f24dbe7d28..8703e97242 100644 --- a/src/validation/rules/KnownDirectivesRule.ts +++ b/src/validation/rules/KnownDirectivesRule.ts @@ -86,8 +86,13 @@ function getDirectiveLocationForASTPath( return DirectiveLocation.INLINE_FRAGMENT; case Kind.FRAGMENT_DEFINITION: return DirectiveLocation.FRAGMENT_DEFINITION; - case Kind.VARIABLE_DEFINITION: - return DirectiveLocation.VARIABLE_DEFINITION; + case Kind.VARIABLE_DEFINITION: { + const parentNode = ancestors[ancestors.length - 3]; + invariant('kind' in parentNode); + return parentNode.kind === Kind.OPERATION_DEFINITION + ? DirectiveLocation.VARIABLE_DEFINITION + : DirectiveLocation.FRAGMENT_VARIABLE_DEFINITION; + } case Kind.SCHEMA_DEFINITION: case Kind.SCHEMA_EXTENSION: return DirectiveLocation.SCHEMA; diff --git a/src/validation/rules/NoUndefinedVariablesRule.ts b/src/validation/rules/NoUndefinedVariablesRule.ts index 3d499b5dcc..c91ede1e84 100644 --- a/src/validation/rules/NoUndefinedVariablesRule.ts +++ b/src/validation/rules/NoUndefinedVariablesRule.ts @@ -25,7 +25,10 @@ export function NoUndefinedVariablesRule( leave(operation) { const usages = context.getRecursiveVariableUsages(operation); - for (const { node } of usages) { + for (const { node, fragmentVariableDefinition } of usages) { + if (fragmentVariableDefinition) { + continue; + } const varName = node.name.value; if (variableNameDefined[varName] !== true) { context.reportError( diff --git a/src/validation/rules/NoUnusedVariablesRule.ts b/src/validation/rules/NoUnusedVariablesRule.ts index 5083af4f28..fb67b35ebf 100644 --- a/src/validation/rules/NoUnusedVariablesRule.ts +++ b/src/validation/rules/NoUnusedVariablesRule.ts @@ -1,6 +1,5 @@ import { GraphQLError } from '../../error/GraphQLError'; -import type { VariableDefinitionNode } from '../../language/ast'; import type { ASTVisitor } from '../../language/visitor'; import type { ValidationContext } from '../ValidationContext'; @@ -14,38 +13,53 @@ import type { ValidationContext } from '../ValidationContext'; * See https://spec.graphql.org/draft/#sec-All-Variables-Used */ export function NoUnusedVariablesRule(context: ValidationContext): ASTVisitor { - let variableDefs: Array = []; - return { - OperationDefinition: { - enter() { - variableDefs = []; - }, - leave(operation) { - const variableNameUsed = Object.create(null); - const usages = context.getRecursiveVariableUsages(operation); - - for (const { node } of usages) { - variableNameUsed[node.name.value] = true; + FragmentDefinition(fragment) { + const usages = context.getVariableUsages(fragment); + const argumentNameUsed = new Set( + usages.map(({ node }) => node.name.value), + ); + // FIXME: https://github.com/graphql/graphql-js/issues/2203 + /* c8 ignore next */ + const variableDefinitions = fragment.variableDefinitions ?? []; + for (const varDef of variableDefinitions) { + const argName = varDef.variable.name.value; + if (!argumentNameUsed.has(argName)) { + context.reportError( + new GraphQLError( + `Variable "$${argName}" is never used in fragment "${fragment.name.value}".`, + { nodes: varDef }, + ), + ); } + } + }, + OperationDefinition(operation) { + const usages = context.getRecursiveVariableUsages(operation); + const operationVariableNameUsed = new Set(); + for (const { node, fragmentVariableDefinition } of usages) { + const varName = node.name.value; + if (!fragmentVariableDefinition) { + operationVariableNameUsed.add(varName); + } + } - for (const variableDef of variableDefs) { - const variableName = variableDef.variable.name.value; - if (variableNameUsed[variableName] !== true) { - context.reportError( - new GraphQLError( - operation.name - ? `Variable "$${variableName}" is never used in operation "${operation.name.value}".` - : `Variable "$${variableName}" is never used.`, - { nodes: variableDef }, - ), - ); - } + // FIXME: https://github.com/graphql/graphql-js/issues/2203 + /* c8 ignore next */ + const variableDefinitions = operation.variableDefinitions ?? []; + for (const variableDef of variableDefinitions) { + const variableName = variableDef.variable.name.value; + if (!operationVariableNameUsed.has(variableName)) { + context.reportError( + new GraphQLError( + operation.name + ? `Variable "$${variableName}" is never used in operation "${operation.name.value}".` + : `Variable "$${variableName}" is never used.`, + { nodes: variableDef }, + ), + ); } - }, - }, - VariableDefinition(def) { - variableDefs.push(def); + } }, }; } diff --git a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts index 4305064a6f..51b5408e58 100644 --- a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts +++ b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts @@ -5,9 +5,11 @@ import type { ObjMap } from '../../jsutils/ObjMap'; import { GraphQLError } from '../../error/GraphQLError'; import type { - DirectiveNode, + ArgumentNode, FieldNode, + FragmentArgumentNode, FragmentDefinitionNode, + FragmentSpreadNode, SelectionSetNode, ValueNode, } from '../../language/ast'; @@ -64,16 +66,16 @@ export function OverlappingFieldsCanBeMergedRule( // dramatically improve the performance of this validator. const comparedFragmentPairs = new PairSet(); - // A cache for the "field map" and list of fragment names found in any given + // A cache for the "field map" and list of fragment spreads found in any given // selection set. Selection sets may be asked for this information multiple // times, so this improves the performance of this validator. - const cachedFieldsAndFragmentNames = new Map(); + const cachedFieldsAndFragmentSpreads = new Map(); return { SelectionSet(selectionSet) { const conflicts = findConflictsWithinSelectionSet( context, - cachedFieldsAndFragmentNames, + cachedFieldsAndFragmentSpreads, comparedFragmentPairs, context.getParentType(), selectionSet, @@ -104,8 +106,16 @@ type NodeAndDef = [ ]; // Map of array of those. type NodeAndDefCollection = ObjMap>; -type FragmentNames = Array; -type FieldsAndFragmentNames = readonly [NodeAndDefCollection, FragmentNames]; +interface FragmentSpread { + key: string; + node: FragmentSpreadNode; + varMap: Map | undefined; +} +type FragmentSpreads = ReadonlyArray; +type FieldsAndFragmentSpreads = readonly [ + NodeAndDefCollection, + FragmentSpreads, +]; /** * Algorithm: @@ -167,56 +177,60 @@ type FieldsAndFragmentNames = readonly [NodeAndDefCollection, FragmentNames]; // GraphQL Document. function findConflictsWithinSelectionSet( context: ValidationContext, - cachedFieldsAndFragmentNames: Map, + cachedFieldsAndFragmentSpreads: Map< + SelectionSetNode, + FieldsAndFragmentSpreads + >, comparedFragmentPairs: PairSet, parentType: Maybe, selectionSet: SelectionSetNode, ): Array { const conflicts: Array = []; - const [fieldMap, fragmentNames] = getFieldsAndFragmentNames( + const [fieldMap, fragmentSpreads] = getFieldsAndFragmentSpreads( context, - cachedFieldsAndFragmentNames, + cachedFieldsAndFragmentSpreads, parentType, selectionSet, + undefined, ); - // (A) Find find all conflicts "within" the fields of this selection set. + // (A) First find all conflicts "within" the fields and fragment-spreads of this selection set. // Note: this is the *only place* `collectConflictsWithin` is called. collectConflictsWithin( context, conflicts, - cachedFieldsAndFragmentNames, + cachedFieldsAndFragmentSpreads, comparedFragmentPairs, fieldMap, ); - if (fragmentNames.length !== 0) { + if (fragmentSpreads.length !== 0) { // (B) Then collect conflicts between these fields and those represented by // each spread fragment name found. - for (let i = 0; i < fragmentNames.length; i++) { + for (let i = 0; i < fragmentSpreads.length; i++) { collectConflictsBetweenFieldsAndFragment( context, conflicts, - cachedFieldsAndFragmentNames, + cachedFieldsAndFragmentSpreads, comparedFragmentPairs, false, fieldMap, - fragmentNames[i], + fragmentSpreads[i], ); // (C) Then compare this fragment with all other fragments found in this // selection set to collect conflicts between fragments spread together. // This compares each item in the list of fragment names to every other // item in that same list (except for itself). - for (let j = i + 1; j < fragmentNames.length; j++) { + for (let j = i + 1; j < fragmentSpreads.length; j++) { collectConflictsBetweenFragments( context, conflicts, - cachedFieldsAndFragmentNames, + cachedFieldsAndFragmentSpreads, comparedFragmentPairs, false, - fragmentNames[i], - fragmentNames[j], + fragmentSpreads[i], + fragmentSpreads[j], ); } } @@ -229,22 +243,27 @@ function findConflictsWithinSelectionSet( function collectConflictsBetweenFieldsAndFragment( context: ValidationContext, conflicts: Array, - cachedFieldsAndFragmentNames: Map, + cachedFieldsAndFragmentSpreads: Map< + SelectionSetNode, + FieldsAndFragmentSpreads + >, comparedFragmentPairs: PairSet, areMutuallyExclusive: boolean, fieldMap: NodeAndDefCollection, - fragmentName: string, + fragmentSpread: FragmentSpread, ): void { + const fragmentName = fragmentSpread.node.name.value; const fragment = context.getFragment(fragmentName); if (!fragment) { return; } const [fieldMap2, referencedFragmentNames] = - getReferencedFieldsAndFragmentNames( + getReferencedFieldsAndFragmentSpreads( context, - cachedFieldsAndFragmentNames, + cachedFieldsAndFragmentSpreads, fragment, + fragmentSpread.varMap, ); // Do not compare a fragment's fieldMap to itself. @@ -257,40 +276,44 @@ function collectConflictsBetweenFieldsAndFragment( collectConflictsBetween( context, conflicts, - cachedFieldsAndFragmentNames, + cachedFieldsAndFragmentSpreads, comparedFragmentPairs, areMutuallyExclusive, fieldMap, + undefined, fieldMap2, + fragmentSpread.varMap, ); // (E) Then collect any conflicts between the provided collection of fields // and any fragment names found in the given fragment. - for (const referencedFragmentName of referencedFragmentNames) { + for (const referencedFragmentSpread of Object.values( + referencedFragmentNames, + )) { // Memoize so two fragments are not compared for conflicts more than once. if ( comparedFragmentPairs.has( - referencedFragmentName, - fragmentName, + referencedFragmentSpread.key, + fragmentSpread.key, areMutuallyExclusive, ) ) { continue; } comparedFragmentPairs.add( - referencedFragmentName, - fragmentName, + referencedFragmentSpread.key, + fragmentSpread.key, areMutuallyExclusive, ); collectConflictsBetweenFieldsAndFragment( context, conflicts, - cachedFieldsAndFragmentNames, + cachedFieldsAndFragmentSpreads, comparedFragmentPairs, areMutuallyExclusive, fieldMap, - referencedFragmentName, + referencedFragmentSpread, ); } } @@ -300,46 +323,73 @@ function collectConflictsBetweenFieldsAndFragment( function collectConflictsBetweenFragments( context: ValidationContext, conflicts: Array, - cachedFieldsAndFragmentNames: Map, + cachedFieldsAndFragmentSpreads: Map< + SelectionSetNode, + FieldsAndFragmentSpreads + >, comparedFragmentPairs: PairSet, areMutuallyExclusive: boolean, - fragmentName1: string, - fragmentName2: string, + fragmentSpread1: FragmentSpread, + fragmentSpread2: FragmentSpread, ): void { // No need to compare a fragment to itself. - if (fragmentName1 === fragmentName2) { + if (fragmentSpread1.key === fragmentSpread2.key) { return; } - // Memoize so two fragments are not compared for conflicts more than once. + if (fragmentSpread1.node.name.value === fragmentSpread2.node.name.value) { + if ( + !sameArguments( + fragmentSpread1.node.arguments, + fragmentSpread1.varMap, + fragmentSpread2.node.arguments, + fragmentSpread2.varMap, + ) + ) { + context.reportError( + new GraphQLError( + `Spreads "${fragmentSpread1.node.name.value}" conflict because ${fragmentSpread1.key} and ${fragmentSpread2.key} have different fragment arguments.`, + { nodes: [fragmentSpread1.node, fragmentSpread2.node] }, + ), + ); + return; + } + } + if ( comparedFragmentPairs.has( - fragmentName1, - fragmentName2, + fragmentSpread1.key, + fragmentSpread2.key, areMutuallyExclusive, ) ) { return; } - comparedFragmentPairs.add(fragmentName1, fragmentName2, areMutuallyExclusive); + comparedFragmentPairs.add( + fragmentSpread1.key, + fragmentSpread2.key, + areMutuallyExclusive, + ); - const fragment1 = context.getFragment(fragmentName1); - const fragment2 = context.getFragment(fragmentName2); + const fragment1 = context.getFragment(fragmentSpread1.node.name.value); + const fragment2 = context.getFragment(fragmentSpread2.node.name.value); if (!fragment1 || !fragment2) { return; } - const [fieldMap1, referencedFragmentNames1] = - getReferencedFieldsAndFragmentNames( + const [fieldMap1, referencedFragmentSpreads1] = + getReferencedFieldsAndFragmentSpreads( context, - cachedFieldsAndFragmentNames, + cachedFieldsAndFragmentSpreads, fragment1, + fragmentSpread1.varMap, ); - const [fieldMap2, referencedFragmentNames2] = - getReferencedFieldsAndFragmentNames( + const [fieldMap2, referencedFragmentSpreads2] = + getReferencedFieldsAndFragmentSpreads( context, - cachedFieldsAndFragmentNames, + cachedFieldsAndFragmentSpreads, fragment2, + fragmentSpread2.varMap, ); // (F) First, collect all conflicts between these two collections of fields @@ -347,38 +397,40 @@ function collectConflictsBetweenFragments( collectConflictsBetween( context, conflicts, - cachedFieldsAndFragmentNames, + cachedFieldsAndFragmentSpreads, comparedFragmentPairs, areMutuallyExclusive, fieldMap1, + fragmentSpread1.varMap, fieldMap2, + fragmentSpread2.varMap, ); // (G) Then collect conflicts between the first fragment and any nested // fragments spread in the second fragment. - for (const referencedFragmentName2 of referencedFragmentNames2) { + for (const referencedFragmentSpread2 of referencedFragmentSpreads2) { collectConflictsBetweenFragments( context, conflicts, - cachedFieldsAndFragmentNames, + cachedFieldsAndFragmentSpreads, comparedFragmentPairs, areMutuallyExclusive, - fragmentName1, - referencedFragmentName2, + fragmentSpread1, + referencedFragmentSpread2, ); } // (G) Then collect conflicts between the second fragment and any nested // fragments spread in the first fragment. - for (const referencedFragmentName1 of referencedFragmentNames1) { + for (const referencedFragmentSpread1 of referencedFragmentSpreads1) { collectConflictsBetweenFragments( context, conflicts, - cachedFieldsAndFragmentNames, + cachedFieldsAndFragmentSpreads, comparedFragmentPairs, areMutuallyExclusive, - referencedFragmentName1, - fragmentName2, + referencedFragmentSpread1, + fragmentSpread2, ); } } @@ -388,81 +440,90 @@ function collectConflictsBetweenFragments( // between the sub-fields of two overlapping fields. function findConflictsBetweenSubSelectionSets( context: ValidationContext, - cachedFieldsAndFragmentNames: Map, + cachedFieldsAndFragmentSpreads: Map< + SelectionSetNode, + FieldsAndFragmentSpreads + >, comparedFragmentPairs: PairSet, areMutuallyExclusive: boolean, parentType1: Maybe, selectionSet1: SelectionSetNode, + varMap1: Map | undefined, parentType2: Maybe, selectionSet2: SelectionSetNode, + varMap2: Map | undefined, ): Array { const conflicts: Array = []; - const [fieldMap1, fragmentNames1] = getFieldsAndFragmentNames( + const [fieldMap1, fragmentSpreads1] = getFieldsAndFragmentSpreads( context, - cachedFieldsAndFragmentNames, + cachedFieldsAndFragmentSpreads, parentType1, selectionSet1, + varMap1, ); - const [fieldMap2, fragmentNames2] = getFieldsAndFragmentNames( + const [fieldMap2, fragmentSpreads2] = getFieldsAndFragmentSpreads( context, - cachedFieldsAndFragmentNames, + cachedFieldsAndFragmentSpreads, parentType2, selectionSet2, + varMap2, ); // (H) First, collect all conflicts between these two collections of field. collectConflictsBetween( context, conflicts, - cachedFieldsAndFragmentNames, + cachedFieldsAndFragmentSpreads, comparedFragmentPairs, areMutuallyExclusive, fieldMap1, + varMap1, fieldMap2, + varMap2, ); // (I) Then collect conflicts between the first collection of fields and // those referenced by each fragment name associated with the second. - for (const fragmentName2 of fragmentNames2) { + for (const fragmentSpread2 of fragmentSpreads2) { collectConflictsBetweenFieldsAndFragment( context, conflicts, - cachedFieldsAndFragmentNames, + cachedFieldsAndFragmentSpreads, comparedFragmentPairs, areMutuallyExclusive, fieldMap1, - fragmentName2, + fragmentSpread2, ); } // (I) Then collect conflicts between the second collection of fields and // those referenced by each fragment name associated with the first. - for (const fragmentName1 of fragmentNames1) { + for (const fragmentSpread1 of fragmentSpreads1) { collectConflictsBetweenFieldsAndFragment( context, conflicts, - cachedFieldsAndFragmentNames, + cachedFieldsAndFragmentSpreads, comparedFragmentPairs, areMutuallyExclusive, fieldMap2, - fragmentName1, + fragmentSpread1, ); } - // (J) Also collect conflicts between any fragment names by the first and - // fragment names by the second. This compares each item in the first set of - // names to each item in the second set of names. - for (const fragmentName1 of fragmentNames1) { - for (const fragmentName2 of fragmentNames2) { + // (J) Also collect conflicts between any fragment spreads by the first and + // fragment spreads by the second. This compares each item in the first set of + // spreads to each item in the second set of spreads. + for (const fragmentSpread1 of fragmentSpreads1) { + for (const fragmentSpread2 of fragmentSpreads2) { collectConflictsBetweenFragments( context, conflicts, - cachedFieldsAndFragmentNames, + cachedFieldsAndFragmentSpreads, comparedFragmentPairs, areMutuallyExclusive, - fragmentName1, - fragmentName2, + fragmentSpread1, + fragmentSpread2, ); } } @@ -473,7 +534,10 @@ function findConflictsBetweenSubSelectionSets( function collectConflictsWithin( context: ValidationContext, conflicts: Array, - cachedFieldsAndFragmentNames: Map, + cachedFieldsAndFragmentSpreads: Map< + SelectionSetNode, + FieldsAndFragmentSpreads + >, comparedFragmentPairs: PairSet, fieldMap: NodeAndDefCollection, ): void { @@ -490,12 +554,14 @@ function collectConflictsWithin( for (let j = i + 1; j < fields.length; j++) { const conflict = findConflict( context, - cachedFieldsAndFragmentNames, + cachedFieldsAndFragmentSpreads, comparedFragmentPairs, false, // within one collection is never mutually exclusive responseName, fields[i], + undefined, fields[j], + undefined, ); if (conflict) { conflicts.push(conflict); @@ -514,11 +580,16 @@ function collectConflictsWithin( function collectConflictsBetween( context: ValidationContext, conflicts: Array, - cachedFieldsAndFragmentNames: Map, + cachedFieldsAndFragmentSpreads: Map< + SelectionSetNode, + FieldsAndFragmentSpreads + >, comparedFragmentPairs: PairSet, parentFieldsAreMutuallyExclusive: boolean, fieldMap1: NodeAndDefCollection, + varMap1: Map | undefined, fieldMap2: NodeAndDefCollection, + varMap2: Map | undefined, ): void { // A field map is a keyed collection, where each key represents a response // name and the value at that key is a list of all fields which provide that @@ -532,12 +603,14 @@ function collectConflictsBetween( for (const field2 of fields2) { const conflict = findConflict( context, - cachedFieldsAndFragmentNames, + cachedFieldsAndFragmentSpreads, comparedFragmentPairs, parentFieldsAreMutuallyExclusive, responseName, field1, + varMap1, field2, + varMap2, ); if (conflict) { conflicts.push(conflict); @@ -552,12 +625,17 @@ function collectConflictsBetween( // comparing their sub-fields. function findConflict( context: ValidationContext, - cachedFieldsAndFragmentNames: Map, + cachedFieldsAndFragmentSpreads: Map< + SelectionSetNode, + FieldsAndFragmentSpreads + >, comparedFragmentPairs: PairSet, parentFieldsAreMutuallyExclusive: boolean, responseName: string, field1: NodeAndDef, + varMap1: Map | undefined, field2: NodeAndDef, + varMap2: Map | undefined, ): Maybe { const [parentType1, node1, def1] = field1; const [parentType2, node2, def2] = field2; @@ -589,7 +667,7 @@ function findConflict( } // Two field calls must have the same arguments. - if (!sameArguments(node1, node2)) { + if (!sameArguments(node1.arguments, varMap1, node2.arguments, varMap2)) { return [ [responseName, 'they have differing arguments'], [node1], @@ -615,7 +693,7 @@ function findConflict( ]; } - // Collect and compare sub-fields. Use the same "visited fragment names" list + // Collect and compare sub-fields. Use the same "visited fragment spreads" list // for both collections so fields in a fragment reference are never // compared to themselves. const selectionSet1 = node1.selectionSet; @@ -623,25 +701,26 @@ function findConflict( if (selectionSet1 && selectionSet2) { const conflicts = findConflictsBetweenSubSelectionSets( context, - cachedFieldsAndFragmentNames, + cachedFieldsAndFragmentSpreads, comparedFragmentPairs, areMutuallyExclusive, getNamedType(type1), selectionSet1, + varMap1, getNamedType(type2), selectionSet2, + varMap2, ); return subfieldConflicts(conflicts, responseName, node1, node2); } } -function sameArguments( - node1: FieldNode | DirectiveNode, - node2: FieldNode | DirectiveNode, +function sameArguments( + args1: ReadonlyArray | undefined, + varMap1: Map | undefined, + args2: ReadonlyArray | undefined, + varMap2: Map | undefined, ): boolean { - const args1 = node1.arguments; - const args2 = node2.arguments; - if (args1 === undefined || args1.length === 0) { return args2 === undefined || args2.length === 0; } @@ -656,9 +735,17 @@ function sameArguments( /* c8 ignore next */ } - const values2 = new Map(args2.map(({ name, value }) => [name.value, value])); + const values2 = new Map( + args2.map(({ name, value }) => [ + name.value, + varMap2 === undefined ? value : replaceFragmentVariables(value, varMap2), + ]), + ); return args1.every((arg1) => { - const value1 = arg1.value; + let value1 = arg1.value; + if (varMap1) { + value1 = replaceFragmentVariables(value1, varMap1); + } const value2 = values2.get(arg1.name.value); if (value2 === undefined) { return false; @@ -668,6 +755,34 @@ function sameArguments( }); } +function replaceFragmentVariables( + valueNode: ValueNode, + varMap: ReadonlyMap, +): ValueNode { + switch (valueNode.kind) { + case Kind.VARIABLE: + return varMap.get(valueNode.name.value) ?? valueNode; + case Kind.LIST: + return { + ...valueNode, + values: valueNode.values.map((node) => + replaceFragmentVariables(node, varMap), + ), + }; + case Kind.OBJECT: + return { + ...valueNode, + fields: valueNode.fields.map((field) => ({ + ...field, + value: replaceFragmentVariables(field.value, varMap), + })), + }; + default: { + return valueNode; + } + } +} + function stringifyValue(value: ValueNode): string | null { return print(sortValueNode(value)); } @@ -704,49 +819,61 @@ function doTypesConflict( // Given a selection set, return the collection of fields (a mapping of response // name to field nodes and definitions) as well as a list of fragment names // referenced via fragment spreads. -function getFieldsAndFragmentNames( +function getFieldsAndFragmentSpreads( context: ValidationContext, - cachedFieldsAndFragmentNames: Map, + cachedFieldsAndFragmentSpreads: Map< + SelectionSetNode, + FieldsAndFragmentSpreads + >, parentType: Maybe, selectionSet: SelectionSetNode, -): FieldsAndFragmentNames { - const cached = cachedFieldsAndFragmentNames.get(selectionSet); + varMap: Map | undefined, +): FieldsAndFragmentSpreads { + const cached = cachedFieldsAndFragmentSpreads.get(selectionSet); if (cached) { return cached; } const nodeAndDefs: NodeAndDefCollection = Object.create(null); - const fragmentNames: ObjMap = Object.create(null); + const fragmentSpreads: ObjMap = Object.create(null); _collectFieldsAndFragmentNames( context, parentType, selectionSet, nodeAndDefs, - fragmentNames, + fragmentSpreads, + varMap, ); - const result = [nodeAndDefs, Object.keys(fragmentNames)] as const; - cachedFieldsAndFragmentNames.set(selectionSet, result); + const result: FieldsAndFragmentSpreads = [ + nodeAndDefs, + Array.from(Object.values(fragmentSpreads)), + ]; + cachedFieldsAndFragmentSpreads.set(selectionSet, result); return result; } // Given a reference to a fragment, return the represented collection of fields // as well as a list of nested fragment names referenced via fragment spreads. -function getReferencedFieldsAndFragmentNames( +function getReferencedFieldsAndFragmentSpreads( context: ValidationContext, - cachedFieldsAndFragmentNames: Map, + cachedFieldsAndFragmentSpreads: Map< + SelectionSetNode, + FieldsAndFragmentSpreads + >, fragment: FragmentDefinitionNode, + varMap: Map | undefined, ) { - // Short-circuit building a type from the node if possible. - const cached = cachedFieldsAndFragmentNames.get(fragment.selectionSet); + const cached = cachedFieldsAndFragmentSpreads.get(fragment.selectionSet); if (cached) { return cached; } const fragmentType = typeFromAST(context.getSchema(), fragment.typeCondition); - return getFieldsAndFragmentNames( + return getFieldsAndFragmentSpreads( context, - cachedFieldsAndFragmentNames, + cachedFieldsAndFragmentSpreads, fragmentType, fragment.selectionSet, + varMap, ); } @@ -755,7 +882,8 @@ function _collectFieldsAndFragmentNames( parentType: Maybe, selectionSet: SelectionSetNode, nodeAndDefs: NodeAndDefCollection, - fragmentNames: ObjMap, + fragmentSpreads: ObjMap, + varMap: Map | undefined, ): void { for (const selection of selectionSet.selections) { switch (selection.kind) { @@ -774,9 +902,11 @@ function _collectFieldsAndFragmentNames( nodeAndDefs[responseName].push([parentType, selection, fieldDef]); break; } - case Kind.FRAGMENT_SPREAD: - fragmentNames[selection.name.value] = true; + case Kind.FRAGMENT_SPREAD: { + const fragmentSpread = getFragmentSpread(context, selection, varMap); + fragmentSpreads[fragmentSpread.key] = fragmentSpread; break; + } case Kind.INLINE_FRAGMENT: { const typeCondition = selection.typeCondition; const inlineFragmentType = typeCondition @@ -787,7 +917,8 @@ function _collectFieldsAndFragmentNames( inlineFragmentType, selection.selectionSet, nodeAndDefs, - fragmentNames, + fragmentSpreads, + varMap, ); break; } @@ -795,6 +926,50 @@ function _collectFieldsAndFragmentNames( } } +function getFragmentSpread( + context: ValidationContext, + fragmentSpreadNode: FragmentSpreadNode, + varMap: Map | undefined, +): FragmentSpread { + let key = ''; + const newVarMap = new Map(); + const fragmentSignature = context.getFragmentSignatureByName()( + fragmentSpreadNode.name.value, + ); + const argMap = new Map(); + if (fragmentSpreadNode.arguments) { + for (const arg of fragmentSpreadNode.arguments) { + argMap.set(arg.name.value, arg.value); + } + } + if (fragmentSignature?.variableDefinitions) { + key += fragmentSpreadNode.name.value + '('; + for (const [varName, variable] of Object.entries( + fragmentSignature.variableDefinitions, + )) { + const value = argMap.get(varName); + if (value) { + key += varName + ': ' + print(sortValueNode(value)); + } + const arg = argMap.get(varName); + if (arg !== undefined) { + newVarMap.set( + varName, + varMap !== undefined ? replaceFragmentVariables(arg, varMap) : arg, + ); + } else if (variable.defaultValue) { + newVarMap.set(varName, variable.defaultValue); + } + } + key += ')'; + } + return { + key, + node: fragmentSpreadNode, + varMap: newVarMap.size > 0 ? newVarMap : undefined, + }; +} + // Given a series of Conflicts which occurred between two sub-fields, generate // a single Conflict. function subfieldConflicts( diff --git a/src/validation/rules/ProvidedRequiredArgumentsRule.ts b/src/validation/rules/ProvidedRequiredArgumentsRule.ts index b111dcee1b..e6a425e970 100644 --- a/src/validation/rules/ProvidedRequiredArgumentsRule.ts +++ b/src/validation/rules/ProvidedRequiredArgumentsRule.ts @@ -4,7 +4,10 @@ import type { ObjMap } from '../../jsutils/ObjMap'; import { GraphQLError } from '../../error/GraphQLError'; -import type { InputValueDefinitionNode } from '../../language/ast'; +import type { + InputValueDefinitionNode, + VariableDefinitionNode, +} from '../../language/ast'; import { Kind } from '../../language/kinds'; import { print } from '../../language/printer'; import type { ASTVisitor } from '../../language/visitor'; @@ -13,6 +16,8 @@ import type { GraphQLArgument } from '../../type/definition'; import { isRequiredArgument, isType } from '../../type/definition'; import { specifiedDirectives } from '../../type/directives'; +import { typeFromAST } from '../../utilities/typeFromAST'; + import type { SDLValidationContext, ValidationContext, @@ -37,7 +42,6 @@ export function ProvidedRequiredArgumentsRule( if (!fieldDef) { return false; } - const providedArgs = new Set( // FIXME: https://github.com/graphql/graphql-js/issues/2203 /* c8 ignore next */ @@ -56,6 +60,41 @@ export function ProvidedRequiredArgumentsRule( } }, }, + FragmentSpread: { + // Validate on leave to allow for deeper errors to appear first. + leave(spreadNode) { + const fragmentSignature = context.getFragmentSignature(); + if (!fragmentSignature) { + return false; + } + + const providedArgs = new Set( + // FIXME: https://github.com/graphql/graphql-js/issues/2203 + /* c8 ignore next */ + spreadNode.arguments?.map((arg) => arg.name.value), + ); + for (const [varName, variableDefinition] of Object.entries( + fragmentSignature.variableDefinitions, + )) { + if ( + !providedArgs.has(varName) && + isRequiredArgumentNode(variableDefinition) + ) { + const type = typeFromAST( + context.getSchema(), + variableDefinition.type, + ); + const argTypeStr = inspect(type); + context.reportError( + new GraphQLError( + `Fragment "${spreadNode.name.value}" argument "${varName}" of type "${argTypeStr}" is required, but it was not provided.`, + { nodes: spreadNode }, + ), + ); + } + } + }, + }, }; } @@ -122,6 +161,8 @@ export function ProvidedRequiredArgumentsOnDirectivesRule( }; } -function isRequiredArgumentNode(arg: InputValueDefinitionNode): boolean { +function isRequiredArgumentNode( + arg: InputValueDefinitionNode | VariableDefinitionNode, +): boolean { return arg.type.kind === Kind.NON_NULL_TYPE && arg.defaultValue == null; } diff --git a/src/validation/rules/SingleFieldSubscriptionsRule.ts b/src/validation/rules/SingleFieldSubscriptionsRule.ts index 21cb1abaf6..d8e40e4499 100644 --- a/src/validation/rules/SingleFieldSubscriptionsRule.ts +++ b/src/validation/rules/SingleFieldSubscriptionsRule.ts @@ -2,13 +2,11 @@ import type { ObjMap } from '../../jsutils/ObjMap'; import { GraphQLError } from '../../error/GraphQLError'; -import type { - FragmentDefinitionNode, - OperationDefinitionNode, -} from '../../language/ast'; +import type { OperationDefinitionNode } from '../../language/ast'; import { Kind } from '../../language/kinds'; import type { ASTVisitor } from '../../language/visitor'; +import type { FragmentDetails } from '../../execution/collectFields'; import { collectFields } from '../../execution/collectFields'; import type { ValidationContext } from '../ValidationContext'; @@ -35,10 +33,10 @@ export function SingleFieldSubscriptionsRule( [variable: string]: any; } = Object.create(null); const document = context.getDocument(); - const fragments: ObjMap = Object.create(null); + const fragments: ObjMap = Object.create(null); for (const definition of document.definitions) { if (definition.kind === Kind.FRAGMENT_DEFINITION) { - fragments[definition.name.value] = definition; + fragments[definition.name.value] = { definition }; } } const fields = collectFields( @@ -57,20 +55,20 @@ export function SingleFieldSubscriptionsRule( operationName != null ? `Subscription "${operationName}" must select only one top level field.` : 'Anonymous Subscription must select only one top level field.', - { nodes: extraFieldSelections }, + { nodes: extraFieldSelections.map((entry) => entry.node) }, ), ); } for (const fieldNodes of fields.values()) { const field = fieldNodes[0]; - const fieldName = field.name.value; + const fieldName = field.node.name.value; if (fieldName.startsWith('__')) { context.reportError( new GraphQLError( operationName != null ? `Subscription "${operationName}" must not select an introspection top level field.` : 'Anonymous Subscription must not select an introspection top level field.', - { nodes: fieldNodes }, + { nodes: fieldNodes.map((entry) => entry.node) }, ), ); } diff --git a/src/validation/rules/VariablesInAllowedPositionRule.ts b/src/validation/rules/VariablesInAllowedPositionRule.ts index a0b7e991a6..0b616d2aa9 100644 --- a/src/validation/rules/VariablesInAllowedPositionRule.ts +++ b/src/validation/rules/VariablesInAllowedPositionRule.ts @@ -36,9 +36,18 @@ export function VariablesInAllowedPositionRule( leave(operation) { const usages = context.getRecursiveVariableUsages(operation); - for (const { node, type, defaultValue } of usages) { + for (const { + node, + type, + defaultValue, + fragmentVariableDefinition, + } of usages) { const varName = node.name.value; - const varDef = varDefMap[varName]; + let varDef = fragmentVariableDefinition; + if (!varDef) { + varDef = varDefMap[varName]; + } + if (varDef && type) { // A var type is allowed if it is the same or more strict (e.g. is // a subtype of) than the expected type. It can be more strict if