Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use @param tags from constructor with property promotion #628

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/FieldsBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,12 @@ public function __construct(
private readonly InputFieldMiddlewareInterface $inputFieldMiddleware,
)
{
$this->typeMapper = new TypeHandler($this->argumentResolver, $this->rootTypeMapper, $this->typeResolver);
$this->typeMapper = new TypeHandler(
$this->argumentResolver,
$this->rootTypeMapper,
$this->typeResolver,
$this->cachedDocBlockFactory,
);
}

/**
Expand Down
24 changes: 24 additions & 0 deletions src/Mappers/Parameters/TypeHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use GraphQL\Type\Definition\Type as GraphQLType;
use InvalidArgumentException;
use phpDocumentor\Reflection\DocBlock;
use phpDocumentor\Reflection\DocBlock\Tags\Param;
use phpDocumentor\Reflection\DocBlock\Tags\Return_;
use phpDocumentor\Reflection\DocBlock\Tags\Var_;
use phpDocumentor\Reflection\Fqsen;
Expand Down Expand Up @@ -41,6 +42,7 @@
use TheCodingMachine\GraphQLite\Parameters\InputTypeParameter;
use TheCodingMachine\GraphQLite\Parameters\InputTypeProperty;
use TheCodingMachine\GraphQLite\Parameters\ParameterInterface;
use TheCodingMachine\GraphQLite\Reflection\CachedDocBlockFactory;
use TheCodingMachine\GraphQLite\Types\ArgumentResolver;
use TheCodingMachine\GraphQLite\Types\TypeResolver;

Expand All @@ -66,6 +68,7 @@ public function __construct(
private readonly ArgumentResolver $argumentResolver,
private readonly RootTypeMapperInterface $rootTypeMapper,
private readonly TypeResolver $typeResolver,
private readonly CachedDocBlockFactory $cachedDocBlockFactory,
)
{
$this->phpDocumentorTypeResolver = new PhpDocumentorTypeResolver();
Expand Down Expand Up @@ -124,6 +127,27 @@ private function getDocBlockPropertyType(DocBlock $docBlock, ReflectionProperty
$varTags = $docBlock->getTagsByName('var');

if (! $varTags) {
// If we don't have any @var tags, was this property promoted, and if so, do we have an
// @param tag on the constructor docblock? If so, use that for the type.
if ($refProperty->isPromoted()) {
$refConstructor = $refProperty->getDeclaringClass()->getConstructor();
if (! $refConstructor) {
return null;
}

$docBlock = $this->cachedDocBlockFactory->getDocBlock($refConstructor);
$paramTags = $docBlock->getTagsByName('param');
foreach ($paramTags as $paramTag) {
if (! $paramTag instanceof Param) {
continue;
}

if ($paramTag->getVariableName() === $refProperty->getName()) {
return $paramTag->getType();
}
}
}

return null;
}

Expand Down
11 changes: 10 additions & 1 deletion tests/AbstractQueryProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,15 @@ protected function getParameterMiddlewarePipe(): ParameterMiddlewarePipe
return $this->parameterMiddlewarePipe;
}

protected function getCachedDocBlockFactory(): CachedDocBlockFactory
{
$arrayAdapter = new ArrayAdapter();
$arrayAdapter->setLogger(new ExceptionLogger());
$psr16Cache = new Psr16Cache($arrayAdapter);

return new CachedDocBlockFactory($psr16Cache);
}

protected function buildFieldsBuilder(): FieldsBuilder
{
$arrayAdapter = new ArrayAdapter();
Expand Down Expand Up @@ -308,7 +317,7 @@ protected function buildFieldsBuilder(): FieldsBuilder
$this->getTypeMapper(),
$this->getArgumentResolver(),
$this->getTypeResolver(),
new CachedDocBlockFactory($psr16Cache),
$this->getCachedDocBlockFactory(),
new NamingStrategy(),
$this->buildRootTypeMapper(),
$parameterMiddlewarePipe,
Expand Down
43 changes: 43 additions & 0 deletions tests/FieldsBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use GraphQL\Type\Definition\StringType;
use GraphQL\Type\Definition\ObjectType;
use GraphQL\Type\Definition\UnionType;
use PhpParser\Builder\Property;
use ReflectionMethod;
use stdClass;
use Symfony\Component\Cache\Adapter\ArrayAdapter;
Expand Down Expand Up @@ -69,6 +70,8 @@
use TheCodingMachine\GraphQLite\Security\VoidAuthenticationService;
use TheCodingMachine\GraphQLite\Security\VoidAuthorizationService;
use TheCodingMachine\GraphQLite\Annotations\Query;
use TheCodingMachine\GraphQLite\Fixtures80\PropertyPromotionInputType;
use TheCodingMachine\GraphQLite\Fixtures80\PropertyPromotionInputTypeWithoutGenericDoc;
use TheCodingMachine\GraphQLite\Types\DateTimeType;
use TheCodingMachine\GraphQLite\Types\VoidType;

Expand Down Expand Up @@ -197,6 +200,46 @@ public function test($typeHintInDocBlock)
$this->assertInstanceOf(StringType::class, $query->getType()->getWrappedType());
}

/**
* Tests that the fields builder will fail when a parameter is missing it's generic docblock
* definition, when required - an array, for instance, or could be a collection (List types)
*
* @requires PHP >= 8.0
*/
public function testTypeMissingForPropertyPromotionWithoutGenericDoc(): void
{
$fieldsBuilder = $this->buildFieldsBuilder();

$this->expectException(CannotMapTypeException::class);

$fieldsBuilder->getInputFields(
PropertyPromotionInputTypeWithoutGenericDoc::class,
'PropertyPromotionInputTypeWithoutGenericDocInput',
);
}

