Extract check for unique type names into separate rule (#998)

Author: vhenzl

CHANGELOG.md

@@ -54,6 +54,7 @@ You can find and compare releases at the [GitHub release page](https://github.co
 - Add ability to remove custom validation rules after adding them via `DocumentValidator::removeRule()`
 - Allow lazy enum values
 - Make `Node` implement `JsonSerializable`
+- Add SDL validation rule `UniqueTypeNames` (#998)
 
 ### Optimized
 

src/Utils/BuildSchema.php

@@ -140,12 +140,7 @@ public function buildSchema(): Schema
                     $schemaDef = $definition;
                     break;
                 case $definition instanceof TypeDefinitionNode:
-                    $typeName = $definition->name->value;
-                    if (isset($this->nodeMap[$typeName])) {
-                        throw new Error('Type "' . $typeName . '" was defined more than once.');
-                    }
-
-                    $this->nodeMap[$typeName] = $definition;
+                    $this->nodeMap[$definition->name->value] = $definition;
                     break;
                 case $definition instanceof DirectiveDefinitionNode:
                     $directiveDefs[] = $definition;

src/Utils/SchemaExtender.php

@@ -579,15 +579,7 @@ public static function extend(
             } elseif ($def instanceof SchemaTypeExtensionNode) {
                 $schemaExtensions[] = $def;
             } elseif ($def instanceof TypeDefinitionNode) {
-                $typeName = $def->name->value;
-
-                $type = $schema->getType($typeName);
-
-                if ($type !== null) {
-                    throw new Error("Type \"{$typeName}\" already exists in the schema. It cannot also be defined in this type definition.", [$def]);
-                }
-
-                $typeDefinitionMap[$typeName] = $def;
+                $typeDefinitionMap[$def->name->value] = $def;
             } elseif ($def instanceof TypeExtensionNode) {
                 $extendedTypeName = $def->name->value;
                 $existingType = $schema->getType($extendedTypeName);

src/Validator/DocumentValidator.php

@@ -40,6 +40,7 @@
 use GraphQL\Validator\Rules\UniqueInputFieldNames;
 use GraphQL\Validator\Rules\UniqueOperationNames;
 use GraphQL\Validator\Rules\UniqueOperationTypes;
+use GraphQL\Validator\Rules\UniqueTypeNames;
 use GraphQL\Validator\Rules\UniqueVariableNames;
 use GraphQL\Validator\Rules\ValidationRule;
 use GraphQL\Validator\Rules\ValuesOfCorrectType;
@@ -201,6 +202,7 @@ public static function sdlRules(): array
         return self::$sdlRules ??= [
             LoneSchemaDefinition::class => new LoneSchemaDefinition(),
             UniqueOperationTypes::class => new UniqueOperationTypes(),
+            UniqueTypeNames::class => new UniqueTypeNames(),
             KnownDirectives::class => new KnownDirectives(),
             KnownArgumentNamesOnDirectives::class => new KnownArgumentNamesOnDirectives(),
             UniqueDirectivesPerLocation::class => new UniqueDirectivesPerLocation(),

src/Validator/Rules/UniqueTypeNames.php

@@ -0,0 +1,67 @@
+<?php
+
+declare(strict_types=1);
+
+namespace GraphQL\Validator\Rules;
+
+use function array_key_exists;
+use GraphQL\Error\Error;
+use GraphQL\Language\AST\NameNode;
+use GraphQL\Language\AST\NodeKind;
+use GraphQL\Language\Visitor;
+use GraphQL\Language\VisitorOperation;
+use GraphQL\Validator\SDLValidationContext;
+
+/**
+ * Unique type names.
+ *
+ * A GraphQL document is only valid if all defined types have unique names.
+ */
+class UniqueTypeNames extends ValidationRule
+{
+    public function getSDLVisitor(SDLValidationContext $context): array
+    {
+        $schema = $context->getSchema();
+        /** @var array<string, NameNode> $knownTypeNames */
+        $knownTypeNames = [];
+        $checkTypeName = static function ($node) use ($context, $schema, &$knownTypeNames): ?VisitorOperation {
+            $typeName = $node->name->value;
+
+            if ($schema !== null && $schema->getType($typeName) !== null) {
+                $context->reportError(
+                    new Error(
+                        'Type "' . $typeName . '" already exists in the schema. It cannot also be defined in this type definition.',
+                        $node->name,
+                    ),
+                );
+
+                return null;
+            }
+
+            if (array_key_exists($typeName, $knownTypeNames)) {
+                $context->reportError(
+                    new Error(
+                        'There can be only one type named "' . $typeName . '".',
+                        [
+                            $knownTypeNames[$typeName],
+                            $node->name,
+                        ]
+                    ),
+                );
+            } else {
+                $knownTypeNames[$typeName] = $node->name;
+            }
+
+            return Visitor::skipNode();
+        };
+
+        return [
+            NodeKind::SCALAR_TYPE_DEFINITION => $checkTypeName,
+            NodeKind::OBJECT_TYPE_DEFINITION => $checkTypeName,
+            NodeKind::INTERFACE_TYPE_DEFINITION => $checkTypeName,
+            NodeKind::UNION_TYPE_DEFINITION => $checkTypeName,
+            NodeKind::ENUM_TYPE_DEFINITION => $checkTypeName,
+            NodeKind::INPUT_OBJECT_TYPE_DEFINITION => $checkTypeName,
+        ];
+    }
+}

tests/Utils/BuildSchemaLegacyTest.php

@@ -14,7 +14,6 @@
  * Their counterparts have been removed from `buildASTSchema-test.js` and moved elsewhere,
  * but these changes to `graphql-js` haven't been reflected in `graphql-php` yet.
  * TODO align with:
- *   - https://github.com/graphql/graphql-js/commit/257797a0ebdddd3da6e75b7c237fdc12a1a7c75a
  *   - https://github.com/graphql/graphql-js/commit/3b9ea61f2348215dee755f779caef83df749d2bb
  *   - https://github.com/graphql/graphql-js/commit/64a5c3448a201737f9218856786c51d66f2deabd.
  */
@@ -319,28 +318,4 @@ public function testDoesNotConsiderFragmentNames(): void
         $doc = Parser::parse($sdl);
         BuildSchema::buildAST($doc);
     }
-
-    /**
-     * @see it('Forbids duplicate type definitions')
-     */
-    public function testForbidsDuplicateTypeDefinitions(): void
-    {
-        $sdl = '
-            schema {
-              query: Repeated
-            }
-            
-            type Repeated {
-              id: Int
-            }
-            
-            type Repeated {
-              id: String
-            }
-        ';
-        $doc = Parser::parse($sdl);
-        $this->expectException(Error::class);
-        $this->expectExceptionMessage('Type "Repeated" was defined more than once.');
-        BuildSchema::buildAST($doc);
-    }
 }

tests/Utils/SchemaExtenderLegacyTest.php

@@ -29,7 +29,6 @@
  * but these changes to `graphql-js` haven't been reflected in `graphql-php` yet.
  * TODO align with:
  *   - https://github.com/graphql/graphql-js/commit/c1745228b2ae5ec89b8de36ea766d544607e21ea
- *   - https://github.com/graphql/graphql-js/commit/257797a0ebdddd3da6e75b7c237fdc12a1a7c75a
  *   - https://github.com/graphql/graphql-js/commit/3b9ea61f2348215dee755f779caef83df749d2bb
  *   - https://github.com/graphql/graphql-js/commit/e6a3f08cc92594f68a6e61d3d4b46a6d279f845e.
  */
@@ -264,84 +263,6 @@ public function testDoesNotAllowReplacingAnExistingField(): void
         }
     }
 
-    // Extract check for unique type names into separate rule
-
-    /**
-     * @see it('does not allow replacing an existing type')
-     */
-    public function testDoesNotAllowReplacingAnExistingType(): void
-    {
-        $existingTypeError = static function ($type): string {
-            return 'Type "' . $type . '" already exists in the schema. It cannot also be defined in this type definition.';
-        };
-
-        $typeSDL = '
-            type Bar
-        ';
-
-        try {
-            $this->extendTestSchema($typeSDL);
-            self::fail();
-        } catch (Error $error) {
-            self::assertEquals($existingTypeError('Bar'), $error->getMessage());
-        }
-
-        $scalarSDL = '
-          scalar SomeScalar
-        ';
-
-        try {
-            $this->extendTestSchema($scalarSDL);
-            self::fail();
-        } catch (Error $error) {
-            self::assertEquals($existingTypeError('SomeScalar'), $error->getMessage());
-        }
-
-        $interfaceSDL = '
-          interface SomeInterface
-        ';
-
-        try {
-            $this->extendTestSchema($interfaceSDL);
-            self::fail();
-        } catch (Error $error) {
-            self::assertEquals($existingTypeError('SomeInterface'), $error->getMessage());
-        }
-
-        $enumSDL = '
-          enum SomeEnum
-        ';
-
-        try {
-            $this->extendTestSchema($enumSDL);
-            self::fail();
-        } catch (Error $error) {
-            self::assertEquals($existingTypeError('SomeEnum'), $error->getMessage());
-        }
-
-        $unionSDL = '
-          union SomeUnion
-        ';
-
-        try {
-            $this->extendTestSchema($unionSDL);
-            self::fail();
-        } catch (Error $error) {
-            self::assertEquals($existingTypeError('SomeUnion'), $error->getMessage());
-        }
-
-        $inputSDL = '
-          input SomeInput
-        ';
-
-        try {
-            $this->extendTestSchema($inputSDL);
-            self::fail();
-        } catch (Error $error) {
-            self::assertEquals($existingTypeError('SomeInput'), $error->getMessage());
-        }
-    }
-
     // Validation: add support of SDL to KnownTypeNames
 
     /**