From b633f61eb457d952239370d1086886203aefdc16 Mon Sep 17 00:00:00 2001 From: James Bellenger Date: Tue, 17 Feb 2026 04:21:07 -0800 Subject: [PATCH 1/5] check oneof inhabitability --- src/type/__tests__/validation-test.ts | 193 ++++++++++++++++++++++++++ src/type/validate.ts | 65 +++++++++ 2 files changed, 258 insertions(+) diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index c1ed85d7b6..7c7a24660c 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -2409,6 +2409,199 @@ describe('Type System: OneOf Input Object fields must be nullable', () => { }); }); +describe('Type System: OneOf Input Objects must be inhabitable', () => { + it('accepts a OneOf Input Object with a scalar field', () => { + const schema = buildSchema(` + type Query { + test(arg: A): String + } + + input A @oneOf { + a: String + b: Int + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a OneOf Input Object with an enum field', () => { + const schema = buildSchema(` + type Query { + test(arg: A): String + } + + enum Color { RED GREEN BLUE } + + input A @oneOf { + a: Color + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a OneOf Input Object with a list field', () => { + const schema = buildSchema(` + type Query { + test(arg: A): String + } + + input A @oneOf { + a: [A] + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a OneOf Input Object referencing a non-OneOf input object', () => { + const schema = buildSchema(` + type Query { + test(arg: A): String + } + + input A @oneOf { + a: RegularInput + } + + input RegularInput { + x: String + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a OneOf Input Object with at least one escape field', () => { + const schema = buildSchema(` + type Query { + test(arg: A): String + } + + input A @oneOf { + b: B + escape: String + } + + input B @oneOf { + a: A + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts mutually referencing OneOf types where one has a scalar escape', () => { + const schema = buildSchema(` + type Query { + test(arg: A): String + } + + input A @oneOf { + b: B + } + + input B @oneOf { + a: A + escape: Int + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a OneOf referencing a non-OneOf which references back', () => { + const schema = buildSchema(` + type Query { + test(arg: A): String + } + + input A @oneOf { + b: RegularInput + } + + input RegularInput { + back: A + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a OneOf with multiple fields where one escapes through chained OneOf types', () => { + const schema = buildSchema(` + type Query { + test(arg: A): String + } + + input A @oneOf { + b: B + c: C + } + + input B @oneOf { + a: A + } + + input C @oneOf { + a: A + escape: String + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('rejects a closed subgraph of one OneOf type', () => { + const schema = buildSchema(` + type Query { + test(arg: A): String + } + + input A @oneOf { + self: A + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'OneOf Input Object A must be inhabitable but all fields recursively reference only other OneOf Input Objects forming an unresolvable cycle.', + locations: [{ line: 6, column: 7 }], + }, + ]); + }); + + it('rejects a closed subgraph of multiple OneOf types', () => { + const schema = buildSchema(` + type Query { + test(arg: A): String + } + + input A @oneOf { + b: B + } + + input B @oneOf { + c: C + } + + input C @oneOf { + a: A + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'OneOf Input Object A must be inhabitable but all fields recursively reference only other OneOf Input Objects forming an unresolvable cycle.', + locations: [{ line: 6, column: 7 }], + }, + { + message: + 'OneOf Input Object B must be inhabitable but all fields recursively reference only other OneOf Input Objects forming an unresolvable cycle.', + locations: [{ line: 10, column: 7 }], + }, + { + message: + 'OneOf Input Object C must be inhabitable but all fields recursively reference only other OneOf Input Objects forming an unresolvable cycle.', + locations: [{ line: 14, column: 7 }], + }, + ]); + }); +}); + describe('Objects must adhere to Interface they implement', () => { it('accepts an Object which implements an Interface', () => { const schema = buildSchema(` diff --git a/src/type/validate.ts b/src/type/validate.ts index 1ddb4a9b54..79798856ec 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -378,6 +378,8 @@ function validateTypes(context: SchemaValidationContext): void { createInputObjectNonNullCircularRefsValidator(context); const validateInputObjectDefaultValueCircularRefs = createInputObjectDefaultValueCircularRefsValidator(context); + const validateOneOfInputObjectInhabitability = + createOneOfInputObjectInhabitabilityValidator(context); const typeMap = context.schema.getTypeMap(); for (const type of Object.values(typeMap)) { // Ensure all provided types are in fact GraphQL type. @@ -422,6 +424,11 @@ function validateTypes(context: SchemaValidationContext): void { // Ensure Input Objects do not contain invalid default value circular references. validateInputObjectDefaultValueCircularRefs(type); + + // Ensure OneOf Input Objects are inhabitable. + if (type.isOneOf) { + validateOneOfInputObjectInhabitability(type); + } } } } @@ -956,6 +963,64 @@ function createInputObjectDefaultValueCircularRefsValidator( } } +function createOneOfInputObjectInhabitabilityValidator( + context: SchemaValidationContext, +): (inputObj: GraphQLInputObjectType) => void { + // Tracks already validated types to maintain O(N) across top-level calls. + const visitedTypes = new Set(); + + return function validateOneOfInputObjectInhabitability( + inputObj: GraphQLInputObjectType, + ): void { + if (visitedTypes.has(inputObj)) { + return; + } + + if (!isInhabitable(inputObj, new Set())) { + context.reportError( + `OneOf Input Object ${inputObj} must be inhabitable but all fields recursively reference only other OneOf Input Objects forming an unresolvable cycle.`, + inputObj.astNode, + ); + } + + visitedTypes.add(inputObj); + }; + + function isInhabitable( + inputObj: GraphQLInputObjectType, + visited: ReadonlySet, + ): boolean { + if (visited.has(inputObj)) { + return false; + } + + const nextVisited = new Set(visited); + nextVisited.add(inputObj); + + for (const field of Object.values(inputObj.getFields())) { + if (isListType(field.type)) { + return true; + } + + const namedType = getNamedType(field.type); + + if (!isInputObjectType(namedType)) { + return true; + } + + if (!namedType.isOneOf) { + return true; + } + + if (isInhabitable(namedType, nextVisited)) { + return true; + } + } + + return false; + } +} + function getAllImplementsInterfaceNodes( type: GraphQLObjectType | GraphQLInterfaceType, iface: GraphQLInterfaceType, From c5a600faae7920de723d9d0abd9639f136ca30ff Mon Sep 17 00:00:00 2001 From: James Bellenger Date: Sat, 28 Feb 2026 04:51:24 -0800 Subject: [PATCH 2/5] Detect uninhabitable input types formed by OneOf and non-OneOf cycles Rework the circular references validation to detect input object types that cannot be provided a finite value. This covers: - Self-recursive OneOf types (e.g. input A @oneOf { a: A }) - Mixed OneOf/non-OneOf cycles with no escape path - Standard non-null circular references (existing behavior preserved) Rename algorithms to match spec terminology: - InputObjectHasUnbreakableCycle (was InputObjectCanBeProvidedAFiniteValue) - InputFieldTypeHasUnbreakableCycle (was FieldTypeCanBeProvidedAFiniteValue) --- src/type/__tests__/validation-test.ts | 207 +++++++++++++------- src/type/validate.ts | 259 ++++++++++++++------------ 2 files changed, 277 insertions(+), 189 deletions(-) diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index 7c7a24660c..5786a114ec 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -924,7 +924,7 @@ describe('Type System: Input Objects must have fields', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Invalid circular reference. The Input Object SomeInputObject references itself in the non-null field SomeInputObject.nonNullSelf.', + 'Input Object SomeInputObject references itself via the required fields: SomeInputObject.nonNullSelf.', locations: [{ line: 7, column: 9 }], }, ]); @@ -952,13 +952,31 @@ describe('Type System: Input Objects must have fields', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Invalid circular reference. The Input Object SomeInputObject references itself via the non-null fields: SomeInputObject.startLoop, AnotherInputObject.nextInLoop, YetAnotherInputObject.closeLoop.', + 'Input Object SomeInputObject references itself via the required fields: SomeInputObject.startLoop, AnotherInputObject.nextInLoop, YetAnotherInputObject.closeLoop.', locations: [ { line: 7, column: 9 }, { line: 11, column: 9 }, { line: 15, column: 9 }, ], }, + { + message: + 'Input Object AnotherInputObject references itself via the required fields: AnotherInputObject.nextInLoop, YetAnotherInputObject.closeLoop, SomeInputObject.startLoop.', + locations: [ + { line: 11, column: 9 }, + { line: 15, column: 9 }, + { line: 7, column: 9 }, + ], + }, + { + message: + 'Input Object YetAnotherInputObject references itself via the required fields: YetAnotherInputObject.closeLoop, SomeInputObject.startLoop, AnotherInputObject.nextInLoop.', + locations: [ + { line: 15, column: 9 }, + { line: 7, column: 9 }, + { line: 11, column: 9 }, + ], + }, ]); }); @@ -986,7 +1004,7 @@ describe('Type System: Input Objects must have fields', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Invalid circular reference. The Input Object SomeInputObject references itself via the non-null fields: SomeInputObject.startLoop, AnotherInputObject.closeLoop.', + 'Input Object SomeInputObject references itself via the required fields: SomeInputObject.startLoop, AnotherInputObject.closeLoop.', locations: [ { line: 7, column: 9 }, { line: 11, column: 9 }, @@ -994,16 +1012,20 @@ describe('Type System: Input Objects must have fields', () => { }, { message: - 'Invalid circular reference. The Input Object AnotherInputObject references itself via the non-null fields: AnotherInputObject.startSecondLoop, YetAnotherInputObject.closeSecondLoop.', + 'Input Object AnotherInputObject references itself via the required fields: AnotherInputObject.closeLoop, SomeInputObject.startLoop.', locations: [ - { line: 12, column: 9 }, - { line: 16, column: 9 }, + { line: 11, column: 9 }, + { line: 7, column: 9 }, ], }, { message: - 'Invalid circular reference. The Input Object YetAnotherInputObject references itself in the non-null field YetAnotherInputObject.nonNullSelf.', - locations: [{ line: 17, column: 9 }], + 'Input Object YetAnotherInputObject references itself via the required fields: YetAnotherInputObject.closeSecondLoop, AnotherInputObject.closeLoop, SomeInputObject.startLoop.', + locations: [ + { line: 16, column: 9 }, + { line: 11, column: 9 }, + { line: 7, column: 9 }, + ], }, ]); }); @@ -2409,173 +2431,202 @@ describe('Type System: OneOf Input Object fields must be nullable', () => { }); }); -describe('Type System: OneOf Input Objects must be inhabitable', () => { +describe('Type System: Input Objects must not have unbreakable cycles', () => { it('accepts a OneOf Input Object with a scalar field', () => { const schema = buildSchema(` type Query { - test(arg: A): String + test(arg: A): Int } input A @oneOf { - a: String - b: Int + a: Int } `); expectJSON(validateSchema(schema)).toDeepEqual([]); }); - it('accepts a OneOf Input Object with an enum field', () => { + it('accepts a OneOf Input Object with a recursive list field', () => { const schema = buildSchema(` type Query { - test(arg: A): String + test(arg: A): Int } - enum Color { RED GREEN BLUE } - input A @oneOf { - a: Color + a: [A!] } `); expectJSON(validateSchema(schema)).toDeepEqual([]); }); - it('accepts a OneOf Input Object with a list field', () => { + it('accepts a OneOf Input Object referencing a non-OneOf input object', () => { const schema = buildSchema(` type Query { - test(arg: A): String + test(arg: A): Int } input A @oneOf { - a: [A] + b: B + } + + input B { + x: Int } `); expectJSON(validateSchema(schema)).toDeepEqual([]); }); - it('accepts a OneOf Input Object referencing a non-OneOf input object', () => { + it('accepts a OneOf/OneOf cycle with a scalar escape', () => { const schema = buildSchema(` type Query { - test(arg: A): String + test(arg: A): Int } input A @oneOf { - a: RegularInput + b: B + escape: Int } - input RegularInput { - x: String + input B @oneOf { + a: A } `); expectJSON(validateSchema(schema)).toDeepEqual([]); }); - it('accepts a OneOf Input Object with at least one escape field', () => { + it('accepts a OneOf/non-OneOf cycle with a nullable escape', () => { const schema = buildSchema(` type Query { - test(arg: A): String + test(arg: A): Int } input A @oneOf { b: B - escape: String } - input B @oneOf { + input B { a: A } `); expectJSON(validateSchema(schema)).toDeepEqual([]); }); - it('accepts mutually referencing OneOf types where one has a scalar escape', () => { + it('rejects a self-referencing OneOf type with no escapes', () => { const schema = buildSchema(` type Query { - test(arg: A): String + test(arg: A): Int } input A @oneOf { - b: B - } - - input B @oneOf { - a: A - escape: Int + self: A } `); - expectJSON(validateSchema(schema)).toDeepEqual([]); + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Input Object A references itself via the required fields: A.self.', + locations: [{ line: 7, column: 9 }], + }, + ]); }); - it('accepts a OneOf referencing a non-OneOf which references back', () => { + it('rejects a mixed OneOf/non-OneOf cycle with no escapes', () => { const schema = buildSchema(` type Query { - test(arg: A): String + test(arg: A): Int } input A @oneOf { - b: RegularInput + b: B } - input RegularInput { - back: A + input B { + a: A! } `); - expectJSON(validateSchema(schema)).toDeepEqual([]); + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Input Object A references itself via the required fields: A.b, B.a.', + locations: [ + { line: 7, column: 9 }, + { line: 11, column: 9 }, + ], + }, + { + message: + 'Input Object B references itself via the required fields: B.a, A.b.', + locations: [ + { line: 11, column: 9 }, + { line: 7, column: 9 }, + ], + }, + ]); }); - it('accepts a OneOf with multiple fields where one escapes through chained OneOf types', () => { + it('accepts a OneOf/non-OneOf with scalar escape', () => { const schema = buildSchema(` type Query { - test(arg: A): String + test(arg: A): Int } input A @oneOf { b: B - c: C + escape: Int } - input B @oneOf { - a: A + input B { + a: A! } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); - input C @oneOf { + it('accepts a non-OneOf/non-OneOf cycle with a nullable escape', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A { + b: B! + } + + input B { a: A - escape: String } `); expectJSON(validateSchema(schema)).toDeepEqual([]); }); - it('rejects a closed subgraph of one OneOf type', () => { + it('accepts a non-OneOf/non-OneOf cycle with a list escape', () => { const schema = buildSchema(` type Query { - test(arg: A): String + test(arg: A): Int } - input A @oneOf { - self: A + input A { + b: [B!]! + } + + input B { + a: A! } `); - expectJSON(validateSchema(schema)).toDeepEqual([ - { - message: - 'OneOf Input Object A must be inhabitable but all fields recursively reference only other OneOf Input Objects forming an unresolvable cycle.', - locations: [{ line: 6, column: 7 }], - }, - ]); + expectJSON(validateSchema(schema)).toDeepEqual([]); }); - it('rejects a closed subgraph of multiple OneOf types', () => { + it('rejects a larger mixed OneOf/non-OneOf cycle with no escapes', () => { const schema = buildSchema(` type Query { - test(arg: A): String + test(arg: A): Int } input A @oneOf { b: B } - input B @oneOf { - c: C + input B { + c: C! } input C @oneOf { @@ -2585,18 +2636,30 @@ describe('Type System: OneOf Input Objects must be inhabitable', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'OneOf Input Object A must be inhabitable but all fields recursively reference only other OneOf Input Objects forming an unresolvable cycle.', - locations: [{ line: 6, column: 7 }], + 'Input Object A references itself via the required fields: A.b, B.c, C.a.', + locations: [ + { line: 7, column: 9 }, + { line: 11, column: 9 }, + { line: 15, column: 9 }, + ], }, { message: - 'OneOf Input Object B must be inhabitable but all fields recursively reference only other OneOf Input Objects forming an unresolvable cycle.', - locations: [{ line: 10, column: 7 }], + 'Input Object B references itself via the required fields: B.c, C.a, A.b.', + locations: [ + { line: 11, column: 9 }, + { line: 15, column: 9 }, + { line: 7, column: 9 }, + ], }, { message: - 'OneOf Input Object C must be inhabitable but all fields recursively reference only other OneOf Input Objects forming an unresolvable cycle.', - locations: [{ line: 14, column: 7 }], + 'Input Object C references itself via the required fields: C.a, A.b, B.c.', + locations: [ + { line: 15, column: 9 }, + { line: 7, column: 9 }, + { line: 11, column: 9 }, + ], }, ]); }); diff --git a/src/type/validate.ts b/src/type/validate.ts index 79798856ec..dc1d6f3f79 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -373,14 +373,15 @@ function validateName( } function validateTypes(context: SchemaValidationContext): void { - // Ensure Input Objects do not contain non-nullable circular references. - const validateInputObjectNonNullCircularRefs = - createInputObjectNonNullCircularRefsValidator(context); + const inputObjectUnbreakableCycleCheck = + createInputObjectUnbreakableCycleCheck(); const validateInputObjectDefaultValueCircularRefs = createInputObjectDefaultValueCircularRefsValidator(context); - const validateOneOfInputObjectInhabitability = - createOneOfInputObjectInhabitabilityValidator(context); const typeMap = context.schema.getTypeMap(); + + // Collect Input Object types that have unbreakable cycles. + const typesWithUnbreakableCycles = new Set(); + for (const type of Object.values(typeMap)) { // Ensure all provided types are in fact GraphQL type. if (!isNamedType(type)) { @@ -418,19 +419,25 @@ function validateTypes(context: SchemaValidationContext): void { // Ensure Input Object fields are valid. validateInputFields(context, type); - // Ensure Input Objects do not contain invalid field circular references. - // Ensure Input Objects do not contain non-nullable circular references. - validateInputObjectNonNullCircularRefs(type); + // Ensure Input Objects do not have unbreakable cycles. + if (inputObjectUnbreakableCycleCheck(type)) { + typesWithUnbreakableCycles.add(type); + } // Ensure Input Objects do not contain invalid default value circular references. validateInputObjectDefaultValueCircularRefs(type); - - // Ensure OneOf Input Objects are inhabitable. - if (type.isOneOf) { - validateOneOfInputObjectInhabitability(type); - } } } + + // Report errors for Input Object types that have unbreakable cycles. + for (const type of typesWithUnbreakableCycles) { + const cyclePath = traceUnbreakableCycle(type, typesWithUnbreakableCycles); + const pathStr = cyclePath.map((p) => p.fieldStr).join(', '); + context.reportError( + `Input Object ${type} references itself via the required fields: ${pathStr}.`, + cyclePath.map((p) => p.astNode), + ); + } } function validateFields( @@ -746,66 +753,142 @@ function validateOneOfInputObjectField( } } -function createInputObjectNonNullCircularRefsValidator( - context: SchemaValidationContext, -): (inputObj: GraphQLInputObjectType) => void { - // Modified copy of algorithm from 'src/validation/rules/NoFragmentCycles.js'. - // Tracks already visited types to maintain O(N) and to ensure that cycles - // are not redundantly reported. - const visitedTypes = new Set(); +// Implements the spec's InputObjectHasUnbreakableCycle algorithm. +// Tracks already checked types to maintain O(N) and to ensure that types +// are not redundantly checked. +function createInputObjectUnbreakableCycleCheck(): ( + inputObj: GraphQLInputObjectType, +) => boolean { + const knownNoCycle = new Set(); + const visited = new Set(); - // Array of types nodes used to produce meaningful errors - const fieldPath: Array<{ fieldStr: string; astNode: Maybe }> = []; + return inputObjectHasUnbreakableCycle; - // Position in the type path - const fieldPathIndexByTypeName: ObjMap = - Object.create(null); + function inputObjectHasUnbreakableCycle( + inputObj: GraphQLInputObjectType, + ): boolean { + if (knownNoCycle.has(inputObj)) { + return false; + } + if (visited.has(inputObj)) { + return true; + } - return detectCycleRecursive; + visited.add(inputObj); - // This does a straight-forward DFS to find cycles. - // It does not terminate when a cycle was found but continues to explore - // the graph to find all possible cycles. - function detectCycleRecursive(inputObj: GraphQLInputObjectType): void { - if (visitedTypes.has(inputObj)) { - return; + let result: boolean; + + if (inputObj.isOneOf) { + // OneOf Input Objects have an unbreakable cycle if every field has one. + result = true; + for (const field of Object.values(inputObj.getFields())) { + if (!inputFieldTypeHasUnbreakableCycle(field.type)) { + result = false; + break; + } + } + } else { + // Normal Input Objects have an unbreakable cycle if any non-null field has one. + result = false; + for (const field of Object.values(inputObj.getFields())) { + if ( + isNonNullType(field.type) && + inputFieldTypeHasUnbreakableCycle(field.type.ofType) + ) { + result = true; + break; + } + } } - visitedTypes.add(inputObj); - fieldPathIndexByTypeName[inputObj.name] = fieldPath.length; + visited.delete(inputObj); - const fields = Object.values(inputObj.getFields()); - for (const field of fields) { - if (isNonNullType(field.type) && isInputObjectType(field.type.ofType)) { - const fieldType = field.type.ofType; - const cycleIndex = fieldPathIndexByTypeName[fieldType.name]; + if (!result) { + knownNoCycle.add(inputObj); + } + return result; + } - fieldPath.push({ - fieldStr: `${inputObj}.${field.name}`, + function inputFieldTypeHasUnbreakableCycle( + fieldType: GraphQLInputType, + ): boolean { + if (isListType(fieldType)) { + return false; + } + if (isNonNullType(fieldType)) { + return inputFieldTypeHasUnbreakableCycle(fieldType.ofType); + } + if (!isInputObjectType(fieldType)) { + return false; + } + return inputObjectHasUnbreakableCycle(fieldType); + } +} + +// For an Input Object type with an unbreakable cycle, traces a witness cycle +// path by following required edges to other types with unbreakable cycles. +function traceUnbreakableCycle( + startType: GraphQLInputObjectType, + typesWithUnbreakableCycles: ReadonlySet, +): Array<{ fieldStr: string; astNode: Maybe }> { + const path: Array<{ fieldStr: string; astNode: Maybe }> = []; + const seen = new Set(); + + let current: Maybe = startType; + while (current != null && !seen.has(current)) { + seen.add(current); + let next: Maybe; + + for (const field of Object.values(current.getFields())) { + let target: Maybe; + + if (current.isOneOf) { + if ( + isInputObjectType(field.type) && + typesWithUnbreakableCycles.has(field.type) + ) { + target = field.type; + } + } else if (isNonNullType(field.type)) { + target = unwrapToUnbreakableCycleType( + field.type.ofType, + typesWithUnbreakableCycles, + ); + } + + if (target != null) { + path.push({ + fieldStr: `${current}.${field.name}`, astNode: field.astNode, }); - if (cycleIndex === undefined) { - detectCycleRecursive(fieldType); - } else { - const cyclePath = fieldPath.slice(cycleIndex); - const pathStr = cyclePath - .map((fieldObj) => fieldObj.fieldStr) - .join(', '); - context.reportError( - `Invalid circular reference. The Input Object ${fieldType} references itself ${ - cyclePath.length > 1 - ? 'via the non-null fields:' - : 'in the non-null field' - } ${pathStr}.`, - cyclePath.map((fieldObj) => fieldObj.astNode), - ); - } - fieldPath.pop(); + next = target; + break; } } - fieldPathIndexByTypeName[inputObj.name] = undefined; + current = next; + } + + return path; +} + +function unwrapToUnbreakableCycleType( + type: GraphQLInputType, + typesWithUnbreakableCycles: ReadonlySet, +): Maybe { + if (isListType(type)) { + return undefined; + } + if (isNonNullType(type)) { + return unwrapToUnbreakableCycleType( + type.ofType, + typesWithUnbreakableCycles, + ); + } + if (isInputObjectType(type) && typesWithUnbreakableCycles.has(type)) { + return type; } + return undefined; } function createInputObjectDefaultValueCircularRefsValidator( @@ -963,64 +1046,6 @@ function createInputObjectDefaultValueCircularRefsValidator( } } -function createOneOfInputObjectInhabitabilityValidator( - context: SchemaValidationContext, -): (inputObj: GraphQLInputObjectType) => void { - // Tracks already validated types to maintain O(N) across top-level calls. - const visitedTypes = new Set(); - - return function validateOneOfInputObjectInhabitability( - inputObj: GraphQLInputObjectType, - ): void { - if (visitedTypes.has(inputObj)) { - return; - } - - if (!isInhabitable(inputObj, new Set())) { - context.reportError( - `OneOf Input Object ${inputObj} must be inhabitable but all fields recursively reference only other OneOf Input Objects forming an unresolvable cycle.`, - inputObj.astNode, - ); - } - - visitedTypes.add(inputObj); - }; - - function isInhabitable( - inputObj: GraphQLInputObjectType, - visited: ReadonlySet, - ): boolean { - if (visited.has(inputObj)) { - return false; - } - - const nextVisited = new Set(visited); - nextVisited.add(inputObj); - - for (const field of Object.values(inputObj.getFields())) { - if (isListType(field.type)) { - return true; - } - - const namedType = getNamedType(field.type); - - if (!isInputObjectType(namedType)) { - return true; - } - - if (!namedType.isOneOf) { - return true; - } - - if (isInhabitable(namedType, nextVisited)) { - return true; - } - } - - return false; - } -} - function getAllImplementsInterfaceNodes( type: GraphQLObjectType | GraphQLInterfaceType, iface: GraphQLInterfaceType, From ad802ae5087ab24de08bb22d8b44f7b244c0cac9 Mon Sep 17 00:00:00 2001 From: James Bellenger Date: Thu, 7 May 2026 05:03:28 -0700 Subject: [PATCH 3/5] simplify type unwrapping and add test to satisfy coverage checker --- src/type/__tests__/validation-test.ts | 36 +++++++++++++++++++++++++++ src/type/validate.ts | 33 ++++++------------------ 2 files changed, 43 insertions(+), 26 deletions(-) diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index 5786a114ec..06ad2022fa 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -2615,6 +2615,42 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { expectJSON(validateSchema(schema)).toDeepEqual([]); }); + it('rejects a non-OneOf/non-OneOf cycle with required scalar and list fields', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A { + list: [B]! + b: B! + } + + input B { + value: Int! + a: A! + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Input Object A references itself via the required fields: A.b, B.a.', + locations: [ + { line: 8, column: 9 }, + { line: 13, column: 9 }, + ], + }, + { + message: + 'Input Object B references itself via the required fields: B.a, A.b.', + locations: [ + { line: 13, column: 9 }, + { line: 8, column: 9 }, + ], + }, + ]); + }); + it('rejects a larger mixed OneOf/non-OneOf cycle with no escapes', () => { const schema = buildSchema(` type Query { diff --git a/src/type/validate.ts b/src/type/validate.ts index dc1d6f3f79..e170672d66 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -815,9 +815,6 @@ function createInputObjectUnbreakableCycleCheck(): ( if (isListType(fieldType)) { return false; } - if (isNonNullType(fieldType)) { - return inputFieldTypeHasUnbreakableCycle(fieldType.ofType); - } if (!isInputObjectType(fieldType)) { return false; } @@ -850,10 +847,13 @@ function traceUnbreakableCycle( target = field.type; } } else if (isNonNullType(field.type)) { - target = unwrapToUnbreakableCycleType( - field.type.ofType, - typesWithUnbreakableCycles, - ); + const nullableType = field.type.ofType; + if ( + isInputObjectType(nullableType) && + typesWithUnbreakableCycles.has(nullableType) + ) { + target = nullableType; + } } if (target != null) { @@ -872,25 +872,6 @@ function traceUnbreakableCycle( return path; } -function unwrapToUnbreakableCycleType( - type: GraphQLInputType, - typesWithUnbreakableCycles: ReadonlySet, -): Maybe { - if (isListType(type)) { - return undefined; - } - if (isNonNullType(type)) { - return unwrapToUnbreakableCycleType( - type.ofType, - typesWithUnbreakableCycles, - ); - } - if (isInputObjectType(type) && typesWithUnbreakableCycles.has(type)) { - return type; - } - return undefined; -} - function createInputObjectDefaultValueCircularRefsValidator( context: SchemaValidationContext, ): (inputObj: GraphQLInputObjectType) => void { From 014fbf247cc9dc535da7c4654862235b173e08d2 Mon Sep 17 00:00:00 2001 From: James Bellenger Date: Thu, 7 May 2026 10:11:05 -0700 Subject: [PATCH 4/5] perf and style - add memoization to improve validation performance - graceful handling of empty oneof defintions - add missing test coverage --- src/type/__tests__/validation-test.ts | 151 ++++++++++++++++++++ src/type/validate.ts | 190 ++++++++++++++++++++------ 2 files changed, 303 insertions(+), 38 deletions(-) diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index 06ad2022fa..0425739093 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -3,6 +3,7 @@ import { describe, it } from 'mocha'; import { dedent } from '../../__testUtils__/dedent.js'; import { expectJSON } from '../../__testUtils__/expectJSON.js'; +import { spyOnMethod } from '../../__testUtils__/spyOn.js'; import { inspect } from '../../jsutils/inspect.js'; @@ -888,6 +889,24 @@ describe('Type System: Input Objects must have fields', () => { ]); }); + it('rejects a OneOf Input Object type with missing fields', () => { + const schema = buildSchema(` + type Query { + field(arg: SomeInputObject): String + } + + input SomeInputObject @oneOf + `); + + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Input Object type SomeInputObject must define one or more fields.', + locations: [{ line: 6, column: 7 }], + }, + ]); + }); + it('accepts an Input Object with breakable circular reference', () => { const schema = buildSchema(` type Query { @@ -2475,6 +2494,64 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { expectJSON(validateSchema(schema)).toDeepEqual([]); }); + it('accepts a OneOf Input Object referencing an already checked input object', () => { + const schema = buildSchema(` + type Query { + b(arg: B): Int + a(arg: A): Int + } + + input B { + value: Int + } + + input A @oneOf { + b: B + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a OneOf Input Object with multiple acyclic input object fields', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A @oneOf { + b: B + c: C + } + + input B { + value: Int + } + + input C { + value: Int + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a OneOf Input Object with an input object field and scalar escape', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A @oneOf { + b: B + escape: Int + } + + input B { + value: Int + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + it('accepts a OneOf/OneOf cycle with a scalar escape', () => { const schema = buildSchema(` type Query { @@ -2529,6 +2606,80 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { ]); }); + it('rejects a normal Input Object requiring an unbreakable OneOf cycle', () => { + const schema = buildSchema(` + type Query { + t(arg: T): Int + a(arg: A): Int + } + + input T @oneOf { + self: T + } + + input A { + t: T! + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Input Object T references itself via the required fields: T.self.', + locations: [{ line: 8, column: 9 }], + }, + { + message: + 'Input Object A references itself via the required fields: A.t, T.self.', + locations: [ + { line: 12, column: 9 }, + { line: 8, column: 9 }, + ], + }, + ]); + }); + + it('caches shared unbreakable OneOf subgraphs', () => { + const chainLength = 16; + const types: Array = []; + types[0] = new GraphQLInputObjectType({ + name: 'T0', + isOneOf: true, + fields: () => ({ self: { type: types[0] } }), + }); + + for (let i = 1; i <= chainLength; ++i) { + const previousType = types[i - 1]; + types[i] = new GraphQLInputObjectType({ + name: `T${i}`, + isOneOf: true, + fields: { + a: { type: previousType }, + b: { type: previousType }, + }, + }); + } + + const getFieldsSpies = types.map((type) => spyOnMethod(type, 'getFields')); + + const schema = new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'Query', + fields: { + test: { + type: GraphQLInt, + args: { input: { type: types[chainLength] } }, + }, + }, + }), + types, + }); + + expect(validateSchema(schema)).to.have.lengthOf(types.length); + expect( + getFieldsSpies.reduce((sum, spy) => sum + spy.callCount, 0), + ).to.be.lessThan(500); + }); + it('rejects a mixed OneOf/non-OneOf cycle with no escapes', () => { const schema = buildSchema(` type Query { diff --git a/src/type/validate.ts b/src/type/validate.ts index e170672d66..2a46e71773 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -754,71 +754,185 @@ function validateOneOfInputObjectField( } // Implements the spec's InputObjectHasUnbreakableCycle algorithm. -// Tracks already checked types to maintain O(N) and to ensure that types -// are not redundantly checked. +// Tracks already checked types to ensure that types are not redundantly checked. function createInputObjectUnbreakableCycleCheck(): ( inputObj: GraphQLInputObjectType, ) => boolean { - const knownNoCycle = new Set(); - const visited = new Set(); + const cycleMemo = new Map(); return inputObjectHasUnbreakableCycle; function inputObjectHasUnbreakableCycle( inputObj: GraphQLInputObjectType, ): boolean { - if (knownNoCycle.has(inputObj)) { - return false; + const cachedResult = cycleMemo.get(inputObj); + if (cachedResult !== undefined) { + return cachedResult; } - if (visited.has(inputObj)) { - return true; + + // Unknown reachable types mapped to types that depend on them. + const candidates = new AccumulatorMap< + GraphQLInputObjectType, + GraphQLInputObjectType + >(); + for (const candidate of collectReachableInputObjects(inputObj)) { + candidates.set(candidate, []); } - visited.add(inputObj); + // Tracks unresolved fields for normal Input Objects. + const normalTypeStates = new Map< + GraphQLInputObjectType, + { hasKnownCycleField: boolean; unresolvedFieldCount: number } + >(); - let result: boolean; + // Types proven not to have cycles and ready to remove. + const typesToRemove: Array = []; - if (inputObj.isOneOf) { - // OneOf Input Objects have an unbreakable cycle if every field has one. - result = true; - for (const field of Object.values(inputObj.getFields())) { - if (!inputFieldTypeHasUnbreakableCycle(field.type)) { - result = false; - break; + for (const candidate of candidates.keys()) { + const fields = Object.values(candidate.getFields()); + + if (candidate.isOneOf) { + // OneOf Input Objects have an unbreakable cycle if every field leads to an unbreakable cycle. + if (fields.length === 0) { + typesToRemove.push(candidate); + continue; } - } - } else { - // Normal Input Objects have an unbreakable cycle if any non-null field has one. - result = false; - for (const field of Object.values(inputObj.getFields())) { - if ( - isNonNullType(field.type) && - inputFieldTypeHasUnbreakableCycle(field.type.ofType) - ) { - result = true; - break; + + for (const field of fields) { + const target = getUnbreakableCycleTarget(candidate, field.type); + + if (target == null) { + typesToRemove.push(candidate); + break; + } + + const targetResult = cycleMemo.get(target); + if (targetResult === false) { + typesToRemove.push(candidate); + break; + } + + if (targetResult === undefined) { + candidates.add(target, candidate); + } + } + } else { + // Normal Input Objects have an unbreakable cycle if any non-null field has one. + let hasKnownCycleField = false; + let unresolvedFieldCount = 0; + + for (const field of fields) { + const target = getUnbreakableCycleTarget(candidate, field.type); + + if (target == null) { + continue; + } + + const targetResult = cycleMemo.get(target); + if (targetResult === false) { + continue; + } + + if (targetResult === true) { + hasKnownCycleField = true; + } else { + ++unresolvedFieldCount; + candidates.add(target, candidate); + } + } + + normalTypeStates.set(candidate, { + hasKnownCycleField, + unresolvedFieldCount, + }); + + if (!hasKnownCycleField && unresolvedFieldCount === 0) { + typesToRemove.push(candidate); } } } - visited.delete(inputObj); + while (typesToRemove.length > 0) { + const type = typesToRemove.pop(); + invariant(type != null); + + const dependents = candidates.get(type); + if (dependents === undefined) { + continue; + } + + candidates.delete(type); + cycleMemo.set(type, false); + + for (const dependent of dependents) { + if (!candidates.has(dependent)) { + continue; + } + + if (dependent.isOneOf) { + typesToRemove.push(dependent); + } else { + const state = normalTypeStates.get(dependent); + invariant(state !== undefined); - if (!result) { - knownNoCycle.add(inputObj); + --state.unresolvedFieldCount; + if (!state.hasKnownCycleField && state.unresolvedFieldCount === 0) { + typesToRemove.push(dependent); + } + } + } + } + + for (const candidate of candidates.keys()) { + cycleMemo.set(candidate, true); } + + const result = cycleMemo.get(inputObj); + invariant(result !== undefined); return result; } - function inputFieldTypeHasUnbreakableCycle( + function collectReachableInputObjects( + inputObj: GraphQLInputObjectType, + ): Set { + const reachable = new Set(); + const visited = new Set(); + + collect(inputObj); + return reachable; + + function collect(type: GraphQLInputObjectType): void { + if (visited.has(type) || cycleMemo.has(type)) { + return; + } + + visited.add(type); + reachable.add(type); + + for (const field of Object.values(type.getFields())) { + const target = getUnbreakableCycleTarget(type, field.type); + + if (target != null) { + collect(target); + } + } + } + } + + function getUnbreakableCycleTarget( + inputObj: GraphQLInputObjectType, fieldType: GraphQLInputType, - ): boolean { - if (isListType(fieldType)) { - return false; + ): Maybe { + if (inputObj.isOneOf) { + if (isInputObjectType(fieldType)) { + return fieldType; + } + return undefined; } - if (!isInputObjectType(fieldType)) { - return false; + + if (isNonNullType(fieldType) && isInputObjectType(fieldType.ofType)) { + return fieldType.ofType; } - return inputObjectHasUnbreakableCycle(fieldType); } } From c4706c3a213f8ac09a84e7f9b8cfece87bd38d60 Mon Sep 17 00:00:00 2001 From: James Bellenger Date: Thu, 7 May 2026 16:27:18 -0700 Subject: [PATCH 5/5] error reporting updates Update error reporting for uninhabited cycles to match the previous implementation, which would report 1 error per distinct cycle. Also, update the errors to use the spec language "cannot be provided a finite value" --- src/type/__tests__/validation-test.ts | 119 ++++++++++--------------- src/type/validate.ts | 122 +++++++++++++------------- 2 files changed, 107 insertions(+), 134 deletions(-) diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index 0425739093..e232e8e77a 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -943,7 +943,7 @@ describe('Type System: Input Objects must have fields', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Input Object SomeInputObject references itself via the required fields: SomeInputObject.nonNullSelf.', + 'Input Object SomeInputObject cannot be provided a finite value because it references itself through fields: SomeInputObject.nonNullSelf.', locations: [{ line: 7, column: 9 }], }, ]); @@ -971,31 +971,13 @@ describe('Type System: Input Objects must have fields', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Input Object SomeInputObject references itself via the required fields: SomeInputObject.startLoop, AnotherInputObject.nextInLoop, YetAnotherInputObject.closeLoop.', + 'Input Object SomeInputObject cannot be provided a finite value because it references itself through fields: SomeInputObject.startLoop, AnotherInputObject.nextInLoop, YetAnotherInputObject.closeLoop.', locations: [ { line: 7, column: 9 }, { line: 11, column: 9 }, { line: 15, column: 9 }, ], }, - { - message: - 'Input Object AnotherInputObject references itself via the required fields: AnotherInputObject.nextInLoop, YetAnotherInputObject.closeLoop, SomeInputObject.startLoop.', - locations: [ - { line: 11, column: 9 }, - { line: 15, column: 9 }, - { line: 7, column: 9 }, - ], - }, - { - message: - 'Input Object YetAnotherInputObject references itself via the required fields: YetAnotherInputObject.closeLoop, SomeInputObject.startLoop, AnotherInputObject.nextInLoop.', - locations: [ - { line: 15, column: 9 }, - { line: 7, column: 9 }, - { line: 11, column: 9 }, - ], - }, ]); }); @@ -1023,7 +1005,7 @@ describe('Type System: Input Objects must have fields', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Input Object SomeInputObject references itself via the required fields: SomeInputObject.startLoop, AnotherInputObject.closeLoop.', + 'Input Object SomeInputObject cannot be provided a finite value because it references itself through fields: SomeInputObject.startLoop, AnotherInputObject.closeLoop.', locations: [ { line: 7, column: 9 }, { line: 11, column: 9 }, @@ -1031,20 +1013,16 @@ describe('Type System: Input Objects must have fields', () => { }, { message: - 'Input Object AnotherInputObject references itself via the required fields: AnotherInputObject.closeLoop, SomeInputObject.startLoop.', + 'Input Object AnotherInputObject cannot be provided a finite value because it references itself through fields: AnotherInputObject.startSecondLoop, YetAnotherInputObject.closeSecondLoop.', locations: [ - { line: 11, column: 9 }, - { line: 7, column: 9 }, + { line: 12, column: 9 }, + { line: 16, column: 9 }, ], }, { message: - 'Input Object YetAnotherInputObject references itself via the required fields: YetAnotherInputObject.closeSecondLoop, AnotherInputObject.closeLoop, SomeInputObject.startLoop.', - locations: [ - { line: 16, column: 9 }, - { line: 11, column: 9 }, - { line: 7, column: 9 }, - ], + 'Input Object YetAnotherInputObject cannot be provided a finite value because it references itself through fields: YetAnotherInputObject.nonNullSelf.', + locations: [{ line: 17, column: 9 }], }, ]); }); @@ -2600,7 +2578,7 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Input Object A references itself via the required fields: A.self.', + 'Input Object A cannot be provided a finite value because it references itself through fields: A.self.', locations: [{ line: 7, column: 9 }], }, ]); @@ -2624,17 +2602,9 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Input Object T references itself via the required fields: T.self.', + 'Input Object T cannot be provided a finite value because it references itself through fields: T.self.', locations: [{ line: 8, column: 9 }], }, - { - message: - 'Input Object A references itself via the required fields: A.t, T.self.', - locations: [ - { line: 12, column: 9 }, - { line: 8, column: 9 }, - ], - }, ]); }); @@ -2674,7 +2644,7 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { types, }); - expect(validateSchema(schema)).to.have.lengthOf(types.length); + expect(validateSchema(schema)).to.have.lengthOf(1); expect( getFieldsSpies.reduce((sum, spy) => sum + spy.callCount, 0), ).to.be.lessThan(500); @@ -2697,18 +2667,49 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Input Object A references itself via the required fields: A.b, B.a.', + 'Input Object A cannot be provided a finite value because it references itself through fields: A.b, B.a.', locations: [ { line: 7, column: 9 }, { line: 11, column: 9 }, ], }, + ]); + }); + + it('rejects multiple OneOf branches without duplicate cycle reports', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A @oneOf { + b: B + c: C + } + + input B { + a: A! + } + + input C { + a: A! + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Input Object B references itself via the required fields: B.a, A.b.', + 'Input Object A cannot be provided a finite value because it references itself through fields: A.b, B.a.', locations: [ - { line: 11, column: 9 }, { line: 7, column: 9 }, + { line: 12, column: 9 }, + ], + }, + { + message: + 'Input Object A cannot be provided a finite value because it references itself through fields: A.c, C.a.', + locations: [ + { line: 8, column: 9 }, + { line: 16, column: 9 }, ], }, ]); @@ -2785,20 +2786,12 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Input Object A references itself via the required fields: A.b, B.a.', + 'Input Object A cannot be provided a finite value because it references itself through fields: A.b, B.a.', locations: [ { line: 8, column: 9 }, { line: 13, column: 9 }, ], }, - { - message: - 'Input Object B references itself via the required fields: B.a, A.b.', - locations: [ - { line: 13, column: 9 }, - { line: 8, column: 9 }, - ], - }, ]); }); @@ -2823,31 +2816,13 @@ describe('Type System: Input Objects must not have unbreakable cycles', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Input Object A references itself via the required fields: A.b, B.c, C.a.', + 'Input Object A cannot be provided a finite value because it references itself through fields: A.b, B.c, C.a.', locations: [ { line: 7, column: 9 }, { line: 11, column: 9 }, { line: 15, column: 9 }, ], }, - { - message: - 'Input Object B references itself via the required fields: B.c, C.a, A.b.', - locations: [ - { line: 11, column: 9 }, - { line: 15, column: 9 }, - { line: 7, column: 9 }, - ], - }, - { - message: - 'Input Object C references itself via the required fields: C.a, A.b, B.c.', - locations: [ - { line: 15, column: 9 }, - { line: 7, column: 9 }, - { line: 11, column: 9 }, - ], - }, ]); }); }); diff --git a/src/type/validate.ts b/src/type/validate.ts index 2a46e71773..39e76074a6 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -429,15 +429,7 @@ function validateTypes(context: SchemaValidationContext): void { } } - // Report errors for Input Object types that have unbreakable cycles. - for (const type of typesWithUnbreakableCycles) { - const cyclePath = traceUnbreakableCycle(type, typesWithUnbreakableCycles); - const pathStr = cyclePath.map((p) => p.fieldStr).join(', '); - context.reportError( - `Input Object ${type} references itself via the required fields: ${pathStr}.`, - cyclePath.map((p) => p.astNode), - ); - } + reportInputObjectUnbreakableCycles(context, typesWithUnbreakableCycles); } function validateFields( @@ -918,72 +910,78 @@ function createInputObjectUnbreakableCycleCheck(): ( } } } +} - function getUnbreakableCycleTarget( - inputObj: GraphQLInputObjectType, - fieldType: GraphQLInputType, - ): Maybe { - if (inputObj.isOneOf) { - if (isInputObjectType(fieldType)) { - return fieldType; - } - return undefined; +function getUnbreakableCycleTarget( + inputObj: GraphQLInputObjectType, + fieldType: GraphQLInputType, +): Maybe { + if (inputObj.isOneOf) { + if (isInputObjectType(fieldType)) { + return fieldType; } + return undefined; + } - if (isNonNullType(fieldType) && isInputObjectType(fieldType.ofType)) { - return fieldType.ofType; - } + if (isNonNullType(fieldType) && isInputObjectType(fieldType.ofType)) { + return fieldType.ofType; } } -// For an Input Object type with an unbreakable cycle, traces a witness cycle -// path by following required edges to other types with unbreakable cycles. -function traceUnbreakableCycle( - startType: GraphQLInputObjectType, +function reportInputObjectUnbreakableCycles( + context: SchemaValidationContext, typesWithUnbreakableCycles: ReadonlySet, -): Array<{ fieldStr: string; astNode: Maybe }> { - const path: Array<{ fieldStr: string; astNode: Maybe }> = []; - const seen = new Set(); - - let current: Maybe = startType; - while (current != null && !seen.has(current)) { - seen.add(current); - let next: Maybe; - - for (const field of Object.values(current.getFields())) { - let target: Maybe; - - if (current.isOneOf) { - if ( - isInputObjectType(field.type) && - typesWithUnbreakableCycles.has(field.type) - ) { - target = field.type; - } - } else if (isNonNullType(field.type)) { - const nullableType = field.type.ofType; - if ( - isInputObjectType(nullableType) && - typesWithUnbreakableCycles.has(nullableType) - ) { - target = nullableType; - } +): void { + // Tracks already visited types to ensure that cycles are not redundantly + // reported. + const visitedTypes = new Set(); + + // Array of fields used to produce meaningful errors. + const fieldPath: Array<{ fieldStr: string; astNode: Maybe }> = []; + + // Position in the field path. + const fieldPathIndexByType = new Map(); + + for (const type of typesWithUnbreakableCycles) { + reportCycleRecursive(type); + } + + function reportCycleRecursive(inputObj: GraphQLInputObjectType): void { + if (visitedTypes.has(inputObj)) { + return; + } + + visitedTypes.add(inputObj); + fieldPathIndexByType.set(inputObj, fieldPath.length); + + for (const field of Object.values(inputObj.getFields())) { + const target = getUnbreakableCycleTarget(inputObj, field.type); + if (target == null || !typesWithUnbreakableCycles.has(target)) { + continue; } - if (target != null) { - path.push({ - fieldStr: `${current}.${field.name}`, - astNode: field.astNode, - }); - next = target; - break; + const cycleIndex = fieldPathIndexByType.get(target); + fieldPath.push({ + fieldStr: `${inputObj}.${field.name}`, + astNode: field.astNode, + }); + + if (cycleIndex === undefined) { + reportCycleRecursive(target); + } else { + const cyclePath = fieldPath.slice(cycleIndex); + const pathStr = cyclePath.map((p) => p.fieldStr).join(', '); + context.reportError( + `Input Object ${target} cannot be provided a finite value because it references itself through fields: ${pathStr}.`, + cyclePath.map((p) => p.astNode), + ); } + + fieldPath.pop(); } - current = next; + fieldPathIndexByType.delete(inputObj); } - - return path; } function createInputObjectDefaultValueCircularRefsValidator(