diff --git a/src/jsutils/didYouMean.ts b/src/jsutils/didYouMean.ts index 33e10a42c1..ef1931c7d7 100644 --- a/src/jsutils/didYouMean.ts +++ b/src/jsutils/didYouMean.ts @@ -1,3 +1,5 @@ +import { orList } from './formatList'; + const MAX_SUGGESTIONS = 5; /** @@ -12,26 +14,21 @@ export function didYouMean( firstArg: string | ReadonlyArray, secondArg?: ReadonlyArray, ) { - const [subMessage, suggestionsArg] = secondArg + const [subMessage, suggestions] = secondArg ? [firstArg as string, secondArg] : [undefined, firstArg as ReadonlyArray]; + if (suggestions.length === 0) { + return ''; + } + let message = ' Did you mean '; if (subMessage) { message += subMessage + ' '; } - const suggestions = suggestionsArg.map((x) => `"${x}"`); - switch (suggestions.length) { - case 0: - return ''; - case 1: - return message + suggestions[0] + '?'; - case 2: - return message + suggestions[0] + ' or ' + suggestions[1] + '?'; - } - - const selected = suggestions.slice(0, MAX_SUGGESTIONS); - const lastItem = selected.pop(); - return message + selected.join(', ') + ', or ' + lastItem + '?'; + const suggestionList = orList( + suggestions.slice(0, MAX_SUGGESTIONS).map((x) => `"${x}"`), + ); + return message + suggestionList + '?'; } diff --git a/src/jsutils/formatList.ts b/src/jsutils/formatList.ts new file mode 100644 index 0000000000..fe3afe53a6 --- /dev/null +++ b/src/jsutils/formatList.ts @@ -0,0 +1,30 @@ +import { invariant } from './invariant'; + +/** + * Given [ A, B, C ] return 'A, B, or C'. + */ +export function orList(items: ReadonlyArray): string { + return formatList('or', items); +} + +/** + * Given [ A, B, C ] return 'A, B, and C'. + */ +export function andList(items: ReadonlyArray): string { + return formatList('and', items); +} + +function formatList(conjunction: string, items: ReadonlyArray): string { + invariant(items.length !== 0); + + switch (items.length) { + case 1: + return items[0]; + case 2: + return items[0] + ' ' + conjunction + ' ' + items[1]; + } + + const allButLast = items.slice(0, -1); + const lastItem = items[items.length - 1]; + return allButLast.join(', ') + ', ' + conjunction + ' ' + lastItem; +} diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index a71314f534..af82d30fbb 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -434,107 +434,80 @@ describe('Type System: A Schema must have Object root types', () => { }); describe('Type System: Root types must all be different if provided', () => { - it('rejects a Schema where the same type is used for the "query" and "mutation" root types', () => { - const schema = new GraphQLSchema({ - query: SomeObjectType, - mutation: SomeObjectType, - }); - expectJSON(validateSchema(schema)).toDeepEqual([ - { - message: - 'All root types must be different, SomeObject is already used for "query" and cannot also be used for "mutation".', - locations: [{ line: 6, column: 3 }], - }, - ]); + it('accepts a Schema with different root types', () => { + const schema = buildSchema(` + type SomeObject1 { + field: String + } - const schemaFromSDL = buildSchema(` - type SomeObject { - f: SomeObject + type SomeObject2 { + field: String + } + + type SomeObject3 { + field: String } schema { - query: SomeObject - mutation: SomeObject + query: SomeObject1 + mutation: SomeObject2 + subscription: SomeObject3 } `); - expectJSON(validateSchema(schemaFromSDL)).toDeepEqual([ - { - message: - 'All root types must be different, SomeObject is already used for "query" and cannot also be used for "mutation".', - locations: [{ line: 8, column: 19 }], - }, - ]); + expectJSON(validateSchema(schema)).toDeepEqual([]); }); - it('rejects a Schema where the same type is used for the "query" and "subscription" root types', () => { - const schema = new GraphQLSchema({ - query: SomeObjectType, - subscription: SomeObjectType, - }); - expectJSON(validateSchema(schema)).toDeepEqual([ - { - message: - 'All root types must be different, SomeObject is already used for "query" and cannot also be used for "subscription".', - locations: [{ line: 6, column: 3 }], - }, - ]); - - const schemaFromSDL = buildSchema(` + it('rejects a Schema where the same type is used for multiple root types', () => { + const schema = buildSchema(` type SomeObject { - f: SomeObject + field: String + } + + type UniqueObject { + field: String } schema { query: SomeObject + mutation: UniqueObject subscription: SomeObject } `); - expectJSON(validateSchema(schemaFromSDL)).toDeepEqual([ - { - message: - 'All root types must be different, SomeObject is already used for "query" and cannot also be used for "subscription".', - locations: [{ line: 8, column: 23 }], - }, - ]); - }); - it('rejects a Schema where the same type is used for the "mutation" and "subscription" root types', () => { - const schema = new GraphQLSchema({ - query: new GraphQLObjectType({ - name: 'Query', - fields: { hello: { type: GraphQLString } }, - }), - mutation: SomeObjectType, - subscription: SomeObjectType, - }); expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'All root types must be different, SomeObject is already used for "mutation" and cannot also be used for "subscription".', - locations: [{ line: 6, column: 3 }], + 'All root types must be different, "SomeObject" type is used as query and subscription root types.', + locations: [ + { line: 11, column: 16 }, + { line: 13, column: 23 }, + ], }, ]); + }); - const schemaFromSDL = buildSchema(` + it('rejects a Schema where the same type is used for all root types', () => { + const schema = buildSchema(` type SomeObject { - f: SomeObject - } - - type Query { - f: SomeObject + field: String } schema { - query: Query + query: SomeObject mutation: SomeObject subscription: SomeObject } `); - expectJSON(validateSchema(schemaFromSDL)).toDeepEqual([ + + expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'All root types must be different, SomeObject is already used for "mutation" and cannot also be used for "subscription".', - locations: [{ line: 13, column: 23 }], + 'All root types must be different, "SomeObject" type is used as query, mutation, and subscription root types.', + locations: [ + { line: 7, column: 16 }, + { line: 8, column: 19 }, + { line: 9, column: 23 }, + ], }, ]); }); diff --git a/src/type/validate.ts b/src/type/validate.ts index 65a522937c..c6385d27fb 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -1,4 +1,6 @@ +import { AccumulatorMap } from '../jsutils/AccumulatorMap'; import { capitalize } from '../jsutils/capitalize'; +import { andList } from '../jsutils/formatList'; import { inspect } from '../jsutils/inspect'; import type { Maybe } from '../jsutils/Maybe'; @@ -118,50 +120,40 @@ function validateRootTypes(context: SchemaValidationContext): void { context.reportError('Query root type must be provided.', schema.astNode); } + const rootTypesMap = new AccumulatorMap< + GraphQLObjectType, + OperationTypeNode + >(); for (const operationType of Object.values(OperationTypeNode)) { const rootType = schema.getRootType(operationType); - if (rootType != null && !isObjectType(rootType)) { - const operationTypeStr = capitalize(operationType); - const rootTypeStr = inspect(rootType); - context.reportError( - operationType === OperationTypeNode.QUERY - ? `${operationTypeStr} root type must be Object type, it cannot be ${rootTypeStr}.` - : `${operationTypeStr} root type must be Object type if provided, it cannot be ${rootTypeStr}.`, - getOperationTypeNode(schema, operationType) ?? - (rootType as any).astNode, - ); + if (rootType != null) { + if (!isObjectType(rootType)) { + const operationTypeStr = capitalize(operationType); + const rootTypeStr = inspect(rootType); + context.reportError( + operationType === OperationTypeNode.QUERY + ? `${operationTypeStr} root type must be Object type, it cannot be ${rootTypeStr}.` + : `${operationTypeStr} root type must be Object type if provided, it cannot be ${rootTypeStr}.`, + getOperationTypeNode(schema, operationType) ?? + (rootType as any).astNode, + ); + } else { + rootTypesMap.add(rootType, operationType); + } } } - if (queryType && mutationType && queryType === mutationType) { - context.reportError( - 'All root types must be different, ' + - queryType.name + - ' is already used for "query" and cannot also be used for "mutation".', - getOperationTypeNode(schema, OperationTypeNode.MUTATION) ?? - mutationType.astNode, - ); - } - - if (queryType && subscriptionType && queryType === subscriptionType) { - context.reportError( - 'All root types must be different, ' + - queryType.name + - ' is already used for "query" and cannot also be used for "subscription".', - getOperationTypeNode(schema, OperationTypeNode.SUBSCRIPTION) ?? - subscriptionType.astNode, - ); - } - - if (mutationType && subscriptionType && mutationType === subscriptionType) { - context.reportError( - 'All root types must be different, ' + - mutationType.name + - ' is already used for "mutation" and cannot also be used for "subscription".', - getOperationTypeNode(schema, OperationTypeNode.SUBSCRIPTION) ?? - subscriptionType.astNode, - ); + for (const [rootType, operationTypes] of rootTypesMap.entries()) { + if (operationTypes.length > 1) { + const operationList = andList(operationTypes); + context.reportError( + `All root types must be different, "${rootType.name}" type is used as ${operationList} root types.`, + operationTypes.map((operationType) => + getOperationTypeNode(schema, operationType), + ), + ); + } } }