From d70a9a5e538ce0028bd50b27501ba979071b2761 Mon Sep 17 00:00:00 2001 From: Daniel Tschinder Date: Sun, 11 Feb 2018 13:27:26 +0100 Subject: [PATCH] Update to match SDL changes This changes the parsing grammar and validation rules to more correctly implement the current state of the GraphQL SDL proposal (facebook/graphql#90) ref: graphql/graphl-js#1102 --- docs/reference.md | 20 +-- docs/type-system/directives.md | 2 +- src/Executor/Executor.php | 5 - src/Language/AST/NodeKind.php | 4 +- src/Language/AST/ObjectTypeExtensionNode.php | 30 ++++ .../AST/TypeExtensionDefinitionNode.php | 15 -- src/Language/AST/TypeExtensionNode.php | 10 ++ src/Language/AST/TypeSystemDefinitionNode.php | 5 +- src/Language/DirectiveLocation.php | 60 +++++++ src/Language/Parser.php | 170 +++++++++++++----- src/Language/Printer.php | 14 +- src/Language/Visitor.php | 2 +- src/Type/Definition/Directive.php | 30 +--- src/Type/Definition/DirectiveLocation.php | 27 --- src/Type/Definition/ObjectType.php | 4 +- src/Type/Introspection.php | 5 +- src/Utils/ASTDefinitionBuilder.php | 1 - src/Validator/DocumentValidator.php | 2 + src/Validator/Rules/ExecutableDefinitions.php | 47 +++++ src/Validator/Rules/KnownDirectives.php | 7 +- tests/Executor/ExecutorTest.php | 15 +- tests/Language/SchemaParserTest.php | 119 +++++++++--- tests/Language/SchemaPrinterTest.php | 4 +- tests/Language/schema-kitchen-sink.graphql | 4 +- tests/Utils/BuildSchemaTest.php | 4 +- tests/Validator/ExecutableDefinitionsTest.php | 79 ++++++++ tests/Validator/KnownDirectivesTest.php | 2 + tests/Validator/TestCase.php | 27 +++ tools/gendocs.php | 2 +- 29 files changed, 522 insertions(+), 194 deletions(-) create mode 100644 src/Language/AST/ObjectTypeExtensionNode.php delete mode 100644 src/Language/AST/TypeExtensionDefinitionNode.php create mode 100644 src/Language/AST/TypeExtensionNode.php create mode 100644 src/Language/DirectiveLocation.php delete mode 100644 src/Type/Definition/DirectiveLocation.php create mode 100644 src/Validator/Rules/ExecutableDefinitions.php create mode 100644 tests/Validator/ExecutableDefinitionsTest.php diff --git a/docs/reference.md b/docs/reference.md index aa033dc..1f86412 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -374,28 +374,28 @@ public $variableValues; */ function getFieldSelection($depth = 0) ``` -# GraphQL\Type\Definition\DirectiveLocation +# GraphQL\Language\DirectiveLocation List of available directive locations **Class Constants:** ```php -const IFACE = "INTERFACE"; -const SUBSCRIPTION = "SUBSCRIPTION"; -const FRAGMENT_SPREAD = "FRAGMENT_SPREAD"; const QUERY = "QUERY"; const MUTATION = "MUTATION"; +const SUBSCRIPTION = "SUBSCRIPTION"; +const FIELD = "FIELD"; const FRAGMENT_DEFINITION = "FRAGMENT_DEFINITION"; -const INPUT_OBJECT = "INPUT_OBJECT"; +const FRAGMENT_SPREAD = "FRAGMENT_SPREAD"; const INLINE_FRAGMENT = "INLINE_FRAGMENT"; -const UNION = "UNION"; +const SCHEMA = "SCHEMA"; const SCALAR = "SCALAR"; +const OBJECT = "OBJECT"; const FIELD_DEFINITION = "FIELD_DEFINITION"; const ARGUMENT_DEFINITION = "ARGUMENT_DEFINITION"; +const IFACE = "INTERFACE"; +const UNION = "UNION"; const ENUM = "ENUM"; -const OBJECT = "OBJECT"; const ENUM_VALUE = "ENUM_VALUE"; -const FIELD = "FIELD"; -const SCHEMA = "SCHEMA"; +const INPUT_OBJECT = "INPUT_OBJECT"; const INPUT_FIELD_DEFINITION = "INPUT_FIELD_DEFINITION"; ``` @@ -936,7 +936,7 @@ const UNION_TYPE_DEFINITION = "UnionTypeDefinition"; const ENUM_TYPE_DEFINITION = "EnumTypeDefinition"; const ENUM_VALUE_DEFINITION = "EnumValueDefinition"; const INPUT_OBJECT_TYPE_DEFINITION = "InputObjectTypeDefinition"; -const TYPE_EXTENSION_DEFINITION = "TypeExtensionDefinition"; +const OBJECT_TYPE_EXTENSION = "ObjectTypeExtension"; const DIRECTIVE_DEFINITION = "DirectiveDefinition"; ``` diff --git a/docs/type-system/directives.md b/docs/type-system/directives.md index 6b12588..454ce4c 100644 --- a/docs/type-system/directives.md +++ b/docs/type-system/directives.md @@ -35,9 +35,9 @@ In **graphql-php** custom directive is an instance of `GraphQL\Type\Definition\D ```php name->value] = $definition; break; - default: - throw new Error( - "GraphQL cannot execute a request containing a {$definition->kind}.", - [$definition] - ); } } diff --git a/src/Language/AST/NodeKind.php b/src/Language/AST/NodeKind.php index e797618..22287c4 100644 --- a/src/Language/AST/NodeKind.php +++ b/src/Language/AST/NodeKind.php @@ -65,7 +65,7 @@ class NodeKind // Type Extensions - const TYPE_EXTENSION_DEFINITION = 'TypeExtensionDefinition'; + const OBJECT_TYPE_EXTENSION = 'ObjectTypeExtension'; // Directive Definitions @@ -127,7 +127,7 @@ class NodeKind NodeKind::INPUT_OBJECT_TYPE_DEFINITION =>InputObjectTypeDefinitionNode::class, // Type Extensions - NodeKind::TYPE_EXTENSION_DEFINITION => TypeExtensionDefinitionNode::class, + NodeKind::OBJECT_TYPE_EXTENSION => ObjectTypeExtensionNode::class, // Directive Definitions NodeKind::DIRECTIVE_DEFINITION => DirectiveDefinitionNode::class diff --git a/src/Language/AST/ObjectTypeExtensionNode.php b/src/Language/AST/ObjectTypeExtensionNode.php new file mode 100644 index 0000000..c8eab0a --- /dev/null +++ b/src/Language/AST/ObjectTypeExtensionNode.php @@ -0,0 +1,30 @@ + self::QUERY, + self::MUTATION => self::MUTATION, + self::SUBSCRIPTION => self::SUBSCRIPTION, + self::FIELD => self::FIELD, + self::FRAGMENT_DEFINITION => self::FRAGMENT_DEFINITION, + self::FRAGMENT_SPREAD => self::FRAGMENT_SPREAD, + self::INLINE_FRAGMENT => self::INLINE_FRAGMENT, + self::SCHEMA => self::SCHEMA, + self::SCALAR => self::SCALAR, + self::OBJECT => self::OBJECT, + self::FIELD_DEFINITION => self::FIELD_DEFINITION, + self::ARGUMENT_DEFINITION => self::ARGUMENT_DEFINITION, + self::IFACE => self::IFACE, + self::UNION => self::UNION, + self::ENUM => self::ENUM, + self::ENUM_VALUE => self::ENUM_VALUE, + self::INPUT_OBJECT => self::INPUT_OBJECT, + self::INPUT_FIELD_DEFINITION => self::INPUT_FIELD_DEFINITION, + ]; + + /** + * @param string $name + * @return bool + */ + public static function has($name) + { + return isset(self::$locations[$name]); + } +} diff --git a/src/Language/Parser.php b/src/Language/Parser.php index c0c139c..b5a16fc 100644 --- a/src/Language/Parser.php +++ b/src/Language/Parser.php @@ -36,7 +36,8 @@ use GraphQL\Language\AST\ScalarTypeDefinitionNode; use GraphQL\Language\AST\SchemaDefinitionNode; use GraphQL\Language\AST\SelectionSetNode; use GraphQL\Language\AST\StringValueNode; -use GraphQL\Language\AST\TypeExtensionDefinitionNode; +use GraphQL\Language\AST\ObjectTypeExtensionNode; +use GraphQL\Language\AST\TypeExtensionNode; use GraphQL\Language\AST\TypeSystemDefinitionNode; use GraphQL\Language\AST\UnionTypeDefinitionNode; use GraphQL\Language\AST\VariableNode; @@ -393,7 +394,7 @@ class Parser 'operation' => $operation, 'name' => $name, 'variableDefinitions' => $this->parseVariableDefinitions(), - 'directives' => $this->parseDirectives(), + 'directives' => $this->parseDirectives(false), 'selectionSet' => $this->parseSelectionSet(), 'loc' => $this->loc($start) ]); @@ -494,6 +495,7 @@ class Parser /** * @return FieldNode + * @throws SyntaxError */ function parseField() { @@ -511,20 +513,23 @@ class Parser return new FieldNode([ 'alias' => $alias, 'name' => $name, - 'arguments' => $this->parseArguments(), - 'directives' => $this->parseDirectives(), + 'arguments' => $this->parseArguments(false), + 'directives' => $this->parseDirectives(false), 'selectionSet' => $this->peek(Token::BRACE_L) ? $this->parseSelectionSet() : null, 'loc' => $this->loc($start) ]); } /** + * @param bool $isConst * @return ArgumentNode[]|NodeList + * @throws SyntaxError */ - function parseArguments() + function parseArguments($isConst) { + $item = $isConst ? 'parseConstArgument' : 'parseArgument'; return $this->peek(Token::PAREN_L) ? - $this->many(Token::PAREN_L, [$this, 'parseArgument'], Token::PAREN_R) : + $this->many(Token::PAREN_L, [$this, $item], Token::PAREN_R) : new NodeList([]); } @@ -547,6 +552,25 @@ class Parser ]); } + /** + * @return ArgumentNode + * @throws SyntaxError + */ + function parseConstArgument() + { + $start = $this->lexer->token; + $name = $this->parseName(); + + $this->expect(Token::COLON); + $value = $this->parseConstValue(); + + return new ArgumentNode([ + 'name' => $name, + 'value' => $value, + 'loc' => $this->loc($start) + ]); + } + // Implements the parsing rules in the Fragments section. /** @@ -561,7 +585,7 @@ class Parser if ($this->peek(Token::NAME) && $this->lexer->token->value !== 'on') { return new FragmentSpreadNode([ 'name' => $this->parseFragmentName(), - 'directives' => $this->parseDirectives(), + 'directives' => $this->parseDirectives(false), 'loc' => $this->loc($start) ]); } @@ -574,7 +598,7 @@ class Parser return new InlineFragmentNode([ 'typeCondition' => $typeCondition, - 'directives' => $this->parseDirectives(), + 'directives' => $this->parseDirectives(false), 'selectionSet' => $this->parseSelectionSet(), 'loc' => $this->loc($start) ]); @@ -596,7 +620,7 @@ class Parser return new FragmentDefinitionNode([ 'name' => $name, 'typeCondition' => $typeCondition, - 'directives' => $this->parseDirectives(), + 'directives' => $this->parseDirectives(false), 'selectionSet' => $this->parseSelectionSet(), 'loc' => $this->loc($start) ]); @@ -775,28 +799,31 @@ class Parser // Implements the parsing rules in the Directives section. /** + * @param bool $isConst * @return DirectiveNode[]|NodeList + * @throws SyntaxError */ - function parseDirectives() + function parseDirectives($isConst) { $directives = []; while ($this->peek(Token::AT)) { - $directives[] = $this->parseDirective(); + $directives[] = $this->parseDirective($isConst); } return new NodeList($directives); } /** + * @param bool $isConst * @return DirectiveNode * @throws SyntaxError */ - function parseDirective() + function parseDirective($isConst) { $start = $this->lexer->token; $this->expect(Token::AT); return new DirectiveNode([ 'name' => $this->parseName(), - 'arguments' => $this->parseArguments(), + 'arguments' => $this->parseArguments($isConst), 'loc' => $this->loc($start) ]); } @@ -849,7 +876,7 @@ class Parser * TypeSystemDefinition : * - SchemaDefinition * - TypeDefinition - * - TypeExtensionDefinition + * - TypeExtension * - DirectiveDefinition * * TypeDefinition : @@ -879,12 +906,12 @@ class Parser case 'union': return $this->parseUnionTypeDefinition(); case 'enum': return $this->parseEnumTypeDefinition(); case 'input': return $this->parseInputObjectTypeDefinition(); - case 'extend': return $this->parseTypeExtensionDefinition(); + case 'extend': return $this->parseTypeExtension(); case 'directive': return $this->parseDirectiveDefinition(); } } - throw $this->unexpected(); + throw $this->unexpected($keywordToken); } /** @@ -911,7 +938,7 @@ class Parser { $start = $this->lexer->token; $this->expectKeyword('schema'); - $directives = $this->parseDirectives(); + $directives = $this->parseDirectives(true); $operationTypes = $this->many( Token::BRACE_L, @@ -953,7 +980,7 @@ class Parser $description = $this->parseDescription(); $this->expectKeyword('scalar'); $name = $this->parseName(); - $directives = $this->parseDirectives(); + $directives = $this->parseDirectives(true); return new ScalarTypeDefinitionNode([ 'name' => $name, @@ -974,13 +1001,8 @@ class Parser $this->expectKeyword('type'); $name = $this->parseName(); $interfaces = $this->parseImplementsInterfaces(); - $directives = $this->parseDirectives(); - - $fields = $this->any( - Token::BRACE_L, - [$this, 'parseFieldDefinition'], - Token::BRACE_R - ); + $directives = $this->parseDirectives(true); + $fields = $this->parseFieldDefinitions(); return new ObjectTypeDefinitionNode([ 'name' => $name, @@ -1007,6 +1029,19 @@ class Parser return $types; } + /** + * @return FieldDefinitionNode[]|NodeList + * @throws SyntaxError + */ + function parseFieldDefinitions() + { + return $this->many( + Token::BRACE_L, + [$this, 'parseFieldDefinition'], + Token::BRACE_R + ); + } + /** * @return FieldDefinitionNode * @throws SyntaxError @@ -1019,7 +1054,7 @@ class Parser $args = $this->parseArgumentDefs(); $this->expect(Token::COLON); $type = $this->parseTypeReference(); - $directives = $this->parseDirectives(); + $directives = $this->parseDirectives(true); return new FieldDefinitionNode([ 'name' => $name, @@ -1057,7 +1092,7 @@ class Parser if ($this->skip(Token::EQUALS)) { $defaultValue = $this->parseConstValue(); } - $directives = $this->parseDirectives(); + $directives = $this->parseDirectives(true); return new InputValueDefinitionNode([ 'name' => $name, 'type' => $type, @@ -1078,12 +1113,8 @@ class Parser $description = $this->parseDescription(); $this->expectKeyword('interface'); $name = $this->parseName(); - $directives = $this->parseDirectives(); - $fields = $this->any( - Token::BRACE_L, - [$this, 'parseFieldDefinition'], - Token::BRACE_R - ); + $directives = $this->parseDirectives(true); + $fields = $this->parseFieldDefinitions(); return new InterfaceTypeDefinitionNode([ 'name' => $name, @@ -1104,7 +1135,7 @@ class Parser $description = $this->parseDescription(); $this->expectKeyword('union'); $name = $this->parseName(); - $directives = $this->parseDirectives(); + $directives = $this->parseDirectives(true); $this->expect(Token::EQUALS); $types = $this->parseUnionMembers(); @@ -1146,7 +1177,7 @@ class Parser $description = $this->parseDescription(); $this->expectKeyword('enum'); $name = $this->parseName(); - $directives = $this->parseDirectives(); + $directives = $this->parseDirectives(true); $values = $this->many( Token::BRACE_L, [$this, 'parseEnumValueDefinition'], @@ -1164,13 +1195,14 @@ class Parser /** * @return EnumValueDefinitionNode + * @throws SyntaxError */ function parseEnumValueDefinition() { $start = $this->lexer->token; $description = $this->parseDescription(); $name = $this->parseName(); - $directives = $this->parseDirectives(); + $directives = $this->parseDirectives(true); return new EnumValueDefinitionNode([ 'name' => $name, @@ -1190,8 +1222,8 @@ class Parser $description = $this->parseDescription(); $this->expectKeyword('input'); $name = $this->parseName(); - $directives = $this->parseDirectives(); - $fields = $this->any( + $directives = $this->parseDirectives(true); + $fields = $this->many( Token::BRACE_L, [$this, 'parseInputValueDef'], Token::BRACE_R @@ -1207,17 +1239,51 @@ class Parser } /** - * @return TypeExtensionDefinitionNode + * @return TypeExtensionNode * @throws SyntaxError */ - function parseTypeExtensionDefinition() + function parseTypeExtension() { + $keywordToken = $this->lexer->lookahead(); + + if ($keywordToken->kind === Token::NAME) { + switch ($keywordToken->value) { + case 'type': + return $this->parseObjectTypeExtension(); + } + } + + throw $this->unexpected($keywordToken); + } + + /** + * @return ObjectTypeExtensionNode + * @throws SyntaxError + */ + function parseObjectTypeExtension() { $start = $this->lexer->token; $this->expectKeyword('extend'); - $definition = $this->parseObjectTypeDefinition(); + $this->expectKeyword('type'); + $name = $this->parseName(); + $interfaces = $this->parseImplementsInterfaces(); + $directives = $this->parseDirectives(true); + $fields = $this->peek(Token::BRACE_L) + ? $this->parseFieldDefinitions() + : []; - return new TypeExtensionDefinitionNode([ - 'definition' => $definition, + if ( + count($interfaces) === 0 && + count($directives) === 0 && + count($fields) === 0 + ) { + throw $this->unexpected(); + } + + return new ObjectTypeExtensionNode([ + 'name' => $name, + 'interfaces' => $interfaces, + 'directives' => $directives, + 'fields' => $fields, 'loc' => $this->loc($start) ]); } @@ -1251,6 +1317,7 @@ class Parser /** * @return NameNode[] + * @throws SyntaxError */ function parseDirectiveLocations() { @@ -1258,8 +1325,23 @@ class Parser $this->skip(Token::PIPE); $locations = []; do { - $locations[] = $this->parseName(); + $locations[] = $this->parseDirectiveLocation(); } while ($this->skip(Token::PIPE)); return $locations; } + + /** + * @return NameNode + * @throws SyntaxError + */ + function parseDirectiveLocation() + { + $start = $this->lexer->token; + $name = $this->parseName(); + if (DirectiveLocation::has($name->value)) { + return $name; + } + + throw $this->unexpected($start); + } } diff --git a/src/Language/Printer.php b/src/Language/Printer.php index 0e5566d..b835b4d 100644 --- a/src/Language/Printer.php +++ b/src/Language/Printer.php @@ -35,7 +35,7 @@ use GraphQL\Language\AST\ScalarTypeDefinitionNode; use GraphQL\Language\AST\SchemaDefinitionNode; use GraphQL\Language\AST\SelectionSetNode; use GraphQL\Language\AST\StringValueNode; -use GraphQL\Language\AST\TypeExtensionDefinitionNode; +use GraphQL\Language\AST\ObjectTypeExtensionNode; use GraphQL\Language\AST\UnionTypeDefinitionNode; use GraphQL\Language\AST\VariableDefinitionNode; use GraphQL\Utils\Utils; @@ -278,8 +278,14 @@ class Printer ], ' ') ], "\n"); }, - NodeKind::TYPE_EXTENSION_DEFINITION => function(TypeExtensionDefinitionNode $def) { - return "extend {$def->definition}"; + NodeKind::OBJECT_TYPE_EXTENSION => function(ObjectTypeExtensionNode $def) { + return $this->join([ + 'extend type', + $def->name, + $this->wrap('implements ', $this->join($def->interfaces, ', ')), + $this->join($def->directives, ' '), + $this->block($def->fields), + ], ' '); }, NodeKind::DIRECTIVE_DEFINITION => function(DirectiveDefinitionNode $def) { return $this->join([ @@ -309,7 +315,7 @@ class Printer { return ($array && $this->length($array)) ? "{\n" . $this->indent($this->join($array, "\n")) . "\n}" - : '{}'; + : ''; } public function indent($maybeString) diff --git a/src/Language/Visitor.php b/src/Language/Visitor.php index 9ddca60..fb149cd 100644 --- a/src/Language/Visitor.php +++ b/src/Language/Visitor.php @@ -142,7 +142,7 @@ class Visitor NodeKind::ENUM_TYPE_DEFINITION => ['description', 'name', 'directives', 'values'], NodeKind::ENUM_VALUE_DEFINITION => ['description', 'name', 'directives'], NodeKind::INPUT_OBJECT_TYPE_DEFINITION => ['description', 'name', 'directives', 'fields'], - NodeKind::TYPE_EXTENSION_DEFINITION => [ 'definition' ], + NodeKind::OBJECT_TYPE_EXTENSION => [ 'name', 'interfaces', 'directives', 'fields' ], NodeKind::DIRECTIVE_DEFINITION => ['description', 'name', 'arguments', 'locations'] ]; diff --git a/src/Type/Definition/Directive.php b/src/Type/Definition/Directive.php index 63ea075..fcf6c2a 100644 --- a/src/Type/Definition/Directive.php +++ b/src/Type/Definition/Directive.php @@ -2,6 +2,7 @@ namespace GraphQL\Type\Definition; use GraphQL\Language\AST\DirectiveDefinitionNode; +use GraphQL\Language\DirectiveLocation; /** * Class Directive @@ -18,35 +19,6 @@ class Directive // Schema Definitions - - /** - * @var array - * @deprecated as of 8.0 (use DirectiveLocation constants directly) - */ - public static $directiveLocations = [ - // Operations: - DirectiveLocation::QUERY => DirectiveLocation::QUERY, - DirectiveLocation::MUTATION => DirectiveLocation::MUTATION, - DirectiveLocation::SUBSCRIPTION => DirectiveLocation::SUBSCRIPTION, - DirectiveLocation::FIELD => DirectiveLocation::FIELD, - DirectiveLocation::FRAGMENT_DEFINITION => DirectiveLocation::FRAGMENT_DEFINITION, - DirectiveLocation::FRAGMENT_SPREAD => DirectiveLocation::FRAGMENT_SPREAD, - DirectiveLocation::INLINE_FRAGMENT => DirectiveLocation::INLINE_FRAGMENT, - - // Schema Definitions - DirectiveLocation::SCHEMA => DirectiveLocation::SCHEMA, - DirectiveLocation::SCALAR => DirectiveLocation::SCALAR, - DirectiveLocation::OBJECT => DirectiveLocation::OBJECT, - DirectiveLocation::FIELD_DEFINITION => DirectiveLocation::FIELD_DEFINITION, - DirectiveLocation::ARGUMENT_DEFINITION => DirectiveLocation::ARGUMENT_DEFINITION, - DirectiveLocation::IFACE => DirectiveLocation::IFACE, - DirectiveLocation::UNION => DirectiveLocation::UNION, - DirectiveLocation::ENUM => DirectiveLocation::ENUM, - DirectiveLocation::ENUM_VALUE => DirectiveLocation::ENUM_VALUE, - DirectiveLocation::INPUT_OBJECT => DirectiveLocation::INPUT_OBJECT, - DirectiveLocation::INPUT_FIELD_DEFINITION => DirectiveLocation::INPUT_FIELD_DEFINITION - ]; - /** * @return Directive */ diff --git a/src/Type/Definition/DirectiveLocation.php b/src/Type/Definition/DirectiveLocation.php deleted file mode 100644 index fee4d7a..0000000 --- a/src/Type/Definition/DirectiveLocation.php +++ /dev/null @@ -1,27 +0,0 @@ - [ 'type' => Type::nonNull(self::_type()), - 'resolve' => function ($field) { + 'resolve' => function (FieldDefinition $field) { return $field->getType(); } ], diff --git a/src/Utils/ASTDefinitionBuilder.php b/src/Utils/ASTDefinitionBuilder.php index ed73dd5..7649796 100644 --- a/src/Utils/ASTDefinitionBuilder.php +++ b/src/Utils/ASTDefinitionBuilder.php @@ -30,7 +30,6 @@ use GraphQL\Type\Definition\NonNull; use GraphQL\Type\Definition\ObjectType; use GraphQL\Type\Definition\Type; use GraphQL\Type\Definition\UnionType; -use GraphQL\Type\Introspection; class ASTDefinitionBuilder { diff --git a/src/Validator/DocumentValidator.php b/src/Validator/DocumentValidator.php index 1750c84..2b6e242 100644 --- a/src/Validator/DocumentValidator.php +++ b/src/Validator/DocumentValidator.php @@ -22,6 +22,7 @@ use GraphQL\Validator\Rules\AbstractValidationRule; use GraphQL\Validator\Rules\ArgumentsOfCorrectType; use GraphQL\Validator\Rules\DefaultValuesOfCorrectType; use GraphQL\Validator\Rules\DisableIntrospection; +use GraphQL\Validator\Rules\ExecutableDefinitions; use GraphQL\Validator\Rules\FieldsOnCorrectType; use GraphQL\Validator\Rules\FragmentsOnCompositeTypes; use GraphQL\Validator\Rules\KnownArgumentNames; @@ -122,6 +123,7 @@ class DocumentValidator { if (null === self::$defaultRules) { self::$defaultRules = [ + ExecutableDefinitions::class => new ExecutableDefinitions(), UniqueOperationNames::class => new UniqueOperationNames(), LoneAnonymousOperation::class => new LoneAnonymousOperation(), KnownTypeNames::class => new KnownTypeNames(), diff --git a/src/Validator/Rules/ExecutableDefinitions.php b/src/Validator/Rules/ExecutableDefinitions.php new file mode 100644 index 0000000..f512d6d --- /dev/null +++ b/src/Validator/Rules/ExecutableDefinitions.php @@ -0,0 +1,47 @@ + function (DocumentNode $node) use ($context) { + /** @var Node $definition */ + foreach ($node->definitions as $definition) { + if ( + !$definition instanceof OperationDefinitionNode && + !$definition instanceof FragmentDefinitionNode + ) { + $context->reportError(new Error( + self::nonExecutableDefinitionMessage($definition->name->value), + [$definition->name] + )); + } + } + + return Visitor::skipNode(); + } + ]; + } +} diff --git a/src/Validator/Rules/KnownDirectives.php b/src/Validator/Rules/KnownDirectives.php index 3043cb6..9d48aa5 100644 --- a/src/Validator/Rules/KnownDirectives.php +++ b/src/Validator/Rules/KnownDirectives.php @@ -5,8 +5,8 @@ use GraphQL\Error\Error; use GraphQL\Language\AST\DirectiveNode; use GraphQL\Language\AST\InputObjectTypeDefinitionNode; use GraphQL\Language\AST\NodeKind; +use GraphQL\Language\DirectiveLocation; use GraphQL\Validator\ValidationContext; -use GraphQL\Type\Definition\DirectiveLocation; class KnownDirectives extends AbstractValidationRule { @@ -37,7 +37,7 @@ class KnownDirectives extends AbstractValidationRule self::unknownDirectiveMessage($node->name->value), [$node] )); - return ; + return; } $candidateLocation = $this->getDirectiveLocationForASTPath($ancestors); @@ -73,7 +73,8 @@ class KnownDirectives extends AbstractValidationRule case NodeKind::FRAGMENT_DEFINITION: return DirectiveLocation::FRAGMENT_DEFINITION; case NodeKind::SCHEMA_DEFINITION: return DirectiveLocation::SCHEMA; case NodeKind::SCALAR_TYPE_DEFINITION: return DirectiveLocation::SCALAR; - case NodeKind::OBJECT_TYPE_DEFINITION: return DirectiveLocation::OBJECT; + case NodeKind::OBJECT_TYPE_DEFINITION: + case NodeKind::OBJECT_TYPE_EXTENSION: return DirectiveLocation::OBJECT; case NodeKind::FIELD_DEFINITION: return DirectiveLocation::FIELD_DEFINITION; case NodeKind::INTERFACE_TYPE_DEFINITION: return DirectiveLocation::IFACE; case NodeKind::UNION_TYPE_DEFINITION: return DirectiveLocation::UNION; diff --git a/tests/Executor/ExecutorTest.php b/tests/Executor/ExecutorTest.php index c38045e..bd6a16d 100644 --- a/tests/Executor/ExecutorTest.php +++ b/tests/Executor/ExecutorTest.php @@ -965,14 +965,14 @@ class ExecutorTest extends \PHPUnit_Framework_TestCase } /** - * @it fails to execute a query containing a type definition + * @it executes ignoring invalid non-executable definitions */ - public function testFailsToExecuteQueryContainingTypeDefinition() + public function testExecutesIgnoringInvalidNonExecutableDefinitions() { $query = Parser::parse(' { foo } - type Query { foo: String } + type Query { bar: String } '); $schema = new Schema([ @@ -988,12 +988,9 @@ class ExecutorTest extends \PHPUnit_Framework_TestCase $result = Executor::execute($schema, $query); $expected = [ - 'errors' => [ - [ - 'message' => 'GraphQL cannot execute a request containing a ObjectTypeDefinition.', - 'locations' => [['line' => 4, 'column' => 7]], - ] - ] + 'data' => [ + 'foo' => null, + ], ]; $this->assertArraySubset($expected, $result->toArray()); diff --git a/tests/Language/SchemaParserTest.php b/tests/Language/SchemaParserTest.php index 81d8a3f..77e2e92 100644 --- a/tests/Language/SchemaParserTest.php +++ b/tests/Language/SchemaParserTest.php @@ -150,21 +150,16 @@ extend type Hello { 'kind' => NodeKind::DOCUMENT, 'definitions' => [ [ - 'kind' => NodeKind::TYPE_EXTENSION_DEFINITION, - 'definition' => [ - 'kind' => NodeKind::OBJECT_TYPE_DEFINITION, - 'name' => $this->nameNode('Hello', $loc(13, 18)), - 'interfaces' => [], - 'directives' => [], - 'fields' => [ - $this->fieldNode( - $this->nameNode('world', $loc(23, 28)), - $this->typeNode('String', $loc(30, 36)), - $loc(23, 36) - ) - ], - 'loc' => $loc(8, 38), - 'description' => null + 'kind' => NodeKind::OBJECT_TYPE_EXTENSION, + 'name' => $this->nameNode('Hello', $loc(13, 18)), + 'interfaces' => [], + 'directives' => [], + 'fields' => [ + $this->fieldNode( + $this->nameNode('world', $loc(23, 28)), + $this->typeNode('String', $loc(30, 36)), + $loc(23, 36) + ) ], 'loc' => $loc(1, 38) ] @@ -174,16 +169,59 @@ extend type Hello { $this->assertEquals($expected, TestUtils::nodeToArray($doc)); } + /** + * @it Extension without fields + */ + public function testExtensionWithoutFields() + { + $body = 'extend type Hello implements Greeting'; + $doc = Parser::parse($body); + $loc = function($start, $end) { + return TestUtils::locArray($start, $end); + }; + $expected = [ + 'kind' => NodeKind::DOCUMENT, + 'definitions' => [ + [ + 'kind' => NodeKind::OBJECT_TYPE_EXTENSION, + 'name' => $this->nameNode('Hello', $loc(12, 17)), + 'interfaces' => [ + $this->typeNode('Greeting', $loc(29, 37)), + ], + 'directives' => [], + 'fields' => [], + 'loc' => $loc(0, 37) + ] + ], + 'loc' => $loc(0, 37) + ]; + $this->assertEquals($expected, TestUtils::nodeToArray($doc)); + } + /** * @it Extension do not include descriptions * @expectedException \GraphQL\Error\SyntaxError - * @expectedExceptionMessage Syntax Error GraphQL (2:1) + * @expectedExceptionMessage Syntax Error GraphQL (3:7) */ public function testExtensionDoNotIncludeDescriptions() { $body = ' -"Description" -extend type Hello { - world: String + "Description" + extend type Hello { + world: String + }'; + Parser::parse($body); + } + + /** + * @it Extension do not include descriptions + * @expectedException \GraphQL\Error\SyntaxError + * @expectedExceptionMessage Syntax Error GraphQL (2:14) + */ + public function testExtensionDoNotIncludeDescriptions2() { + $body = ' + extend "Description" type Hello { + world: String + } }'; Parser::parse($body); } @@ -236,7 +274,7 @@ type Hello { */ public function testSimpleTypeInheritingInterface() { - $body = 'type Hello implements World { }'; + $body = 'type Hello implements World { field: String }'; $loc = function($start, $end) { return TestUtils::locArray($start, $end); }; $doc = Parser::parse($body); @@ -250,12 +288,18 @@ type Hello { $this->typeNode('World', $loc(22, 27)) ], 'directives' => [], - 'fields' => [], - 'loc' => $loc(0,31), + 'fields' => [ + $this->fieldNode( + $this->nameNode('field', $loc(30, 35)), + $this->typeNode('String', $loc(37, 43)), + $loc(30, 43) + ) + ], + 'loc' => $loc(0, 45), 'description' => null ] ], - 'loc' => $loc(0,31) + 'loc' => $loc(0, 45) ]; $this->assertEquals($expected, TestUtils::nodeToArray($doc)); @@ -266,7 +310,7 @@ type Hello { */ public function testSimpleTypeInheritingMultipleInterfaces() { - $body = 'type Hello implements Wo, rld { }'; + $body = 'type Hello implements Wo, rld { field: String }'; $loc = function($start, $end) {return TestUtils::locArray($start, $end);}; $doc = Parser::parse($body); @@ -281,12 +325,18 @@ type Hello { $this->typeNode('rld', $loc(26,29)) ], 'directives' => [], - 'fields' => [], - 'loc' => $loc(0, 33), + 'fields' => [ + $this->fieldNode( + $this->nameNode('field', $loc(32, 37)), + $this->typeNode('String', $loc(39, 45)), + $loc(32, 45) + ) + ], + 'loc' => $loc(0, 47), 'description' => null ] ], - 'loc' => $loc(0, 33) + 'loc' => $loc(0, 47) ]; $this->assertEquals($expected, TestUtils::nodeToArray($doc)); @@ -754,6 +804,7 @@ input Hello { /** * @it Simple input object with args should fail + * @expectedException \GraphQL\Error\SyntaxError */ public function testSimpleInputObjectWithArgsShouldFail() { @@ -761,7 +812,19 @@ input Hello { input Hello { world(foo: Int): String }'; - $this->setExpectedException('GraphQL\Error\SyntaxError'); + Parser::parse($body); + } + + /** + * @it Directive with incorrect locations + * @expectedException \GraphQL\Error\SyntaxError + * @expectedExceptionMessage Syntax Error GraphQL (2:33) Unexpected Name "INCORRECT_LOCATION" + */ + public function testDirectiveWithIncorrectLocationShouldFail() + { + $body = ' + directive @foo on FIELD | INCORRECT_LOCATION +'; Parser::parse($body); } diff --git a/tests/Language/SchemaPrinterTest.php b/tests/Language/SchemaPrinterTest.php index 3b9a098..3141f06 100644 --- a/tests/Language/SchemaPrinterTest.php +++ b/tests/Language/SchemaPrinterTest.php @@ -116,9 +116,7 @@ extend type Foo { seven(argument: [String]): Type } -extend type Foo @onType {} - -type NoFields {} +extend type Foo @onType directive @skip(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT diff --git a/tests/Language/schema-kitchen-sink.graphql b/tests/Language/schema-kitchen-sink.graphql index 4b3fbaa..016707d 100644 --- a/tests/Language/schema-kitchen-sink.graphql +++ b/tests/Language/schema-kitchen-sink.graphql @@ -68,9 +68,7 @@ extend type Foo { seven(argument: [String]): Type } -extend type Foo @onType {} - -type NoFields {} +extend type Foo @onType directive @skip(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT diff --git a/tests/Utils/BuildSchemaTest.php b/tests/Utils/BuildSchemaTest.php index b77220a..6c4eaa6 100644 --- a/tests/Utils/BuildSchemaTest.php +++ b/tests/Utils/BuildSchemaTest.php @@ -1025,7 +1025,9 @@ schema { query: Hello } -type Hello implements Bar { } +type Hello implements Bar { + field: String +} '; $doc = Parser::parse($body); $schema = BuildSchema::buildAST($doc); diff --git a/tests/Validator/ExecutableDefinitionsTest.php b/tests/Validator/ExecutableDefinitionsTest.php new file mode 100644 index 0000000..2b9eda3 --- /dev/null +++ b/tests/Validator/ExecutableDefinitionsTest.php @@ -0,0 +1,79 @@ +expectPassesRule(new ExecutableDefinitions, ' + query Foo { + dog { + name + } + } + '); + } + + /** + * @it with operation and fragment + */ + public function testWithOperationAndFragment() + { + $this->expectPassesRule(new ExecutableDefinitions, ' + query Foo { + dog { + name + ...Frag + } + } + + fragment Frag on Dog { + name + } + '); + } + + /** + * @it with typeDefinition + */ + public function testWithTypeDefinition() + { + $this->expectFailsRule(new ExecutableDefinitions, ' + query Foo { + dog { + name + } + } + + type Cow { + name: String + } + + extend type Dog { + color: String + } + ', + [ + $this->nonExecutableDefinition('Cow', 8, 12), + $this->nonExecutableDefinition('Dog', 12, 19), + ]); + } + + private function nonExecutableDefinition($defName, $line, $column) + { + return FormattedError::create( + ExecutableDefinitions::nonExecutableDefinitionMessage($defName), + [ new SourceLocation($line, $column) ] + ); + } +} diff --git a/tests/Validator/KnownDirectivesTest.php b/tests/Validator/KnownDirectivesTest.php index d581d1c..3e7998e 100644 --- a/tests/Validator/KnownDirectivesTest.php +++ b/tests/Validator/KnownDirectivesTest.php @@ -136,6 +136,8 @@ class KnownDirectivesTest extends TestCase myField(myArg: Int @onArgumentDefinition): String @onFieldDefinition } + extend type MyObj @onObject + scalar MyScalar @onScalar interface MyInterface @onInterface { diff --git a/tests/Validator/TestCase.php b/tests/Validator/TestCase.php index 4277bfc..028ac24 100644 --- a/tests/Validator/TestCase.php +++ b/tests/Validator/TestCase.php @@ -3,6 +3,7 @@ namespace GraphQL\Tests\Validator; use GraphQL\Language\Parser; use GraphQL\Type\Schema; +use GraphQL\Type\Definition\CustomScalarType; use GraphQL\Type\Definition\Directive; use GraphQL\Type\Definition\EnumType; use GraphQL\Type\Definition\InputObjectType; @@ -259,6 +260,24 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase ] ]); + $invalidScalar = new CustomScalarType([ + 'name' => 'Invalid', + 'serialize' => function ($value) { return $value; }, + 'parseLiteral' => function ($node) { + throw new \Exception('Invalid scalar is always invalid: ' . $node->value); + }, + 'parseValue' => function ($value) { + throw new \Exception('Invalid scalar is always invalid: ' . $value); + }, + ]); + + $anyScalar = new CustomScalarType([ + 'name' => 'Any', + 'serialize' => function ($value) { return $value; }, + 'parseLiteral' => function ($node) { return $node; }, // Allows any value + 'parseValue' => function ($value) { return $value; }, // Allows any value + ]); + $queryRoot = new ObjectType([ 'name' => 'QueryRoot', 'fields' => [ @@ -274,6 +293,14 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase 'dogOrHuman' => ['type' => $DogOrHuman], 'humanOrAlien' => ['type' => $HumanOrAlien], 'complicatedArgs' => ['type' => $ComplicatedArgs], + 'invalidArg' => [ + 'args' => ['arg' => ['type' => $invalidScalar]], + 'type' => Type::string(), + ], + 'anyArg' => [ + 'args' => ['arg' => ['type' => $anyScalar]], + 'type' => Type::string(), + ], ] ]); diff --git a/tools/gendocs.php b/tools/gendocs.php index 7a2d16c..95b5b96 100644 --- a/tools/gendocs.php +++ b/tools/gendocs.php @@ -9,7 +9,7 @@ $entries = [ \GraphQL\GraphQL::class, \GraphQL\Type\Definition\Type::class, \GraphQL\Type\Definition\ResolveInfo::class, - \GraphQL\Type\Definition\DirectiveLocation::class => ['constants' => true], + \GraphQL\Language\DirectiveLocation::class => ['constants' => true], \GraphQL\Type\SchemaConfig::class, \GraphQL\Type\Schema::class, \GraphQL\Language\Parser::class,