From 192c78ffad11bf998d87222bfdbd9dd3eda0e7aa Mon Sep 17 00:00:00 2001 From: Jacob Thomason Date: Sat, 18 Apr 2026 01:48:50 -0400 Subject: [PATCH 1/8] feat: explicit description attribute + SchemaFactory docblock toggle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds explicit `description` argument to every schema-defining attribute (Query, Mutation, Subscription, Type, ExtendType, Factory) and a SchemaFactory toggle to disable the docblock-as-description fallback. Addresses #453 and the type-level portion of #740. Behaviour highlights: - Explicit attribute description always wins; `''` deliberately blocks the docblock fallback; `null` falls through when the toggle is on. - `setDocblockDescriptionsEnabled(false)` suppresses every docblock fallback path so internal developer docblocks stop leaking to public schema consumers. - Duplicate descriptions across `#[Type]` + `#[ExtendType]` (or multiple `#[ExtendType]`s on the same class) now throw a DuplicateDescriptionOnTypeException naming every offending source. - `InputTypeGenerator::mapFactoryMethod()` closes the long-standing `// TODO: add comment argument.` — factory-produced input types now participate in description resolution. Consistency renames (breaking): - `QueryFieldDescriptor`/`InputFieldDescriptor` `$comment` -> `$description` and paired method renames (getDescription/withDescription/ withAddedDescriptionLines). - `AbstractRequest` -> `AbstractGraphQLElement` and `AnnotationReader::getRequestAnnotation()` -> `getGraphQLElementAnnotation()`. The new name reflects the class's role as the shared base for GraphQL schema-element attributes. Tests: +26 new (resolver precedence matrix, per-attribute integration, conflict exception, docblock-off regression). Full suite 528 passing across cs-check, phpstan, and phpunit. --- src/AnnotationReader.php | 8 +- src/Annotations/AbstractGraphQLElement.php | 62 +++++++ src/Annotations/AbstractRequest.php | 37 ---- .../DuplicateDescriptionOnTypeException.php | 38 ++++ src/Annotations/ExtendType.php | 16 ++ src/Annotations/Factory.php | 23 ++- src/Annotations/Field.php | 2 +- src/Annotations/Mutation.php | 2 +- src/Annotations/Query.php | 2 +- src/Annotations/Subscription.php | 2 +- src/Annotations/Type.php | 16 ++ src/FieldsBuilder.php | 87 +++++----- src/GlobControllerQueryProvider.php | 6 +- src/InputField.php | 6 +- src/InputFieldDescriptor.php | 10 +- src/InputTypeGenerator.php | 42 ++++- src/Mappers/Parameters/TypeHandler.php | 14 +- src/Mappers/Root/EnumTypeMapper.php | 19 +- src/Middlewares/CostFieldMiddleware.php | 4 +- src/QueryField.php | 8 +- src/QueryFieldDescriptor.php | 18 +- src/SchemaFactory.php | 32 +++- src/TypeGenerator.php | 150 +++++++++++----- .../ResolvableMutableInputObjectType.php | 17 +- src/Utils/DescriptionResolver.php | 60 +++++++ tests/AnnotationReaderTest.php | 4 +- tests/Annotations/ExtendTypeTest.php | 18 ++ tests/Annotations/FactoryTest.php | 18 ++ tests/Annotations/QueryTest.php | 61 +++++++ tests/Annotations/TypeTest.php | 26 +++ tests/Fixtures/Description/Author.php | 30 ++++ .../Fixtures/Description/AuthorExtension.php | 23 +++ tests/Fixtures/Description/Book.php | 29 ++++ .../Description/BookSearchCriteria.php | 13 ++ tests/Fixtures/Description/Genre.php | 18 ++ .../Description/LibraryController.php | 55 ++++++ tests/Fixtures/DescriptionDuplicate/Book.php | 18 ++ .../DescriptionDuplicate/BookExtension.php | 22 +++ .../DuplicateController.php | 16 ++ tests/Integration/DescriptionTest.php | 164 ++++++++++++++++++ .../AuthorizationInputFieldMiddlewareTest.php | 2 +- tests/Middlewares/CostFieldMiddlewareTest.php | 2 +- tests/QueryFieldDescriptorTest.php | 19 +- tests/Utils/DescriptionResolverTest.php | 84 +++++++++ website/docs/CHANGELOG.md | 5 + website/docs/annotations-reference.md | 6 + website/docs/descriptions.md | 106 +++++++++++ website/docs/field-middlewares.md | 5 +- website/docs/other-frameworks.mdx | 5 + website/sidebars.json | 1 + 50 files changed, 1241 insertions(+), 190 deletions(-) create mode 100644 src/Annotations/AbstractGraphQLElement.php delete mode 100644 src/Annotations/AbstractRequest.php create mode 100644 src/Annotations/Exceptions/DuplicateDescriptionOnTypeException.php create mode 100644 src/Utils/DescriptionResolver.php create mode 100644 tests/Annotations/QueryTest.php create mode 100644 tests/Fixtures/Description/Author.php create mode 100644 tests/Fixtures/Description/AuthorExtension.php create mode 100644 tests/Fixtures/Description/Book.php create mode 100644 tests/Fixtures/Description/BookSearchCriteria.php create mode 100644 tests/Fixtures/Description/Genre.php create mode 100644 tests/Fixtures/Description/LibraryController.php create mode 100644 tests/Fixtures/DescriptionDuplicate/Book.php create mode 100644 tests/Fixtures/DescriptionDuplicate/BookExtension.php create mode 100644 tests/Fixtures/DescriptionDuplicate/DuplicateController.php create mode 100644 tests/Integration/DescriptionTest.php create mode 100644 tests/Utils/DescriptionResolverTest.php create mode 100644 website/docs/descriptions.md diff --git a/src/AnnotationReader.php b/src/AnnotationReader.php index 39ebb6706c..bfde61c324 100644 --- a/src/AnnotationReader.php +++ b/src/AnnotationReader.php @@ -8,7 +8,7 @@ use ReflectionMethod; use ReflectionParameter; use ReflectionProperty; -use TheCodingMachine\GraphQLite\Annotations\AbstractRequest; +use TheCodingMachine\GraphQLite\Annotations\AbstractGraphQLElement; use TheCodingMachine\GraphQLite\Annotations\Decorate; use TheCodingMachine\GraphQLite\Annotations\EnumType; use TheCodingMachine\GraphQLite\Annotations\Exceptions\ClassNotFoundException; @@ -201,11 +201,11 @@ public function getEnumTypeAnnotation(ReflectionClass $refClass): EnumType|null return $this->getClassAnnotation($refClass, EnumType::class); } - /** @param class-string $annotationClass */ - public function getRequestAnnotation(ReflectionMethod $refMethod, string $annotationClass): AbstractRequest|null + /** @param class-string $annotationClass */ + public function getGraphQLElementAnnotation(ReflectionMethod $refMethod, string $annotationClass): AbstractGraphQLElement|null { $queryAnnotation = $this->getMethodAnnotation($refMethod, $annotationClass); - assert($queryAnnotation instanceof AbstractRequest || $queryAnnotation === null); + assert($queryAnnotation instanceof AbstractGraphQLElement || $queryAnnotation === null); return $queryAnnotation; } diff --git a/src/Annotations/AbstractGraphQLElement.php b/src/Annotations/AbstractGraphQLElement.php new file mode 100644 index 0000000000..f6ecccc504 --- /dev/null +++ b/src/Annotations/AbstractGraphQLElement.php @@ -0,0 +1,62 @@ +outputType = $outputType ?? $attributes['outputType'] ?? null; + $this->name = $name ?? $attributes['name'] ?? null; + $this->description = $description ?? $attributes['description'] ?? null; + } + + /** + * Returns the GraphQL return type for this schema element (as a string). + * The string can represent the FQCN of the type or an entry in the container resolving to the GraphQL type. + */ + public function getOutputType(): string|null + { + return $this->outputType; + } + + /** + * Returns the GraphQL name of the query/mutation/subscription/field. + * If not specified, the name of the PHP method is used instead. + */ + public function getName(): string|null + { + return $this->name; + } + + /** + * Returns the explicit description for this schema element, or null if none was provided. + * + * A null return means "no explicit description" and the schema builder may fall back to the + * docblock summary (if docblock descriptions are enabled on the SchemaFactory). An explicit + * empty string blocks the docblock fallback and produces an empty description. + */ + public function getDescription(): string|null + { + return $this->description; + } +} diff --git a/src/Annotations/AbstractRequest.php b/src/Annotations/AbstractRequest.php deleted file mode 100644 index cf0f858b55..0000000000 --- a/src/Annotations/AbstractRequest.php +++ /dev/null @@ -1,37 +0,0 @@ -outputType = $outputType ?? $attributes['outputType'] ?? null; - $this->name = $name ?? $attributes['name'] ?? null; - } - - /** - * Returns the GraphQL return type of the request (as a string). - * The string can represent the FQCN of the type or an entry in the container resolving to the GraphQL type. - */ - public function getOutputType(): string|null - { - return $this->outputType; - } - - /** - * Returns the name of the GraphQL query/mutation/field. - * If not specified, the name of the method should be used instead. - */ - public function getName(): string|null - { - return $this->name; - } -} diff --git a/src/Annotations/Exceptions/DuplicateDescriptionOnTypeException.php b/src/Annotations/Exceptions/DuplicateDescriptionOnTypeException.php new file mode 100644 index 0000000000..4be6e8bc7d --- /dev/null +++ b/src/Annotations/Exceptions/DuplicateDescriptionOnTypeException.php @@ -0,0 +1,38 @@ + $targetClass + * @param list $sources Human-readable descriptions of the attribute sources + * that contributed a description (e.g. class names). + */ + public static function forType(string $targetClass, array $sources): self + { + return new self( + 'A GraphQL type may only have a description declared on the #[Type] attribute OR on exactly one #[ExtendType] attribute, never more than one. ' + . 'Target type class "' . $targetClass . '" received descriptions from multiple sources: ' + . implode(', ', $sources) . '. ' + . 'Keep the description on the #[Type] attribute, or move it to at most one #[ExtendType] attribute.', + ); + } +} diff --git a/src/Annotations/ExtendType.php b/src/Annotations/ExtendType.php index 55831b23ea..3c0267e98b 100644 --- a/src/Annotations/ExtendType.php +++ b/src/Annotations/ExtendType.php @@ -21,12 +21,14 @@ class ExtendType /** @var class-string|null */ private string|null $class; private string|null $name; + private string|null $description; /** @param mixed[] $attributes */ public function __construct( array $attributes = [], string|null $class = null, string|null $name = null, + string|null $description = null, ) { $className = isset($attributes['class']) ? ltrim($attributes['class'], '\\') : null; $className = $className ?? $class; @@ -35,6 +37,7 @@ public function __construct( } $this->name = $name ?? $attributes['name'] ?? null; $this->class = $className; + $this->description = $description ?? $attributes['description'] ?? null; if (! $this->class && ! $this->name) { throw new BadMethodCallException('In attribute #[ExtendType], missing one of the compulsory parameter "class" or "name".'); } @@ -55,4 +58,17 @@ public function getName(): string|null { return $this->name; } + + /** + * Returns the explicit description contributed by this type extension, or null if none was provided. + * + * A GraphQL type carries exactly one description. If both the base #[Type] and this #[ExtendType] + * (or multiple #[ExtendType] attributes targeting the same class) provide a description, the + * schema builder throws DuplicateDescriptionOnTypeException. Descriptions may therefore live on + * #[Type] OR on at most one #[ExtendType], never on both. + */ + public function getDescription(): string|null + { + return $this->description; + } } diff --git a/src/Annotations/Factory.php b/src/Annotations/Factory.php index 6032d8dddb..f35d4e8516 100644 --- a/src/Annotations/Factory.php +++ b/src/Annotations/Factory.php @@ -15,13 +15,19 @@ class Factory { private string|null $name; private bool $default; + private string|null $description; /** @param mixed[] $attributes */ - public function __construct(array $attributes = [], string|null $name = null, bool|null $default = null) - { + public function __construct( + array $attributes = [], + string|null $name = null, + bool|null $default = null, + string|null $description = null, + ) { $this->name = $name ?? $attributes['name'] ?? null; // This IS the default if no name is set and no "default" attribute is passed. $this->default = $default ?? $attributes['default'] ?? ! isset($attributes['name']); + $this->description = $description ?? $attributes['description'] ?? null; if ($this->name === null && $this->default === false) { throw new GraphQLRuntimeException('A #[Factory] that has "default=false" attribute must be given a name (i.e. add a name="FooBarInput" attribute).'); @@ -44,4 +50,17 @@ public function isDefault(): bool { return $this->default; } + + /** + * Returns the explicit description for the GraphQL input type produced by this factory, + * or null if none was provided. + * + * A null return means "no explicit description" and the schema builder may fall back to the + * docblock summary (if docblock descriptions are enabled on the SchemaFactory). An explicit + * empty string blocks the docblock fallback and produces an empty description. + */ + public function getDescription(): string|null + { + return $this->description; + } } diff --git a/src/Annotations/Field.php b/src/Annotations/Field.php index 82044c5d29..438a31d14f 100644 --- a/src/Annotations/Field.php +++ b/src/Annotations/Field.php @@ -11,7 +11,7 @@ use const E_USER_DEPRECATED; #[Attribute(Attribute::TARGET_PROPERTY | Attribute::TARGET_METHOD | Attribute::IS_REPEATABLE)] -class Field extends AbstractRequest +class Field extends AbstractGraphQLElement { private string|null $prefetchMethod; diff --git a/src/Annotations/Mutation.php b/src/Annotations/Mutation.php index 39db2aa594..425f51a2b2 100644 --- a/src/Annotations/Mutation.php +++ b/src/Annotations/Mutation.php @@ -7,6 +7,6 @@ use Attribute; #[Attribute(Attribute::TARGET_METHOD)] -class Mutation extends AbstractRequest +class Mutation extends AbstractGraphQLElement { } diff --git a/src/Annotations/Query.php b/src/Annotations/Query.php index 8213ae5e10..520d7086a3 100644 --- a/src/Annotations/Query.php +++ b/src/Annotations/Query.php @@ -7,6 +7,6 @@ use Attribute; #[Attribute(Attribute::TARGET_METHOD)] -class Query extends AbstractRequest +class Query extends AbstractGraphQLElement { } diff --git a/src/Annotations/Subscription.php b/src/Annotations/Subscription.php index 7da35e9661..20b7b9158c 100644 --- a/src/Annotations/Subscription.php +++ b/src/Annotations/Subscription.php @@ -7,6 +7,6 @@ use Attribute; #[Attribute(Attribute::TARGET_METHOD)] -class Subscription extends AbstractRequest +class Subscription extends AbstractGraphQLElement { } diff --git a/src/Annotations/Type.php b/src/Annotations/Type.php index f8465782a5..48d8536902 100644 --- a/src/Annotations/Type.php +++ b/src/Annotations/Type.php @@ -34,6 +34,8 @@ class Type implements TypeInterface private bool $useEnumValues = false; + private string|null $description = null; + /** * @param mixed[] $attributes * @param class-string|null $class @@ -45,6 +47,7 @@ public function __construct( bool|null $default = null, bool|null $external = null, bool|null $useEnumValues = null, + string|null $description = null, ) { $external = $external ?? $attributes['external'] ?? null; $class = $class ?? $attributes['class'] ?? null; @@ -59,6 +62,7 @@ public function __construct( // If no value is passed for default, "default" = true $this->default = $default ?? $attributes['default'] ?? true; $this->useEnumValues = $useEnumValues ?? $attributes['useEnumValues'] ?? false; + $this->description = $description ?? $attributes['description'] ?? null; if ($external === null) { return; @@ -127,4 +131,16 @@ public function useEnumValues(): bool { return $this->useEnumValues; } + + /** + * Returns the explicit description for this GraphQL type, or null if none was provided. + * + * A null return means "no explicit description" and the schema builder may fall back to the + * docblock summary (if docblock descriptions are enabled on the SchemaFactory). An explicit + * empty string blocks the docblock fallback and produces an empty description. + */ + public function getDescription(): string|null + { + return $this->description; + } } diff --git a/src/FieldsBuilder.php b/src/FieldsBuilder.php index 8ed8c87a57..94f80bb940 100644 --- a/src/FieldsBuilder.php +++ b/src/FieldsBuilder.php @@ -17,7 +17,7 @@ use ReflectionMethod; use ReflectionParameter; use ReflectionProperty; -use TheCodingMachine\GraphQLite\Annotations\AbstractRequest; +use TheCodingMachine\GraphQLite\Annotations\AbstractGraphQLElement; use TheCodingMachine\GraphQLite\Annotations\Exceptions\InvalidParameterException; use TheCodingMachine\GraphQLite\Annotations\Field; use TheCodingMachine\GraphQLite\Annotations\Mutation; @@ -50,6 +50,7 @@ use TheCodingMachine\GraphQLite\Types\ArgumentResolver; use TheCodingMachine\GraphQLite\Types\MutableObjectType; use TheCodingMachine\GraphQLite\Types\TypeResolver; +use TheCodingMachine\GraphQLite\Utils\DescriptionResolver; use TheCodingMachine\GraphQLite\Utils\PropertyAccessor; use function array_diff_key; @@ -90,6 +91,7 @@ public function __construct( private readonly ParameterMiddlewareInterface $parameterMapper, private readonly FieldMiddlewareInterface $fieldMiddleware, private readonly InputFieldMiddlewareInterface $inputFieldMiddleware, + private readonly DescriptionResolver $descriptionResolver = new DescriptionResolver(true), ) { $this->typeMapper = new TypeHandler( @@ -97,6 +99,7 @@ public function __construct( $this->rootTypeMapper, $this->typeResolver, $this->docBlockFactory, + $this->descriptionResolver, ); } @@ -310,7 +313,6 @@ public function getSelfFields(string $className, string|null $typeName = null): public function getParameters(ReflectionMethod $refMethod, int $skip = 0): array { $docBlockObj = $this->docBlockFactory->create($refMethod); - //$docBlockComment = $docBlockObj->getSummary()."\n".$docBlockObj->getDescription()->render(); $parameters = array_slice($refMethod->getParameters(), $skip); @@ -332,7 +334,7 @@ public function getParametersForDecorator(ReflectionMethod $refMethod): array /** * @param object|class-string $controller The controller instance, or the name of the source class name - * @param class-string $annotationName + * @param class-string $annotationName * @param bool $injectSource Whether to inject the source object or not as the first argument. True for @Field (unless @Type has no class attribute), false for @Query, @Mutation, and @Subscription. * @param string|null $typeName Type name for which fields should be extracted for. * @@ -418,7 +420,7 @@ private function getFieldsByAnnotations($controller, string $annotationName, boo /** * Gets fields by class method annotations. * - * @param class-string $annotationName + * @param class-string $annotationName * * @return array * @@ -440,7 +442,6 @@ private function getFieldsByMethodAnnotations( $annotations = $this->annotationReader->getMethodAnnotations($refMethod, $annotationName); foreach ($annotations as $queryAnnotation) { - $description = null; $methodName = $refMethod->getName(); if ($queryAnnotation instanceof Field) { @@ -448,17 +449,16 @@ private function getFieldsByMethodAnnotations( if ($typeName && $for && ! in_array($typeName, $for)) { continue; } - - $description = $queryAnnotation->getDescription(); } $docBlockObj = $this->docBlockFactory->create($refMethod); $name = $queryAnnotation->getName() ?: $this->namingStrategy->getFieldNameFromMethodName($methodName); - if (! $description) { - $description = $docBlockObj->getSummary() . "\n" . $docBlockObj->getDescription()->render(); - } + $description = $this->descriptionResolver->resolve( + $queryAnnotation->getDescription(), + $docBlockObj->getSummary() . "\n" . $docBlockObj->getDescription()->render(), + ); $outputType = $queryAnnotation->getOutputType(); if ($outputType) { @@ -504,7 +504,7 @@ private function getFieldsByMethodAnnotations( originalResolver: $resolver, parameters: $args, injectSource: $injectSource, - comment: trim($description), + description: $description !== null ? trim($description) : null, deprecationReason: $this->getDeprecationReason($docBlockObj), middlewareAnnotations: $this->annotationReader->getMiddlewareAnnotations($refMethod), ); @@ -532,7 +532,7 @@ public function handle(QueryFieldDescriptor $fieldDescriptor): FieldDefinition /** * Gets fields by class property annotations. * - * @param class-string $annotationName + * @param class-string $annotationName * * @return array * @@ -552,32 +552,31 @@ private function getFieldsByPropertyAnnotations( $fields = []; $annotations = $this->annotationReader->getPropertyAnnotations($refProperty, $annotationName); foreach ($annotations as $queryAnnotation) { - $description = null; - if ($queryAnnotation instanceof Field) { $for = $queryAnnotation->getFor(); if ($typeName && $for && ! in_array($typeName, $for)) { continue; } - - $description = $queryAnnotation->getDescription(); } $docBlock = $this->docBlockFactory->create($refProperty); $name = $queryAnnotation->getName() ?: $refProperty->getName(); - if (! $description) { - $description = $docBlock->getSummary() . PHP_EOL . $docBlock->getDescription()->render(); + $docblockDescription = $docBlock->getSummary() . PHP_EOL . $docBlock->getDescription()->render(); - /** @var Var_[] $varTags */ - $varTags = $docBlock->getTagsByName('var'); - $varTag = reset($varTags); - if ($varTag) { - $description .= PHP_EOL . $varTag->getDescription(); - } + /** @var Var_[] $varTags */ + $varTags = $docBlock->getTagsByName('var'); + $varTag = reset($varTags); + if ($varTag) { + $docblockDescription .= PHP_EOL . $varTag->getDescription(); } + $description = $this->descriptionResolver->resolve( + $queryAnnotation->getDescription(), + $docblockDescription, + ); + $outputType = $queryAnnotation->getOutputType(); if ($outputType) { $type = $this->typeResolver->mapNameToOutputType($outputType); @@ -597,7 +596,7 @@ private function getFieldsByPropertyAnnotations( resolver: $resolver, originalResolver: $originalResolver, injectSource: false, - comment: trim($description), + description: $description !== null ? trim($description) : null, deprecationReason: $this->getDeprecationReason($docBlock), middlewareAnnotations: $this->annotationReader->getMiddlewareAnnotations($refProperty), ); @@ -683,7 +682,7 @@ private function getQueryFieldsFromSourceFields( } $docBlockObj = $this->docBlockFactory->create($refMethod); - $docBlockComment = rtrim($docBlockObj->getSummary() . "\n" . $docBlockObj->getDescription()->render()); + $docBlockDescription = rtrim($docBlockObj->getSummary() . "\n" . $docBlockObj->getDescription()->render()); $deprecated = $docBlockObj->getTagsByName('deprecated'); $deprecationReason = null; @@ -691,7 +690,10 @@ private function getQueryFieldsFromSourceFields( $deprecationReason = trim((string) $deprecated[0]); } - $description = $sourceField->getDescription() ?? $docBlockComment; + $description = $this->descriptionResolver->resolve( + $sourceField->getDescription(), + $docBlockDescription, + ); $args = $this->mapParameters($refMethod->getParameters(), $docBlockObj, $sourceField); $outputType = $sourceField->getOutputType(); @@ -712,7 +714,7 @@ private function getQueryFieldsFromSourceFields( resolver: $resolver, originalResolver: $resolver, parameters: $args, - comment: $description, + description: $description, deprecationReason: $deprecationReason ?? null, ); } else { @@ -734,7 +736,7 @@ private function getQueryFieldsFromSourceFields( type: $type, resolver: $resolver, originalResolver: $resolver, - comment: $sourceField->getDescription(), + description: $sourceField->getDescription(), ); } @@ -994,7 +996,7 @@ private function getPrefetchParameter( /** * Gets input fields by class method annotations. * - * @param class-string $annotationName + * @param class-string $annotationName * @param array $defaultProperties * * @return array @@ -1019,7 +1021,6 @@ private function getInputFieldsByMethodAnnotations( $annotations = $this->annotationReader->getMethodAnnotations($refMethod, $annotationName); foreach ($annotations as $fieldAnnotations) { - $description = null; if (! ($fieldAnnotations instanceof Field)) { continue; } @@ -1036,10 +1037,10 @@ private function getInputFieldsByMethodAnnotations( } $name = $fieldAnnotations->getName() ?: $this->namingStrategy->getInputFieldNameFromMethodName($methodName); - $description = $fieldAnnotations->getDescription(); - if (! $description) { - $description = $docBlockObj->getSummary() . "\n" . $docBlockObj->getDescription()->render(); - } + $description = $this->descriptionResolver->resolve( + $fieldAnnotations->getDescription(), + $docBlockObj->getSummary() . "\n" . $docBlockObj->getDescription()->render(), + ); $parameters = $refMethod->getParameters(); if ($injectSource === true) { @@ -1076,7 +1077,7 @@ private function getInputFieldsByMethodAnnotations( originalResolver: $resolver, parameters: $args, injectSource: $injectSource, - comment: trim($description), + description: $description !== null ? trim($description) : null, middlewareAnnotations: $this->annotationReader->getMiddlewareAnnotations($refMethod), isUpdate: $isUpdate, hasDefaultValue: $isUpdate, @@ -1103,7 +1104,7 @@ public function handle(InputFieldDescriptor $inputFieldDescriptor): InputField /** * Gets input fields by class property annotations. * - * @param class-string $annotationName + * @param class-string $annotationName * @param array $defaultProperties * * @return array @@ -1128,8 +1129,6 @@ private function getInputFieldsByPropertyAnnotations( $annotations = $this->annotationReader->getPropertyAnnotations($refProperty, $annotationName); $docBlock = $this->docBlockFactory->create($refProperty); foreach ($annotations as $annotation) { - $description = null; - if (! ($annotation instanceof Field)) { continue; } @@ -1139,15 +1138,15 @@ private function getInputFieldsByPropertyAnnotations( continue; } - $description = $annotation->getDescription(); $name = $annotation->getName() ?: $refProperty->getName(); $inputType = $annotation->getInputType(); $constructerParameters = $this->getClassConstructParameterNames($refClass); $inputProperty = $this->typeMapper->mapInputProperty($refProperty, $docBlock, $name, $inputType, $defaultProperties[$refProperty->getName()] ?? null, $isUpdate ? true : null); - if (! $description) { - $description = $inputProperty->getDescription(); - } + $description = $this->descriptionResolver->resolve( + $annotation->getDescription(), + $inputProperty->getDescription(), + ); $type = $inputProperty->getType(); if (! $inputType && $isUpdate && $type instanceof NonNull) { @@ -1171,7 +1170,7 @@ private function getInputFieldsByPropertyAnnotations( parameters: [$inputProperty->getName() => $inputProperty], injectSource: false, forConstructorHydration: $forConstructorHydration, - comment: trim($description), + description: $description !== null ? trim($description) : null, middlewareAnnotations: $this->annotationReader->getMiddlewareAnnotations($refProperty), isUpdate: $isUpdate, hasDefaultValue: $inputProperty->hasDefaultValue(), diff --git a/src/GlobControllerQueryProvider.php b/src/GlobControllerQueryProvider.php index 69f66e0d70..3e9e69df9f 100644 --- a/src/GlobControllerQueryProvider.php +++ b/src/GlobControllerQueryProvider.php @@ -84,15 +84,15 @@ function (ReflectionClass $classReflection): string|null { private function hasOperations(ReflectionClass $reflectionClass): bool { foreach ($reflectionClass->getMethods(ReflectionMethod::IS_PUBLIC) as $refMethod) { - $queryAnnotation = $this->annotationReader->getRequestAnnotation($refMethod, Query::class); + $queryAnnotation = $this->annotationReader->getGraphQLElementAnnotation($refMethod, Query::class); if ($queryAnnotation !== null) { return true; } - $mutationAnnotation = $this->annotationReader->getRequestAnnotation($refMethod, Mutation::class); + $mutationAnnotation = $this->annotationReader->getGraphQLElementAnnotation($refMethod, Mutation::class); if ($mutationAnnotation !== null) { return true; } - $subscriptionAnnotation = $this->annotationReader->getRequestAnnotation($refMethod, Subscription::class); + $subscriptionAnnotation = $this->annotationReader->getGraphQLElementAnnotation($refMethod, Subscription::class); if ($subscriptionAnnotation !== null) { return true; } diff --git a/src/InputField.php b/src/InputField.php index c290175b35..1002e4008a 100644 --- a/src/InputField.php +++ b/src/InputField.php @@ -45,7 +45,7 @@ public function __construct( ResolverInterface $originalResolver, callable $resolver, private bool $forConstructorHydration, - string|null $comment, + string|null $description, bool $isUpdate, bool $hasDefaultValue, mixed $defaultValue, @@ -54,7 +54,7 @@ public function __construct( $config = [ 'name' => $name, 'type' => $type, - 'description' => $comment, + 'description' => $description, ]; if (! (! $hasDefaultValue || $isUpdate)) { @@ -133,7 +133,7 @@ private static function fromDescriptor(InputFieldDescriptor $fieldDescriptor): s $fieldDescriptor->getOriginalResolver(), $fieldDescriptor->getResolver(), $fieldDescriptor->isForConstructorHydration(), - $fieldDescriptor->getComment(), + $fieldDescriptor->getDescription(), $fieldDescriptor->isUpdate(), $fieldDescriptor->hasDefaultValue(), $fieldDescriptor->getDefaultValue(), diff --git a/src/InputFieldDescriptor.php b/src/InputFieldDescriptor.php index d59b19cb89..523a75fbaa 100644 --- a/src/InputFieldDescriptor.php +++ b/src/InputFieldDescriptor.php @@ -36,7 +36,7 @@ public function __construct( private readonly array $parameters = [], private readonly bool $injectSource = false, private readonly bool $forConstructorHydration = false, - private readonly string|null $comment = null, + private readonly string|null $description = null, private readonly MiddlewareAnnotations $middlewareAnnotations = new MiddlewareAnnotations([]), private readonly bool $isUpdate = false, private readonly bool $hasDefaultValue = false, @@ -127,14 +127,14 @@ public function withForConstructorHydration(bool $forConstructorHydration): self return $this->with(forConstructorHydration: $forConstructorHydration); } - public function getComment(): string|null + public function getDescription(): string|null { - return $this->comment; + return $this->description; } - public function withComment(string|null $comment): self + public function withDescription(string|null $description): self { - return $this->with(comment: $comment); + return $this->with(description: $description); } public function getMiddlewareAnnotations(): MiddlewareAnnotations diff --git a/src/InputTypeGenerator.php b/src/InputTypeGenerator.php index 2dbb070dab..d2bc247978 100644 --- a/src/InputTypeGenerator.php +++ b/src/InputTypeGenerator.php @@ -8,10 +8,12 @@ use Psr\Container\ContainerInterface; use ReflectionFunctionAbstract; use ReflectionMethod; +use TheCodingMachine\GraphQLite\Reflection\DocBlock\DocBlockFactory; use TheCodingMachine\GraphQLite\Types\InputType; use TheCodingMachine\GraphQLite\Types\InputTypeValidatorInterface; use TheCodingMachine\GraphQLite\Types\ResolvableMutableInputInterface; use TheCodingMachine\GraphQLite\Types\ResolvableMutableInputObjectType; +use TheCodingMachine\GraphQLite\Utils\DescriptionResolver; use function array_shift; use function assert; @@ -31,6 +33,9 @@ public function __construct( private InputTypeUtils $inputTypeUtils, private FieldsBuilder $fieldsBuilder, private InputTypeValidatorInterface|null $inputTypeValidator = null, + private AnnotationReader|null $annotationReader = null, + private DocBlockFactory|null $docBlockFactory = null, + private DescriptionResolver $descriptionResolver = new DescriptionResolver(true), ) { } @@ -47,13 +52,46 @@ public function mapFactoryMethod(string $factory, string $methodName, ContainerI [$inputName, $className] = $this->inputTypeUtils->getInputTypeNameAndClassName($method); if (! isset($this->factoryCache[$inputName])) { - // TODO: add comment argument. - $this->factoryCache[$inputName] = new ResolvableMutableInputObjectType($inputName, $this->fieldsBuilder, $object, $methodName, null, $this->canBeInstantiatedWithoutParameter($method, false)); + $this->factoryCache[$inputName] = new ResolvableMutableInputObjectType( + $inputName, + $this->fieldsBuilder, + $object, + $methodName, + $this->resolveFactoryDescription($method), + $this->canBeInstantiatedWithoutParameter($method, false), + ); } return $this->factoryCache[$inputName]; } + /** + * Resolves the description for an input type produced by a #[Factory]-annotated method, + * combining an explicit {@see Factory::$description} with an optional docblock fallback + * according to the configured {@see DescriptionResolver}. + * + * Replaces the long-standing `// TODO: add comment argument.` gap — input types generated by + * factories now participate in the same description resolution pipeline as every other + * schema element. + */ + private function resolveFactoryDescription(ReflectionMethod $method): string|null + { + if ($this->annotationReader === null) { + return null; + } + + $factoryAnnotation = $this->annotationReader->getFactoryAnnotation($method); + $explicit = $factoryAnnotation?->getDescription(); + + $docblockDerived = null; + if ($this->docBlockFactory !== null && $this->descriptionResolver->isDocblockFallbackEnabled()) { + $summary = $this->docBlockFactory->create($method)->getSummary(); + $docblockDerived = $summary === '' ? null : $summary; + } + + return $this->descriptionResolver->resolve($explicit, $docblockDerived); + } + /** @param class-string $className */ public function mapInput(string $className, string $inputName, string|null $description, bool $isUpdate): InputType { diff --git a/src/Mappers/Parameters/TypeHandler.php b/src/Mappers/Parameters/TypeHandler.php index 77fdad3f55..14b07ce43b 100644 --- a/src/Mappers/Parameters/TypeHandler.php +++ b/src/Mappers/Parameters/TypeHandler.php @@ -46,6 +46,7 @@ use TheCodingMachine\GraphQLite\Reflection\DocBlock\DocBlockFactory; use TheCodingMachine\GraphQLite\Types\ArgumentResolver; use TheCodingMachine\GraphQLite\Types\TypeResolver; +use TheCodingMachine\GraphQLite\Utils\DescriptionResolver; use function array_map; use function array_unique; @@ -69,6 +70,7 @@ public function __construct( private readonly RootTypeMapperInterface $rootTypeMapper, private readonly TypeResolver $typeResolver, private readonly DocBlockFactory $docBlockFactory, + private readonly DescriptionResolver $descriptionResolver = new DescriptionResolver(true), ) { $this->phpDocumentorTypeResolver = new PhpDocumentorTypeResolver(); @@ -220,7 +222,9 @@ public function mapParameter( } } - $description = $this->getParameterDescriptionFromDocBlock($docBlock, $parameter); + $description = $this->descriptionResolver->isDocblockFallbackEnabled() + ? $this->getParameterDescriptionFromDocBlock($docBlock, $parameter) + : null; $hasDefaultValue = false; $defaultValue = null; @@ -309,13 +313,13 @@ public function mapInputProperty( bool|null $isNullable = null, ): InputTypeProperty { - $docBlockComment = $docBlock->getSummary() . PHP_EOL . $docBlock->getDescription()->render(); + $docBlockDescription = $docBlock->getSummary() . PHP_EOL . $docBlock->getDescription()->render(); /** @var Var_[] $varTags */ $varTags = $docBlock->getTagsByName('var'); $varTag = reset($varTags); if ($varTag) { - $docBlockComment .= PHP_EOL . $varTag->getDescription(); + $docBlockDescription .= PHP_EOL . $varTag->getDescription(); if ($isNullable === null) { $varType = $varTag->getType(); @@ -339,11 +343,13 @@ public function mapInputProperty( $hasDefault = $defaultValue !== null || $isNullable; $fieldName = $argumentName ?? $refProperty->getName(); + $resolvedDescription = $this->descriptionResolver->resolve(null, $docBlockDescription); + return new InputTypeProperty( propertyName: $refProperty->getName(), fieldName: $fieldName, type: $inputType, - description: trim($docBlockComment), + description: $resolvedDescription !== null ? trim($resolvedDescription) : '', hasDefaultValue: $hasDefault, defaultValue: $defaultValue, argumentResolver: $this->argumentResolver, diff --git a/src/Mappers/Root/EnumTypeMapper.php b/src/Mappers/Root/EnumTypeMapper.php index 87e575bb6c..cb39c9d9c9 100644 --- a/src/Mappers/Root/EnumTypeMapper.php +++ b/src/Mappers/Root/EnumTypeMapper.php @@ -22,6 +22,7 @@ use TheCodingMachine\GraphQLite\Discovery\ClassFinder; use TheCodingMachine\GraphQLite\Reflection\DocBlock\DocBlockFactory; use TheCodingMachine\GraphQLite\Types\EnumType; +use TheCodingMachine\GraphQLite\Utils\DescriptionResolver; use UnitEnum; use function array_filter; @@ -49,6 +50,7 @@ public function __construct( private readonly DocBlockFactory $docBlockFactory, private readonly ClassFinder $classFinder, private readonly ClassFinderComputedCache $classFinderComputedCache, + private readonly DescriptionResolver $descriptionResolver = new DescriptionResolver(true), ) { } @@ -126,9 +128,16 @@ private function mapByClassName(string $enumClass): EnumType|null && $reflectionEnum->isBacked() && (string) $reflectionEnum->getBackingType() === 'string'; - $enumDescription = $this->docBlockFactory - ->create($reflectionEnum) - ->getSummary() ?: null; + $explicitEnumDescription = $typeAnnotation instanceof TypeAnnotation + ? $typeAnnotation->getDescription() + : null; + + $enumDescription = $this->descriptionResolver->resolve( + $explicitEnumDescription, + $this->descriptionResolver->isDocblockFallbackEnabled() + ? ($this->docBlockFactory->create($reflectionEnum)->getSummary() ?: null) + : null, + ); /** @var array $enumCaseDescriptions */ $enumCaseDescriptions = []; @@ -138,7 +147,9 @@ private function mapByClassName(string $enumClass): EnumType|null foreach ($reflectionEnum->getCases() as $reflectionEnumCase) { $docBlock = $this->docBlockFactory->create($reflectionEnumCase); - $enumCaseDescriptions[$reflectionEnumCase->getName()] = $docBlock->getSummary() ?: null; + $enumCaseDescriptions[$reflectionEnumCase->getName()] = $this->descriptionResolver->isDocblockFallbackEnabled() + ? ($docBlock->getSummary() ?: null) + : null; $deprecation = $docBlock->getTagsByName('deprecated')[0] ?? null; // phpcs:ignore diff --git a/src/Middlewares/CostFieldMiddleware.php b/src/Middlewares/CostFieldMiddleware.php index 3e4865ca49..01c32ec5ca 100644 --- a/src/Middlewares/CostFieldMiddleware.php +++ b/src/Middlewares/CostFieldMiddleware.php @@ -25,7 +25,7 @@ public function process(QueryFieldDescriptor $queryFieldDescriptor, FieldHandler } $field = $fieldHandler->handle( - $queryFieldDescriptor->withAddedCommentLines($this->buildQueryComment($costAttribute)), + $queryFieldDescriptor->withAddedDescriptionLines($this->buildCostDescription($costAttribute)), ); if (! $field) { @@ -61,7 +61,7 @@ public function process(QueryFieldDescriptor $queryFieldDescriptor, FieldHandler return $field; } - private function buildQueryComment(Cost $costAttribute): string + private function buildCostDescription(Cost $costAttribute): string { return "\nCost: " . implode(', ', [ diff --git a/src/QueryField.php b/src/QueryField.php index 0d6e011b6c..83a2e28acc 100644 --- a/src/QueryField.php +++ b/src/QueryField.php @@ -45,7 +45,7 @@ public function __construct( array $arguments, ResolverInterface $originalResolver, callable $resolver, - string|null $comment, + string|null $description, string|null $deprecationReason, array $additionalConfig = [], ) @@ -55,8 +55,8 @@ public function __construct( 'type' => $type, 'args' => InputTypeUtils::getInputTypeArgs($arguments), ]; - if ($comment) { - $config['description'] = $comment; + if ($description) { + $config['description'] = $description; } if ($deprecationReason) { $config['deprecationReason'] = $deprecationReason; @@ -163,7 +163,7 @@ private static function fromDescriptor(QueryFieldDescriptor $fieldDescriptor): s $fieldDescriptor->getParameters(), $fieldDescriptor->getOriginalResolver(), $fieldDescriptor->getResolver(), - $fieldDescriptor->getComment(), + $fieldDescriptor->getDescription(), $fieldDescriptor->getDeprecationReason(), ); } diff --git a/src/QueryFieldDescriptor.php b/src/QueryFieldDescriptor.php index aab5f70c6f..462c45d207 100644 --- a/src/QueryFieldDescriptor.php +++ b/src/QueryFieldDescriptor.php @@ -35,7 +35,7 @@ public function __construct( private readonly SourcePropertyResolver|MagicPropertyResolver|SourceMethodResolver|ServiceResolver $originalResolver, private readonly array $parameters = [], private readonly bool $injectSource = false, - private readonly string|null $comment = null, + private readonly string|null $description = null, private readonly string|null $deprecationReason = null, private readonly MiddlewareAnnotations $middlewareAnnotations = new MiddlewareAnnotations([]), ) @@ -84,23 +84,23 @@ public function withInjectSource(bool $injectSource): self return $this->with(injectSource: $injectSource); } - public function getComment(): string|null + public function getDescription(): string|null { - return $this->comment; + return $this->description; } - public function withComment(string|null $comment): self + public function withDescription(string|null $description): self { - return $this->with(comment: $comment); + return $this->with(description: $description); } - public function withAddedCommentLines(string $comment): self + public function withAddedDescriptionLines(string $description): self { - if (! $this->comment) { - return $this->withComment($comment); + if (! $this->description) { + return $this->withDescription($description); } - return $this->withComment($this->comment . "\n" . $comment); + return $this->withDescription($this->description . "\n" . $description); } public function getDeprecationReason(): string|null diff --git a/src/SchemaFactory.php b/src/SchemaFactory.php index 037910f281..2e8762584f 100644 --- a/src/SchemaFactory.php +++ b/src/SchemaFactory.php @@ -67,6 +67,7 @@ use TheCodingMachine\GraphQLite\Types\ArgumentResolver; use TheCodingMachine\GraphQLite\Types\InputTypeValidatorInterface; use TheCodingMachine\GraphQLite\Types\TypeResolver; +use TheCodingMachine\GraphQLite\Utils\DescriptionResolver; use TheCodingMachine\GraphQLite\Utils\NamespacedCache; use function array_reverse; @@ -119,6 +120,14 @@ class SchemaFactory private bool $devMode = true; + /** + * When true (default), docblock summaries are used as a fallback description for any schema + * element whose attribute did not specify an explicit description. When false, only explicit + * descriptions populate the schema, preventing internal developer docblocks from leaking to + * public API consumers. + */ + private bool $useDocblockDescriptions = true; + /** @var array */ private array $fieldMiddlewares = []; @@ -320,6 +329,21 @@ public function devMode(): self return $this; } + /** + * Controls whether docblock summaries are used as a fallback description for schema elements + * whose attributes did not provide an explicit `description` argument. + * + * Enabled by default for backwards compatibility. Disable this to ensure only explicit + * descriptions populate the GraphQL schema, preventing internal developer docblocks + * (implementation notes, @see references, reminders) from leaking to API consumers. + */ + public function setDocblockDescriptionsEnabled(bool $enabled): self + { + $this->useDocblockDescriptions = $enabled; + + return $this; + } + /** * Registers a field middleware (used to parse custom annotations that modify the GraphQLite behaviour in Fields/Queries/Mutations. */ @@ -367,6 +391,7 @@ public function createSchema(): Schema $classBoundCache, PhpDocumentorDocBlockFactory::default(), ); + $descriptionResolver = new DescriptionResolver($this->useDocblockDescriptions); $namingStrategy = $this->namingStrategy ?: new NamingStrategy(); $typeRegistry = new TypeRegistry(); $classFinder = $this->createClassFinder(); @@ -404,7 +429,7 @@ public function createSchema(): Schema $errorRootTypeMapper = new FinalRootTypeMapper($recursiveTypeMapper); $rootTypeMapper = new BaseTypeMapper($errorRootTypeMapper, $recursiveTypeMapper, $topRootTypeMapper); - $rootTypeMapper = new EnumTypeMapper($rootTypeMapper, $annotationReader, $docBlockFactory, $classFinder, $classFinderComputedCache); + $rootTypeMapper = new EnumTypeMapper($rootTypeMapper, $annotationReader, $docBlockFactory, $classFinder, $classFinderComputedCache, $descriptionResolver); if (class_exists(Enum::class)) { // Annotation support - deprecated @@ -450,6 +475,7 @@ classBoundCache: $classBoundCache, $parameterMiddlewarePipe, $fieldMiddlewarePipe, $inputFieldMiddlewarePipe, + $descriptionResolver, ); $parameterizedCallableResolver = new ParameterizedCallableResolver($fieldsBuilder, $this->container); @@ -461,9 +487,9 @@ classBoundCache: $classBoundCache, $parameterMiddlewarePipe->pipe(new ContainerParameterHandler($this->container)); $parameterMiddlewarePipe->pipe(new InjectUserParameterHandler($authenticationService)); - $typeGenerator = new TypeGenerator($annotationReader, $namingStrategy, $typeRegistry, $this->container, $recursiveTypeMapper, $fieldsBuilder); + $typeGenerator = new TypeGenerator($annotationReader, $namingStrategy, $typeRegistry, $this->container, $recursiveTypeMapper, $fieldsBuilder, $docBlockFactory, $descriptionResolver); $inputTypeUtils = new InputTypeUtils($annotationReader, $namingStrategy); - $inputTypeGenerator = new InputTypeGenerator($inputTypeUtils, $fieldsBuilder, $this->inputTypeValidator); + $inputTypeGenerator = new InputTypeGenerator($inputTypeUtils, $fieldsBuilder, $this->inputTypeValidator, $annotationReader, $docBlockFactory, $descriptionResolver); if ($this->namespaces) { $compositeTypeMapper->addTypeMapper(new ClassFinderTypeMapper( diff --git a/src/TypeGenerator.php b/src/TypeGenerator.php index d77b119d26..1220fa6797 100644 --- a/src/TypeGenerator.php +++ b/src/TypeGenerator.php @@ -7,13 +7,19 @@ use Psr\Container\ContainerInterface; use ReflectionClass; use ReflectionException; +use TheCodingMachine\GraphQLite\Annotations\Exceptions\DuplicateDescriptionOnTypeException; +use TheCodingMachine\GraphQLite\Annotations\ExtendType; +use TheCodingMachine\GraphQLite\Annotations\Type as TypeAnnotation; use TheCodingMachine\GraphQLite\Mappers\RecursiveTypeMapperInterface; +use TheCodingMachine\GraphQLite\Reflection\DocBlock\DocBlockFactory; use TheCodingMachine\GraphQLite\Types\MutableInterface; use TheCodingMachine\GraphQLite\Types\MutableInterfaceType; use TheCodingMachine\GraphQLite\Types\MutableObjectType; use TheCodingMachine\GraphQLite\Types\TypeAnnotatedInterfaceType; use TheCodingMachine\GraphQLite\Types\TypeAnnotatedObjectType; +use TheCodingMachine\GraphQLite\Utils\DescriptionResolver; +use function assert; use function interface_exists; /** @@ -22,6 +28,18 @@ */ class TypeGenerator { + /** + * Tracks every GraphQL type name for which an explicit description has already been contributed + * via a #[Type] or #[ExtendType] attribute. Used to reject ambiguous multi-source descriptions + * on the same type with a clear error. + * + * Keys are the resolved GraphQL type name, values are a human-readable source label (e.g. the + * annotated class name) so the resulting exception points at both offending sources. + * + * @var array + */ + private array $explicitDescriptionSources = []; + public function __construct( private AnnotationReader $annotationReader, private NamingStrategyInterface $namingStrategy, @@ -29,6 +47,8 @@ public function __construct( private ContainerInterface $container, private RecursiveTypeMapperInterface $recursiveTypeMapper, private FieldsBuilder $fieldsBuilder, + private DocBlockFactory|null $docBlockFactory = null, + private DescriptionResolver $descriptionResolver = new DescriptionResolver(true), ) { } @@ -50,6 +70,12 @@ public function mapAnnotatedObject(string $annotatedObjectClassName): MutableInt throw MissingAnnotationException::missingTypeException($annotatedObjectClassName); } + // AnnotationReader::getTypeAnnotation only resolves #[Type] attributes, so the concrete + // Type class is the only possible implementation of TypeInterface returned here. The + // narrower type grants access to the Type-specific description API without widening + // TypeInterface (which would be a BC break for external implementations). + assert($typeField instanceof TypeAnnotation); + $typeName = $this->namingStrategy->getOutputTypeName($refTypeClass->getName(), $typeField); if ($this->typeRegistry->hasType($typeName)) { @@ -67,34 +93,39 @@ public function mapAnnotatedObject(string $annotatedObjectClassName): MutableInt $isInterface = $refTypeClass->isInterface(); } + $resolvedDescription = $this->descriptionResolver->resolve( + $typeField->getDescription(), + $this->extractClassDocblockSummary($refTypeClass), + ); + + if ($typeField->getDescription() !== null) { + $this->explicitDescriptionSources[$typeName] = '#[Type] on ' . $refTypeClass->getName(); + } + if ($isInterface) { - return TypeAnnotatedInterfaceType::createFromAnnotatedClass($typeName, $typeField->getClass(), $annotatedObject, $this->fieldsBuilder, $this->recursiveTypeMapper); + $type = TypeAnnotatedInterfaceType::createFromAnnotatedClass( + $typeName, + $typeField->getClass(), + $annotatedObject, + $this->fieldsBuilder, + $this->recursiveTypeMapper, + ); + } else { + $type = TypeAnnotatedObjectType::createFromAnnotatedClass( + $typeName, + $typeField->getClass(), + $annotatedObject, + $this->fieldsBuilder, + $this->recursiveTypeMapper, + ! $typeField->isDefault(), + ); } - return TypeAnnotatedObjectType::createFromAnnotatedClass($typeName, $typeField->getClass(), $annotatedObject, $this->fieldsBuilder, $this->recursiveTypeMapper, ! $typeField->isDefault()); - - /*return new ObjectType([ - 'name' => $typeName, - 'fields' => function() use ($annotatedObject, $recursiveTypeMapper, $typeField) { - $parentClass = get_parent_class($typeField->getClass()); - $parentType = null; - if ($parentClass !== false) { - if ($recursiveTypeMapper->canMapClassToType($parentClass)) { - $parentType = $recursiveTypeMapper->mapClassToType($parentClass, null); - } - } - - $fieldProvider = $this->controllerQueryProviderFactory->buildFieldsBuilder($recursiveTypeMapper); - $fields = $fieldProvider->getFields($annotatedObject); - if ($parentType !== null) { - $fields = $parentType->getFields() + $fields; - } - return $fields; - }, - 'interfaces' => function() use ($typeField, $recursiveTypeMapper) { - return $recursiveTypeMapper->findInterfaces($typeField->getClass()); - } - ]);*/ + if ($resolvedDescription !== null) { + $type->description = $resolvedDescription; + } + + return $type; } /** @@ -113,27 +144,64 @@ public function extendAnnotatedObject(object $annotatedObject, MutableInterface throw MissingAnnotationException::missingExtendTypeException(); } - //$typeName = $this->namingStrategy->getOutputTypeName($refTypeClass->getName(), $extendTypeAnnotation); $typeName = $type->name; - /*if ($this->typeRegistry->hasType($typeName)) { - throw new GraphQLException(sprintf('Tried to extend GraphQL type "%s" that is already stored in the type registry.', $typeName)); - } - */ + $this->applyExtendTypeDescription($refTypeClass, $extendTypeAnnotation, $type, $typeName); $type->addFields(function () use ($annotatedObject, $typeName) { - /*$parentClass = get_parent_class($extendTypeAnnotation->getClass()); - $parentType = null; - if ($parentClass !== false) { - if ($recursiveTypeMapper->canMapClassToType($parentClass)) { - $parentType = $recursiveTypeMapper->mapClassToType($parentClass, null); - } - }*/ return $this->fieldsBuilder->getFields($annotatedObject, $typeName); - - /*if ($parentType !== null) { - $fields = $parentType->getFields() + $fields; - }*/ }); } + + /** + * Enforces the rule that a GraphQL type description may live on the base #[Type] OR on at most + * one #[ExtendType], never both, and applies the #[ExtendType] description to the underlying + * type when valid. + * + * @param ReflectionClass $refTypeClass + * @param MutableInterface&(MutableObjectType|MutableInterfaceType) $type + */ + private function applyExtendTypeDescription( + ReflectionClass $refTypeClass, + ExtendType $extendTypeAnnotation, + MutableInterface $type, + string $typeName, + ): void { + $explicitDescription = $extendTypeAnnotation->getDescription(); + if ($explicitDescription === null) { + return; + } + + $sourceLabel = '#[ExtendType] on ' . $refTypeClass->getName(); + + if (isset($this->explicitDescriptionSources[$typeName])) { + $targetClass = $extendTypeAnnotation->getClass() ?? $refTypeClass->getName(); + throw DuplicateDescriptionOnTypeException::forType( + $targetClass, + [$this->explicitDescriptionSources[$typeName], $sourceLabel], + ); + } + + $this->explicitDescriptionSources[$typeName] = $sourceLabel; + $type->description = $explicitDescription; + } + + /** + * Returns the docblock summary for a class, or null when: + * - docblock fallback is disabled on the resolver, + * - no DocBlockFactory was injected, + * - the class has no docblock summary. + * + * @param ReflectionClass $refTypeClass + */ + private function extractClassDocblockSummary(ReflectionClass $refTypeClass): string|null + { + if (! $this->descriptionResolver->isDocblockFallbackEnabled() || $this->docBlockFactory === null) { + return null; + } + + $summary = $this->docBlockFactory->create($refTypeClass)->getSummary(); + + return $summary === '' ? null : $summary; + } } diff --git a/src/Types/ResolvableMutableInputObjectType.php b/src/Types/ResolvableMutableInputObjectType.php index 056f04174d..85d0ecd1c3 100644 --- a/src/Types/ResolvableMutableInputObjectType.php +++ b/src/Types/ResolvableMutableInputObjectType.php @@ -48,9 +48,16 @@ class ResolvableMutableInputObjectType extends MutableInputObjectType implements */ private array $decoratorsParameters = []; - /** @param array{name?: string|null,description?: string|null,parseValue?: callable(array): mixed,astNode?: InputObjectTypeDefinitionNode|null,extensionASTNodes?: array|null} $additionalConfig */ - public function __construct(string $name, private FieldsBuilder $fieldsBuilder, object|string $factory, string $methodName, string|null $comment, private bool $canBeInstantiatedWithoutParameters, array $additionalConfig = []) - { + /** @param array{name?: string|null, description?: string|null, parseValue?: callable(array): mixed, astNode?: InputObjectTypeDefinitionNode|null, extensionASTNodes?: array|null} $additionalConfig */ + public function __construct( + string $name, + private FieldsBuilder $fieldsBuilder, + object|string $factory, + string $methodName, + string|null $description, + private bool $canBeInstantiatedWithoutParameters, + array $additionalConfig = [], + ) { $resolve = [$factory, $methodName]; assert(is_callable($resolve)); $this->resolve = $resolve; @@ -68,8 +75,8 @@ public function __construct(string $name, private FieldsBuilder $fieldsBuilder, 'name' => $name, 'fields' => $fields, ]; - if ($comment) { - $config['description'] = $comment; + if ($description) { + $config['description'] = $description; } $config += $additionalConfig; diff --git a/src/Utils/DescriptionResolver.php b/src/Utils/DescriptionResolver.php new file mode 100644 index 0000000000..f586037183 --- /dev/null +++ b/src/Utils/DescriptionResolver.php @@ -0,0 +1,60 @@ +useDocblockFallback; + } + + /** + * @param string|null $explicit Description provided explicitly via an attribute argument. + * Null means "not provided"; an empty string means "explicit empty". + * @param string|null $docblockDerived Description string the caller extracted from the docblock + * (or null if there was no docblock, or if docblock extraction + * yielded nothing meaningful). Ignored when docblock fallback + * is disabled. + */ + public function resolve(string|null $explicit, string|null $docblockDerived): string|null + { + if ($explicit !== null) { + return $explicit; + } + + if (! $this->useDocblockFallback) { + return null; + } + + return $docblockDerived; + } +} diff --git a/tests/AnnotationReaderTest.php b/tests/AnnotationReaderTest.php index 36e1a1f220..bdf696aef4 100644 --- a/tests/AnnotationReaderTest.php +++ b/tests/AnnotationReaderTest.php @@ -55,7 +55,7 @@ public function testMethodWithBadAnnotation(): void { $annotationReader = new AnnotationReader(); - $type = $annotationReader->getRequestAnnotation( + $type = $annotationReader->getGraphQLElementAnnotation( new ReflectionMethod(ClassWithInvalidClassAnnotation::class, 'testMethod'), Field::class, ); @@ -117,7 +117,7 @@ public function testPhp8AttributeMethodAnnotation(): void { $annotationReader = new AnnotationReader(); - $type = $annotationReader->getRequestAnnotation( + $type = $annotationReader->getGraphQLElementAnnotation( new ReflectionMethod(TestType::class, 'getField'), Field::class, ); diff --git a/tests/Annotations/ExtendTypeTest.php b/tests/Annotations/ExtendTypeTest.php index ab0d7627a3..732641bb4f 100644 --- a/tests/Annotations/ExtendTypeTest.php +++ b/tests/Annotations/ExtendTypeTest.php @@ -14,4 +14,22 @@ public function testException(): void $this->expectExceptionMessage('In attribute #[ExtendType], missing one of the compulsory parameter "class" or "name".'); new ExtendType([]); } + + public function testDescriptionDefaultsToNull(): void + { + $extendType = new ExtendType(['name' => 'SomeType']); + $this->assertNull($extendType->getDescription()); + } + + public function testDescriptionFromConstructor(): void + { + $extendType = new ExtendType(['name' => 'SomeType'], description: 'Extension description'); + $this->assertSame('Extension description', $extendType->getDescription()); + } + + public function testDescriptionFromAttributesArray(): void + { + $extendType = new ExtendType(['name' => 'SomeType', 'description' => 'From attributes']); + $this->assertSame('From attributes', $extendType->getDescription()); + } } diff --git a/tests/Annotations/FactoryTest.php b/tests/Annotations/FactoryTest.php index 5023373a7a..9ceea3b2e0 100644 --- a/tests/Annotations/FactoryTest.php +++ b/tests/Annotations/FactoryTest.php @@ -13,4 +13,22 @@ public function testExceptionInConstruct(): void $this->expectException(GraphQLRuntimeException::class); new Factory(['default'=>false]); } + + public function testDescriptionDefaultsToNull(): void + { + $factory = new Factory(); + $this->assertNull($factory->getDescription()); + } + + public function testDescriptionFromConstructor(): void + { + $factory = new Factory(description: 'Factory description'); + $this->assertSame('Factory description', $factory->getDescription()); + } + + public function testDescriptionFromAttributesArray(): void + { + $factory = new Factory(['description' => 'From attributes']); + $this->assertSame('From attributes', $factory->getDescription()); + } } diff --git a/tests/Annotations/QueryTest.php b/tests/Annotations/QueryTest.php new file mode 100644 index 0000000000..665bc4f3b5 --- /dev/null +++ b/tests/Annotations/QueryTest.php @@ -0,0 +1,61 @@ +assertNull($query->getDescription()); + } + + public function testDescriptionFromConstructor(): void + { + $query = new Query(description: 'Explicit query description'); + $this->assertSame('Explicit query description', $query->getDescription()); + } + + public function testDescriptionFromAttributesArray(): void + { + $query = new Query(['description' => 'From attributes array']); + $this->assertSame('From attributes array', $query->getDescription()); + } + + public function testDescriptionPreservesEmptyString(): void + { + // '' is the deliberate "explicit empty" signal that suppresses docblock fallback; it + // must round-trip intact through the attribute so downstream resolvers can distinguish + // it from "no description provided" (null). + $query = new Query(description: ''); + $this->assertSame('', $query->getDescription()); + } + + public function testDescriptionAlongsideNameAndOutputType(): void + { + $query = new Query(name: 'myQuery', outputType: 'String!', description: 'Desc'); + $this->assertSame('myQuery', $query->getName()); + $this->assertSame('String!', $query->getOutputType()); + $this->assertSame('Desc', $query->getDescription()); + } + + public function testMutationInheritsDescriptionSupport(): void + { + $mutation = new Mutation(description: 'Mutation desc'); + $this->assertSame('Mutation desc', $mutation->getDescription()); + } + + public function testSubscriptionInheritsDescriptionSupport(): void + { + $subscription = new Subscription(description: 'Subscription desc'); + $this->assertSame('Subscription desc', $subscription->getDescription()); + } +} diff --git a/tests/Annotations/TypeTest.php b/tests/Annotations/TypeTest.php index 486befd85a..6b2904863a 100644 --- a/tests/Annotations/TypeTest.php +++ b/tests/Annotations/TypeTest.php @@ -30,4 +30,30 @@ public function testException2() $this->expectExceptionMessage('Problem in attribute #[Type] for interface "TheCodingMachine\GraphQLite\Fixtures\AnnotatedInterfaces\Types\FooInterface": you cannot use the default="false" attribute on interfaces'); $type->setClass(FooInterface::class); } + + public function testDescriptionDefaultsToNull(): void + { + $type = new Type([]); + $this->assertNull($type->getDescription()); + } + + public function testDescriptionFromConstructor(): void + { + $type = new Type([], description: 'Explicit description'); + $this->assertSame('Explicit description', $type->getDescription()); + } + + public function testDescriptionFromAttributesArray(): void + { + $type = new Type(['description' => 'From attributes']); + $this->assertSame('From attributes', $type->getDescription()); + } + + public function testDescriptionPreservesEmptyString(): void + { + // An empty string is a deliberate "explicit empty" signal that suppresses the docblock + // fallback further down the pipeline; it must round-trip unchanged through the attribute. + $type = new Type([], description: ''); + $this->assertSame('', $type->getDescription()); + } } diff --git a/tests/Fixtures/Description/Author.php b/tests/Fixtures/Description/Author.php new file mode 100644 index 0000000000..b53651ce60 --- /dev/null +++ b/tests/Fixtures/Description/Author.php @@ -0,0 +1,30 @@ +name; + } +} diff --git a/tests/Fixtures/Description/AuthorExtension.php b/tests/Fixtures/Description/AuthorExtension.php new file mode 100644 index 0000000000..a83506ef1c --- /dev/null +++ b/tests/Fixtures/Description/AuthorExtension.php @@ -0,0 +1,23 @@ +title; + } +} diff --git a/tests/Fixtures/Description/BookSearchCriteria.php b/tests/Fixtures/Description/BookSearchCriteria.php new file mode 100644 index 0000000000..816762454d --- /dev/null +++ b/tests/Fixtures/Description/BookSearchCriteria.php @@ -0,0 +1,13 @@ +setAuthenticationService(new VoidAuthenticationService()); + $factory->setAuthorizationService(new VoidAuthorizationService()); + $factory->addNamespace((new ReflectionClass($fixtureClass))->getNamespaceName()); + $factory->setDocblockDescriptionsEnabled($docblockDescriptions); + + return $factory->createSchema(); + } + + public function testExplicitDescriptionOnQueryOverridesDocblock(): void + { + $schema = $this->buildSchema(Book::class); + + $bookField = $schema->getQueryType()->getField('book'); + $this->assertSame('Fetch a single library book.', $bookField->description); + } + + public function testExplicitDescriptionOnMutation(): void + { + $schema = $this->buildSchema(Book::class); + + $mutationField = $schema->getMutationType()->getField('borrowBook'); + $this->assertSame('Borrow a book from the library.', $mutationField->description); + } + + public function testExplicitDescriptionOnType(): void + { + $schema = $this->buildSchema(Book::class); + + $bookType = $schema->getType('Book'); + $this->assertSame('A library book available for checkout.', $bookType->description); + } + + public function testExplicitDescriptionOnFieldOverridesDocblock(): void + { + $schema = $this->buildSchema(Book::class); + + $titleField = $schema->getType('Book')->getField('title'); + $this->assertSame('The book title as it appears on the cover.', $titleField->description); + } + + public function testExplicitDescriptionOnNativeEnumViaType(): void + { + $schema = $this->buildSchema(Book::class); + + $genreType = $schema->getType('Genre'); + $this->assertSame('Editorial classification of a book.', $genreType->description); + } + + public function testExtendTypeSuppliesDescriptionWhenBaseTypeHasNone(): void + { + $schema = $this->buildSchema(Book::class); + + $authorType = $schema->getType('Author'); + $this->assertSame('A person who writes books.', $authorType->description); + } + + public function testDocblockFallbackProvidesFieldDescriptionByDefault(): void + { + $schema = $this->buildSchema(Book::class); + + $authorNameField = $schema->getType('Author')->getField('name'); + $this->assertNotNull($authorNameField->description); + $this->assertStringContainsString( + 'Docblock summary that should populate the field description via the fallback path.', + $authorNameField->description, + ); + } + + public function testDisablingDocblockFallbackSuppressesFieldDescription(): void + { + $schema = $this->buildSchema( + Book::class, + docblockDescriptions: false, + ); + + // The `author` query has no explicit description; its description came from the docblock. + // With the toggle off, that docblock must not leak into the public schema. + $authorQuery = $schema->getQueryType()->getField('author'); + $this->assertTrue( + $authorQuery->description === null || $authorQuery->description === '', + 'Expected no description when docblock fallback is disabled, got: ' . var_export($authorQuery->description, true), + ); + + // The `name` field on Author also relied on docblock — it too must be suppressed. + $nameField = $schema->getType('Author')->getField('name'); + $this->assertTrue( + $nameField->description === null || $nameField->description === '', + 'Expected no description when docblock fallback is disabled, got: ' . var_export($nameField->description, true), + ); + + // Explicit descriptions still land in the schema because they do not rely on the toggle. + $bookQuery = $schema->getQueryType()->getField('book'); + $this->assertSame('Fetch a single library book.', $bookQuery->description); + } + + public function testDuplicateDescriptionAcrossTypeAndExtendTypeThrows(): void + { + try { + $schema = $this->buildSchema(DuplicateBook::class); + + // Force resolution — extensions are processed lazily when the target type is first touched. + $schema->getQueryType()->getField('book'); + $schema->getType('DuplicateBook'); + + $this->fail('Expected DuplicateDescriptionOnTypeException to be thrown.'); + } catch (DuplicateDescriptionOnTypeException $exception) { + // The message must name both offending sources so the user can jump straight to the fix. + $this->assertStringContainsString('#[Type] on', $exception->getMessage()); + $this->assertStringContainsString('#[ExtendType] on', $exception->getMessage()); + $this->assertStringContainsString('DescriptionDuplicate\\Book', $exception->getMessage()); + $this->assertStringContainsString('DescriptionDuplicate\\BookExtension', $exception->getMessage()); + } + } +} diff --git a/tests/Middlewares/AuthorizationInputFieldMiddlewareTest.php b/tests/Middlewares/AuthorizationInputFieldMiddlewareTest.php index 604fbaa9e9..ced49626be 100644 --- a/tests/Middlewares/AuthorizationInputFieldMiddlewareTest.php +++ b/tests/Middlewares/AuthorizationInputFieldMiddlewareTest.php @@ -109,7 +109,7 @@ public function handle(InputFieldDescriptor $inputFieldDescriptor): InputField|n originalResolver: $inputFieldDescriptor->getOriginalResolver(), resolver: $inputFieldDescriptor->getResolver(), forConstructorHydration: false, - comment: null, + description: null, isUpdate: false, hasDefaultValue: false, defaultValue: null, diff --git a/tests/Middlewares/CostFieldMiddlewareTest.php b/tests/Middlewares/CostFieldMiddlewareTest.php index b53659360c..3c112c0f82 100644 --- a/tests/Middlewares/CostFieldMiddlewareTest.php +++ b/tests/Middlewares/CostFieldMiddlewareTest.php @@ -96,7 +96,7 @@ public function testAddsCostInDescription(string $expectedDescription, Cost $cos $queryFieldDescriptor->method('getMiddlewareAnnotations') ->willReturn(new MiddlewareAnnotations([$cost])); $queryFieldDescriptor->expects($this->once()) - ->method('withAddedCommentLines') + ->method('withAddedDescriptionLines') ->with($expectedDescription) ->willReturnSelf(); diff --git a/tests/QueryFieldDescriptorTest.php b/tests/QueryFieldDescriptorTest.php index 5626d4272a..f58f778421 100644 --- a/tests/QueryFieldDescriptorTest.php +++ b/tests/QueryFieldDescriptorTest.php @@ -5,13 +5,16 @@ use GraphQL\Type\Definition\Type; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; -use stdClass; use TheCodingMachine\GraphQLite\Middlewares\ServiceResolver; class QueryFieldDescriptorTest extends TestCase { - #[DataProvider('withAddedCommentLineProvider')] - public function testWithAddedCommentLine(string $expected, string|null $previous, string $added): void + #[DataProvider('withAddedDescriptionLineProvider')] + public function testWithAddedDescriptionLine( + string $expected, + string|null $previous, + string $added, + ): void { $resolver = fn () => null; @@ -20,16 +23,16 @@ public function testWithAddedCommentLine(string $expected, string|null $previous Type::string(), resolver: $resolver, originalResolver: new ServiceResolver($resolver), - comment: $previous, - ))->withAddedCommentLines($added); + description: $previous, + ))->withAddedDescriptionLines($added); - self::assertSame($expected, $descriptor->getComment()); + self::assertSame($expected, $descriptor->getDescription()); } - public static function withAddedCommentLineProvider(): iterable + public static function withAddedDescriptionLineProvider(): iterable { yield ['', null, '']; yield ['Asd', null, 'Asd']; - yield ["Some comment\nAsd", 'Some comment', 'Asd']; + yield ["Some description\nAsd", 'Some description', 'Asd']; } } diff --git a/tests/Utils/DescriptionResolverTest.php b/tests/Utils/DescriptionResolverTest.php new file mode 100644 index 0000000000..93624f2d9c --- /dev/null +++ b/tests/Utils/DescriptionResolverTest.php @@ -0,0 +1,84 @@ +resolve($explicit, $docblockDerived)); + } + + public static function provideResolveCases(): iterable + { + // Precedence matrix — covers every combination of explicit value, docblock value, + // and the SchemaFactory-level fallback toggle. + + yield 'explicit non-empty wins over docblock (fallback on)' => [ + 'expected' => 'Explicit wins', + 'useDocblockFallback' => true, + 'explicit' => 'Explicit wins', + 'docblockDerived' => 'From docblock', + ]; + + yield 'explicit non-empty wins when fallback off' => [ + 'expected' => 'Explicit only', + 'useDocblockFallback' => false, + 'explicit' => 'Explicit only', + 'docblockDerived' => 'Would be ignored anyway', + ]; + + yield 'explicit empty string blocks docblock fallback' => [ + 'expected' => '', + 'useDocblockFallback' => true, + 'explicit' => '', + 'docblockDerived' => 'This docblock must NOT leak', + ]; + + yield 'null explicit falls through to docblock when fallback on' => [ + 'expected' => 'From docblock', + 'useDocblockFallback' => true, + 'explicit' => null, + 'docblockDerived' => 'From docblock', + ]; + + yield 'null explicit returns null when fallback off' => [ + 'expected' => null, + 'useDocblockFallback' => false, + 'explicit' => null, + 'docblockDerived' => 'Ignored because fallback disabled', + ]; + + yield 'null explicit + null docblock returns null' => [ + 'expected' => null, + 'useDocblockFallback' => true, + 'explicit' => null, + 'docblockDerived' => null, + ]; + + yield 'null explicit + empty docblock returns empty (caller passes through)' => [ + 'expected' => '', + 'useDocblockFallback' => true, + 'explicit' => null, + 'docblockDerived' => '', + ]; + } + + public function testIsDocblockFallbackEnabledReflectsConstructor(): void + { + self::assertTrue((new DescriptionResolver(true))->isDocblockFallbackEnabled()); + self::assertFalse((new DescriptionResolver(false))->isDocblockFallbackEnabled()); + } +} diff --git a/website/docs/CHANGELOG.md b/website/docs/CHANGELOG.md index 30b292424f..d3a5984e05 100644 --- a/website/docs/CHANGELOG.md +++ b/website/docs/CHANGELOG.md @@ -4,6 +4,11 @@ title: Changelog sidebar_label: Changelog --- +## >8.0.0 + +**For all future changelog details, refer to the [Releases](https://github.com/thecodingmachine/graphqlite/releases). +This CHANGELOG.md will no longer be independently maintained.** + ## 8.0.0 ### Breaking Changes diff --git a/website/docs/annotations-reference.md b/website/docs/annotations-reference.md index 485b1f561c..7ec64e6f6f 100644 --- a/website/docs/annotations-reference.md +++ b/website/docs/annotations-reference.md @@ -17,6 +17,7 @@ Attribute | Compulsory | Type | Definition ---------------|------------|------|-------- name | *no* | string | The name of the query. If skipped, the name of the method is used instead. [outputType](custom-types.mdx) | *no* | string | Forces the GraphQL output type of a query. +description | *no* | string | Description of the query in the documentation. When omitted, the method's PHP docblock summary is used (see [schema descriptions](descriptions.md)). An explicit empty string `''` deliberately suppresses the docblock fallback. ## #[Mutation] @@ -28,6 +29,7 @@ Attribute | Compulsory | Type | Definition ---------------|------------|------|-------- name | *no* | string | The name of the mutation. If skipped, the name of the method is used instead. [outputType](custom-types.mdx) | *no* | string | Forces the GraphQL output type of a query. +description | *no* | string | Description of the mutation in the documentation. When omitted, the method's PHP docblock summary is used (see [schema descriptions](descriptions.md)). An explicit empty string `''` deliberately suppresses the docblock fallback. ## #[Subscription] @@ -39,6 +41,7 @@ Attribute | Compulsory | Type | Definition ---------------|------------|------|-------- name | *no* | string | The name of the subscription. If skipped, the name of the method is used instead. [outputType](custom-types.mdx) | *no* | string | Defines the GraphQL output type that will be sent for the subscription. +description | *no* | string | Description of the subscription in the documentation. When omitted, the method's PHP docblock summary is used (see [schema descriptions](descriptions.md)). An explicit empty string `''` deliberately suppresses the docblock fallback. ## #[Type] @@ -53,6 +56,7 @@ class | *no* | string | The targeted class/enum for the actual ty name | *no* | string | The name of the GraphQL type generated. If not passed, the name of the class is used. If the class ends with "Type", the "Type" suffix is removed default | *no* | bool | Defaults to *true*. Whether the targeted PHP class should be mapped by default to this type. external | *no* | bool | Whether this is an [external type declaration](external-type-declaration.mdx) or not. You usually do not need to use this attribute since this value defaults to true if a "class" attribute is set. This is only useful if you are declaring a type with no PHP class mapping using the "name" attribute. +description | *no* | string | Description of the GraphQL type in the schema documentation. When omitted, the class docblock summary is used (see [schema descriptions](descriptions.md)). An explicit empty string `''` deliberately suppresses the docblock fallback. ## #[ExtendType] @@ -64,6 +68,7 @@ Attribute | Compulsory | Type | Definition ---------------|------------|------|-------- class | see below | string | The targeted class. [The class annotated with `#[ExtendType]` is a service](extend-type.mdx). name | see below | string | The targeted GraphQL output type. +description | *no* | string | Description of the extended GraphQL type. Use this only when the base `#[Type]` does not already declare a description — see [description uniqueness on `#[ExtendType]`](descriptions.md#description-uniqueness-on-extendtype). Cannot be combined with a `description` on the base `#[Type]` or on another `#[ExtendType]` targeting the same class. One and only one of "class" and "name" parameter can be passed at the same time. @@ -219,6 +224,7 @@ Attribute | Compulsory | Type | Definition ---------------|------------|------|-------- name | *no* | string | The name of the input type. If skipped, the name of class returned by the factory is used instead. default | *no* | bool | If `true`, this factory will be used by default for its PHP return type. If set to `false`, you must explicitly [reference this factory using the `#[Parameter]` attribute](input-types.mdx#declaring-several-input-types-for-the-same-php-class). +description | *no* | string | Description of the GraphQL input type produced by this factory. When omitted, the factory method's docblock summary is used (see [schema descriptions](descriptions.md)). An explicit empty string `''` deliberately suppresses the docblock fallback. ## #[UseInputType] diff --git a/website/docs/descriptions.md b/website/docs/descriptions.md new file mode 100644 index 0000000000..dbb2307f1d --- /dev/null +++ b/website/docs/descriptions.md @@ -0,0 +1,106 @@ +--- +id: descriptions +title: Schema descriptions +sidebar_label: Descriptions +--- + +Every schema element that a GraphQL client can see — operations, types, fields, arguments, input +types, enum types — carries a human-readable **description** that GraphiQL and other tooling +surface to API consumers. GraphQLite lets you control that description in two complementary ways: +an explicit argument on the PHP attribute, or a PHP docblock summary used as a fallback. + +## Setting an explicit description + +Every schema-defining attribute accepts a `description` argument: + +```php +use TheCodingMachine\GraphQLite\Annotations\Query; +use TheCodingMachine\GraphQLite\Annotations\Type; + +#[Type(description: 'A library book available for checkout.')] +class Book +{ + #[Field(description: 'The book title as it appears on the cover.')] + public function getTitle(): string { /* ... */ } +} + +class LibraryController +{ + #[Query(description: 'Fetch a single library book.')] + public function book(): Book { /* ... */ } +} +``` + +The attributes that accept `description`: + +- `#[Query]`, `#[Mutation]`, `#[Subscription]` — operations +- `#[Type]`, `#[ExtendType]` — object and enum types +- `#[Factory]` — input types produced by factories +- `#[Field]`, `#[Input]`, `#[SourceField]`, `#[MagicField]` — fields on output and input types + +## Docblock fallback + +When an attribute does not specify a `description`, GraphQLite falls back to the corresponding PHP +docblock summary: + +```php +/** + * Fetch a single library book. + */ +#[Query] +public function book(): Book { /* ... */ } +``` + +Both examples above produce the same GraphQL description (`Fetch a single library book.`) in the +generated schema. Docblock fallback is enabled by default for backwards compatibility. + +## Precedence + +At every schema element, GraphQLite resolves the description from the first source that provides +one: + +1. **Explicit `description: '…'`** on the attribute — always wins. +2. **Explicit `description: ''`** (empty string) — also wins; deliberately publishes an empty + description and suppresses the docblock fallback at that site. Use this when an internal + docblock exists but no public description is desired. +3. **Docblock summary** — used when the attribute did not provide a `description` and docblock + fallback is enabled on the `SchemaFactory`. +4. Otherwise the schema description is empty. + +## Disabling the docblock fallback + +Docblocks double as developer-facing notes (implementation reminders, `@see` references, TODOs). +Publishing them verbatim to the public schema can accidentally leak internal context. Disable the +fallback on the `SchemaFactory` to guarantee that only descriptions explicitly written for API +consumers ever reach the schema: + +```php +$factory = new SchemaFactory($cache, $container); +$factory->setDocblockDescriptionsEnabled(false); +``` + +When disabled, every schema element that lacked an explicit `description` on its attribute will +have no description at all. To publish an empty description for a specific element without +disabling the whole fallback, pass an empty string: + +```php +#[Query(description: '')] +public function internalOnly(): Foo { /* ... */ } +``` + +## Description uniqueness on `#[ExtendType]` + +A GraphQL type has exactly one description, so GraphQLite enforces that the description for a +given type is declared in exactly one place. Valid configurations: + +- `#[Type(description: '…')]` alone — the base type owns the description. +- `#[Type]` with no description plus one `#[ExtendType(description: '…')]` — the extension owns + it. This is useful when an application describes a third-party library's type that did not ship + with a description. +- Neither declares a description — the description falls back to the class docblock summary when + docblock descriptions are enabled, otherwise it is empty. + +If a `description` is declared on both the `#[Type]` and any `#[ExtendType]`, or on more than one +`#[ExtendType]` targeting the same class, schema construction fails fast with a +`TheCodingMachine\GraphQLite\Annotations\Exceptions\DuplicateDescriptionOnTypeException`. The +exception message names every offending source so the conflict can be resolved without guesswork. diff --git a/website/docs/field-middlewares.md b/website/docs/field-middlewares.md index df7b2bfaee..63cf50e6b8 100644 --- a/website/docs/field-middlewares.md +++ b/website/docs/field-middlewares.md @@ -47,8 +47,9 @@ class QueryFieldDescriptor public function withTargetMethodOnSource(?string $targetMethodOnSource): self { /* ... */ } public function isInjectSource(): bool { /* ... */ } public function withInjectSource(bool $injectSource): self { /* ... */ } - public function getComment(): ?string { /* ... */ } - public function withComment(?string $comment): self { /* ... */ } + public function getDescription(): ?string { /* ... */ } + public function withDescription(?string $description): self { /* ... */ } + public function withAddedDescriptionLines(string $description): self { /* ... */ } public function getMiddlewareAnnotations(): MiddlewareAnnotations { /* ... */ } public function withMiddlewareAnnotations(MiddlewareAnnotations $middlewareAnnotations): self { /* ... */ } public function getOriginalResolver(): ResolverInterface { /* ... */ } diff --git a/website/docs/other-frameworks.mdx b/website/docs/other-frameworks.mdx index e2684c0e9f..73e27861a9 100644 --- a/website/docs/other-frameworks.mdx +++ b/website/docs/other-frameworks.mdx @@ -70,6 +70,11 @@ $factory->addQueryProviderFactory($queryProviderFactory); $factory->setInputTypeValidator($validator); // Add custom options to the Webonyx underlying Schema. $factory->setSchemaConfig($schemaConfig); +// Control whether PHP docblock summaries are used as a fallback description for schema elements +// that do not provide an explicit `description` on their attribute. Enabled by default for +// backwards compatibility. See [Schema descriptions](descriptions.md) for the precedence rule +// and security rationale. +$factory->setDocblockDescriptionsEnabled(false); // Configures the time-to-live for the GraphQLite cache. Defaults to 2 seconds in dev mode. $factory->setGlobTtl(2); // Enables prod-mode (cache settings optimized for best performance). diff --git a/website/sidebars.json b/website/sidebars.json index 571f243dba..4762f71519 100755 --- a/website/sidebars.json +++ b/website/sidebars.json @@ -15,6 +15,7 @@ "mutations", "subscriptions", "type-mapping", + "descriptions", "autowiring", "extend-type", "external-type-declaration", From cc04b22d0b14c5fbf2653b16a4386b943a81913f Mon Sep 17 00:00:00 2001 From: Jacob Thomason Date: Sat, 18 Apr 2026 16:59:24 -0400 Subject: [PATCH 2/8] feat: #[EnumValue] attribute for per-case enum metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the deeper portion of #740 that the original PR left as future work: native PHP 8.1+ enum cases can now carry an explicit GraphQL description and deprecation reason without relying on docblock parsing. ```php #[Type] enum Genre: string { #[EnumValue(description: 'Fiction works including novels and short stories.')] case Fiction = 'fiction'; #[EnumValue(deprecationReason: 'Use Fiction::Verse instead.')] case Poetry = 'poetry'; } ``` Naming: follows the GraphQL specification's term ("enum values", §3.5.2, `__EnumValue`, `enumValues`) rather than the PHP language term "case". This matches every other graphqlite attribute (`Type`, `Field`, `Query`, `ExtendType`, …) which mirrors GraphQL spec names, and webonyx/graphql-php's internal `EnumValueDefinition`. Targets `Attribute::TARGET_CLASS_CONSTANT` so it applies to PHP enum cases. Precedence: explicit `description` / `deprecationReason` on the attribute win over docblock summary / `@deprecated` tag. Passing `''` deliberately suppresses the fallback at that site, matching every other `description` argument across the library. No-attribute cases continue to fall back to docblock — BC preserved. Adds `AnnotationReader::getEnumValueAnnotation(ReflectionEnumUnitCase)` helper and threads it through `EnumTypeMapper::mapByClassName()` where enum case metadata is aggregated. Tests: 5 new unit tests (attribute defaults / description / deprecation reason / both / empty string) plus 4 new integration tests over a new fixture enum (attribute-supplied description, docblock fallback, explicit deprecation, toggle-off suppresses docblock-only case). Full suite 537/537 green across cs-check, phpstan, and phpunit. --- src/AnnotationReader.php | 20 ++++++++++ src/Annotations/EnumValue.php | 52 ++++++++++++++++++++++++++ src/Mappers/Root/EnumTypeMapper.php | 22 +++++++++-- tests/Annotations/EnumValueTest.php | 53 +++++++++++++++++++++++++++ tests/Fixtures/Description/Genre.php | 9 +++++ tests/Integration/DescriptionTest.php | 48 ++++++++++++++++++++++++ website/docs/annotations-reference.md | 24 ++++++++++++ website/docs/descriptions.md | 41 +++++++++++++++++++++ 8 files changed, 265 insertions(+), 4 deletions(-) create mode 100644 src/Annotations/EnumValue.php create mode 100644 tests/Annotations/EnumValueTest.php diff --git a/src/AnnotationReader.php b/src/AnnotationReader.php index bfde61c324..fd1fc137fe 100644 --- a/src/AnnotationReader.php +++ b/src/AnnotationReader.php @@ -5,12 +5,14 @@ namespace TheCodingMachine\GraphQLite; use ReflectionClass; +use ReflectionEnumUnitCase; use ReflectionMethod; use ReflectionParameter; use ReflectionProperty; use TheCodingMachine\GraphQLite\Annotations\AbstractGraphQLElement; use TheCodingMachine\GraphQLite\Annotations\Decorate; use TheCodingMachine\GraphQLite\Annotations\EnumType; +use TheCodingMachine\GraphQLite\Annotations\EnumValue; use TheCodingMachine\GraphQLite\Annotations\Exceptions\ClassNotFoundException; use TheCodingMachine\GraphQLite\Annotations\Exceptions\InvalidParameterException; use TheCodingMachine\GraphQLite\Annotations\ExtendType; @@ -201,6 +203,24 @@ public function getEnumTypeAnnotation(ReflectionClass $refClass): EnumType|null return $this->getClassAnnotation($refClass, EnumType::class); } + /** + * Returns the {@see EnumValue} attribute declared on a PHP enum case, or null when no + * attribute is present. Callers use this to resolve the explicit description and deprecation + * reason before falling back to docblock parsing. + */ + public function getEnumValueAnnotation(ReflectionEnumUnitCase $refCase): EnumValue|null + { + $attribute = $refCase->getAttributes(EnumValue::class)[0] ?? null; + if ($attribute === null) { + return null; + } + + $instance = $attribute->newInstance(); + assert($instance instanceof EnumValue); + + return $instance; + } + /** @param class-string $annotationClass */ public function getGraphQLElementAnnotation(ReflectionMethod $refMethod, string $annotationClass): AbstractGraphQLElement|null { diff --git a/src/Annotations/EnumValue.php b/src/Annotations/EnumValue.php new file mode 100644 index 0000000000..b1ad82c998 --- /dev/null +++ b/src/Annotations/EnumValue.php @@ -0,0 +1,52 @@ + $enumCaseDescriptions */ + /** @var array $enumCaseDescriptions */ $enumCaseDescriptions = []; /** @var array $enumCaseDeprecationReasons */ $enumCaseDeprecationReasons = []; foreach ($reflectionEnum->getCases() as $reflectionEnumCase) { $docBlock = $this->docBlockFactory->create($reflectionEnumCase); + $enumValueAttribute = $this->annotationReader->getEnumValueAnnotation($reflectionEnumCase); + + $enumCaseDescriptions[$reflectionEnumCase->getName()] = $this->descriptionResolver->resolve( + $enumValueAttribute?->description, + $docBlock->getSummary() ?: null, + ); + + $explicitDeprecation = $enumValueAttribute?->deprecationReason; + if ($explicitDeprecation !== null) { + // Explicit `deprecationReason` always wins; an empty string deliberately clears + // any @deprecated tag on the case docblock the same way an empty description + // blocks the docblock fallback. + if ($explicitDeprecation !== '') { + $enumCaseDeprecationReasons[$reflectionEnumCase->getName()] = $explicitDeprecation; + } + continue; + } - $enumCaseDescriptions[$reflectionEnumCase->getName()] = $this->descriptionResolver->isDocblockFallbackEnabled() - ? ($docBlock->getSummary() ?: null) - : null; $deprecation = $docBlock->getTagsByName('deprecated')[0] ?? null; // phpcs:ignore diff --git a/tests/Annotations/EnumValueTest.php b/tests/Annotations/EnumValueTest.php new file mode 100644 index 0000000000..856458adca --- /dev/null +++ b/tests/Annotations/EnumValueTest.php @@ -0,0 +1,53 @@ +assertNull($enumValue->description); + $this->assertNull($enumValue->deprecationReason); + } + + public function testDescriptionOnly(): void + { + $enumValue = new EnumValue(description: 'Fiction genre.'); + + $this->assertSame('Fiction genre.', $enumValue->description); + $this->assertNull($enumValue->deprecationReason); + } + + public function testDeprecationReasonOnly(): void + { + $enumValue = new EnumValue(deprecationReason: 'Use Essay instead.'); + + $this->assertNull($enumValue->description); + $this->assertSame('Use Essay instead.', $enumValue->deprecationReason); + } + + public function testBothValues(): void + { + $enumValue = new EnumValue( + description: 'Fiction works.', + deprecationReason: 'Use a subgenre.', + ); + + $this->assertSame('Fiction works.', $enumValue->description); + $this->assertSame('Use a subgenre.', $enumValue->deprecationReason); + } + + public function testDescriptionPreservesEmptyString(): void + { + // '' is the deliberate "explicit empty" signal that blocks docblock fallback downstream. + $enumValue = new EnumValue(description: ''); + + $this->assertSame('', $enumValue->description); + } +} diff --git a/tests/Fixtures/Description/Genre.php b/tests/Fixtures/Description/Genre.php index e3e9732056..cf2907e746 100644 --- a/tests/Fixtures/Description/Genre.php +++ b/tests/Fixtures/Description/Genre.php @@ -4,6 +4,7 @@ namespace TheCodingMachine\GraphQLite\Fixtures\Description; +use TheCodingMachine\GraphQLite\Annotations\EnumValue; use TheCodingMachine\GraphQLite\Annotations\Type; /** @@ -12,7 +13,15 @@ #[Type(description: 'Editorial classification of a book.')] enum Genre: string { + #[EnumValue(description: 'Fiction works including novels and short stories.')] case Fiction = 'fiction'; + + /** + * This docblock description should appear on the NonFiction enum value because no + * #[EnumValue] attribute is declared — it exercises the docblock fallback. + */ case NonFiction = 'non-fiction'; + + #[EnumValue(deprecationReason: 'Use Fiction::Verse instead.')] case Poetry = 'poetry'; } diff --git a/tests/Integration/DescriptionTest.php b/tests/Integration/DescriptionTest.php index 6d17ba8f74..ccb20c878d 100644 --- a/tests/Integration/DescriptionTest.php +++ b/tests/Integration/DescriptionTest.php @@ -96,6 +96,54 @@ public function testExplicitDescriptionOnNativeEnumViaType(): void $this->assertSame('Editorial classification of a book.', $genreType->description); } + public function testEnumValueAttributeProvidesCaseDescription(): void + { + $schema = $this->buildSchema(Book::class); + + $genreType = $schema->getType('Genre'); + $fictionValue = $genreType->getValue('Fiction'); + $this->assertSame('Fiction works including novels and short stories.', $fictionValue->description); + } + + public function testEnumCaseWithoutAttributeFallsBackToDocblock(): void + { + $schema = $this->buildSchema(Book::class); + + $genreType = $schema->getType('Genre'); + $nonFictionValue = $genreType->getValue('NonFiction'); + // The NonFiction case has no #[EnumValue] attribute, so its description comes from the docblock. + $this->assertNotNull($nonFictionValue->description); + $this->assertStringContainsString( + 'This docblock description should appear on the NonFiction enum value', + $nonFictionValue->description, + ); + } + + public function testEnumValueAttributeProvidesDeprecationReason(): void + { + $schema = $this->buildSchema(Book::class); + + $genreType = $schema->getType('Genre'); + $poetryValue = $genreType->getValue('Poetry'); + $this->assertSame('Use Fiction::Verse instead.', $poetryValue->deprecationReason); + } + + public function testDisablingDocblockFallbackSuppressesEnumCaseDescription(): void + { + $schema = $this->buildSchema(Book::class, docblockDescriptions: false); + + $genreType = $schema->getType('Genre'); + + // Fiction has an explicit #[EnumValue] description — still present. + $this->assertSame( + 'Fiction works including novels and short stories.', + $genreType->getValue('Fiction')->description, + ); + + // NonFiction relied on its docblock summary — with the toggle off, it must disappear. + $this->assertNull($genreType->getValue('NonFiction')->description); + } + public function testExtendTypeSuppliesDescriptionWhenBaseTypeHasNone(): void { $schema = $this->buildSchema(Book::class); diff --git a/website/docs/annotations-reference.md b/website/docs/annotations-reference.md index 7ec64e6f6f..9016bf8c1f 100644 --- a/website/docs/annotations-reference.md +++ b/website/docs/annotations-reference.md @@ -311,6 +311,30 @@ Attribute | Compulsory | Type | Definition *for* | *yes* | string | The name of the PHP parameter *constraint* | *yes | annotation | One (or many) Symfony validation attributes. +## #[EnumValue] + +The `#[EnumValue]` attribute attaches GraphQL schema metadata (description, deprecation reason) +to an individual case of a PHP 8.1+ native enum that is exposed as a GraphQL enum type. + +**Applies on**: cases of an enum annotated (directly or indirectly) with `#[Type]`. + +Attribute | Compulsory | Type | Definition +------------------|------------|--------|----------- +description | *no* | string | Description of the enum value. When omitted, the case's PHP docblock summary is used (see [schema descriptions](descriptions.md#enum-value-descriptions)). An explicit empty string `''` deliberately suppresses the docblock fallback. +deprecationReason | *no* | string | Deprecation reason published to the schema. When omitted, the `@deprecated` tag on the case docblock is used. An explicit empty string `''` deliberately clears any inherited `@deprecated` tag. + +```php +#[Type] +enum Genre: string +{ + #[EnumValue(description: 'Fiction works including novels and short stories.')] + case Fiction = 'fiction'; + + #[EnumValue(deprecationReason: 'Use Fiction::Verse instead.')] + case Poetry = 'poetry'; +} +``` + ## ~~@EnumType~~ *Deprecated: Use [PHP 8.1's native Enums](https://www.php.net/manual/en/language.types.enumerations.php) instead with a [#[Type]](#type-annotation).* diff --git a/website/docs/descriptions.md b/website/docs/descriptions.md index dbb2307f1d..4358254a1b 100644 --- a/website/docs/descriptions.md +++ b/website/docs/descriptions.md @@ -37,6 +37,7 @@ The attributes that accept `description`: - `#[Type]`, `#[ExtendType]` — object and enum types - `#[Factory]` — input types produced by factories - `#[Field]`, `#[Input]`, `#[SourceField]`, `#[MagicField]` — fields on output and input types +- `#[EnumValue]` — individual cases of an enum type (see [Enum value descriptions](#enum-value-descriptions)) ## Docblock fallback @@ -88,6 +89,46 @@ disabling the whole fallback, pass an empty string: public function internalOnly(): Foo { /* ... */ } ``` +## Enum value descriptions + +Native PHP 8.1 enums mapped to GraphQL enum types get per-case metadata via the `#[EnumValue]` +attribute applied to individual cases: + +```php +use TheCodingMachine\GraphQLite\Annotations\EnumValue; +use TheCodingMachine\GraphQLite\Annotations\Type; + +#[Type] +enum Genre: string +{ + #[EnumValue(description: 'Fiction works including novels and short stories.')] + case Fiction = 'fiction'; + + #[EnumValue(deprecationReason: 'Use Fiction::Verse instead.')] + case Poetry = 'poetry'; + + /** + * Works grounded in verifiable facts. + */ + case NonFiction = 'non-fiction'; // no attribute — description comes from the docblock +} +``` + +The attribute name mirrors the GraphQL specification's term ("enum values", see +[spec §3.5.2](https://spec.graphql.org/October2021/#sec-Enum-Values)) and matches webonyx/graphql-php's +`EnumValueDefinition`. The underlying PHP construct is a `case`; the GraphQL element it produces +is an enum value. + +`#[EnumValue]` accepts: + +- `description` — schema description for this enum value. Omitting it falls back to the case + docblock summary, subject to the same precedence rules as every other attribute's + `description` argument. An explicit empty string `''` deliberately suppresses the docblock + fallback. +- `deprecationReason` — published as the enum value's `deprecationReason` in the schema. + Omitting it falls back to the `@deprecated` tag on the case docblock. An explicit empty string + `''` deliberately clears any inherited `@deprecated` tag. + ## Description uniqueness on `#[ExtendType]` A GraphQL type has exactly one description, so GraphQLite enforces that the description for a From 3fe283fba751431486cebfbfa13ebe683a232008 Mon Sep 17 00:00:00 2001 From: Jacob Thomason Date: Sat, 18 Apr 2026 17:33:15 -0400 Subject: [PATCH 3/8] feat: deprecation notice for enum cases missing #[EnumValue] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Announces the upcoming opt-in migration for PHP enums mapped to GraphQL enum types. Today every case is automatically exposed; a future major release will require #[EnumValue] on each case that should participate in the schema, matching the opt-in model #[Field] already uses for methods and properties on object types. The practical win is selective exposure: an internal enum case can opt out of the public schema simply by omitting #[EnumValue], instead of forcing schema authors to split an enum or rename cases. Runtime behaviour is unchanged. When an enum annotated with #[Type] exposes any case without an #[EnumValue] attribute, EnumTypeMapper emits an E_USER_DEPRECATED notice that names the specific cases so the migration path is mechanical — matching graphqlite's existing deprecation pattern (e.g. addControllerNamespace, setGlobTTL). Tests: +1 explicit deprecation assertion (using PHPUnit 11's expectUserDeprecationMessageMatches). Existing integration tests that exercise the Genre fixture surface the notice via PHPUnit's deprecation reporting, confirming the warning fires in realistic scenarios. Full suite 538/538 green across cs-check, phpstan, and phpunit; 4 intentional deprecation events reported. Docs: descriptions.md gains a "Future migration" subsection describing the planned opt-in model and the current advisory notice. --- src/Mappers/Root/EnumTypeMapper.php | 45 +++++++++++++++++++++++++++ tests/Integration/DescriptionTest.php | 14 +++++++++ website/docs/descriptions.md | 19 +++++++++++ 3 files changed, 78 insertions(+) diff --git a/src/Mappers/Root/EnumTypeMapper.php b/src/Mappers/Root/EnumTypeMapper.php index 9a0927020b..25e25598e6 100644 --- a/src/Mappers/Root/EnumTypeMapper.php +++ b/src/Mappers/Root/EnumTypeMapper.php @@ -30,7 +30,12 @@ use function array_values; use function assert; use function enum_exists; +use function implode; use function ltrim; +use function sprintf; +use function trigger_error; + +use const E_USER_DEPRECATED; /** * Maps an enum class to a GraphQL type (only available in PHP>=8.1) @@ -143,11 +148,17 @@ private function mapByClassName(string $enumClass): EnumType|null $enumCaseDescriptions = []; /** @var array $enumCaseDeprecationReasons */ $enumCaseDeprecationReasons = []; + /** @var list $casesMissingEnumValueAttribute */ + $casesMissingEnumValueAttribute = []; foreach ($reflectionEnum->getCases() as $reflectionEnumCase) { $docBlock = $this->docBlockFactory->create($reflectionEnumCase); $enumValueAttribute = $this->annotationReader->getEnumValueAnnotation($reflectionEnumCase); + if ($enumValueAttribute === null) { + $casesMissingEnumValueAttribute[] = $reflectionEnumCase->getName(); + } + $enumCaseDescriptions[$reflectionEnumCase->getName()] = $this->descriptionResolver->resolve( $enumValueAttribute?->description, $docBlock->getSummary() ?: null, @@ -172,11 +183,45 @@ private function mapByClassName(string $enumClass): EnumType|null } } + $this->warnAboutCasesMissingEnumValueAttribute($enumClass, $casesMissingEnumValueAttribute); + $type = new EnumType($enumClass, $typeName, $enumDescription, $enumCaseDescriptions, $enumCaseDeprecationReasons, $useValues); return $this->cacheByName[$type->name] = $this->cacheByClass[$enumClass] = $type; } + /** + * Emits a deprecation notice when a GraphQL-mapped enum exposes one or more cases without a + * matching {@see EnumValue} attribute. + * + * Today every case is automatically exposed in the schema — this call site keeps that + * behaviour intact. The notice announces the planned migration: a future major release will + * require `#[EnumValue]` on each case that should participate in the schema, mirroring + * `#[Field]`'s opt-in model on classes. Until then, adopters can start annotating cases + * incrementally; the warning lists the specific cases that would be dropped after the + * future default flip so the migration path is mechanical. + * + * @param class-string $enumClass + * @param list $casesMissingAttribute + */ + private function warnAboutCasesMissingEnumValueAttribute(string $enumClass, array $casesMissingAttribute): void + { + if ($casesMissingAttribute === []) { + return; + } + + trigger_error( + sprintf( + 'Enum "%s" is mapped to a GraphQL enum type but exposes one or more cases without a #[EnumValue] attribute. ' + . 'Today every case is automatically exposed; a future major release will require #[EnumValue] on each case that should participate in the schema, mirroring #[Field]\'s opt-in model. ' + . 'Add #[EnumValue] to each case you want to keep exposed. Cases currently exposed without an attribute: %s.', + $enumClass, + implode(', ', $casesMissingAttribute), + ), + E_USER_DEPRECATED, + ); + } + private function getTypeName(ReflectionClass $reflectionClass): string { $typeAnnotation = $this->annotationReader->getTypeAnnotation($reflectionClass); diff --git a/tests/Integration/DescriptionTest.php b/tests/Integration/DescriptionTest.php index ccb20c878d..2ca4f5e92c 100644 --- a/tests/Integration/DescriptionTest.php +++ b/tests/Integration/DescriptionTest.php @@ -105,6 +105,20 @@ public function testEnumValueAttributeProvidesCaseDescription(): void $this->assertSame('Fiction works including novels and short stories.', $fictionValue->description); } + public function testEnumWithCasesMissingEnumValueAttributeTriggersDeprecation(): void + { + // The Genre fixture deliberately leaves NonFiction without #[EnumValue] to exercise the + // docblock-fallback path and to surface the deprecation announcing the future opt-in + // migration. PHPUnit 11's expectUserDeprecationMessageMatches hooks into the library's + // own error handler so the assertion works consistently with how deprecations surface + // in CI output. + $this->expectUserDeprecationMessageMatches('/#\[EnumValue\].*future major.*NonFiction/s'); + + $schema = $this->buildSchema(Book::class); + // Force enum resolution — types are lazy-mapped until referenced. + $schema->getType('Genre'); + } + public function testEnumCaseWithoutAttributeFallsBackToDocblock(): void { $schema = $this->buildSchema(Book::class); diff --git a/website/docs/descriptions.md b/website/docs/descriptions.md index 4358254a1b..40ccd377d0 100644 --- a/website/docs/descriptions.md +++ b/website/docs/descriptions.md @@ -129,6 +129,25 @@ is an enum value. Omitting it falls back to the `@deprecated` tag on the case docblock. An explicit empty string `''` deliberately clears any inherited `@deprecated` tag. +### Future migration: `#[EnumValue]` will become required per case + +Today every case of a `#[Type]`-mapped enum is automatically exposed in the GraphQL schema. A +future major release will flip this to an opt-in model: **only cases carrying an explicit +`#[EnumValue]` attribute will be exposed**, mirroring the way `#[Field]` opts individual +methods and properties into an object type. + +The benefit is selective exposure — today there is no way to map a subset of a PHP enum into +GraphQL, which forces schema authors to split an enum into two or rename cases. Under the +opt-in model, an internal enum case can simply omit `#[EnumValue]` to stay out of the public +schema. + +To surface the upcoming change, GraphQLite already emits a PHP `E_USER_DEPRECATED` notice at +schema build time when an enum annotated with `#[Type]` exposes any case without an +`#[EnumValue]` attribute. The notice names the specific cases that would be dropped after the +flip so the migration path is mechanical: add `#[EnumValue]` to every case you want to keep. +No runtime behaviour changes today — the notice only signals what the future default will +require. + ## Description uniqueness on `#[ExtendType]` A GraphQL type has exactly one description, so GraphQLite enforces that the description for a From 0fb27ec9c3ce16902f101b9d80bbd9b5ae6a7eb3 Mon Sep 17 00:00:00 2001 From: Jacob Thomason Date: Sat, 18 Apr 2026 17:36:02 -0400 Subject: [PATCH 4/8] docs: clarify runtime behaviour for unmapped enum cases after future flip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The advisory deprecation already announces the opt-in migration. Add a second paragraph spelling out what happens if a resolver returns a case that lacks #[EnumValue] after the default flips: webonyx/graphql-php's native enum serialization rejects it, the same spec-compliant behaviour that applies to any unknown enum value. That is the mechanism that makes selective exposure safe — internal cases cannot leak via a resolver — and clarifies that omitting #[EnumValue] is a deliberate 'do not expose this value' signal, not an oversight to work around. --- website/docs/descriptions.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/website/docs/descriptions.md b/website/docs/descriptions.md index 40ccd377d0..74794c3804 100644 --- a/website/docs/descriptions.md +++ b/website/docs/descriptions.md @@ -148,6 +148,14 @@ flip so the migration path is mechanical: add `#[EnumValue]` to every case you w No runtime behaviour changes today — the notice only signals what the future default will require. +**Runtime implication after the flip.** Cases without `#[EnumValue]` will not exist in the +GraphQL schema, so a resolver that returns such a case will trigger webonyx/graphql-php's +standard enum serialization error (the value is not listed in the enum type's `values` +config). This is the same spec-compliant behaviour that applies to any unknown enum value and +is the mechanism that makes selective exposure safe: internal cases cannot accidentally leak +via a resolver. Developers who want a case to remain returnable must keep `#[EnumValue]` on +it; omitting the attribute is a deliberate "do not expose this value" signal. + ## Description uniqueness on `#[ExtendType]` A GraphQL type has exactly one description, so GraphQLite enforces that the description for a From 077a4842a47a22dfd1eaa2c529777f9f3ddf785b Mon Sep 17 00:00:00 2001 From: Jacob Thomason Date: Sat, 18 Apr 2026 17:46:30 -0400 Subject: [PATCH 5/8] fix: narrow enum advisory to fire only when zero cases carry #[EnumValue] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Partial annotation is the mechanism that will hide internal cases from the public schema once the default flips to opt-in; leaving some cases unannotated is deliberate and must not produce a warning. Under the previous logic the advisory fired for every partial annotation, which would punish exactly the pattern the migration encourages. New rule: fire the E_USER_DEPRECATED advisory only when a #[Type]-mapped enum declares zero #[EnumValue] attributes across all its cases — the signal that the developer has not yet engaged with the opt-in model. Annotating even a single case acknowledges the migration and silences the notice; from that point any combination of annotated and unannotated cases is correct and intentional. Implementation: EnumTypeMapper::mapByClassName() now tracks a single `sawAnyEnumValueAttribute` flag while iterating cases and emits the notice only when the flag stays false. Message rewritten to explain the "at least one case" requirement and point the reader at the intended migration path — including the fact that a bare #[EnumValue] on a single case is a valid acknowledgement. Tests: - Replaces the previous "any case missing" assertion with testEnumWithZeroEnumValueAttributesTriggersDeprecation using a new DescriptionLegacyEnum/Era fixture that declares no #[EnumValue] at all. - Adds testEnumWithPartialEnumValueAttributesIsSilent to lock in the partial-annotation contract against regressions. - Pre-existing Color/Size/Position fixtures gain a bare #[EnumValue] on their first case so their integration tests stop triggering the advisory — demonstrates the minimal upgrade path recommended in the docs. Full suite 539/539 green across cs-check, phpstan, and phpunit, with exactly 1 intentional deprecation coming from the dedicated advisory test. The docs in descriptions.md are updated to reflect the corrected semantics — "partial annotation is intentional and silent". --- src/Mappers/Root/EnumTypeMapper.php | 47 +++++++++--------- tests/Fixtures/DescriptionLegacyEnum/Era.php | 19 ++++++++ .../DescriptionLegacyEnum/EraController.php | 16 +++++++ tests/Fixtures/Integration/Models/Color.php | 5 ++ .../Fixtures/Integration/Models/Position.php | 2 + tests/Fixtures/Integration/Models/Size.php | 2 + tests/Integration/DescriptionTest.php | 48 +++++++++++++++---- website/docs/descriptions.md | 40 +++++++++++----- 8 files changed, 133 insertions(+), 46 deletions(-) create mode 100644 tests/Fixtures/DescriptionLegacyEnum/Era.php create mode 100644 tests/Fixtures/DescriptionLegacyEnum/EraController.php diff --git a/src/Mappers/Root/EnumTypeMapper.php b/src/Mappers/Root/EnumTypeMapper.php index 25e25598e6..13ca56c285 100644 --- a/src/Mappers/Root/EnumTypeMapper.php +++ b/src/Mappers/Root/EnumTypeMapper.php @@ -30,7 +30,6 @@ use function array_values; use function assert; use function enum_exists; -use function implode; use function ltrim; use function sprintf; use function trigger_error; @@ -148,15 +147,14 @@ private function mapByClassName(string $enumClass): EnumType|null $enumCaseDescriptions = []; /** @var array $enumCaseDeprecationReasons */ $enumCaseDeprecationReasons = []; - /** @var list $casesMissingEnumValueAttribute */ - $casesMissingEnumValueAttribute = []; + $sawAnyEnumValueAttribute = false; foreach ($reflectionEnum->getCases() as $reflectionEnumCase) { $docBlock = $this->docBlockFactory->create($reflectionEnumCase); $enumValueAttribute = $this->annotationReader->getEnumValueAnnotation($reflectionEnumCase); - if ($enumValueAttribute === null) { - $casesMissingEnumValueAttribute[] = $reflectionEnumCase->getName(); + if ($enumValueAttribute !== null) { + $sawAnyEnumValueAttribute = true; } $enumCaseDescriptions[$reflectionEnumCase->getName()] = $this->descriptionResolver->resolve( @@ -183,7 +181,9 @@ private function mapByClassName(string $enumClass): EnumType|null } } - $this->warnAboutCasesMissingEnumValueAttribute($enumClass, $casesMissingEnumValueAttribute); + if (! $sawAnyEnumValueAttribute) { + $this->warnEnumHasNoEnumValueAttributes($enumClass); + } $type = new EnumType($enumClass, $typeName, $enumDescription, $enumCaseDescriptions, $enumCaseDeprecationReasons, $useValues); @@ -191,32 +191,29 @@ private function mapByClassName(string $enumClass): EnumType|null } /** - * Emits a deprecation notice when a GraphQL-mapped enum exposes one or more cases without a - * matching {@see EnumValue} attribute. + * Emits a deprecation notice when a GraphQL-mapped enum declares zero {@see EnumValue} + * attributes across its cases — the signal that the developer has not yet engaged with + * the opt-in model that a future major release will require. * - * Today every case is automatically exposed in the schema — this call site keeps that - * behaviour intact. The notice announces the planned migration: a future major release will - * require `#[EnumValue]` on each case that should participate in the schema, mirroring - * `#[Field]`'s opt-in model on classes. Until then, adopters can start annotating cases - * incrementally; the warning lists the specific cases that would be dropped after the - * future default flip so the migration path is mechanical. + * Today every case is automatically exposed in the schema regardless of `#[EnumValue]` — + * this call site keeps that behaviour intact. The notice announces the planned migration: + * a future major release will require at least one `#[EnumValue]`-annotated case per + * `#[Type]`-mapped enum, and only annotated cases will participate in the schema + * (mirroring `#[Field]`'s opt-in model on classes). Partial annotation is deliberately + * allowed and intentionally silent — leaving some cases unannotated is the mechanism that + * hides internal cases from the public schema once the default flips, so it must not + * itself produce a warning. * - * @param class-string $enumClass - * @param list $casesMissingAttribute + * @param class-string $enumClass */ - private function warnAboutCasesMissingEnumValueAttribute(string $enumClass, array $casesMissingAttribute): void + private function warnEnumHasNoEnumValueAttributes(string $enumClass): void { - if ($casesMissingAttribute === []) { - return; - } - trigger_error( sprintf( - 'Enum "%s" is mapped to a GraphQL enum type but exposes one or more cases without a #[EnumValue] attribute. ' - . 'Today every case is automatically exposed; a future major release will require #[EnumValue] on each case that should participate in the schema, mirroring #[Field]\'s opt-in model. ' - . 'Add #[EnumValue] to each case you want to keep exposed. Cases currently exposed without an attribute: %s.', + 'Enum "%s" is mapped to a GraphQL enum type but declares no #[EnumValue] attributes on any case. ' + . 'Today every case is automatically exposed; a future major release will require at least one #[EnumValue]-annotated case per #[Type]-mapped enum, and only annotated cases will participate in the schema (mirroring #[Field]\'s opt-in model on classes). ' + . 'Add #[EnumValue] to the case(s) you want to expose — annotating at least one case acknowledges the opt-in model and silences this notice. Cases left unannotated will be hidden from the schema after the future default flip, which is the intended way to keep internal values out of the public API.', $enumClass, - implode(', ', $casesMissingAttribute), ), E_USER_DEPRECATED, ); diff --git a/tests/Fixtures/DescriptionLegacyEnum/Era.php b/tests/Fixtures/DescriptionLegacyEnum/Era.php new file mode 100644 index 0000000000..8af32c6004 --- /dev/null +++ b/tests/Fixtures/DescriptionLegacyEnum/Era.php @@ -0,0 +1,19 @@ +assertSame('Fiction works including novels and short stories.', $fictionValue->description); } - public function testEnumWithCasesMissingEnumValueAttributeTriggersDeprecation(): void + public function testEnumWithZeroEnumValueAttributesTriggersDeprecation(): void { - // The Genre fixture deliberately leaves NonFiction without #[EnumValue] to exercise the - // docblock-fallback path and to surface the deprecation announcing the future opt-in - // migration. PHPUnit 11's expectUserDeprecationMessageMatches hooks into the library's - // own error handler so the assertion works consistently with how deprecations surface - // in CI output. - $this->expectUserDeprecationMessageMatches('/#\[EnumValue\].*future major.*NonFiction/s'); + // The Era fixture deliberately declares zero #[EnumValue] attributes — the signal that + // the developer has not yet engaged with the opt-in migration. That is the scenario the + // advisory targets; partial annotation on other enums (like Genre in the Description + // namespace) is deliberately silent because it already acknowledges the new model. + $this->expectUserDeprecationMessageMatches('/declares no #\[EnumValue\] attributes.*future major/s'); - $schema = $this->buildSchema(Book::class); + $schema = $this->buildSchema(Era::class); // Force enum resolution — types are lazy-mapped until referenced. - $schema->getType('Genre'); + $schema->getType('Era'); + } + + public function testEnumWithPartialEnumValueAttributesIsSilent(): void + { + // Genre has #[EnumValue] on Fiction and Poetry but not NonFiction. Partial annotation is + // deliberately OK: leaving a case unannotated is the mechanism for hiding it from the + // public schema after the future default flip, and therefore must not itself produce an + // advisory. Asserting no deprecation here locks that contract in. + $captured = []; + set_error_handler( + static function (int $errno, string $errstr) use (&$captured): bool { + if ($errno === E_USER_DEPRECATED && str_contains($errstr, 'EnumValue')) { + $captured[] = $errstr; + + return true; + } + + return false; + }, + E_USER_DEPRECATED, + ); + + try { + $schema = $this->buildSchema(Book::class); + $schema->getType('Genre'); + } finally { + restore_error_handler(); + } + + $this->assertSame([], $captured, 'Partial #[EnumValue] annotation must not trigger the advisory notice.'); } public function testEnumCaseWithoutAttributeFallsBackToDocblock(): void diff --git a/website/docs/descriptions.md b/website/docs/descriptions.md index 74794c3804..b1afef1270 100644 --- a/website/docs/descriptions.md +++ b/website/docs/descriptions.md @@ -129,24 +129,40 @@ is an enum value. Omitting it falls back to the `@deprecated` tag on the case docblock. An explicit empty string `''` deliberately clears any inherited `@deprecated` tag. -### Future migration: `#[EnumValue]` will become required per case +### Future migration: `#[EnumValue]` will become required per `#[Type]`-mapped enum Today every case of a `#[Type]`-mapped enum is automatically exposed in the GraphQL schema. A -future major release will flip this to an opt-in model: **only cases carrying an explicit -`#[EnumValue]` attribute will be exposed**, mirroring the way `#[Field]` opts individual -methods and properties into an object type. +future major release will flip this to an opt-in model: **`#[Type]`-mapped enums will need at +least one `#[EnumValue]`-annotated case, and only annotated cases will participate in the +schema**, mirroring the way `#[Field]` opts individual methods and properties into an object +type. The benefit is selective exposure — today there is no way to map a subset of a PHP enum into GraphQL, which forces schema authors to split an enum into two or rename cases. Under the opt-in model, an internal enum case can simply omit `#[EnumValue]` to stay out of the public -schema. - -To surface the upcoming change, GraphQLite already emits a PHP `E_USER_DEPRECATED` notice at -schema build time when an enum annotated with `#[Type]` exposes any case without an -`#[EnumValue]` attribute. The notice names the specific cases that would be dropped after the -flip so the migration path is mechanical: add `#[EnumValue]` to every case you want to keep. -No runtime behaviour changes today — the notice only signals what the future default will -require. +schema. **Partial annotation is intentional and deliberately silent**: leaving some cases +unannotated is the mechanism for hiding them from the public schema once the default flips, +not a migration oversight. + +To surface the upcoming change, GraphQLite emits a PHP `E_USER_DEPRECATED` notice at schema +build time only when a `#[Type]`-mapped enum declares **zero** `#[EnumValue]` attributes +across its cases — the signal that the developer has not yet engaged with the opt-in model at +all. Annotating at least one case acknowledges the new model and silences the notice; after +that, any combination of annotated and unannotated cases is correct and intentional. + +Adopters who aren't ready to decide which cases to expose can silence the advisory with a +bare `#[EnumValue]` on a single case — no description, no deprecation, just the acknowledgement: + +```php +#[Type] +enum Size +{ + #[EnumValue] // acknowledges the opt-in migration; runtime behaviour unchanged + case S; + case M; + case L; +} +``` **Runtime implication after the flip.** Cases without `#[EnumValue]` will not exist in the GraphQL schema, so a resolver that returns such a case will trigger webonyx/graphql-php's From bb2a3962a70377d761a596a5c9581acdd329f039 Mon Sep 17 00:00:00 2001 From: Jacob Thomason Date: Sat, 18 Apr 2026 17:48:52 -0400 Subject: [PATCH 6/8] =?UTF-8?q?docs:=20trim=20the=20enum=20migration=20sub?= =?UTF-8?q?section=20=E2=80=94=20short-lived=20transitional=20content?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- website/docs/descriptions.md | 48 ++++++------------------------------ 1 file changed, 8 insertions(+), 40 deletions(-) diff --git a/website/docs/descriptions.md b/website/docs/descriptions.md index b1afef1270..9f00fe771d 100644 --- a/website/docs/descriptions.md +++ b/website/docs/descriptions.md @@ -129,48 +129,16 @@ is an enum value. Omitting it falls back to the `@deprecated` tag on the case docblock. An explicit empty string `''` deliberately clears any inherited `@deprecated` tag. -### Future migration: `#[EnumValue]` will become required per `#[Type]`-mapped enum - -Today every case of a `#[Type]`-mapped enum is automatically exposed in the GraphQL schema. A -future major release will flip this to an opt-in model: **`#[Type]`-mapped enums will need at -least one `#[EnumValue]`-annotated case, and only annotated cases will participate in the -schema**, mirroring the way `#[Field]` opts individual methods and properties into an object -type. - -The benefit is selective exposure — today there is no way to map a subset of a PHP enum into -GraphQL, which forces schema authors to split an enum into two or rename cases. Under the -opt-in model, an internal enum case can simply omit `#[EnumValue]` to stay out of the public -schema. **Partial annotation is intentional and deliberately silent**: leaving some cases -unannotated is the mechanism for hiding them from the public schema once the default flips, -not a migration oversight. - -To surface the upcoming change, GraphQLite emits a PHP `E_USER_DEPRECATED` notice at schema -build time only when a `#[Type]`-mapped enum declares **zero** `#[EnumValue]` attributes -across its cases — the signal that the developer has not yet engaged with the opt-in model at -all. Annotating at least one case acknowledges the new model and silences the notice; after -that, any combination of annotated and unannotated cases is correct and intentional. - -Adopters who aren't ready to decide which cases to expose can silence the advisory with a -bare `#[EnumValue]` on a single case — no description, no deprecation, just the acknowledgement: +### Future migration -```php -#[Type] -enum Size -{ - #[EnumValue] // acknowledges the opt-in migration; runtime behaviour unchanged - case S; - case M; - case L; -} -``` +A future major release will require at least one `#[EnumValue]`-annotated case per +`#[Type]`-mapped enum; only annotated cases will be exposed in the schema (mirroring +`#[Field]`'s opt-in model). Today every case is still auto-exposed, so nothing breaks. -**Runtime implication after the flip.** Cases without `#[EnumValue]` will not exist in the -GraphQL schema, so a resolver that returns such a case will trigger webonyx/graphql-php's -standard enum serialization error (the value is not listed in the enum type's `values` -config). This is the same spec-compliant behaviour that applies to any unknown enum value and -is the mechanism that makes selective exposure safe: internal cases cannot accidentally leak -via a resolver. Developers who want a case to remain returnable must keep `#[EnumValue]` on -it; omitting the attribute is a deliberate "do not expose this value" signal. +GraphQLite emits a deprecation notice only when a `#[Type]`-mapped enum has **zero** +`#[EnumValue]` attributes — annotating any single case silences it and acknowledges the +model. Partial annotation is intentional: an unannotated case is how you hide a value from +the public schema once the default flips. ## Description uniqueness on `#[ExtendType]` From a655c49bc397266a8f9a2d29f2881e5d02683de5 Mon Sep 17 00:00:00 2001 From: Jacob Thomason Date: Sat, 18 Apr 2026 17:58:43 -0400 Subject: [PATCH 7/8] refactor: sawAnyEnumValueAttribute -> hasEnumValueAttribute Matches PHP's isser/haser naming convention and reads as a present-state question instead of imperative history. Also aligns the private helper name with the boolean semantics (plural -> singular since the check is 'has at least one attribute anywhere on the enum'). --- src/Mappers/Root/EnumTypeMapper.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Mappers/Root/EnumTypeMapper.php b/src/Mappers/Root/EnumTypeMapper.php index 13ca56c285..db21050770 100644 --- a/src/Mappers/Root/EnumTypeMapper.php +++ b/src/Mappers/Root/EnumTypeMapper.php @@ -147,14 +147,14 @@ private function mapByClassName(string $enumClass): EnumType|null $enumCaseDescriptions = []; /** @var array $enumCaseDeprecationReasons */ $enumCaseDeprecationReasons = []; - $sawAnyEnumValueAttribute = false; + $hasEnumValueAttribute = false; foreach ($reflectionEnum->getCases() as $reflectionEnumCase) { $docBlock = $this->docBlockFactory->create($reflectionEnumCase); $enumValueAttribute = $this->annotationReader->getEnumValueAnnotation($reflectionEnumCase); if ($enumValueAttribute !== null) { - $sawAnyEnumValueAttribute = true; + $hasEnumValueAttribute = true; } $enumCaseDescriptions[$reflectionEnumCase->getName()] = $this->descriptionResolver->resolve( @@ -181,8 +181,8 @@ private function mapByClassName(string $enumClass): EnumType|null } } - if (! $sawAnyEnumValueAttribute) { - $this->warnEnumHasNoEnumValueAttributes($enumClass); + if (! $hasEnumValueAttribute) { + $this->warnEnumHasNoEnumValueAttribute($enumClass); } $type = new EnumType($enumClass, $typeName, $enumDescription, $enumCaseDescriptions, $enumCaseDeprecationReasons, $useValues); @@ -206,7 +206,7 @@ private function mapByClassName(string $enumClass): EnumType|null * * @param class-string $enumClass */ - private function warnEnumHasNoEnumValueAttributes(string $enumClass): void + private function warnEnumHasNoEnumValueAttribute(string $enumClass): void { trigger_error( sprintf( From 159f237375ab7d1e6509f70d8f3a56cf0ac3fe21 Mon Sep 17 00:00:00 2001 From: Jacob Thomason Date: Sat, 18 Apr 2026 18:06:50 -0400 Subject: [PATCH 8/8] docs: drop the 'bare single #[EnumValue] to silence' shortcut recommendation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Recommending users annotate a single case to silence the advisory is actively misleading: once the default flips, every other case would disappear from the schema — the exact opposite of what a user reaching for the shortcut wanted. The honest recommendation is 'annotate every case you want to keep exposed', which aligns their intent with the post-flip behaviour. - Warning message rewritten around 'add to every case you want exposed, omit only from cases you want hidden' instead of 'at least one case silences this notice'. - descriptions.md migration subsection mirrors the same framing. - Color/Size/Position test fixtures now annotate every case, not just the first one, so they demonstrate the correct migration pattern rather than the misleading shortcut. --- src/Mappers/Root/EnumTypeMapper.php | 14 ++++++-------- tests/Fixtures/Integration/Models/Color.php | 4 +--- tests/Fixtures/Integration/Models/Position.php | 1 + tests/Fixtures/Integration/Models/Size.php | 2 ++ website/docs/descriptions.md | 16 ++++++++-------- 5 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/Mappers/Root/EnumTypeMapper.php b/src/Mappers/Root/EnumTypeMapper.php index db21050770..d09f2af2a0 100644 --- a/src/Mappers/Root/EnumTypeMapper.php +++ b/src/Mappers/Root/EnumTypeMapper.php @@ -197,12 +197,10 @@ private function mapByClassName(string $enumClass): EnumType|null * * Today every case is automatically exposed in the schema regardless of `#[EnumValue]` — * this call site keeps that behaviour intact. The notice announces the planned migration: - * a future major release will require at least one `#[EnumValue]`-annotated case per - * `#[Type]`-mapped enum, and only annotated cases will participate in the schema - * (mirroring `#[Field]`'s opt-in model on classes). Partial annotation is deliberately - * allowed and intentionally silent — leaving some cases unannotated is the mechanism that - * hides internal cases from the public schema once the default flips, so it must not - * itself produce a warning. + * a future major release will require `#[EnumValue]` on each case that should participate + * in the schema, and unannotated cases will be hidden (mirroring `#[Field]`'s opt-in + * model on classes). Partial annotation is deliberately allowed and intentionally silent + * so that leaving some cases unannotated can be used to hide them once the default flips. * * @param class-string $enumClass */ @@ -211,8 +209,8 @@ private function warnEnumHasNoEnumValueAttribute(string $enumClass): void trigger_error( sprintf( 'Enum "%s" is mapped to a GraphQL enum type but declares no #[EnumValue] attributes on any case. ' - . 'Today every case is automatically exposed; a future major release will require at least one #[EnumValue]-annotated case per #[Type]-mapped enum, and only annotated cases will participate in the schema (mirroring #[Field]\'s opt-in model on classes). ' - . 'Add #[EnumValue] to the case(s) you want to expose — annotating at least one case acknowledges the opt-in model and silences this notice. Cases left unannotated will be hidden from the schema after the future default flip, which is the intended way to keep internal values out of the public API.', + . 'Today every case is automatically exposed; a future major release will require #[EnumValue] on each case that should participate in the schema, and unannotated cases will be hidden (mirroring #[Field]\'s opt-in model on classes). ' + . 'Add #[EnumValue] to every case you want to keep exposed. Omit it only from cases you want hidden from the public schema after the future default flip.', $enumClass, ), E_USER_DEPRECATED, diff --git a/tests/Fixtures/Integration/Models/Color.php b/tests/Fixtures/Integration/Models/Color.php index 50eb6a7b4f..0d19b2f5e4 100644 --- a/tests/Fixtures/Integration/Models/Color.php +++ b/tests/Fixtures/Integration/Models/Color.php @@ -13,10 +13,8 @@ )] enum Color: string { - // A bare #[EnumValue] acknowledges the opt-in migration without altering runtime behaviour. - // Remove the attribute or annotate specific cases once the enum actually needs per-case - // description/deprecation metadata. #[EnumValue] case Green = 'green'; + #[EnumValue] case Red = 'red'; } diff --git a/tests/Fixtures/Integration/Models/Position.php b/tests/Fixtures/Integration/Models/Position.php index 1488216df2..c75e4f6147 100644 --- a/tests/Fixtures/Integration/Models/Position.php +++ b/tests/Fixtures/Integration/Models/Position.php @@ -12,5 +12,6 @@ enum Position: int { #[EnumValue] case Off = 0; + #[EnumValue] case On = 1; } diff --git a/tests/Fixtures/Integration/Models/Size.php b/tests/Fixtures/Integration/Models/Size.php index 24ae364bba..716ee24228 100644 --- a/tests/Fixtures/Integration/Models/Size.php +++ b/tests/Fixtures/Integration/Models/Size.php @@ -12,6 +12,8 @@ enum Size { #[EnumValue] case S; + #[EnumValue] case M; + #[EnumValue] case L; } diff --git a/website/docs/descriptions.md b/website/docs/descriptions.md index 9f00fe771d..4c427a8640 100644 --- a/website/docs/descriptions.md +++ b/website/docs/descriptions.md @@ -131,14 +131,14 @@ is an enum value. ### Future migration -A future major release will require at least one `#[EnumValue]`-annotated case per -`#[Type]`-mapped enum; only annotated cases will be exposed in the schema (mirroring -`#[Field]`'s opt-in model). Today every case is still auto-exposed, so nothing breaks. - -GraphQLite emits a deprecation notice only when a `#[Type]`-mapped enum has **zero** -`#[EnumValue]` attributes — annotating any single case silences it and acknowledges the -model. Partial annotation is intentional: an unannotated case is how you hide a value from -the public schema once the default flips. +A future major release will require `#[EnumValue]` on each case that should participate in +the schema; unannotated cases will be hidden (mirroring `#[Field]`'s opt-in model). Today +every case is still auto-exposed, so nothing breaks. Add `#[EnumValue]` to every case you +want to keep exposed — omitting it from a case is the mechanism for hiding internal values +once the default flips. + +GraphQLite emits a deprecation notice when a `#[Type]`-mapped enum has **zero** +`#[EnumValue]` attributes at all (partial annotation is intentional and stays silent). ## Description uniqueness on `#[ExtendType]`