From 8afce0413f2e57226b6c7f714916350ac4b9b75a Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Fri, 11 Dec 2020 14:50:33 +0100 Subject: [PATCH 1/8] Reify AST node types and remove unneeded nullability This achieves a few things: - make the code easier to understand both by humans and static analyzers - improve performance by eliminating unnecessary null checks - improve usability through a clarified API --- phpstan-baseline.neon | 5 - src/Language/AST/EnumTypeDefinitionNode.php | 2 +- src/Language/AST/FieldNode.php | 4 +- src/Language/AST/InlineFragmentNode.php | 2 +- .../AST/InputObjectTypeDefinitionNode.php | 4 +- .../AST/InputObjectTypeExtensionNode.php | 4 +- .../AST/InterfaceTypeDefinitionNode.php | 4 +- .../AST/InterfaceTypeExtensionNode.php | 4 +- src/Language/AST/Location.php | 4 +- src/Language/AST/ObjectTypeDefinitionNode.php | 2 +- src/Language/AST/ScalarTypeExtensionNode.php | 2 +- src/Language/AST/SchemaTypeExtensionNode.php | 4 +- src/Language/AST/StringValueNode.php | 2 +- src/Language/AST/UnionTypeDefinitionNode.php | 2 +- src/Language/AST/UnionTypeExtensionNode.php | 4 +- src/Language/Parser.php | 7 +- src/Type/Schema.php | 4 +- src/Utils/ASTDefinitionBuilder.php | 245 +++++++++--------- src/Utils/BuildSchema.php | 17 +- src/Utils/SchemaExtender.php | 29 ++- tests/Type/ValidationTest.php | 17 +- tests/Utils/SchemaExtenderTest.php | 6 +- 22 files changed, 189 insertions(+), 185 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index a1c6dd677..ae37284e5 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -310,11 +310,6 @@ parameters: count: 1 path: src/Utils/ASTDefinitionBuilder.php - - - message: "#^Only booleans are allowed in a ternary operator condition, GraphQL\\\\Language\\\\AST\\\\NodeList\\\\|null given\\.$#" - count: 1 - path: src/Utils/ASTDefinitionBuilder.php - - message: "#^Only booleans are allowed in &&, array\\\\|null given on the left side\\.$#" count: 1 diff --git a/src/Language/AST/EnumTypeDefinitionNode.php b/src/Language/AST/EnumTypeDefinitionNode.php index 519d225b0..5d8c208ed 100644 --- a/src/Language/AST/EnumTypeDefinitionNode.php +++ b/src/Language/AST/EnumTypeDefinitionNode.php @@ -15,7 +15,7 @@ class EnumTypeDefinitionNode extends Node implements TypeDefinitionNode /** @var NodeList */ public $directives; - /** @var NodeList|null */ + /** @var NodeList */ public $values; /** @var StringValueNode|null */ diff --git a/src/Language/AST/FieldNode.php b/src/Language/AST/FieldNode.php index a0e643b8f..174dbd979 100644 --- a/src/Language/AST/FieldNode.php +++ b/src/Language/AST/FieldNode.php @@ -15,10 +15,10 @@ class FieldNode extends Node implements SelectionNode /** @var NameNode|null */ public $alias; - /** @var NodeList|null */ + /** @var NodeList */ public $arguments; - /** @var NodeList|null */ + /** @var NodeList */ public $directives; /** @var SelectionSetNode|null */ diff --git a/src/Language/AST/InlineFragmentNode.php b/src/Language/AST/InlineFragmentNode.php index c9187987f..0c12e22f3 100644 --- a/src/Language/AST/InlineFragmentNode.php +++ b/src/Language/AST/InlineFragmentNode.php @@ -12,7 +12,7 @@ class InlineFragmentNode extends Node implements SelectionNode /** @var NamedTypeNode */ public $typeCondition; - /** @var NodeList|null */ + /** @var NodeList */ public $directives; /** @var SelectionSetNode */ diff --git a/src/Language/AST/InputObjectTypeDefinitionNode.php b/src/Language/AST/InputObjectTypeDefinitionNode.php index b5114d7bd..cc3245451 100644 --- a/src/Language/AST/InputObjectTypeDefinitionNode.php +++ b/src/Language/AST/InputObjectTypeDefinitionNode.php @@ -12,10 +12,10 @@ class InputObjectTypeDefinitionNode extends Node implements TypeDefinitionNode /** @var NameNode */ public $name; - /** @var NodeList|null */ + /** @var NodeList */ public $directives; - /** @var NodeList|null */ + /** @var NodeList */ public $fields; /** @var StringValueNode|null */ diff --git a/src/Language/AST/InputObjectTypeExtensionNode.php b/src/Language/AST/InputObjectTypeExtensionNode.php index c102ab06e..e470958e5 100644 --- a/src/Language/AST/InputObjectTypeExtensionNode.php +++ b/src/Language/AST/InputObjectTypeExtensionNode.php @@ -12,9 +12,9 @@ class InputObjectTypeExtensionNode extends Node implements TypeExtensionNode /** @var NameNode */ public $name; - /** @var NodeList|null */ + /** @var NodeList */ public $directives; - /** @var NodeList|null */ + /** @var NodeList */ public $fields; } diff --git a/src/Language/AST/InterfaceTypeDefinitionNode.php b/src/Language/AST/InterfaceTypeDefinitionNode.php index e56c38459..477edf965 100644 --- a/src/Language/AST/InterfaceTypeDefinitionNode.php +++ b/src/Language/AST/InterfaceTypeDefinitionNode.php @@ -12,10 +12,10 @@ class InterfaceTypeDefinitionNode extends Node implements TypeDefinitionNode /** @var NameNode */ public $name; - /** @var NodeList|null */ + /** @var NodeList */ public $directives; - /** @var NodeList|null */ + /** @var NodeList */ public $fields; /** @var StringValueNode|null */ diff --git a/src/Language/AST/InterfaceTypeExtensionNode.php b/src/Language/AST/InterfaceTypeExtensionNode.php index 38e340e15..f79896290 100644 --- a/src/Language/AST/InterfaceTypeExtensionNode.php +++ b/src/Language/AST/InterfaceTypeExtensionNode.php @@ -12,9 +12,9 @@ class InterfaceTypeExtensionNode extends Node implements TypeExtensionNode /** @var NameNode */ public $name; - /** @var NodeList|null */ + /** @var NodeList */ public $directives; - /** @var NodeList|null */ + /** @var NodeList */ public $fields; } diff --git a/src/Language/AST/Location.php b/src/Language/AST/Location.php index 6f2e6d4af..dfd70b68b 100644 --- a/src/Language/AST/Location.php +++ b/src/Language/AST/Location.php @@ -30,14 +30,14 @@ class Location /** * The Token at which this Node begins. * - * @var Token + * @var Token|null */ public $startToken; /** * The Token at which this Node ends. * - * @var Token + * @var Token|null */ public $endToken; diff --git a/src/Language/AST/ObjectTypeDefinitionNode.php b/src/Language/AST/ObjectTypeDefinitionNode.php index fa9498c80..d221a101e 100644 --- a/src/Language/AST/ObjectTypeDefinitionNode.php +++ b/src/Language/AST/ObjectTypeDefinitionNode.php @@ -18,7 +18,7 @@ class ObjectTypeDefinitionNode extends Node implements TypeDefinitionNode /** @var NodeList */ public $directives; - /** @var NodeList|null */ + /** @var NodeList */ public $fields; /** @var StringValueNode|null */ diff --git a/src/Language/AST/ScalarTypeExtensionNode.php b/src/Language/AST/ScalarTypeExtensionNode.php index 737541743..ea7d16a69 100644 --- a/src/Language/AST/ScalarTypeExtensionNode.php +++ b/src/Language/AST/ScalarTypeExtensionNode.php @@ -12,6 +12,6 @@ class ScalarTypeExtensionNode extends Node implements TypeExtensionNode /** @var NameNode */ public $name; - /** @var NodeList|null */ + /** @var NodeList */ public $directives; } diff --git a/src/Language/AST/SchemaTypeExtensionNode.php b/src/Language/AST/SchemaTypeExtensionNode.php index 7c9973819..c96bcdf67 100644 --- a/src/Language/AST/SchemaTypeExtensionNode.php +++ b/src/Language/AST/SchemaTypeExtensionNode.php @@ -9,9 +9,9 @@ class SchemaTypeExtensionNode extends Node implements TypeExtensionNode /** @var string */ public $kind = NodeKind::SCHEMA_EXTENSION; - /** @var NodeList|null */ + /** @var NodeList */ public $directives; - /** @var NodeList|null */ + /** @var NodeList */ public $operationTypes; } diff --git a/src/Language/AST/StringValueNode.php b/src/Language/AST/StringValueNode.php index b1fe0dfef..059a7e310 100644 --- a/src/Language/AST/StringValueNode.php +++ b/src/Language/AST/StringValueNode.php @@ -12,6 +12,6 @@ class StringValueNode extends Node implements ValueNode /** @var string */ public $value; - /** @var bool|null */ + /** @var bool */ public $block; } diff --git a/src/Language/AST/UnionTypeDefinitionNode.php b/src/Language/AST/UnionTypeDefinitionNode.php index ff94bff00..d725d8efb 100644 --- a/src/Language/AST/UnionTypeDefinitionNode.php +++ b/src/Language/AST/UnionTypeDefinitionNode.php @@ -15,7 +15,7 @@ class UnionTypeDefinitionNode extends Node implements TypeDefinitionNode /** @var NodeList */ public $directives; - /** @var NodeList|null */ + /** @var NodeList */ public $types; /** @var StringValueNode|null */ diff --git a/src/Language/AST/UnionTypeExtensionNode.php b/src/Language/AST/UnionTypeExtensionNode.php index a19b9a148..b10eebd5b 100644 --- a/src/Language/AST/UnionTypeExtensionNode.php +++ b/src/Language/AST/UnionTypeExtensionNode.php @@ -12,9 +12,9 @@ class UnionTypeExtensionNode extends Node implements TypeExtensionNode /** @var NameNode */ public $name; - /** @var NodeList|null */ + /** @var NodeList */ public $directives; - /** @var NodeList|null */ + /** @var NodeList */ public $types; } diff --git a/src/Language/Parser.php b/src/Language/Parser.php index 462de219a..6659781cb 100644 --- a/src/Language/Parser.php +++ b/src/Language/Parser.php @@ -629,8 +629,9 @@ private function parseVariableDefinition() : VariableDefinitionNode return new VariableDefinitionNode([ 'variable' => $var, 'type' => $type, - 'defaultValue' => - ($this->skip(Token::EQUALS) ? $this->parseValueLiteral(true) : null), + 'defaultValue' => $this->skip(Token::EQUALS) + ? $this->parseValueLiteral(true) + : null, 'directives' => $this->parseDirectives(true), 'loc' => $this->loc($start), ]); @@ -1551,7 +1552,7 @@ private function parseSchemaTypeExtension() : SchemaTypeExtensionNode [$this, 'parseOperationTypeDefinition'], Token::BRACE_R ) - : []; + : new NodeList([]); if (count($directives) === 0 && count($operationTypes) === 0) { $this->unexpected(); } diff --git a/src/Type/Schema.php b/src/Type/Schema.php index 628d94b26..867a6b496 100644 --- a/src/Type/Schema.php +++ b/src/Type/Schema.php @@ -194,11 +194,11 @@ private function resolveAdditionalTypes() * * This operation requires full schema scan. Do not use in production environment. * - * @return Type[] + * @return array * * @api */ - public function getTypeMap() + public function getTypeMap() : array { if (! $this->fullyLoaded) { $this->resolvedTypes = $this->collectAllTypes(); diff --git a/src/Utils/ASTDefinitionBuilder.php b/src/Utils/ASTDefinitionBuilder.php index 9f44c4ce4..283afb51b 100644 --- a/src/Utils/ASTDefinitionBuilder.php +++ b/src/Utils/ASTDefinitionBuilder.php @@ -15,10 +15,13 @@ use GraphQL\Language\AST\InterfaceTypeDefinitionNode; use GraphQL\Language\AST\ListTypeNode; use GraphQL\Language\AST\NamedTypeNode; +use GraphQL\Language\AST\NameNode; use GraphQL\Language\AST\Node; +use GraphQL\Language\AST\NodeList; use GraphQL\Language\AST\NonNullTypeNode; use GraphQL\Language\AST\ObjectTypeDefinitionNode; use GraphQL\Language\AST\ScalarTypeDefinitionNode; +use GraphQL\Language\AST\TypeDefinitionNode; use GraphQL\Language\AST\TypeNode; use GraphQL\Language\AST\UnionTypeDefinitionNode; use GraphQL\Language\Token; @@ -27,7 +30,6 @@ use GraphQL\Type\Definition\EnumType; use GraphQL\Type\Definition\FieldArgument; use GraphQL\Type\Definition\InputObjectType; -use GraphQL\Type\Definition\InputType; use GraphQL\Type\Definition\InterfaceType; use GraphQL\Type\Definition\ObjectType; use GraphQL\Type\Definition\Type; @@ -41,28 +43,28 @@ class ASTDefinitionBuilder { - /** @var Node[] */ + /** @var array */ private $typeDefinitionsMap; /** @var callable */ private $typeConfigDecorator; - /** @var bool[] */ + /** @var array */ private $options; /** @var callable */ private $resolveType; - /** @var Type[] */ + /** @var array */ private $cache; /** - * @param Node[] $typeDefinitionsMap - * @param bool[] $options + * @param array $typeDefinitionsMap + * @param array $options */ public function __construct( array $typeDefinitionsMap, - $options, + array $options, callable $resolveType, ?callable $typeConfigDecorator = null ) { @@ -74,31 +76,32 @@ public function __construct( $this->cache = Type::getAllBuiltInTypes(); } - public function buildDirective(DirectiveDefinitionNode $directiveNode) + public function buildDirective(DirectiveDefinitionNode $directiveNode) : Directive { return new Directive([ - 'name' => $directiveNode->name->value, - 'description' => $this->getDescription($directiveNode), - 'args' => isset($directiveNode->arguments) ? FieldArgument::createMap($this->makeInputValues($directiveNode->arguments)) : null, - 'isRepeatable' => $directiveNode->repeatable, - 'locations' => Utils::map( + 'name' => $directiveNode->name->value, + 'description' => $this->getDescription($directiveNode), + 'args' => FieldArgument::createMap($this->makeInputValues($directiveNode->arguments)), + 'isRepeatable' => $directiveNode->repeatable, + 'locations' => Utils::map( $directiveNode->locations, - static function ($node) { + static function (NameNode $node) : string { return $node->value; } ), - 'astNode' => $directiveNode, + 'astNode' => $directiveNode, ]); } /** * Given an ast node, returns its string description. */ - private function getDescription($node) + private function getDescription(Node $node) : ?string { - if ($node->description) { + if (isset($node->description)) { return $node->description->value; } + if (isset($this->options['commentDescriptions'])) { $rawValue = $this->getLeadingCommentBlock($node); if ($rawValue !== null) { @@ -109,19 +112,21 @@ private function getDescription($node) return null; } - private function getLeadingCommentBlock($node) + private function getLeadingCommentBlock(Node $node) : ?string { $loc = $node->loc; - if (! $loc || ! $loc->startToken) { + if ($loc === null || $loc->startToken === null) { return null; } + $comments = []; $token = $loc->startToken->prev; - while ($token && - $token->kind === Token::COMMENT && - $token->next && $token->prev && - $token->line + 1 === $token->next->line && - $token->line !== $token->prev->line + while ($token !== null + && $token->kind === Token::COMMENT + && $token->next !== null + && $token->prev !== null + && $token->line + 1 === $token->next->line + && $token->line !== $token->prev->line ) { $value = $token->value; $comments[] = $value; @@ -131,14 +136,17 @@ private function getLeadingCommentBlock($node) return implode("\n", array_reverse($comments)); } - private function makeInputValues($values) + /** + * @return array> + */ + private function makeInputValues(NodeList $values) : array { return Utils::keyValMap( $values, - static function ($value) { + static function (InputValueDefinitionNode $value) : string { return $value->name->value; }, - function ($value) : array { + function (InputValueDefinitionNode $value) : array { // Note: While this could make assertions to get the correctly typed // value, that would throw immediately while type system validation // with validateSchema() will produce more actionable results. @@ -159,16 +167,12 @@ function ($value) : array { ); } - /** - * @return Type|InputType - * - * @throws Error - */ - private function buildWrappedType(TypeNode $typeNode) + private function buildWrappedType(TypeNode $typeNode) : Type { if ($typeNode instanceof ListTypeNode) { return Type::listOf($this->buildWrappedType($typeNode->type)); } + if ($typeNode instanceof NonNullTypeNode) { return Type::nonNull($this->buildWrappedType($typeNode->type)); } @@ -177,13 +181,9 @@ private function buildWrappedType(TypeNode $typeNode) } /** - * @param string|NamedTypeNode $ref - * - * @return Type - * - * @throws Error + * @param string|(Node&NamedTypeNode)|(Node&TypeDefinitionNode) $ref */ - public function buildType($ref) + public function buildType($ref) : Type { if (is_string($ref)) { return $this->internalBuildType($ref); @@ -193,14 +193,11 @@ public function buildType($ref) } /** - * @param string $typeName - * @param NamedTypeNode|null $typeNode - * - * @return Type + * @param (Node&NamedTypeNode)|(Node&TypeDefinitionNode)|null $typeNode * * @throws Error */ - private function internalBuildType($typeName, $typeNode = null) + private function internalBuildType(string $typeName, ?Node $typeNode = null) : Type { if (! isset($this->cache[$typeName])) { if (isset($this->typeDefinitionsMap[$typeName])) { @@ -248,7 +245,7 @@ private function internalBuildType($typeName, $typeNode = null) * * @throws Error */ - private function makeSchemaDef(Node $def) + private function makeSchemaDef(Node $def) : Type { switch (true) { case $def instanceof ObjectTypeDefinitionNode: @@ -268,39 +265,43 @@ private function makeSchemaDef(Node $def) } } - private function makeTypeDef(ObjectTypeDefinitionNode $def) + private function makeTypeDef(ObjectTypeDefinitionNode $def) : ObjectType { - $typeName = $def->name->value; - return new ObjectType([ - 'name' => $typeName, + 'name' => $def->name->value, 'description' => $this->getDescription($def), - 'fields' => function () use ($def) { + 'fields' => function () use ($def) : array { return $this->makeFieldDefMap($def); }, - 'interfaces' => function () use ($def) { + 'interfaces' => function () use ($def) : array { return $this->makeImplementedInterfaces($def); }, 'astNode' => $def, ]); } - private function makeFieldDefMap($def) + /** + * @param ObjectTypeDefinitionNode|InterfaceTypeDefinitionNode $def + * + * @return array> + */ + private function makeFieldDefMap(Node $def) : array { - return $def->fields - ? Utils::keyValMap( - $def->fields, - static function ($field) { - return $field->name->value; - }, - function ($field) { - return $this->buildField($field); - } - ) - : []; + return Utils::keyValMap( + $def->fields, + static function (FieldDefinitionNode $field) : string { + return $field->name->value; + }, + function (FieldDefinitionNode $field) : array { + return $this->buildField($field); + } + ); } - public function buildField(FieldDefinitionNode $field) + /** + * @return array + */ + public function buildField(FieldDefinitionNode $field) : array { return [ // Note: While this could make assertions to get the correctly typed @@ -308,7 +309,7 @@ public function buildField(FieldDefinitionNode $field) // with validateSchema() will produce more actionable results. 'type' => $this->buildWrappedType($field->type), 'description' => $this->getDescription($field), - 'args' => isset($field->arguments) ? $this->makeInputValues($field->arguments) : null, + 'args' => $this->makeInputValues($field->arguments), 'deprecationReason' => $this->getDeprecationReason($field), 'astNode' => $field, ]; @@ -318,73 +319,69 @@ public function buildField(FieldDefinitionNode $field) * Given a collection of directives, returns the string value for the * deprecation reason. * - * @param EnumValueDefinitionNode | FieldDefinitionNode $node - * - * @return string + * @param EnumValueDefinitionNode|FieldDefinitionNode $node */ - private function getDeprecationReason($node) + private function getDeprecationReason(Node $node) : ?string { - $deprecated = Values::getDirectiveValues(Directive::deprecatedDirective(), $node); + $deprecated = Values::getDirectiveValues( + Directive::deprecatedDirective(), + $node + ); return $deprecated['reason'] ?? null; } - private function makeImplementedInterfaces(ObjectTypeDefinitionNode $def) + /** + * @return array + */ + private function makeImplementedInterfaces(ObjectTypeDefinitionNode $def) : array { - if ($def->interfaces !== null) { - // Note: While this could make early assertions to get the correctly - // typed values, that would throw immediately while type system - // validation with validateSchema() will produce more actionable results. - return Utils::map( - $def->interfaces, - function ($iface) : Type { - return $this->buildType($iface); - } - ); - } - - return null; + // Note: While this could make early assertions to get the correctly + // typed values, that would throw immediately while type system + // validation with validateSchema() will produce more actionable results. + return Utils::map( + $def->interfaces, + function (NamedTypeNode $iface) : Type { + return $this->buildType($iface); + } + ); } - private function makeInterfaceDef(InterfaceTypeDefinitionNode $def) + private function makeInterfaceDef(InterfaceTypeDefinitionNode $def) : InterfaceType { - $typeName = $def->name->value; - return new InterfaceType([ - 'name' => $typeName, + 'name' => $def->name->value, 'description' => $this->getDescription($def), - 'fields' => function () use ($def) { + 'fields' => function () use ($def) : array { return $this->makeFieldDefMap($def); }, 'astNode' => $def, ]); } - private function makeEnumDef(EnumTypeDefinitionNode $def) + private function makeEnumDef(EnumTypeDefinitionNode $def) : EnumType { return new EnumType([ 'name' => $def->name->value, 'description' => $this->getDescription($def), - 'values' => $def->values - ? Utils::keyValMap( - $def->values, - static function ($enumValue) { - return $enumValue->name->value; - }, - function ($enumValue) : array { - return [ - 'description' => $this->getDescription($enumValue), - 'deprecationReason' => $this->getDeprecationReason($enumValue), - 'astNode' => $enumValue, - ]; - } - ) - : [], + 'values' => Utils::keyValMap( + $def->values, + static function ($enumValue) { + return $enumValue->name->value; + }, + function ($enumValue) : array { + return [ + 'description' => $this->getDescription($enumValue), + 'deprecationReason' => $this->getDeprecationReason($enumValue), + 'astNode' => $enumValue, + ]; + } + ), 'astNode' => $def, ]); } - private function makeUnionDef(UnionTypeDefinitionNode $def) + private function makeUnionDef(UnionTypeDefinitionNode $def) : UnionType { return new UnionType([ 'name' => $def->name->value, @@ -392,21 +389,19 @@ private function makeUnionDef(UnionTypeDefinitionNode $def) // Note: While this could make assertions to get the correctly typed // values below, that would throw immediately while type system // validation with validateSchema() will produce more actionable results. - 'types' => isset($def->types) - ? function () use ($def) { - return Utils::map( - $def->types, - function ($typeNode) : Type { - return $this->buildType($typeNode); - } - ); - } - : [], + 'types' => function () use ($def) { + return Utils::map( + $def->types, + function ($typeNode) : Type { + return $this->buildType($typeNode); + } + ); + }, 'astNode' => $def, ]); } - private function makeScalarDef(ScalarTypeDefinitionNode $def) + private function makeScalarDef(ScalarTypeDefinitionNode $def) : CustomScalarType { return new CustomScalarType([ 'name' => $def->name->value, @@ -418,28 +413,26 @@ private function makeScalarDef(ScalarTypeDefinitionNode $def) ]); } - private function makeInputObjectDef(InputObjectTypeDefinitionNode $def) + private function makeInputObjectDef(InputObjectTypeDefinitionNode $def) : InputObjectType { return new InputObjectType([ 'name' => $def->name->value, 'description' => $this->getDescription($def), - 'fields' => function () use ($def) { - return $def->fields !== null - ? $this->makeInputValues($def->fields) - : []; + 'fields' => function () use ($def) : array { + return $this->makeInputValues($def->fields); }, 'astNode' => $def, ]); } /** - * @param mixed[] $config + * @param array $config * * @return CustomScalarType|EnumType|InputObjectType|InterfaceType|ObjectType|UnionType * * @throws Error */ - private function makeSchemaDefFromConfig(Node $def, array $config) + private function makeSchemaDefFromConfig(Node $def, array $config) : Type { switch (true) { case $def instanceof ObjectTypeDefinitionNode: @@ -460,7 +453,7 @@ private function makeSchemaDefFromConfig(Node $def, array $config) } /** - * @return mixed[] + * @return array */ public function buildInputField(InputValueDefinitionNode $value) : array { @@ -481,7 +474,7 @@ public function buildInputField(InputValueDefinitionNode $value) : array } /** - * @return mixed[] + * @return array */ public function buildEnumValue(EnumValueDefinitionNode $value) : array { diff --git a/src/Utils/BuildSchema.php b/src/Utils/BuildSchema.php index 5fb1ecfbd..3e0577547 100644 --- a/src/Utils/BuildSchema.php +++ b/src/Utils/BuildSchema.php @@ -33,17 +33,17 @@ class BuildSchema /** @var DocumentNode */ private $ast; - /** @var TypeDefinitionNode[] */ + /** @var array */ private $nodeMap; /** @var callable|null */ private $typeConfigDecorator; - /** @var bool[] */ + /** @var array */ private $options; /** - * @param bool[] $options + * @param array $options */ public function __construct(DocumentNode $ast, ?callable $typeConfigDecorator = null, array $options = []) { @@ -57,7 +57,7 @@ public function __construct(DocumentNode $ast, ?callable $typeConfigDecorator = * document. * * @param DocumentNode|Source|string $source - * @param bool[] $options + * @param array $options * * @return Schema * @@ -65,7 +65,9 @@ public function __construct(DocumentNode $ast, ?callable $typeConfigDecorator = */ public static function build($source, ?callable $typeConfigDecorator = null, array $options = []) { - $doc = $source instanceof DocumentNode ? $source : Parser::parse($source); + $doc = $source instanceof DocumentNode + ? $source + : Parser::parse($source); return self::buildAST($doc, $typeConfigDecorator, $options); } @@ -86,7 +88,7 @@ public static function build($source, ?callable $typeConfigDecorator = null, arr * Provide true to use preceding comments as the description. * This option is provided to ease adoption and will be removed in v16. * - * @param bool[] $options + * @param array $options * * @return Schema * @@ -111,6 +113,7 @@ public function buildSchema() $schemaDef = null; $typeDefs = []; $this->nodeMap = []; + /** @var array $directiveDefs */ $directiveDefs = []; foreach ($this->ast->definitions as $definition) { switch (true) { @@ -149,7 +152,7 @@ static function ($typeName) : void { ); $directives = array_map( - static function ($def) use ($DefinitionBuilder) { + static function (DirectiveDefinitionNode $def) use ($DefinitionBuilder) : Directive { return $DefinitionBuilder->buildDirective($def); }, $directiveDefs diff --git a/src/Utils/SchemaExtender.php b/src/Utils/SchemaExtender.php index 951b8c3ee..63b14af49 100644 --- a/src/Utils/SchemaExtender.php +++ b/src/Utils/SchemaExtender.php @@ -474,7 +474,7 @@ protected static function getMergedDirectives(Schema $schema, array $directiveDe return array_merge( $existingDirectives, - array_map(static function (DirectiveDefinitionNode $directive) { + array_map(static function (DirectiveDefinitionNode $directive) : Directive { return static::$astBuilder->buildDirective($directive); }, $directiveDefinitions) ); @@ -492,20 +492,21 @@ protected static function extendDirective(Directive $directive) : Directive } /** - * @param mixed[]|null $options + * @param array $options */ - public static function extend(Schema $schema, DocumentNode $documentAST, ?array $options = null) : Schema + public static function extend(Schema $schema, DocumentNode $documentAST, array $options = []) : Schema { - if ($options === null || ! (isset($options['assumeValid']) || isset($options['assumeValidSDL']))) { + if (! (isset($options['assumeValid']) || isset($options['assumeValidSDL']))) { DocumentValidator::assertValidSDLExtension($documentAST, $schema); } + /** @var array $typeDefinitionMap */ $typeDefinitionMap = []; static::$typeExtensionsMap = []; $directiveDefinitions = []; /** @var SchemaDefinitionNode|null $schemaDef */ $schemaDef = null; - /** @var SchemaTypeExtensionNode[] $schemaExtensions */ + /** @var array $schemaExtensions */ $schemaExtensions = []; $definitionsCount = count($documentAST->definitions); @@ -552,11 +553,11 @@ public static function extend(Schema $schema, DocumentNode $documentAST, ?array } } - if (count(static::$typeExtensionsMap) === 0 && - count($typeDefinitionMap) === 0 && - count($directiveDefinitions) === 0 && - count($schemaExtensions) === 0 && - $schemaDef === null + if (count(static::$typeExtensionsMap) === 0 + && count($typeDefinitionMap) === 0 + && count($directiveDefinitions) === 0 + && count($schemaExtensions) === 0 + && $schemaDef === null ) { return $schema; } @@ -615,13 +616,13 @@ static function (string $typeName) use ($schema) { $types = array_merge( // Iterate through all types, getting the type definition for each, ensuring // that any type not directly referenced by a field will get created. - array_map(static function ($type) { + array_map(static function (Type $type) : Type { return static::extendNamedType($type); - }, array_values($schema->getTypeMap())), + }, $schema->getTypeMap()), // Do the same with new types. - array_map(static function ($type) : Type { + array_map(static function (TypeDefinitionNode $type) : Type { return static::$astBuilder->buildType($type); - }, array_values($typeDefinitionMap)) + }, $typeDefinitionMap) ); return new Schema([ diff --git a/tests/Type/ValidationTest.php b/tests/Type/ValidationTest.php index b60474a3e..a2b93332c 100644 --- a/tests/Type/ValidationTest.php +++ b/tests/Type/ValidationTest.php @@ -889,7 +889,10 @@ public function testRejectsAUnionTypeWithNonObjectMembersType() : void | TypeB '); - $schema = SchemaExtender::extend($schema, Parser::parse('extend union BadUnion = Int')); + $schema = SchemaExtender::extend( + $schema, + Parser::parse('extend union BadUnion = Int') + ); $this->assertMatchesValidationMessage( $schema->validate(), @@ -1443,7 +1446,7 @@ interface AnotherInterface { */ public function testRejectsAnObjectImplementingTheExtendedInterfaceDueToMissingField() { - $schema = BuildSchema::build(' + $schema = BuildSchema::build(' type Query { test: AnotherObject } @@ -1455,6 +1458,7 @@ interface AnotherInterface { type AnotherObject implements AnotherInterface { field: String }'); + $extendedSchema = SchemaExtender::extend( $schema, Parser::parse(' @@ -1467,6 +1471,7 @@ interface AnotherInterface { } ') ); + $this->assertMatchesValidationMessage( $extendedSchema->validate(), [[ @@ -1486,7 +1491,7 @@ interface AnotherInterface { */ public function testRejectsAnObjectImplementingTheExtendedInterfaceDueToMissingFieldArgs() { - $schema = BuildSchema::build(' + $schema = BuildSchema::build(' type Query { test: AnotherObject } @@ -1498,6 +1503,7 @@ interface AnotherInterface { type AnotherObject implements AnotherInterface { field: String }'); + $extendedSchema = SchemaExtender::extend( $schema, Parser::parse(' @@ -1510,6 +1516,7 @@ interface AnotherInterface { } ') ); + $this->assertMatchesValidationMessage( $extendedSchema->validate(), [[ @@ -1528,7 +1535,7 @@ interface AnotherInterface { */ public function testRejectsObjectsImplementingTheExtendedInterfaceDueToMismatchingInterfaceType() { - $schema = BuildSchema::build(' + $schema = BuildSchema::build(' type Query { test: AnotherObject } @@ -1540,6 +1547,7 @@ interface AnotherInterface { type AnotherObject implements AnotherInterface { field: String }'); + $extendedSchema = SchemaExtender::extend( $schema, Parser::parse(' @@ -1565,6 +1573,7 @@ interface MismatchingInterface { } ') ); + $this->assertMatchesValidationMessage( $extendedSchema->validate(), [[ diff --git a/tests/Utils/SchemaExtenderTest.php b/tests/Utils/SchemaExtenderTest.php index ea5fbc476..7780fac72 100644 --- a/tests/Utils/SchemaExtenderTest.php +++ b/tests/Utils/SchemaExtenderTest.php @@ -204,13 +204,14 @@ protected function dedent(string $str) : string } /** - * @param mixed[]|null $options + * @param array $options */ - protected function extendTestSchema(string $sdl, ?array $options = null) : Schema + protected function extendTestSchema(string $sdl, array $options = []) : Schema { $originalPrint = SchemaPrinter::doPrint($this->testSchema); $ast = Parser::parse($sdl); $extendedSchema = SchemaExtender::extend($this->testSchema, $ast, $options); + self::assertEquals(SchemaPrinter::doPrint($this->testSchema), $originalPrint); return $extendedSchema; @@ -1190,6 +1191,7 @@ public function testMayExtendMutationsAndSubscriptions() $originalPrint = SchemaPrinter::doPrint($mutationSchema); $extendedSchema = SchemaExtender::extend($mutationSchema, $ast); + self::assertNotEquals($mutationSchema, $extendedSchema); self::assertEquals(SchemaPrinter::doPrint($mutationSchema), $originalPrint); self::assertEquals(SchemaPrinter::doPrint($extendedSchema), $this->dedent(' From af38eb684a0f1432dbfa4384f7a991a64852746d Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Fri, 11 Dec 2020 14:53:57 +0100 Subject: [PATCH 2/8] fix codestyle --- src/Utils/ASTDefinitionBuilder.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Utils/ASTDefinitionBuilder.php b/src/Utils/ASTDefinitionBuilder.php index 283afb51b..1924b15e0 100644 --- a/src/Utils/ASTDefinitionBuilder.php +++ b/src/Utils/ASTDefinitionBuilder.php @@ -181,7 +181,7 @@ private function buildWrappedType(TypeNode $typeNode) : Type } /** - * @param string|(Node&NamedTypeNode)|(Node&TypeDefinitionNode) $ref + * @param string|(Node &NamedTypeNode)|(Node&TypeDefinitionNode) $ref */ public function buildType($ref) : Type { @@ -193,7 +193,7 @@ public function buildType($ref) : Type } /** - * @param (Node&NamedTypeNode)|(Node&TypeDefinitionNode)|null $typeNode + * @param (Node &NamedTypeNode)|(Node&TypeDefinitionNode)|null $typeNode * * @throws Error */ From 2717bb7f0dc7a6da63b14dfbbd009aa3b4a7a653 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Fri, 11 Dec 2020 17:04:37 +0100 Subject: [PATCH 3/8] fix EnumTypeExtensionNode.php --- src/Language/AST/EnumTypeExtensionNode.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Language/AST/EnumTypeExtensionNode.php b/src/Language/AST/EnumTypeExtensionNode.php index c99b84edb..2c90fef63 100644 --- a/src/Language/AST/EnumTypeExtensionNode.php +++ b/src/Language/AST/EnumTypeExtensionNode.php @@ -12,9 +12,9 @@ class EnumTypeExtensionNode extends Node implements TypeExtensionNode /** @var NameNode */ public $name; - /** @var NodeList|null */ + /** @var NodeList */ public $directives; - /** @var NodeList|null */ + /** @var NodeList */ public $values; } From b78e6fd4cc7f5e1761d8fae402b1029fa8c9af85 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 5 Jan 2021 12:16:49 +0100 Subject: [PATCH 4/8] Update src/Utils/ASTDefinitionBuilder.php Co-authored-by: Simon Podlipsky --- src/Utils/ASTDefinitionBuilder.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Utils/ASTDefinitionBuilder.php b/src/Utils/ASTDefinitionBuilder.php index 1924b15e0..8c5c80fbf 100644 --- a/src/Utils/ASTDefinitionBuilder.php +++ b/src/Utils/ASTDefinitionBuilder.php @@ -59,7 +59,7 @@ class ASTDefinitionBuilder private $cache; /** - * @param array $typeDefinitionsMap + * @param array $typeDefinitionsMap * @param array $options */ public function __construct( From a2566a6232072340c61a8c295d3cb4a4fb3514cb Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 5 Jan 2021 12:29:47 +0100 Subject: [PATCH 5/8] Update ASTDefinitionBuilder.php --- src/Utils/ASTDefinitionBuilder.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Utils/ASTDefinitionBuilder.php b/src/Utils/ASTDefinitionBuilder.php index 8c5c80fbf..2ab99593b 100644 --- a/src/Utils/ASTDefinitionBuilder.php +++ b/src/Utils/ASTDefinitionBuilder.php @@ -389,7 +389,7 @@ private function makeUnionDef(UnionTypeDefinitionNode $def) : UnionType // Note: While this could make assertions to get the correctly typed // values below, that would throw immediately while type system // validation with validateSchema() will produce more actionable results. - 'types' => function () use ($def) { + 'types' => function () use ($def) : array { return Utils::map( $def->types, function ($typeNode) : Type { From 10e5481064bb717555ef1adf92fc973ece2022dd Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 5 Jan 2021 12:31:20 +0100 Subject: [PATCH 6/8] Update src/Utils/ASTDefinitionBuilder.php --- src/Utils/ASTDefinitionBuilder.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Utils/ASTDefinitionBuilder.php b/src/Utils/ASTDefinitionBuilder.php index 2ab99593b..7e7e0a4c2 100644 --- a/src/Utils/ASTDefinitionBuilder.php +++ b/src/Utils/ASTDefinitionBuilder.php @@ -59,7 +59,7 @@ class ASTDefinitionBuilder private $cache; /** - * @param array $typeDefinitionsMap + * @param array $typeDefinitionsMap * @param array $options */ public function __construct( From 4b937c43d1af8c84cd3c905022b57a3640d6cc79 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 5 Jan 2021 12:39:42 +0100 Subject: [PATCH 7/8] Fix intersection type sniffer --- src/Utils/ASTDefinitionBuilder.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Utils/ASTDefinitionBuilder.php b/src/Utils/ASTDefinitionBuilder.php index 7e7e0a4c2..e42710e2a 100644 --- a/src/Utils/ASTDefinitionBuilder.php +++ b/src/Utils/ASTDefinitionBuilder.php @@ -59,7 +59,9 @@ class ASTDefinitionBuilder private $cache; /** - * @param array $typeDefinitionsMap + * code sniffer doesn't understand this syntax. Pr with a fix here: waiting on https://github.com/squizlabs/PHP_CodeSniffer/pull/2919 + * phpcs:disable Squiz.Commenting.FunctionComment.SpacingAfterParamType + * @param array $typeDefinitionsMap * @param array $options */ public function __construct( From bda44f84209a6614d6246c2db01e63e316783f37 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Sat, 23 Jan 2021 13:36:02 +0100 Subject: [PATCH 8/8] Fix interfaces property --- src/Language/AST/InterfaceTypeDefinitionNode.php | 2 +- src/Utils/ASTDefinitionBuilder.php | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Language/AST/InterfaceTypeDefinitionNode.php b/src/Language/AST/InterfaceTypeDefinitionNode.php index d600afd5b..d0cae0eba 100644 --- a/src/Language/AST/InterfaceTypeDefinitionNode.php +++ b/src/Language/AST/InterfaceTypeDefinitionNode.php @@ -15,7 +15,7 @@ class InterfaceTypeDefinitionNode extends Node implements TypeDefinitionNode /** @var NodeList */ public $directives; - /** @var NodeList */ + /** @var NodeList */ public $interfaces; /** @var NodeList */ diff --git a/src/Utils/ASTDefinitionBuilder.php b/src/Utils/ASTDefinitionBuilder.php index f1d4c180d..92e7d27a8 100644 --- a/src/Utils/ASTDefinitionBuilder.php +++ b/src/Utils/ASTDefinitionBuilder.php @@ -334,9 +334,11 @@ private function getDeprecationReason(Node $node) : ?string } /** + * @param ObjectTypeDefinitionNode|InterfaceTypeDefinitionNode $def + * * @return array */ - private function makeImplementedInterfaces(ObjectTypeDefinitionNode $def) : array + private function makeImplementedInterfaces($def) : array { // Note: While this could make early assertions to get the correctly // typed values, that would throw immediately while type system