Add strict native type hints for Schema and SchemaConfig (#917)

Author: spawnia

CHANGELOG.md

@@ -17,6 +17,7 @@ You can find and compare releases at the [GitHub release page](https://github.co
 - Use native PHP types for properties of `Type` and its subclasses
 - Throw `SerializationError` over client safe `Error` when failing to serialize leaf types
 - Move debug entries in errors under `extensions` key
+- Use native PHP types for `Schema` and `SchemaConfig`
 
 ### Added
 

benchmarks/Utils/QueryGenerator.php

@@ -51,14 +51,14 @@ public function __construct(Schema $schema, float $percentOfLeafFields)
 
     public function buildQuery(): string
     {
-        $qtype = $this->schema->getQueryType();
+        $queryType = $this->schema->getQueryType();
 
         $ast = new DocumentNode([
             'definitions' => new NodeList([
                 new OperationDefinitionNode([
                     'name' => new NameNode(['value' => 'TestQuery']),
                     'operation' => 'query',
-                    'selectionSet' => $this->buildSelectionSet($qtype->getFields()),
+                    'selectionSet' => $this->buildSelectionSet($queryType->getFields()),
                     'variableDefinitions' => new NodeList([]),
                     'directives' => new NodeList([]),
                 ]),

src/Executor/ReferenceExecutor.php

@@ -607,11 +607,14 @@ protected function getFieldDef(Schema $schema, ObjectType $parentType, string $f
         $schemaMetaFieldDef   ??= Introspection::schemaMetaFieldDef();
         $typeMetaFieldDef     ??= Introspection::typeMetaFieldDef();
         $typeNameMetaFieldDef ??= Introspection::typeNameMetaFieldDef();
-        if ($fieldName === $schemaMetaFieldDef->name && $schema->getQueryType() === $parentType) {
+
+        $queryType = $schema->getQueryType();
+
+        if ($fieldName === $schemaMetaFieldDef->name && $queryType === $parentType) {
             return $schemaMetaFieldDef;
         }
 
-        if ($fieldName === $typeMetaFieldDef->name && $schema->getQueryType() === $parentType) {
+        if ($fieldName === $typeMetaFieldDef->name && $queryType === $parentType) {
             return $typeMetaFieldDef;
         }
 

src/Type/Schema.php

@@ -51,38 +51,35 @@
  */
 class Schema
 {
-    /** @var SchemaConfig */
-    private $config;
+    private SchemaConfig $config;
 
     /**
      * Contains currently resolved schema types
      *
-     * @var Type[]
+     * @var array<string, Type>
      */
-    private $resolvedTypes = [];
+    private array $resolvedTypes = [];
 
     /**
      * Lazily initialised.
      *
      * @var array<string, InterfaceImplementations>
      */
-    private $implementationsMap;
+    private array $implementationsMap;
 
     /**
-     * True when $resolvedTypes contain all possible schema types
-     *
-     * @var bool
+     * True when $resolvedTypes contains all possible schema types.
      */
-    private $fullyLoaded = false;
+    private bool $fullyLoaded = false;
 
-    /** @var Error[] */
-    private $validationErrors;
+    /** @var array<int, Error> */
+    private array $validationErrors;
 
     /** @var array<int, SchemaTypeExtensionNode> */
     public $extensionASTNodes = [];
 
     /**
-     * @param mixed[]|SchemaConfig $config
+     * @param SchemaConfig|array<string, mixed> $config
      *
      * @api
      */
@@ -128,31 +125,38 @@ public function __construct($config)
         $this->config            = $config;
         $this->extensionASTNodes = $config->extensionASTNodes;
 
-        if ($config->query !== null) {
-            $this->resolvedTypes[$config->query->name] = $config->query;
+        // TODO can we make the following assumption hold true?
+        // No need to check for the existence of the root query type
+        // since we already validated the schema thus it must exist.
+        $query = $config->query;
+        if ($query !== null) {
+            $this->resolvedTypes[$query->name] = $query;
         }
 
-        if ($config->mutation !== null) {
-            $this->resolvedTypes[$config->mutation->name] = $config->mutation;
+        $mutation = $config->mutation;
+        if ($mutation !== null) {
+            $this->resolvedTypes[$mutation->name] = $mutation;
         }
 
-        if ($config->subscription !== null) {
-            $this->resolvedTypes[$config->subscription->name] = $config->subscription;
+        $subscription = $config->subscription;
+        if ($subscription !== null) {
+            $this->resolvedTypes[$subscription->name] = $subscription;
         }
 
         if (is_array($this->config->types)) {
             foreach ($this->resolveAdditionalTypes() as $type) {
-                if (isset($this->resolvedTypes[$type->name])) {
+                $typeName = $type->name;
+                if (isset($this->resolvedTypes[$typeName])) {
                     Utils::invariant(
-                        $type === $this->resolvedTypes[$type->name],
+                        $type === $this->resolvedTypes[$typeName],
                         sprintf(
                             'Schema must contain unique named types but contains multiple types named "%s" (see https://webonyx.github.io/graphql-php/type-definitions/#type-registry).',
                             $type
                         )
                     );
                 }
 
-                $this->resolvedTypes[$type->name] = $type;
+                $this->resolvedTypes[$typeName] = $type;
             }
         }
 
@@ -196,12 +200,11 @@ private function resolveAdditionalTypes(): Generator
     }
 
     /**
-     * Returns array of all types in this schema. Keys of this array represent type names, values are instances
-     * of corresponding type definitions
+     * Returns all types in this schema.
      *
-     * This operation requires full schema scan. Do not use in production environment.
+     * This operation requires a full schema scan. Do not use in production environment.
      *
-     * @return array<string, Type>
+     * @return array<string, Type> Keys represent type names, values are instances of corresponding type definitions
      *
      * @api
      */
@@ -216,9 +219,9 @@ public function getTypeMap(): array
     }
 
     /**
-     * @return Type[]
+     * @return array<Type>
      */
-    private function collectAllTypes()
+    private function collectAllTypes(): array
     {
         $typeMap = [];
         foreach ($this->resolvedTypes as $type) {
@@ -246,21 +249,16 @@ private function collectAllTypes()
     /**
      * Returns a list of directives supported by this schema
      *
-     * @return Directive[]
+     * @return array<Directive>
      *
      * @api
      */
-    public function getDirectives()
+    public function getDirectives(): array
     {
         return $this->config->directives ?? GraphQL::getStandardDirectives();
     }
 
-    /**
-     * @param string $operation
-     *
-     * @return ObjectType|null
-     */
-    public function getOperationType($operation)
+    public function getOperationType(string $operation): ?ObjectType
     {
         switch ($operation) {
             case 'query':
@@ -278,53 +276,45 @@ public function getOperationType($operation)
     }
 
     /**
-     * Returns schema query type
-     *
-     * @return ObjectType|null
+     * Returns root query type.
      *
      * @api
      */
-    public function getQueryType(): ?Type
+    public function getQueryType(): ?ObjectType
     {
         return $this->config->query;
     }
 
     /**
-     * Returns schema mutation type
-     *
-     * @return ObjectType|null
+     * Returns root mutation type.
      *
      * @api
      */
-    public function getMutationType(): ?Type
+    public function getMutationType(): ?ObjectType
     {
         return $this->config->mutation;
     }
 
     /**
      * Returns schema subscription
      *
-     * @return ObjectType|null
-     *
      * @api
      */
-    public function getSubscriptionType(): ?Type
+    public function getSubscriptionType(): ?ObjectType
     {
         return $this->config->subscription;
     }
 
     /**
-     * @return SchemaConfig
-     *
      * @api
      */
-    public function getConfig()
+    public function getConfig(): SchemaConfig
     {
         return $this->config;
     }
 
     /**
-     * Returns type by its name
+     * Returns a type by name.
      *
      * @api
      */
@@ -534,15 +524,15 @@ public function getAstNode(): ?SchemaDefinitionNode
     }
 
     /**
-     * Validates schema.
+     * Throws if the schema is not valid.
      *
-     * This operation requires full schema scan. Do not use in production environment.
+     * This operation requires a full schema scan. Do not use in production environment.
      *
      * @throws InvariantViolation
      *
      * @api
      */
-    public function assertValid()
+    public function assertValid(): void
     {
         $errors = $this->validate();
 
@@ -574,18 +564,18 @@ public function assertValid()
     }
 
     /**
-     * Validates schema.
+     * Validate the schema and return any errors.
      *
-     * This operation requires full schema scan. Do not use in production environment.
+     * This operation requires a full schema scan. Do not use in production environment.
      *
-     * @return InvariantViolation[]|Error[]
+     * @return array<int, Error>
      *
      * @api
      */
-    public function validate()
+    public function validate(): array
     {
         // If this Schema has already been validated, return the previous results.
-        if ($this->validationErrors !== null) {
+        if (isset($this->validationErrors)) {
             return $this->validationErrors;
         }