From 5dad6b003419d9ee2190c42d284d27e0f5733941 Mon Sep 17 00:00:00 2001 From: Jacob Thomason Date: Fri, 9 Oct 2026 05:20:07 -0400 Subject: [PATCH 1/2] fix: Undefined input handling (refuse explicit null, default to Undefined) and list-formatted description metadata - An explicit null for an argument or input field whose receiving PHP type contains Undefined but not null is refused with NullArgumentException, naming the field, instead of failing with a TypeError - Undefined takes precedence over the implicit null default, so an omitted T|null|Undefined field without a PHP default resolves to Undefined::VALUE and prints no `= null`; omitted fields without a default are hydrated with Undefined unless the constructor assigned them - Undefined-typed arguments and input fields say in their description whether null is accepted, controlled by SchemaFactory::setUndefinedDescriptionsEnabled() - Description metadata (Cost, the Undefined sentence, middleware items) renders as a Markdown list via withAddedDescriptionItem(); withAddedDescriptionLines() is deprecated - Argument @param descriptions respect setDocblockDescriptionsEnabled(false) (#793) --- src/FieldsBuilder.php | 85 +++- src/InputField.php | 44 +- src/InputFieldDescriptor.php | 30 ++ src/Mappers/Parameters/TypeHandler.php | 41 +- src/Middlewares/CostFieldMiddleware.php | 4 +- src/Parameters/InputTypeParameter.php | 29 +- src/Parameters/InputTypeProperty.php | 4 + src/Parameters/NullArgumentException.php | 21 + src/QueryField.php | 3 +- src/QueryFieldDescriptor.php | 32 ++ src/SchemaFactory.php | 19 +- src/Types/InputType.php | 8 + src/Utils/DescriptionResolver.php | 62 ++- tests/Fixtures/DescriptionItems/Audience.php | 17 + .../AudienceFieldMiddleware.php | 22 + .../AudienceInputFieldMiddleware.php | 22 + .../DescriptionItems/BookController.php | 52 +++ tests/Fixtures/DescriptionItems/BookInput.php | 26 ++ .../Controllers/OmittableController.php | 131 ++++++ .../Integration/Models/OmittableInput.php | 108 +++++ .../Models/OmittableMagicSetterInput.php | 64 +++ .../Models/OmittablePropertiesInput.php | 29 ++ .../Integration/Models/OmittableValue.php | 27 ++ .../Models/OmittableWithoutDefaultInput.php | 72 +++ .../UndefinedDescriptionController.php | 37 ++ .../UndefinedDescriptionInput.php | 81 ++++ tests/Integration/DescriptionTest.php | 271 ++++++++++- tests/Integration/EndToEndTest.php | 442 ++++++++++++++++++ tests/Integration/QueryComplexityTest.php | 6 +- tests/Mappers/Parameters/TypeMapperTest.php | 132 ++++++ tests/Middlewares/CostFieldMiddlewareTest.php | 8 +- tests/QueryFieldDescriptorTest.php | 27 ++ tests/Utils/DescriptionResolverTest.php | 28 ++ website/docs/CHANGELOG.md | 20 + website/docs/descriptions.md | 23 + website/docs/field-middlewares.md | 7 + website/docs/input-types.mdx | 38 +- website/docs/operation-complexity.md | 13 + website/docs/other-frameworks.mdx | 4 + 39 files changed, 2057 insertions(+), 32 deletions(-) create mode 100644 src/Parameters/NullArgumentException.php create mode 100644 tests/Fixtures/DescriptionItems/Audience.php create mode 100644 tests/Fixtures/DescriptionItems/AudienceFieldMiddleware.php create mode 100644 tests/Fixtures/DescriptionItems/AudienceInputFieldMiddleware.php create mode 100644 tests/Fixtures/DescriptionItems/BookController.php create mode 100644 tests/Fixtures/DescriptionItems/BookInput.php create mode 100644 tests/Fixtures/Integration/Controllers/OmittableController.php create mode 100644 tests/Fixtures/Integration/Models/OmittableInput.php create mode 100644 tests/Fixtures/Integration/Models/OmittableMagicSetterInput.php create mode 100644 tests/Fixtures/Integration/Models/OmittablePropertiesInput.php create mode 100644 tests/Fixtures/Integration/Models/OmittableValue.php create mode 100644 tests/Fixtures/Integration/Models/OmittableWithoutDefaultInput.php create mode 100644 tests/Fixtures/UndefinedDescription/UndefinedDescriptionController.php create mode 100644 tests/Fixtures/UndefinedDescription/UndefinedDescriptionInput.php diff --git a/src/FieldsBuilder.php b/src/FieldsBuilder.php index 09898cc96b..da732f3ed8 100644 --- a/src/FieldsBuilder.php +++ b/src/FieldsBuilder.php @@ -43,6 +43,7 @@ use TheCodingMachine\GraphQLite\Middlewares\SourceInputPropertyResolver; use TheCodingMachine\GraphQLite\Middlewares\SourceMethodResolver; use TheCodingMachine\GraphQLite\Middlewares\SourcePropertyResolver; +use TheCodingMachine\GraphQLite\Parameters\InputTypeParameter; use TheCodingMachine\GraphQLite\Parameters\InputTypeParameterInterface; use TheCodingMachine\GraphQLite\Parameters\ParameterInterface; use TheCodingMachine\GraphQLite\Parameters\PrefetchDataParameter; @@ -57,6 +58,7 @@ use function array_diff_key; use function array_fill_keys; use function array_intersect_key; +use function array_key_exists; use function array_keys; use function array_merge; use function array_shift; @@ -1069,6 +1071,11 @@ private function getInputFieldsByMethodAnnotations( assert($type instanceof InputType); + $parameter = $args[$name]; + $descriptionItems = $parameter instanceof InputTypeParameter && $parameter->acceptsUndefined() && ! $type instanceof NonNull + ? $this->descriptionResolver->describeUndefined($parameter->refusesNull()) + : []; + $resolver = new SourceMethodResolver($refMethod); $inputFieldDescriptor = new InputFieldDescriptor( @@ -1079,10 +1086,11 @@ private function getInputFieldsByMethodAnnotations( parameters: $args, injectSource: $injectSource, description: $description !== null ? trim($description) : null, + descriptionItems: $descriptionItems, middlewareAnnotations: $this->annotationReader->getMiddlewareAnnotations($refMethod), isUpdate: $isUpdate, hasDefaultValue: $isUpdate, - defaultValue: $args[$name]->getDefaultValue(), + defaultValue: $parameter->getDefaultValue(), ); $field = $this->inputFieldMiddleware->process($inputFieldDescriptor, new class implements InputFieldHandlerInterface { @@ -1142,7 +1150,19 @@ private function getInputFieldsByPropertyAnnotations( $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, isset($defaultProperties[$refProperty->getName()])); + $forConstructorHydration = in_array($name, $constructerParameters); + $constructorParameter = $forConstructorHydration ? $this->findConstructorParameter($refClass, $name) : null; + $hasDefaultValue = $this->hasDefaultValue($refProperty, $defaultProperties); + $inputProperty = $this->typeMapper->mapInputProperty( + $refProperty, + $docBlock, + $name, + $inputType, + $defaultProperties[$refProperty->getName()] ?? null, + $isUpdate ? true : null, + $hasDefaultValue, + $this->hydrationAcceptsNull($refClass, $refProperty, $constructorParameter), + ); $description = $this->descriptionResolver->resolve( $annotation->getDescription(), @@ -1154,7 +1174,11 @@ private function getInputFieldsByPropertyAnnotations( $type = $type->getWrappedType(); } assert($type instanceof InputType); - $forConstructorHydration = in_array($name, $constructerParameters); + + $descriptionItems = $inputProperty->acceptsUndefined() && ! $type instanceof NonNull + ? $this->descriptionResolver->describeUndefined($inputProperty->refusesNull()) + : []; + $resolver = $forConstructorHydration ? new SourceConstructorParameterResolver( $refProperty->getDeclaringClass()->getName(), @@ -1172,10 +1196,14 @@ private function getInputFieldsByPropertyAnnotations( injectSource: false, forConstructorHydration: $forConstructorHydration, description: $description !== null ? trim($description) : null, + descriptionItems: $descriptionItems, middlewareAnnotations: $this->annotationReader->getMiddlewareAnnotations($refProperty), isUpdate: $isUpdate, hasDefaultValue: $inputProperty->hasDefaultValue(), defaultValue: $inputProperty->getDefaultValue(), + // Otherwise an omitted field leaves the property uninitialized, or the constructor without its argument + undefinedWhenOmitted: $inputProperty->isDefaultValueUndefined() + && ! ($constructorParameter?->isDefaultValueAvailable() ?? $hasDefaultValue), ); $field = $this->inputFieldMiddleware->process($inputFieldDescriptor, new class implements InputFieldHandlerInterface { @@ -1195,6 +1223,57 @@ public function handle(InputFieldDescriptor $inputFieldDescriptor): InputField return $fields; } + /** + * Whether the constructor parameter or setter that receives an input property's value accepts null + * + * Mirrors {@see PropertyAccessor::setValue()}. Null when neither receives it, so the property type decides. + */ + private function hydrationAcceptsNull( + ReflectionClass $refClass, + ReflectionProperty $refProperty, + ReflectionParameter|null $constructorParameter, + ): bool|null + { + if ($constructorParameter !== null) { + return $constructorParameter->allowsNull(); + } + + $setter = PropertyAccessor::findSetter($refClass->getName(), $refProperty->getName()); + if ($setter === null) { + return null; + } + + $parameters = $refClass->getMethod($setter)->getParameters(); + + return ! isset($parameters[0]) || $parameters[0]->allowsNull(); + } + + private function findConstructorParameter(ReflectionClass $refClass, string $name): ReflectionParameter|null + { + foreach ($refClass->getConstructor()?->getParameters() ?? [] as $parameter) { + if ($parameter->getName() === $name) { + return $parameter; + } + } + + return null; + } + + /** + * Whether an input property has a PHP default + * + * An untyped property's `null` default can't be told apart from no default at all, so it counts as none. + * + * @param array $defaultProperties + */ + private function hasDefaultValue(ReflectionProperty $refProperty, array $defaultProperties): bool + { + $name = $refProperty->getName(); + + return isset($defaultProperties[$name]) + || ($refProperty->hasType() && array_key_exists($name, $defaultProperties)); + } + /** @return string[] */ private function getClassConstructParameterNames(ReflectionClass $refClass): array { diff --git a/src/InputField.php b/src/InputField.php index 886fb51da9..d80423bf59 100644 --- a/src/InputField.php +++ b/src/InputField.php @@ -12,11 +12,14 @@ use GraphQL\Type\Definition\NonNull; use GraphQL\Type\Definition\ResolveInfo; use GraphQL\Type\Definition\Type; +use ReflectionProperty; use TheCodingMachine\GraphQLite\Exceptions\GraphQLAggregateException; use TheCodingMachine\GraphQLite\Middlewares\ResolverInterface; +use TheCodingMachine\GraphQLite\Middlewares\SourceInputPropertyResolver; use TheCodingMachine\GraphQLite\Parameters\MissingArgumentException; use TheCodingMachine\GraphQLite\Parameters\ParameterInterface; use TheCodingMachine\GraphQLite\Parameters\SourceParameter; +use TheCodingMachine\GraphQLite\Utils\DescriptionResolver; use Throwable; /** @@ -40,7 +43,7 @@ public function __construct( string $name, InputType $type, array $arguments, - ResolverInterface $originalResolver, + private readonly ResolverInterface $originalResolver, callable $resolver, private bool $forConstructorHydration, string|null $description, @@ -48,6 +51,7 @@ public function __construct( bool $hasDefaultValue, mixed $defaultValue, array|null $additionalConfig = null, + private readonly bool $undefinedWhenOmitted = false, ) { $config = [ 'name' => $name, @@ -122,6 +126,41 @@ public function forConstructorHydration(): bool return $this->forConstructorHydration; } + /** Whether an omitted field hydrates its constructor parameter or property with Undefined */ + public function isUndefinedWhenOmitted(): bool + { + return $this->undefinedWhenOmitted; + } + + /** + * Hydrates an omitted field with Undefined, returning the field's value + * + * Skips the middleware: nothing was sent, so there is nothing to authorize. A property the constructor + * already assigned keeps that value, as it would without Undefined (and a readonly one can't be reassigned). + */ + public function resolveOmitted(object|null $source): mixed + { + $resolver = $this->originalResolver; + if ($source !== null && $resolver instanceof SourceInputPropertyResolver) { + $property = $resolver->propertyReflection(); + if (self::isAssigned($property, $source)) { + return $property->getValue($source); + } + } + + return $resolver($source, Undefined::VALUE); + } + + /** Whether the property holds a value, given an untyped property starts out null rather than uninitialized */ + private static function isAssigned(ReflectionProperty $property, object $source): bool + { + if ($property->hasType()) { + return $property->isInitialized($source); + } + + return $property->getValue($source) !== null; + } + private static function fromDescriptor(InputFieldDescriptor $fieldDescriptor): self { return new self( @@ -131,10 +170,11 @@ private static function fromDescriptor(InputFieldDescriptor $fieldDescriptor): s $fieldDescriptor->getOriginalResolver(), $fieldDescriptor->getResolver(), $fieldDescriptor->isForConstructorHydration(), - $fieldDescriptor->getDescription(), + DescriptionResolver::appendItems($fieldDescriptor->getDescription(), $fieldDescriptor->getDescriptionItems()), $fieldDescriptor->isUpdate(), $fieldDescriptor->hasDefaultValue(), $fieldDescriptor->getDefaultValue(), + undefinedWhenOmitted: $fieldDescriptor->isUndefinedWhenOmitted(), ); } diff --git a/src/InputFieldDescriptor.php b/src/InputFieldDescriptor.php index 523a75fbaa..e9861fdf63 100644 --- a/src/InputFieldDescriptor.php +++ b/src/InputFieldDescriptor.php @@ -27,6 +27,7 @@ class InputFieldDescriptor * @param array $parameters * @param callable $resolver * @param bool $injectSource Whether we should inject the source as the first parameter or not. + * @param list $descriptionItems Metadata rendered as a list after the description */ public function __construct( private readonly string $name, @@ -41,10 +42,23 @@ public function __construct( private readonly bool $isUpdate = false, private readonly bool $hasDefaultValue = false, private readonly mixed $defaultValue = null, + private readonly bool $undefinedWhenOmitted = false, + private readonly array $descriptionItems = [], ) { } + /** Whether an omitted field hydrates its constructor parameter or property with Undefined */ + public function isUndefinedWhenOmitted(): bool + { + return $this->undefinedWhenOmitted; + } + + public function withUndefinedWhenOmitted(bool $undefinedWhenOmitted): self + { + return $this->with(undefinedWhenOmitted: $undefinedWhenOmitted); + } + public function isUpdate(): bool { return $this->isUpdate; @@ -137,6 +151,22 @@ public function withDescription(string|null $description): self return $this->with(description: $description); } + /** @return list */ + public function getDescriptionItems(): array + { + return $this->descriptionItems; + } + + /** + * Appends a metadata item, rendered as a Markdown list after the description + * + * Items keep the order they were added in and survive {@see withDescription()}. + */ + public function withAddedDescriptionItem(string $item): self + { + return $this->with(descriptionItems: [...$this->descriptionItems, $item]); + } + public function getMiddlewareAnnotations(): MiddlewareAnnotations { return $this->middlewareAnnotations; diff --git a/src/Mappers/Parameters/TypeHandler.php b/src/Mappers/Parameters/TypeHandler.php index fe881bc876..5664669657 100644 --- a/src/Mappers/Parameters/TypeHandler.php +++ b/src/Mappers/Parameters/TypeHandler.php @@ -5,6 +5,7 @@ namespace TheCodingMachine\GraphQLite\Mappers\Parameters; use GraphQL\Type\Definition\InputType; +use GraphQL\Type\Definition\NonNull; use GraphQL\Type\Definition\OutputType; use GraphQL\Type\Definition\Type as GraphQLType; use InvalidArgumentException; @@ -237,7 +238,9 @@ public function mapParameter( $defaultValue = $parameter->getDefaultValue(); } - if (! $hasDefaultValue && UndefinedTypeMapper::containsUndefined($phpdocType)) { + $containsUndefined = self::containsUndefined($phpdocType, $paramTagType); + + if (! $hasDefaultValue && $containsUndefined) { $hasDefaultValue = true; $defaultValue = Undefined::VALUE; } @@ -247,19 +250,36 @@ public function mapParameter( $defaultValue = null; } - $description = $this->getParameterDescriptionFromDocBlock($docBlock, $parameter); + $refusesNull = $containsUndefined && ! $parameter->allowsNull(); + $descriptionItems = $containsUndefined && ! $type instanceof NonNull + ? $this->descriptionResolver->describeUndefined($refusesNull) + : []; return new InputTypeParameter( name: $parameter->getName(), type: $type, description: $description, + descriptionItems: $descriptionItems, hasDefaultValue: $hasDefaultValue, defaultValue: $defaultValue, defaultValueImplicit: $defaultValue === Undefined::VALUE, argumentResolver: $this->argumentResolver, + refusesNull: $refusesNull, + acceptsUndefined: $containsUndefined, ); } + /** Whether the native type includes Undefined, or the docblock type does when the native type is absent or mixed */ + private static function containsUndefined(Type $phpdocType, Type|null $docBlockType): bool + { + $nativeType = $phpdocType instanceof Nullable ? $phpdocType->getActualType() : $phpdocType; + if ($nativeType instanceof Mixed_ && $docBlockType !== null) { + return UndefinedTypeMapper::containsUndefined($docBlockType); + } + + return UndefinedTypeMapper::containsUndefined($phpdocType); + } + private function getParameterDescriptionFromDocBlock(DocBlock $docBlock, ReflectionParameter $parameter): string|null { /** @var DocBlock\Tags\Param[] $paramTags */ @@ -316,6 +336,9 @@ public function mapPropertyType( /** * Maps class property into input property. * + * $acceptsNull says whether the constructor parameter or setter that receives the value accepts null. + * It defaults to the property's own type. + * * @throws CannotMapTypeException */ public function mapInputProperty( @@ -326,6 +349,7 @@ public function mapInputProperty( mixed $defaultValue = null, bool|null $isNullable = null, bool $hasDefaultValue = false, + bool|null $acceptsNull = null, ): InputTypeProperty { $docBlockDescription = $docBlock->getSummary() . PHP_EOL . $docBlock->getDescription()->render(); @@ -362,14 +386,17 @@ public function mapInputProperty( assert($inputType instanceof InputType); } - if (! $hasDefaultValue && $isNullable) { + // Undefined before null: GraphQL would fill a printed `null` default in for an omitted `T|null|Undefined` field. + $containsUndefined = self::containsUndefined($phpdocType, $this->getDocBlockPropertyType($docBlock, $refProperty)); + + if (! $hasDefaultValue && $containsUndefined) { $hasDefaultValue = true; - $defaultValue = null; + $defaultValue = Undefined::VALUE; } - if (! $hasDefaultValue && UndefinedTypeMapper::containsUndefined($phpdocType)) { + if (! $hasDefaultValue && $isNullable) { $hasDefaultValue = true; - $defaultValue = Undefined::VALUE; + $defaultValue = null; } $fieldName = $argumentName ?? $refProperty->getName(); @@ -385,6 +412,8 @@ public function mapInputProperty( defaultValue: $defaultValue, defaultValueImplicit: $defaultValue === Undefined::VALUE, argumentResolver: $this->argumentResolver, + refusesNull: $containsUndefined && ! ($acceptsNull ?? $propertyType?->allowsNull() ?? true), + acceptsUndefined: $containsUndefined, ); } diff --git a/src/Middlewares/CostFieldMiddleware.php b/src/Middlewares/CostFieldMiddleware.php index 01c32ec5ca..327b3b0b6e 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->withAddedDescriptionLines($this->buildCostDescription($costAttribute)), + $queryFieldDescriptor->withAddedDescriptionItem($this->buildCostDescription($costAttribute)), ); if (! $field) { @@ -63,7 +63,7 @@ public function process(QueryFieldDescriptor $queryFieldDescriptor, FieldHandler private function buildCostDescription(Cost $costAttribute): string { - return "\nCost: " . + return 'Cost: ' . implode(', ', [ 'complexity = ' . $costAttribute->complexity, 'multipliers = [' . implode(', ', $costAttribute->multipliers) . ']', diff --git a/src/Parameters/InputTypeParameter.php b/src/Parameters/InputTypeParameter.php index e828f9ac3d..3ccbe44c7c 100644 --- a/src/Parameters/InputTypeParameter.php +++ b/src/Parameters/InputTypeParameter.php @@ -9,11 +9,13 @@ use GraphQL\Type\Definition\Type; use TheCodingMachine\GraphQLite\Types\ArgumentResolver; use TheCodingMachine\GraphQLite\Types\ResolvableMutableInputObjectType; +use TheCodingMachine\GraphQLite\Utils\DescriptionResolver; use function array_key_exists; class InputTypeParameter implements InputTypeParameterInterface { + /** @param list $descriptionItems Metadata rendered as a list after the description */ public function __construct( private readonly string $name, private readonly InputType&Type $type, @@ -22,6 +24,9 @@ public function __construct( private readonly mixed $defaultValue, private readonly bool $defaultValueImplicit, private readonly ArgumentResolver $argumentResolver, + private readonly bool $refusesNull = false, + private readonly bool $acceptsUndefined = false, + private readonly array $descriptionItems = [], ) { } @@ -30,6 +35,10 @@ public function __construct( public function resolve(object|null $source, array $args, mixed $context, ResolveInfo $info): mixed { if (array_key_exists($this->name, $args)) { + if ($args[$this->name] === null && $this->refusesNull) { + throw NullArgumentException::create($this->name); + } + return $this->argumentResolver->resolve($source, $args[$this->name], $context, $info, $this->type); } @@ -72,8 +81,26 @@ public function getDefaultValue(): mixed return ! $this->defaultValueImplicit ? $this->defaultValue : null; } + /** Whether an omitted value resolves to Undefined, a default GraphQL can't print */ + public function isDefaultValueUndefined(): bool + { + return $this->hasDefaultValue && $this->defaultValueImplicit; + } + + /** Whether the receiving PHP type contains Undefined */ + public function acceptsUndefined(): bool + { + return $this->acceptsUndefined; + } + + /** Whether an explicit null is refused, because the receiving PHP type contains Undefined but not null */ + public function refusesNull(): bool + { + return $this->refusesNull; + } + public function getDescription(): string { - return $this->description ?? ''; + return DescriptionResolver::appendItems($this->description, $this->descriptionItems) ?? ''; } } diff --git a/src/Parameters/InputTypeProperty.php b/src/Parameters/InputTypeProperty.php index 76226d1b49..80446fc410 100644 --- a/src/Parameters/InputTypeProperty.php +++ b/src/Parameters/InputTypeProperty.php @@ -19,6 +19,8 @@ public function __construct( mixed $defaultValue, bool $defaultValueImplicit, ArgumentResolver $argumentResolver, + bool $refusesNull = false, + bool $acceptsUndefined = false, ) { parent::__construct( @@ -29,6 +31,8 @@ public function __construct( $defaultValue, $defaultValueImplicit, $argumentResolver, + $refusesNull, + $acceptsUndefined, ); } diff --git a/src/Parameters/NullArgumentException.php b/src/Parameters/NullArgumentException.php new file mode 100644 index 0000000000..2d8e9466dd --- /dev/null +++ b/src/Parameters/NullArgumentException.php @@ -0,0 +1,21 @@ +getParameters(), $fieldDescriptor->getOriginalResolver(), $fieldDescriptor->getResolver(), - $fieldDescriptor->getDescription(), + DescriptionResolver::appendItems($fieldDescriptor->getDescription(), $fieldDescriptor->getDescriptionItems()), $fieldDescriptor->getDeprecationReason(), ); } diff --git a/src/QueryFieldDescriptor.php b/src/QueryFieldDescriptor.php index 462c45d207..8b7bdb1348 100644 --- a/src/QueryFieldDescriptor.php +++ b/src/QueryFieldDescriptor.php @@ -14,6 +14,10 @@ use TheCodingMachine\GraphQLite\Parameters\ParameterInterface; use TheCodingMachine\GraphQLite\Utils\Cloneable; +use function trigger_error; + +use const E_USER_DEPRECATED; + /** * A class that describes a field to be created. * To contains getters and setters to alter the field behaviour. @@ -27,6 +31,7 @@ class QueryFieldDescriptor * @param array $parameters * @param callable $resolver * @param bool $injectSource Whether we should inject the source as the first parameter or not. + * @param list $descriptionItems Metadata rendered as a list after the description */ public function __construct( private readonly string $name, @@ -38,6 +43,7 @@ public function __construct( private readonly string|null $description = null, private readonly string|null $deprecationReason = null, private readonly MiddlewareAnnotations $middlewareAnnotations = new MiddlewareAnnotations([]), + private readonly array $descriptionItems = [], ) { } @@ -94,8 +100,34 @@ public function withDescription(string|null $description): self return $this->with(description: $description); } + /** @return list */ + public function getDescriptionItems(): array + { + return $this->descriptionItems; + } + + /** + * Appends a metadata item, rendered as a Markdown list after the description + * + * Items keep the order they were added in and survive {@see withDescription()}. + */ + public function withAddedDescriptionItem(string $item): self + { + return $this->with(descriptionItems: [...$this->descriptionItems, $item]); + } + + /** + * Appends raw text to the description, on a new line + * + * @deprecated Use {@see withAddedDescriptionItem()}, which renders metadata as a list after the description + */ public function withAddedDescriptionLines(string $description): self { + trigger_error( + 'QueryFieldDescriptor::withAddedDescriptionLines() is deprecated; use withAddedDescriptionItem() instead.', + E_USER_DEPRECATED, + ); + if (! $this->description) { return $this->withDescription($description); } diff --git a/src/SchemaFactory.php b/src/SchemaFactory.php index 0b2b721c33..739e4b4e0b 100644 --- a/src/SchemaFactory.php +++ b/src/SchemaFactory.php @@ -137,6 +137,9 @@ class SchemaFactory */ private bool $useDocblockDescriptions = true; + /** When true (default), arguments and input fields typed `T|Undefined` say in their description whether null is accepted */ + private bool $useUndefinedDescriptions = true; + /** @var array */ private array $fieldMiddlewares = []; @@ -370,6 +373,20 @@ public function setDocblockDescriptionsEnabled(bool $enabled): self return $this; } + /** + * Controls whether arguments and input fields whose PHP type contains `Undefined` list an item after their + * description: "May be omitted; null is accepted." or "May be omitted; null is not accepted." + * + * GraphQL types can't express "optional but not nullable", and applied directives aren't exposed by + * introspection, so the description is the only place every client sees it. Enabled by default. + */ + public function setUndefinedDescriptionsEnabled(bool $enabled): self + { + $this->useUndefinedDescriptions = $enabled; + + return $this; + } + /** * Registers a field middleware (used to parse custom annotations that modify the GraphQLite behaviour in Fields/Queries/Mutations. */ @@ -417,7 +434,7 @@ public function createSchema(): Schema $classBoundCache, PhpDocumentorDocBlockFactory::default(), ); - $descriptionResolver = new DescriptionResolver($this->useDocblockDescriptions); + $descriptionResolver = new DescriptionResolver($this->useDocblockDescriptions, $this->useUndefinedDescriptions); $fieldAccessorPrefixes = $this->fieldAccessorPrefixes ?? new FieldAccessorPrefixes(); $namingStrategy = $this->namingStrategy ?: new NamingStrategy($fieldAccessorPrefixes); $typeRegistry = new TypeRegistry(); diff --git a/src/Types/InputType.php b/src/Types/InputType.php index 4135a3ffb9..96e2ab99eb 100644 --- a/src/Types/InputType.php +++ b/src/Types/InputType.php @@ -74,6 +74,10 @@ public function resolve(object|null $source, array $args, mixed $context, Resolv foreach ($this->inputFields as $inputField) { $name = $inputField->name; if (! array_key_exists($name, $args)) { + if ($inputField->isUndefinedWhenOmitted()) { + $inputField->resolveOmitted($source); + } + continue; } @@ -106,6 +110,10 @@ private function makeConstructorArgs(object|null $source, array $args, mixed $co $resolve = $constructorInputField->getResolve(); if (! array_key_exists($name, $args)) { + if ($constructorInputField->isUndefinedWhenOmitted()) { + $constructorArgs[$name] = $constructorInputField->resolveOmitted($source); + } + continue; } diff --git a/src/Utils/DescriptionResolver.php b/src/Utils/DescriptionResolver.php index f586037183..1158496823 100644 --- a/src/Utils/DescriptionResolver.php +++ b/src/Utils/DescriptionResolver.php @@ -4,6 +4,13 @@ namespace TheCodingMachine\GraphQLite\Utils; +use function array_map; +use function array_unique; +use function implode; +use function rtrim; +use function str_contains; +use function trim; + /** * Resolves a GraphQL schema description from an explicit attribute value with optional * docblock fallback. @@ -21,10 +28,19 @@ * whatever strategy matches the schema element (summary only, summary + description, * summary + description + @var tag, etc.). This class only encodes the explicit-vs-docblock * precedence rule so it can be applied consistently across every extraction site. + * + * It also renders the metadata items appended to a description (such as `#[Cost]` or the sentence saying a + * `T|Undefined` value may be omitted) as a Markdown list after it. */ final class DescriptionResolver { - public function __construct(private readonly bool $useDocblockFallback) + public const UNDEFINED_ACCEPTS_NULL = 'May be omitted; null is accepted.'; + public const UNDEFINED_REFUSES_NULL = 'May be omitted; null is not accepted.'; + + public function __construct( + private readonly bool $useDocblockFallback, + private readonly bool $useUndefinedDescriptions = true, + ) { } @@ -57,4 +73,48 @@ public function resolve(string|null $explicit, string|null $docblockDerived): st return $docblockDerived; } + + /** + * Returns the description item saying a `T|Undefined` value may be omitted and whether null is accepted + * + * GraphQL types can't say "optional but not nullable". Empty when Undefined descriptions are disabled. + * + * @return list + */ + public function describeUndefined(bool $refusesNull): array + { + if (! $this->useUndefinedDescriptions) { + return []; + } + + return [$refusesNull ? self::UNDEFINED_REFUSES_NULL : self::UNDEFINED_ACCEPTS_NULL]; + } + + /** + * Renders a description followed by its metadata items as a Markdown list, after a blank line + * + * An item the description already contains is left out, so a developer can word it themselves. + * + * @param list $items + */ + public static function appendItems(string|null $description, array $items): string|null + { + $original = rtrim($description ?? ''); + $lines = []; + foreach (array_unique(array_map(static fn (string $item): string => trim($item), $items)) as $item) { + if ($item === '' || str_contains($original, $item)) { + continue; + } + + $lines[] = '- ' . $item; + } + + if ($lines === []) { + return $description; + } + + $list = implode("\n", $lines); + + return $original === '' ? $list : $original . "\n\n" . $list; + } } diff --git a/tests/Fixtures/DescriptionItems/Audience.php b/tests/Fixtures/DescriptionItems/Audience.php new file mode 100644 index 0000000000..14d4f30763 --- /dev/null +++ b/tests/Fixtures/DescriptionItems/Audience.php @@ -0,0 +1,17 @@ +getMiddlewareAnnotations()->getAnnotationsByType(Audience::class) as $audience) { + $queryFieldDescriptor = $queryFieldDescriptor->withAddedDescriptionItem('Audience: ' . $audience->name); + } + + return $fieldHandler->handle($queryFieldDescriptor); + } +} diff --git a/tests/Fixtures/DescriptionItems/AudienceInputFieldMiddleware.php b/tests/Fixtures/DescriptionItems/AudienceInputFieldMiddleware.php new file mode 100644 index 0000000000..37356b807e --- /dev/null +++ b/tests/Fixtures/DescriptionItems/AudienceInputFieldMiddleware.php @@ -0,0 +1,22 @@ +getMiddlewareAnnotations()->getAnnotationsByType(Audience::class) as $audience) { + $inputFieldDescriptor = $inputFieldDescriptor->withAddedDescriptionItem('Audience: ' . $audience->name); + } + + return $inputFieldHandler->handle($inputFieldDescriptor); + } +} diff --git a/tests/Fixtures/DescriptionItems/BookController.php b/tests/Fixtures/DescriptionItems/BookController.php new file mode 100644 index 0000000000..8fa09b615f --- /dev/null +++ b/tests/Fixtures/DescriptionItems/BookController.php @@ -0,0 +1,52 @@ +|Undefined $list + * + * @return string[] + */ + #[Query] + public function omittableArguments( + int|Undefined $argument, + int|null|Undefined $nullableArgument, + array|Undefined $list, + ): array + { + return [ + 'argument: ' . self::describe($argument), + 'nullableArgument: ' . self::describe($nullableArgument), + 'list: ' . self::describe($list), + ]; + } + + /** @return string[] */ + #[Mutation] + public function updateOmittable(OmittableInput $input): array + { + return [ + 'property: ' . self::describe($input->property), + 'nullableProperty: ' . self::describe($input->nullableProperty), + 'setter: ' . self::describe($input->getSetter()), + 'nullableSetter: ' . self::describe($input->getNullableSetter()), + 'promotedParameter: ' . self::describe($input->promotedParameter), + 'nullablePromotedParameter: ' . self::describe($input->nullablePromotedParameter), + 'constructorParameter: ' . self::describe($input->constructorParameter), + 'nullableConstructorParameter: ' . self::describe($input->nullableConstructorParameter), + 'propertyWithNullableConstructorParameter: ' . self::describe($input->propertyWithNullableConstructorParameter), + 'nullablePropertyWithConstructorParameter: ' . self::describe($input->nullablePropertyWithConstructorParameter), + 'propertyWithNullableSetter: ' . self::describe($input->propertyWithNullableSetter), + 'nullablePropertyWithSetter: ' . self::describe($input->nullablePropertyWithSetter), + 'nullablePropertyWithNullDefault: ' . self::describe($input->nullablePropertyWithNullDefault), + 'list: ' . self::describe($input->list), + 'nullableList: ' . self::describe($input->nullableList), + ]; + } + + /** @return string[] */ + #[Mutation] + public function updateOmittableWithoutDefault(OmittableWithoutDefaultInput $input): array + { + return [ + 'property: ' . self::describe($input->property), + 'nullableProperty: ' . self::describe($input->nullableProperty), + 'docBlockProperty: ' . self::describe($input->docBlockProperty), + 'nullablePromotedParameter: ' . self::describe($input->nullablePromotedParameter), + 'nullableConstructorParameter: ' . self::describe($input->nullableConstructorParameter), + 'nullablePropertyWithSetter: ' . self::describe($input->nullablePropertyWithSetter), + 'nullableSetter: ' . self::describe($input->getNullableSetter()), + 'assignedInConstructor: ' . self::describe($input->assignedInConstructor), + 'docBlockAssignedInConstructor: ' . self::describe($input->docBlockAssignedInConstructor), + 'readonlyAssignedInConstructor: ' . self::describe($input->readonlyAssignedInConstructor), + ]; + } + + /** @return string[] */ + #[Mutation] + public function updateOmittableProperties(OmittablePropertiesInput $input): array + { + return [ + 'property: ' . self::describe($input->property), + 'nullableProperty: ' . self::describe($input->nullableProperty), + 'propertyWithoutDefault: ' . self::describe($input->propertyWithoutDefault), + 'nullablePropertyWithoutDefault: ' . self::describe($input->nullablePropertyWithoutDefault), + ]; + } + + /** @return string[] */ + #[Mutation] + public function updateOmittableMagicSetter(OmittableMagicSetterInput $input): array + { + return [ + 'property: ' . self::describe($input->getProperty()), + 'nullableProperty: ' . self::describe($input->getNullableProperty()), + 'propertyWithoutDefault: ' . self::describe($input->getPropertyWithoutDefault()), + 'nullablePropertyWithoutDefault: ' . self::describe($input->getNullablePropertyWithoutDefault()), + ]; + } + + /** + * @param OmittableInput[] $inputs + * + * @return string[] + */ + #[Mutation] + public function updateOmittableList(array $inputs): array + { + return array_merge(...array_map($this->updateOmittable(...), $inputs)); + } + + /** @return string[] */ + #[Query] + public function omittableFactory(OmittableValue $value): array + { + return [ + 'value: ' . self::describe($value->value), + 'nullableValue: ' . self::describe($value->nullableValue), + ]; + } + + private static function describe(mixed $value): string + { + return $value === Undefined::VALUE ? 'undefined' : json_encode($value); + } +} diff --git a/tests/Fixtures/Integration/Models/OmittableInput.php b/tests/Fixtures/Integration/Models/OmittableInput.php new file mode 100644 index 0000000000..4d6f1e09d9 --- /dev/null +++ b/tests/Fixtures/Integration/Models/OmittableInput.php @@ -0,0 +1,108 @@ +|Undefined $list + * @param list|Undefined|null $nullableList + */ + public function __construct( + #[Field] + public readonly int|Undefined $promotedParameter = Undefined::VALUE, + #[Field] + public readonly int|null|Undefined $nullablePromotedParameter = Undefined::VALUE, + #[Field] + public readonly array|Undefined $list = Undefined::VALUE, + #[Field] + public readonly array|Undefined|null $nullableList = Undefined::VALUE, + int|Undefined $constructorParameter = Undefined::VALUE, + int|null|Undefined $nullableConstructorParameter = Undefined::VALUE, + int|null|Undefined $propertyWithNullableConstructorParameter = Undefined::VALUE, + int|Undefined $nullablePropertyWithConstructorParameter = Undefined::VALUE, + ) + { + $this->constructorParameter = $constructorParameter; + $this->nullableConstructorParameter = $nullableConstructorParameter; + $this->propertyWithNullableConstructorParameter = $propertyWithNullableConstructorParameter ?? 0; + $this->nullablePropertyWithConstructorParameter = $nullablePropertyWithConstructorParameter; + } + + public function setPropertyWithNullableSetter(int|null|Undefined $value): void + { + $this->propertyWithNullableSetter = $value ?? 0; + } + + public function setNullablePropertyWithSetter(int|Undefined $value): void + { + $this->nullablePropertyWithSetter = $value; + } + + #[Field] + public function setSetter(int|Undefined $setter): void + { + $this->setter = $setter; + } + + #[Field] + public function setNullableSetter(int|null|Undefined $nullableSetter): void + { + $this->nullableSetter = $nullableSetter; + } + + public function getSetter(): int|Undefined + { + return $this->setter; + } + + public function getNullableSetter(): int|null|Undefined + { + return $this->nullableSetter; + } +} diff --git a/tests/Fixtures/Integration/Models/OmittableMagicSetterInput.php b/tests/Fixtures/Integration/Models/OmittableMagicSetterInput.php new file mode 100644 index 0000000000..c830ae7b06 --- /dev/null +++ b/tests/Fixtures/Integration/Models/OmittableMagicSetterInput.php @@ -0,0 +1,64 @@ + $arguments */ + public function __call(string $method, array $arguments): void + { + if (! str_starts_with($method, 'set')) { + throw new BadMethodCallException('Call to undefined method ' . self::class . '::' . $method . '()'); + } + + $this->{lcfirst(substr($method, 3))} = $arguments[0]; + } + + public function getProperty(): int|Undefined + { + return $this->property; + } + + public function getNullableProperty(): int|null|Undefined + { + return $this->nullableProperty; + } + + public function getPropertyWithoutDefault(): int|Undefined + { + return $this->propertyWithoutDefault; + } + + public function getNullablePropertyWithoutDefault(): int|null|Undefined + { + return $this->nullablePropertyWithoutDefault; + } +} diff --git a/tests/Fixtures/Integration/Models/OmittablePropertiesInput.php b/tests/Fixtures/Integration/Models/OmittablePropertiesInput.php new file mode 100644 index 0000000000..52fc64126e --- /dev/null +++ b/tests/Fixtures/Integration/Models/OmittablePropertiesInput.php @@ -0,0 +1,29 @@ +nullableConstructorParameter = $nullableConstructorParameter; + $this->assignedInConstructor = 10; + $this->docBlockAssignedInConstructor = 11; + $this->readonlyAssignedInConstructor = Undefined::VALUE; + } + + public function setNullablePropertyWithSetter(int|null|Undefined $value): void + { + $this->nullablePropertyWithSetter = $value; + } + + #[Field] + public function setNullableSetter(int|null|Undefined $nullableSetter): void + { + $this->nullableSetter = $nullableSetter; + } + + public function getNullableSetter(): int|null|Undefined + { + return $this->nullableSetter; + } +} diff --git a/tests/Fixtures/UndefinedDescription/UndefinedDescriptionController.php b/tests/Fixtures/UndefinedDescription/UndefinedDescriptionController.php new file mode 100644 index 0000000000..fd2bc762d2 --- /dev/null +++ b/tests/Fixtures/UndefinedDescription/UndefinedDescriptionController.php @@ -0,0 +1,37 @@ +setAuthenticationService(new VoidAuthenticationService()); $factory->setAuthorizationService(new VoidAuthorizationService()); $factory->addNamespace((new ReflectionClass($fixtureClass))->getNamespaceName()); $factory->setDocblockDescriptionsEnabled($docblockDescriptions); + $factory->setUndefinedDescriptionsEnabled($undefinedDescriptions); + $factory->addFieldMiddleware(new AudienceFieldMiddleware()); + $factory->addInputFieldMiddleware(new AudienceInputFieldMiddleware()); return $factory->createSchema(); } @@ -254,4 +276,249 @@ public function testDuplicateDescriptionAcrossTypeAndExtendTypeThrows(): void $this->assertStringContainsString('DescriptionDuplicate\\BookExtension', $exception->getMessage()); } } + + public function testUndefinedDescriptionsAreAppendedByDefault(): void + { + $schema = $this->buildSchema(UndefinedDescriptionController::class); + + $this->assertSame('May be omitted; null is not accepted.', DescriptionResolver::UNDEFINED_REFUSES_NULL); + $this->assertSame('May be omitted; null is accepted.', DescriptionResolver::UNDEFINED_ACCEPTS_NULL); + $refuses = '- ' . DescriptionResolver::UNDEFINED_REFUSES_NULL; + $accepts = '- ' . DescriptionResolver::UNDEFINED_ACCEPTS_NULL; + + $this->assertSame([ + 'property' => $refuses, + 'nullableProperty' => $accepts, + 'describedProperty' => "Age in years.\n\n" . $refuses, + 'describedNullableProperty' => "Age in years.\n\n" . $accepts, + // Not duplicated when the developer already wrote it + 'alreadyDescribedProperty' => 'Age in years. May be omitted; null is not accepted.', + 'plainProperty' => 'Age in years.', + // A forced non-null type says it is required, so no sentence + 'nonNullProperty' => '', + 'promotedParameter' => $refuses, + 'nullablePromotedParameter' => $accepts, + 'describedPromotedParameter' => "Age in years.\n\n" . $refuses, + 'describedNullablePromotedParameter' => "Age in years.\n\n" . $accepts, + 'setter' => $refuses, + 'nullableSetter' => $accepts, + 'describedSetter' => "Age in years.\n\n" . $refuses, + 'describedNullableSetter' => "Age in years.\n\n" . $accepts, + 'nonNullSetter' => '', + ], $this->introspectInputFieldDescriptions($schema)); + + $this->assertSame([ + 'argument' => $refuses, + 'nullableArgument' => $accepts, + 'describedArgument' => "Age in years.\n\n" . $refuses, + 'describedNullableArgument' => "Age in years.\n\n" . $accepts, + 'plainArgument' => null, + 'nonNullArgument' => null, + ], $this->introspectArgumentDescriptions($schema)); + + $printedInput = SchemaPrinter::printType($schema->getType('UndefinedDescriptionInput')); + $this->assertStringContainsString( + " \"\"\"\n Age in years.\n \n - May be omitted; null is not accepted.\n \"\"\"\n describedProperty: Int\n", + $printedInput, + ); + $this->assertStringContainsString(" \"- May be omitted; null is accepted.\"\n nullableSetter: Int\n", $printedInput); + $this->assertStringContainsString(" \"\"\n nonNullProperty: Int!\n", $printedInput); + $this->assertStringContainsString(" \"\"\n nonNullSetter: Int!\n", $printedInput); + $this->assertStringContainsString( + " plainArgument: Int = null\n nonNullArgument: Int!\n", + SchemaPrinter::printType($schema->getQueryType()), + ); + } + + public function testUndefinedDescriptionsCanBeDisabled(): void + { + $schema = $this->buildSchema(UndefinedDescriptionController::class, undefinedDescriptions: false); + + $this->assertSame([ + 'property' => '', + 'nullableProperty' => '', + 'describedProperty' => 'Age in years.', + 'describedNullableProperty' => 'Age in years.', + 'alreadyDescribedProperty' => 'Age in years. May be omitted; null is not accepted.', + 'plainProperty' => 'Age in years.', + 'nonNullProperty' => '', + 'promotedParameter' => '', + 'nullablePromotedParameter' => '', + 'describedPromotedParameter' => 'Age in years.', + 'describedNullablePromotedParameter' => 'Age in years.', + 'setter' => '', + 'nullableSetter' => '', + 'describedSetter' => 'Age in years.', + 'describedNullableSetter' => 'Age in years.', + 'nonNullSetter' => '', + ], $this->introspectInputFieldDescriptions($schema)); + + $this->assertSame([ + 'argument' => null, + 'nullableArgument' => null, + 'describedArgument' => 'Age in years.', + 'describedNullableArgument' => 'Age in years.', + 'plainArgument' => null, + 'nonNullArgument' => null, + ], $this->introspectArgumentDescriptions($schema)); + } + + public function testUndefinedDescriptionsSurviveDisabledDocblockFallback(): void + { + $schema = $this->buildSchema(UndefinedDescriptionController::class, docblockDescriptions: false); + + $refuses = '- ' . DescriptionResolver::UNDEFINED_REFUSES_NULL; + $accepts = '- ' . DescriptionResolver::UNDEFINED_ACCEPTS_NULL; + $this->assertSame([ + 'property' => $refuses, + 'nullableProperty' => $accepts, + 'describedProperty' => $refuses, + 'describedNullableProperty' => $accepts, + 'alreadyDescribedProperty' => 'Age in years. May be omitted; null is not accepted.', + 'plainProperty' => null, + 'nonNullProperty' => null, + 'promotedParameter' => $refuses, + 'nullablePromotedParameter' => $accepts, + 'describedPromotedParameter' => "Age in years.\n\n" . $refuses, + 'describedNullablePromotedParameter' => "Age in years.\n\n" . $accepts, + 'setter' => $refuses, + 'nullableSetter' => $accepts, + 'describedSetter' => $refuses, + 'describedNullableSetter' => $accepts, + 'nonNullSetter' => null, + ], $this->introspectInputFieldDescriptions($schema)); + + $this->assertSame([ + 'argument' => $refuses, + 'nullableArgument' => $accepts, + // The @param description stays out of the schema with the docblock fallback off + 'describedArgument' => $refuses, + 'describedNullableArgument' => $accepts, + 'plainArgument' => null, + 'nonNullArgument' => null, + ], $this->introspectArgumentDescriptions($schema)); + } + + public function testArgumentDocblockDescriptionsFollowTheToggle(): void + { + $enabled = $this->buildSchema(BookController::class); + $this->assertSame( + 'Number of Books to return', + $enabled->getQueryType()->getField('books')->getArg('first')->description, + ); + + $disabled = $this->buildSchema(BookController::class, docblockDescriptions: false); + $this->assertNull($disabled->getQueryType()->getField('books')->getArg('first')->description); + } + + public function testDescriptionItemsAreListedAfterTheDescription(): void + { + $schema = $this->buildSchema(BookController::class); + $query = $schema->getQueryType(); + + // Items keep the middleware pipe order: user middlewares, then #[Cost] + $this->assertSame( + "Paginated list of Books, optionally filtered\n" + . "\n" + . "- Audience: librarians\n" + . "- Audience: members\n" + . '- Cost: complexity = 5, multipliers = [first], defaultMultiplier = null', + $query->getField('books')->description, + ); + $this->assertSame( + "Counts the Books on loan.\n\n- Cost: complexity = 2, multipliers = [], defaultMultiplier = null", + $query->getField('loanedBookCount')->description, + ); + $this->assertSame( + "- Audience: librarians\n- Cost: complexity = 3, multipliers = [], defaultMultiplier = null", + $query->getField('undescribedBooks')->description, + ); + $this->assertSame( + '- Cost: complexity = 4, multipliers = [], defaultMultiplier = null', + $query->getField('emptyDescribedBooks')->description, + ); + + $input = $schema->getType('BookInput'); + assert($input instanceof InputObjectType); + + // The Undefined item comes first, since it is added before the input field middlewares run + $this->assertSame( + "Title of the Book.\n\n- May be omitted; null is not accepted.\n- Audience: librarians", + $input->getField('title')->description, + ); + $this->assertSame( + "Publication date, if any\n\n- May be omitted; null is accepted.\n- Audience: librarians", + $input->getField('publishedOn')->description, + ); + $this->assertSame('- Audience: librarians', $input->getField('notes')->description); + } + + public function testDescriptionItemsWithoutDocblockFallback(): void + { + $schema = $this->buildSchema(BookController::class, docblockDescriptions: false); + $query = $schema->getQueryType(); + + $this->assertSame( + '- Cost: complexity = 2, multipliers = [], defaultMultiplier = null', + $query->getField('loanedBookCount')->description, + ); + $this->assertStringStartsWith( + "Paginated list of Books, optionally filtered\n\n- Audience: librarians\n", + $query->getField('books')->description, + ); + + $input = $schema->getType('BookInput'); + assert($input instanceof InputObjectType); + $this->assertSame( + "- May be omitted; null is not accepted.\n- Audience: librarians", + $input->getField('title')->description, + ); + } + + public function testDescriptionItemsAreNotRepeatedAcrossBuildsSharingACache(): void + { + $cache = new Psr16Cache(new ArrayAdapter()); + $descriptions = []; + for ($build = 0; $build < 2; $build++) { + $schema = $this->buildSchema(BookController::class, cache: $cache); + $input = $schema->getType('BookInput'); + assert($input instanceof InputObjectType); + $descriptions[] = [ + $schema->getQueryType()->getField('books')->description, + $input->getField('title')->description, + ]; + } + + $this->assertSame($descriptions[0], $descriptions[1]); + $this->assertSame(1, substr_count($descriptions[1][0], 'Cost:')); + $this->assertSame(1, substr_count($descriptions[1][1], 'May be omitted')); + } + + /** @return array */ + private function introspectInputFieldDescriptions(Schema $schema): array + { + $result = GraphQL::executeQuery( + $schema, + '{ __type(name: "UndefinedDescriptionInput") { inputFields { name description } } }', + )->toArray(DebugFlag::RETHROW_INTERNAL_EXCEPTIONS); + + return array_column($result['data']['__type']['inputFields'], 'description', 'name'); + } + + /** @return array */ + private function introspectArgumentDescriptions(Schema $schema): array + { + $result = GraphQL::executeQuery( + $schema, + '{ __type(name: "Query") { fields { name args { name description } } } }', + )->toArray(DebugFlag::RETHROW_INTERNAL_EXCEPTIONS); + + foreach ($result['data']['__type']['fields'] as $field) { + if ($field['name'] === 'undefinedDescriptionArguments') { + return array_column($field['args'], 'description', 'name'); + } + } + + $this->fail('The undefinedDescriptionArguments query is missing'); + } } diff --git a/tests/Integration/EndToEndTest.php b/tests/Integration/EndToEndTest.php index 42d44e80bc..e494162d4d 100644 --- a/tests/Integration/EndToEndTest.php +++ b/tests/Integration/EndToEndTest.php @@ -9,7 +9,10 @@ use GraphQL\Server\Helper; use GraphQL\Server\OperationParams; use GraphQL\Server\ServerConfig; +use GraphQL\Type\Definition\InputObjectType; +use GraphQL\Type\Definition\Type; use GraphQL\Utils\SchemaPrinter; +use PHPUnit\Framework\Attributes\DataProvider; use Psr\Container\ContainerInterface; use stdClass; use Symfony\Component\Cache\Adapter\ArrayAdapter; @@ -2377,6 +2380,445 @@ public function testEndToEndInputUndefinedListValue(): void $this->assertNull($data['nullified']['tags']); } + public function testEndToEndInputUndefinedOmittedValues(): void + { + $schema = $this->mainContainer->get(Schema::class); + assert($schema instanceof Schema); + + $result = GraphQL::executeQuery($schema, ' + mutation { + updateOmittable(input: {}) + } + '); + + $this->assertSame([ + 'property: undefined', + 'nullableProperty: undefined', + 'setter: undefined', + 'nullableSetter: undefined', + 'promotedParameter: undefined', + 'nullablePromotedParameter: undefined', + 'constructorParameter: undefined', + 'nullableConstructorParameter: undefined', + 'propertyWithNullableConstructorParameter: undefined', + 'nullablePropertyWithConstructorParameter: undefined', + 'propertyWithNullableSetter: undefined', + 'nullablePropertyWithSetter: undefined', + 'nullablePropertyWithNullDefault: null', + 'list: undefined', + 'nullableList: undefined', + ], $this->getSuccessResult($result)['updateOmittable']); + + $result = GraphQL::executeQuery($schema, ' + query { + omittableArguments + } + '); + + $this->assertSame([ + 'argument: undefined', + 'nullableArgument: undefined', + 'list: undefined', + ], $this->getSuccessResult($result)['omittableArguments']); + + $result = GraphQL::executeQuery($schema, ' + query { + omittableFactory(value: {}) + } + '); + + $this->assertSame([ + 'value: undefined', + 'nullableValue: undefined', + ], $this->getSuccessResult($result)['omittableFactory']); + } + + public function testEndToEndInputUndefinedProvidedValues(): void + { + $schema = $this->mainContainer->get(Schema::class); + assert($schema instanceof Schema); + + $result = GraphQL::executeQuery($schema, ' + mutation { + updateOmittable(input: { + property: 1, + nullableProperty: 2, + setter: 3, + nullableSetter: 4, + promotedParameter: 5, + nullablePromotedParameter: 6, + constructorParameter: 7, + nullableConstructorParameter: 8, + propertyWithNullableConstructorParameter: 9, + nullablePropertyWithConstructorParameter: 10, + propertyWithNullableSetter: 11, + nullablePropertyWithSetter: 12, + nullablePropertyWithNullDefault: 16, + list: [13, 14], + nullableList: [15], + }) + } + '); + + $this->assertSame([ + 'property: 1', + 'nullableProperty: 2', + 'setter: 3', + 'nullableSetter: 4', + 'promotedParameter: 5', + 'nullablePromotedParameter: 6', + 'constructorParameter: 7', + 'nullableConstructorParameter: 8', + 'propertyWithNullableConstructorParameter: 9', + 'nullablePropertyWithConstructorParameter: 10', + 'propertyWithNullableSetter: 11', + 'nullablePropertyWithSetter: 12', + 'nullablePropertyWithNullDefault: 16', + 'list: [13,14]', + 'nullableList: [15]', + ], $this->getSuccessResult($result)['updateOmittable']); + + $result = GraphQL::executeQuery($schema, ' + query { + omittableArguments(argument: 1, nullableArgument: 2, list: [3]) + } + '); + + $this->assertSame([ + 'argument: 1', + 'nullableArgument: 2', + 'list: [3]', + ], $this->getSuccessResult($result)['omittableArguments']); + + $result = GraphQL::executeQuery($schema, ' + query { + omittableFactory(value: { value: 1, nullableValue: 2 }) + } + '); + + $this->assertSame([ + 'value: 1', + 'nullableValue: 2', + ], $this->getSuccessResult($result)['omittableFactory']); + } + + public function testEndToEndInputUndefinedNullableAcceptsNull(): void + { + $schema = $this->mainContainer->get(Schema::class); + assert($schema instanceof Schema); + + $result = GraphQL::executeQuery($schema, ' + mutation { + updateOmittable(input: { + nullableProperty: null, + nullableSetter: null, + nullablePromotedParameter: null, + nullableConstructorParameter: null, + propertyWithNullableConstructorParameter: null, + propertyWithNullableSetter: null, + nullableList: null, + }) + } + '); + + // The constructor parameter and setter that accept null for a non-nullable property normalize it to 0. + $this->assertSame([ + 'property: undefined', + 'nullableProperty: null', + 'setter: undefined', + 'nullableSetter: null', + 'promotedParameter: undefined', + 'nullablePromotedParameter: null', + 'constructorParameter: undefined', + 'nullableConstructorParameter: null', + 'propertyWithNullableConstructorParameter: 0', + 'nullablePropertyWithConstructorParameter: undefined', + 'propertyWithNullableSetter: 0', + 'nullablePropertyWithSetter: undefined', + 'nullablePropertyWithNullDefault: null', + 'list: undefined', + 'nullableList: null', + ], $this->getSuccessResult($result)['updateOmittable']); + + $result = GraphQL::executeQuery($schema, ' + mutation { + updateOmittableList(inputs: [ + { propertyWithNullableConstructorParameter: null }, + { propertyWithNullableSetter: null }, + ]) + } + '); + + $data = $this->getSuccessResult($result)['updateOmittableList']; + $this->assertContains('propertyWithNullableConstructorParameter: 0', $data); + $this->assertContains('propertyWithNullableSetter: 0', $data); + + $result = GraphQL::executeQuery($schema, ' + query { + omittableArguments(nullableArgument: null) + } + '); + + $this->assertSame([ + 'argument: undefined', + 'nullableArgument: null', + 'list: undefined', + ], $this->getSuccessResult($result)['omittableArguments']); + + $result = GraphQL::executeQuery($schema, ' + query { + omittableFactory(value: { nullableValue: null }) + } + '); + + $this->assertSame([ + 'value: undefined', + 'nullableValue: null', + ], $this->getSuccessResult($result)['omittableFactory']); + } + + /** @return iterable */ + public static function undefinedRefusesNullProvider(): iterable + { + $fields = [ + 'property', + 'setter', + 'promotedParameter', + 'constructorParameter', + 'nullablePropertyWithConstructorParameter', + 'nullablePropertyWithSetter', + 'list', + ]; + foreach ($fields as $field) { + yield 'input field ' . $field => ['mutation { updateOmittable(input: { ' . $field . ': null }) }', $field]; + } + + yield 'input field without a default' => ['mutation { updateOmittableWithoutDefault(input: { property: null }) }', 'property']; + + foreach (['updateOmittableProperties', 'updateOmittableMagicSetter'] as $mutation) { + foreach (['property', 'propertyWithoutDefault'] as $field) { + yield $mutation . ' field ' . $field => ['mutation { ' . $mutation . '(input: { ' . $field . ': null }) }', $field]; + } + } + + yield 'input field in a list' =>['mutation { updateOmittableList(inputs: [{ property: 1 }, { property: null }]) }', 'property']; + yield 'factory parameter' => ['query { omittableFactory(value: { value: null }) }', 'value']; + + foreach (['argument', 'list'] as $argument) { + yield 'argument ' . $argument => ['query { omittableArguments(' . $argument . ': null) }', $argument]; + } + + yield 'variable' => ['mutation($value: Int) { updateOmittable(input: { property: $value }) }', 'property']; + yield 'argument variable' => ['query($value: Int) { omittableArguments(argument: $value) }', 'argument']; + } + + #[DataProvider('undefinedRefusesNullProvider')] + public function testEndToEndInputUndefinedRefusesNull(string $queryString, string $name): void + { + $schema = $this->mainContainer->get(Schema::class); + assert($schema instanceof Schema); + + $result = GraphQL::executeQuery($schema, $queryString, variableValues: ['value' => null]); + + // Unsafe exceptions, such as a TypeError from assigning null, are rethrown here. + $errors = $result->toArray(DebugFlag::RETHROW_UNSAFE_EXCEPTIONS)['errors']; + $this->assertCount(1, $errors); + $this->assertSame("Argument '" . $name . "' may be omitted but cannot be null", $errors[0]['message']); + } + + public function testEndToEndInputUndefinedVariables(): void + { + $schema = $this->mainContainer->get(Schema::class); + assert($schema instanceof Schema); + + $mutation = 'mutation($value: Int) { updateOmittable(input: { property: $value, nullableProperty: $value }) }'; + $query = 'query($value: Int) { omittableArguments(argument: $value, nullableArgument: $value) }'; + + // A variable that isn't provided leaves the field or argument omitted + $data = $this->getSuccessResult(GraphQL::executeQuery($schema, $mutation, variableValues: []))['updateOmittable']; + $this->assertContains('property: undefined', $data); + $this->assertContains('nullableProperty: undefined', $data); + + $data = $this->getSuccessResult(GraphQL::executeQuery($schema, $query, variableValues: []))['omittableArguments']; + $this->assertContains('argument: undefined', $data); + $this->assertContains('nullableArgument: undefined', $data); + + // A variable provided as null is an explicit null, which only `T|null|Undefined` accepts + $result = GraphQL::executeQuery( + $schema, + 'mutation($value: Int) { updateOmittable(input: { nullableProperty: $value }) }', + variableValues: ['value' => null], + ); + $this->assertContains('nullableProperty: null', $this->getSuccessResult($result)['updateOmittable']); + + $result = GraphQL::executeQuery( + $schema, + 'query($value: Int) { omittableArguments(nullableArgument: $value) }', + variableValues: ['value' => null], + ); + $this->assertContains('nullableArgument: null', $this->getSuccessResult($result)['omittableArguments']); + + foreach ([[$mutation, 'property'], [$query, 'argument']] as [$queryString, $name]) { + $errors = GraphQL::executeQuery($schema, $queryString, variableValues: ['value' => null]) + ->toArray(DebugFlag::RETHROW_UNSAFE_EXCEPTIONS)['errors']; + $this->assertCount(1, $errors); + $this->assertSame("Argument '" . $name . "' may be omitted but cannot be null", $errors[0]['message']); + } + } + + public function testEndToEndInputUndefinedFactoryDescriptions(): void + { + $schema = $this->mainContainer->get(Schema::class); + assert($schema instanceof Schema); + + $inputType = Type::getNamedType($schema->getQueryType()->getField('omittableFactory')->getArg('value')->getType()); + assert($inputType instanceof InputObjectType); + + $this->assertSame('- May be omitted; null is not accepted.', $inputType->getField('value')->description); + $this->assertSame('- May be omitted; null is accepted.', $inputType->getField('nullableValue')->description); + } + + public function testEndToEndInputUndefinedWithoutDefault(): void + { + $schema = $this->mainContainer->get(Schema::class); + assert($schema instanceof Schema); + + // Undefined is not a GraphQL default, so none is printed, not even `null` for the nullable fields. + $inputType = $schema->getType('OmittableWithoutDefaultInput'); + assert($inputType instanceof InputObjectType); + foreach ($inputType->getFields() as $field) { + $this->assertFalse($field->defaultValueExists(), $field->name); + } + + $result = GraphQL::executeQuery($schema, ' + mutation { + updateOmittableWithoutDefault(input: {}) + } + '); + + $this->assertSame([ + 'property: undefined', + 'nullableProperty: undefined', + 'docBlockProperty: undefined', + 'nullablePromotedParameter: undefined', + 'nullableConstructorParameter: undefined', + 'nullablePropertyWithSetter: undefined', + 'nullableSetter: undefined', + // A property the constructor assigned keeps that value, so a readonly one isn't assigned twice + 'assignedInConstructor: 10', + 'docBlockAssignedInConstructor: 11', + 'readonlyAssignedInConstructor: undefined', + ], $this->getSuccessResult($result)['updateOmittableWithoutDefault']); + + $result = GraphQL::executeQuery($schema, ' + mutation { + updateOmittableWithoutDefault(input: { + nullableProperty: null, + docBlockProperty: null, + nullablePromotedParameter: null, + nullableConstructorParameter: null, + nullablePropertyWithSetter: null, + nullableSetter: null, + }) + } + '); + + $this->assertSame([ + 'property: undefined', + 'nullableProperty: null', + 'docBlockProperty: null', + 'nullablePromotedParameter: null', + 'nullableConstructorParameter: null', + 'nullablePropertyWithSetter: null', + 'nullableSetter: null', + 'assignedInConstructor: 10', + 'docBlockAssignedInConstructor: 11', + 'readonlyAssignedInConstructor: undefined', + ], $this->getSuccessResult($result)['updateOmittableWithoutDefault']); + + $result = GraphQL::executeQuery($schema, ' + mutation { + updateOmittableWithoutDefault(input: { + property: 1, + nullableProperty: 2, + docBlockProperty: 3, + nullablePromotedParameter: 4, + nullableConstructorParameter: 5, + nullablePropertyWithSetter: 6, + nullableSetter: 7, + assignedInConstructor: 8, + docBlockAssignedInConstructor: 9, + }) + } + '); + + $this->assertSame([ + 'property: 1', + 'nullableProperty: 2', + 'docBlockProperty: 3', + 'nullablePromotedParameter: 4', + 'nullableConstructorParameter: 5', + 'nullablePropertyWithSetter: 6', + 'nullableSetter: 7', + 'assignedInConstructor: 8', + 'docBlockAssignedInConstructor: 9', + 'readonlyAssignedInConstructor: undefined', + ], $this->getSuccessResult($result)['updateOmittableWithoutDefault']); + } + + /** @return iterable */ + public static function undefinedWithoutConstructorProvider(): iterable + { + yield 'public properties' => ['OmittablePropertiesInput', 'updateOmittableProperties']; + yield 'magic setter' => ['OmittableMagicSetterInput', 'updateOmittableMagicSetter']; + } + + #[DataProvider('undefinedWithoutConstructorProvider')] + public function testEndToEndInputUndefinedWithoutConstructor(string $typeName, string $mutation): void + { + $schema = $this->mainContainer->get(Schema::class); + assert($schema instanceof Schema); + + // Undefined is not a GraphQL default, so none is printed, not even `null` for the nullable fields. + $inputType = $schema->getType($typeName); + assert($inputType instanceof InputObjectType); + foreach ($inputType->getFields() as $field) { + $this->assertFalse($field->defaultValueExists(), $field->name); + } + $this->assertStringNotContainsString('=', SchemaPrinter::printType($inputType)); + + $result = GraphQL::executeQuery($schema, 'mutation { ' . $mutation . '(input: {}) }'); + + $this->assertSame([ + 'property: undefined', + 'nullableProperty: undefined', + 'propertyWithoutDefault: undefined', + 'nullablePropertyWithoutDefault: undefined', + ], $this->getSuccessResult($result)[$mutation]); + + $result = GraphQL::executeQuery($schema, 'mutation { + ' . $mutation . '(input: { property: 1, nullableProperty: 2, propertyWithoutDefault: 3, nullablePropertyWithoutDefault: 4 }) + }'); + + $this->assertSame([ + 'property: 1', + 'nullableProperty: 2', + 'propertyWithoutDefault: 3', + 'nullablePropertyWithoutDefault: 4', + ], $this->getSuccessResult($result)[$mutation]); + + $result = GraphQL::executeQuery($schema, 'mutation { + ' . $mutation . '(input: { nullableProperty: null, nullablePropertyWithoutDefault: null }) + }'); + + $this->assertSame([ + 'property: undefined', + 'nullableProperty: null', + 'propertyWithoutDefault: undefined', + 'nullablePropertyWithoutDefault: null', + ], $this->getSuccessResult($result)[$mutation]); + } + public function testEndToEndSchemaIsPrintable(): void { $this->expectNotToPerformAssertions(); diff --git a/tests/Integration/QueryComplexityTest.php b/tests/Integration/QueryComplexityTest.php index 23ffddb569..a8d9e9cf65 100644 --- a/tests/Integration/QueryComplexityTest.php +++ b/tests/Integration/QueryComplexityTest.php @@ -172,7 +172,7 @@ public function testReportsQueryCostInIntrospection(string|null $expectedDescrip public static function reportsQueryCostInIntrospectionProvider(): iterable { yield [ - "\nCost: complexity = 5, multipliers = [take], defaultMultiplier = 500", + "- Cost: complexity = 5, multipliers = [take], defaultMultiplier = 500", 'Query', 'articles', ]; @@ -184,13 +184,13 @@ public static function reportsQueryCostInIntrospectionProvider(): iterable ]; yield [ - "\nCost: complexity = 5, multipliers = [], defaultMultiplier = null", + "- Cost: complexity = 5, multipliers = [], defaultMultiplier = null", 'Post', 'comment', ]; yield [ - "\nCost: complexity = 3, multipliers = [], defaultMultiplier = null", + "- Cost: complexity = 3, multipliers = [], defaultMultiplier = null", 'Post', 'author', ]; diff --git a/tests/Mappers/Parameters/TypeMapperTest.php b/tests/Mappers/Parameters/TypeMapperTest.php index f0f2be771e..b80b5ffe54 100644 --- a/tests/Mappers/Parameters/TypeMapperTest.php +++ b/tests/Mappers/Parameters/TypeMapperTest.php @@ -8,6 +8,7 @@ use GraphQL\Type\Definition\ResolveInfo; use GraphQL\Type\Definition\UnionType; use ReflectionMethod; +use ReflectionProperty; use Symfony\Component\Cache\Adapter\ArrayAdapter; use Symfony\Component\Cache\Psr16Cache; use TheCodingMachine\GraphQLite\AbstractQueryProvider; @@ -16,6 +17,7 @@ use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeException; use TheCodingMachine\GraphQLite\Parameters\DefaultValueParameter; use TheCodingMachine\GraphQLite\Parameters\InputTypeParameter; +use TheCodingMachine\GraphQLite\Parameters\NullArgumentException; use TheCodingMachine\GraphQLite\Undefined; use function assert; @@ -24,6 +26,15 @@ class TypeMapperTest extends AbstractQueryProvider { + private int|Undefined $undefinedProperty = Undefined::VALUE; + + private int|null|Undefined $nullableUndefinedProperty = Undefined::VALUE; + + private int|null|Undefined $nullableUndefinedPropertyWithoutDefault; + + /** @var int|null|Undefined */ + private $docBlockUndefinedProperty; + public function testMapScalarUnionException(): void { $docBlockFactory = $this->getDocBlockFactory(); @@ -123,6 +134,114 @@ public function testMapUndefinedListParameterDoesNotCreateForbiddenUnion(): void $this->assertNull($parameter->getDefaultValue()); } + public function testUndefinedParameterRefusesNullUnlessNullable(): void + { + $docBlockFactory = $this->getDocBlockFactory(); + + $typeMapper = new TypeHandler( + $this->getArgumentResolver(), + $this->getRootTypeMapper(), + $this->getTypeResolver(), + $docBlockFactory, + ); + + $refMethod = new ReflectionMethod($this, 'withUndefined'); + $docBlockObj = $docBlockFactory->create($refMethod); + $resolveInfo = $this->createMock(ResolveInfo::class); + [$refParameter, $refNullableParameter] = $refMethod->getParameters(); + $annotations = $this->getAnnotationReader()->getParameterAnnotationsPerParameter($refMethod->getParameters()); + + $nullableParameter = $typeMapper->mapParameter($refNullableParameter, $docBlockObj, null, $annotations['nullableFoo']); + $this->assertNull($nullableParameter->resolve(null, ['nullableFoo' => null], null, $resolveInfo)); + $this->assertSame(Undefined::VALUE, $nullableParameter->resolve(null, [], null, $resolveInfo)); + + $parameter = $typeMapper->mapParameter($refParameter, $docBlockObj, null, $annotations['foo']); + $this->assertSame('Int', $parameter->getType()->toString()); + $this->assertSame(Undefined::VALUE, $parameter->resolve(null, [], null, $resolveInfo)); + + $this->expectException(NullArgumentException::class); + $this->expectExceptionMessage("Argument 'foo' may be omitted but cannot be null"); + $parameter->resolve(null, ['foo' => null], null, $resolveInfo); + } + + public function testUndefinedInputPropertyRefusesNullUnlessNullable(): void + { + $docBlockFactory = $this->getDocBlockFactory(); + + $typeMapper = new TypeHandler( + $this->getArgumentResolver(), + $this->getRootTypeMapper(), + $this->getTypeResolver(), + $docBlockFactory, + ); + + $resolveInfo = $this->createMock(ResolveInfo::class); + + $refNullableProperty = new ReflectionProperty($this, 'nullableUndefinedProperty'); + $nullableProperty = $typeMapper->mapInputProperty($refNullableProperty, $docBlockFactory->create($refNullableProperty), 'nullableUndefinedProperty'); + $this->assertNull($nullableProperty->resolve(null, ['nullableUndefinedProperty' => null], null, $resolveInfo)); + + $refProperty = new ReflectionProperty($this, 'undefinedProperty'); + $property = $typeMapper->mapInputProperty($refProperty, $docBlockFactory->create($refProperty), 'undefinedProperty'); + $this->assertSame('Int', $property->getType()->toString()); + $this->assertSame(1, $property->resolve(null, ['undefinedProperty' => 1], null, $resolveInfo)); + + $this->expectException(NullArgumentException::class); + $this->expectExceptionMessage("Argument 'undefinedProperty' may be omitted but cannot be null"); + $property->resolve(null, ['undefinedProperty' => null], null, $resolveInfo); + } + + public function testNullableUndefinedInputPropertyDefaultsToUndefined(): void + { + $docBlockFactory = $this->getDocBlockFactory(); + + $typeMapper = new TypeHandler( + $this->getArgumentResolver(), + $this->getRootTypeMapper(), + $this->getTypeResolver(), + $docBlockFactory, + ); + + $resolveInfo = $this->createMock(ResolveInfo::class); + + foreach (['nullableUndefinedPropertyWithoutDefault', 'docBlockUndefinedProperty'] as $name) { + $refProperty = new ReflectionProperty($this, $name); + $property = $typeMapper->mapInputProperty($refProperty, $docBlockFactory->create($refProperty), $name); + + $this->assertSame('Int', $property->getType()->toString(), $name); + // Undefined, not a printed `null` that GraphQL would fill in for an omitted field. + $this->assertFalse($property->hasDefaultValue(), $name); + $this->assertTrue($property->isDefaultValueUndefined(), $name); + $this->assertSame(Undefined::VALUE, $property->resolve(null, [], null, $resolveInfo), $name); + $this->assertNull($property->resolve(null, [$name => null], null, $resolveInfo), $name); + } + } + + public function testDocBlockUndefinedParameterDefaultsToUndefined(): void + { + $docBlockFactory = $this->getDocBlockFactory(); + + $typeMapper = new TypeHandler( + $this->getArgumentResolver(), + $this->getRootTypeMapper(), + $this->getTypeResolver(), + $docBlockFactory, + ); + + $refMethod = new ReflectionMethod($this, 'withDocBlockUndefined'); + $refParameter = $refMethod->getParameters()[0]; + $docBlockObj = $docBlockFactory->create($refMethod); + $paramTags = $docBlockObj->getTagsByName('param'); + $annotations = $this->getAnnotationReader()->getParameterAnnotationsPerParameter([$refParameter])['foo']; + + $parameter = $typeMapper->mapParameter($refParameter, $docBlockObj, reset($paramTags)->getType(), $annotations); + assert($parameter instanceof InputTypeParameter); + + $this->assertSame('Int', $parameter->getType()->toString()); + $this->assertFalse($parameter->hasDefaultValue()); + $this->assertSame(Undefined::VALUE, $parameter->resolve(null, [], null, $this->createMock(ResolveInfo::class))); + } + public function testHideParameter(): void { $docBlockFactory = $this->getDocBlockFactory(); @@ -199,6 +318,19 @@ private function withUndefinedList(array|Undefined|null $foo = Undefined::VALUE) { } + private function withUndefined(int|Undefined $foo, int|null|Undefined $nullableFoo): void + { + } + + /** + * @param int|null|Undefined $foo + * + * @phpcsSuppress SlevomatCodingStandard.TypeHints.ParameterTypeHint.MissingNativeTypeHint + */ + private function withDocBlockUndefined($foo): void + { + } + /** @param int $foo Foo parameter */ private function withParamDescription(int $foo): void { diff --git a/tests/Middlewares/CostFieldMiddlewareTest.php b/tests/Middlewares/CostFieldMiddlewareTest.php index 3c112c0f82..e19bf3ad15 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('withAddedDescriptionLines') + ->method('withAddedDescriptionItem') ->with($expectedDescription) ->willReturnSelf(); @@ -106,17 +106,17 @@ public function testAddsCostInDescription(string $expectedDescription, Cost $cos public static function addsCostInDescriptionProvider(): iterable { yield [ - "\nCost: complexity = 1, multipliers = [], defaultMultiplier = null", + "Cost: complexity = 1, multipliers = [], defaultMultiplier = null", new Cost(), ]; yield [ - "\nCost: complexity = 5, multipliers = [take], defaultMultiplier = 500", + "Cost: complexity = 5, multipliers = [take], defaultMultiplier = 500", new Cost(complexity: 5, multipliers: ['take'], defaultMultiplier: 500) ]; yield [ - "\nCost: complexity = 5, multipliers = [take, null], defaultMultiplier = null", + "Cost: complexity = 5, multipliers = [take, null], defaultMultiplier = null", new Cost(complexity: 5, multipliers: ['take', 'null'], defaultMultiplier: null) ]; } diff --git a/tests/QueryFieldDescriptorTest.php b/tests/QueryFieldDescriptorTest.php index f58f778421..d944a07988 100644 --- a/tests/QueryFieldDescriptorTest.php +++ b/tests/QueryFieldDescriptorTest.php @@ -16,6 +16,10 @@ public function testWithAddedDescriptionLine( string $added, ): void { + $this->expectUserDeprecationMessage( + 'QueryFieldDescriptor::withAddedDescriptionLines() is deprecated; use withAddedDescriptionItem() instead.', + ); + $resolver = fn () => null; $descriptor = (new QueryFieldDescriptor( @@ -35,4 +39,27 @@ public static function withAddedDescriptionLineProvider(): iterable yield ['Asd', null, 'Asd']; yield ["Some description\nAsd", 'Some description', 'Asd']; } + + public function testDescriptionItemsAreKeptApartFromTheDescription(): void + { + $resolver = fn () => null; + + $descriptor = (new QueryFieldDescriptor( + 'test', + Type::string(), + resolver: $resolver, + originalResolver: new ServiceResolver($resolver), + description: 'From the docblock', + )) + ->withAddedDescriptionItem('Audience: librarians') + ->withAddedDescriptionItem('Cost: complexity = 5') + ->withDescription('Explicit'); + + self::assertSame('Explicit', $descriptor->getDescription()); + self::assertSame(['Audience: librarians', 'Cost: complexity = 5'], $descriptor->getDescriptionItems()); + self::assertSame( + "Explicit\n\n- Audience: librarians\n- Cost: complexity = 5", + QueryField::fromFieldDescriptor($descriptor)->description, + ); + } } diff --git a/tests/Utils/DescriptionResolverTest.php b/tests/Utils/DescriptionResolverTest.php index 93624f2d9c..e0a66728f4 100644 --- a/tests/Utils/DescriptionResolverTest.php +++ b/tests/Utils/DescriptionResolverTest.php @@ -81,4 +81,32 @@ public function testIsDocblockFallbackEnabledReflectsConstructor(): void self::assertTrue((new DescriptionResolver(true))->isDocblockFallbackEnabled()); self::assertFalse((new DescriptionResolver(false))->isDocblockFallbackEnabled()); } + + public function testDescribeUndefined(): void + { + $resolver = new DescriptionResolver(true); + + self::assertSame(['May be omitted; null is not accepted.'], $resolver->describeUndefined(true)); + self::assertSame(['May be omitted; null is accepted.'], $resolver->describeUndefined(false)); + self::assertSame([], (new DescriptionResolver(true, false))->describeUndefined(true)); + } + + /** @param list $items */ + #[DataProvider('provideAppendItemsCases')] + public function testAppendItems(string|null $expected, string|null $description, array $items): void + { + self::assertSame($expected, DescriptionResolver::appendItems($description, $items)); + } + + public static function provideAppendItemsCases(): iterable + { + yield 'no items keeps no description' => [null, null, []]; + yield 'no items keeps the description untouched' => ["Age in years\n", "Age in years\n", []]; + yield 'items without a description' => ["- Audience: librarians\n- Cost: complexity = 5", null, ['Audience: librarians', 'Cost: complexity = 5']]; + yield 'items after an empty description' => ['- Cost: complexity = 5', '', ['Cost: complexity = 5']]; + yield 'items after a blank line' => ["Age in years\n\n- First\n- Second", "Age in years\n", ['First', 'Second']]; + yield 'item the description already contains' => ['Age. May be omitted; null is not accepted.', 'Age. May be omitted; null is not accepted.', ['May be omitted; null is not accepted.']]; + yield 'repeated item' => ["Age\n\n- First", 'Age', ['First', 'First']]; + yield 'surrounding whitespace and blank items' => ["Age\n\n- Cost: complexity = 1", 'Age', ["\nCost: complexity = 1", ' ']]; + } } diff --git a/website/docs/CHANGELOG.md b/website/docs/CHANGELOG.md index 621818b057..4a8cb1ac7a 100644 --- a/website/docs/CHANGELOG.md +++ b/website/docs/CHANGELOG.md @@ -20,6 +20,15 @@ sidebar_label: Changelog field name. **Migration**: if a `#[Field]`/`#[SourceField]` method relied on the old behavior (for example `issue()` exposing a field named `sue`), set an explicit field name with `#[Field(name:)]` or the source field's `name`. +- Description metadata is rendered as a Markdown list after a blank line, so the `#[Cost]` text in + introspection and printed schemas changes from `Cost: complexity = ...` on its own line to the list item + `- Cost: complexity = ...`. **Migration**: update anything that parses or snapshots field descriptions. + @oojacoboo +- An omitted input field typed `T|null|Undefined` without a PHP default now resolves to `Undefined::VALUE` + instead of `null`, and no longer prints a `= null` default in the schema. A constructor parameter or + property without a PHP default gets `Undefined::VALUE` too, instead of being left unset, unless the + constructor already assigned the property. **Migration**: code that treated `null` as "not sent" for such + a field should compare against `Undefined::VALUE`. @oojacoboo ### New Features @@ -35,11 +44,22 @@ sidebar_label: Changelog - [#822 Accept PHP callables as `#[Security]` rules](https://github.com/thecodingmachine/graphqlite/pull/822) @oojacoboo, alongside the existing expression form, with access to the field context and custom refusal messages. +- Arguments and input fields typed `T|Undefined` now say in their GraphQL description that they may be + omitted and whether null is accepted ("May be omitted; null is not accepted." or "May be omitted; null is + accepted."), since GraphQL types can't express it. Disable with + `SchemaFactory::setUndefinedDescriptionsEnabled(false)`. @oojacoboo +- Field and input field middlewares add description metadata as list items with + `withAddedDescriptionItem()`. `QueryFieldDescriptor::withAddedDescriptionLines()` is deprecated in favor of + it. @oojacoboo ### Bug Fixes - [#819 Fix undefined array input type](https://github.com/thecodingmachine/graphqlite/pull/819) @michael-georgiadis +- An explicit `null` sent for an argument or input field typed `T|Undefined` (without `null`) is now + refused with a client error naming it, instead of failing with a `TypeError`. @oojacoboo +- [#793](https://github.com/thecodingmachine/graphqlite/pull/793) Argument `@param` descriptions no longer + reach the schema when `SchemaFactory::setDocblockDescriptionsEnabled(false)` is set. @oojacoboo ## >8.0.0 diff --git a/website/docs/descriptions.md b/website/docs/descriptions.md index ffeae92a09..339d94a05a 100644 --- a/website/docs/descriptions.md +++ b/website/docs/descriptions.md @@ -68,6 +68,29 @@ one: fallback is enabled on the `SchemaFactory`. 4. Otherwise the schema description is empty. +## Metadata items + +GraphQLite lists some facts about a field after its description, as a Markdown list following a blank line: +the `#[Cost]` of a field (see [Operation complexity](operation-complexity.md)), and whether an argument or +input field typed `T|Undefined` accepts null (see [Optional input fields](input-types.mdx#optional-input-fields), +with its own toggle, `setUndefinedDescriptionsEnabled()`). Items are listed even after an explicit +`description: ''`, and without a description only the list remains: + +```graphql +""" +Paginated list of Books, optionally filtered + +- Cost: complexity = 5, multipliers = [first], defaultMultiplier = null +""" +books(first: Int): [Book!]! +``` + +A [field middleware](field-middlewares.md) adds its own item with +`QueryFieldDescriptor::withAddedDescriptionItem()`, and an input field middleware with +`InputFieldDescriptor::withAddedDescriptionItem()`. Items are listed in the order the middlewares add +them, are kept when a middleware replaces the description with `withDescription()`, and an item the +description already contains is left out. + ## Disabling the docblock fallback Docblocks double as developer-facing notes (implementation reminders, `@see` references, TODOs). diff --git a/website/docs/field-middlewares.md b/website/docs/field-middlewares.md index 63cf50e6b8..6690e2145d 100644 --- a/website/docs/field-middlewares.md +++ b/website/docs/field-middlewares.md @@ -49,6 +49,9 @@ class QueryFieldDescriptor public function withInjectSource(bool $injectSource): self { /* ... */ } public function getDescription(): ?string { /* ... */ } public function withDescription(?string $description): self { /* ... */ } + public function getDescriptionItems(): array { /* ... */ } + public function withAddedDescriptionItem(string $item): self { /* ... */ } + /** @deprecated Use withAddedDescriptionItem() */ public function withAddedDescriptionLines(string $description): self { /* ... */ } public function getMiddlewareAnnotations(): MiddlewareAnnotations { /* ... */ } public function withMiddlewareAnnotations(MiddlewareAnnotations $middlewareAnnotations): self { /* ... */ } @@ -60,6 +63,10 @@ class QueryFieldDescriptor The role of a middleware is to analyze the `QueryFieldDescriptor` and modify it (or to directly return a `FieldDefinition`). +To publish metadata about a field (an access rule, a cost), add it with `withAddedDescriptionItem()` rather than +editing the description: items are listed after the description as a Markdown list (see +[Metadata items](descriptions.md#metadata-items)). + If you want the field to purely disappear, your middleware can return `null`, although this should be used with caution: field middlewares only get called once per Schema instance. If you use a long-running server (like Laravel Octane, Swoole, RoadRunner etc) and share the same Schema instance across requests, you will not be able to hide fields based on request data. diff --git a/website/docs/input-types.mdx b/website/docs/input-types.mdx index 5e11a3c62f..9ee63cae9c 100644 --- a/website/docs/input-types.mdx +++ b/website/docs/input-types.mdx @@ -175,7 +175,7 @@ class UpdateUserInput public string $username; #[Field] - protected int|null|Undefined $age; + public int|null|Undefined $age; } ``` @@ -210,11 +210,37 @@ class Article } ``` -For field parameters and input field properties, an explicit `= Undefined::VALUE` default is not required: -when the type includes `Undefined`, GraphQLite resolves omitted values as `Undefined::VALUE`. Adding the -default explicitly is optional and can still be useful for clarity. Constructor parameters and promoted -properties are different: if you want an omitted value there to resolve to `Undefined::VALUE`, you still -need a real PHP default such as `= Undefined::VALUE`. +An explicit `= Undefined::VALUE` default is not required: when the type includes `Undefined`, GraphQLite +resolves omitted values as `Undefined::VALUE`, including for nullable types and for properties hydrated through +a constructor parameter or setter. Adding the default explicitly is optional and can still be useful for +clarity. A different PHP default, such as `= null`, is kept as the GraphQL default instead, and a property +the constructor assigns keeps that value. A setter method carrying `#[Field]`, itself, is not called when its +field is omitted. + +GraphQL can't express "optional but not nullable", so a type like `int|Undefined` is exposed as a nullable `Int`. +An explicit `null` for it is refused with a GraphQL error naming the field or argument, before the resolver runs. +Add `null` to the PHP type (`int|null|Undefined`) when `null` is a meaningful value. For a property hydrated +through a constructor parameter or setter, that parameter's type decides. + +Since the GraphQL type can't carry this, GraphQLite says it in the description of every argument and input field +whose PHP type contains `Undefined`, as a list item after any description you wrote +(see [Metadata items](descriptions.md#metadata-items)): + +- `int|Undefined`: "May be omitted; null is not accepted." +- `int|null|Undefined`: "May be omitted; null is accepted." + +```graphql +""" +Age in years. + +- May be omitted; null is not accepted. +""" +age: Int +``` + +The item isn't added if your description already contains the sentence. It is left off when `inputType` or +`#[UseInputType]` forces a non-null GraphQL type, since that type already says the value is required. Disable it +with `$schemaFactory->setUndefinedDescriptionsEnabled(false)`, for instance to word it yourself. ## Factory diff --git a/website/docs/operation-complexity.md b/website/docs/operation-complexity.md index bf946721ee..88902c88d9 100644 --- a/website/docs/operation-complexity.md +++ b/website/docs/operation-complexity.md @@ -208,6 +208,19 @@ query { } ``` +## Cost in the schema description + +GraphQLite lists each field's cost after its description, so clients can see it through introspection: + +```graphql +""" +Fetches books. + +- Cost: complexity = 5, multipliers = [take], defaultMultiplier = 200 +""" +books(take: Int): [Book!]! +``` + ## Setup As with query depth, automatic query complexity is configured through PSR15 middleware: diff --git a/website/docs/other-frameworks.mdx b/website/docs/other-frameworks.mdx index ace517fb37..57cdb16380 100644 --- a/website/docs/other-frameworks.mdx +++ b/website/docs/other-frameworks.mdx @@ -80,6 +80,10 @@ $factory->setSchemaConfig($schemaConfig); // backwards compatibility. See [Schema descriptions](descriptions.md) for the precedence rule // and security rationale. $factory->setDocblockDescriptionsEnabled(false); +// Control whether arguments and input fields typed `T|Undefined` get a sentence appended to their description +// saying they may be omitted and whether null is accepted. Enabled by default. +// See [Optional input fields](input-types.mdx#optional-input-fields). +$factory->setUndefinedDescriptionsEnabled(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). From fec72e3ef6b5c48dba8954256f58758141b4f2ab Mon Sep 17 00:00:00 2001 From: Jacob Thomason Date: Fri, 9 Oct 2026 23:00:59 -0400 Subject: [PATCH 2/2] docs: link #838 in the CHANGELOG --- website/docs/CHANGELOG.md | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/website/docs/CHANGELOG.md b/website/docs/CHANGELOG.md index 4a8cb1ac7a..ab5c4d1502 100644 --- a/website/docs/CHANGELOG.md +++ b/website/docs/CHANGELOG.md @@ -23,12 +23,13 @@ sidebar_label: Changelog - Description metadata is rendered as a Markdown list after a blank line, so the `#[Cost]` text in introspection and printed schemas changes from `Cost: complexity = ...` on its own line to the list item `- Cost: complexity = ...`. **Migration**: update anything that parses or snapshots field descriptions. - @oojacoboo + ([#838](https://github.com/thecodingmachine/graphqlite/pull/838)) @oojacoboo - An omitted input field typed `T|null|Undefined` without a PHP default now resolves to `Undefined::VALUE` instead of `null`, and no longer prints a `= null` default in the schema. A constructor parameter or property without a PHP default gets `Undefined::VALUE` too, instead of being left unset, unless the constructor already assigned the property. **Migration**: code that treated `null` as "not sent" for such - a field should compare against `Undefined::VALUE`. @oojacoboo + a field should compare against `Undefined::VALUE`. + ([#838](https://github.com/thecodingmachine/graphqlite/pull/838)) @oojacoboo ### New Features @@ -47,19 +48,23 @@ sidebar_label: Changelog - Arguments and input fields typed `T|Undefined` now say in their GraphQL description that they may be omitted and whether null is accepted ("May be omitted; null is not accepted." or "May be omitted; null is accepted."), since GraphQL types can't express it. Disable with - `SchemaFactory::setUndefinedDescriptionsEnabled(false)`. @oojacoboo + `SchemaFactory::setUndefinedDescriptionsEnabled(false)`. + ([#838](https://github.com/thecodingmachine/graphqlite/pull/838)) @oojacoboo - Field and input field middlewares add description metadata as list items with `withAddedDescriptionItem()`. `QueryFieldDescriptor::withAddedDescriptionLines()` is deprecated in favor of - it. @oojacoboo + it. + ([#838](https://github.com/thecodingmachine/graphqlite/pull/838)) @oojacoboo ### Bug Fixes - [#819 Fix undefined array input type](https://github.com/thecodingmachine/graphqlite/pull/819) @michael-georgiadis - An explicit `null` sent for an argument or input field typed `T|Undefined` (without `null`) is now - refused with a client error naming it, instead of failing with a `TypeError`. @oojacoboo + refused with a client error naming it, instead of failing with a `TypeError`. + ([#838](https://github.com/thecodingmachine/graphqlite/pull/838)) @oojacoboo - [#793](https://github.com/thecodingmachine/graphqlite/pull/793) Argument `@param` descriptions no longer - reach the schema when `SchemaFactory::setDocblockDescriptionsEnabled(false)` is set. @oojacoboo + reach the schema when `SchemaFactory::setDocblockDescriptionsEnabled(false)` is set. + ([#838](https://github.com/thecodingmachine/graphqlite/pull/838)) @oojacoboo ## >8.0.0