diff --git a/CHANGELOG.md b/CHANGELOG.md index 728da8c90..82f19d6b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,7 @@ You can find and compare releases at the [GitHub release page](https://github.co - Allow lazy enum values - Make `Node` implement `JsonSerializable` - Add SDL validation rule `UniqueTypeNames` (#998) +- Add support for SDL validation to `KnownTypeNames` rule (#999) ### Optimized diff --git a/src/Utils/BuildSchema.php b/src/Utils/BuildSchema.php index 5e4ee55de..8e08b4f45 100644 --- a/src/Utils/BuildSchema.php +++ b/src/Utils/BuildSchema.php @@ -212,26 +212,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; - - if (! isset($this->nodeMap[$typeName])) { - throw new Error('Specified ' . $operation . ' type "' . $typeName . '" not found in document.'); - } - - $opTypes[$operation] = $typeName; + $operationTypes[$operationType->operation] = $operationType->type->name->value; } - return $opTypes; + return $operationTypes; } public static function unknownType(string $typeName): Error diff --git a/src/Utils/SchemaExtender.php b/src/Utils/SchemaExtender.php index 9300bf549..1029b1087 100644 --- a/src/Utils/SchemaExtender.php +++ b/src/Utils/SchemaExtender.php @@ -614,11 +614,11 @@ public static function extend( $typeDefinitionMap, static function (string $typeName) use ($schema): Type { $existingType = $schema->getType($typeName); - if ($existingType !== null) { - return static::extendNamedType($existingType); + if ($existingType === null) { + 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 119be619c..5e41a9166 100644 --- a/src/Validator/DocumentValidator.php +++ b/src/Validator/DocumentValidator.php @@ -203,6 +203,7 @@ public static function sdlRules(): array LoneSchemaDefinition::class => new LoneSchemaDefinition(), UniqueOperationTypes::class => new UniqueOperationTypes(), UniqueTypeNames::class => new UniqueTypeNames(), + 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 e7a96b527..05c69aca1 100644 --- a/src/Validator/Rules/KnownTypeNames.php +++ b/src/Validator/Rules/KnownTypeNames.php @@ -7,45 +7,86 @@ use GraphQL\Error\Error; use GraphQL\Language\AST\NamedTypeNode; 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\Language\AST\TypeSystemExtensionNode; +use GraphQL\Type\Definition\Type; use GraphQL\Utils\Utils; use GraphQL\Validator\QueryValidationContext; +use GraphQL\Validator\SDLValidationContext; +use GraphQL\Validator\ValidationContext; +use function in_array; /** * Known type names. * * 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(QueryValidationContext $context): array { - $skip = static function (): VisitorOperation { - return Visitor::skipNode(); - }; + return $this->getASTVisitor($context); + } + + public function getSDLVisitor(SDLValidationContext $context): array + { + return $this->getASTVisitor($context); + } + + /** + * @phpstan-return VisitorArray + */ + public function getASTVisitor(ValidationContext $context): array + { + /** @var array $definedTypes */ + $definedTypes = []; + foreach ($context->getDocument()->definitions as $def) { + if ($def instanceof TypeDefinitionNode) { + $definedTypes[] = $def->name->value; + } + } + + $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, $definedTypes, $standardTypeNames): void { $typeName = $node->name->value; - $type = $schema->getType($typeName); - if ($type !== null) { + $schema = $context->getSchema(); + + if (in_array($typeName, $definedTypes, true)) { + return; + } + + if ($schema !== null && $schema->hasType($typeName)) { + return; + } + + $definitionNode = $ancestors[2] ?? $parent; + $isSDL = $definitionNode instanceof TypeSystemDefinitionNode || $definitionNode instanceof TypeSystemExtensionNode; + if ($isSDL && in_array($typeName, $standardTypeNames, true)) { return; } + $existingTypesMap = $schema !== null + ? $schema->getTypeMap() + : []; + $typeNames = [ + ...array_keys($existingTypesMap), + ...$definedTypes, + ]; $context->reportError(new Error( static::unknownTypeMessage( $typeName, - Utils::suggestionList($typeName, array_keys($schema->getTypeMap())) + Utils::suggestionList( + $typeName, + $isSDL + ? [...$standardTypeNames, ...$typeNames] + : $typeNames + ) ), [$node] )); diff --git a/tests/Utils/BuildSchemaLegacyTest.php b/tests/Utils/BuildSchemaLegacyTest.php index 6418a8568..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; @@ -14,7 +13,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/3b9ea61f2348215dee755f779caef83df749d2bb * - https://github.com/graphql/graphql-js/commit/64a5c3448a201737f9218856786c51d66f2deabd. */ class BuildSchemaLegacyTest extends TestCase @@ -145,177 +143,4 @@ interface Character { $result = GraphQL::executeQuery($schema, $source, $rootValue); self::assertEquals($expected, $result->toArray(DebugFlag::INCLUDE_DEBUG_MESSAGE)); } - - // 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); - } } diff --git a/tests/Utils/SchemaExtenderLegacyTest.php b/tests/Utils/SchemaExtenderLegacyTest.php index 38c208a68..47efdb5ce 100644 --- a/tests/Utils/SchemaExtenderLegacyTest.php +++ b/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/3b9ea61f2348215dee755f779caef83df749d2bb * - https://github.com/graphql/graphql-js/commit/e6a3f08cc92594f68a6e61d3d4b46a6d279f845e. */ class SchemaExtenderLegacyTest extends TestCase @@ -263,66 +262,6 @@ public function testDoesNotAllowReplacingAnExistingField(): void } } - // 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 a1ef1f34b..3cbe24ad9 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]], + ], ] ); } @@ -66,7 +74,7 @@ public function testUnknownTypeNamesAreInvalid(): void */ public function testReferencesToStandardScalarsThatAreMissingInSchema(): void { - self::markTestSkipped('Missing implementation for SDL validation for now'); + 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) { @@ -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]], + ], ] ); } @@ -154,7 +171,6 @@ public function testReferenceTypesDefinedInsideTheSameDocument(): void */ public function testUnknownTypeReferences(): void { - self::markTestSkipped('Missing implementation for SDL validation for now'); $this->expectSDLErrorsFromRule( new KnownTypeNames(), ' @@ -242,7 +258,6 @@ interface SomeInterface { */ public function testDoesNotConsiderNonTypeDefinitions(): void { - self::markTestSkipped('Missing implementation for SDL validation for now'); $this->expectSDLErrorsFromRule( new KnownTypeNames(), ' @@ -319,10 +334,8 @@ 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 type SomeObject implements C { @@ -355,69 +368,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)] - ); - } }