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

Author: spawnia

src/Language/Printer.php

@@ -77,9 +77,7 @@ public static function doPrint(Node $ast): string
         return $instance->printAST($ast);
     }
 
-    protected function __construct()
-    {
-    }
+    protected function __construct() {}
 
     /**
      * Recursively traverse an AST depth-first and produce a pretty string.

src/Utils/SchemaPrinter.php

@@ -137,61 +137,58 @@ protected static function printFilteredSchema(Schema $schema, callable $directiv
         return \implode("\n\n", \array_filter($elements)) . "\n";
     }
 
-    protected static function printSchemaDefinition(Schema $schema): string
+    protected static function printSchemaDefinition(Schema $schema): ?string
     {
-        if (static::isSchemaOfCommonNames($schema)) {
-            return '';
-        }
-
-        $operationTypes = [];
-
         $queryType = $schema->getQueryType();
-        if ($queryType !== null) {
-            $operationTypes[] = "  query: {$queryType->name}";
-        }
-
         $mutationType = $schema->getMutationType();
-        if ($mutationType !== null) {
-            $operationTypes[] = "  mutation: {$mutationType->name}";
-        }
-
         $subscriptionType = $schema->getSubscriptionType();
-        if ($subscriptionType !== null) {
-            $operationTypes[] = "  subscription: {$subscriptionType->name}";
+
+        // Special case: When a schema has no root operation types, no valid schema
+        // definition can be printed.
+        if ($queryType === null && $mutationType === null && $subscriptionType === null) {
+            return null;
         }
 
-        $typesString = \implode("\n", $operationTypes);
+        // TODO add condition for schema.description
+        // Only print a schema definition if there is a description or if it should
+        // not be omitted because of having default type names.
+        if (! self::hasDefaultRootOperationTypes($schema)) {
+            return "schema {\n"
+                . ($queryType !== null ? "  query: {$queryType->name}\n" : '')
+                . ($mutationType !== null ? "  mutation: {$mutationType->name}\n" : '')
+                . ($subscriptionType !== null ? "  subscription: {$subscriptionType->name}\n" : '')
+                . '}';
+        }
 
-        return "schema {\n{$typesString}\n}";
+        return null;
     }
 
     /**
      * GraphQL schema define root types for each type of operation. These types are
      * the same as any other type and can be named in any manner, however there is
      * a common naming convention:.
      *
+     * ```graphql
      *   schema {
      *     query: Query
      *     mutation: Mutation
+     *     subscription: Subscription
      *   }
+     * ```
      *
      * 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.
      */
-    protected static function isSchemaOfCommonNames(Schema $schema): bool
+    protected static function hasDefaultRootOperationTypes(Schema $schema): bool
     {
-        $queryType = $schema->getQueryType();
-        if ($queryType !== null && $queryType->name !== 'Query') {
-            return false;
-        }
-
-        $mutationType = $schema->getMutationType();
-        if ($mutationType !== null && $mutationType->name !== 'Mutation') {
-            return false;
-        }
-
-        $subscriptionType = $schema->getSubscriptionType();
-
-        return $subscriptionType === null || $subscriptionType->name === 'Subscription';
+        return $schema->getQueryType() === $schema->getType('Query')
+            && $schema->getMutationType() === $schema->getType('Mutation')
+            && $schema->getSubscriptionType() === $schema->getType('Subscription');
     }
 
     /**

tests/Language/SchemaPrinterTest.php

@@ -7,6 +7,8 @@
 use GraphQL\Language\AST\ScalarTypeDefinitionNode;
 use GraphQL\Language\Parser;
 use GraphQL\Language\Printer;
+use GraphQL\Utils\BuildSchema;
+use GraphQL\Utils\SchemaPrinter;
 use PHPUnit\Framework\TestCase;
 
 use function Safe\file_get_contents;
@@ -177,4 +179,14 @@ enum UndefinedEnum
             $printed
         );
     }
+
+    /**
+     * it('prints viral schema correctly', () => {.
+     */
+    public function testPrintsViralSchemaCorrectly(): void
+    {
+        $schemaSDL = \Safe\file_get_contents(__DIR__ . '/../viralSchema.graphql');
+        $schema = BuildSchema::build($schemaSDL);
+        self::assertSame($schemaSDL, SchemaPrinter::doPrint($schema));
+    }
 }

tests/Utils/BuildSchemaTest.php

@@ -1386,6 +1386,22 @@ public function testThrowsOnUnknownTypes(): void
         BuildSchema::build($sdl, null, ['assumeValidSDL' => true])->assertValid();
     }
 
+    /**
+     * it('correctly processes viral schema', () => {.
+     */
+    public function testCorrectlyProcessesViralSchema(): void
+    {
+        $schema = BuildSchema::build(\Safe\file_get_contents(__DIR__ . '/../viralSchema.graphql'));
+
+        $queryType = $schema->getQueryType();
+        self::assertNotNull($queryType);
+        self::assertSame('Query', $queryType->name);
+        self::assertNotNull($schema->getType('Virus'));
+        self::assertNotNull($schema->getType('Mutation'));
+        // Though the viral schema has a 'Mutation' type, it is not used for the 'mutation' operation.
+        self::assertNull($schema->getMutationType());
+    }
+
     /**
      * @see https://github.com/webonyx/graphql-php/issues/997
      */

tests/viralSchema.graphql

@@ -0,0 +1,17 @@
+schema {
+  query: Query
+}
+
+type Query {
+  viruses: [Virus!]
+}
+
+type Virus {
+  name: String!
+  knownMutations: [Mutation!]!
+}
+
+type Mutation {
+  name: String!
+  geneSequence: String!
+}