Fix ambiguity around when schema definition may be omitted (#3839)

This PR implements the tests and fixes necessary for [Spec RFC
#987](graphql/graphql-spec#987).

This PR is made of three main commits:

1. Test printing a schema that has `Query`, `Mutation` and `Virus`
types, but only supports `query` operations (via the `Query` type) and
does _not_ support `mutation` operations.
2. Test parsing this same schema text, and assert that the schema does
not have a mutation type.
3. Fix the printing of the schema.

Co-authored-by: Lee Byron <[email protected]>

Author: benjie

src/__testUtils__/viralSDL.ts

@@ -0,0 +1,19 @@
+export const viralSDL = `\
+schema {
+  query: Query
+}
+
+type Query {
+  viruses: [Virus!]
+}
+
+type Virus {
+  name: String!
+  knownMutations: [Mutation!]!
+}
+
+type Mutation {
+  name: String!
+  geneSequence: String!
+}\
+`;

src/__testUtils__/viralSchema.ts

@@ -0,0 +1,44 @@
+import {
+  GraphQLList,
+  GraphQLNonNull,
+  GraphQLObjectType,
+} from '../type/definition.js';
+import { GraphQLString } from '../type/scalars.js';
+import { GraphQLSchema } from '../type/schema.js';
+
+const Mutation = new GraphQLObjectType({
+  name: 'Mutation',
+  fields: {
+    name: {
+      type: new GraphQLNonNull(GraphQLString),
+    },
+    geneSequence: {
+      type: new GraphQLNonNull(GraphQLString),
+    },
+  },
+});
+
+const Virus = new GraphQLObjectType({
+  name: 'Virus',
+  fields: {
+    name: {
+      type: new GraphQLNonNull(GraphQLString),
+    },
+    knownMutations: {
+      type: new GraphQLNonNull(new GraphQLList(new GraphQLNonNull(Mutation))),
+    },
+  },
+});
+
+const Query = new GraphQLObjectType({
+  name: 'Query',
+  fields: {
+    viruses: {
+      type: new GraphQLList(new GraphQLNonNull(Virus)),
+    },
+  },
+});
+
+export const viralSchema = new GraphQLSchema({
+  query: Query,
+});

src/utilities/__tests__/buildASTSchema-test.ts

@@ -2,6 +2,7 @@ import { assert, expect } from 'chai';
 import { describe, it } from 'mocha';
 
 import { dedent } from '../../__testUtils__/dedent.js';
+import { viralSDL } from '../../__testUtils__/viralSDL.js';
 
 import type { Maybe } from '../../jsutils/Maybe.js';
 
@@ -1092,4 +1093,14 @@ describe('Schema Builder', () => {
       'Unknown type: "UnknownType".',
     );
   });
+
+  it('correctly processes viral schema', () => {
+    const schema = buildSchema(viralSDL);
+    expect(schema.getQueryType()).to.contain({ name: 'Query' });
+    expect(schema.getType('Virus')).to.contain({ name: 'Virus' });
+    expect(schema.getType('Mutation')).to.contain({ name: 'Mutation' });
+    // Though the viral schema has a 'Mutation' type, it is not used for the
+    // 'mutation' operation.
+    expect(schema.getMutationType()).to.equal(undefined);
+  });
 });

src/utilities/__tests__/printSchema-test.ts

@@ -2,6 +2,8 @@ import { expect } from 'chai';
 import { describe, it } from 'mocha';
 
 import { dedent, dedentString } from '../../__testUtils__/dedent.js';
+import { viralSchema } from '../../__testUtils__/viralSchema.js';
+import { viralSDL } from '../../__testUtils__/viralSDL.js';
 
 import { DirectiveLocation } from '../../language/directiveLocation.js';
 
@@ -867,4 +869,8 @@ describe('Type System Printer', () => {
       }
     `);
   });
+  it('prints viral schema correctly', () => {
+    const printed = printSchema(viralSchema);
+    expect(printed).to.equal(viralSDL);
+  });
 });

src/utilities/printSchema.ts

@@ -70,28 +70,28 @@ function printFilteredSchema(
 }
 
 function printSchemaDefinition(schema: GraphQLSchema): Maybe<string> {
-  if (schema.description == null && isSchemaOfCommonNames(schema)) {
-    return;
-  }
-
-  const operationTypes = [];
-
   const queryType = schema.getQueryType();
-  if (queryType) {
-    operationTypes.push(`  query: ${queryType.name}`);
-  }
-
   const mutationType = schema.getMutationType();
-  if (mutationType) {
-    operationTypes.push(`  mutation: ${mutationType.name}`);
-  }
-
   const subscriptionType = schema.getSubscriptionType();
-  if (subscriptionType) {
-    operationTypes.push(`  subscription: ${subscriptionType.name}`);
+
+  // Special case: When a schema has no root operation types, no valid schema
+  // definition can be printed.
+  if (!queryType && !mutationType && !subscriptionType) {
+    return;
   }
 
-  return printDescription(schema) + `schema {\n${operationTypes.join('\n')}\n}`;
+  // Only print a schema definition if there is a description or if it should
+  // not be omitted because of having default type names.
+  if (schema.description || !hasDefaultRootOperationTypes(schema)) {
+    return (
+      printDescription(schema) +
+      'schema {\n' +
+      (queryType ? `  query: ${queryType.name}\n` : '') +
+      (mutationType ? `  mutation: ${mutationType.name}\n` : '') +
+      (subscriptionType ? `  subscription: ${subscriptionType.name}\n` : '') +
+      '}'
+    );
+  }
 }
 
 /**
@@ -107,25 +107,20 @@ function printSchemaDefinition(schema: GraphQLSchema): Maybe<string> {
  *   }
  * ```
  *
- * When using this naming convention, the schema description can be omitted.
+ * When using this naming convention, the schema description can be omitted so
+ * long as these names are only used for operation types.
+ *
+ * Note however that if any of these default names are used elsewhere in the
+ * schema but not as a root operation type, the schema definition must still
+ * be printed to avoid ambiguity.
  */
-function isSchemaOfCommonNames(schema: GraphQLSchema): boolean {
-  const queryType = schema.getQueryType();
-  if (queryType && queryType.name !== 'Query') {
-    return false;
-  }
-
-  const mutationType = schema.getMutationType();
-  if (mutationType && mutationType.name !== 'Mutation') {
-    return false;
-  }
-
-  const subscriptionType = schema.getSubscriptionType();
-  if (subscriptionType && subscriptionType.name !== 'Subscription') {
-    return false;
-  }
-
-  return true;
+function hasDefaultRootOperationTypes(schema: GraphQLSchema): boolean {
+  /* eslint-disable eqeqeq */
+  return (
+    schema.getQueryType() == schema.getType('Query') &&
+    schema.getMutationType() == schema.getType('Mutation') &&
+    schema.getSubscriptionType() == schema.getType('Subscription')
+  );
 }
 
 export function printType(type: GraphQLNamedType): string {