From 800c8940940ce4822c8905105b40e522c43d3610 Mon Sep 17 00:00:00 2001 From: Michael Georgiadis <38563912+michael-georgiadis@users.noreply.github.com> Date: Mon, 6 Jul 2026 13:39:22 +0200 Subject: [PATCH 1/7] Implement discovery, registry, wiring --- src/Directives/DirectiveRegistry.php | 88 ++++++++++++++++-- src/Directives/DirectiveValidator.php | 92 ++++++++++++++++++- .../Discovery/DirectiveClassFinder.php | 74 +++++++++++++++ .../Discovery/GlobDirectivesCache.php | 21 +++++ src/SchemaFactory.php | 6 +- 5 files changed, 267 insertions(+), 14 deletions(-) create mode 100644 src/Directives/Discovery/DirectiveClassFinder.php create mode 100644 src/Directives/Discovery/GlobDirectivesCache.php diff --git a/src/Directives/DirectiveRegistry.php b/src/Directives/DirectiveRegistry.php index c4718a47e9..021521601d 100644 --- a/src/Directives/DirectiveRegistry.php +++ b/src/Directives/DirectiveRegistry.php @@ -9,12 +9,18 @@ use TheCodingMachine\GraphQLite\AnnotationReader; use TheCodingMachine\GraphQLite\Directives\BuiltIn\Deprecated; use TheCodingMachine\GraphQLite\Directives\BuiltIn\OneOf; +use TheCodingMachine\GraphQLite\Directives\Discovery\DirectiveClassFinder; use TheCodingMachine\GraphQLite\Directives\Exceptions\InvalidDirectiveException; +use function array_key_exists; +use function assert; +use function method_exists; + /** - * Holds the directives known to a schema. In this layer that's the built-ins that bind PHP behavior - * to directives webonyx already declares (`@oneOf`, `@deprecated`); a later layer adds user-defined - * directives on top. + * Holds the directives known to a schema: the user-defined directives discovered in the configured + * namespaces plus the built-ins that bind PHP behavior to directives webonyx already declares + * (`@oneOf`, `@deprecated`). For each discovered class it runs {@see DirectiveValidator}, caches what + * {@see DirectiveResolver} produces, and enforces name uniqueness across the set. * * The dispatcher middlewares query it at apply time for a directive's argument shape, which the AST * builder needs to encode each arg as a GraphQL value. @@ -24,7 +30,15 @@ final class DirectiveRegistry /** @var array, ResolvedDirective> */ private array $resolvedByClass = []; - /** Attribute classes that bind PHP behavior to webonyx's built-in directives. */ + /** @var array> */ + private array $classByName = []; + + /** + * Attribute classes that bind PHP behavior to webonyx's pre-existing built-in directives. + * Also the source of truth for reserved names: a custom (non-built-in) directive can't claim + * one of these names unless it declares `builtIn: true` to override the bundled binding. + * Registered after user discovery so such an override wins. + */ private const BUILT_IN_ATTRIBUTES = [ OneOf::class, Deprecated::class, @@ -32,20 +46,76 @@ final class DirectiveRegistry public function __construct( private readonly AnnotationReader $annotationReader, + private readonly DirectiveClassFinder $classFinder, ) { } - /** Resolve the built-in directives once. Idempotent. */ + /** Discover + validate the user directives, then the built-ins. Idempotent. */ public function discover(): void { + // User classes first: an override of a built-in (same name, builtIn: true) needs to land + // before our bundled copy registers. + foreach ($this->classFinder->findDirectives() as $directiveClass) { + $this->register($directiveClass); + } foreach (self::BUILT_IN_ATTRIBUTES as $directiveClass) { - /** @var class-string $directiveClass */ - if (isset($this->resolvedByClass[$directiveClass])) { - continue; + $this->register($directiveClass); + } + } + + /** + * @param class-string $directiveClass + * + * @throws InvalidDirectiveException + */ + private function register(string $directiveClass): void + { + // Registering the same class twice is a no-op; discovery and the built-in list share this + // registry, so duplicates are expected. + if (isset($this->resolvedByClass[$directiveClass])) { + return; + } + + if (! method_exists($directiveClass, 'definition')) { + throw InvalidDirectiveException::noDefinitionMethod($directiveClass); + } + + $definition = $directiveClass::definition(); + assert($definition instanceof DirectiveDefinition); + + DirectiveValidator::validate($directiveClass, $definition); + + if (! $definition->builtIn && $this->isReservedBuiltInName($definition->name)) { + throw InvalidDirectiveException::reservedName($definition->name, $directiveClass); + } + + if (array_key_exists($definition->name, $this->classByName)) { + // A name clash is fine when this side is also built-in: the user supplied their own + // implementation and we defer to it. Two custom directives sharing a name is an error. + if ($definition->builtIn) { + return; } + throw InvalidDirectiveException::duplicateName( + $definition->name, + $this->classByName[$definition->name], + $directiveClass, + ); + } - $this->resolvedByClass[$directiveClass] = DirectiveResolver::resolve($directiveClass, $directiveClass::definition()); + $this->resolvedByClass[$directiveClass] = DirectiveResolver::resolve($directiveClass, $definition); + $this->classByName[$definition->name] = $directiveClass; + } + + /** Whether $name is bound by one of our built-in attributes, so only a `builtIn: true` override may take it. */ + private function isReservedBuiltInName(string $name): bool + { + foreach (self::BUILT_IN_ATTRIBUTES as $builtInClass) { + if ($builtInClass::definition()->name === $name) { + return true; + } } + + return false; } public function hasAny(): bool diff --git a/src/Directives/DirectiveValidator.php b/src/Directives/DirectiveValidator.php index bf3d700ef2..dfa21667c1 100644 --- a/src/Directives/DirectiveValidator.php +++ b/src/Directives/DirectiveValidator.php @@ -4,6 +4,7 @@ namespace TheCodingMachine\GraphQLite\Directives; +use Attribute; use ReflectionClass; use TheCodingMachine\GraphQLite\Directives\Exceptions\InvalidDirectiveException; @@ -11,15 +12,40 @@ use function is_a; /** - * Validates that directives are applied where they're allowed. PHP's `#[Attribute]` targets can't - * tell a `#[Type]` class from an `#[Input]` class (both are `TARGET_CLASS`), so a class-level - * directive like `#[OneOf]` could be placed on the wrong kind of class. This reports that instead - * of letting the interface-based collectors drop the misplaced directive silently. + * Validates directives at two points: + * + * - {@see self::validate()} at discovery time: the `#[Attribute(...)]` PHP target covers every + * declared GraphQL location, and each family interface has a matching location declared (and + * vice versa). + * - {@see self::assertDirectivesUsableAt()} at apply time: a directive placed on a class is + * allowed at that class's location. PHP's `#[Attribute]` targets can't tell a `#[Type]` class + * from an `#[Input]` class (both are `TARGET_CLASS`), so `#[OneOf]` could be placed on the wrong + * kind of class; this reports it instead of letting the collectors drop it silently. + * + * Producing the arguments, repeatability, and webonyx directive is {@see DirectiveResolver}'s job; + * name uniqueness is checked in {@see DirectiveRegistry}. * * @internal */ final class DirectiveValidator { + /** + * @param class-string $directiveClass + * + * @throws InvalidDirectiveException + */ + public static function validate(string $directiveClass, DirectiveDefinition $definition): void + { + $reflection = new ReflectionClass($directiveClass); + + if ($reflection->getAttributes(Attribute::class) === []) { + throw InvalidDirectiveException::notAttribute($directiveClass); + } + + self::checkPhpTargets($directiveClass, $definition, DirectiveReflection::attributeFlags($reflection)); + self::checkInterfaceAndLocationAgreement($directiveClass, $definition, $reflection); + } + /** * @param ReflectionClass $refClass * @@ -41,4 +67,62 @@ public static function assertDirectivesUsableAt(ReflectionClass $refClass, Direc throw InvalidDirectiveException::notUsableAtLocation($directiveClass, $location, $locations); } } + + private static function checkPhpTargets(string $directiveClass, DirectiveDefinition $definition, int $phpFlags): void + { + foreach ($definition->locations as $location) { + foreach (self::requiredPhpTargetsFor($location) as $requiredTarget => $label) { + if (($phpFlags & $requiredTarget) === $requiredTarget) { + continue; + } + + throw InvalidDirectiveException::phpTargetMissingForLocation($directiveClass, $location, $label); + } + } + } + + /** @param ReflectionClass $reflection */ + private static function checkInterfaceAndLocationAgreement(string $directiveClass, DirectiveDefinition $definition, ReflectionClass $reflection): void + { + $locations = $definition->locations; + $interfacePairs = [ + FieldDirective::class => DirectiveLocation::FIELD_DEFINITION, + InputFieldDirective::class => DirectiveLocation::INPUT_FIELD_DEFINITION, + ObjectTypeDirective::class => DirectiveLocation::OBJECT, + InputObjectTypeDirective::class => DirectiveLocation::INPUT_OBJECT, + ]; + + foreach ($interfacePairs as $interface => $expectedLocation) { + $implements = $reflection->implementsInterface($interface); + $declaresLocation = in_array($expectedLocation, $locations, true); + + if ($implements && ! $declaresLocation) { + throw InvalidDirectiveException::interfaceWithoutMatchingLocation($directiveClass, $interface, $locations); + } + + if (! $implements && $declaresLocation) { + throw InvalidDirectiveException::locationWithoutMatchingInterface($directiveClass, $expectedLocation, $interface); + } + } + } + + /** @return array map of Attribute::TARGET_* flag → human-readable label */ + private static function requiredPhpTargetsFor(DirectiveLocation $location): array + { + return match ($location) { + DirectiveLocation::FIELD_DEFINITION => [ + Attribute::TARGET_METHOD => 'TARGET_METHOD', + Attribute::TARGET_PROPERTY => 'TARGET_PROPERTY', + ], + DirectiveLocation::INPUT_FIELD_DEFINITION => [ + Attribute::TARGET_METHOD => 'TARGET_METHOD', + Attribute::TARGET_PROPERTY => 'TARGET_PROPERTY', + Attribute::TARGET_PARAMETER => 'TARGET_PARAMETER', + ], + DirectiveLocation::OBJECT => [Attribute::TARGET_CLASS => 'TARGET_CLASS'], + DirectiveLocation::INPUT_OBJECT => [Attribute::TARGET_CLASS => 'TARGET_CLASS'], + // Other locations don't have apply hooks yet, so there's nothing to enforce. + default => [], + }; + } } diff --git a/src/Directives/Discovery/DirectiveClassFinder.php b/src/Directives/Discovery/DirectiveClassFinder.php new file mode 100644 index 0000000000..d0f9d09582 --- /dev/null +++ b/src/Directives/Discovery/DirectiveClassFinder.php @@ -0,0 +1,74 @@ +>|null */ + private array|null $cache = null; + + public function __construct( + private readonly ClassFinder $classFinder, + private readonly ClassFinderComputedCache $classFinderComputedCache, + ) { + } + + /** @return list> */ + public function findDirectives(): array + { + if ($this->cache !== null) { + return $this->cache; + } + + /** @var list> $result */ + $result = $this->classFinderComputedCache->compute( + $this->classFinder, + 'customDirectives', + static function (ReflectionClass $refClass): GlobDirectivesCache|null { + if ($refClass->isAbstract() || $refClass->isInterface() || $refClass->isEnum()) { + return null; + } + + if (! $refClass->implementsInterface(TypeSystemDirective::class)) { + return null; + } + + /** @var class-string $directiveClass */ + $directiveClass = $refClass->getName(); + return new GlobDirectivesCache($directiveClass); + }, + static fn (array $entries): array => array_reduce( + $entries, + static function (array $carry, GlobDirectivesCache|null $entry): array { + if ($entry === null) { + return $carry; + } + $carry[] = $entry->directiveClass; + return $carry; + }, + [], + ), + ); + + $this->cache = $result; + return $result; + } +} diff --git a/src/Directives/Discovery/GlobDirectivesCache.php b/src/Directives/Discovery/GlobDirectivesCache.php new file mode 100644 index 0000000000..d587eaba1c --- /dev/null +++ b/src/Directives/Discovery/GlobDirectivesCache.php @@ -0,0 +1,21 @@ + $directiveClass */ + public function __construct(public readonly string $directiveClass) + { + } +} diff --git a/src/SchemaFactory.php b/src/SchemaFactory.php index 4aea95bb3e..3dee20609f 100644 --- a/src/SchemaFactory.php +++ b/src/SchemaFactory.php @@ -20,6 +20,7 @@ use TheCodingMachine\GraphQLite\Cache\SnapshotClassBoundCache; use TheCodingMachine\GraphQLite\Directives\DirectiveAstBuilder; use TheCodingMachine\GraphQLite\Directives\DirectiveRegistry; +use TheCodingMachine\GraphQLite\Directives\Discovery\DirectiveClassFinder; use TheCodingMachine\GraphQLite\Discovery\Cache\HardClassFinderComputedCache; use TheCodingMachine\GraphQLite\Discovery\Cache\SnapshotClassFinderComputedCache; use TheCodingMachine\GraphQLite\Discovery\ClassFinder; @@ -408,7 +409,10 @@ public function createSchema(): Schema $expressionLanguage = $this->expressionLanguage ?: new ExpressionLanguage($symfonyCache); $expressionLanguage->registerProvider(new SecurityExpressionLanguageProvider()); - $directiveRegistry = new DirectiveRegistry($annotationReader); + $directiveRegistry = new DirectiveRegistry( + $annotationReader, + new DirectiveClassFinder($classFinder, $classFinderComputedCache), + ); $directiveRegistry->discover(); $directiveAstBuilder = new DirectiveAstBuilder($directiveRegistry); From bfcffb93acaed7e3ca32c057e6aef785164b4e82 Mon Sep 17 00:00:00 2001 From: Michael Georgiadis <38563912+michael-georgiadis@users.noreply.github.com> Date: Mon, 6 Jul 2026 13:41:21 +0200 Subject: [PATCH 2/7] Add unit-tests --- tests/Directives/BuiltInDirectivesTest.php | 81 ++++++++++++++ tests/Directives/DirectiveDefinitionTest.php | 33 ++++++ tests/Directives/DirectiveLocationTest.php | 49 +++++++++ tests/Directives/DirectiveRegistryTest.php | 104 ++++++++++++++++-- tests/Directives/DirectiveResolverTest.php | 55 +++++++++ tests/Directives/DirectiveValidatorTest.php | 29 +++++ .../Directives/CustomOneOfOverride.php | 37 +++++++ .../InterfaceWithoutLocationDirective.php | 25 +++++ .../Invalid/MissingPhpTargetDirective.php | 26 +++++ .../Invalid/ReservedNameDirective.php | 25 +++++ .../UnsupportedArgumentTypeDirective.php | 31 ++++++ .../Directives/NoteFieldDirective.php | 31 ++++++ .../RevisionInputObjectDirective.php | 30 +++++ 13 files changed, 547 insertions(+), 9 deletions(-) create mode 100644 tests/Directives/BuiltInDirectivesTest.php create mode 100644 tests/Directives/DirectiveDefinitionTest.php create mode 100644 tests/Directives/DirectiveLocationTest.php create mode 100644 tests/Directives/DirectiveResolverTest.php create mode 100644 tests/Directives/DirectiveValidatorTest.php create mode 100644 tests/Fixtures/Directives/CustomOneOfOverride.php create mode 100644 tests/Fixtures/Directives/Invalid/InterfaceWithoutLocationDirective.php create mode 100644 tests/Fixtures/Directives/Invalid/MissingPhpTargetDirective.php create mode 100644 tests/Fixtures/Directives/Invalid/ReservedNameDirective.php create mode 100644 tests/Fixtures/Directives/Invalid/UnsupportedArgumentTypeDirective.php create mode 100644 tests/Fixtures/Directives/NoteFieldDirective.php create mode 100644 tests/Fixtures/Directives/RevisionInputObjectDirective.php diff --git a/tests/Directives/BuiltInDirectivesTest.php b/tests/Directives/BuiltInDirectivesTest.php new file mode 100644 index 0000000000..320f538b28 --- /dev/null +++ b/tests/Directives/BuiltInDirectivesTest.php @@ -0,0 +1,81 @@ +discover(); + + $this->assertNotNull($registry->definitionFor(OneOf::class)); + } + + public function testBuiltInDirectivesAreNotEmittedAsCustomDirectives(): void + { + $registry = self::buildRegistry([]); + $registry->discover(); + + // @oneOf is a webonyx built-in, so the registry shouldn't add it to the custom list. + $names = array_map(static fn (WebonyxDirective $d) => $d->name, $registry->webonyxDirectives()); + + $this->assertSame([], $names); + $this->assertFalse($registry->hasAny()); + } + + public function testOneOfDefinitionMarksItselfAsBuiltIn(): void + { + $definition = OneOf::definition(); + + $this->assertSame('oneOf', $definition->name); + $this->assertTrue($definition->builtIn); + $this->assertSame([DirectiveLocation::INPUT_OBJECT], $definition->locations); + } + + public function testDiscoveryFindingTheBundledBuiltInClassIsIdempotent(): void + { + // If discovery finds our own built-in class (same FQCN as BUILT_IN_ATTRIBUTES), registering + // it twice shouldn't throw. + $registry = self::buildRegistry([OneOf::class]); + $registry->discover(); + + $this->assertNotNull($registry->definitionFor(OneOf::class)); + } + + public function testUserOverrideOfBuiltInWinsOverBundled(): void + { + // User binds their own class to @oneOf (builtIn: true); ours should defer to it. + $registry = self::buildRegistry([CustomOneOfOverride::class]); + $registry->discover(); + + $this->assertNotNull($registry->definitionFor(CustomOneOfOverride::class)); + $this->assertNull($registry->definitionFor(OneOf::class)); + } + + /** @param list> $classes */ + private static function buildRegistry(array $classes): DirectiveRegistry + { + $finder = new DirectiveClassFinder( + new StaticClassFinder($classes), + new HardClassFinderComputedCache(new Psr16Cache(new ArrayAdapter())), + ); + + return new DirectiveRegistry(new AnnotationReader(), $finder); + } +} diff --git a/tests/Directives/DirectiveDefinitionTest.php b/tests/Directives/DirectiveDefinitionTest.php new file mode 100644 index 0000000000..3fa2000f4a --- /dev/null +++ b/tests/Directives/DirectiveDefinitionTest.php @@ -0,0 +1,33 @@ +assertSame('audit', $definition->name); + $this->assertSame([DirectiveLocation::FIELD_DEFINITION, DirectiveLocation::INPUT_FIELD_DEFINITION], $definition->locations); + $this->assertSame('Audit log marker', $definition->description); + } + + public function testDefaultsDescriptionToNull(): void + { + $definition = new DirectiveDefinition( + name: 'noop', + locations: [DirectiveLocation::OBJECT], + ); + + $this->assertNull($definition->description); + } +} diff --git a/tests/Directives/DirectiveLocationTest.php b/tests/Directives/DirectiveLocationTest.php new file mode 100644 index 0000000000..69739c6e94 --- /dev/null +++ b/tests/Directives/DirectiveLocationTest.php @@ -0,0 +1,49 @@ +assertCount(19, DirectiveLocation::cases()); + } + + public function testBackingValuesMatchSpecStrings(): void + { + $this->assertSame('FIELD_DEFINITION', DirectiveLocation::FIELD_DEFINITION->value); + $this->assertSame('OBJECT', DirectiveLocation::OBJECT->value); + $this->assertSame('INPUT_OBJECT', DirectiveLocation::INPUT_OBJECT->value); + $this->assertSame('INPUT_FIELD_DEFINITION', DirectiveLocation::INPUT_FIELD_DEFINITION->value); + } + + public function testIsExecutableClassifiesQueryDocumentLocations(): void + { + $this->assertTrue(DirectiveLocation::QUERY->isExecutable()); + $this->assertTrue(DirectiveLocation::FIELD->isExecutable()); + $this->assertTrue(DirectiveLocation::FRAGMENT_DEFINITION->isExecutable()); + $this->assertTrue(DirectiveLocation::INLINE_FRAGMENT->isExecutable()); + $this->assertTrue(DirectiveLocation::VARIABLE_DEFINITION->isExecutable()); + } + + public function testIsTypeSystemClassifiesSchemaLocations(): void + { + $this->assertTrue(DirectiveLocation::FIELD_DEFINITION->isTypeSystem()); + $this->assertTrue(DirectiveLocation::OBJECT->isTypeSystem()); + $this->assertTrue(DirectiveLocation::INPUT_OBJECT->isTypeSystem()); + $this->assertTrue(DirectiveLocation::INPUT_FIELD_DEFINITION->isTypeSystem()); + $this->assertTrue(DirectiveLocation::ENUM->isTypeSystem()); + $this->assertTrue(DirectiveLocation::SCHEMA->isTypeSystem()); + } + + public function testIsExecutableAndIsTypeSystemArePartitions(): void + { + foreach (DirectiveLocation::cases() as $location) { + $this->assertNotSame($location->isExecutable(), $location->isTypeSystem(), 'Location ' . $location->value . ' should be executable or type-system, not both.'); + } + } +} diff --git a/tests/Directives/DirectiveRegistryTest.php b/tests/Directives/DirectiveRegistryTest.php index e39bb87112..ddebc37366 100644 --- a/tests/Directives/DirectiveRegistryTest.php +++ b/tests/Directives/DirectiveRegistryTest.php @@ -4,24 +4,28 @@ namespace TheCodingMachine\GraphQLite\Directives; +use GraphQL\Type\Definition\Directive as WebonyxDirective; use PHPUnit\Framework\TestCase; use ReflectionClass; +use Symfony\Component\Cache\Adapter\ArrayAdapter; +use Symfony\Component\Cache\Psr16Cache; use TheCodingMachine\GraphQLite\AnnotationReader; use TheCodingMachine\GraphQLite\Directives\BuiltIn\Deprecated; use TheCodingMachine\GraphQLite\Directives\BuiltIn\OneOf; +use TheCodingMachine\GraphQLite\Directives\Discovery\DirectiveClassFinder; use TheCodingMachine\GraphQLite\Directives\Exceptions\InvalidDirectiveException; +use TheCodingMachine\GraphQLite\Discovery\Cache\HardClassFinderComputedCache; +use TheCodingMachine\GraphQLite\Discovery\StaticClassFinder; +use TheCodingMachine\GraphQLite\Fixtures\Directives\InternalFieldDirective; +use TheCodingMachine\GraphQLite\Fixtures\Directives\Invalid\ReservedNameDirective; use TheCodingMachine\GraphQLite\Fixtures\Directives\MisusedOneOfOnType; +use TheCodingMachine\GraphQLite\Fixtures\Directives\NoteFieldDirective; +use TheCodingMachine\GraphQLite\Fixtures\Directives\RevisionInputObjectDirective; + +use function array_map; final class DirectiveRegistryTest extends TestCase { - private function registry(): DirectiveRegistry - { - $registry = new DirectiveRegistry(new AnnotationReader()); - $registry->discover(); - - return $registry; - } - public function testRegistersTheBuiltInDirectives(): void { $registry = $this->registry(); @@ -50,7 +54,7 @@ public function testResolvesBuiltInArguments(): void public function testDiscoverIsIdempotent(): void { - $registry = new DirectiveRegistry(new AnnotationReader()); + $registry = self::buildRegistry(); $registry->discover(); $registry->discover(); @@ -64,4 +68,86 @@ public function testObjectTypeDirectivesRejectsMisplacedDirective(): void $this->registry()->objectTypeDirectives(new ReflectionClass(MisusedOneOfOnType::class)); } + + public function testDiscoversAndRegistersValidDirectives(): void + { + $registry = self::buildRegistry([ + InternalFieldDirective::class, + NoteFieldDirective::class, + RevisionInputObjectDirective::class, + ]); + $registry->discover(); + + $this->assertTrue($registry->hasAny()); + + $webonyx = $registry->webonyxDirectives(); + $this->assertCount(3, $webonyx); + + $byName = []; + foreach ($webonyx as $directive) { + $byName[$directive->name] = $directive; + } + + $this->assertArrayHasKey('internal', $byName); + $this->assertArrayHasKey('note', $byName); + $this->assertArrayHasKey('revision', $byName); + + $note = $byName['note']; + $this->assertTrue($note->isRepeatable); + $this->assertSame('Attaches a freeform note to a field.', $note->description); + $this->assertCount(1, $note->args); + $this->assertSame('text', $note->args[0]->name); + } + + public function testRejectsCustomDirectiveUsingReservedName(): void + { + $registry = self::buildRegistry([ReservedNameDirective::class]); + + $this->expectException(InvalidDirectiveException::class); + $this->expectExceptionMessageMatches('/reserved/'); + + $registry->discover(); + } + + public function testEmptyRegistryReportsNoDirectives(): void + { + $registry = self::buildRegistry([]); + $registry->discover(); + + $this->assertFalse($registry->hasAny()); + $this->assertSame([], $registry->webonyxDirectives()); + } + + public function testWebonyxDirectivesIsolatedFromBuiltins(): void + { + // The built directives shouldn't include webonyx's built-ins; those get merged in at the + // Schema layer, not here. + $registry = self::buildRegistry([InternalFieldDirective::class]); + $registry->discover(); + + $names = array_map(static fn (WebonyxDirective $d) => $d->name, $registry->webonyxDirectives()); + $this->assertNotContains('skip', $names); + $this->assertNotContains('include', $names); + $this->assertNotContains('deprecated', $names); + $this->assertNotContains('oneOf', $names); + } + + private function registry(): DirectiveRegistry + { + $registry = self::buildRegistry(); + $registry->discover(); + + return $registry; + } + + /** @param list> $classes */ + private static function buildRegistry(array $classes = []): DirectiveRegistry + { + $finder = new DirectiveClassFinder( + new StaticClassFinder($classes), + new HardClassFinderComputedCache(new Psr16Cache(new ArrayAdapter())), + ); + + return new DirectiveRegistry(new AnnotationReader(), $finder); + } } diff --git a/tests/Directives/DirectiveResolverTest.php b/tests/Directives/DirectiveResolverTest.php new file mode 100644 index 0000000000..b1c7114f0c --- /dev/null +++ b/tests/Directives/DirectiveResolverTest.php @@ -0,0 +1,55 @@ +assertSame([], $resolved->arguments); + } + + public function testResolvesScalarArgumentWithCorrectTypeAndNullability(): void + { + $resolved = DirectiveResolver::resolve(NoteFieldDirective::class, NoteFieldDirective::definition()); + + $this->assertCount(1, $resolved->arguments); + $this->assertSame('text', $resolved->arguments[0]->name); + $this->assertInstanceOf(NonNull::class, $resolved->arguments[0]->type); + $this->assertInstanceOf(StringType::class, $resolved->arguments[0]->type->getWrappedType()); + $this->assertFalse($resolved->arguments[0]->hasDefaultValue); + } + + public function testInfersRepeatableFromAttributeFlags(): void + { + $note = DirectiveResolver::resolve(NoteFieldDirective::class, NoteFieldDirective::definition())->webonyxDirective; + $internal = DirectiveResolver::resolve(InternalFieldDirective::class, InternalFieldDirective::definition())->webonyxDirective; + + $this->assertInstanceOf(WebonyxDirective::class, $note); + $this->assertTrue($note->isRepeatable); + + $this->assertInstanceOf(WebonyxDirective::class, $internal); + $this->assertFalse($internal->isRepeatable); + } + + public function testRejectsUnsupportedArgumentType(): void + { + $this->expectException(InvalidDirectiveException::class); + $this->expectExceptionMessageMatches('/scalar types/'); + + DirectiveResolver::resolve(UnsupportedArgumentTypeDirective::class, UnsupportedArgumentTypeDirective::definition()); + } +} diff --git a/tests/Directives/DirectiveValidatorTest.php b/tests/Directives/DirectiveValidatorTest.php new file mode 100644 index 0000000000..e28b6b1ce4 --- /dev/null +++ b/tests/Directives/DirectiveValidatorTest.php @@ -0,0 +1,29 @@ +expectException(InvalidDirectiveException::class); + $this->expectExceptionMessageMatches('/TARGET_METHOD/'); + + DirectiveValidator::validate(MissingPhpTargetDirective::class, MissingPhpTargetDirective::definition()); + } + + public function testRejectsInterfaceWithoutMatchingLocation(): void + { + $this->expectException(InvalidDirectiveException::class); + $this->expectExceptionMessageMatches('/FieldDirective/'); + + DirectiveValidator::validate(InterfaceWithoutLocationDirective::class, InterfaceWithoutLocationDirective::definition()); + } +} diff --git a/tests/Fixtures/Directives/CustomOneOfOverride.php b/tests/Fixtures/Directives/CustomOneOfOverride.php new file mode 100644 index 0000000000..c2142821c5 --- /dev/null +++ b/tests/Fixtures/Directives/CustomOneOfOverride.php @@ -0,0 +1,37 @@ +handle($descriptor); + $type->isOneOf = true; + return $type; + } +} diff --git a/tests/Fixtures/Directives/Invalid/InterfaceWithoutLocationDirective.php b/tests/Fixtures/Directives/Invalid/InterfaceWithoutLocationDirective.php new file mode 100644 index 0000000000..0eb38163c8 --- /dev/null +++ b/tests/Fixtures/Directives/Invalid/InterfaceWithoutLocationDirective.php @@ -0,0 +1,25 @@ + Date: Mon, 6 Jul 2026 13:41:34 +0200 Subject: [PATCH 3/7] Add integration tests --- .../Directives/InternalFieldDirective.php | 26 +++ .../Controllers/WidgetController.php | 33 ++++ .../Directives/AuditDirective.php | 27 ++++ .../Directives/SanitizedDirective.php | 22 +++ .../Directives/TaggedDirective.php | 26 +++ .../Directives/UppercaseDirective.php | 39 +++++ .../Directives/VersionedDirective.php | 31 ++++ .../Inputs/OneOfLookup.php | 26 +++ .../Inputs/WidgetLookup.php | 24 +++ .../DirectivesIntegration/Models/Widget.php | 29 ++++ tests/Integration/DirectivesEndToEndTest.php | 151 ++++++++++++++++++ 11 files changed, 434 insertions(+) create mode 100644 tests/Fixtures/Directives/InternalFieldDirective.php create mode 100644 tests/Fixtures/DirectivesIntegration/Controllers/WidgetController.php create mode 100644 tests/Fixtures/DirectivesIntegration/Directives/AuditDirective.php create mode 100644 tests/Fixtures/DirectivesIntegration/Directives/SanitizedDirective.php create mode 100644 tests/Fixtures/DirectivesIntegration/Directives/TaggedDirective.php create mode 100644 tests/Fixtures/DirectivesIntegration/Directives/UppercaseDirective.php create mode 100644 tests/Fixtures/DirectivesIntegration/Directives/VersionedDirective.php create mode 100644 tests/Fixtures/DirectivesIntegration/Inputs/OneOfLookup.php create mode 100644 tests/Fixtures/DirectivesIntegration/Inputs/WidgetLookup.php create mode 100644 tests/Fixtures/DirectivesIntegration/Models/Widget.php create mode 100644 tests/Integration/DirectivesEndToEndTest.php diff --git a/tests/Fixtures/Directives/InternalFieldDirective.php b/tests/Fixtures/Directives/InternalFieldDirective.php new file mode 100644 index 0000000000..c2bb17bee3 --- /dev/null +++ b/tests/Fixtures/Directives/InternalFieldDirective.php @@ -0,0 +1,26 @@ +sku ?? 'id-' . ($lookup->id ?? 0)); + } + + #[Query] + public function findOneOf(OneOfLookup $lookup): Widget + { + return new Widget($lookup->sku ?? 'id-' . ($lookup->id ?? 0)); + } +} diff --git a/tests/Fixtures/DirectivesIntegration/Directives/AuditDirective.php b/tests/Fixtures/DirectivesIntegration/Directives/AuditDirective.php new file mode 100644 index 0000000000..7104860cc3 --- /dev/null +++ b/tests/Fixtures/DirectivesIntegration/Directives/AuditDirective.php @@ -0,0 +1,27 @@ +getResolver(); + $descriptor = $descriptor->withResolver(static function (...$args) use ($resolver): mixed { + $value = $resolver(...$args); + return is_string($value) ? strtoupper($value) : $value; + }); + + return $next->handle($descriptor); + } +} diff --git a/tests/Fixtures/DirectivesIntegration/Directives/VersionedDirective.php b/tests/Fixtures/DirectivesIntegration/Directives/VersionedDirective.php new file mode 100644 index 0000000000..d6ba8affeb --- /dev/null +++ b/tests/Fixtures/DirectivesIntegration/Directives/VersionedDirective.php @@ -0,0 +1,31 @@ +label; + } +} diff --git a/tests/Integration/DirectivesEndToEndTest.php b/tests/Integration/DirectivesEndToEndTest.php new file mode 100644 index 0000000000..3ce52389c2 --- /dev/null +++ b/tests/Integration/DirectivesEndToEndTest.php @@ -0,0 +1,151 @@ +directives` for anyone who wants to print them with their own printer. + */ +final class DirectivesEndToEndTest extends TestCase +{ + private function buildSchema(): Schema + { + $cache = new Psr16Cache(new ArrayAdapter()); + $factory = new SchemaFactory($cache, new BasicAutoWiringContainer(new EmptyContainer())); + $factory->prodMode(); + $factory->addNamespace('TheCodingMachine\\GraphQLite\\Fixtures\\DirectivesIntegration'); + + return $factory->createSchema(); + } + + public function testSchemaPrintsDirectiveDefinitions(): void + { + $schema = $this->buildSchema(); + $sdl = SchemaPrinter::doPrint($schema); + + // Definitions: the webonyx printer emits these once the directives are registered on the + // schema (see SchemaFactory's wiring of DirectiveRegistry::webonyxDirectives()). + $this->assertStringContainsString('directive @uppercase on FIELD_DEFINITION', $sdl); + $this->assertStringContainsString('Marks a field for audit-log tracking.', $sdl); + $this->assertStringContainsString('directive @audit(reason: String!) repeatable on FIELD_DEFINITION', $sdl); + $this->assertStringContainsString('directive @tagged(name: String!) on OBJECT', $sdl); + $this->assertStringContainsString('directive @sanitized on INPUT_FIELD_DEFINITION', $sdl); + $this->assertStringContainsString('Marks an input with a schema version for backwards-compat tracking.', $sdl); + $this->assertStringContainsString('directive @versioned(version: Int!) on INPUT_OBJECT', $sdl); + + // No assertions on directive applications here; webonyx's SchemaPrinter doesn't render them. + // See the class docblock. + + // @oneOf is webonyx's built-in: it prints the application from the isOneOf flag, and we + // don't re-declare it in the custom list. + $this->assertStringNotContainsString('directive @oneOf ', $sdl); + $this->assertStringContainsString('input OneOfLookupInput @oneOf', $sdl); + } + + public function testIntrospectionExposesEveryDirective(): void + { + $schema = $this->buildSchema(); + + $result = GraphQL::executeQuery($schema, Introspection::getIntrospectionQuery())->toArray(); + $this->assertArrayNotHasKey('errors', $result); + + $directives = $result['data']['__schema']['directives']; + assert(is_array($directives)); + + $names = []; + foreach ($directives as $directive) { + $names[] = $directive['name']; + } + + // Built-ins remain present alongside custom directives. + $this->assertContains('skip', $names); + $this->assertContains('include', $names); + $this->assertContains('deprecated', $names); + + // Custom directives present. + $this->assertContains('uppercase', $names); + $this->assertContains('audit', $names); + $this->assertContains('tagged', $names); + $this->assertContains('versioned', $names); + $this->assertContains('sanitized', $names); + } + + public function testFieldDirectiveWrapsResolver(): void + { + $schema = $this->buildSchema(); + + $result = GraphQL::executeQuery($schema, '{ tagline }')->toArray(); + $this->assertArrayNotHasKey('errors', $result); + $this->assertSame('HELLO WORLD', $result['data']['tagline']); + } + + public function testInputObjectFieldsResolveWithDirectiveAttached(): void + { + $schema = $this->buildSchema(); + + $result = GraphQL::executeQuery( + $schema, + '{ findWidget(lookup: { sku: "abc" }) { label } }', + )->toArray(); + + $this->assertArrayNotHasKey('errors', $result); + // getLabel() carries @uppercase, so "abc" comes back uppercased. + $this->assertSame('ABC', $result['data']['findWidget']['label']); + } + + public function testFieldDirectiveUppercasesMixedCaseValue(): void + { + $schema = $this->buildSchema(); + + $result = GraphQL::executeQuery( + $schema, + '{ findWidget(lookup: { sku: "MixedCase" }) { label } }', + )->toArray(); + + $this->assertArrayNotHasKey('errors', $result); + $this->assertSame('MIXEDCASE', $result['data']['findWidget']['label']); + } + + public function testRepeatableDirectiveAttachesOncePerUsageToFieldAst(): void + { + $widget = $this->buildSchema()->getType('Widget'); + assert($widget instanceof ObjectType); + + $astNode = $widget->getField('label')->astNode; + $this->assertNotNull($astNode); + + $names = []; + foreach ($astNode->directives as $directive) { + $names[] = $directive->name->value; + } + + // getLabel() carries @uppercase once and the repeatable @audit twice; every application + // survives onto the field's AST node. + $this->assertContains('uppercase', $names); + $this->assertSame(2, count(array_filter($names, static fn (string $name) => $name === 'audit'))); + } +} From 295d363bbf158bb03bba569436b7763ac23f64af Mon Sep 17 00:00:00 2001 From: Michael Georgiadis <38563912+michael-georgiadis@users.noreply.github.com> Date: Mon, 6 Jul 2026 13:47:56 +0200 Subject: [PATCH 4/7] Split validation to separate units --- src/Directives/DirectiveValidator.php | 71 +++---------------- .../Validation/AttributeTargetValidator.php | 53 ++++++++++++++ .../Validation/InterfaceLocationValidator.php | 56 +++++++++++++++ 3 files changed, 117 insertions(+), 63 deletions(-) create mode 100644 src/Directives/Validation/AttributeTargetValidator.php create mode 100644 src/Directives/Validation/InterfaceLocationValidator.php diff --git a/src/Directives/DirectiveValidator.php b/src/Directives/DirectiveValidator.php index dfa21667c1..e3dbbcd5a5 100644 --- a/src/Directives/DirectiveValidator.php +++ b/src/Directives/DirectiveValidator.php @@ -7,6 +7,8 @@ use Attribute; use ReflectionClass; use TheCodingMachine\GraphQLite\Directives\Exceptions\InvalidDirectiveException; +use TheCodingMachine\GraphQLite\Directives\Validation\AttributeTargetValidator; +use TheCodingMachine\GraphQLite\Directives\Validation\InterfaceLocationValidator; use function in_array; use function is_a; @@ -14,9 +16,10 @@ /** * Validates directives at two points: * - * - {@see self::validate()} at discovery time: the `#[Attribute(...)]` PHP target covers every - * declared GraphQL location, and each family interface has a matching location declared (and - * vice versa). + * - {@see self::validate()} at discovery time, delegating to {@see AttributeTargetValidator} (the + * `#[Attribute(...)]` PHP targets cover every declared GraphQL location) and + * {@see InterfaceLocationValidator} (each family interface has a matching location, and vice + * versa). * - {@see self::assertDirectivesUsableAt()} at apply time: a directive placed on a class is * allowed at that class's location. PHP's `#[Attribute]` targets can't tell a `#[Type]` class * from an `#[Input]` class (both are `TARGET_CLASS`), so `#[OneOf]` could be placed on the wrong @@ -42,8 +45,8 @@ public static function validate(string $directiveClass, DirectiveDefinition $def throw InvalidDirectiveException::notAttribute($directiveClass); } - self::checkPhpTargets($directiveClass, $definition, DirectiveReflection::attributeFlags($reflection)); - self::checkInterfaceAndLocationAgreement($directiveClass, $definition, $reflection); + AttributeTargetValidator::validate($directiveClass, $definition, DirectiveReflection::attributeFlags($reflection)); + InterfaceLocationValidator::validate($directiveClass, $definition, $reflection); } /** @@ -67,62 +70,4 @@ public static function assertDirectivesUsableAt(ReflectionClass $refClass, Direc throw InvalidDirectiveException::notUsableAtLocation($directiveClass, $location, $locations); } } - - private static function checkPhpTargets(string $directiveClass, DirectiveDefinition $definition, int $phpFlags): void - { - foreach ($definition->locations as $location) { - foreach (self::requiredPhpTargetsFor($location) as $requiredTarget => $label) { - if (($phpFlags & $requiredTarget) === $requiredTarget) { - continue; - } - - throw InvalidDirectiveException::phpTargetMissingForLocation($directiveClass, $location, $label); - } - } - } - - /** @param ReflectionClass $reflection */ - private static function checkInterfaceAndLocationAgreement(string $directiveClass, DirectiveDefinition $definition, ReflectionClass $reflection): void - { - $locations = $definition->locations; - $interfacePairs = [ - FieldDirective::class => DirectiveLocation::FIELD_DEFINITION, - InputFieldDirective::class => DirectiveLocation::INPUT_FIELD_DEFINITION, - ObjectTypeDirective::class => DirectiveLocation::OBJECT, - InputObjectTypeDirective::class => DirectiveLocation::INPUT_OBJECT, - ]; - - foreach ($interfacePairs as $interface => $expectedLocation) { - $implements = $reflection->implementsInterface($interface); - $declaresLocation = in_array($expectedLocation, $locations, true); - - if ($implements && ! $declaresLocation) { - throw InvalidDirectiveException::interfaceWithoutMatchingLocation($directiveClass, $interface, $locations); - } - - if (! $implements && $declaresLocation) { - throw InvalidDirectiveException::locationWithoutMatchingInterface($directiveClass, $expectedLocation, $interface); - } - } - } - - /** @return array map of Attribute::TARGET_* flag → human-readable label */ - private static function requiredPhpTargetsFor(DirectiveLocation $location): array - { - return match ($location) { - DirectiveLocation::FIELD_DEFINITION => [ - Attribute::TARGET_METHOD => 'TARGET_METHOD', - Attribute::TARGET_PROPERTY => 'TARGET_PROPERTY', - ], - DirectiveLocation::INPUT_FIELD_DEFINITION => [ - Attribute::TARGET_METHOD => 'TARGET_METHOD', - Attribute::TARGET_PROPERTY => 'TARGET_PROPERTY', - Attribute::TARGET_PARAMETER => 'TARGET_PARAMETER', - ], - DirectiveLocation::OBJECT => [Attribute::TARGET_CLASS => 'TARGET_CLASS'], - DirectiveLocation::INPUT_OBJECT => [Attribute::TARGET_CLASS => 'TARGET_CLASS'], - // Other locations don't have apply hooks yet, so there's nothing to enforce. - default => [], - }; - } } diff --git a/src/Directives/Validation/AttributeTargetValidator.php b/src/Directives/Validation/AttributeTargetValidator.php new file mode 100644 index 0000000000..639c7c07e8 --- /dev/null +++ b/src/Directives/Validation/AttributeTargetValidator.php @@ -0,0 +1,53 @@ +locations as $location) { + foreach (self::requiredPhpTargetsFor($location) as $requiredTarget => $label) { + if (($phpFlags & $requiredTarget) === $requiredTarget) { + continue; + } + + throw InvalidDirectiveException::phpTargetMissingForLocation($directiveClass, $location, $label); + } + } + } + + /** @return array map of Attribute::TARGET_* flag → human-readable label */ + private static function requiredPhpTargetsFor(DirectiveLocation $location): array + { + return match ($location) { + DirectiveLocation::FIELD_DEFINITION => [ + Attribute::TARGET_METHOD => 'TARGET_METHOD', + Attribute::TARGET_PROPERTY => 'TARGET_PROPERTY', + ], + DirectiveLocation::INPUT_FIELD_DEFINITION => [ + Attribute::TARGET_METHOD => 'TARGET_METHOD', + Attribute::TARGET_PROPERTY => 'TARGET_PROPERTY', + Attribute::TARGET_PARAMETER => 'TARGET_PARAMETER', + ], + DirectiveLocation::OBJECT => [Attribute::TARGET_CLASS => 'TARGET_CLASS'], + DirectiveLocation::INPUT_OBJECT => [Attribute::TARGET_CLASS => 'TARGET_CLASS'], + // Other locations don't have apply hooks yet, so there's nothing to enforce. + default => [], + }; + } +} diff --git a/src/Directives/Validation/InterfaceLocationValidator.php b/src/Directives/Validation/InterfaceLocationValidator.php new file mode 100644 index 0000000000..9c19923084 --- /dev/null +++ b/src/Directives/Validation/InterfaceLocationValidator.php @@ -0,0 +1,56 @@ + $reflection + * + * @throws InvalidDirectiveException + */ + public static function validate(string $directiveClass, DirectiveDefinition $definition, ReflectionClass $reflection): void + { + $locations = $definition->locations; + $interfacePairs = [ + FieldDirective::class => DirectiveLocation::FIELD_DEFINITION, + InputFieldDirective::class => DirectiveLocation::INPUT_FIELD_DEFINITION, + ObjectTypeDirective::class => DirectiveLocation::OBJECT, + InputObjectTypeDirective::class => DirectiveLocation::INPUT_OBJECT, + ]; + + foreach ($interfacePairs as $interface => $expectedLocation) { + $implements = $reflection->implementsInterface($interface); + $declaresLocation = in_array($expectedLocation, $locations, true); + + if ($implements && ! $declaresLocation) { + throw InvalidDirectiveException::interfaceWithoutMatchingLocation($directiveClass, $interface, $locations); + } + + if (! $implements && $declaresLocation) { + throw InvalidDirectiveException::locationWithoutMatchingInterface($directiveClass, $expectedLocation, $interface); + } + } + } +} From e163c383fa5c4e63bc1e1c614da890c75dead3f4 Mon Sep 17 00:00:00 2001 From: Michael Georgiadis <38563912+michael-georgiadis@users.noreply.github.com> Date: Mon, 6 Jul 2026 14:14:09 +0200 Subject: [PATCH 5/7] Improve handling of edge case and remove a now unused method --- src/Directives/DirectiveRegistry.php | 23 +++++++++++----- .../Exceptions/InvalidDirectiveException.php | 10 +------ tests/Directives/DirectiveRegistryTest.php | 12 +++++++++ .../Invalid/WebonyxReservedNameDirective.php | 26 +++++++++++++++++++ 4 files changed, 55 insertions(+), 16 deletions(-) create mode 100644 tests/Fixtures/Directives/Invalid/WebonyxReservedNameDirective.php diff --git a/src/Directives/DirectiveRegistry.php b/src/Directives/DirectiveRegistry.php index 021521601d..5948e63312 100644 --- a/src/Directives/DirectiveRegistry.php +++ b/src/Directives/DirectiveRegistry.php @@ -13,8 +13,7 @@ use TheCodingMachine\GraphQLite\Directives\Exceptions\InvalidDirectiveException; use function array_key_exists; -use function assert; -use function method_exists; +use function in_array; /** * Holds the directives known to a schema: the user-defined directives discovered in the configured @@ -44,6 +43,17 @@ final class DirectiveRegistry Deprecated::class, ]; + /** + * Directive names webonyx declares itself that GraphQLite doesn't bind (the execution directives + * and @specifiedBy). A custom directive can't claim these: there's no bundled binding to override + * and emitting our own would clash with webonyx's at schema build. + */ + private const RESERVED_WEBONYX_NAMES = [ + WebonyxDirective::SKIP_NAME, + WebonyxDirective::INCLUDE_NAME, + WebonyxDirective::SPECIFIED_BY_NAME, + ]; + public function __construct( private readonly AnnotationReader $annotationReader, private readonly DirectiveClassFinder $classFinder, @@ -76,15 +86,14 @@ private function register(string $directiveClass): void return; } - if (! method_exists($directiveClass, 'definition')) { - throw InvalidDirectiveException::noDefinitionMethod($directiveClass); - } - $definition = $directiveClass::definition(); - assert($definition instanceof DirectiveDefinition); DirectiveValidator::validate($directiveClass, $definition); + if (! $definition->builtIn && in_array($definition->name, self::RESERVED_WEBONYX_NAMES, true)) { + throw InvalidDirectiveException::reservedName($definition->name, $directiveClass); + } + if (! $definition->builtIn && $this->isReservedBuiltInName($definition->name)) { throw InvalidDirectiveException::reservedName($definition->name, $directiveClass); } diff --git a/src/Directives/Exceptions/InvalidDirectiveException.php b/src/Directives/Exceptions/InvalidDirectiveException.php index e032900b39..2e24582604 100644 --- a/src/Directives/Exceptions/InvalidDirectiveException.php +++ b/src/Directives/Exceptions/InvalidDirectiveException.php @@ -103,17 +103,9 @@ public static function duplicateName(string $name, string $existingClass, string public static function reservedName(string $name, string $directiveClass): self { return new self(sprintf( - 'Directive "%s" declares the name "@%s" which is reserved for a webonyx built-in directive (@skip, @include, @deprecated). Pick a different name.', + 'Directive "%s" declares the name "@%s", which is reserved for a built-in directive. Pick a different name, or set builtIn: true on its definition to intentionally override the built-in.', $directiveClass, $name, )); } - - public static function noDefinitionMethod(string $directiveClass): self - { - return new self(sprintf( - 'Directive "%s" implements DirectiveInterface but does not declare a static `definition(): DirectiveDefinition` method.', - $directiveClass, - )); - } } diff --git a/tests/Directives/DirectiveRegistryTest.php b/tests/Directives/DirectiveRegistryTest.php index ddebc37366..f840c9eeb5 100644 --- a/tests/Directives/DirectiveRegistryTest.php +++ b/tests/Directives/DirectiveRegistryTest.php @@ -18,6 +18,7 @@ use TheCodingMachine\GraphQLite\Discovery\StaticClassFinder; use TheCodingMachine\GraphQLite\Fixtures\Directives\InternalFieldDirective; use TheCodingMachine\GraphQLite\Fixtures\Directives\Invalid\ReservedNameDirective; +use TheCodingMachine\GraphQLite\Fixtures\Directives\Invalid\WebonyxReservedNameDirective; use TheCodingMachine\GraphQLite\Fixtures\Directives\MisusedOneOfOnType; use TheCodingMachine\GraphQLite\Fixtures\Directives\NoteFieldDirective; use TheCodingMachine\GraphQLite\Fixtures\Directives\RevisionInputObjectDirective; @@ -109,6 +110,17 @@ public function testRejectsCustomDirectiveUsingReservedName(): void $registry->discover(); } + public function testRejectsCustomDirectiveUsingWebonyxReservedName(): void + { + // @skip is a webonyx built-in GraphQLite doesn't bind, so it can't be claimed at all. + $registry = self::buildRegistry([WebonyxReservedNameDirective::class]); + + $this->expectException(InvalidDirectiveException::class); + $this->expectExceptionMessageMatches('/reserved/'); + + $registry->discover(); + } + public function testEmptyRegistryReportsNoDirectives(): void { $registry = self::buildRegistry([]); diff --git a/tests/Fixtures/Directives/Invalid/WebonyxReservedNameDirective.php b/tests/Fixtures/Directives/Invalid/WebonyxReservedNameDirective.php new file mode 100644 index 0000000000..7ab7319a8b --- /dev/null +++ b/tests/Fixtures/Directives/Invalid/WebonyxReservedNameDirective.php @@ -0,0 +1,26 @@ + Date: Mon, 6 Jul 2026 14:14:24 +0200 Subject: [PATCH 6/7] Enrich E2E test --- tests/Integration/DirectivesEndToEndTest.php | 59 ++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/tests/Integration/DirectivesEndToEndTest.php b/tests/Integration/DirectivesEndToEndTest.php index 3ce52389c2..fa5cb1581a 100644 --- a/tests/Integration/DirectivesEndToEndTest.php +++ b/tests/Integration/DirectivesEndToEndTest.php @@ -5,6 +5,8 @@ namespace TheCodingMachine\GraphQLite\Integration; use GraphQL\GraphQL; +use GraphQL\Language\AST\DirectiveNode; +use GraphQL\Type\Definition\InputObjectType; use GraphQL\Type\Definition\ObjectType; use GraphQL\Type\Introspection; use GraphQL\Utils\SchemaPrinter; @@ -148,4 +150,61 @@ public function testRepeatableDirectiveAttachesOncePerUsageToFieldAst(): void $this->assertContains('uppercase', $names); $this->assertSame(2, count(array_filter($names, static fn (string $name) => $name === 'audit'))); } + + public function testMetadataDirectivesAttachToTheirElementsAst(): void + { + $schema = $this->buildSchema(); + + // Object, input-object and input-field placements land on each element's AST node with their + // arguments, even though the SDL printer doesn't render applications. + $widget = $schema->getType('Widget'); + assert($widget instanceof ObjectType); + $this->assertSame(['tagged' => ['name' => 'primary']], self::astDirectives($widget->astNode?->directives ?? [])); + + $lookup = $schema->getType('WidgetLookupInput'); + assert($lookup instanceof InputObjectType); + $this->assertSame(['versioned' => ['version' => '2']], self::astDirectives($lookup->astNode?->directives ?? [])); + + $sku = $lookup->getField('sku')->astNode; + $this->assertNotNull($sku); + $this->assertSame(['sanitized' => []], self::astDirectives($sku->directives)); + } + + public function testOneOfInputEnforcesExactlyOneField(): void + { + $schema = $this->buildSchema(); + + // Exactly one field resolves (and @uppercase still applies to the returned label). + $ok = GraphQL::executeQuery($schema, '{ findOneOf(lookup: { sku: "widget-1" }) { label } }')->toArray(); + $this->assertArrayNotHasKey('errors', $ok); + $this->assertSame('WIDGET-1', $ok['data']['findOneOf']['label']); + + // Two fields violates @oneOf. + $both = GraphQL::executeQuery($schema, '{ findOneOf(lookup: { sku: "a", id: 1 }) { label } }')->toArray(); + $this->assertArrayHasKey('errors', $both); + $this->assertStringContainsString('exactly one field', $both['errors'][0]['message']); + + // Zero fields also violates @oneOf. + $none = GraphQL::executeQuery($schema, '{ findOneOf(lookup: {}) { label } }')->toArray(); + $this->assertArrayHasKey('errors', $none); + } + + /** + * @param iterable $directives + * + * @return array> directive name => [argument name => literal value] + */ + private static function astDirectives(iterable $directives): array + { + $result = []; + foreach ($directives as $directive) { + $args = []; + foreach ($directive->arguments as $argument) { + $args[$argument->name->value] = $argument->value->value; + } + $result[$directive->name->value] = $args; + } + + return $result; + } } From a1c40c80a58f6fc86f50091fba7378a514e5c1ec Mon Sep 17 00:00:00 2001 From: Michael Georgiadis <38563912+michael-georgiadis@users.noreply.github.com> Date: Mon, 6 Jul 2026 16:52:11 +0200 Subject: [PATCH 7/7] Trim comments --- src/Directives/DirectiveRegistry.php | 39 ++++--------------- src/Directives/DirectiveValidator.php | 27 ++----------- .../Discovery/DirectiveClassFinder.php | 7 +--- .../Discovery/GlobDirectivesCache.php | 3 +- .../Validation/AttributeTargetValidator.php | 8 ++-- .../Validation/InterfaceLocationValidator.php | 11 ++---- tests/Directives/BuiltInDirectivesTest.php | 7 ++-- tests/Directives/DirectiveRegistryTest.php | 8 ++-- .../Directives/CustomOneOfOverride.php | 5 +-- .../Directives/InternalFieldDirective.php | 5 +-- .../InterfaceWithoutLocationDirective.php | 5 +-- .../Invalid/MissingPhpTargetDirective.php | 5 +-- .../Invalid/ReservedNameDirective.php | 4 +- .../UnsupportedArgumentTypeDirective.php | 5 +-- .../Invalid/WebonyxReservedNameDirective.php | 5 +-- .../Directives/NoteFieldDirective.php | 5 +-- .../RevisionInputObjectDirective.php | 5 +-- .../Directives/VersionedDirective.php | 5 +-- .../Inputs/OneOfLookup.php | 5 +-- tests/Integration/DirectivesEndToEndTest.php | 33 +++++----------- 20 files changed, 48 insertions(+), 149 deletions(-) diff --git a/src/Directives/DirectiveRegistry.php b/src/Directives/DirectiveRegistry.php index 5948e63312..fcab2e0dc2 100644 --- a/src/Directives/DirectiveRegistry.php +++ b/src/Directives/DirectiveRegistry.php @@ -16,13 +16,8 @@ use function in_array; /** - * Holds the directives known to a schema: the user-defined directives discovered in the configured - * namespaces plus the built-ins that bind PHP behavior to directives webonyx already declares - * (`@oneOf`, `@deprecated`). For each discovered class it runs {@see DirectiveValidator}, caches what - * {@see DirectiveResolver} produces, and enforces name uniqueness across the set. - * - * The dispatcher middlewares query it at apply time for a directive's argument shape, which the AST - * builder needs to encode each arg as a GraphQL value. + * Discovers, validates and holds a schema's directives: the user-defined ones plus the built-ins + * (`@oneOf`, `@deprecated`). Middlewares query it at apply time for a directive's argument shape. */ final class DirectiveRegistry { @@ -32,22 +27,13 @@ final class DirectiveRegistry /** @var array> */ private array $classByName = []; - /** - * Attribute classes that bind PHP behavior to webonyx's pre-existing built-in directives. - * Also the source of truth for reserved names: a custom (non-built-in) directive can't claim - * one of these names unless it declares `builtIn: true` to override the bundled binding. - * Registered after user discovery so such an override wins. - */ + /** Attributes we bind to webonyx's built-ins; a custom directive reuses these names only via `builtIn: true`. */ private const BUILT_IN_ATTRIBUTES = [ OneOf::class, Deprecated::class, ]; - /** - * Directive names webonyx declares itself that GraphQLite doesn't bind (the execution directives - * and @specifiedBy). A custom directive can't claim these: there's no bundled binding to override - * and emitting our own would clash with webonyx's at schema build. - */ + /** webonyx directives we don't bind; no custom directive may take these names. */ private const RESERVED_WEBONYX_NAMES = [ WebonyxDirective::SKIP_NAME, WebonyxDirective::INCLUDE_NAME, @@ -60,11 +46,9 @@ public function __construct( ) { } - /** Discover + validate the user directives, then the built-ins. Idempotent. */ + /** Idempotent. User directives first, so a `builtIn: true` override lands before the bundled copy. */ public function discover(): void { - // User classes first: an override of a built-in (same name, builtIn: true) needs to land - // before our bundled copy registers. foreach ($this->classFinder->findDirectives() as $directiveClass) { $this->register($directiveClass); } @@ -73,15 +57,10 @@ public function discover(): void } } - /** - * @param class-string $directiveClass - * - * @throws InvalidDirectiveException - */ + /** @param class-string $directiveClass */ private function register(string $directiveClass): void { - // Registering the same class twice is a no-op; discovery and the built-in list share this - // registry, so duplicates are expected. + // Discovery and the built-in list overlap, so a repeat is expected. if (isset($this->resolvedByClass[$directiveClass])) { return; } @@ -99,8 +78,7 @@ private function register(string $directiveClass): void } if (array_key_exists($definition->name, $this->classByName)) { - // A name clash is fine when this side is also built-in: the user supplied their own - // implementation and we defer to it. Two custom directives sharing a name is an error. + // A builtIn override defers to the already-registered user class; two customs is an error. if ($definition->builtIn) { return; } @@ -115,7 +93,6 @@ private function register(string $directiveClass): void $this->classByName[$definition->name] = $directiveClass; } - /** Whether $name is bound by one of our built-in attributes, so only a `builtIn: true` override may take it. */ private function isReservedBuiltInName(string $name): bool { foreach (self::BUILT_IN_ATTRIBUTES as $builtInClass) { diff --git a/src/Directives/DirectiveValidator.php b/src/Directives/DirectiveValidator.php index e3dbbcd5a5..44af2a718e 100644 --- a/src/Directives/DirectiveValidator.php +++ b/src/Directives/DirectiveValidator.php @@ -14,29 +14,14 @@ use function is_a; /** - * Validates directives at two points: - * - * - {@see self::validate()} at discovery time, delegating to {@see AttributeTargetValidator} (the - * `#[Attribute(...)]` PHP targets cover every declared GraphQL location) and - * {@see InterfaceLocationValidator} (each family interface has a matching location, and vice - * versa). - * - {@see self::assertDirectivesUsableAt()} at apply time: a directive placed on a class is - * allowed at that class's location. PHP's `#[Attribute]` targets can't tell a `#[Type]` class - * from an `#[Input]` class (both are `TARGET_CLASS`), so `#[OneOf]` could be placed on the wrong - * kind of class; this reports it instead of letting the collectors drop it silently. - * - * Producing the arguments, repeatability, and webonyx directive is {@see DirectiveResolver}'s job; - * name uniqueness is checked in {@see DirectiveRegistry}. + * Validates a directive at discovery time ({@see self::validate()}, delegating target and + * interface/location checks) and its placement at apply time ({@see self::assertDirectivesUsableAt()}). * * @internal */ final class DirectiveValidator { - /** - * @param class-string $directiveClass - * - * @throws InvalidDirectiveException - */ + /** @param class-string $directiveClass */ public static function validate(string $directiveClass, DirectiveDefinition $definition): void { $reflection = new ReflectionClass($directiveClass); @@ -49,11 +34,7 @@ public static function validate(string $directiveClass, DirectiveDefinition $def InterfaceLocationValidator::validate($directiveClass, $definition, $reflection); } - /** - * @param ReflectionClass $refClass - * - * @throws InvalidDirectiveException when a directive on $refClass isn't allowed at $location. - */ + /** @param ReflectionClass $refClass */ public static function assertDirectivesUsableAt(ReflectionClass $refClass, DirectiveLocation $location): void { foreach ($refClass->getAttributes() as $attribute) { diff --git a/src/Directives/Discovery/DirectiveClassFinder.php b/src/Directives/Discovery/DirectiveClassFinder.php index d0f9d09582..639215ff64 100644 --- a/src/Directives/Discovery/DirectiveClassFinder.php +++ b/src/Directives/Discovery/DirectiveClassFinder.php @@ -12,11 +12,8 @@ use function array_reduce; /** - * Finds the classes in the configured namespaces that implement {@see TypeSystemDirective}. - * - * Same pattern as {@see \TheCodingMachine\GraphQLite\Mappers\ClassFinderTypeMapper}: map each class - * from {@see ClassFinder} to a {@see GlobDirectivesCache} entry (or null), then collect the FQCNs. - * {@see ClassFinderComputedCache} handles cache invalidation in dev mode. + * Finds classes in the configured namespaces that implement {@see TypeSystemDirective}, cached the + * same way as {@see \TheCodingMachine\GraphQLite\Mappers\ClassFinderTypeMapper}. * * @internal */ diff --git a/src/Directives/Discovery/GlobDirectivesCache.php b/src/Directives/Discovery/GlobDirectivesCache.php index d587eaba1c..f1c2c711f7 100644 --- a/src/Directives/Discovery/GlobDirectivesCache.php +++ b/src/Directives/Discovery/GlobDirectivesCache.php @@ -7,8 +7,7 @@ use TheCodingMachine\GraphQLite\Directives\TypeSystemDirective; /** - * Cache entry for one file scanned by {@see DirectiveClassFinder}, holding the FQCN of the directive - * class it found. Files with no directive class get no entry. + * Cache entry for a directive class {@see DirectiveClassFinder} found. * * @internal */ diff --git a/src/Directives/Validation/AttributeTargetValidator.php b/src/Directives/Validation/AttributeTargetValidator.php index 639c7c07e8..950d1cbc1d 100644 --- a/src/Directives/Validation/AttributeTargetValidator.php +++ b/src/Directives/Validation/AttributeTargetValidator.php @@ -10,14 +10,12 @@ use TheCodingMachine\GraphQLite\Directives\Exceptions\InvalidDirectiveException; /** - * Checks that a directive's `#[Attribute(...)]` PHP targets cover every GraphQL location it declares - * (e.g. a FIELD_DEFINITION directive must allow both TARGET_METHOD and TARGET_PROPERTY). + * Checks a directive's `#[Attribute(...)]` targets cover every GraphQL location it declares. * * @internal */ final class AttributeTargetValidator { - /** @throws InvalidDirectiveException */ public static function validate(string $directiveClass, DirectiveDefinition $definition, int $phpFlags): void { foreach ($definition->locations as $location) { @@ -31,7 +29,7 @@ public static function validate(string $directiveClass, DirectiveDefinition $def } } - /** @return array map of Attribute::TARGET_* flag → human-readable label */ + /** @return array required TARGET_* flag => label */ private static function requiredPhpTargetsFor(DirectiveLocation $location): array { return match ($location) { @@ -46,7 +44,7 @@ private static function requiredPhpTargetsFor(DirectiveLocation $location): arra ], DirectiveLocation::OBJECT => [Attribute::TARGET_CLASS => 'TARGET_CLASS'], DirectiveLocation::INPUT_OBJECT => [Attribute::TARGET_CLASS => 'TARGET_CLASS'], - // Other locations don't have apply hooks yet, so there's nothing to enforce. + // No apply hooks for the other locations yet. default => [], }; } diff --git a/src/Directives/Validation/InterfaceLocationValidator.php b/src/Directives/Validation/InterfaceLocationValidator.php index 9c19923084..d473fab3d3 100644 --- a/src/Directives/Validation/InterfaceLocationValidator.php +++ b/src/Directives/Validation/InterfaceLocationValidator.php @@ -17,19 +17,14 @@ use function in_array; /** - * Checks that a directive's family interfaces and its declared GraphQL locations agree: implementing - * FieldDirective requires the FIELD_DEFINITION location, and vice versa (same for the input-field, - * object and input-object families). + * Checks a directive's family interfaces and declared locations agree (FieldDirective and + * FIELD_DEFINITION imply each other, likewise for the input-field, object and input-object families). * * @internal */ final class InterfaceLocationValidator { - /** - * @param ReflectionClass $reflection - * - * @throws InvalidDirectiveException - */ + /** @param ReflectionClass $reflection */ public static function validate(string $directiveClass, DirectiveDefinition $definition, ReflectionClass $reflection): void { $locations = $definition->locations; diff --git a/tests/Directives/BuiltInDirectivesTest.php b/tests/Directives/BuiltInDirectivesTest.php index 320f538b28..02026c437d 100644 --- a/tests/Directives/BuiltInDirectivesTest.php +++ b/tests/Directives/BuiltInDirectivesTest.php @@ -32,7 +32,7 @@ public function testBuiltInDirectivesAreNotEmittedAsCustomDirectives(): void $registry = self::buildRegistry([]); $registry->discover(); - // @oneOf is a webonyx built-in, so the registry shouldn't add it to the custom list. + // @oneOf is a webonyx built-in, not part of the custom list. $names = array_map(static fn (WebonyxDirective $d) => $d->name, $registry->webonyxDirectives()); $this->assertSame([], $names); @@ -50,8 +50,7 @@ public function testOneOfDefinitionMarksItselfAsBuiltIn(): void public function testDiscoveryFindingTheBundledBuiltInClassIsIdempotent(): void { - // If discovery finds our own built-in class (same FQCN as BUILT_IN_ATTRIBUTES), registering - // it twice shouldn't throw. + // Discovery finding our own built-in shouldn't double-register. $registry = self::buildRegistry([OneOf::class]); $registry->discover(); @@ -60,7 +59,7 @@ public function testDiscoveryFindingTheBundledBuiltInClassIsIdempotent(): void public function testUserOverrideOfBuiltInWinsOverBundled(): void { - // User binds their own class to @oneOf (builtIn: true); ours should defer to it. + // User override wins over the bundled OneOf. $registry = self::buildRegistry([CustomOneOfOverride::class]); $registry->discover(); diff --git a/tests/Directives/DirectiveRegistryTest.php b/tests/Directives/DirectiveRegistryTest.php index f840c9eeb5..9b03735428 100644 --- a/tests/Directives/DirectiveRegistryTest.php +++ b/tests/Directives/DirectiveRegistryTest.php @@ -37,8 +37,7 @@ public function testRegistersTheBuiltInDirectives(): void public function testBuiltInsAreNotAddedToTheSchemaDirectiveList(): void { - // webonyx already declares @oneOf and @deprecated, so the registry contributes nothing to - // SchemaConfig::$directives. + // webonyx already declares these, so the registry adds nothing to the schema. $registry = $this->registry(); $this->assertSame([], $registry->webonyxDirectives()); @@ -112,7 +111,7 @@ public function testRejectsCustomDirectiveUsingReservedName(): void public function testRejectsCustomDirectiveUsingWebonyxReservedName(): void { - // @skip is a webonyx built-in GraphQLite doesn't bind, so it can't be claimed at all. + // @skip is a webonyx built-in we don't bind. $registry = self::buildRegistry([WebonyxReservedNameDirective::class]); $this->expectException(InvalidDirectiveException::class); @@ -132,8 +131,7 @@ public function testEmptyRegistryReportsNoDirectives(): void public function testWebonyxDirectivesIsolatedFromBuiltins(): void { - // The built directives shouldn't include webonyx's built-ins; those get merged in at the - // Schema layer, not here. + // Built-ins merge in at the Schema layer, not here. $registry = self::buildRegistry([InternalFieldDirective::class]); $registry->discover(); diff --git a/tests/Fixtures/Directives/CustomOneOfOverride.php b/tests/Fixtures/Directives/CustomOneOfOverride.php index c2142821c5..70039bc6ca 100644 --- a/tests/Fixtures/Directives/CustomOneOfOverride.php +++ b/tests/Fixtures/Directives/CustomOneOfOverride.php @@ -12,10 +12,7 @@ use TheCodingMachine\GraphQLite\Middlewares\InputObjectTypeHandlerInterface; use TheCodingMachine\GraphQLite\Types\MutableInputObjectType; -/** - * Stands in for a user-supplied replacement of the bundled `OneOf`. Same `@oneOf` name, marked - * `builtIn: true`, so the registry uses it instead of ours. - */ +/** User replacement for the bundled `OneOf` (`@oneOf`, `builtIn: true`). */ #[Attribute(Attribute::TARGET_CLASS)] final class CustomOneOfOverride implements BehavioralInputObjectTypeDirective { diff --git a/tests/Fixtures/Directives/InternalFieldDirective.php b/tests/Fixtures/Directives/InternalFieldDirective.php index c2bb17bee3..b8fb54e9ca 100644 --- a/tests/Fixtures/Directives/InternalFieldDirective.php +++ b/tests/Fixtures/Directives/InternalFieldDirective.php @@ -9,10 +9,7 @@ use TheCodingMachine\GraphQLite\Directives\DirectiveLocation; use TheCodingMachine\GraphQLite\Directives\FieldDirective; -/** - * A no-argument marker field directive. Unit specimen for the "no args, not repeatable" resolver - * path; deliberately not behavioral (nothing invokes it, we only resolve it). - */ +/** No-arg, non-repeatable marker; specimen for the no-args resolver path. */ #[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_PROPERTY)] final class InternalFieldDirective implements FieldDirective { diff --git a/tests/Fixtures/Directives/Invalid/InterfaceWithoutLocationDirective.php b/tests/Fixtures/Directives/Invalid/InterfaceWithoutLocationDirective.php index 0eb38163c8..d801352bf3 100644 --- a/tests/Fixtures/Directives/Invalid/InterfaceWithoutLocationDirective.php +++ b/tests/Fixtures/Directives/Invalid/InterfaceWithoutLocationDirective.php @@ -8,10 +8,7 @@ use TheCodingMachine\GraphQLite\Directives\DirectiveDefinition; use TheCodingMachine\GraphQLite\Directives\FieldDirective; -/** - * Implements FieldDirective but declares no FIELD_DEFINITION location, so the validator's - * interface/location check rejects it. Empty locations keeps the PHP-target check from firing first. - */ +/** FieldDirective with no FIELD_DEFINITION location; rejected by the interface/location check. */ #[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_PROPERTY)] final class InterfaceWithoutLocationDirective implements FieldDirective { diff --git a/tests/Fixtures/Directives/Invalid/MissingPhpTargetDirective.php b/tests/Fixtures/Directives/Invalid/MissingPhpTargetDirective.php index 85eb29a789..d4006b565c 100644 --- a/tests/Fixtures/Directives/Invalid/MissingPhpTargetDirective.php +++ b/tests/Fixtures/Directives/Invalid/MissingPhpTargetDirective.php @@ -9,10 +9,7 @@ use TheCodingMachine\GraphQLite\Directives\DirectiveLocation; use TheCodingMachine\GraphQLite\Directives\FieldDirective; -/** - * Declares FIELD_DEFINITION but the PHP target is TARGET_CLASS only, so the validator rejects it: - * the PHP target has to cover the GraphQL locations. - */ +/** FIELD_DEFINITION with a TARGET_CLASS-only attribute; rejected by the target check. */ #[Attribute(Attribute::TARGET_CLASS)] final class MissingPhpTargetDirective implements FieldDirective { diff --git a/tests/Fixtures/Directives/Invalid/ReservedNameDirective.php b/tests/Fixtures/Directives/Invalid/ReservedNameDirective.php index 2c587ba700..268fff2c29 100644 --- a/tests/Fixtures/Directives/Invalid/ReservedNameDirective.php +++ b/tests/Fixtures/Directives/Invalid/ReservedNameDirective.php @@ -9,9 +9,7 @@ use TheCodingMachine\GraphQLite\Directives\DirectiveLocation; use TheCodingMachine\GraphQLite\Directives\InputObjectTypeDirective; -/** - * Reuses webonyx's built-in `@oneOf` name, so the registry's reserved-name check rejects it. - */ +/** Reuses `@oneOf`; rejected by the reserved-name check. */ #[Attribute(Attribute::TARGET_CLASS)] final class ReservedNameDirective implements InputObjectTypeDirective { diff --git a/tests/Fixtures/Directives/Invalid/UnsupportedArgumentTypeDirective.php b/tests/Fixtures/Directives/Invalid/UnsupportedArgumentTypeDirective.php index 0e319974e5..2b3991f92a 100644 --- a/tests/Fixtures/Directives/Invalid/UnsupportedArgumentTypeDirective.php +++ b/tests/Fixtures/Directives/Invalid/UnsupportedArgumentTypeDirective.php @@ -10,10 +10,7 @@ use TheCodingMachine\GraphQLite\Directives\DirectiveLocation; use TheCodingMachine\GraphQLite\Directives\FieldDirective; -/** - * Constructor parameter is a non-scalar object, which the validator rejects (args must map to a - * scalar). - */ +/** Non-scalar constructor arg; rejected by the resolver. */ #[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_PROPERTY)] final class UnsupportedArgumentTypeDirective implements FieldDirective { diff --git a/tests/Fixtures/Directives/Invalid/WebonyxReservedNameDirective.php b/tests/Fixtures/Directives/Invalid/WebonyxReservedNameDirective.php index 7ab7319a8b..ffce7f7d98 100644 --- a/tests/Fixtures/Directives/Invalid/WebonyxReservedNameDirective.php +++ b/tests/Fixtures/Directives/Invalid/WebonyxReservedNameDirective.php @@ -9,10 +9,7 @@ use TheCodingMachine\GraphQLite\Directives\DirectiveLocation; use TheCodingMachine\GraphQLite\Directives\FieldDirective; -/** - * A well-formed field directive that claims webonyx's `@skip` name, so the registry's reserved-name - * check rejects it even though it passes validation. - */ +/** Well-formed but claims webonyx's `@skip` name; rejected by the reserved-name check. */ #[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_PROPERTY)] final class WebonyxReservedNameDirective implements FieldDirective { diff --git a/tests/Fixtures/Directives/NoteFieldDirective.php b/tests/Fixtures/Directives/NoteFieldDirective.php index ab541391a0..39ba4fd764 100644 --- a/tests/Fixtures/Directives/NoteFieldDirective.php +++ b/tests/Fixtures/Directives/NoteFieldDirective.php @@ -9,10 +9,7 @@ use TheCodingMachine\GraphQLite\Directives\DirectiveLocation; use TheCodingMachine\GraphQLite\Directives\FieldDirective; -/** - * A repeatable field directive carrying a single required scalar argument. Unit specimen for the - * resolver's argument encoding and repeatable inference. - */ +/** Repeatable field directive with one required scalar arg; resolver specimen. */ #[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_PROPERTY | Attribute::IS_REPEATABLE)] final class NoteFieldDirective implements FieldDirective { diff --git a/tests/Fixtures/Directives/RevisionInputObjectDirective.php b/tests/Fixtures/Directives/RevisionInputObjectDirective.php index 30471328bc..5caf7d12a1 100644 --- a/tests/Fixtures/Directives/RevisionInputObjectDirective.php +++ b/tests/Fixtures/Directives/RevisionInputObjectDirective.php @@ -9,10 +9,7 @@ use TheCodingMachine\GraphQLite\Directives\DirectiveLocation; use TheCodingMachine\GraphQLite\Directives\InputObjectTypeDirective; -/** - * A custom input-object directive with a constructor argument. Metadata only (no apply method), - * used to exercise the `INPUT_OBJECT` path: definition, argument encoding, and SDL output. - */ +/** Input-object directive with one arg; specimen for the `INPUT_OBJECT` path. */ #[Attribute(Attribute::TARGET_CLASS)] final class RevisionInputObjectDirective implements InputObjectTypeDirective { diff --git a/tests/Fixtures/DirectivesIntegration/Directives/VersionedDirective.php b/tests/Fixtures/DirectivesIntegration/Directives/VersionedDirective.php index d6ba8affeb..7f19285eff 100644 --- a/tests/Fixtures/DirectivesIntegration/Directives/VersionedDirective.php +++ b/tests/Fixtures/DirectivesIntegration/Directives/VersionedDirective.php @@ -9,10 +9,7 @@ use TheCodingMachine\GraphQLite\Directives\DirectiveLocation; use TheCodingMachine\GraphQLite\Directives\InputObjectTypeDirective; -/** - * Marks an input object with a schema version. A custom input-object directive used in the - * integration test to check the custom path runs alongside the bundled `#[OneOf]`. - */ +/** Input-object directive tagging a schema version; runs alongside the bundled `#[OneOf]`. */ #[Attribute(Attribute::TARGET_CLASS)] final class VersionedDirective implements InputObjectTypeDirective { diff --git a/tests/Fixtures/DirectivesIntegration/Inputs/OneOfLookup.php b/tests/Fixtures/DirectivesIntegration/Inputs/OneOfLookup.php index ad6b399866..dae3c5cff1 100644 --- a/tests/Fixtures/DirectivesIntegration/Inputs/OneOfLookup.php +++ b/tests/Fixtures/DirectivesIntegration/Inputs/OneOfLookup.php @@ -8,10 +8,7 @@ use TheCodingMachine\GraphQLite\Annotations\Input; use TheCodingMachine\GraphQLite\Directives\BuiltIn\OneOf; -/** - * Uses the built-in `@oneOf` directive, which sets webonyx's `isOneOf` flag so callers pass either - * `sku` or `id`, not both. - */ +/** Uses `@oneOf` so callers pass either `sku` or `id`, not both. */ #[Input] #[OneOf] final class OneOfLookup diff --git a/tests/Integration/DirectivesEndToEndTest.php b/tests/Integration/DirectivesEndToEndTest.php index fa5cb1581a..2d043a2290 100644 --- a/tests/Integration/DirectivesEndToEndTest.php +++ b/tests/Integration/DirectivesEndToEndTest.php @@ -24,13 +24,9 @@ use function is_array; /** - * End-to-end test for custom directives. Builds a schema from the - * `tests/Fixtures/DirectivesIntegration` namespaces and checks that directive definitions show up - * in SDL and introspection, and that behavioral directives wrap their resolver at runtime. - * - * We don't assert directive applications in SDL (e.g. `tagline: String! @uppercase`): webonyx's - * SchemaPrinter doesn't render those, and we follow its behavior. The applications are still on each - * element's `astNode->directives` for anyone who wants to print them with their own printer. + * End-to-end custom directives: builds a schema from the `DirectivesIntegration` fixtures and checks + * definitions (SDL/introspection), runtime behavior, and applications on each element's AST node. + * webonyx's SchemaPrinter doesn't render applications, so those are checked via `astNode->directives`. */ final class DirectivesEndToEndTest extends TestCase { @@ -49,8 +45,6 @@ public function testSchemaPrintsDirectiveDefinitions(): void $schema = $this->buildSchema(); $sdl = SchemaPrinter::doPrint($schema); - // Definitions: the webonyx printer emits these once the directives are registered on the - // schema (see SchemaFactory's wiring of DirectiveRegistry::webonyxDirectives()). $this->assertStringContainsString('directive @uppercase on FIELD_DEFINITION', $sdl); $this->assertStringContainsString('Marks a field for audit-log tracking.', $sdl); $this->assertStringContainsString('directive @audit(reason: String!) repeatable on FIELD_DEFINITION', $sdl); @@ -59,11 +53,7 @@ public function testSchemaPrintsDirectiveDefinitions(): void $this->assertStringContainsString('Marks an input with a schema version for backwards-compat tracking.', $sdl); $this->assertStringContainsString('directive @versioned(version: Int!) on INPUT_OBJECT', $sdl); - // No assertions on directive applications here; webonyx's SchemaPrinter doesn't render them. - // See the class docblock. - - // @oneOf is webonyx's built-in: it prints the application from the isOneOf flag, and we - // don't re-declare it in the custom list. + // @oneOf is webonyx's: printed from the isOneOf flag, not re-declared in the custom list. $this->assertStringNotContainsString('directive @oneOf ', $sdl); $this->assertStringContainsString('input OneOfLookupInput @oneOf', $sdl); } @@ -83,12 +73,11 @@ public function testIntrospectionExposesEveryDirective(): void $names[] = $directive['name']; } - // Built-ins remain present alongside custom directives. + // Built-ins present alongside custom directives. $this->assertContains('skip', $names); $this->assertContains('include', $names); $this->assertContains('deprecated', $names); - // Custom directives present. $this->assertContains('uppercase', $names); $this->assertContains('audit', $names); $this->assertContains('tagged', $names); @@ -115,7 +104,7 @@ public function testInputObjectFieldsResolveWithDirectiveAttached(): void )->toArray(); $this->assertArrayNotHasKey('errors', $result); - // getLabel() carries @uppercase, so "abc" comes back uppercased. + // getLabel() carries @uppercase. $this->assertSame('ABC', $result['data']['findWidget']['label']); } @@ -145,8 +134,7 @@ public function testRepeatableDirectiveAttachesOncePerUsageToFieldAst(): void $names[] = $directive->name->value; } - // getLabel() carries @uppercase once and the repeatable @audit twice; every application - // survives onto the field's AST node. + // @uppercase once, repeatable @audit twice. $this->assertContains('uppercase', $names); $this->assertSame(2, count(array_filter($names, static fn (string $name) => $name === 'audit'))); } @@ -155,8 +143,7 @@ public function testMetadataDirectivesAttachToTheirElementsAst(): void { $schema = $this->buildSchema(); - // Object, input-object and input-field placements land on each element's AST node with their - // arguments, even though the SDL printer doesn't render applications. + // Applications land on each element's AST node, though the printer omits them. $widget = $schema->getType('Widget'); assert($widget instanceof ObjectType); $this->assertSame(['tagged' => ['name' => 'primary']], self::astDirectives($widget->astNode?->directives ?? [])); @@ -174,7 +161,7 @@ public function testOneOfInputEnforcesExactlyOneField(): void { $schema = $this->buildSchema(); - // Exactly one field resolves (and @uppercase still applies to the returned label). + // One field resolves; @uppercase still applies to the label. $ok = GraphQL::executeQuery($schema, '{ findOneOf(lookup: { sku: "widget-1" }) { label } }')->toArray(); $this->assertArrayNotHasKey('errors', $ok); $this->assertSame('WIDGET-1', $ok['data']['findOneOf']['label']); @@ -192,7 +179,7 @@ public function testOneOfInputEnforcesExactlyOneField(): void /** * @param iterable $directives * - * @return array> directive name => [argument name => literal value] + * @return array> name => [arg => value] */ private static function astDirectives(iterable $directives): array {