Skip to content

Commit 87baadc

Browse files
spawniaspawnia
authored
Add validation rule UniqueEnumValueNames (#1000)
1 parent 647e2cf commit 87baadc

9 files changed

+362
-98
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ You can find and compare releases at the [GitHub release page](https://github.co
3333
- Expose structured enumeration of directive locations
3434
- Add `AST::concatAST()` utility
3535
- Allow lazy input object fields
36+
- Add validation rule `UniqueEnumValueNames`
3637

3738
### Optimized
3839

‎src/Language/VisitorOperation.php

+3-6
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,9 @@
66

77
class VisitorOperation
88
{
9-
/** @var bool */
10-
public $doBreak;
9+
public bool $doBreak = false;
1110

12-
/** @var bool */
13-
public $doContinue;
11+
public bool $doContinue = false;
1412

15-
/** @var bool */
16-
public $removeNode;
13+
public bool $removeNode = false;
1714
}

‎src/Type/SchemaValidationContext.php

-33
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
use GraphQL\Language\AST\DirectiveNode;
1010
use GraphQL\Language\AST\EnumTypeDefinitionNode;
1111
use GraphQL\Language\AST\EnumTypeExtensionNode;
12-
use GraphQL\Language\AST\EnumValueDefinitionNode;
1312
use GraphQL\Language\AST\FieldDefinitionNode;
1413
use GraphQL\Language\AST\InputObjectTypeDefinitionNode;
1514
use GraphQL\Language\AST\InputObjectTypeExtensionNode;
@@ -958,15 +957,6 @@ private function validateEnumValues(EnumType $enumType): void
958957
foreach ($enumValues as $enumValue) {
959958
$valueName = $enumValue->name;
960959

961-
// Ensure no duplicates
962-
$allNodes = $this->getEnumValueNodes($enumType, $valueName);
963-
if (count($allNodes) > 1) {
964-
$this->reportError(
965-
sprintf('Enum type %s can include value %s only once.', $enumType->name, $valueName),
966-
$allNodes
967-
);
968-
}
969-
970960
// Ensure valid name.
971961
$this->validateName($enumValue);
972962
if ($valueName === 'true' || $valueName === 'false' || $valueName === 'null') {
@@ -988,29 +978,6 @@ private function validateEnumValues(EnumType $enumType): void
988978
}
989979
}
990980

991-
/**
992-
* @return array<int, EnumValueDefinitionNode>
993-
*/
994-
private function getEnumValueNodes(EnumType $enum, string $valueName): array
995-
{
996-
$allNodes = $enum->astNode !== null
997-
? array_merge([$enum->astNode], $enum->extensionASTNodes)
998-
: $enum->extensionASTNodes;
999-
1000-
$values = [];
1001-
foreach ($allNodes as $node) {
1002-
foreach ($node->values as $value) {
1003-
if ($value->name->value !== $valueName) {
1004-
continue;
1005-
}
1006-
1007-
$values[] = $value;
1008-
}
1009-
}
1010-
1011-
return $values;
1012-
}
1013-
1014981
private function validateInputFields(InputObjectType $inputObj): void
1015982
{
1016983
$fieldMap = $inputObj->getFields();

‎src/Utils/SchemaExtender.php

+3-14
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
use GraphQL\Type\Definition\CustomScalarType;
2121
use GraphQL\Type\Definition\Directive;
2222
use GraphQL\Type\Definition\EnumType;
23-
use GraphQL\Type\Definition\EnumValueDefinition;
2423
use GraphQL\Type\Definition\FieldArgument;
2524
use GraphQL\Type\Definition\ImplementingType;
2625
use GraphQL\Type\Definition\InputObjectType;
@@ -215,14 +214,9 @@ protected static function extendInputFieldMap(InputObjectType $type): array
215214
protected static function extendEnumValueMap(EnumType $type): array
216215
{
217216
$newValueMap = [];
218-
/** @var array<string, EnumValueDefinition> $oldValueMap */
219-
$oldValueMap = [];
220-
foreach ($type->getValues() as $value) {
221-
$oldValueMap[$value->name] = $value;
222-
}
223217

224-
foreach ($oldValueMap as $key => $value) {
225-
$newValueMap[$key] = [
218+
foreach ($type->getValues() as $value) {
219+
$newValueMap[$value->name] = [
226220
'name' => $value->name,
227221
'description' => $value->description,
228222
'value' => $value->value,
@@ -239,12 +233,7 @@ protected static function extendEnumValueMap(EnumType $type): array
239233
*/
240234
foreach (static::$typeExtensionsMap[$type->name] as $extension) {
241235
foreach ($extension->values as $value) {
242-
$valueName = $value->name->value;
243-
if (isset($oldValueMap[$valueName])) {
244-
throw new Error('Enum value "' . $type->name . '.' . $valueName . '" already exists in the schema. It cannot also be defined in this type extension.', [$value]);
245-
}
246-
247-
$newValueMap[$valueName] = static::$astBuilder->buildEnumValue($value);
236+
$newValueMap[$value->name->value] = static::$astBuilder->buildEnumValue($value);
248237
}
249238
}
250239
}

‎src/Validator/DocumentValidator.php

+2
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
use GraphQL\Validator\Rules\SingleFieldSubscription;
3737
use GraphQL\Validator\Rules\UniqueArgumentNames;
3838
use GraphQL\Validator\Rules\UniqueDirectivesPerLocation;
39+
use GraphQL\Validator\Rules\UniqueEnumValueNames;
3940
use GraphQL\Validator\Rules\UniqueFragmentNames;
4041
use GraphQL\Validator\Rules\UniqueInputFieldNames;
4142
use GraphQL\Validator\Rules\UniqueOperationNames;
@@ -201,6 +202,7 @@ public static function sdlRules()
201202
KnownArgumentNamesOnDirectives::class => new KnownArgumentNamesOnDirectives(),
202203
UniqueDirectivesPerLocation::class => new UniqueDirectivesPerLocation(),
203204
UniqueArgumentNames::class => new UniqueArgumentNames(),
205+
UniqueEnumValueNames::class => new UniqueEnumValueNames(),
204206
UniqueInputFieldNames::class => new UniqueInputFieldNames(),
205207
ProvidedRequiredArgumentsOnDirectives::class => new ProvidedRequiredArgumentsOnDirectives(),
206208
];

‎src/Validator/Rules/UniqueEnumValueNames.php

+70
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace GraphQL\Validator\Rules;
6+
7+
use GraphQL\Error\Error;
8+
use GraphQL\Language\AST\EnumTypeDefinitionNode;
9+
use GraphQL\Language\AST\EnumTypeExtensionNode;
10+
use GraphQL\Language\AST\EnumValueNode;
11+
use GraphQL\Language\AST\NodeKind;
12+
use GraphQL\Language\Visitor;
13+
use GraphQL\Language\VisitorOperation;
14+
use GraphQL\Type\Definition\EnumType;
15+
use GraphQL\Validator\SDLValidationContext;
16+
17+
class UniqueEnumValueNames extends ValidationRule
18+
{
19+
public function getSDLVisitor(SDLValidationContext $context): array
20+
{
21+
/** @var array<string, array<string, EnumValueNode>> $knownValueNames */
22+
$knownValueNames = [];
23+
24+
/**
25+
* @param EnumTypeDefinitionNode|EnumTypeExtensionNode $enum
26+
*/
27+
$checkValueUniqueness = static function ($enum) use ($context, &$knownValueNames): VisitorOperation {
28+
$typeName = $enum->name->value;
29+
30+
$schema = $context->getSchema();
31+
$existingType = $schema !== null
32+
? $schema->getType($typeName)
33+
: null;
34+
35+
$valueNodes = $enum->values;
36+
37+
if (! isset($knownValueNames[$typeName])) {
38+
$knownValueNames[$typeName] = [];
39+
}
40+
41+
$valueNames = &$knownValueNames[$typeName];
42+
43+
foreach ($valueNodes as $valueDef) {
44+
$valueNameNode = $valueDef->name;
45+
$valueName = $valueNameNode->value;
46+
47+
if ($existingType instanceof EnumType && $existingType->getValue($valueName) !== null) {
48+
$context->reportError(new Error(
49+
"Enum value \"${typeName}.${valueName}\" already exists in the schema. It cannot also be defined in this type extension.",
50+
$valueNameNode
51+
));
52+
} elseif (isset($valueNames[$valueName])) {
53+
$context->reportError(new Error(
54+
"Enum value \"${typeName}.${valueName}\" can only be defined once.",
55+
[$valueNames[$valueName], $valueNameNode]
56+
));
57+
} else {
58+
$valueNames[$valueName] = $valueNameNode;
59+
}
60+
}
61+
62+
return Visitor::skipNode();
63+
};
64+
65+
return [
66+
NodeKind::ENUM_TYPE_DEFINITION => $checkValueUniqueness,
67+
NodeKind::ENUM_TYPE_EXTENSION => $checkValueUniqueness,
68+
];
69+
}
70+
}

‎tests/Type/ValidationTest.php

-26
Original file line numberDiff line numberDiff line change
@@ -1093,32 +1093,6 @@ enum SomeEnum
10931093
);
10941094
}
10951095

1096-
/**
1097-
* @see it('rejects an Enum type with duplicate values')
1098-
*/
1099-
public function testRejectsAnEnumTypeWithDuplicateValues(): void
1100-
{
1101-
$schema = BuildSchema::build('
1102-
type Query {
1103-
field: SomeEnum
1104-
}
1105-
1106-
enum SomeEnum {
1107-
SOME_VALUE
1108-
SOME_VALUE
1109-
}
1110-
');
1111-
$this->assertMatchesValidationMessage(
1112-
$schema->validate(),
1113-
[
1114-
[
1115-
'message' => 'Enum type SomeEnum can include value SOME_VALUE only once.',
1116-
'locations' => [['line' => 7, 'column' => 9], ['line' => 8, 'column' => 9]],
1117-
],
1118-
]
1119-
);
1120-
}
1121-
11221096
public function testDoesNotAllowIsDeprecatedWithoutDeprecationReasonOnEnum(): void
11231097
{
11241098
$enum = new EnumType([

‎tests/Utils/SchemaExtenderTest.php

-19
Original file line numberDiff line numberDiff line change
@@ -1392,25 +1392,6 @@ public function testDoesNotAllowReplacingADefaultDirective(): void
13921392
}
13931393
}
13941394

1395-
/**
1396-
* @see it('does not allow replacing an existing enum value')
1397-
*/
1398-
public function testDoesNotAllowReplacingAnExistingEnumValue(): void
1399-
{
1400-
$sdl = '
1401-
extend enum SomeEnum {
1402-
ONE
1403-
}
1404-
';
1405-
1406-
try {
1407-
$this->extendTestSchema($sdl);
1408-
self::fail();
1409-
} catch (Error $error) {
1410-
self::assertEquals('Enum value "SomeEnum.ONE" already exists in the schema. It cannot also be defined in this type extension.', $error->getMessage());
1411-
}
1412-
}
1413-
14141395
/**
14151396
* @see it('does not automatically include common root type names')
14161397
*/

0 commit comments

Comments
 (0)