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 921ea87c4c..af82d30fbb 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -433,6 +433,86 @@ describe('Type System: A Schema must have Object root types', () => { }); }); +describe('Type System: Root types must all be different if provided', () => { + it('accepts a Schema with different root types', () => { + const schema = buildSchema(` + type SomeObject1 { + field: String + } + + type SomeObject2 { + field: String + } + + type SomeObject3 { + field: String + } + + schema { + query: SomeObject1 + mutation: SomeObject2 + subscription: SomeObject3 + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('rejects a Schema where the same type is used for multiple root types', () => { + const schema = buildSchema(` + type SomeObject { + field: String + } + + type UniqueObject { + field: String + } + + schema { + query: SomeObject + mutation: UniqueObject + subscription: SomeObject + } + `); + + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + '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 }, + ], + }, + ]); + }); + + it('rejects a Schema where the same type is used for all root types', () => { + const schema = buildSchema(` + type SomeObject { + field: String + } + + schema { + query: SomeObject + mutation: SomeObject + subscription: SomeObject + } + `); + + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + '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 }, + ], + }, + ]); + }); +}); + describe('Type System: Objects must have fields', () => { it('accepts an Object type with fields object', () => { const schema = buildSchema(` diff --git a/src/type/validate.ts b/src/type/validate.ts index 9ce665435a..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,18 +120,38 @@ 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); + 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); + } + } + } + + for (const [rootType, operationTypes] of rootTypesMap.entries()) { + if (operationTypes.length > 1) { + const operationList = andList(operationTypes); 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, + `All root types must be different, "${rootType.name}" type is used as ${operationList} root types.`, + operationTypes.map((operationType) => + getOperationTypeNode(schema, operationType), + ), ); } }