Review changed

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<string>,
   secondArg?: ReadonlyArray<string>,
 ) {
-  const [subMessage, suggestionsArg] = secondArg
+  const [subMessage, suggestions] = secondArg
     ? [firstArg as string, secondArg]
     : [undefined, firstArg as ReadonlyArray<string>];
 
+  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 + '?';
 }

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>): string {
+  return formatList('or', items);
+}
+
+/**
+ * Given [ A, B, C ] return 'A, B, and C'.
+ */
+export function andList(items: ReadonlyArray<string>): string {
+  return formatList('and', items);
+}
+
+function formatList(conjunction: string, items: ReadonlyArray<string>): 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;
+}

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 },
+        ],
       },
     ]);
   });

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),
+        ),
+      );
+    }
   }
 }