Add support of SDL to KnownTypeNames validation rule (#999)

webonyx · Mar 14, 2022 · 08d9db9 · 08d9db9
1 parent b6ac0ff
commit 08d9db9
Show file tree

Hide file tree

Showing 8 changed files with 106 additions and 311 deletions.
diff --git 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
@@ -212,26 +212,17 @@ static function (string $typeName): Type {
     }
 
     /**
-     * @throws Error
-     *
      * @return array<string, string>
      */
     private function getOperationTypes(SchemaDefinitionNode $schemaDef): array
     {
-        $opTypes = [];
-
+        /** @var array<string, string> $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
@@ -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
@@ -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
@@ -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<int, string> $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
@@ -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);
-    }
 }