Add validation rule UniqueEnumValueNames

CHANGELOG.md

@@ -33,6 +33,7 @@ You can find and compare releases at the [GitHub release page](https://github.co
 - Expose structured enumeration of directive locations
 - Add `AST::concatAST()` utility
 - Allow lazy input object fields
+- Add validation rule `UniqueEnumValueNames`
 
 ### Optimized
 

src/Language/VisitorOperation.php

@@ -6,12 +6,9 @@
 
 class VisitorOperation
 {
-    /** @var bool */
-    public $doBreak;
+    public bool $doBreak = false;
 
-    /** @var bool */
-    public $doContinue;
+    public bool $doContinue = false;
 
-    /** @var bool */
-    public $removeNode;
+    public bool $removeNode = false;
 }

src/Type/SchemaValidationContext.php

@@ -9,7 +9,6 @@
 use GraphQL\Language\AST\DirectiveNode;
 use GraphQL\Language\AST\EnumTypeDefinitionNode;
 use GraphQL\Language\AST\EnumTypeExtensionNode;
-use GraphQL\Language\AST\EnumValueDefinitionNode;
 use GraphQL\Language\AST\FieldDefinitionNode;
 use GraphQL\Language\AST\InputObjectTypeDefinitionNode;
 use GraphQL\Language\AST\InputObjectTypeExtensionNode;
@@ -958,15 +957,6 @@ private function validateEnumValues(EnumType $enumType): void
         foreach ($enumValues as $enumValue) {
             $valueName = $enumValue->name;
 
-            // Ensure no duplicates
-            $allNodes = $this->getEnumValueNodes($enumType, $valueName);
-            if (count($allNodes) > 1) {
-                $this->reportError(
-                    sprintf('Enum type %s can include value %s only once.', $enumType->name, $valueName),
-                    $allNodes
-                );
-            }
-
             // Ensure valid name.
             $this->validateName($enumValue);
             if ($valueName === 'true' || $valueName === 'false' || $valueName === 'null') {
@@ -988,29 +978,6 @@ private function validateEnumValues(EnumType $enumType): void
         }
     }
 
-    /**
-     * @return array<int, EnumValueDefinitionNode>
-     */
-    private function getEnumValueNodes(EnumType $enum, string $valueName): array
-    {
-        $allNodes = $enum->astNode !== null
-            ? array_merge([$enum->astNode], $enum->extensionASTNodes)
-            : $enum->extensionASTNodes;
-
-        $values = [];
-        foreach ($allNodes as $node) {
-            foreach ($node->values as $value) {
-                if ($value->name->value !== $valueName) {
-                    continue;
-                }
-
-                $values[] = $value;
-            }
-        }
-
-        return $values;
-    }
-
     private function validateInputFields(InputObjectType $inputObj): void
     {
         $fieldMap = $inputObj->getFields();

src/Utils/SchemaExtender.php

@@ -20,7 +20,6 @@
 use GraphQL\Type\Definition\CustomScalarType;
 use GraphQL\Type\Definition\Directive;
 use GraphQL\Type\Definition\EnumType;
-use GraphQL\Type\Definition\EnumValueDefinition;
 use GraphQL\Type\Definition\FieldArgument;
 use GraphQL\Type\Definition\ImplementingType;
 use GraphQL\Type\Definition\InputObjectType;
@@ -215,14 +214,9 @@ protected static function extendInputFieldMap(InputObjectType $type): array
     protected static function extendEnumValueMap(EnumType $type): array
     {
         $newValueMap = [];
-        /** @var array<string, EnumValueDefinition> $oldValueMap */
-        $oldValueMap = [];
-        foreach ($type->getValues() as $value) {
-            $oldValueMap[$value->name] = $value;
-        }
 
-        foreach ($oldValueMap as $key => $value) {
-            $newValueMap[$key] = [
+        foreach ($type->getValues() as $value) {
+            $newValueMap[$value->name] = [
                 'name' => $value->name,
                 'description' => $value->description,
                 'value' => $value->value,
@@ -239,12 +233,7 @@ protected static function extendEnumValueMap(EnumType $type): array
              */
             foreach (static::$typeExtensionsMap[$type->name] as $extension) {
                 foreach ($extension->values as $value) {
-                    $valueName = $value->name->value;
-                    if (isset($oldValueMap[$valueName])) {
-                        throw new Error('Enum value "' . $type->name . '.' . $valueName . '" already exists in the schema. It cannot also be defined in this type extension.', [$value]);
-                    }
-
-                    $newValueMap[$valueName] = static::$astBuilder->buildEnumValue($value);
+                    $newValueMap[$value->name->value] = static::$astBuilder->buildEnumValue($value);
                 }
             }
         }

src/Validator/DocumentValidator.php

@@ -36,6 +36,7 @@
 use GraphQL\Validator\Rules\SingleFieldSubscription;
 use GraphQL\Validator\Rules\UniqueArgumentNames;
 use GraphQL\Validator\Rules\UniqueDirectivesPerLocation;
+use GraphQL\Validator\Rules\UniqueEnumValueNames;
 use GraphQL\Validator\Rules\UniqueFragmentNames;
 use GraphQL\Validator\Rules\UniqueInputFieldNames;
 use GraphQL\Validator\Rules\UniqueOperationNames;
@@ -201,6 +202,7 @@ public static function sdlRules()
                 KnownArgumentNamesOnDirectives::class        => new KnownArgumentNamesOnDirectives(),
                 UniqueDirectivesPerLocation::class           => new UniqueDirectivesPerLocation(),
                 UniqueArgumentNames::class                   => new UniqueArgumentNames(),
+                UniqueEnumValueNames::class                  => new UniqueEnumValueNames(),
                 UniqueInputFieldNames::class                 => new UniqueInputFieldNames(),
                 ProvidedRequiredArgumentsOnDirectives::class => new ProvidedRequiredArgumentsOnDirectives(),
             ];

src/Validator/Rules/UniqueEnumValueNames.php

@@ -0,0 +1,70 @@
+<?php
+
+declare(strict_types=1);
+
+namespace GraphQL\Validator\Rules;
+
+use GraphQL\Error\Error;
+use GraphQL\Language\AST\EnumTypeDefinitionNode;
+use GraphQL\Language\AST\EnumTypeExtensionNode;
+use GraphQL\Language\AST\EnumValueNode;
+use GraphQL\Language\AST\NodeKind;
+use GraphQL\Language\Visitor;
+use GraphQL\Language\VisitorOperation;
+use GraphQL\Type\Definition\EnumType;
+use GraphQL\Validator\SDLValidationContext;
+
+class UniqueEnumValueNames extends ValidationRule
+{
+    public function getSDLVisitor(SDLValidationContext $context): array
+    {
+        /** @var array<string, array<string, EnumValueNode>> $knownValueNames */
+        $knownValueNames = [];
+
+        /**
+         * @param EnumTypeDefinitionNode|EnumTypeExtensionNode $enum
+         */
+        $checkValueUniqueness = static function ($enum) use ($context, &$knownValueNames): VisitorOperation {
+            $typeName = $enum->name->value;
+
+            $schema       = $context->getSchema();
+            $existingType = $schema !== null
+                ? $schema->getType($typeName)
+                : null;
+
+            $valueNodes = $enum->values;
+
+            if (! isset($knownValueNames[$typeName])) {
+                $knownValueNames[$typeName] = [];
+            }
+
+            $valueNames = &$knownValueNames[$typeName];
+
+            foreach ($valueNodes as $valueDef) {
+                $valueNameNode = $valueDef->name;
+                $valueName     = $valueNameNode->value;
+
+                if ($existingType instanceof EnumType && $existingType->getValue($valueName) !== null) {
+                    $context->reportError(new Error(
+                        "Enum value \"${typeName}.${valueName}\" already exists in the schema. It cannot also be defined in this type extension.",
+                        $valueNameNode
+                    ));
+                } elseif (isset($valueNames[$valueName])) {
+                    $context->reportError(new Error(
+                        "Enum value \"${typeName}.${valueName}\" can only be defined once.",
+                        [$valueNames[$valueName], $valueNameNode]
+                    ));
+                } else {
+                    $valueNames[$valueName] = $valueNameNode;
+                }
+            }
+
+            return Visitor::skipNode();
+        };
+
+        return [
+            NodeKind::ENUM_TYPE_DEFINITION => $checkValueUniqueness,
+            NodeKind::ENUM_TYPE_EXTENSION  => $checkValueUniqueness,
+        ];
+    }
+}

tests/Type/ValidationTest.php

@@ -1093,32 +1093,6 @@ enum SomeEnum
         );
     }
 
-    /**
-     * @see it('rejects an Enum type with duplicate values')
-     */
-    public function testRejectsAnEnumTypeWithDuplicateValues(): void
-    {
-        $schema = BuildSchema::build('
-      type Query {
-        field: SomeEnum
-      }
-
-      enum SomeEnum {
-        SOME_VALUE
-        SOME_VALUE
-      }
-        ');
-        $this->assertMatchesValidationMessage(
-            $schema->validate(),
-            [
-                [
-                    'message'   => 'Enum type SomeEnum can include value SOME_VALUE only once.',
-                    'locations' => [['line' => 7, 'column' => 9], ['line' => 8, 'column' => 9]],
-                ],
-            ]
-        );
-    }
-
     public function testDoesNotAllowIsDeprecatedWithoutDeprecationReasonOnEnum(): void
     {
         $enum = new EnumType([

tests/Utils/SchemaExtenderTest.php

@@ -1392,25 +1392,6 @@ public function testDoesNotAllowReplacingADefaultDirective(): void
         }
     }
 
-    /**
-     * @see it('does not allow replacing an existing enum value')
-     */
-    public function testDoesNotAllowReplacingAnExistingEnumValue(): void
-    {
-        $sdl = '
-          extend enum SomeEnum {
-            ONE
-          }
-        ';
-
-        try {
-            $this->extendTestSchema($sdl);
-            self::fail();
-        } catch (Error $error) {
-            self::assertEquals('Enum value "SomeEnum.ONE" already exists in the schema. It cannot also be defined in this type extension.', $error->getMessage());
-        }
-    }
-
     /**
      * @see it('does not automatically include common root type names')
      */