From 91b55bb221b0e9c02a4a6eed3da54d23a2e511ba Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Mon, 8 Jun 2020 19:46:32 +0200 Subject: [PATCH] Support repeatable directives (#643) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Support repeatable directives See https://github.com/graphql/graphql-js/pull/1965 * Fix introspection See https://github.com/graphql/graphql-js/pull/2416 * Fix codestyle * Canonical ordering * Update baseline * Make DirectiveTest.php final Co-Authored-By: Šimon Podlipský * Fix baseline Co-authored-by: Šimon Podlipský --- phpstan-baseline.neon | 5 -- src/Language/AST/DirectiveDefinitionNode.php | 9 +- src/Language/Parser.php | 57 ++++++------ src/Language/Printer.php | 1 + src/Type/Definition/Directive.php | 30 +++++-- src/Type/Introspection.php | 86 +++++++++++-------- src/Utils/ASTDefinitionBuilder.php | 3 +- src/Utils/BuildClientSchema.php | 3 +- src/Utils/SchemaPrinter.php | 70 ++++++++------- .../Rules/UniqueDirectivesPerLocation.php | 35 +++++++- tests/Language/SchemaParserTest.php | 81 +++++++++++++++++ tests/Language/SchemaPrinterTest.php | 2 + tests/Language/schema-kitchen-sink.graphql | 4 + tests/Type/DirectiveTest.php | 15 ++++ tests/Type/IntrospectionTest.php | 70 ++++++++++----- tests/Utils/BuildClientSchemaTest.php | 10 ++- tests/Utils/BuildSchemaTest.php | 2 + tests/Utils/SchemaExtenderTest.php | 4 +- tests/Utils/SchemaPrinterTest.php | 40 +++++++-- .../UniqueDirectivesPerLocationTest.php | 74 ++++++++++------ tests/Validator/ValidatorTestCase.php | 29 +++++-- 21 files changed, 446 insertions(+), 184 deletions(-) create mode 100644 tests/Type/DirectiveTest.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index f57b1c63d..e47bdc0d7 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -265,11 +265,6 @@ parameters: count: 2 path: src/Server/OperationParams.php - - - message: "#^Variable property access on \\$this\\(GraphQL\\\\Type\\\\Definition\\\\Directive\\)\\.$#" - count: 1 - path: src/Type/Definition/Directive.php - - message: "#^Only booleans are allowed in a negated boolean, ArrayObject\\ given\\.$#" count: 1 diff --git a/src/Language/AST/DirectiveDefinitionNode.php b/src/Language/AST/DirectiveDefinitionNode.php index 825c7f789..26aa23e3c 100644 --- a/src/Language/AST/DirectiveDefinitionNode.php +++ b/src/Language/AST/DirectiveDefinitionNode.php @@ -12,12 +12,15 @@ class DirectiveDefinitionNode extends Node implements TypeSystemDefinitionNode /** @var NameNode */ public $name; + /** @var StringValueNode|null */ + public $description; + /** @var ArgumentNode[] */ public $arguments; + /** @var bool */ + public $repeatable; + /** @var NameNode[] */ public $locations; - - /** @var StringValueNode|null */ - public $description; } diff --git a/src/Language/Parser.php b/src/Language/Parser.php index 1050ecd2c..d18a0f93d 100644 --- a/src/Language/Parser.php +++ b/src/Language/Parser.php @@ -327,26 +327,39 @@ private function expect(string $kind) : Token } /** - * If the next token is a keyword with the given value, return that token after - * advancing the parser. Otherwise, do not change the parser state and return - * false. + * If the next token is a keyword with the given value, advance the lexer. + * Otherwise, throw an error. * * @throws SyntaxError */ - private function expectKeyword(string $value) : Token + private function expectKeyword(string $value) : void { $token = $this->lexer->token; + if ($token->kind !== Token::NAME || $token->value !== $value) { + throw new SyntaxError( + $this->lexer->source, + $token->start, + 'Expected "' . $value . '", found ' . $token->getDescription() + ); + } + + $this->lexer->advance(); + } + /** + * If the next token is a given keyword, return "true" after advancing + * the lexer. Otherwise, do not change the parser state and return "false". + */ + private function expectOptionalKeyword(string $value) : bool + { + $token = $this->lexer->token; if ($token->kind === Token::NAME && $token->value === $value) { $this->lexer->advance(); - return $token; + return true; } - throw new SyntaxError( - $this->lexer->source, - $token->start, - 'Expected "' . $value . '", found ' . $token->getDescription() - ); + + return false; } private function unexpected(?Token $atToken = null) : SyntaxError @@ -716,7 +729,8 @@ private function parseFragment() : SelectionNode $start = $this->lexer->token; $this->expect(Token::SPREAD); - if ($this->peek(Token::NAME) && $this->lexer->token->value !== 'on') { + $hasTypeCondition = $this->expectOptionalKeyword('on'); + if (! $hasTypeCondition && $this->peek(Token::NAME)) { return new FragmentSpreadNode([ 'name' => $this->parseFragmentName(), 'directives' => $this->parseDirectives(false), @@ -724,14 +738,8 @@ private function parseFragment() : SelectionNode ]); } - $typeCondition = null; - if ($this->lexer->token->value === 'on') { - $this->lexer->advance(); - $typeCondition = $this->parseNamedType(); - } - return new InlineFragmentNode([ - 'typeCondition' => $typeCondition, + 'typeCondition' => $hasTypeCondition ? $this->parseNamedType() : null, 'directives' => $this->parseDirectives(false), 'selectionSet' => $this->parseSelectionSet(), 'loc' => $this->loc($start), @@ -1172,8 +1180,7 @@ private function parseObjectTypeDefinition() : ObjectTypeDefinitionNode private function parseImplementsInterfaces() : array { $types = []; - if ($this->lexer->token->value === 'implements') { - $this->lexer->advance(); + if ($this->expectOptionalKeyword('implements')) { // Optional leading ampersand $this->skip(Token::AMP); do { @@ -1668,7 +1675,7 @@ private function parseInputObjectTypeExtension() : InputObjectTypeExtensionNode /** * DirectiveDefinition : - * - directive @ Name ArgumentsDefinition? on DirectiveLocations + * - Description? directive @ Name ArgumentsDefinition? `repeatable`? on DirectiveLocations * * @throws SyntaxError */ @@ -1678,17 +1685,19 @@ private function parseDirectiveDefinition() : DirectiveDefinitionNode $description = $this->parseDescription(); $this->expectKeyword('directive'); $this->expect(Token::AT); - $name = $this->parseName(); - $args = $this->parseArgumentsDefinition(); + $name = $this->parseName(); + $args = $this->parseArgumentsDefinition(); + $repeatable = $this->expectOptionalKeyword('repeatable'); $this->expectKeyword('on'); $locations = $this->parseDirectiveLocations(); return new DirectiveDefinitionNode([ 'name' => $name, + 'description' => $description, 'arguments' => $args, + 'repeatable' => $repeatable, 'locations' => $locations, 'loc' => $this->loc($start), - 'description' => $description, ]); } diff --git a/src/Language/Printer.php b/src/Language/Printer.php index 7aa13b748..8aa27d235 100644 --- a/src/Language/Printer.php +++ b/src/Language/Printer.php @@ -446,6 +446,7 @@ function (InterfaceTypeDefinitionNode $def) { . ($noIndent ? $this->wrap('(', $this->join($def->arguments, ', '), ')') : $this->wrap("(\n", $this->indent($this->join($def->arguments, "\n")), "\n")) + . ($def->repeatable ? ' repeatable' : '') . ' on ' . $this->join($def->locations, ' | '); }), ], diff --git a/src/Type/Definition/Directive.php b/src/Type/Definition/Directive.php index 75bc03407..c27025c50 100644 --- a/src/Type/Definition/Directive.php +++ b/src/Type/Definition/Directive.php @@ -4,9 +4,9 @@ namespace GraphQL\Type\Definition; +use GraphQL\Error\InvariantViolation; use GraphQL\Language\AST\DirectiveDefinitionNode; use GraphQL\Language\DirectiveLocation; -use GraphQL\Utils\Utils; use function array_key_exists; use function is_array; @@ -31,12 +31,15 @@ class Directive /** @var string|null */ public $description; - /** @var string[] */ - public $locations; - /** @var FieldArgument[] */ public $args = []; + /** @var bool */ + public $isRepeatable; + + /** @var string[] */ + public $locations; + /** @var DirectiveDefinitionNode|null */ public $astNode; @@ -48,6 +51,13 @@ class Directive */ public function __construct(array $config) { + if (! isset($config['name'])) { + throw new InvariantViolation('Directive must be named.'); + } + $this->name = $config['name']; + + $this->description = $config['description'] ?? null; + if (isset($config['args'])) { $args = []; foreach ($config['args'] as $name => $arg) { @@ -58,14 +68,16 @@ public function __construct(array $config) } } $this->args = $args; - unset($config['args']); } - foreach ($config as $key => $value) { - $this->{$key} = $value; + + if (! isset($config['locations']) || ! is_array($config['locations'])) { + throw new InvariantViolation('Must provide locations for directive.'); } + $this->locations = $config['locations']; + + $this->isRepeatable = $config['isRepeatable'] ?? false; + $this->astNode = $config['astNode'] ?? null; - Utils::invariant($this->name, 'Directive must be named.'); - Utils::invariant(is_array($this->locations), 'Must provide locations for directive.'); $this->config = $config; } diff --git a/src/Type/Introspection.php b/src/Type/Introspection.php index 0178a13ec..7f0b0f582 100644 --- a/src/Type/Introspection.php +++ b/src/Type/Introspection.php @@ -27,6 +27,7 @@ use GraphQL\Utils\Utils; use function array_filter; use function array_key_exists; +use function array_merge; use function array_values; use function is_bool; use function method_exists; @@ -43,28 +44,28 @@ class Introspection private static $map = []; /** - * Options: - * - descriptions - * Whether to include descriptions in the introspection result. - * Default: true - * - * @param bool[]|bool $options + * @param array $options + * Available options: + * - descriptions + * Whether to include descriptions in the introspection result. + * Default: true + * - directiveIsRepeatable + * Whether to include `isRepeatable` flag on directives. + * Default: false * * @return string + * + * @api */ - public static function getIntrospectionQuery($options = []) + public static function getIntrospectionQuery(array $options = []) { - if (is_bool($options)) { - trigger_error( - 'Calling Introspection::getIntrospectionQuery(boolean) is deprecated. ' . - 'Please use Introspection::getIntrospectionQuery(["descriptions" => boolean]).', - E_USER_DEPRECATED - ); - $descriptions = $options; - } else { - $descriptions = ! array_key_exists('descriptions', $options) || $options['descriptions'] === true; - } - $descriptionField = $descriptions ? 'description' : ''; + $optionsWithDefaults = array_merge([ + 'descriptions' => true, + 'directiveIsRepeatable' => false, + ], $options); + + $descriptions = $optionsWithDefaults['descriptions'] ? 'description' : ''; + $directiveIsRepeatable = $optionsWithDefaults['directiveIsRepeatable'] ? 'isRepeatable' : ''; return << $options + * Available options: + * - descriptions + * Whether to include `isRepeatable` flag on directives. + * Default: true + * - directiveIsRepeatable + * Whether to include descriptions in the introspection result. + * Default: true * * @return array>|null + * + * @api */ public static function fromSchema(Schema $schema, array $options = []) : ?array { + $optionsWithDefaults = array_merge(['directiveIsRepeatable' => true], $options); + $result = GraphQL::executeQuery( $schema, - self::getIntrospectionQuery($options) + self::getIntrospectionQuery($optionsWithDefaults) ); return $result->data; @@ -651,6 +659,18 @@ public static function _directive() return $obj->description; }, ], + 'args' => [ + 'type' => Type::nonNull(Type::listOf(Type::nonNull(self::_inputValue()))), + 'resolve' => static function (Directive $directive) { + return $directive->args ?? []; + }, + ], + 'isRepeatable' => [ + 'type' => Type::nonNull(Type::boolean()), + 'resolve' => static function (Directive $directive) : bool { + return $directive->isRepeatable; + }, + ], 'locations' => [ 'type' => Type::nonNull(Type::listOf(Type::nonNull( self::_directiveLocation() @@ -659,12 +679,6 @@ public static function _directive() return $obj->locations; }, ], - 'args' => [ - 'type' => Type::nonNull(Type::listOf(Type::nonNull(self::_inputValue()))), - 'resolve' => static function (Directive $directive) { - return $directive->args ?? []; - }, - ], ], ]); } diff --git a/src/Utils/ASTDefinitionBuilder.php b/src/Utils/ASTDefinitionBuilder.php index c032442ed..05a3b5ad5 100644 --- a/src/Utils/ASTDefinitionBuilder.php +++ b/src/Utils/ASTDefinitionBuilder.php @@ -79,13 +79,14 @@ public function buildDirective(DirectiveDefinitionNode $directiveNode) return new Directive([ 'name' => $directiveNode->name->value, 'description' => $this->getDescription($directiveNode), + 'args' => $directiveNode->arguments ? FieldArgument::createMap($this->makeInputValues($directiveNode->arguments)) : null, + 'isRepeatable' => $directiveNode->repeatable, 'locations' => Utils::map( $directiveNode->locations, static function ($node) { return $node->value; } ), - 'args' => $directiveNode->arguments ? FieldArgument::createMap($this->makeInputValues($directiveNode->arguments)) : null, 'astNode' => $directiveNode, ]); } diff --git a/src/Utils/BuildClientSchema.php b/src/Utils/BuildClientSchema.php index 57cef3284..a013c1ac1 100644 --- a/src/Utils/BuildClientSchema.php +++ b/src/Utils/BuildClientSchema.php @@ -472,8 +472,9 @@ public function buildDirective(array $directive) : Directive return new Directive([ 'name' => $directive['name'], 'description' => $directive['description'], - 'locations' => $directive['locations'], 'args' => $this->buildInputValueDefMap($directive['args']), + 'isRepeatable' => $directive['isRepeatable'], + 'locations' => $directive['locations'], ]); } } diff --git a/src/Utils/SchemaPrinter.php b/src/Utils/SchemaPrinter.php index 795c4cc2d..cc68d26c8 100644 --- a/src/Utils/SchemaPrinter.php +++ b/src/Utils/SchemaPrinter.php @@ -38,13 +38,11 @@ class SchemaPrinter { /** - * Accepts options as a second argument: - * + * @param array $options + * Available options: * - commentDescriptions: * Provide true to use preceding comments as the description. * - * @param bool[] $options - * * @api */ public static function doPrint(Schema $schema, array $options = []) : string @@ -62,16 +60,11 @@ static function ($type) : bool { } /** - * @param bool[] $options + * @param array $options */ - private static function printFilteredSchema(Schema $schema, $directiveFilter, $typeFilter, $options) : string + private static function printFilteredSchema(Schema $schema, callable $directiveFilter, callable $typeFilter, array $options) : string { - $directives = array_filter( - $schema->getDirectives(), - static function ($directive) use ($directiveFilter) { - return $directiveFilter($directive); - } - ); + $directives = array_filter($schema->getDirectives(), $directiveFilter); $types = $schema->getTypeMap(); ksort($types); @@ -85,7 +78,7 @@ static function ($directive) use ($directiveFilter) { array_merge( [self::printSchemaDefinition($schema)], array_map( - static function ($directive) use ($options) : string { + static function (Directive $directive) use ($options) : string { return self::printDirective($directive, $options); }, $directives @@ -157,14 +150,22 @@ private static function isSchemaOfCommonNames(Schema $schema) : bool return $subscriptionType === null || $subscriptionType->name === 'Subscription'; } - private static function printDirective($directive, $options) : string + /** + * @param array $options + */ + private static function printDirective(Directive $directive, array $options) : string { - return self::printDescription($options, $directive) . - 'directive @' . $directive->name . self::printArgs($options, $directive->args) . - ' on ' . implode(' | ', $directive->locations); + return self::printDescription($options, $directive) + . 'directive @' . $directive->name + . self::printArgs($options, $directive->args) + . ($directive->isRepeatable ? ' repeatable' : '') + . ' on ' . implode(' | ', $directive->locations); } - private static function printDescription($options, $def, $indentation = '', $firstInBlock = true) : string + /** + * @param array $options + */ + private static function printDescription(array $options, $def, $indentation = '', $firstInBlock = true) : string { if (! $def->description) { return ''; @@ -264,7 +265,10 @@ private static function escapeQuote($line) : string return str_replace('"""', '\\"""', $line); } - private static function printArgs($options, $args, $indentation = '') : string + /** + * @param array $options + */ + private static function printArgs(array $options, $args, $indentation = '') : string { if (! $args) { return ''; @@ -308,7 +312,7 @@ private static function printInputValue($arg) : string } /** - * @param bool[] $options + * @param array $options */ public static function printType(Type $type, array $options = []) : string { @@ -340,7 +344,7 @@ public static function printType(Type $type, array $options = []) : string } /** - * @param bool[] $options + * @param array $options */ private static function printScalar(ScalarType $type, array $options) : string { @@ -348,7 +352,7 @@ private static function printScalar(ScalarType $type, array $options) : string } /** - * @param bool[] $options + * @param array $options */ private static function printObject(ObjectType $type, array $options) : string { @@ -357,8 +361,8 @@ private static function printObject(ObjectType $type, array $options) : string ? ' implements ' . implode( ' & ', array_map( - static function ($i) : string { - return $i->name; + static function (InterfaceType $interface) : string { + return $interface->name; }, $interfaces ) @@ -370,9 +374,9 @@ static function ($i) : string { } /** - * @param bool[] $options + * @param array $options */ - private static function printFields($options, $type) : string + private static function printFields(array $options, $type) : string { $fields = array_values($type->getFields()); @@ -405,7 +409,7 @@ private static function printDeprecated($fieldOrEnumVal) : string } /** - * @param bool[] $options + * @param array $options */ private static function printInterface(InterfaceType $type, array $options) : string { @@ -414,7 +418,7 @@ private static function printInterface(InterfaceType $type, array $options) : st } /** - * @param bool[] $options + * @param array $options */ private static function printUnion(UnionType $type, array $options) : string { @@ -423,7 +427,7 @@ private static function printUnion(UnionType $type, array $options) : string } /** - * @param bool[] $options + * @param array $options */ private static function printEnum(EnumType $type, array $options) : string { @@ -432,9 +436,9 @@ private static function printEnum(EnumType $type, array $options) : string } /** - * @param bool[] $options + * @param array $options */ - private static function printEnumValues($values, $options) : string + private static function printEnumValues($values, array $options) : string { return implode( "\n", @@ -450,7 +454,7 @@ static function ($value, $i) use ($options) : string { } /** - * @param bool[] $options + * @param array $options */ private static function printInputObject(InputObjectType $type, array $options) : string { @@ -474,7 +478,7 @@ static function ($f, $i) use ($options) : string { } /** - * @param bool[] $options + * @param array $options * * @api */ diff --git a/src/Validator/Rules/UniqueDirectivesPerLocation.php b/src/Validator/Rules/UniqueDirectivesPerLocation.php index 6860222d5..02844af73 100644 --- a/src/Validator/Rules/UniqueDirectivesPerLocation.php +++ b/src/Validator/Rules/UniqueDirectivesPerLocation.php @@ -5,13 +5,21 @@ namespace GraphQL\Validator\Rules; use GraphQL\Error\Error; +use GraphQL\Language\AST\DirectiveDefinitionNode; use GraphQL\Language\AST\DirectiveNode; use GraphQL\Language\AST\Node; +use GraphQL\Type\Definition\Directive; use GraphQL\Validator\ASTValidationContext; use GraphQL\Validator\SDLValidationContext; use GraphQL\Validator\ValidationContext; use function sprintf; +/** + * Unique directive names per location + * + * A GraphQL document is only valid if all non-repeatable directives at + * a given location are uniquely named. + */ class UniqueDirectivesPerLocation extends ValidationRule { public function getVisitor(ValidationContext $context) @@ -26,16 +34,41 @@ public function getSDLVisitor(SDLValidationContext $context) public function getASTVisitor(ASTValidationContext $context) { + $uniqueDirectiveMap = []; + + $schema = $context->getSchema(); + $definedDirectives = $schema !== null + ? $schema->getDirectives() + : Directive::getInternalDirectives(); + foreach ($definedDirectives as $directive) { + $uniqueDirectiveMap[$directive->name] = ! $directive->isRepeatable; + } + + $astDefinitions = $context->getDocument()->definitions; + foreach ($astDefinitions as $definition) { + if (! ($definition instanceof DirectiveDefinitionNode)) { + continue; + } + + $uniqueDirectiveMap[$definition->name->value] = $definition->repeatable; + } + return [ - 'enter' => static function (Node $node) use ($context) : void { + 'enter' => static function (Node $node) use ($uniqueDirectiveMap, $context) : void { if (! isset($node->directives)) { return; } $knownDirectives = []; + /** @var DirectiveNode $directive */ foreach ($node->directives as $directive) { $directiveName = $directive->name->value; + + if (! isset($uniqueDirectiveMap[$directiveName])) { + continue; + } + if (isset($knownDirectives[$directiveName])) { $context->reportError(new Error( self::duplicateDirectiveMessage($directiveName), diff --git a/tests/Language/SchemaParserTest.php b/tests/Language/SchemaParserTest.php index 267db2246..cea96e12b 100644 --- a/tests/Language/SchemaParserTest.php +++ b/tests/Language/SchemaParserTest.php @@ -6,6 +6,7 @@ use GraphQL\Error\SyntaxError; use GraphQL\Language\AST\NodeKind; +use GraphQL\Language\DirectiveLocation; use GraphQL\Language\Parser; use GraphQL\Language\SourceLocation; use GraphQL\Tests\PHPUnit\ArraySubsetAsserts; @@ -1045,6 +1046,86 @@ public function testSimpleInputObjectWithArgsShouldFail() : void ); } + /** + * @see it('Directive definition', () => { + */ + public function testDirectiveDefinition() : void + { + $body = 'directive @foo on OBJECT | INTERFACE'; + $doc = Parser::parse($body); + $loc = static function ($start, $end) : array { + return TestUtils::locArray($start, $end); + }; + + $expected = [ + 'kind' => NodeKind::DOCUMENT, + 'definitions' => [ + [ + 'kind' => NodeKind::DIRECTIVE_DEFINITION, + 'name' => $this->nameNode('foo', $loc(11, 14)), + 'description' => null, + 'arguments' => [], + 'repeatable' => false, + 'locations' => [ + [ + 'kind' => NodeKind::NAME, + 'value' => DirectiveLocation::OBJECT, + 'loc' => $loc(18, 24), + ], + [ + 'kind' => NodeKind::NAME, + 'value' => DirectiveLocation::IFACE, + 'loc' => $loc(27, 36), + ], + ], + 'loc' => $loc(0, 36), + ], + ], + 'loc' => $loc(0, 36), + ]; + self::assertEquals($expected, TestUtils::nodeToArray($doc)); + } + + /** + * @see it('Repeatable directive definition', () => { + */ + public function testRepeatableDirectiveDefinition() : void + { + $body = 'directive @foo repeatable on OBJECT | INTERFACE'; + $doc = Parser::parse($body); + $loc = static function ($start, $end) : array { + return TestUtils::locArray($start, $end); + }; + + $expected = [ + 'kind' => NodeKind::DOCUMENT, + 'definitions' => [ + [ + 'kind' => NodeKind::DIRECTIVE_DEFINITION, + 'name' => $this->nameNode('foo', $loc(11, 14)), + 'description' => null, + 'arguments' => [], + 'repeatable' => true, + 'locations' => [ + [ + 'kind' => NodeKind::NAME, + 'value' => DirectiveLocation::OBJECT, + 'loc' => $loc(29, 35), + ], + [ + 'kind' => NodeKind::NAME, + 'value' => DirectiveLocation::IFACE, + 'loc' => $loc(38, 47), + ], + ], + 'loc' => $loc(0, 47), + ], + ], + 'loc' => $loc(0, 47), + ]; + self::assertEquals($expected, TestUtils::nodeToArray($doc)); + } + /** * @see it('Directive with incorrect locations') */ diff --git a/tests/Language/SchemaPrinterTest.php b/tests/Language/SchemaPrinterTest.php index b5769835a..05a326189 100644 --- a/tests/Language/SchemaPrinterTest.php +++ b/tests/Language/SchemaPrinterTest.php @@ -176,6 +176,8 @@ enum UndefinedEnum directive @include(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT directive @include2(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT + +directive @myRepeatableDir(name: String!) repeatable on OBJECT | INTERFACE '; self::assertEquals($expected, $printed); } diff --git a/tests/Language/schema-kitchen-sink.graphql b/tests/Language/schema-kitchen-sink.graphql index 3ad830cbf..b912f06d5 100644 --- a/tests/Language/schema-kitchen-sink.graphql +++ b/tests/Language/schema-kitchen-sink.graphql @@ -123,3 +123,7 @@ directive @include2(if: Boolean!) on | FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT + +directive @myRepeatableDir(name: String!) repeatable on + | OBJECT + | INTERFACE diff --git a/tests/Type/DirectiveTest.php b/tests/Type/DirectiveTest.php new file mode 100644 index 000000000..c98265379 --- /dev/null +++ b/tests/Type/DirectiveTest.php @@ -0,0 +1,15 @@ + { + */ +final class DirectiveTest extends TestCase +{ + // TODO implement https://github.com/graphql/graphql-js/blob/master/src/type/__tests__/directive-test.js +} diff --git a/tests/Type/IntrospectionTest.php b/tests/Type/IntrospectionTest.php index fac1c9934..de8363172 100644 --- a/tests/Type/IntrospectionTest.php +++ b/tests/Type/IntrospectionTest.php @@ -35,7 +35,10 @@ public function testExecutesAnIntrospectionQuery() : void ]), ]); - $request = Introspection::getIntrospectionQuery(['descriptions' => false]); + $request = Introspection::getIntrospectionQuery([ + 'descriptions' => false, + 'directiveIsRepeatable' => true, + ]); $expected = [ 'data' => [ @@ -794,7 +797,7 @@ public function testExecutesAnIntrospectionQuery() : void ], 2 => [ - 'name' => 'locations', + 'name' => 'args', 'args' => [], 'type' => @@ -811,8 +814,8 @@ public function testExecutesAnIntrospectionQuery() : void 'name' => null, 'ofType' => [ - 'kind' => 'ENUM', - 'name' => '__DirectiveLocation', + 'kind' => 'OBJECT', + 'name' => '__InputValue', ], ], ], @@ -822,7 +825,25 @@ public function testExecutesAnIntrospectionQuery() : void ], 3 => [ - 'name' => 'args', + 'name' => 'isRepeatable', + 'args' => + [], + 'type' => + [ + 'kind' => 'NON_NULL', + 'name' => null, + 'ofType' => [ + 'kind' => 'SCALAR', + 'name' => 'Boolean', + 'ofType' => null, + ], + ], + 'isDeprecated' => false, + 'deprecationReason' => null, + ], + 4 => + [ + 'name' => 'locations', 'args' => [], 'type' => @@ -839,8 +860,8 @@ public function testExecutesAnIntrospectionQuery() : void 'name' => null, 'ofType' => [ - 'kind' => 'OBJECT', - 'name' => '__InputValue', + 'kind' => 'ENUM', + 'name' => '__DirectiveLocation', ], ], ], @@ -975,12 +996,7 @@ public function testExecutesAnIntrospectionQuery() : void 0 => [ 'name' => 'include', - 'locations' => - [ - 0 => 'FIELD', - 1 => 'FRAGMENT_SPREAD', - 2 => 'INLINE_FRAGMENT', - ], + 'isRepeatable' => false, 'args' => [ 0 => @@ -999,16 +1015,17 @@ public function testExecutesAnIntrospectionQuery() : void ], ], ], - ], - 1 => - [ - 'name' => 'skip', 'locations' => [ 0 => 'FIELD', 1 => 'FRAGMENT_SPREAD', 2 => 'INLINE_FRAGMENT', ], + ], + 1 => + [ + 'name' => 'skip', + 'isRepeatable' => false, 'args' => [ 0 => @@ -1027,15 +1044,17 @@ public function testExecutesAnIntrospectionQuery() : void ], ], ], + 'locations' => + [ + 0 => 'FIELD', + 1 => 'FRAGMENT_SPREAD', + 2 => 'INLINE_FRAGMENT', + ], ], 2 => [ 'name' => 'deprecated', - 'locations' => - [ - 0 => 'FIELD_DEFINITION', - 1 => 'ENUM_VALUE', - ], + 'isRepeatable' => false, 'args' => [ 0 => @@ -1050,6 +1069,11 @@ public function testExecutesAnIntrospectionQuery() : void ], ], ], + 'locations' => + [ + 0 => 'FIELD_DEFINITION', + 1 => 'ENUM_VALUE', + ], ], ], ], @@ -1608,7 +1632,7 @@ public function testExecutesAnIntrospectionQueryWithoutCallingGlobalFieldResolve ]); $schema = new Schema([ 'query' => $QueryRoot ]); - $source = Introspection::getIntrospectionQuery(); + $source = Introspection::getIntrospectionQuery(['directiveIsRepeatable' => true]); $calledForFields = []; /* istanbul ignore next */ diff --git a/tests/Utils/BuildClientSchemaTest.php b/tests/Utils/BuildClientSchemaTest.php index 814f69dd7..0d82fd4f3 100644 --- a/tests/Utils/BuildClientSchemaTest.php +++ b/tests/Utils/BuildClientSchemaTest.php @@ -23,10 +23,12 @@ class BuildClientSchemaTest extends TestCase { protected static function assertCycleIntrospection(string $sdl) : void { + $options = ['directiveIsRepeatable' => true]; + $serverSchema = BuildSchema::build($sdl); - $initialIntrospection = Introspection::fromSchema($serverSchema); + $initialIntrospection = Introspection::fromSchema($serverSchema, $options); $clientSchema = BuildClientSchema::build($initialIntrospection); - $secondIntrospection = Introspection::fromSchema($clientSchema); + $secondIntrospection = Introspection::fromSchema($clientSchema, $options); self::assertSame($initialIntrospection, $secondIntrospection); } @@ -489,8 +491,8 @@ public function testBuildsASchemaWithCustomDirectives() : void { self::assertCycleIntrospection(' """This is a custom directive""" - directive @customDirective on FIELD - + directive @customDirective repeatable on FIELD + type Query { string: String } diff --git a/tests/Utils/BuildSchemaTest.php b/tests/Utils/BuildSchemaTest.php index 3990093fa..7da00b10a 100644 --- a/tests/Utils/BuildSchemaTest.php +++ b/tests/Utils/BuildSchemaTest.php @@ -106,6 +106,8 @@ public function testWithDirectives() : void $body = ' directive @foo(arg: Int) on FIELD +directive @repeatableFoo(arg: Int) repeatable on FIELD + type Query { str: String } diff --git a/tests/Utils/SchemaExtenderTest.php b/tests/Utils/SchemaExtenderTest.php index a7d9729bc..ea5fbc476 100644 --- a/tests/Utils/SchemaExtenderTest.php +++ b/tests/Utils/SchemaExtenderTest.php @@ -522,7 +522,7 @@ interfaceField: String type TestType implements TestInterface { interfaceField: String } - directive @test(arg: Int) on FIELD | SCALAR + directive @test(arg: Int) repeatable on FIELD | SCALAR '); $extendedTwiceSchema = SchemaExtender::extend($extendedSchema, $ast); @@ -1263,7 +1263,7 @@ public function testSetsCorrectDescriptionUsingLegacyComments() public function testMayExtendDirectivesWithNewComplexDirective() { $extendedSchema = $this->extendTestSchema(' - directive @profile(enable: Boolean! tag: String) on QUERY | FIELD + directive @profile(enable: Boolean! tag: String) repeatable on QUERY | FIELD '); $extendedDirective = $extendedSchema->getDirective('profile'); diff --git a/tests/Utils/SchemaPrinterTest.php b/tests/Utils/SchemaPrinterTest.php index ab54cdf1b..be8c4d7db 100644 --- a/tests/Utils/SchemaPrinterTest.php +++ b/tests/Utils/SchemaPrinterTest.php @@ -706,26 +706,50 @@ public function testPrintsCustomDirectives() : void $query = new ObjectType([ 'name' => 'Query', 'fields' => [ - 'field' => ['type' => Type::string()], + 'field' => [ + 'type' => Type::string(), + ], ], ]); - $customDirectives = new Directive([ - 'name' => 'customDirective', + $simpleDirective = new Directive([ + 'name' => 'simpleDirective', 'locations' => [ DirectiveLocation::FIELD, ], ]); + $complexDirective = new Directive([ + 'name' => 'complexDirective', + 'description' => 'Complex Directive', + 'args' => [ + 'stringArg' => [ + 'type' => Type::string(), + ], + 'intArg' => [ + 'type' => Type::int(), + 'defaultValue' => -1, + ], + ], + 'isRepeatable' => true, + 'locations' => [ + DirectiveLocation::FIELD, + DirectiveLocation::QUERY, + ], + ]); + $schema = new Schema([ 'query' => $query, - 'directives' => [$customDirectives], + 'directives' => [$simpleDirective, $complexDirective], ]); $output = $this->printForTest($schema); self::assertEquals( ' -directive @customDirective on FIELD +directive @simpleDirective on FIELD + +"""Complex Directive""" +directive @complexDirective(stringArg: String, intArg: Int = -1) repeatable on FIELD | QUERY type Query { field: String @@ -871,8 +895,9 @@ public function testPrintIntrospectionSchema() : void type __Directive { name: String! description: String - locations: [__DirectiveLocation!]! args: [__InputValue!]! + isRepeatable: Boolean! + locations: [__DirectiveLocation!]! } """ @@ -1111,8 +1136,9 @@ public function testPrintIntrospectionSchemaWithCommentDescriptions() : void type __Directive { name: String! description: String - locations: [__DirectiveLocation!]! args: [__InputValue!]! + isRepeatable: Boolean! + locations: [__DirectiveLocation!]! } # A Directive can be adjacent to many parts of the GraphQL language, a diff --git a/tests/Validator/UniqueDirectivesPerLocationTest.php b/tests/Validator/UniqueDirectivesPerLocationTest.php index c71d02c65..5e7d656c6 100644 --- a/tests/Validator/UniqueDirectivesPerLocationTest.php +++ b/tests/Validator/UniqueDirectivesPerLocationTest.php @@ -89,6 +89,39 @@ public function testSameDirectivesInSimilarLocations() : void ); } + /** + * @see it('repeatable directives in same location', () => { + */ + public function testRepeatableDirectivesInSameLocation() : void + { + $this->expectPassesRule( + new UniqueDirectivesPerLocation(), + ' + fragment Test on Type @repeatable @repeatable { + field @repeatable @repeatable + } + ' + ); + } + + /** + * @see it('unknown directives must be ignored', () => { + */ + public function testUnknownDirectivesMustBeIgnored() : void + { + $this->expectPassesRule( + new UniqueDirectivesPerLocation(), + ' + type Test @unknown @unknown { + field: String! @unknown @unknown + } + extend type Test @unknown { + anotherField: String! + } + ' + ); + } + /** * @see it('duplicate directives in one location') */ @@ -180,38 +213,25 @@ public function testDuplicateDirectivesOnSDLDefinitions() { $this->expectSDLErrors( ' - schema @directive @directive { query: Dummy } - extend schema @directive @directive - - scalar TestScalar @directive @directive - extend scalar TestScalar @directive @directive - - type TestObject @directive @directive - extend type TestObject @directive @directive - - interface TestInterface @directive @directive - extend interface TestInterface @directive @directive + directive @nonRepeatable on + SCHEMA | SCALAR | OBJECT | INTERFACE | UNION | INPUT_OBJECT - union TestUnion @directive @directive - extend union TestUnion @directive @directive + schema @nonRepeatable @nonRepeatable { query: Dummy } - input TestInput @directive @directive - extend input TestInput @directive @directive + scalar TestScalar @nonRepeatable @nonRepeatable + type TestObject @nonRepeatable @nonRepeatable + interface TestInterface @nonRepeatable @nonRepeatable + union TestUnion @nonRepeatable @nonRepeatable + input TestInput @nonRepeatable @nonRepeatable ', null, [ - $this->duplicateDirective('directive', 2, 14, 2, 25), - $this->duplicateDirective('directive', 3, 21, 3, 32), - $this->duplicateDirective('directive', 5, 25, 5, 36), - $this->duplicateDirective('directive', 6, 32, 6, 43), - $this->duplicateDirective('directive', 8, 23, 8, 34), - $this->duplicateDirective('directive', 9, 30, 9, 41), - $this->duplicateDirective('directive', 11, 31, 11, 42), - $this->duplicateDirective('directive', 12, 38, 12, 49), - $this->duplicateDirective('directive', 14, 23, 14, 34), - $this->duplicateDirective('directive', 15, 30, 15, 41), - $this->duplicateDirective('directive', 17, 23, 17, 34), - $this->duplicateDirective('directive', 18, 30, 18, 41), + $this->duplicateDirective('nonRepeatable', 5, 14, 5, 29), + $this->duplicateDirective('nonRepeatable', 7, 25, 7, 40), + $this->duplicateDirective('nonRepeatable', 8, 23, 8, 38), + $this->duplicateDirective('nonRepeatable', 9, 31, 9, 46), + $this->duplicateDirective('nonRepeatable', 10, 23, 10, 38), + $this->duplicateDirective('nonRepeatable', 11, 23, 11, 38), ] ); } diff --git a/tests/Validator/ValidatorTestCase.php b/tests/Validator/ValidatorTestCase.php index 0944f0e5c..6af9b580c 100644 --- a/tests/Validator/ValidatorTestCase.php +++ b/tests/Validator/ValidatorTestCase.php @@ -5,6 +5,7 @@ namespace GraphQL\Tests\Validator; use Exception; +use GraphQL\Language\DirectiveLocation; use GraphQL\Language\Parser; use GraphQL\Type\Definition\CustomScalarType; use GraphQL\Type\Definition\Directive; @@ -354,37 +355,49 @@ public static function getTestSchema() 'directives' => [ Directive::includeDirective(), Directive::skipDirective(), + new Directive([ + 'name' => 'directive', + 'locations' => [DirectiveLocation::FIELD], + ]), + new Directive([ + 'name' => 'directiveA', + 'locations' => [DirectiveLocation::FIELD], + ]), + new Directive([ + 'name' => 'directiveB', + 'locations' => [DirectiveLocation::FIELD], + ]), new Directive([ 'name' => 'onQuery', - 'locations' => ['QUERY'], + 'locations' => [DirectiveLocation::QUERY], ]), new Directive([ 'name' => 'onMutation', - 'locations' => ['MUTATION'], + 'locations' => [DirectiveLocation::MUTATION], ]), new Directive([ 'name' => 'onSubscription', - 'locations' => ['SUBSCRIPTION'], + 'locations' => [DirectiveLocation::SUBSCRIPTION], ]), new Directive([ 'name' => 'onField', - 'locations' => ['FIELD'], + 'locations' => [DirectiveLocation::FIELD], ]), new Directive([ 'name' => 'onFragmentDefinition', - 'locations' => ['FRAGMENT_DEFINITION'], + 'locations' => [DirectiveLocation::FRAGMENT_DEFINITION], ]), new Directive([ 'name' => 'onFragmentSpread', - 'locations' => ['FRAGMENT_SPREAD'], + 'locations' => [DirectiveLocation::FRAGMENT_SPREAD], ]), new Directive([ 'name' => 'onInlineFragment', - 'locations' => ['INLINE_FRAGMENT'], + 'locations' => [DirectiveLocation::INLINE_FRAGMENT], ]), new Directive([ 'name' => 'onVariableDefinition', - 'locations' => ['VARIABLE_DEFINITION'], + 'locations' => [DirectiveLocation::VARIABLE_DEFINITION], ]), ], ]);