Fix ambiguity around when schema definition may be omitted

Replicates graphql/graphql-js@f201681

Author: Cito

src/graphql/utilities/print_schema.py

@@ -70,51 +70,59 @@ def print_filtered_schema(
 
 def print_schema_definition(schema: GraphQLSchema) -> Optional[str]:
     """Print GraphQL schema definitions."""
-    if schema.description is None and is_schema_of_common_names(schema):
-        return None
-
-    operation_types = []
-
     query_type = schema.query_type
-    if query_type:
-        operation_types.append(f"  query: {query_type.name}")
-
     mutation_type = schema.mutation_type
-    if mutation_type:
-        operation_types.append(f"  mutation: {mutation_type.name}")
-
     subscription_type = schema.subscription_type
-    if subscription_type:
-        operation_types.append(f"  subscription: {subscription_type.name}")
 
-    return print_description(schema) + "schema {\n" + "\n".join(operation_types) + "\n}"
+    # Special case: When a schema has no root operation types, no valid schema
+    # definition can be printed.
+    if not query_type and not mutation_type and not subscription_type:
+        return None
+
+    # 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 or not has_default_root_operation_types(schema):
+        return (
+            print_description(schema)
+            + "schema {\n"
+            + (f"  query: {query_type.name}\n" if query_type else "")
+            + (f"  mutation: {mutation_type.name}\n" if mutation_type else "")
+            + (
+                f"  subscription: {subscription_type.name}\n"
+                if subscription_type
+                else ""
+            )
+            + "}"
+        )
+
+    return None
 
 
-def is_schema_of_common_names(schema: GraphQLSchema) -> bool:
-    """Check whether this schema uses the common naming convention.
+def has_default_root_operation_types(schema: GraphQLSchema) -> bool:
+    """Check whether a schema uses the default root operation type names.
 
     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:
+    naming convention::
 
-    schema {
-      query: Query
-      mutation: Mutation
-      subscription: Subscription
-    }
+        schema {
+          query: Query
+          mutation: Mutation
+          subscription: Subscription
+        }
 
-    When using this naming convention, the schema description can be omitted.
-    """
-    query_type = schema.query_type
-    if query_type and query_type.name != "Query":
-        return False
-
-    mutation_type = schema.mutation_type
-    if mutation_type and mutation_type.name != "Mutation":
-        return False
+    When using this naming convention, the schema description can be omitted so
+    long as these names are only used for operation types.
 
-    subscription_type = schema.subscription_type
-    return not subscription_type or subscription_type.name == "Subscription"
+    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.
+    """
+    return (
+        schema.query_type is schema.get_type("Query")
+        and schema.mutation_type is schema.get_type("Mutation")
+        and schema.subscription_type is schema.get_type("Subscription")
+    )
 
 
 def print_type(type_: GraphQLNamedType) -> str:

tests/utilities/test_build_ast_schema.py

@@ -38,7 +38,7 @@
 
 from ..fixtures import big_schema_sdl  # noqa: F401
 from ..star_wars_schema import star_wars_schema
-from ..utils import dedent
+from ..utils import dedent, viral_sdl
 
 try:
     from typing import TypeAlias
@@ -1188,6 +1188,21 @@ def throws_on_unknown_types():
             build_schema(sdl, assume_valid_sdl=True)
         assert str(exc_info.value).endswith("Unknown type: 'UnknownType'.")
 
+    def correctly_processes_viral_schema():
+        schema = build_schema(viral_sdl)
+        query_type = schema.query_type
+        assert isinstance(query_type, GraphQLNamedType)
+        assert query_type.name == "Query"
+        virus_type = schema.get_type("Virus")
+        assert isinstance(virus_type, GraphQLNamedType)
+        assert virus_type.name == "Virus"
+        mutation_type = schema.get_type("Mutation")
+        assert isinstance(mutation_type, GraphQLNamedType)
+        assert mutation_type.name == "Mutation"
+        # Though the viral schema has a 'Mutation' type, it is not used for the
+        # 'mutation' operation.
+        assert schema.mutation_type is None
+
     def describe_deepcopy_and_pickle():  # pragma: no cover
         sdl = print_schema(star_wars_schema)
 

tests/utilities/test_print_schema.py

@@ -27,7 +27,7 @@
     print_value,
 )
 
-from ..utils import dedent
+from ..utils import dedent, viral_schema, viral_sdl
 
 
 def expect_printed_schema(schema: GraphQLSchema) -> str:
@@ -865,6 +865,10 @@ def prints_introspection_schema():
             '''  # noqa: E501
         )
 
+    def prints_viral_schema_correctly():
+        printed = print_schema(viral_schema)
+        assert printed == viral_sdl
+
 
 def describe_print_value():
     def print_value_convenience_function():

tests/utils/__init__.py

@@ -4,10 +4,14 @@
 from .assert_matching_values import assert_matching_values
 from .dedent import dedent
 from .gen_fuzz_strings import gen_fuzz_strings
+from .viral_schema import viral_schema
+from .viral_sdl import viral_sdl
 
 __all__ = [
     "assert_matching_values",
     "assert_equal_awaitables_or_values",
     "dedent",
     "gen_fuzz_strings",
+    "viral_schema",
+    "viral_sdl",
 ]

tests/utils/viral_schema.py

@@ -0,0 +1,34 @@
+from graphql import GraphQLSchema
+from graphql.type import (
+    GraphQLField,
+    GraphQLList,
+    GraphQLNonNull,
+    GraphQLObjectType,
+    GraphQLString,
+)
+
+__all__ = ["viral_schema"]
+
+Mutation = GraphQLObjectType(
+    "Mutation",
+    {
+        "name": GraphQLField(GraphQLNonNull(GraphQLString)),
+        "geneSequence": GraphQLField(GraphQLNonNull(GraphQLString)),
+    },
+)
+
+Virus = GraphQLObjectType(
+    "Virus",
+    {
+        "name": GraphQLField(GraphQLNonNull(GraphQLString)),
+        "knownMutations": GraphQLField(
+            GraphQLNonNull(GraphQLList(GraphQLNonNull(Mutation)))
+        ),
+    },
+)
+
+Query = GraphQLObjectType(
+    "Query", {"viruses": GraphQLField(GraphQLList(GraphQLNonNull(Virus)))}
+)
+
+viral_schema = GraphQLSchema(Query)

tests/utils/viral_sdl.py

@@ -0,0 +1,21 @@
+__all__ = ["viral_sdl"]
+
+viral_sdl = """
+schema {
+  query: Query
+}
+
+type Query {
+  viruses: [Virus!]
+}
+
+type Virus {
+  name: String!
+  knownMutations: [Mutation!]!
+}
+
+type Mutation {
+  name: String!
+  geneSequence: String!
+}
+""".strip()