/**
* Tests that the fields builder will properly build an input type using property promotion
* with the generic docblock defined on the constructor and not the property directly.
*
* @requires PHP >= 8.0
oojacoboo marked this conversation as resolved.
Show resolved Hide resolved
*/
public function testTypeInDocBlockWithPropertyPromotion(): void
{
$fieldsBuilder = $this->buildFieldsBuilder();

// Techncially at this point, we already know it's working, since an exception would have been
// thrown otherwise, requiring the generic type to be specified.
// @see self::testTypeMissingForPropertyPromotionWithoutGenericDoc
$inputFields = $fieldsBuilder->getInputFields(
PropertyPromotionInputType::class,
'PropertyPromotionInputTypeInput',
);

$this->assertCount(1, $inputFields);
$this->assertEquals('amounts', reset($inputFields)->name);
}

public function testQueryProviderWithFixedReturnType(): void
{
$controller = new TestController();
Expand Down
22 changes: 22 additions & 0 deletions tests/Fixtures80/PropertyPromotionInputType.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php

declare(strict_types=1);

namespace TheCodingMachine\GraphQLite\Fixtures80;

use TheCodingMachine\GraphQLite\Annotations as GraphQLite;

#[GraphQLite\Input]
class PropertyPromotionInputType
{

/**
* Constructor
*
* @param array<int> $amounts
*/
public function __construct(
#[GraphQLite\Field]
public array $amounts,
) {}
}
20 changes: 20 additions & 0 deletions tests/Fixtures80/PropertyPromotionInputTypeWithoutGenericDoc.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

declare(strict_types=1);

namespace TheCodingMachine\GraphQLite\Fixtures80;

use TheCodingMachine\GraphQLite\Annotations as GraphQLite;

#[GraphQLite\Input]
class PropertyPromotionInputTypeWithoutGenericDoc
{

/**
* We expect this to fail since the array must have a generic type `array<int>`
*/
public function __construct(
#[GraphQLite\Field]
public array $amounts,
) {}
}
54 changes: 42 additions & 12 deletions tests/Mappers/Parameters/TypeMapperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,12 @@ class TypeMapperTest extends AbstractQueryProviderTest

public function testMapScalarUnionException(): void
{
$typeMapper = new TypeHandler($this->getArgumentResolver(), $this->getRootTypeMapper(), $this->getTypeResolver());
$typeMapper = new TypeHandler(
$this->getArgumentResolver(),
$this->getRootTypeMapper(),
$this->getTypeResolver(),
$this->getCachedDocBlockFactory(),
);

$cachedDocBlockFactory = new CachedDocBlockFactory(new Psr16Cache(new ArrayAdapter()));

Expand All @@ -39,9 +44,14 @@ public function testMapScalarUnionException(): void
*/
public function testMapObjectUnionWorks(): void
{
$typeMapper = new TypeHandler($this->getArgumentResolver(), $this->getRootTypeMapper(), $this->getTypeResolver());
$cachedDocBlockFactory = $this->getCachedDocBlockFactory();

$cachedDocBlockFactory = new CachedDocBlockFactory(new Psr16Cache(new ArrayAdapter()));
$typeMapper = new TypeHandler(
$this->getArgumentResolver(),
$this->getRootTypeMapper(),
$this->getTypeResolver(),
$cachedDocBlockFactory,
);

$refMethod = new ReflectionMethod(UnionOutputType::class, 'objectUnion');
$docBlockObj = $cachedDocBlockFactory->getDocBlock($refMethod);
Expand All @@ -61,9 +71,14 @@ public function testMapObjectUnionWorks(): void
*/
public function testMapObjectNullableUnionWorks(): void
{
$typeMapper = new TypeHandler($this->getArgumentResolver(), $this->getRootTypeMapper(), $this->getTypeResolver());
$cachedDocBlockFactory = $this->getCachedDocBlockFactory();

$cachedDocBlockFactory = new CachedDocBlockFactory(new Psr16Cache(new ArrayAdapter()));
$typeMapper = new TypeHandler(
$this->getArgumentResolver(),
$this->getRootTypeMapper(),
$this->getTypeResolver(),
$cachedDocBlockFactory,
);

$refMethod = new ReflectionMethod(UnionOutputType::class, 'nullableObjectUnion');
$docBlockObj = $cachedDocBlockFactory->getDocBlock($refMethod);
Expand All @@ -82,9 +97,14 @@ public function testMapObjectNullableUnionWorks(): void

public function testHideParameter(): void
{
$typeMapper = new TypeHandler($this->getArgumentResolver(), $this->getRootTypeMapper(), $this->getTypeResolver());
$cachedDocBlockFactory = $this->getCachedDocBlockFactory();

$cachedDocBlockFactory = new CachedDocBlockFactory(new Psr16Cache(new ArrayAdapter()));
$typeMapper = new TypeHandler(
$this->getArgumentResolver(),
$this->getRootTypeMapper(),
$this->getTypeResolver(),
$cachedDocBlockFactory,
);

$refMethod = new ReflectionMethod($this, 'withDefaultValue');
$refParameter = $refMethod->getParameters()[0];
Expand All @@ -101,9 +121,14 @@ public function testHideParameter(): void

public function testParameterWithDescription(): void
{
$typeMapper = new TypeHandler($this->getArgumentResolver(), $this->getRootTypeMapper(), $this->getTypeResolver());
$cachedDocBlockFactory = $this->getCachedDocBlockFactory();

$cachedDocBlockFactory = new CachedDocBlockFactory(new Psr16Cache(new ArrayAdapter()));
$typeMapper = new TypeHandler(
$this->getArgumentResolver(),
$this->getRootTypeMapper(),
$this->getTypeResolver(),
$cachedDocBlockFactory,
);

$refMethod = new ReflectionMethod($this, 'withParamDescription');
$docBlockObj = $cachedDocBlockFactory->getDocBlock($refMethod);
Expand All @@ -117,9 +142,14 @@ public function testParameterWithDescription(): void

public function testHideParameterException(): void
{
$typeMapper = new TypeHandler($this->getArgumentResolver(), $this->getRootTypeMapper(), $this->getTypeResolver());

$cachedDocBlockFactory = new CachedDocBlockFactory(new Psr16Cache(new ArrayAdapter()));
$cachedDocBlockFactory = $this->getCachedDocBlockFactory();

$typeMapper = new TypeHandler(
$this->getArgumentResolver(),
$this->getRootTypeMapper(),
$this->getTypeResolver(),
$cachedDocBlockFactory,
);

$refMethod = new ReflectionMethod($this, 'withoutDefaultValue');
$refParameter = $refMethod->getParameters()[0];
Expand Down