Skip to content

Fix callable type mapping to be Closure mapping #753

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

Merged
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
17 changes: 11 additions & 6 deletions src/Mappers/CannotMapTypeException.php
Original file line number Diff line number Diff line change
Expand Up @@ -181,18 +181,23 @@ public static function createForNonNullReturnByTypeMapper(): self
return new self('a type mapper returned a GraphQL\Type\Definition\NonNull instance. All instances returned by type mappers should be nullable. It is the role of the NullableTypeMapperAdapter class to make a GraphQL type in a "NonNull". Note: this is an error in the TypeMapper code or in GraphQLite itself. Please check your custom type mappers or open an issue on GitHub if you don\'t have any custom type mapper.');
}

public static function createForUnexpectedCallableParameters(): self
public static function createForUnexpectedCallable(): self
{
return new self('callable() type-hint must not specify any parameters.');
return new self('callable() type-hint is not supported. Use Closure: Closure(): int');
}

public static function createForMissingCallableReturnType(): self
public static function createForUnexpectedClosureParameters(): self
{
return new self('callable() type-hint must specify its return type. For instance: callable(): int');
return new self('Closure() type-hint must not specify any parameters.');
}

public static function createForCallableAsInput(): self
public static function createForMissingClosureReturnType(): self
{
return new self('callable() type-hint can only be used as output type.');
return new self('Closure() type-hint must specify its return type. For instance: Closure(): int');
}

public static function createForClosureAsInput(): self
{
return new self('Closure() type-hint can only be used as output type.');
}
}
61 changes: 0 additions & 61 deletions src/Mappers/Root/CallableTypeMapper.php

This file was deleted.

99 changes: 99 additions & 0 deletions src/Mappers/Root/ClosureTypeMapper.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
<?php

declare(strict_types=1);

namespace TheCodingMachine\GraphQLite\Mappers\Root;

use Closure;
use GraphQL\Type\Definition\InputType;
use GraphQL\Type\Definition\NamedType;
use GraphQL\Type\Definition\OutputType;
use GraphQL\Type\Definition\Type as GraphQLType;
use phpDocumentor\Reflection\DocBlock;
use phpDocumentor\Reflection\Fqsen;
use phpDocumentor\Reflection\Type;
use phpDocumentor\Reflection\Types\Callable_;
use phpDocumentor\Reflection\Types\Compound;
use phpDocumentor\Reflection\Types\Object_;
use ReflectionMethod;
use ReflectionProperty;
use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeException;

use function count;
use function iterator_to_array;

/**
* This mapper maps callable types into their return types, so that fields can defer their execution.
*/
class ClosureTypeMapper implements RootTypeMapperInterface
{
private Object_ $closureType;

public function __construct(
private readonly RootTypeMapperInterface $next,
private readonly RootTypeMapperInterface $topRootTypeMapper,
) {
$this->closureType = new Object_(new Fqsen('\\' . Closure::class));
}

public function toGraphQLOutputType(Type $type, OutputType|null $subType, ReflectionMethod|ReflectionProperty $reflector, DocBlock $docBlockObj): OutputType&GraphQLType
{
// This check exists because any string may be a callable (referring to a global function),
// so if a string that looks like a callable is returned from a resolver, it will get wrapped
// in `Deferred`, even though it wasn't supposed to be a deferred value. This could be fixed
// by combining `QueryField`'s resolver and `CallableTypeMapper` into one place, but
// that's not currently possible with GraphQLite's design.
if ($type instanceof Callable_) {
throw CannotMapTypeException::createForUnexpectedCallable();
}

if (! $type instanceof Compound || ! $type->contains($this->closureType)) {
return $this->next->toGraphQLOutputType($type, $subType, $reflector, $docBlockObj);
}

$allTypes = iterator_to_array($type);

if (count($allTypes) !== 2) {
return $this->next->toGraphQLOutputType($type, $subType, $reflector, $docBlockObj);
}

$callableType = $this->findCallableType($allTypes);
$returnType = $callableType?->getReturnType();

if (! $returnType) {
throw CannotMapTypeException::createForMissingClosureReturnType();
}

if ($callableType->getParameters()) {
throw CannotMapTypeException::createForUnexpectedClosureParameters();
}

return $this->topRootTypeMapper->toGraphQLOutputType($returnType, null, $reflector, $docBlockObj);
}

public function toGraphQLInputType(Type $type, InputType|null $subType, string $argumentName, ReflectionMethod|ReflectionProperty $reflector, DocBlock $docBlockObj): InputType&GraphQLType
{
if (! $type instanceof Callable_) {
return $this->next->toGraphQLInputType($type, $subType, $argumentName, $reflector, $docBlockObj);
}

throw CannotMapTypeException::createForClosureAsInput();
}

public function mapNameToType(string $typeName): NamedType&GraphQLType
{
return $this->next->mapNameToType($typeName);
}

/** @param array<int, Type> $types */
private function findCallableType(array $types): Callable_|null
{
foreach ($types as $type) {
if ($type instanceof Callable_) {
return $type;
}
}

return null;
}
}
6 changes: 2 additions & 4 deletions src/QueryField.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@
use TheCodingMachine\GraphQLite\Parameters\ParameterInterface;
use TheCodingMachine\GraphQLite\Parameters\SourceParameter;

use function is_callable;

