Reorganize AST interfaces related to schema and type extensions (#1073)

spawnia · web-flow · commit ceb6e94f5789 · 2022-02-18T22:37:45.000+01:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -40,6 +40,7 @@ You can find and compare releases at the [GitHub release page](https://github.co
 - `PromiseAdapter::all()` accepts `iterable`
 - Throw if `Introspection::fromSchema()` returns no data
 - Reorganize abstract class `ASTValidationContext` to interface `ValidationContext`
+- Reorganize AST interfaces related to schema and type extensions
 
 ### Added
 
diff --git a/src/Language/AST/DefinitionNode.php b/src/Language/AST/DefinitionNode.php
@@ -5,7 +5,8 @@
 /**
  * export type DefinitionNode =
  *   | ExecutableDefinitionNode
- *   | TypeSystemDefinitionNode;.
+ *   | TypeSystemDefinitionNode
+ *   | TypeSystemExtensionNode;.
  */
 interface DefinitionNode
 {
diff --git a/src/Language/AST/SchemaExtensionNode.php b/src/Language/AST/SchemaExtensionNode.php
@@ -2,7 +2,7 @@
 
 namespace GraphQL\Language\AST;
 
-class SchemaTypeExtensionNode extends Node implements TypeExtensionNode
+class SchemaExtensionNode extends Node implements TypeSystemExtensionNode
 {
     public string $kind = NodeKind::SCHEMA_EXTENSION;
 
diff --git a/src/Language/AST/TypeDefinitionNode.php b/src/Language/AST/TypeDefinitionNode.php
@@ -9,6 +9,8 @@
  * | UnionTypeDefinitionNode
  * | EnumTypeDefinitionNode
  * | InputObjectTypeDefinitionNode.
+ *
+ * @property NameNode $name
  */
 interface TypeDefinitionNode extends TypeSystemDefinitionNode
 {
diff --git a/src/Language/AST/TypeExtensionNode.php b/src/Language/AST/TypeExtensionNode.php
@@ -10,7 +10,9 @@
  * | UnionTypeExtensionNode
  * | EnumTypeExtensionNode
  * | InputObjectTypeExtensionNode;.
+ *
+ * @property NameNode $name
  */
-interface TypeExtensionNode extends TypeSystemDefinitionNode
+interface TypeExtensionNode extends TypeSystemExtensionNode
 {
 }
diff --git a/src/Language/AST/TypeSystemDefinitionNode.php b/src/Language/AST/TypeSystemDefinitionNode.php
@@ -6,10 +6,7 @@
  * export type TypeSystemDefinitionNode =
  * | SchemaDefinitionNode
  * | TypeDefinitionNode
- * | TypeExtensionNode
  * | DirectiveDefinitionNode.
- *
- * @property NameNode $name
  */
 interface TypeSystemDefinitionNode extends DefinitionNode
 {
diff --git a/src/Language/AST/TypeSystemExtensionNode.php b/src/Language/AST/TypeSystemExtensionNode.php
@@ -0,0 +1,10 @@
+<?php declare(strict_types=1);
+
+namespace GraphQL\Language\AST;
+
+/**
+ * export type TypeSystemExtensionNode = SchemaExtensionNode | TypeExtensionNode;.
+ */
+interface TypeSystemExtensionNode extends DefinitionNode
+{
+}
diff --git a/src/Language/Parser.php b/src/Language/Parser.php
@@ -45,13 +45,14 @@
 use GraphQL\Language\AST\ScalarTypeDefinitionNode;
 use GraphQL\Language\AST\ScalarTypeExtensionNode;
 use GraphQL\Language\AST\SchemaDefinitionNode;
-use GraphQL\Language\AST\SchemaTypeExtensionNode;
+use GraphQL\Language\AST\SchemaExtensionNode;
 use GraphQL\Language\AST\SelectionNode;
 use GraphQL\Language\AST\SelectionSetNode;
 use GraphQL\Language\AST\StringValueNode;
 use GraphQL\Language\AST\TypeExtensionNode;
 use GraphQL\Language\AST\TypeNode;
 use GraphQL\Language\AST\TypeSystemDefinitionNode;
+use GraphQL\Language\AST\TypeSystemExtensionNode;
 use GraphQL\Language\AST\UnionTypeDefinitionNode;
 use GraphQL\Language\AST\UnionTypeExtensionNode;
 use GraphQL\Language\AST\ValueNode;
@@ -161,7 +162,7 @@
  * @method static InputObjectTypeDefinitionNode inputObjectTypeDefinition(Source|string $source, bool[] $options = [])
  * @method static NodeList<InputValueDefinitionNode> inputFieldsDefinition(Source|string $source, bool[] $options = [])
  * @method static TypeExtensionNode typeExtension(Source|string $source, bool[] $options = [])
- * @method static SchemaTypeExtensionNode schemaTypeExtension(Source|string $source, bool[] $options = [])
+ * @method static SchemaExtensionNode schemaTypeExtension(Source|string $source, bool[] $options = [])
  * @method static ScalarTypeExtensionNode scalarTypeExtension(Source|string $source, bool[] $options = [])
  * @method static ObjectTypeExtensionNode objectTypeExtension(Source|string $source, bool[] $options = [])
  * @method static InterfaceTypeExtensionNode interfaceTypeExtension(Source|string $source, bool[] $options = [])
@@ -508,10 +509,12 @@ private function parseDefinition(): DefinitionNode
                 case 'union':
                 case 'enum':
                 case 'input':
-                case 'extend':
                 case 'directive':
                     // Note: The schema definition language is an experimental addition.
                     return $this->parseTypeSystemDefinition();
+
+                case 'extend':
+                    return $this->parseTypeSystemExtension();
             }
         } elseif ($this->peek(Token::BRACE_L)) {
             return $this->parseExecutableDefinition();
@@ -1055,9 +1058,6 @@ private function parseTypeSystemDefinition(): TypeSystemDefinitionNode
                 case 'input':
                     return $this->parseInputObjectTypeDefinition();
 
-                case 'extend':
-                    return $this->parseTypeExtension();
-
                 case 'directive':
                     return $this->parseDirectiveDefinition();
             }
@@ -1404,9 +1404,9 @@ function (): InputValueDefinitionNode {
     }
 
     /**
-     * @return TypeExtensionNode&Node
+     * @return TypeSystemExtensionNode&Node
      */
-    private function parseTypeExtension(): TypeExtensionNode
+    private function parseTypeSystemExtension(): TypeSystemExtensionNode
     {
         $keywordToken = $this->lexer->lookahead();
 
@@ -1438,7 +1438,7 @@ private function parseTypeExtension(): TypeExtensionNode
         throw $this->unexpected($keywordToken);
     }
 
-    private function parseSchemaTypeExtension(): SchemaTypeExtensionNode
+    private function parseSchemaTypeExtension(): SchemaExtensionNode
     {
         $start = $this->lexer->token;
         $this->expectKeyword('extend');
@@ -1456,7 +1456,7 @@ private function parseSchemaTypeExtension(): SchemaTypeExtensionNode
             $this->unexpected();
         }
 
-        return new SchemaTypeExtensionNode([
+        return new SchemaExtensionNode([
             'directives' => $directives,
             'operationTypes' => $operationTypes,
             'loc' => $this->loc($start),
diff --git a/src/Language/Printer.php b/src/Language/Printer.php
@@ -42,7 +42,7 @@
 use GraphQL\Language\AST\ScalarTypeDefinitionNode;
 use GraphQL\Language\AST\ScalarTypeExtensionNode;
 use GraphQL\Language\AST\SchemaDefinitionNode;
-use GraphQL\Language\AST\SchemaTypeExtensionNode;
+use GraphQL\Language\AST\SchemaExtensionNode;
 use GraphQL\Language\AST\SelectionSetNode;
 use GraphQL\Language\AST\StringValueNode;
 use GraphQL\Language\AST\UnionTypeDefinitionNode;
@@ -389,7 +389,7 @@ protected function p(?Node $node, bool $isDescription = false): string
                     ' '
                 );
 
-            case $node instanceof SchemaTypeExtensionNode:
+            case $node instanceof SchemaExtensionNode:
                 return $this->join(
                     [
                         'extend schema',
diff --git a/src/Type/Schema.php b/src/Type/Schema.php
@@ -7,7 +7,7 @@
 use GraphQL\Error\InvariantViolation;
 use GraphQL\GraphQL;
 use GraphQL\Language\AST\SchemaDefinitionNode;
-use GraphQL\Language\AST\SchemaTypeExtensionNode;
+use GraphQL\Language\AST\SchemaExtensionNode;
 use GraphQL\Type\Definition\AbstractType;
 use GraphQL\Type\Definition\Directive;
 use GraphQL\Type\Definition\ImplementingType;
@@ -70,7 +70,7 @@ class Schema
     /** @var array<int, Error> */
     private array $validationErrors;
 
-    /** @var array<int, SchemaTypeExtensionNode> */
+    /** @var array<int, SchemaExtensionNode> */
     public array $extensionASTNodes = [];
 
     /**
diff --git a/src/Type/SchemaConfig.php b/src/Type/SchemaConfig.php
@@ -4,7 +4,7 @@
 
 use function count;
 use GraphQL\Language\AST\SchemaDefinitionNode;
-use GraphQL\Language\AST\SchemaTypeExtensionNode;
+use GraphQL\Language\AST\SchemaExtensionNode;
 use GraphQL\Type\Definition\Directive;
 use GraphQL\Type\Definition\NamedType;
 use GraphQL\Type\Definition\ObjectType;
@@ -56,7 +56,7 @@ class SchemaConfig
 
     public ?SchemaDefinitionNode $astNode = null;
 
-    /** @var array<SchemaTypeExtensionNode> */
+    /** @var array<SchemaExtensionNode> */
     public array $extensionASTNodes = [];
 
     /**
@@ -260,15 +260,15 @@ public function setAstNode(SchemaDefinitionNode $astNode): self
     }
 
     /**
-     * @return array<SchemaTypeExtensionNode>
+     * @return array<SchemaExtensionNode>
      */
     public function getExtensionASTNodes(): array
     {
         return $this->extensionASTNodes;
     }
 
     /**
-     * @param array<SchemaTypeExtensionNode> $extensionASTNodes
+     * @param array<SchemaExtensionNode> $extensionASTNodes
      */
     public function setExtensionASTNodes(array $extensionASTNodes): self
     {
diff --git a/src/Type/SchemaValidationContext.php b/src/Type/SchemaValidationContext.php
@@ -25,7 +25,7 @@
 use GraphQL\Language\AST\ObjectTypeDefinitionNode;
 use GraphQL\Language\AST\ObjectTypeExtensionNode;
 use GraphQL\Language\AST\SchemaDefinitionNode;
-use GraphQL\Language\AST\SchemaTypeExtensionNode;
+use GraphQL\Language\AST\SchemaExtensionNode;
 use GraphQL\Language\AST\TypeNode;
 use GraphQL\Language\AST\UnionTypeDefinitionNode;
 use GraphQL\Language\AST\UnionTypeExtensionNode;
@@ -470,7 +470,7 @@ private function validateFields(Type $type): void
     /**
      * @param Schema|ObjectType|InterfaceType|UnionType|EnumType|InputObjectType|Directive $obj
      *
-     * @return array<int, SchemaDefinitionNode|SchemaTypeExtensionNode>|array<int, ObjectTypeDefinitionNode|ObjectTypeExtensionNode>|array<int, InterfaceTypeDefinitionNode|InterfaceTypeExtensionNode>|array<int, UnionTypeDefinitionNode|UnionTypeExtensionNode>|array<int, EnumTypeDefinitionNode|EnumTypeExtensionNode>|array<int, InputObjectTypeDefinitionNode|InputObjectTypeExtensionNode>|array<int, DirectiveDefinitionNode>
+     * @return array<int, SchemaDefinitionNode|SchemaExtensionNode>|array<int, ObjectTypeDefinitionNode|ObjectTypeExtensionNode>|array<int, InterfaceTypeDefinitionNode|InterfaceTypeExtensionNode>|array<int, UnionTypeDefinitionNode|UnionTypeExtensionNode>|array<int, EnumTypeDefinitionNode|EnumTypeExtensionNode>|array<int, InputObjectTypeDefinitionNode|InputObjectTypeExtensionNode>|array<int, DirectiveDefinitionNode>
      */
     private function getAllNodes(object $obj): array
     {
@@ -633,7 +633,7 @@ private function getDirectives(object $object): NodeList
         /**
          * Excluding directiveNode, since $object is not Directive.
          *
-         * @var SchemaDefinitionNode|SchemaTypeExtensionNode|ObjectTypeDefinitionNode|ObjectTypeExtensionNode|InterfaceTypeDefinitionNode|InterfaceTypeExtensionNode|UnionTypeDefinitionNode|UnionTypeExtensionNode|EnumTypeDefinitionNode|EnumTypeExtensionNode|InputObjectTypeDefinitionNode|InputObjectTypeExtensionNode $node
+         * @var SchemaDefinitionNode|SchemaExtensionNode|ObjectTypeDefinitionNode|ObjectTypeExtensionNode|InterfaceTypeDefinitionNode|InterfaceTypeExtensionNode|UnionTypeDefinitionNode|UnionTypeExtensionNode|EnumTypeDefinitionNode|EnumTypeExtensionNode|InputObjectTypeDefinitionNode|InputObjectTypeExtensionNode $node
          */
         // @phpstan-ignore-next-line union types are not pervasive
         foreach ($this->getAllNodes($object) as $node) {
diff --git a/src/Utils/SchemaExtender.php b/src/Utils/SchemaExtender.php
@@ -17,7 +17,7 @@
 use GraphQL\Language\AST\ObjectTypeExtensionNode;
 use GraphQL\Language\AST\ScalarTypeExtensionNode;
 use GraphQL\Language\AST\SchemaDefinitionNode;
-use GraphQL\Language\AST\SchemaTypeExtensionNode;
+use GraphQL\Language\AST\SchemaExtensionNode;
 use GraphQL\Language\AST\TypeDefinitionNode;
 use GraphQL\Language\AST\TypeExtensionNode;
 use GraphQL\Language\AST\UnionTypeExtensionNode;
@@ -570,13 +570,13 @@ public static function extend(
         /** @var SchemaDefinitionNode|null $schemaDef */
         $schemaDef = null;
 
-        /** @var array<int, SchemaTypeExtensionNode> $schemaExtensions */
+        /** @var array<int, SchemaExtensionNode> $schemaExtensions */
         $schemaExtensions = [];
 
         foreach ($documentAST->definitions as $def) {
             if ($def instanceof SchemaDefinitionNode) {
                 $schemaDef = $def;
-            } elseif ($def instanceof SchemaTypeExtensionNode) {
+            } elseif ($def instanceof SchemaExtensionNode) {
                 $schemaExtensions[] = $def;
             } elseif ($def instanceof TypeDefinitionNode) {
                 $typeDefinitionMap[$def->name->value] = $def;
diff --git a/src/Validator/Rules/ExecutableDefinitions.php b/src/Validator/Rules/ExecutableDefinitions.php
@@ -6,7 +6,10 @@
 use GraphQL\Language\AST\DocumentNode;
 use GraphQL\Language\AST\ExecutableDefinitionNode;
 use GraphQL\Language\AST\NodeKind;
-use GraphQL\Language\AST\TypeSystemDefinitionNode;
+use GraphQL\Language\AST\SchemaDefinitionNode;
+use GraphQL\Language\AST\SchemaExtensionNode;
+use GraphQL\Language\AST\TypeDefinitionNode;
+use GraphQL\Language\AST\TypeExtensionNode;
 use GraphQL\Language\Visitor;
 use GraphQL\Language\VisitorOperation;
 use GraphQL\Validator\QueryValidationContext;
@@ -25,11 +28,19 @@ public function getVisitor(QueryValidationContext $context): array
             NodeKind::DOCUMENT => static function (DocumentNode $node) use ($context): VisitorOperation {
                 foreach ($node->definitions as $definition) {
                     if (! $definition instanceof ExecutableDefinitionNode) {
-                        assert($definition instanceof TypeSystemDefinitionNode, 'only other option');
+                        if ($definition instanceof SchemaDefinitionNode || $definition instanceof SchemaExtensionNode) {
+                            $defName = 'schema';
+                        } else {
+                            assert(
+                                $definition instanceof TypeDefinitionNode || $definition instanceof TypeExtensionNode,
+                                'only other option'
+                            );
+                            $defName = "\"{$definition->name->value}\"";
+                        }
 
                         $context->reportError(new Error(
-                            static::nonExecutableDefinitionMessage($definition->name->value),
-                            [$definition->name]
+                            static::nonExecutableDefinitionMessage($defName),
+                            [$definition]
                         ));
                     }
                 }
@@ -41,6 +52,6 @@ public function getVisitor(QueryValidationContext $context): array
 
     public static function nonExecutableDefinitionMessage(string $defName): string
     {
-        return "The \"{$defName}\" definition is not executable.";
+        return "The {$defName} definition is not executable.";
     }
 }
diff --git a/src/Validator/Rules/KnownDirectives.php b/src/Validator/Rules/KnownDirectives.php
@@ -30,7 +30,7 @@
 use GraphQL\Language\AST\ScalarTypeDefinitionNode;
 use GraphQL\Language\AST\ScalarTypeExtensionNode;
 use GraphQL\Language\AST\SchemaDefinitionNode;
-use GraphQL\Language\AST\SchemaTypeExtensionNode;
+use GraphQL\Language\AST\SchemaExtensionNode;
 use GraphQL\Language\AST\UnionTypeDefinitionNode;
 use GraphQL\Language\AST\UnionTypeExtensionNode;
 use GraphQL\Language\AST\VariableDefinitionNode;
@@ -166,7 +166,7 @@ protected function getDirectiveLocationForASTPath(array $ancestors): string
                 return DirectiveLocation::VARIABLE_DEFINITION;
 
             case $appliedTo instanceof SchemaDefinitionNode:
-            case $appliedTo instanceof SchemaTypeExtensionNode:
+            case $appliedTo instanceof SchemaExtensionNode:
                 return DirectiveLocation::SCHEMA;
 
             case $appliedTo instanceof ScalarTypeDefinitionNode:
diff --git a/src/Validator/Rules/UniqueOperationTypes.php b/src/Validator/Rules/UniqueOperationTypes.php
@@ -5,7 +5,7 @@
 use GraphQL\Error\Error;
 use GraphQL\Language\AST\NodeKind;
 use GraphQL\Language\AST\SchemaDefinitionNode;
-use GraphQL\Language\AST\SchemaTypeExtensionNode;
+use GraphQL\Language\AST\SchemaExtensionNode;
 use GraphQL\Language\Visitor;
 use GraphQL\Language\VisitorOperation;
 use GraphQL\Validator\SDLValidationContext;
@@ -30,7 +30,7 @@ public function getSDLVisitor(SDLValidationContext $context): array
             : [];
 
         /**
-         * @param SchemaDefinitionNode|SchemaTypeExtensionNode $node
+         * @param SchemaDefinitionNode|SchemaExtensionNode $node
          */
         $checkOperationTypes = static function ($node) use ($context, &$definedOperationTypes, $existingOperationTypes): VisitorOperation {
             foreach ($node->operationTypes as $operationType) {
diff --git a/tests/Validator/CustomRuleTest.php b/tests/Validator/CustomRuleTest.php
@@ -44,8 +44,8 @@ public static function nonExecutableDefinitionMessage(string $defName): string
         ',
             [
                 ErrorHelper::create(
-                    'Custom message including Cow',
-                    [new SourceLocation(8, 12)]
+                    'Custom message including "Cow"',
+                    [new SourceLocation(8, 7)]
                 ),
             ]
         );
diff --git a/tests/Validator/ExecutableDefinitionsTest.php b/tests/Validator/ExecutableDefinitionsTest.php
@@ -75,8 +75,8 @@ public function testWithTypeDefinition(): void
       }
         ',
             [
-                $this->nonExecutableDefinition('Cow', 8, 12),
-                $this->nonExecutableDefinition('Dog', 12, 19),
+                $this->nonExecutableDefinition('"Cow"', 8, 7),
+                $this->nonExecutableDefinition('"Dog"', 12, 7),
             ]
         );
     }

Original file line number	Diff line number	Diff line change
`@@ -2,7 +2,7 @@`
`2`	`2`
`3`	`3`	`namespace GraphQL\Language\AST;`
`4`	`4`
`5`		`-class SchemaTypeExtensionNode extends Node implements TypeExtensionNode`
	`5`	`+class SchemaExtensionNode extends Node implements TypeSystemExtensionNode`
`6`	`6`	`{`
`7`	`7`	`public string $kind = NodeKind::SCHEMA_EXTENSION;`
`8`	`8`
Original file line number	Diff line number	Diff line change
`@@ -9,6 +9,8 @@`
`9`	`9`	`* \| UnionTypeDefinitionNode`
`10`	`10`	`* \| EnumTypeDefinitionNode`
`11`	`11`	`* \| InputObjectTypeDefinitionNode.`
	`12`	`+ *`
	`13`	`+ * @property NameNode $name`
`12`	`14`	`*/`
`13`	`15`	`interface TypeDefinitionNode extends TypeSystemDefinitionNode`
`14`	`16`	`{`
Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,9 @@`
`10`	`10`	`* \| UnionTypeExtensionNode`
`11`	`11`	`* \| EnumTypeExtensionNode`
`12`	`12`	`* \| InputObjectTypeExtensionNode;.`
	`13`	`+ *`
	`14`	`+ * @property NameNode $name`
`13`	`15`	`*/`
`14`		`-interface TypeExtensionNode extends TypeSystemDefinitionNode`
	`16`	`+interface TypeExtensionNode extends TypeSystemExtensionNode`
`15`	`17`	`{`
`16`	`18`	`}`