Skip to content

Commit 61c85e1

Browse files
committed
Check resolver function return type integrity with field type
1 parent 7340bc9 commit 61c85e1

14 files changed

Lines changed: 341 additions & 35 deletions
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
declare(strict_types = 1);
4+
5+
namespace Graphpinator\Typesystem\Exception;
6+
7+
final class FieldResolverNotIterable extends TypeError
8+
{
9+
public const MESSAGE = 'Field %s has list type, but its resolver function return type is not iterable.';
10+
11+
public function __construct(
12+
string $fieldName,
13+
)
14+
{
15+
parent::__construct([$fieldName]);
16+
}
17+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
declare(strict_types = 1);
4+
5+
namespace Graphpinator\Typesystem\Exception;
6+
7+
final class FieldResolverNullabilityMismatch extends TypeError
8+
{
9+
public const MESSAGE = 'Field %s has non-nullable type, but its resolver function return type allows null.';
10+
11+
public function __construct(
12+
string $fieldName,
13+
)
14+
{
15+
parent::__construct([$fieldName]);
16+
}
17+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
declare(strict_types = 1);
4+
5+
namespace Graphpinator\Typesystem\Exception;
6+
7+
final class FieldResolverVoidReturnType extends TypeError
8+
{
9+
public const MESSAGE = 'Field %s resolver function cannot have void return type.';
10+
11+
public function __construct(
12+
string $fieldName,
13+
)
14+
{
15+
parent::__construct([$fieldName]);
16+
}
17+
}

src/Typesystem/Visitor/ValidateIntegrityVisitor.php

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919
use Graphpinator\Typesystem\Exception\EnumItemInvalid;
2020
use Graphpinator\Typesystem\Exception\FieldDirectiveNotCovariant;
2121
use Graphpinator\Typesystem\Exception\FieldInvalidTypeUsage;
22+
use Graphpinator\Typesystem\Exception\FieldResolverNotIterable;
23+
use Graphpinator\Typesystem\Exception\FieldResolverNullabilityMismatch;
24+
use Graphpinator\Typesystem\Exception\FieldResolverVoidReturnType;
2225
use Graphpinator\Typesystem\Exception\InputCycleDetected;
2326
use Graphpinator\Typesystem\Exception\InputTypeMustDefineOneOreMoreFields;
2427
use Graphpinator\Typesystem\Exception\InterfaceContractArgumentTypeMismatch;
@@ -34,8 +37,10 @@
3437
use Graphpinator\Typesystem\Exception\UnionTypeMustDefineOneOrMoreTypes;
3538
use Graphpinator\Typesystem\Exception\VarianceError;
3639
use Graphpinator\Typesystem\Field\Field;
40+
use Graphpinator\Typesystem\Field\ResolvableField;
3741
use Graphpinator\Typesystem\InputType;
3842
use Graphpinator\Typesystem\InterfaceType;
43+
use Graphpinator\Typesystem\ListType;
3944
use Graphpinator\Typesystem\Location\ArgumentDefinitionLocation;
4045
use Graphpinator\Typesystem\Location\EnumItemLocation;
4146
use Graphpinator\Typesystem\Location\EnumLocation;
@@ -210,6 +215,14 @@ public function visitField(Field $field) : null
210215
throw new FieldInvalidTypeUsage($field->getName(), $field->getType()->accept(new PrintNameVisitor()));
211216
}
212217

218+
foreach ($field->getArguments() as $argument) {
219+
$argument->accept($this);
220+
}
221+
222+
if ($field instanceof ResolvableField) {
223+
self::validateFieldResolverFunction($field);
224+
}
225+
213226
foreach ($field->getDirectiveUsages() as $usage) {
214227
$usage->accept($this);
215228
$directive = $usage->getDirective();
@@ -440,6 +453,7 @@ private static function compareVariance(DirectiveUsageSet $biggerSet, DirectiveU
440453

441454
foreach ($biggerSet as $usage) {
442455
$directive = $usage->getDirective();
456+
\assert($directive instanceof FieldDefinitionLocation || $directive instanceof ArgumentDefinitionLocation);
443457

444458
if ($smallerSet->offsetExists($childIndex) && $directive instanceof ($smallerSet->offsetGet($childIndex)->getDirective())) {
445459
$directive->validateVariance($usage->getArgumentValues(), $smallerSet->offsetGet($childIndex)->getArgumentValues());
@@ -533,4 +547,73 @@ private function validateContainer(Container $container) : void
533547
$directive->accept($this);
534548
}
535549
}
550+
551+
private static function validateFieldResolverFunction(ResolvableField $field) : void
552+
{
553+
$functionReturnType = (new \ReflectionFunction($field->getResolveFunction()))->getReturnType();
554+
555+
if (!$functionReturnType instanceof \ReflectionType) {
556+
return; // the return type is not present -> skip validation
557+
}
558+
559+
if ($functionReturnType instanceof \ReflectionNamedType) {
560+
if ($functionReturnType->getName() === 'void') {
561+
throw new FieldResolverVoidReturnType($field->getName());
562+
}
563+
564+
if ($functionReturnType->getName() === 'never') {
565+
return;
566+
}
567+
}
568+
569+
$fieldType = $field->getType();
570+
$isFieldNotNull = $fieldType instanceof NotNullType;
571+
572+
if ($functionReturnType->allowsNull() === $isFieldNotNull) {
573+
throw new FieldResolverNullabilityMismatch($field->getName());
574+
}
575+
576+
$shapingType = $fieldType->accept(new GetShapingTypeVisitor());
577+
578+
if ($shapingType instanceof ListType && !self::isReturnTypeIterable($functionReturnType)) {
579+
throw new FieldResolverNotIterable($field->getName());
580+
}
581+
}
582+
583+
private static function isReturnTypeIterable(\ReflectionType $type) : bool
584+
{
585+
if ($type instanceof \ReflectionNamedType) {
586+
$typeName = $type->getName();
587+
588+
return $typeName === 'array'
589+
|| $typeName === 'iterable'
590+
|| \is_a($typeName, \Traversable::class, true);
591+
}
592+
593+
if ($type instanceof \ReflectionUnionType) {
594+
foreach ($type->getTypes() as $subType) {
595+
if ($subType instanceof \ReflectionNamedType && $subType->getName() === 'null') {
596+
continue;
597+
}
598+
599+
if (!self::isReturnTypeIterable($subType)) {
600+
return false;
601+
}
602+
}
603+
604+
return true;
605+
}
606+
607+
if ($type instanceof \ReflectionIntersectionType) {
608+
foreach ($type->getTypes() as $subType) {
609+
if (self::isReturnTypeIterable($subType)) {
610+
return true;
611+
}
612+
}
613+
614+
return false;
615+
}
616+
617+
return false;
618+
}
536619
}

tests/Feature/DeprecatedDirectiveTest.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ protected function getFieldDefinition() : ResolvableFieldSet
5757
ResolvableField::create(
5858
'testFieldDeprecatedNull',
5959
Container::String(),
60-
static function () : string {
60+
static function () : ?string {
6161
return 'test';
6262
},
6363
)->setDeprecated(
@@ -69,7 +69,7 @@ static function () : string {
6969
])),
7070
ResolvableField::create(
7171
'testFieldDeprecatedNotNull',
72-
Container::String(),
72+
Container::String()->notNull(),
7373
static function () : string {
7474
return 'test';
7575
},
@@ -397,7 +397,8 @@ protected function getFieldDefinition() : ResolvableFieldSet
397397
new ResolvableField(
398398
'field',
399399
Container::String(),
400-
static function () : void {
400+
static function () : ?string {
401+
return null;
401402
},
402403
),
403404
]);

tests/Feature/InputObjectRepresentationTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ protected function getFieldDefinition() : ResolvableFieldSet
121121
return new ResolvableFieldSet([
122122
ResolvableField::create(
123123
'field1',
124-
Container::Int(),
124+
Container::Int()->notNull(),
125125
static function($parent, InputObject $arg) : int {
126126
\assert($arg->simpleInput2 instanceof InputObject2);
127127
\assert($arg->simpleInput3 instanceof \stdClass);

tests/Feature/InputTypeCoercionTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ protected function getFieldDefinition() : ResolvableFieldSet
9999
return new ResolvableFieldSet([
100100
ResolvableField::create(
101101
'field1',
102-
Container::String(),
102+
Container::String()->notNull(),
103103
static function($parent, \stdClass $arg) : string {
104104
$first = \property_exists($arg, 'string')
105105
? $arg->string

tests/Feature/MutationTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public function validateNonNullValue(mixed $rawValue) : bool
3030
protected function getFieldDefinition() : ResolvableFieldSet
3131
{
3232
return new ResolvableFieldSet([
33-
ResolvableField::create('dummy', Container::String(), \random_bytes(...)),
33+
ResolvableField::create('dummy', Container::String()->notNull(), \random_bytes(...)),
3434
]);
3535
}
3636
};

tests/Feature/NativeEnumTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ protected function getFieldDefinition() : ResolvableFieldSet
5959
return new ResolvableFieldSet([
6060
ResolvableField::create(
6161
'field',
62-
$this->enumType,
62+
$this->enumType->notNull(),
6363
static function ($parent, NativeEnum $arg) : NativeEnum {
6464
if ($arg !== NativeEnum::ABC) {
6565
throw new \RuntimeException();
@@ -75,7 +75,7 @@ static function ($parent, NativeEnum $arg) : NativeEnum {
7575
])),
7676
ResolvableField::create(
7777
'fieldDefault',
78-
$this->enumType,
78+
$this->enumType->notNull(),
7979
static function ($parent, NativeEnum $arg) : NativeEnum {
8080
if ($arg !== NativeEnum::ABC) {
8181
throw new \RuntimeException();

tests/Feature/SingleExecutionTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,14 @@ protected function getFieldDefinition() : ResolvableFieldSet
4040
return new ResolvableFieldSet([
4141
ResolvableField::create(
4242
'firstField',
43-
Container::Int(),
43+
Container::Int()->notNull(),
4444
static function($parent) : int {
4545
return 123;
4646
},
4747
),
4848
ResolvableField::create(
4949
'secondField',
50-
Container::Int(),
50+
Container::Int()->notNull(),
5151
static function($parent) : int {
5252
return 456;
5353
},

0 commit comments

Comments
 (0)