Extract check for unique directive names into separate rule

Replicates graphql/graphql-js@c174522

Author: Cito

README.md

@@ -13,7 +13,7 @@ a query language for APIs created by Facebook.
 [![Code Style](https://img.shields.io/badge/code%20style-black-000000.svg)](https://github.com/ambv/black)
 
 The current version 1.0.1 of GraphQL-core-next is up-to-date with GraphQL.js version
-14.0.2. All parts of the API are covered by an extensive test suite of currently 1668
+14.0.2. All parts of the API are covered by an extensive test suite of currently 1675
 unit tests.
 
 

graphql/utilities/extend_schema.py

@@ -124,14 +124,6 @@ def extend_schema(
             check_extension_node(existing_type, def_)
             type_extensions_map[extended_type_name].append(def_)
         elif isinstance(def_, DirectiveDefinitionNode):
-            directive_name = def_.name.value
-            existing_directive = schema.get_directive(directive_name)
-            if existing_directive:
-                raise GraphQLError(
-                    f"Directive '{directive_name}' already exists"
-                    " in the schema. It cannot be redefined.",
-                    [def_],
-                )
             directive_definitions.append(def_)
 
     # If this document contains no new types, extensions, or directives then return the

graphql/validation/rules/unique_directive_names.py

@@ -0,0 +1,55 @@
+from typing import Dict
+
+from ...error import GraphQLError
+from ...language import NameNode, DirectiveDefinitionNode
+from . import SDLValidationContext, SDLValidationRule
+
+__all__ = [
+    "UniqueDirectiveNamesRule",
+    "duplicate_directive_name_message",
+    "existed_directive_name_message",
+]
+
+
+def duplicate_directive_name_message(directive_name: str) -> str:
+    return f"There can be only one directive named '{directive_name}'."
+
+
+def existed_directive_name_message(directive_name: str) -> str:
+    return (
+        f"Directive '{directive_name}' already exists in the schema."
+        " It cannot be redefined."
+    )
+
+
+class UniqueDirectiveNamesRule(SDLValidationRule):
+    """Unique directive names
+
+    A GraphQL document is only valid if all defined directives have unique names.
+    """
+
+    def __init__(self, context: SDLValidationContext) -> None:
+        super().__init__(context)
+        self.known_directive_names: Dict[str, NameNode] = {}
+        self.schema = context.schema
+
+    def enter_directive_definition(self, node: DirectiveDefinitionNode, *_args):
+        directive_name = node.name.value
+
+        if self.schema and self.schema.get_directive(directive_name):
+            self.report_error(
+                GraphQLError(
+                    existed_directive_name_message(directive_name), [node.name]
+                )
+            )
+        else:
+            if directive_name in self.known_directive_names:
+                self.report_error(
+                    GraphQLError(
+                        duplicate_directive_name_message(directive_name),
+                        [self.known_directive_names[directive_name], node.name],
+                    )
+                )
+            else:
+                self.known_directive_names[directive_name] = node.name
+            return self.SKIP

graphql/validation/rules/unique_type_names.py

@@ -4,7 +4,11 @@
 from ...language import NameNode, TypeDefinitionNode
 from . import SDLValidationContext, SDLValidationRule
 
-__all__ = ["UniqueTypeNamesRule", "existed_type_name_message"]
+__all__ = [
+    "UniqueTypeNamesRule",
+    "duplicate_type_name_message",
+    "existed_type_name_message",
+]
 
 
 def duplicate_type_name_message(type_name: str) -> str:

graphql/validation/specified_rules.py

@@ -83,6 +83,7 @@
 # Schema definition language:
 from .rules.lone_schema_definition import LoneSchemaDefinitionRule
 from .rules.unique_type_names import UniqueTypeNamesRule
+from .rules.unique_directive_names import UniqueDirectiveNamesRule
 from .rules.known_argument_names import KnownArgumentNamesOnDirectivesRule
 from .rules.provided_required_arguments import ProvidedRequiredArgumentsOnDirectivesRule
 
@@ -126,6 +127,7 @@
 specified_sdl_rules: List[RuleType] = [
     LoneSchemaDefinitionRule,
     UniqueTypeNamesRule,
+    UniqueDirectiveNamesRule,
     KnownDirectivesRule,
     UniqueDirectivesPerLocationRule,
     KnownArgumentNamesOnDirectivesRule,

tests/utilities/test_extend_schema.py

@@ -1049,70 +1049,13 @@ def does_not_allow_replacing_a_default_directive():
         sdl = """
             directive @include(if: Boolean!) on FIELD | FRAGMENT_SPREAD
             """
-        with raises(GraphQLError) as exc_info:
+        with raises(TypeError) as exc_info:
             extend_test_schema(sdl)
         assert str(exc_info.value).startswith(
             "Directive 'include' already exists in the schema."
             " It cannot be redefined."
         )
 
-    def does_not_allow_replacing_a_custom_directive():
-        extended_schema = extend_test_schema(
-            """
-            directive @meow(if: Boolean!) on FIELD | FRAGMENT_SPREAD
-            """
-        )
-
-        replacement_ast = parse(
-            """
-            directive @meow(if: Boolean!) on FIELD | QUERY
-            """
-        )
-
-        with raises(GraphQLError) as exc_info:
-            extend_schema(extended_schema, replacement_ast)
-        assert str(exc_info.value).startswith(
-            "Directive 'meow' already exists in the schema. It cannot be redefined."
-        )
-
-    def does_not_allow_replacing_an_existing_field():
-        def existing_field_error(type_, field):
-            return (
-                f"Field '{type_}.{field}' already exists in the schema."
-                " It cannot also be defined in this type extension."
-            )
-
-        type_sdl = """
-            extend type Bar {
-              foo: Foo
-            }
-            """
-        with raises(GraphQLError) as exc_info:
-            extend_test_schema(type_sdl)
-        assert str(exc_info.value).startswith(existing_field_error("Bar", "foo"))
-
-        interface_sdl = """
-            extend interface SomeInterface {
-              some: Foo
-            }
-            """
-        with raises(GraphQLError) as exc_info:
-            extend_test_schema(interface_sdl)
-        assert str(exc_info.value).startswith(
-            existing_field_error("SomeInterface", "some")
-        )
-
-        input_sdl = """
-            extend input SomeInput {
-              fooArg: String
-            }
-            """
-        with raises(GraphQLError) as exc_info:
-            extend_test_schema(input_sdl)
-        assert str(exc_info.value).startswith(
-            existing_field_error("SomeInput", "fooArg")
-        )
-
     def does_not_allow_replacing_an_existing_enum_value():
         sdl = """
             extend enum SomeEnum {

tests/validation/test_unique_directive_names.py

@@ -0,0 +1,103 @@
+from functools import partial
+
+from graphql.utilities import build_schema
+from graphql.validation.rules.unique_directive_names import (
+    UniqueDirectiveNamesRule,
+    duplicate_directive_name_message,
+    existed_directive_name_message,
+)
+
+from .harness import assert_sdl_validation_errors
+
+assert_errors = partial(assert_sdl_validation_errors, UniqueDirectiveNamesRule)
+
+assert_valid = partial(assert_errors, errors=[])
+
+
+def describe_validate_unique_directive_names():
+    def no_directive():
+        assert_valid(
+            """
+            type Foo
+            """
+        )
+
+    def one_directive():
+        assert_valid(
+            """
+            directive @foo on SCHEMA
+            """
+        )
+
+    def many_directives():
+        assert_valid(
+            """
+            directive @foo on SCHEMA
+            directive @bar on SCHEMA
+            directive @baz on SCHEMA
+            """
+        )
+
+    def directive_and_non_directive_definitions_named_the_same():
+        assert_valid(
+            """
+            query foo { __typename }
+            fragment foo on foo { __typename }
+            type foo
+
+            directive @foo on SCHEMA
+            """
+        )
+
+    def directives_named_the_same():
+        assert_errors(
+            """
+            directive @foo on SCHEMA
+
+            directive @foo on SCHEMA
+            """,
+            [
+                {
+                    "message": duplicate_directive_name_message("foo"),
+                    "locations": [(2, 24), (4, 24)],
+                }
+            ],
+        )
+
+    def adding_new_directive_to_existing_schema():
+        schema = build_schema("directive @foo on SCHEMA")
+
+        assert_valid("directive @bar on SCHEMA", schema=schema)
+
+    def adding_new_directive_with_standard_name_to_existing_schema():
+        schema = build_schema("type foo")
+
+        assert_errors(
+            "directive @skip on SCHEMA",
+            [
+                {
+                    "message": existed_directive_name_message("skip"),
+                    "locations": [(1, 12)],
+                }
+            ],
+            schema,
+        )
+
+    def adding_new_directive_to_existing_schema_with_same_named_type():
+        schema = build_schema("type foo")
+
+        assert_valid("directive @foo on SCHEMA", schema=schema)
+
+    def adding_conflicting_directives_to_existing_schema():
+        schema = build_schema("directive @foo on SCHEMA")
+
+        assert_errors(
+            "directive @foo on SCHEMA",
+            [
+                {
+                    "message": existed_directive_name_message("foo"),
+                    "locations": [(1, 12)],
+                }
+            ],
+            schema,
+        )