From e1835921ef9476f09fcc9f697d140909e8b1a878 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Va=C5=A1ek=20Henzl?= Date: Sun, 31 Oct 2021 14:39:09 +1300 Subject: [PATCH 01/14] Align `KnownTypeNamesTest` with `graphql-js` --- tests/Validator/KnownTypeNamesTest.php | 76 +++++++++++++------------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/tests/Validator/KnownTypeNamesTest.php b/tests/Validator/KnownTypeNamesTest.php index a1ef1f34b..089090fb8 100644 --- a/tests/Validator/KnownTypeNamesTest.php +++ b/tests/Validator/KnownTypeNamesTest.php @@ -2,8 +2,6 @@ namespace GraphQL\Tests\Validator; -use GraphQL\Language\SourceLocation; -use GraphQL\Tests\ErrorHelper; use GraphQL\Utils\BuildSchema; use GraphQL\Validator\Rules\KnownTypeNames; @@ -28,6 +26,7 @@ public function testKnownTypeNamesAreValid(): void pets { ... on Pet { name }, ...PetFields, ... { name } } } } + fragment PetFields on Pet { name } @@ -43,7 +42,7 @@ public function testUnknownTypeNamesAreInvalid(): void $this->expectFailsRule( new KnownTypeNames(), ' - query Foo($var: [JumbledUpLetters!]!) { + query Foo($var: JumbledUpLetters) { user(id: 4) { name pets { ... on Badger { name }, ...PetFields } @@ -54,9 +53,18 @@ public function testUnknownTypeNamesAreInvalid(): void } ', [ - $this->unknownType('JumbledUpLetters', [], 2, 24), - $this->unknownType('Badger', [], 5, 25), - $this->unknownType('Peat', ['Pet', 'Cat'], 8, 29), + [ + 'message' => 'Unknown type "JumbledUpLetters".', + 'locations' => [['line' => 2, 'column' => 23]], + ], + [ + 'message' => 'Unknown type "Badger".', + 'locations' => [['line' => 5, 'column' => 25]], + ], + [ + 'message' => 'Unknown type "Peat". Did you mean "Pet" or "Cat"?', + 'locations' => [['line' => 8, 'column' => 29]], + ], ] ); } @@ -78,9 +86,18 @@ public function testReferencesToStandardScalarsThatAreMissingInSchema(): void new KnownTypeNames(), $query, [ - $this->unknownType('Unknown type "ID".', [], 2, 19), - $this->unknownType('Unknown type "Float".', [], 2, 31), - $this->unknownType('Unknown type "Int".', [], 2, 44), + [ + 'message' => 'Unknown type "ID".', + 'locations' => [['line' => 2, 'column' => 19]], + ], + [ + 'message' => 'Unknown type "Float".', + 'locations' => [['line' => 2, 'column' => 31]], + ], + [ + 'message' => 'Unknown type "Int".', + 'locations' => [['line' => 2, 'column' => 44]], + ], ] ); } @@ -322,7 +339,6 @@ public function testUnknownTypeReferencesInsideExtensionDocument(): void self::markTestSkipped('Missing implementation for SDL validation for now'); $schema = BuildSchema::build('type A'); $sdl = ' - type B type SomeObject implements C { @@ -355,69 +371,53 @@ interface SomeInterface { [ [ 'message' => 'Unknown type "C". Did you mean "A" or "B"?', - 'locations' => [['line' => 5, 'column' => 36]], + 'locations' => [['line' => 4, 'column' => 36]], ], [ 'message' => 'Unknown type "D". Did you mean "A", "B", or "ID"?', - 'locations' => [['line' => 6, 'column' => 16]], + 'locations' => [['line' => 5, 'column' => 16]], ], [ 'message' => 'Unknown type "E". Did you mean "A" or "B"?', - 'locations' => [['line' => 6, 'column' => 20]], + 'locations' => [['line' => 5, 'column' => 20]], ], [ 'message' => 'Unknown type "F". Did you mean "A" or "B"?', - 'locations' => [['line' => 9, 'column' => 27]], + 'locations' => [['line' => 8, 'column' => 27]], ], [ 'message' => 'Unknown type "G". Did you mean "A" or "B"?', - 'locations' => [['line' => 9, 'column' => 31]], + 'locations' => [['line' => 8, 'column' => 31]], ], [ 'message' => 'Unknown type "H". Did you mean "A" or "B"?', - 'locations' => [['line' => 12, 'column' => 16]], + 'locations' => [['line' => 11, 'column' => 16]], ], [ 'message' => 'Unknown type "I". Did you mean "A", "B", or "ID"?', - 'locations' => [['line' => 12, 'column' => 20]], + 'locations' => [['line' => 11, 'column' => 20]], ], [ 'message' => 'Unknown type "J". Did you mean "A" or "B"?', - 'locations' => [['line' => 16, 'column' => 14]], + 'locations' => [['line' => 15, 'column' => 14]], ], [ 'message' => 'Unknown type "K". Did you mean "A" or "B"?', - 'locations' => [['line' => 19, 'column' => 37]], + 'locations' => [['line' => 18, 'column' => 37]], ], [ 'message' => 'Unknown type "L". Did you mean "A" or "B"?', - 'locations' => [['line' => 22, 'column' => 18]], + 'locations' => [['line' => 21, 'column' => 18]], ], [ 'message' => 'Unknown type "M". Did you mean "A" or "B"?', - 'locations' => [['line' => 23, 'column' => 21]], + 'locations' => [['line' => 22, 'column' => 21]], ], [ 'message' => 'Unknown type "N". Did you mean "A" or "B"?', - 'locations' => [['line' => 24, 'column' => 25]], + 'locations' => [['line' => 23, 'column' => 25]], ], ] ); } - - /** - * @param array $suggestedTypes - * - * @return array{ - * message: string, - * locations?: array - * } - */ - private function unknownType(string $typeName, array $suggestedTypes, int $line, int $column) - { - return ErrorHelper::create( - KnownTypeNames::unknownTypeMessage($typeName, $suggestedTypes), - [new SourceLocation($line, $column)] - ); - } } From 675299752d7a96bfe1ceaa65e848c7d55f4306bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Va=C5=A1ek=20Henzl?= Date: Sun, 31 Oct 2021 14:39:49 +1300 Subject: [PATCH 02/14] Add support of SDL to `KnownTypeNames` validation rule See https://github.com/graphql/graphql-js/commit/3b9ea61f2348215dee755f779caef83df749d2bb --- src/Utils/BuildSchema.php | 5 - src/Utils/SchemaExtender.php | 7 +- src/Validator/DocumentValidator.php | 1 + src/Validator/Rules/KnownTypeNames.php | 86 +++++++++--- tests/Utils/BuildSchemaLegacyTest.php | 172 ----------------------- tests/Utils/SchemaExtenderLegacyTest.php | 61 -------- tests/Validator/KnownTypeNamesTest.php | 4 - 7 files changed, 74 insertions(+), 262 deletions(-) diff --git a/src/Utils/BuildSchema.php b/src/Utils/BuildSchema.php index ac38caf7a..e0bc9b27d 100644 --- a/src/Utils/BuildSchema.php +++ b/src/Utils/BuildSchema.php @@ -228,11 +228,6 @@ private function getOperationTypes(SchemaDefinitionNode $schemaDef): array foreach ($schemaDef->operationTypes as $operationType) { $typeName = $operationType->type->name->value; $operation = $operationType->operation; - - if (! isset($this->nodeMap[$typeName])) { - throw new Error('Specified ' . $operation . ' type "' . $typeName . '" not found in document.'); - } - $opTypes[$operation] = $typeName; } diff --git a/src/Utils/SchemaExtender.php b/src/Utils/SchemaExtender.php index f780b176c..94978335d 100644 --- a/src/Utils/SchemaExtender.php +++ b/src/Utils/SchemaExtender.php @@ -7,6 +7,7 @@ use function array_merge; use function count; use GraphQL\Error\Error; +use GraphQL\Error\InvariantViolation; use GraphQL\Language\AST\DirectiveDefinitionNode; use GraphQL\Language\AST\DocumentNode; use GraphQL\Language\AST\EnumTypeExtensionNode; @@ -621,11 +622,11 @@ public static function extend( $typeDefinitionMap, static function (string $typeName) use ($schema): Type { $existingType = $schema->getType($typeName); - if (null !== $existingType) { - return static::extendNamedType($existingType); + if (null === $existingType) { + throw new InvariantViolation('Unknown type: "' . $typeName . '".'); } - throw new Error('Unknown type: "' . $typeName . '". Ensure that this type exists either in the original schema, or is added in a type definition.'); + return static::extendNamedType($existingType); }, $typeConfigDecorator ); diff --git a/src/Validator/DocumentValidator.php b/src/Validator/DocumentValidator.php index 5198fa9c7..448a8bdcf 100644 --- a/src/Validator/DocumentValidator.php +++ b/src/Validator/DocumentValidator.php @@ -185,6 +185,7 @@ public static function sdlRules(): array return self::$sdlRules ??= [ LoneSchemaDefinition::class => new LoneSchemaDefinition(), UniqueOperationTypes::class => new UniqueOperationTypes(), + KnownTypeNames::class => new KnownTypeNames(), KnownDirectives::class => new KnownDirectives(), KnownArgumentNamesOnDirectives::class => new KnownArgumentNamesOnDirectives(), UniqueDirectivesPerLocation::class => new UniqueDirectivesPerLocation(), diff --git a/src/Validator/Rules/KnownTypeNames.php b/src/Validator/Rules/KnownTypeNames.php index d06d3de8c..d6a9902b3 100644 --- a/src/Validator/Rules/KnownTypeNames.php +++ b/src/Validator/Rules/KnownTypeNames.php @@ -2,15 +2,21 @@ namespace GraphQL\Validator\Rules; +use function array_key_exists; use function array_keys; +use Closure; use function count; use GraphQL\Error\Error; use GraphQL\Language\AST\NamedTypeNode; +use GraphQL\Language\AST\Node; use GraphQL\Language\AST\NodeKind; -use GraphQL\Language\Visitor; -use GraphQL\Language\VisitorOperation; +use GraphQL\Language\AST\TypeDefinitionNode; +use GraphQL\Language\AST\TypeSystemDefinitionNode; +use GraphQL\Type\Definition\Type; use GraphQL\Utils\Utils; +use GraphQL\Validator\SDLValidationContext; use GraphQL\Validator\ValidationContext; +use function in_array; /** * Known type names. @@ -22,30 +28,66 @@ class KnownTypeNames extends ValidationRule { public function getVisitor(ValidationContext $context): array { - $skip = static function (): VisitorOperation { - return Visitor::skipNode(); - }; + return $this->getVisitorInternal($context); + } + + public function getSDLVisitor(SDLValidationContext $context): array + { + return $this->getVisitorInternal($context); + } + + /** + * @param ValidationContext|SDLValidationContext $context + * + * @return Closure[] + */ + private function getVisitorInternal($context): array + { + $schema = $context->getSchema(); + $existingTypesMap = null !== $schema + ? $schema->getTypeMap() + : []; + + /** @var array $definedTypes */ + $definedTypes = []; + foreach ($context->getDocument()->definitions as $def) { + if (! ($def instanceof TypeDefinitionNode)) { + continue; + } + + $definedTypes[$def->name->value] = true; + } + + $typeNames = [ + ...array_keys($existingTypesMap), + ...array_keys($definedTypes), + ]; + + $standardTypeNames = array_keys(Type::getAllBuiltInTypes()); return [ - // TODO: when validating IDL, re-enable these. Experimental version does not - // add unreferenced types, resulting in false-positive errors. Squelched - // errors for now. - NodeKind::OBJECT_TYPE_DEFINITION => $skip, - NodeKind::INTERFACE_TYPE_DEFINITION => $skip, - NodeKind::UNION_TYPE_DEFINITION => $skip, - NodeKind::INPUT_OBJECT_TYPE_DEFINITION => $skip, - NodeKind::NAMED_TYPE => static function (NamedTypeNode $node) use ($context): void { - $schema = $context->getSchema(); + NodeKind::NAMED_TYPE => static function (NamedTypeNode $node, $_1, $parent, $_2, $ancestors) use ($context, $existingTypesMap, $definedTypes, $typeNames, $standardTypeNames): void { $typeName = $node->name->value; - $type = $schema->getType($typeName); - if (null !== $type) { + if (array_key_exists($typeName, $existingTypesMap) || array_key_exists($typeName, $definedTypes)) { + return; + } + + $definitionNode = $ancestors[2] ?? $parent; + $isSDL = null !== $definitionNode && self::isSDLNode($definitionNode); + if ($isSDL && in_array($typeName, $standardTypeNames, true)) { return; } $context->reportError(new Error( static::unknownTypeMessage( $typeName, - Utils::suggestionList($typeName, array_keys($schema->getTypeMap())) + Utils::suggestionList( + $typeName, + $isSDL + // TODO: order + ? [...$typeNames, ...$standardTypeNames] + : $typeNames + ) ), [$node] )); @@ -65,4 +107,14 @@ public static function unknownTypeMessage(string $type, array $suggestedTypes): return $message; } + + /** + * @param Node|array $value + */ + public static function isSDLNode($value): bool + { + return $value instanceof Node + // TODO: should be (TypeSystemDefinitionNode || TypeSystemExtensionNode) + && $value instanceof TypeSystemDefinitionNode; + } } diff --git a/tests/Utils/BuildSchemaLegacyTest.php b/tests/Utils/BuildSchemaLegacyTest.php index 0bebde004..737895f8c 100644 --- a/tests/Utils/BuildSchemaLegacyTest.php +++ b/tests/Utils/BuildSchemaLegacyTest.php @@ -15,7 +15,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/257797a0ebdddd3da6e75b7c237fdc12a1a7c75a - * - https://github.com/graphql/graphql-js/commit/3b9ea61f2348215dee755f779caef83df749d2bb * - https://github.com/graphql/graphql-js/commit/64a5c3448a201737f9218856786c51d66f2deabd. */ class BuildSchemaLegacyTest extends TestCase @@ -149,177 +148,6 @@ interface Character { // Describe: Failures - /** - * @see it('Unknown type referenced') - */ - public function testUnknownTypeReferenced(): void - { - $this->expectExceptionObject(BuildSchema::unknownType('Bar')); - $sdl = ' - schema { - query: Hello - } - - type Hello { - bar: Bar - } - '; - $doc = Parser::parse($sdl); - $schema = BuildSchema::buildAST($doc); - $schema->getTypeMap(); - } - - /** - * @see it('Unknown type in interface list') - */ - public function testUnknownTypeInInterfaceList(): void - { - $this->expectExceptionObject(BuildSchema::unknownType('Bar')); - $sdl = ' - type Query implements Bar { - field: String - } - '; - $doc = Parser::parse($sdl); - $schema = BuildSchema::buildAST($doc); - $schema->getTypeMap(); - } - - /** - * @see it('Unknown type in union list') - */ - public function testUnknownTypeInUnionList(): void - { - $this->expectExceptionObject(BuildSchema::unknownType('Bar')); - $sdl = ' - union TestUnion = Bar - type Query { testUnion: TestUnion } - '; - $doc = Parser::parse($sdl); - $schema = BuildSchema::buildAST($doc); - $schema->getTypeMap(); - } - - /** - * @see it('Unknown query type') - */ - public function testUnknownQueryType(): void - { - $this->expectException(Error::class); - $this->expectExceptionMessage('Specified query type "Wat" not found in document.'); - $sdl = ' - schema { - query: Wat - } - - type Hello { - str: String - } - '; - $doc = Parser::parse($sdl); - BuildSchema::buildAST($doc); - } - - /** - * @see it('Unknown mutation type') - */ - public function testUnknownMutationType(): void - { - $this->expectException(Error::class); - $this->expectExceptionMessage('Specified mutation type "Wat" not found in document.'); - $sdl = ' - schema { - query: Hello - mutation: Wat - } - - type Hello { - str: String - } - '; - $doc = Parser::parse($sdl); - BuildSchema::buildAST($doc); - } - - /** - * @see it('Unknown subscription type') - */ - public function testUnknownSubscriptionType(): void - { - $this->expectException(Error::class); - $this->expectExceptionMessage('Specified subscription type "Awesome" not found in document.'); - $sdl = ' - schema { - query: Hello - mutation: Wat - subscription: Awesome - } - - type Hello { - str: String - } - - type Wat { - str: String - } - '; - $doc = Parser::parse($sdl); - BuildSchema::buildAST($doc); - } - - /** - * @see it('Does not consider directive names') - */ - public function testDoesNotConsiderDirectiveNames(): void - { - $sdl = ' - schema { - query: Foo - } - - directive @Foo on QUERY - '; - $doc = Parser::parse($sdl); - $this->expectExceptionMessage('Specified query type "Foo" not found in document.'); - BuildSchema::build($doc); - } - - /** - * @see it('Does not consider operation names') - */ - public function testDoesNotConsiderOperationNames(): void - { - $this->expectException(Error::class); - $this->expectExceptionMessage('Specified query type "Foo" not found in document.'); - $sdl = ' - schema { - query: Foo - } - - query Foo { field } - '; - $doc = Parser::parse($sdl); - BuildSchema::buildAST($doc); - } - - /** - * @see it('Does not consider fragment names') - */ - public function testDoesNotConsiderFragmentNames(): void - { - $this->expectException(Error::class); - $this->expectExceptionMessage('Specified query type "Foo" not found in document.'); - $sdl = ' - schema { - query: Foo - } - - fragment Foo on Type { field } - '; - $doc = Parser::parse($sdl); - BuildSchema::buildAST($doc); - } - /** * @see it('Forbids duplicate type definitions') */ diff --git a/tests/Utils/SchemaExtenderLegacyTest.php b/tests/Utils/SchemaExtenderLegacyTest.php index d81fcb5f0..a0e88793f 100644 --- a/tests/Utils/SchemaExtenderLegacyTest.php +++ b/tests/Utils/SchemaExtenderLegacyTest.php @@ -30,7 +30,6 @@ * 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. */ class SchemaExtenderLegacyTest extends TestCase @@ -340,66 +339,6 @@ enum SomeEnum } } - // Validation: add support of SDL to KnownTypeNames - - /** - * @see it('does not allow referencing an unknown type') - */ - public function testDoesNotAllowReferencingAnUnknownType(): void - { - $unknownTypeError = 'Unknown type: "Quix". Ensure that this type exists either in the original schema, or is added in a type definition.'; - - $typeSDL = ' - extend type Bar { - quix: Quix - } - '; - - try { - $this->extendTestSchema($typeSDL); - self::fail(); - } catch (Error $error) { - self::assertEquals($unknownTypeError, $error->getMessage()); - } - - $interfaceSDL = ' - extend interface SomeInterface { - quix: Quix - } - '; - - try { - $this->extendTestSchema($interfaceSDL); - self::fail(); - } catch (Error $error) { - self::assertEquals($unknownTypeError, $error->getMessage()); - } - - $unionSDL = ' - extend union SomeUnion = Quix - '; - - try { - $this->extendTestSchema($unionSDL); - self::fail(); - } catch (Error $error) { - self::assertEquals($unknownTypeError, $error->getMessage()); - } - - $inputSDL = ' - extend input SomeInput { - quix: Quix - } - '; - - try { - $this->extendTestSchema($inputSDL); - self::fail(); - } catch (Error $error) { - self::assertEquals($unknownTypeError, $error->getMessage()); - } - } - // Extract check for possible extensions into a separate rule (#1643) /** diff --git a/tests/Validator/KnownTypeNamesTest.php b/tests/Validator/KnownTypeNamesTest.php index 089090fb8..bc3f7356a 100644 --- a/tests/Validator/KnownTypeNamesTest.php +++ b/tests/Validator/KnownTypeNamesTest.php @@ -74,7 +74,6 @@ public function testUnknownTypeNamesAreInvalid(): void */ public function testReferencesToStandardScalarsThatAreMissingInSchema(): void { - self::markTestSkipped('Missing implementation for SDL validation for now'); $schema = BuildSchema::build('type Query { foo: String }'); $query = ' query ($id: ID, $float: Float, $int: Int) { @@ -171,7 +170,6 @@ public function testReferenceTypesDefinedInsideTheSameDocument(): void */ public function testUnknownTypeReferences(): void { - self::markTestSkipped('Missing implementation for SDL validation for now'); $this->expectSDLErrorsFromRule( new KnownTypeNames(), ' @@ -259,7 +257,6 @@ interface SomeInterface { */ public function testDoesNotConsiderNonTypeDefinitions(): void { - self::markTestSkipped('Missing implementation for SDL validation for now'); $this->expectSDLErrorsFromRule( new KnownTypeNames(), ' @@ -336,7 +333,6 @@ public function testReferenceTypesInsideExtensionDocument(): void */ public function testUnknownTypeReferencesInsideExtensionDocument(): void { - self::markTestSkipped('Missing implementation for SDL validation for now'); $schema = BuildSchema::build('type A'); $sdl = ' type B From a253da694a310c9296fb988e50d807be5c4a1fd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Va=C5=A1ek=20Henzl?= Date: Sun, 31 Oct 2021 16:08:01 +1300 Subject: [PATCH 03/14] Optimise `KnownTypeNames` validation rule --- src/Validator/Rules/KnownTypeNames.php | 32 ++++++++++++++------------ tests/Validator/KnownTypeNamesTest.php | 1 + 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/Validator/Rules/KnownTypeNames.php b/src/Validator/Rules/KnownTypeNames.php index d6a9902b3..c222a0768 100644 --- a/src/Validator/Rules/KnownTypeNames.php +++ b/src/Validator/Rules/KnownTypeNames.php @@ -2,7 +2,6 @@ namespace GraphQL\Validator\Rules; -use function array_key_exists; use function array_keys; use Closure; use function count; @@ -43,32 +42,28 @@ public function getSDLVisitor(SDLValidationContext $context): array */ private function getVisitorInternal($context): array { - $schema = $context->getSchema(); - $existingTypesMap = null !== $schema - ? $schema->getTypeMap() - : []; - - /** @var array $definedTypes */ + /** @var array $definedTypes */ $definedTypes = []; foreach ($context->getDocument()->definitions as $def) { if (! ($def instanceof TypeDefinitionNode)) { continue; } - $definedTypes[$def->name->value] = true; + $definedTypes[] = $def->name->value; } - $typeNames = [ - ...array_keys($existingTypesMap), - ...array_keys($definedTypes), - ]; - $standardTypeNames = array_keys(Type::getAllBuiltInTypes()); return [ - NodeKind::NAMED_TYPE => static function (NamedTypeNode $node, $_1, $parent, $_2, $ancestors) use ($context, $existingTypesMap, $definedTypes, $typeNames, $standardTypeNames): void { + NodeKind::NAMED_TYPE => static function (NamedTypeNode $node, $_1, $parent, $_2, $ancestors) use ($context, $definedTypes, $standardTypeNames): void { $typeName = $node->name->value; - if (array_key_exists($typeName, $existingTypesMap) || array_key_exists($typeName, $definedTypes)) { + $schema = $context->getSchema(); + + if (in_array($typeName, $definedTypes, true)) { + return; + } + + if (null !== $schema && $schema->hasType($typeName)) { return; } @@ -78,6 +73,13 @@ private function getVisitorInternal($context): array return; } + $existingTypesMap = null !== $schema + ? $schema->getTypeMap() + : []; + $typeNames = [ + ...array_keys($existingTypesMap), + ...$definedTypes, + ]; $context->reportError(new Error( static::unknownTypeMessage( $typeName, diff --git a/tests/Validator/KnownTypeNamesTest.php b/tests/Validator/KnownTypeNamesTest.php index bc3f7356a..3cbe24ad9 100644 --- a/tests/Validator/KnownTypeNamesTest.php +++ b/tests/Validator/KnownTypeNamesTest.php @@ -74,6 +74,7 @@ public function testUnknownTypeNamesAreInvalid(): void */ public function testReferencesToStandardScalarsThatAreMissingInSchema(): void { + self::markTestSkipped('TODO we differ from graphql-js due to lazy loading, see https://github.com/webonyx/graphql-php/issues/964#issuecomment-945969162'); $schema = BuildSchema::build('type Query { foo: String }'); $query = ' query ($id: ID, $float: Float, $int: Int) { From bef718e23bceca9b498f309138690bfc72bc7ca7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Va=C5=A1ek=20Henzl?= Date: Fri, 18 Feb 2022 18:50:15 +1300 Subject: [PATCH 04/14] Add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 492cce8ce..5874999da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,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 support for SDL validation to `KnownTypeNames` rule (#999) ### Optimized From cfe41e42d18e1fa44e5e5e0811599b177093b0e9 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Fri, 18 Feb 2022 09:44:51 +0100 Subject: [PATCH 05/14] Simplify getOperationTypes --- src/Utils/BuildSchema.php | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Utils/BuildSchema.php b/src/Utils/BuildSchema.php index e0bc9b27d..23edc9c4e 100644 --- a/src/Utils/BuildSchema.php +++ b/src/Utils/BuildSchema.php @@ -217,21 +217,17 @@ static function (string $typeName): Type { } /** - * @throws Error - * * @return array */ private function getOperationTypes(SchemaDefinitionNode $schemaDef): array { - $opTypes = []; - + /** @var array $operationTypes */ + $operationTypes = []; foreach ($schemaDef->operationTypes as $operationType) { - $typeName = $operationType->type->name->value; - $operation = $operationType->operation; - $opTypes[$operation] = $typeName; + $operationTypes[$operationType->operation] = $operationType->type->name->value; } - return $opTypes; + return $operationTypes; } public static function unknownType(string $typeName): Error From a1ed68ea6474d656960085c14638bf7d8056f1ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Va=C5=A1ek=20Henzl?= Date: Sat, 19 Feb 2022 08:58:18 +1300 Subject: [PATCH 06/14] Apply review suggestion Co-authored-by: Benedikt Franke --- src/Validator/Rules/KnownTypeNames.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Validator/Rules/KnownTypeNames.php b/src/Validator/Rules/KnownTypeNames.php index c222a0768..68420d37d 100644 --- a/src/Validator/Rules/KnownTypeNames.php +++ b/src/Validator/Rules/KnownTypeNames.php @@ -45,11 +45,9 @@ private function getVisitorInternal($context): array /** @var array $definedTypes */ $definedTypes = []; foreach ($context->getDocument()->definitions as $def) { - if (! ($def instanceof TypeDefinitionNode)) { - continue; + if ($def instanceof TypeDefinitionNode) { + $definedTypes[] = $def->name->value; } - - $definedTypes[] = $def->name->value; } $standardTypeNames = array_keys(Type::getAllBuiltInTypes()); From b703e149a2b198bf418a54b2fb696a3392182fc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Va=C5=A1ek=20Henzl?= Date: Sat, 19 Feb 2022 09:08:00 +1300 Subject: [PATCH 07/14] Apply review suggestions --- src/Validator/Rules/KnownTypeNames.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/Validator/Rules/KnownTypeNames.php b/src/Validator/Rules/KnownTypeNames.php index 68420d37d..76c33968c 100644 --- a/src/Validator/Rules/KnownTypeNames.php +++ b/src/Validator/Rules/KnownTypeNames.php @@ -3,7 +3,6 @@ namespace GraphQL\Validator\Rules; use function array_keys; -use Closure; use function count; use GraphQL\Error\Error; use GraphQL\Language\AST\NamedTypeNode; @@ -22,25 +21,27 @@ * * A GraphQL document is only valid if referenced types (specifically * variable definitions and fragment conditions) are defined by the type schema. + * + * @phpstan-import-type VisitorArray from \GraphQL\Language\Visitor */ class KnownTypeNames extends ValidationRule { public function getVisitor(ValidationContext $context): array { - return $this->getVisitorInternal($context); + return $this->getASTVisitor($context); } public function getSDLVisitor(SDLValidationContext $context): array { - return $this->getVisitorInternal($context); + return $this->getASTVisitor($context); } /** * @param ValidationContext|SDLValidationContext $context * - * @return Closure[] + * @phpstan-return VisitorArray */ - private function getVisitorInternal($context): array + private function getASTVisitor($context): array { /** @var array $definedTypes */ $definedTypes = []; From 538ace8e9448cc1bc95ec1fd809277bb31ee51ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Va=C5=A1ek=20Henzl?= Date: Sat, 19 Feb 2022 09:16:34 +1300 Subject: [PATCH 08/14] Align `KnownTypeNames::getASTVisitor()` with other `getASTVisitor()`s Not sure though why they are public. --- src/Validator/Rules/KnownTypeNames.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Validator/Rules/KnownTypeNames.php b/src/Validator/Rules/KnownTypeNames.php index 76c33968c..56f8da57c 100644 --- a/src/Validator/Rules/KnownTypeNames.php +++ b/src/Validator/Rules/KnownTypeNames.php @@ -12,6 +12,7 @@ use GraphQL\Language\AST\TypeSystemDefinitionNode; use GraphQL\Type\Definition\Type; use GraphQL\Utils\Utils; +use GraphQL\Validator\ASTValidationContext; use GraphQL\Validator\SDLValidationContext; use GraphQL\Validator\ValidationContext; use function in_array; @@ -37,11 +38,9 @@ public function getSDLVisitor(SDLValidationContext $context): array } /** - * @param ValidationContext|SDLValidationContext $context - * * @phpstan-return VisitorArray */ - private function getASTVisitor($context): array + public function getASTVisitor(ASTValidationContext $context): array { /** @var array $definedTypes */ $definedTypes = []; From f8d2cc98860c9bdc12fd81ea42c6034710746c84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Va=C5=A1ek=20Henzl?= Date: Sat, 19 Feb 2022 09:30:03 +1300 Subject: [PATCH 09/14] Fix after merge --- src/Utils/SchemaExtender.php | 2 +- src/Validator/Rules/KnownTypeNames.php | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Utils/SchemaExtender.php b/src/Utils/SchemaExtender.php index acd9bf1fb..c03c3cb04 100644 --- a/src/Utils/SchemaExtender.php +++ b/src/Utils/SchemaExtender.php @@ -622,7 +622,7 @@ public static function extend( $typeDefinitionMap, static function (string $typeName) use ($schema): Type { $existingType = $schema->getType($typeName); - if (null === $existingType) { + if ($existingType === null) { throw new InvariantViolation('Unknown type: "' . $typeName . '".'); } diff --git a/src/Validator/Rules/KnownTypeNames.php b/src/Validator/Rules/KnownTypeNames.php index a0115fef2..9ab4045dc 100644 --- a/src/Validator/Rules/KnownTypeNames.php +++ b/src/Validator/Rules/KnownTypeNames.php @@ -12,9 +12,9 @@ use GraphQL\Language\AST\TypeSystemDefinitionNode; use GraphQL\Type\Definition\Type; use GraphQL\Utils\Utils; -use GraphQL\Validator\ASTValidationContext; -use GraphQL\Validator\SDLValidationContext; use GraphQL\Validator\QueryValidationContext; +use GraphQL\Validator\SDLValidationContext; +use GraphQL\Validator\ValidationContext; use function in_array; /** @@ -40,7 +40,7 @@ public function getSDLVisitor(SDLValidationContext $context): array /** * @phpstan-return VisitorArray */ - public function getASTVisitor(ASTValidationContext $context): array + public function getASTVisitor(ValidationContext $context): array { /** @var array $definedTypes */ $definedTypes = []; @@ -61,17 +61,17 @@ public function getASTVisitor(ASTValidationContext $context): array return; } - if (null !== $schema && $schema->hasType($typeName)) { + if ($schema !== null && $schema->hasType($typeName)) { return; } $definitionNode = $ancestors[2] ?? $parent; - $isSDL = null !== $definitionNode && self::isSDLNode($definitionNode); + $isSDL = $definitionNode !== null && self::isSDLNode($definitionNode); if ($isSDL && in_array($typeName, $standardTypeNames, true)) { return; } - $existingTypesMap = null !== $schema + $existingTypesMap = $schema !== null ? $schema->getTypeMap() : []; $typeNames = [ From 5684b73bdbb128f098144d1875e933f0623be327 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Va=C5=A1ek=20Henzl?= Date: Sat, 19 Feb 2022 10:30:37 +1300 Subject: [PATCH 10/14] Fix after merge --- tests/Utils/BuildSchemaLegacyTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Utils/BuildSchemaLegacyTest.php b/tests/Utils/BuildSchemaLegacyTest.php index bc7fd2b88..1149a3761 100644 --- a/tests/Utils/BuildSchemaLegacyTest.php +++ b/tests/Utils/BuildSchemaLegacyTest.php @@ -3,7 +3,6 @@ namespace GraphQL\Tests\Utils; use GraphQL\Error\DebugFlag; -use GraphQL\Error\Error; use GraphQL\GraphQL; use GraphQL\Language\Parser; use GraphQL\Utils\BuildSchema; From 52f7968463b719beeb435889bf26b69aabef07ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Va=C5=A1ek=20Henzl?= Date: Sat, 19 Feb 2022 10:13:16 +1300 Subject: [PATCH 11/14] Reflect change to AST interfaces in `isSDLNode()` --- src/Validator/Rules/KnownTypeNames.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Validator/Rules/KnownTypeNames.php b/src/Validator/Rules/KnownTypeNames.php index 9ab4045dc..ff459b5b5 100644 --- a/src/Validator/Rules/KnownTypeNames.php +++ b/src/Validator/Rules/KnownTypeNames.php @@ -10,6 +10,7 @@ use GraphQL\Language\AST\NodeKind; use GraphQL\Language\AST\TypeDefinitionNode; use GraphQL\Language\AST\TypeSystemDefinitionNode; +use GraphQL\Language\AST\TypeSystemExtensionNode; use GraphQL\Type\Definition\Type; use GraphQL\Utils\Utils; use GraphQL\Validator\QueryValidationContext; @@ -114,7 +115,6 @@ public static function unknownTypeMessage(string $type, array $suggestedTypes): public static function isSDLNode($value): bool { return $value instanceof Node - // TODO: should be (TypeSystemDefinitionNode || TypeSystemExtensionNode) - && $value instanceof TypeSystemDefinitionNode; + && ($value instanceof TypeSystemDefinitionNode || $value instanceof TypeSystemExtensionNode); } } From 587bc8b59cd296d5b191b3af4075611337748fc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Va=C5=A1ek=20Henzl?= Date: Sat, 19 Feb 2022 14:12:38 +1300 Subject: [PATCH 12/14] Update code to match the ref implementation --- src/Validator/Rules/KnownTypeNames.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Validator/Rules/KnownTypeNames.php b/src/Validator/Rules/KnownTypeNames.php index ff459b5b5..1929bc910 100644 --- a/src/Validator/Rules/KnownTypeNames.php +++ b/src/Validator/Rules/KnownTypeNames.php @@ -85,8 +85,7 @@ public function getASTVisitor(ValidationContext $context): array Utils::suggestionList( $typeName, $isSDL - // TODO: order - ? [...$typeNames, ...$standardTypeNames] + ? [...$standardTypeNames, ...$typeNames] : $typeNames ) ), From ade2966503429c69c21ae9c3b1bd4979e261ceed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Va=C5=A1ek=20Henzl?= Date: Mon, 14 Mar 2022 22:04:17 +1300 Subject: [PATCH 13/14] Apply suggestion from review --- src/Validator/Rules/KnownTypeNames.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Validator/Rules/KnownTypeNames.php b/src/Validator/Rules/KnownTypeNames.php index 1929bc910..db7a100c0 100644 --- a/src/Validator/Rules/KnownTypeNames.php +++ b/src/Validator/Rules/KnownTypeNames.php @@ -67,7 +67,7 @@ public function getASTVisitor(ValidationContext $context): array } $definitionNode = $ancestors[2] ?? $parent; - $isSDL = $definitionNode !== null && self::isSDLNode($definitionNode); + $isSDL = $definitionNode instanceof TypeSystemDefinitionNode || $definitionNode instanceof TypeSystemExtensionNode; if ($isSDL && in_array($typeName, $standardTypeNames, true)) { return; } From f630e1b54f14f23ee5d6cad438ad39663caf52ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Va=C5=A1ek=20Henzl?= Date: Mon, 14 Mar 2022 22:11:21 +1300 Subject: [PATCH 14/14] Remove unused method --- src/Validator/Rules/KnownTypeNames.php | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/Validator/Rules/KnownTypeNames.php b/src/Validator/Rules/KnownTypeNames.php index db7a100c0..05c69aca1 100644 --- a/src/Validator/Rules/KnownTypeNames.php +++ b/src/Validator/Rules/KnownTypeNames.php @@ -6,7 +6,6 @@ use function count; use GraphQL\Error\Error; use GraphQL\Language\AST\NamedTypeNode; -use GraphQL\Language\AST\Node; use GraphQL\Language\AST\NodeKind; use GraphQL\Language\AST\TypeDefinitionNode; use GraphQL\Language\AST\TypeSystemDefinitionNode; @@ -107,13 +106,4 @@ public static function unknownTypeMessage(string $type, array $suggestedTypes): return $message; } - - /** - * @param Node|array $value - */ - public static function isSDLNode($value): bool - { - return $value instanceof Node - && ($value instanceof TypeSystemDefinitionNode || $value instanceof TypeSystemExtensionNode); - } }