fix(validate): no reusing root types (#3453)

Co-authored-by: Ivan Goncharov <ivan.goncharov.ua@gmail.com>

Author: thomasheyenbrock

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

@@ -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(`

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