Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extract check for unique type names into separate rule #998

Merged
merged 6 commits into from
Feb 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 1 addition & 6 deletions src/Utils/BuildSchema.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 1 addition & 9 deletions src/Utils/SchemaExtender.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions src/Validator/DocumentValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(),
Expand Down
67 changes: 67 additions & 0 deletions src/Validator/Rules/UniqueTypeNames.php
Original file line number Diff line number Diff line change
@@ -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,
];
}
}
25 changes: 0 additions & 25 deletions tests/Utils/BuildSchemaLegacyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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);
}
}
79 changes: 0 additions & 79 deletions tests/Utils/SchemaExtenderLegacyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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

/**
Expand Down
Loading