/**
* A GraphQL field that maps to a PHP method automatically.
*
Expand Down Expand Up @@ -97,8 +95,8 @@ private function resolveWithPromise(mixed $result, ResolverInterface $originalRe
{
// Shorthand for deferring field execution. This does two things:
// - removes the dependency on `GraphQL\Deferred` from user land code
// - allows inferring the type from PHPDoc (callable(): Type), unlike Deferred, which is not generic
if (is_callable($result)) {
// - allows inferring the type from PHPDoc (Closure(): Type), unlike Deferred, which is not generic
if ($result instanceof Closure) {
$result = new Deferred($result);
}

Expand Down
4 changes: 2 additions & 2 deletions src/SchemaFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
use TheCodingMachine\GraphQLite\Mappers\PorpaginasTypeMapper;
use TheCodingMachine\GraphQLite\Mappers\RecursiveTypeMapper;
use TheCodingMachine\GraphQLite\Mappers\Root\BaseTypeMapper;
use TheCodingMachine\GraphQLite\Mappers\Root\CallableTypeMapper;
use TheCodingMachine\GraphQLite\Mappers\Root\ClosureTypeMapper;
use TheCodingMachine\GraphQLite\Mappers\Root\CompoundTypeMapper;
use TheCodingMachine\GraphQLite\Mappers\Root\EnumTypeMapper;
use TheCodingMachine\GraphQLite\Mappers\Root\FinalRootTypeMapper;
Expand Down Expand Up @@ -400,7 +400,7 @@ public function createSchema(): Schema
$lastTopRootTypeMapper = new LastDelegatingTypeMapper();
$topRootTypeMapper = new NullableTypeMapperAdapter($lastTopRootTypeMapper);
$topRootTypeMapper = new VoidTypeMapper($topRootTypeMapper);
$topRootTypeMapper = new CallableTypeMapper($topRootTypeMapper, $lastTopRootTypeMapper);
$topRootTypeMapper = new ClosureTypeMapper($topRootTypeMapper, $lastTopRootTypeMapper);

$errorRootTypeMapper = new FinalRootTypeMapper($recursiveTypeMapper);
$rootTypeMapper = new BaseTypeMapper($errorRootTypeMapper, $recursiveTypeMapper, $topRootTypeMapper);
Expand Down
4 changes: 2 additions & 2 deletions tests/AbstractQueryProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
use TheCodingMachine\GraphQLite\Mappers\Parameters\ResolveInfoParameterHandler;
use TheCodingMachine\GraphQLite\Mappers\RecursiveTypeMapper;
use TheCodingMachine\GraphQLite\Mappers\Root\BaseTypeMapper;
use TheCodingMachine\GraphQLite\Mappers\Root\CallableTypeMapper;
use TheCodingMachine\GraphQLite\Mappers\Root\ClosureTypeMapper;
use TheCodingMachine\GraphQLite\Mappers\Root\CompoundTypeMapper;
use TheCodingMachine\GraphQLite\Mappers\Root\EnumTypeMapper;
use TheCodingMachine\GraphQLite\Mappers\Root\FinalRootTypeMapper;
Expand Down Expand Up @@ -360,7 +360,7 @@ protected function buildRootTypeMapper(): RootTypeMapperInterface
$lastTopRootTypeMapper = new LastDelegatingTypeMapper();
$topRootTypeMapper = new NullableTypeMapperAdapter($lastTopRootTypeMapper);
$topRootTypeMapper = new VoidTypeMapper($topRootTypeMapper);
$topRootTypeMapper = new CallableTypeMapper($topRootTypeMapper, $lastTopRootTypeMapper);
$topRootTypeMapper = new ClosureTypeMapper($topRootTypeMapper, $lastTopRootTypeMapper);

$errorRootTypeMapper = new FinalRootTypeMapper($this->getTypeMapper());
$rootTypeMapper = new BaseTypeMapper(
Expand Down
5 changes: 3 additions & 2 deletions tests/Fixtures/Integration/Models/Blog.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace TheCodingMachine\GraphQLite\Fixtures\Integration\Models;

use Closure;
use GraphQL\Deferred;
use TheCodingMachine\GraphQLite\Annotations\Field;
use TheCodingMachine\GraphQLite\Annotations\Prefetch;
Expand Down Expand Up @@ -81,9 +82,9 @@ public static function prefetchSubBlogs(iterable $blogs): array
return $subBlogs;
}

/** @return callable(): User */
/** @return Closure(): User */
#[Field]
public function author(): callable {
public function author(): Closure {
return fn () => new User('Author', 'author@graphqlite');
}
}
4 changes: 2 additions & 2 deletions tests/Integration/IntegrationTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
use TheCodingMachine\GraphQLite\Mappers\RecursiveTypeMapper;
use TheCodingMachine\GraphQLite\Mappers\RecursiveTypeMapperInterface;
use TheCodingMachine\GraphQLite\Mappers\Root\BaseTypeMapper;
use TheCodingMachine\GraphQLite\Mappers\Root\CallableTypeMapper;
use TheCodingMachine\GraphQLite\Mappers\Root\ClosureTypeMapper;
use TheCodingMachine\GraphQLite\Mappers\Root\CompoundTypeMapper;
use TheCodingMachine\GraphQLite\Mappers\Root\EnumTypeMapper;
use TheCodingMachine\GraphQLite\Mappers\Root\FinalRootTypeMapper;
Expand Down Expand Up @@ -297,7 +297,7 @@ public function createContainer(array $overloadedServices = []): ContainerInterf
);
},
RootTypeMapperInterface::class => static function (ContainerInterface $container) {
return new CallableTypeMapper(
return new ClosureTypeMapper(
new VoidTypeMapper(
new NullableTypeMapperAdapter(
$container->get('topRootTypeMapper')
Expand Down
Loading
Loading