From fb61586049bc3325eb85fb9f0573719aa190fb46 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Thu, 10 Nov 2022 09:40:32 +0100 Subject: [PATCH 01/12] Handle fields omitted through `@skip` or `@include` https://github.com/spawnia/sailor/issues/77 --- .../expected/Operations/SkipNonNullable.php | 48 +++++++++++++++++++ .../SkipNonNullable/SkipNonNullable.php | 47 ++++++++++++++++++ .../SkipNonNullableErrorFreeResult.php | 20 ++++++++ .../SkipNonNullable/SkipNonNullableResult.php | 43 +++++++++++++++++ examples/simple/schema.graphql | 1 + examples/simple/src/clientDirectives.graphql | 4 ++ tests/Integration/SimpleTest.php | 10 ++++ 7 files changed, 173 insertions(+) create mode 100644 examples/simple/expected/Operations/SkipNonNullable.php create mode 100644 examples/simple/expected/Operations/SkipNonNullable/SkipNonNullable.php create mode 100644 examples/simple/expected/Operations/SkipNonNullable/SkipNonNullableErrorFreeResult.php create mode 100644 examples/simple/expected/Operations/SkipNonNullable/SkipNonNullableResult.php diff --git a/examples/simple/expected/Operations/SkipNonNullable.php b/examples/simple/expected/Operations/SkipNonNullable.php new file mode 100644 index 00000000..67d73e3b --- /dev/null +++ b/examples/simple/expected/Operations/SkipNonNullable.php @@ -0,0 +1,48 @@ + + */ +class SkipNonNullable extends \Spawnia\Sailor\Operation +{ + /** + * @param bool $value + */ + public static function execute($value): SkipNonNullable\SkipNonNullableResult + { + return self::executeOperation( + $value, + ); + } + + protected static function converters(): array + { + static $converters; + + return $converters ??= [ + ['value', new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\BooleanConverter)], + ]; + } + + public static function document(): string + { + return /* @lang GraphQL */ 'query SkipNonNullable($value: Boolean!) { + __typename + nonNullable @skip(if: $value) + }'; + } + + public static function endpoint(): string + { + return 'simple'; + } + + public static function config(): string + { + return \Safe\realpath(__DIR__ . '/../../sailor.php'); + } +} diff --git a/examples/simple/expected/Operations/SkipNonNullable/SkipNonNullable.php b/examples/simple/expected/Operations/SkipNonNullable/SkipNonNullable.php new file mode 100644 index 00000000..dd46928d --- /dev/null +++ b/examples/simple/expected/Operations/SkipNonNullable/SkipNonNullable.php @@ -0,0 +1,47 @@ +nonNullable = $nonNullable; + } + $instance->__typename = 'Query'; + + return $instance; + } + + protected function converters(): array + { + static $converters; + + return $converters ??= [ + 'nonNullable' => new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\StringConverter), + '__typename' => new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\StringConverter), + ]; + } + + public static function endpoint(): string + { + return 'simple'; + } + + public static function config(): string + { + return \Safe\realpath(__DIR__ . '/../../../sailor.php'); + } +} diff --git a/examples/simple/expected/Operations/SkipNonNullable/SkipNonNullableErrorFreeResult.php b/examples/simple/expected/Operations/SkipNonNullable/SkipNonNullableErrorFreeResult.php new file mode 100644 index 00000000..7aeecc32 --- /dev/null +++ b/examples/simple/expected/Operations/SkipNonNullable/SkipNonNullableErrorFreeResult.php @@ -0,0 +1,20 @@ +data = SkipNonNullable::fromStdClass($data); + } + + /** + * Useful for instantiation of successful mocked results. + * + * @return static + */ + public static function fromData(SkipNonNullable $data): self + { + $instance = new static; + $instance->data = $data; + + return $instance; + } + + public function errorFree(): SkipNonNullableErrorFreeResult + { + return SkipNonNullableErrorFreeResult::fromResult($this); + } + + public static function endpoint(): string + { + return 'simple'; + } + + public static function config(): string + { + return \Safe\realpath(__DIR__ . '/../../../sailor.php'); + } +} diff --git a/examples/simple/schema.graphql b/examples/simple/schema.graphql index 3bc15a23..3c420a62 100644 --- a/examples/simple/schema.graphql +++ b/examples/simple/schema.graphql @@ -2,6 +2,7 @@ type Query { scalarWithArg(arg: String): ID twoArgs(first: String, second: Int): ID singleObject: SomeObject + nonNullable: String! } type SomeObject { diff --git a/examples/simple/src/clientDirectives.graphql b/examples/simple/src/clientDirectives.graphql index 00d4b21e..3e1218ef 100644 --- a/examples/simple/src/clientDirectives.graphql +++ b/examples/simple/src/clientDirectives.graphql @@ -16,3 +16,7 @@ query ClientDirectiveInlineFragmentQuery($value: Boolean!) { twoArgs } } + +query SkipNonNullable($value: Boolean!) { + nonNullable @skip(if: $value) +} diff --git a/tests/Integration/SimpleTest.php b/tests/Integration/SimpleTest.php index 0a967a13..156f6929 100644 --- a/tests/Integration/SimpleTest.php +++ b/tests/Integration/SimpleTest.php @@ -13,6 +13,7 @@ use Spawnia\Sailor\Simple\Operations\MyObjectNestedQuery\MyObjectNestedQueryResult; use Spawnia\Sailor\Simple\Operations\MyScalarQuery; use Spawnia\Sailor\Simple\Operations\MyScalarQuery\MyScalarQueryResult; +use Spawnia\Sailor\Simple\Operations\SkipNonNullable\SkipNonNullable; use Spawnia\Sailor\Tests\TestCase; final class SimpleTest extends TestCase @@ -207,4 +208,13 @@ public function testNestedObjectNull(): void self::assertNotNull($object); self::assertNull($object->nested); } + + public function testSkipNonNullable(): void + { + SkipNonNullable::fromStdClass((object) [ + 'data' => (object) [ + '__typename' => 'Query', + ], + ]); + } } From 1ed1730549f3986f5f8cc801ab89219127000cde Mon Sep 17 00:00:00 2001 From: spawnia Date: Mon, 16 Feb 2026 23:12:11 +0100 Subject: [PATCH 02/12] fix merge --- tests/Integration/SimpleTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Integration/SimpleTest.php b/tests/Integration/SimpleTest.php index fcac0fa2..5894564e 100644 --- a/tests/Integration/SimpleTest.php +++ b/tests/Integration/SimpleTest.php @@ -11,7 +11,6 @@ use Spawnia\Sailor\Response; use Spawnia\Sailor\Simple\Operations\MyObjectNestedQuery; use Spawnia\Sailor\Simple\Operations\MyScalarQuery; -use Spawnia\Sailor\Simple\Operations\MyScalarQuery\MyScalarQueryResult; use Spawnia\Sailor\Simple\Operations\SkipNonNullable\SkipNonNullable; use Spawnia\Sailor\Tests\TestCase; From 5c9a7c7ac423e7aeafa587c072becdebdfda3197 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 17 Feb 2026 10:16:58 +0000 Subject: [PATCH 03/12] Add comprehensive tests for @skip and @include directive handling Expand test coverage for fields omitted through client directives: - Fix existing test to use correct ObjectLike-level data (not Result envelope) - Add @include on non-nullable field (IncludeNonNullable query + expected code) - Test field-omitted and field-present cases for both @skip and @include - Test nullable fields with @skip/@include (ClientDirectiveQuery) - Test fragment spread and inline fragment @skip scenarios - Test via Result::fromStdClass with full response envelope - All omission tests assert accessing the missing field returns null These are intentionally failing tests that define the expected behavior for the @skip/@include implementation (see #77). https://claude.ai/code/session_0124DAusRQCLHMsFSbeR1gfT --- .../Operations/IncludeNonNullable.php | 48 ++++++ .../IncludeNonNullable/IncludeNonNullable.php | 47 +++++ .../IncludeNonNullableErrorFreeResult.php | 20 +++ .../IncludeNonNullableResult.php | 43 +++++ examples/simple/src/clientDirectives.graphql | 4 + tests/Integration/SimpleTest.php | 160 +++++++++++++++++- 6 files changed, 319 insertions(+), 3 deletions(-) create mode 100644 examples/simple/expected/Operations/IncludeNonNullable.php create mode 100644 examples/simple/expected/Operations/IncludeNonNullable/IncludeNonNullable.php create mode 100644 examples/simple/expected/Operations/IncludeNonNullable/IncludeNonNullableErrorFreeResult.php create mode 100644 examples/simple/expected/Operations/IncludeNonNullable/IncludeNonNullableResult.php diff --git a/examples/simple/expected/Operations/IncludeNonNullable.php b/examples/simple/expected/Operations/IncludeNonNullable.php new file mode 100644 index 00000000..183ba19c --- /dev/null +++ b/examples/simple/expected/Operations/IncludeNonNullable.php @@ -0,0 +1,48 @@ + + */ +class IncludeNonNullable extends \Spawnia\Sailor\Operation +{ + /** + * @param bool $value + */ + public static function execute($value): IncludeNonNullable\IncludeNonNullableResult + { + return self::executeOperation( + $value, + ); + } + + protected static function converters(): array + { + static $converters; + + return $converters ??= [ + ['value', new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\BooleanConverter)], + ]; + } + + public static function document(): string + { + return /* @lang GraphQL */ 'query IncludeNonNullable($value: Boolean!) { + __typename + nonNullable @include(if: $value) + }'; + } + + public static function endpoint(): string + { + return 'simple'; + } + + public static function config(): string + { + return \Safe\realpath(__DIR__ . '/../../sailor.php'); + } +} diff --git a/examples/simple/expected/Operations/IncludeNonNullable/IncludeNonNullable.php b/examples/simple/expected/Operations/IncludeNonNullable/IncludeNonNullable.php new file mode 100644 index 00000000..4b0b62e1 --- /dev/null +++ b/examples/simple/expected/Operations/IncludeNonNullable/IncludeNonNullable.php @@ -0,0 +1,47 @@ +nonNullable = $nonNullable; + } + $instance->__typename = 'Query'; + + return $instance; + } + + protected function converters(): array + { + static $converters; + + return $converters ??= [ + 'nonNullable' => new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\StringConverter), + '__typename' => new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\StringConverter), + ]; + } + + public static function endpoint(): string + { + return 'simple'; + } + + public static function config(): string + { + return \Safe\realpath(__DIR__ . '/../../../sailor.php'); + } +} diff --git a/examples/simple/expected/Operations/IncludeNonNullable/IncludeNonNullableErrorFreeResult.php b/examples/simple/expected/Operations/IncludeNonNullable/IncludeNonNullableErrorFreeResult.php new file mode 100644 index 00000000..624d17fb --- /dev/null +++ b/examples/simple/expected/Operations/IncludeNonNullable/IncludeNonNullableErrorFreeResult.php @@ -0,0 +1,20 @@ +data = IncludeNonNullable::fromStdClass($data); + } + + /** + * Useful for instantiation of successful mocked results. + * + * @return static + */ + public static function fromData(IncludeNonNullable $data): self + { + $instance = new static; + $instance->data = $data; + + return $instance; + } + + public function errorFree(): IncludeNonNullableErrorFreeResult + { + return IncludeNonNullableErrorFreeResult::fromResult($this); + } + + public static function endpoint(): string + { + return 'simple'; + } + + public static function config(): string + { + return \Safe\realpath(__DIR__ . '/../../../sailor.php'); + } +} diff --git a/examples/simple/src/clientDirectives.graphql b/examples/simple/src/clientDirectives.graphql index 3e1218ef..49ae0032 100644 --- a/examples/simple/src/clientDirectives.graphql +++ b/examples/simple/src/clientDirectives.graphql @@ -20,3 +20,7 @@ query ClientDirectiveInlineFragmentQuery($value: Boolean!) { query SkipNonNullable($value: Boolean!) { nonNullable @skip(if: $value) } + +query IncludeNonNullable($value: Boolean!) { + nonNullable @include(if: $value) +} diff --git a/tests/Integration/SimpleTest.php b/tests/Integration/SimpleTest.php index 5894564e..b9cae584 100644 --- a/tests/Integration/SimpleTest.php +++ b/tests/Integration/SimpleTest.php @@ -11,7 +11,11 @@ use Spawnia\Sailor\Response; use Spawnia\Sailor\Simple\Operations\MyObjectNestedQuery; use Spawnia\Sailor\Simple\Operations\MyScalarQuery; -use Spawnia\Sailor\Simple\Operations\SkipNonNullable\SkipNonNullable; +use Spawnia\Sailor\Simple\Operations\ClientDirectiveFragmentSpreadQuery; +use Spawnia\Sailor\Simple\Operations\ClientDirectiveInlineFragmentQuery; +use Spawnia\Sailor\Simple\Operations\ClientDirectiveQuery; +use Spawnia\Sailor\Simple\Operations\IncludeNonNullable; +use Spawnia\Sailor\Simple\Operations\SkipNonNullable; use Spawnia\Sailor\Tests\TestCase; final class SimpleTest extends TestCase @@ -207,12 +211,162 @@ public function testNestedObjectNull(): void self::assertNull($object->nested); } - public function testSkipNonNullable(): void + /** Server omits non-nullable field due to @skip directive. */ + public function testSkipNonNullableFieldOmitted(): void { - SkipNonNullable::fromStdClass((object) [ + $result = SkipNonNullable\SkipNonNullable::fromStdClass((object) [ + '__typename' => 'Query', + ]); + + self::assertNull($result->nonNullable); + } + + /** Server returns non-nullable field despite @skip directive (skip condition was false). */ + public function testSkipNonNullableFieldPresent(): void + { + $result = SkipNonNullable\SkipNonNullable::fromStdClass((object) [ + '__typename' => 'Query', + 'nonNullable' => 'value', + ]); + + self::assertSame('value', $result->nonNullable); + } + + /** Server omits non-nullable field due to @include directive. */ + public function testIncludeNonNullableFieldOmitted(): void + { + $result = IncludeNonNullable\IncludeNonNullable::fromStdClass((object) [ + '__typename' => 'Query', + ]); + + self::assertNull($result->nonNullable); + } + + /** Server returns non-nullable field because @include condition was true. */ + public function testIncludeNonNullableFieldPresent(): void + { + $result = IncludeNonNullable\IncludeNonNullable::fromStdClass((object) [ + '__typename' => 'Query', + 'nonNullable' => 'value', + ]); + + self::assertSame('value', $result->nonNullable); + } + + /** @skip on nullable field — field omitted from response. */ + public function testSkipNullableFieldOmitted(): void + { + $result = ClientDirectiveQuery\ClientDirectiveQuery::fromStdClass((object) [ + '__typename' => 'Query', + 'twoArgs' => 'present', + ]); + + self::assertNull($result->scalarWithArg); + self::assertSame('present', $result->twoArgs); + } + + /** @include on nullable field — field omitted from response. */ + public function testIncludeNullableFieldOmitted(): void + { + $result = ClientDirectiveQuery\ClientDirectiveQuery::fromStdClass((object) [ + '__typename' => 'Query', + 'scalarWithArg' => 'present', + ]); + + self::assertNull($result->twoArgs); + self::assertSame('present', $result->scalarWithArg); + } + + /** All directive-affected fields omitted simultaneously. */ + public function testClientDirectiveAllFieldsOmitted(): void + { + $result = ClientDirectiveQuery\ClientDirectiveQuery::fromStdClass((object) [ + '__typename' => 'Query', + ]); + + self::assertNull($result->scalarWithArg); + self::assertNull($result->twoArgs); + } + + /** All fields present despite directives (conditions evaluated to keep fields). */ + public function testClientDirectiveAllFieldsPresent(): void + { + $result = ClientDirectiveQuery\ClientDirectiveQuery::fromStdClass((object) [ + '__typename' => 'Query', + 'scalarWithArg' => 'foo', + 'twoArgs' => 'bar', + ]); + + self::assertSame('foo', $result->scalarWithArg); + self::assertSame('bar', $result->twoArgs); + } + + /** Fragment spread with @skip — fields from the fragment are omitted. */ + public function testFragmentSpreadSkipOmitsField(): void + { + $result = ClientDirectiveFragmentSpreadQuery\ClientDirectiveFragmentSpreadQuery::fromStdClass((object) [ + '__typename' => 'Query', + ]); + + self::assertNull($result->twoArgs); + } + + /** Fragment spread with @skip — field present when skip condition is false. */ + public function testFragmentSpreadSkipFieldPresent(): void + { + $result = ClientDirectiveFragmentSpreadQuery\ClientDirectiveFragmentSpreadQuery::fromStdClass((object) [ + '__typename' => 'Query', + 'twoArgs' => 'value', + ]); + + self::assertSame('value', $result->twoArgs); + } + + /** Inline fragment with @skip — fields from the fragment are omitted. */ + public function testInlineFragmentSkipOmitsField(): void + { + $result = ClientDirectiveInlineFragmentQuery\ClientDirectiveInlineFragmentQuery::fromStdClass((object) [ + '__typename' => 'Query', + ]); + + self::assertNull($result->twoArgs); + } + + /** Inline fragment with @skip — field present when skip condition is false. */ + public function testInlineFragmentSkipFieldPresent(): void + { + $result = ClientDirectiveInlineFragmentQuery\ClientDirectiveInlineFragmentQuery::fromStdClass((object) [ + '__typename' => 'Query', + 'twoArgs' => 'value', + ]); + + self::assertSame('value', $result->twoArgs); + } + + /** Via Result::fromStdClass with the full response envelope — field omitted. */ + public function testSkipNonNullableViaResult(): void + { + $result = SkipNonNullable\SkipNonNullableResult::fromStdClass((object) [ 'data' => (object) [ '__typename' => 'Query', ], ]); + + self::assertNotNull($result->data); + self::assertNull($result->data->nonNullable); + } + + /** Via Result::fromStdClass with the full response envelope — field present. */ + public function testSkipNonNullableViaResultFieldPresent(): void + { + $result = SkipNonNullable\SkipNonNullableResult::fromStdClass((object) [ + 'data' => (object) [ + '__typename' => 'Query', + 'nonNullable' => 'hello', + ], + ]); + + self::assertNotNull($result->data); + self::assertSame('hello', $result->data->nonNullable); } } From 4ead143bbda28951c8b91fdf9c359de24528f912 Mon Sep 17 00:00:00 2001 From: spawnia <12158000+spawnia@users.noreply.github.com> Date: Tue, 17 Feb 2026 10:30:14 +0000 Subject: [PATCH 04/12] Apply php-cs-fixer changes --- tests/Integration/SimpleTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Integration/SimpleTest.php b/tests/Integration/SimpleTest.php index b9cae584..ce357f49 100644 --- a/tests/Integration/SimpleTest.php +++ b/tests/Integration/SimpleTest.php @@ -9,12 +9,12 @@ use Spawnia\Sailor\Events\ReceiveResponse; use Spawnia\Sailor\Events\StartRequest; use Spawnia\Sailor\Response; -use Spawnia\Sailor\Simple\Operations\MyObjectNestedQuery; -use Spawnia\Sailor\Simple\Operations\MyScalarQuery; use Spawnia\Sailor\Simple\Operations\ClientDirectiveFragmentSpreadQuery; use Spawnia\Sailor\Simple\Operations\ClientDirectiveInlineFragmentQuery; use Spawnia\Sailor\Simple\Operations\ClientDirectiveQuery; use Spawnia\Sailor\Simple\Operations\IncludeNonNullable; +use Spawnia\Sailor\Simple\Operations\MyObjectNestedQuery; +use Spawnia\Sailor\Simple\Operations\MyScalarQuery; use Spawnia\Sailor\Simple\Operations\SkipNonNullable; use Spawnia\Sailor\Tests\TestCase; From ca424d5c7a504b9c9f6ba6827f076eb7aaa579f4 Mon Sep 17 00:00:00 2001 From: spawnia Date: Tue, 17 Feb 2026 13:05:41 +0100 Subject: [PATCH 05/12] fix codegen --- examples/simple/expected/Operations/IncludeNonNullable.php | 5 ++--- .../Operations/IncludeNonNullable/IncludeNonNullable.php | 7 +++---- .../IncludeNonNullableErrorFreeResult.php | 4 +--- .../IncludeNonNullable/IncludeNonNullableResult.php | 4 +--- examples/simple/expected/Operations/SkipNonNullable.php | 5 ++--- .../Operations/SkipNonNullable/SkipNonNullable.php | 7 +++---- .../SkipNonNullable/SkipNonNullableErrorFreeResult.php | 4 +--- .../Operations/SkipNonNullable/SkipNonNullableResult.php | 4 +--- 8 files changed, 14 insertions(+), 26 deletions(-) diff --git a/examples/simple/expected/Operations/IncludeNonNullable.php b/examples/simple/expected/Operations/IncludeNonNullable.php index 183ba19c..96e52493 100644 --- a/examples/simple/expected/Operations/IncludeNonNullable.php +++ b/examples/simple/expected/Operations/IncludeNonNullable.php @@ -1,6 +1,4 @@ -|null $converters */ static $converters; return $converters ??= [ diff --git a/examples/simple/expected/Operations/IncludeNonNullable/IncludeNonNullable.php b/examples/simple/expected/Operations/IncludeNonNullable/IncludeNonNullable.php index 4b0b62e1..c0a693e1 100644 --- a/examples/simple/expected/Operations/IncludeNonNullable/IncludeNonNullable.php +++ b/examples/simple/expected/Operations/IncludeNonNullable/IncludeNonNullable.php @@ -1,6 +1,4 @@ -nonNullable = $nonNullable; + $instance->__set('nonNullable', $nonNullable); } $instance->__typename = 'Query'; @@ -27,6 +25,7 @@ public static function make($nonNullable): self protected function converters(): array { + /** @var array|null $converters */ static $converters; return $converters ??= [ diff --git a/examples/simple/expected/Operations/IncludeNonNullable/IncludeNonNullableErrorFreeResult.php b/examples/simple/expected/Operations/IncludeNonNullable/IncludeNonNullableErrorFreeResult.php index 624d17fb..3ac88245 100644 --- a/examples/simple/expected/Operations/IncludeNonNullable/IncludeNonNullableErrorFreeResult.php +++ b/examples/simple/expected/Operations/IncludeNonNullable/IncludeNonNullableErrorFreeResult.php @@ -1,6 +1,4 @@ -|null $converters */ static $converters; return $converters ??= [ diff --git a/examples/simple/expected/Operations/SkipNonNullable/SkipNonNullable.php b/examples/simple/expected/Operations/SkipNonNullable/SkipNonNullable.php index dd46928d..00c7e12b 100644 --- a/examples/simple/expected/Operations/SkipNonNullable/SkipNonNullable.php +++ b/examples/simple/expected/Operations/SkipNonNullable/SkipNonNullable.php @@ -1,6 +1,4 @@ -nonNullable = $nonNullable; + $instance->__set('nonNullable', $nonNullable); } $instance->__typename = 'Query'; @@ -27,6 +25,7 @@ public static function make($nonNullable): self protected function converters(): array { + /** @var array|null $converters */ static $converters; return $converters ??= [ diff --git a/examples/simple/expected/Operations/SkipNonNullable/SkipNonNullableErrorFreeResult.php b/examples/simple/expected/Operations/SkipNonNullable/SkipNonNullableErrorFreeResult.php index 7aeecc32..51f1d252 100644 --- a/examples/simple/expected/Operations/SkipNonNullable/SkipNonNullableErrorFreeResult.php +++ b/examples/simple/expected/Operations/SkipNonNullable/SkipNonNullableErrorFreeResult.php @@ -1,6 +1,4 @@ - Date: Tue, 17 Feb 2026 13:21:40 +0100 Subject: [PATCH 06/12] Unwrap NonNull for @skip/@include fields and tolerate missing nullable fields In codegen, unwrap NonNull when a field has @skip or @include directives, since the server may omit such fields. At runtime, allow missing fields in fromGraphQL() when the converter is NullConverter instead of throwing. Co-Authored-By: Claude Opus 4.6 --- .../IncludeNonNullable/IncludeNonNullable.php | 13 ++++++------ .../SkipNonNullable/SkipNonNullable.php | 13 ++++++------ src/Codegen/OperationGenerator.php | 20 +++++++++++++++++++ src/ObjectLike.php | 5 +++++ 4 files changed, 39 insertions(+), 12 deletions(-) diff --git a/examples/simple/expected/Operations/IncludeNonNullable/IncludeNonNullable.php b/examples/simple/expected/Operations/IncludeNonNullable/IncludeNonNullable.php index c0a693e1..3ba8fbb4 100644 --- a/examples/simple/expected/Operations/IncludeNonNullable/IncludeNonNullable.php +++ b/examples/simple/expected/Operations/IncludeNonNullable/IncludeNonNullable.php @@ -3,22 +3,23 @@ namespace Spawnia\Sailor\Simple\Operations\IncludeNonNullable; /** - * @property string $nonNullable * @property string $__typename + * @property string|null $nonNullable */ class IncludeNonNullable extends \Spawnia\Sailor\ObjectLike { /** - * @param string $nonNullable + * @param string|null $nonNullable */ - public static function make($nonNullable): self - { + public static function make( + $nonNullable = 'Special default value that allows Sailor to differentiate between explicitly passing null and not passing a value at all.', + ): self { $instance = new self; + $instance->__typename = 'Query'; if ($nonNullable !== self::UNDEFINED) { $instance->__set('nonNullable', $nonNullable); } - $instance->__typename = 'Query'; return $instance; } @@ -29,8 +30,8 @@ protected function converters(): array static $converters; return $converters ??= [ - 'nonNullable' => new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\StringConverter), '__typename' => new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\StringConverter), + 'nonNullable' => new \Spawnia\Sailor\Convert\NullConverter(new \Spawnia\Sailor\Convert\StringConverter), ]; } diff --git a/examples/simple/expected/Operations/SkipNonNullable/SkipNonNullable.php b/examples/simple/expected/Operations/SkipNonNullable/SkipNonNullable.php index 00c7e12b..2edd31af 100644 --- a/examples/simple/expected/Operations/SkipNonNullable/SkipNonNullable.php +++ b/examples/simple/expected/Operations/SkipNonNullable/SkipNonNullable.php @@ -3,22 +3,23 @@ namespace Spawnia\Sailor\Simple\Operations\SkipNonNullable; /** - * @property string $nonNullable * @property string $__typename + * @property string|null $nonNullable */ class SkipNonNullable extends \Spawnia\Sailor\ObjectLike { /** - * @param string $nonNullable + * @param string|null $nonNullable */ - public static function make($nonNullable): self - { + public static function make( + $nonNullable = 'Special default value that allows Sailor to differentiate between explicitly passing null and not passing a value at all.', + ): self { $instance = new self; + $instance->__typename = 'Query'; if ($nonNullable !== self::UNDEFINED) { $instance->__set('nonNullable', $nonNullable); } - $instance->__typename = 'Query'; return $instance; } @@ -29,8 +30,8 @@ protected function converters(): array static $converters; return $converters ??= [ - 'nonNullable' => new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\StringConverter), '__typename' => new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\StringConverter), + 'nonNullable' => new \Spawnia\Sailor\Convert\NullConverter(new \Spawnia\Sailor\Convert\StringConverter), ]; } diff --git a/src/Codegen/OperationGenerator.php b/src/Codegen/OperationGenerator.php index c0172b3f..6411562b 100644 --- a/src/Codegen/OperationGenerator.php +++ b/src/Codegen/OperationGenerator.php @@ -12,7 +12,9 @@ use GraphQL\Language\Visitor; use GraphQL\Language\VisitorOperation; use GraphQL\Type\Definition\CompositeType; +use GraphQL\Type\Definition\Directive; use GraphQL\Type\Definition\InterfaceType; +use GraphQL\Type\Definition\NonNull; use GraphQL\Type\Definition\ObjectType; use GraphQL\Type\Definition\Type; use GraphQL\Type\Definition\UnionType; @@ -196,6 +198,12 @@ public function generate(): iterable $type = $typeInfo->getType(); assert($type !== null, 'schema is validated'); + // @skip and @include directives mean the server may omit the field, + // even if the schema type is non-null + if ($type instanceof NonNull && self::fieldHasSkipOrInclude($field)) { + $type = $type->getWrappedType(); + } + $namedType = Type::getNamedType($type); assert($namedType !== null, 'schema is validated'); // @phpstan-ignore function.alreadyNarrowedType, notIdentical.alwaysTrue (keep for safety across graphql-php versions) @@ -342,4 +350,16 @@ protected function currentNamespace(): string { return implode('\\', $this->namespaceStack); } + + protected static function fieldHasSkipOrInclude(FieldNode $field): bool + { + foreach ($field->directives as $directive) { + $name = $directive->name->value; + if ($name === Directive::SKIP_NAME || $name === Directive::INCLUDE_NAME) { + return true; + } + } + + return false; + } } diff --git a/src/ObjectLike.php b/src/ObjectLike.php index 433e9f13..5627af58 100644 --- a/src/ObjectLike.php +++ b/src/ObjectLike.php @@ -2,6 +2,7 @@ namespace Spawnia\Sailor; +use Spawnia\Sailor\Convert\NullConverter; use Spawnia\Sailor\Convert\TypeConverter; use Spawnia\Sailor\Error\InvalidDataException; @@ -91,6 +92,10 @@ public function fromGraphQL($value): self $converters = $this->converters(); foreach ($converters as $name => $converter) { if (! property_exists($value, $name)) { + if ($converter instanceof NullConverter) { + continue; // Field omitted due to @skip/@include directive + } + $endpoint = static::endpoint(); throw new InvalidDataException("{$endpoint}: Missing field {$name}."); } From f583617c9ba15d93b68922e72b12a1460f5f3257 Mon Sep 17 00:00:00 2001 From: spawnia Date: Tue, 17 Feb 2026 13:29:20 +0100 Subject: [PATCH 07/12] no redundant comments --- tests/Integration/SimpleTest.php | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/tests/Integration/SimpleTest.php b/tests/Integration/SimpleTest.php index ce357f49..48456f5c 100644 --- a/tests/Integration/SimpleTest.php +++ b/tests/Integration/SimpleTest.php @@ -211,8 +211,7 @@ public function testNestedObjectNull(): void self::assertNull($object->nested); } - /** Server omits non-nullable field due to @skip directive. */ - public function testSkipNonNullableFieldOmitted(): void + public function testSkipNonNullableFieldOmittedByServer(): void { $result = SkipNonNullable\SkipNonNullable::fromStdClass((object) [ '__typename' => 'Query', @@ -221,8 +220,7 @@ public function testSkipNonNullableFieldOmitted(): void self::assertNull($result->nonNullable); } - /** Server returns non-nullable field despite @skip directive (skip condition was false). */ - public function testSkipNonNullableFieldPresent(): void + public function testSkipNonNullableFieldPresentWhenSkipConditionFalse(): void { $result = SkipNonNullable\SkipNonNullable::fromStdClass((object) [ '__typename' => 'Query', @@ -232,8 +230,7 @@ public function testSkipNonNullableFieldPresent(): void self::assertSame('value', $result->nonNullable); } - /** Server omits non-nullable field due to @include directive. */ - public function testIncludeNonNullableFieldOmitted(): void + public function testIncludeNonNullableFieldOmittedByServer(): void { $result = IncludeNonNullable\IncludeNonNullable::fromStdClass((object) [ '__typename' => 'Query', @@ -242,8 +239,7 @@ public function testIncludeNonNullableFieldOmitted(): void self::assertNull($result->nonNullable); } - /** Server returns non-nullable field because @include condition was true. */ - public function testIncludeNonNullableFieldPresent(): void + public function testIncludeNonNullableFieldPresentWhenIncludeConditionTrue(): void { $result = IncludeNonNullable\IncludeNonNullable::fromStdClass((object) [ '__typename' => 'Query', @@ -253,7 +249,6 @@ public function testIncludeNonNullableFieldPresent(): void self::assertSame('value', $result->nonNullable); } - /** @skip on nullable field — field omitted from response. */ public function testSkipNullableFieldOmitted(): void { $result = ClientDirectiveQuery\ClientDirectiveQuery::fromStdClass((object) [ @@ -265,7 +260,6 @@ public function testSkipNullableFieldOmitted(): void self::assertSame('present', $result->twoArgs); } - /** @include on nullable field — field omitted from response. */ public function testIncludeNullableFieldOmitted(): void { $result = ClientDirectiveQuery\ClientDirectiveQuery::fromStdClass((object) [ @@ -277,7 +271,6 @@ public function testIncludeNullableFieldOmitted(): void self::assertSame('present', $result->scalarWithArg); } - /** All directive-affected fields omitted simultaneously. */ public function testClientDirectiveAllFieldsOmitted(): void { $result = ClientDirectiveQuery\ClientDirectiveQuery::fromStdClass((object) [ @@ -288,7 +281,6 @@ public function testClientDirectiveAllFieldsOmitted(): void self::assertNull($result->twoArgs); } - /** All fields present despite directives (conditions evaluated to keep fields). */ public function testClientDirectiveAllFieldsPresent(): void { $result = ClientDirectiveQuery\ClientDirectiveQuery::fromStdClass((object) [ @@ -301,7 +293,6 @@ public function testClientDirectiveAllFieldsPresent(): void self::assertSame('bar', $result->twoArgs); } - /** Fragment spread with @skip — fields from the fragment are omitted. */ public function testFragmentSpreadSkipOmitsField(): void { $result = ClientDirectiveFragmentSpreadQuery\ClientDirectiveFragmentSpreadQuery::fromStdClass((object) [ @@ -311,8 +302,7 @@ public function testFragmentSpreadSkipOmitsField(): void self::assertNull($result->twoArgs); } - /** Fragment spread with @skip — field present when skip condition is false. */ - public function testFragmentSpreadSkipFieldPresent(): void + public function testFragmentSpreadSkipFieldPresentWhenConditionFalse(): void { $result = ClientDirectiveFragmentSpreadQuery\ClientDirectiveFragmentSpreadQuery::fromStdClass((object) [ '__typename' => 'Query', @@ -322,7 +312,6 @@ public function testFragmentSpreadSkipFieldPresent(): void self::assertSame('value', $result->twoArgs); } - /** Inline fragment with @skip — fields from the fragment are omitted. */ public function testInlineFragmentSkipOmitsField(): void { $result = ClientDirectiveInlineFragmentQuery\ClientDirectiveInlineFragmentQuery::fromStdClass((object) [ @@ -332,8 +321,7 @@ public function testInlineFragmentSkipOmitsField(): void self::assertNull($result->twoArgs); } - /** Inline fragment with @skip — field present when skip condition is false. */ - public function testInlineFragmentSkipFieldPresent(): void + public function testInlineFragmentSkipFieldPresentWhenConditionFalse(): void { $result = ClientDirectiveInlineFragmentQuery\ClientDirectiveInlineFragmentQuery::fromStdClass((object) [ '__typename' => 'Query', @@ -343,8 +331,7 @@ public function testInlineFragmentSkipFieldPresent(): void self::assertSame('value', $result->twoArgs); } - /** Via Result::fromStdClass with the full response envelope — field omitted. */ - public function testSkipNonNullableViaResult(): void + public function testSkipNonNullableViaResultFieldOmitted(): void { $result = SkipNonNullable\SkipNonNullableResult::fromStdClass((object) [ 'data' => (object) [ @@ -356,7 +343,6 @@ public function testSkipNonNullableViaResult(): void self::assertNull($result->data->nonNullable); } - /** Via Result::fromStdClass with the full response envelope — field present. */ public function testSkipNonNullableViaResultFieldPresent(): void { $result = SkipNonNullable\SkipNonNullableResult::fromStdClass((object) [ From 0dc497f60daf9b95bc81ea8335bf76da7be697d2 Mon Sep 17 00:00:00 2001 From: spawnia Date: Tue, 17 Feb 2026 13:30:10 +0100 Subject: [PATCH 08/12] changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e134cbb..bf09a150 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +## v1.3.0 + +### Fixed + +- Handle fields omitted through `@skip` or `@include` https://github.com/spawnia/sailor/pull/79 + ## v1.2.1 ### Fixed From a68ffe4622bc80354e7ddafe0883d05ff83749aa Mon Sep 17 00:00:00 2001 From: spawnia Date: Tue, 17 Feb 2026 13:40:39 +0100 Subject: [PATCH 09/12] document --- README.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/README.md b/README.md index f659fe4a..20103a5c 100644 --- a/README.md +++ b/README.md @@ -239,6 +239,31 @@ the following is more efficient as it does not instantiate a new object: ->assertErrorFree(); // Throws if there are errors ``` +### Client directives + +When using GraphQL's `@skip` or `@include` directives in your operations, fields can be omitted from the server response. +Sailor marks such fields as nullable in the generated result classes, allowing you to safely handle cases where they are absent: + +```graphql +query UserProfile($skipEmail: Boolean!) { + user { + name + email @skip(if: $skipEmail) + } +} +``` + +The generated `email` field will be nullable, even if it was non-nullable in the schema. +When skipped (or not included), the property will always be `null`, but can be accessed without error. + +```php +UserProfile::execute(skipEmail: true) + ->errorFree() + ->data + ->user + ->email // null +``` + ### Queries with arguments Your generated operation classes will be annotated with the arguments your query defines. From 755413281fc8d0596db117e479776a0a7e0fff4e Mon Sep 17 00:00:00 2001 From: spawnia Date: Tue, 17 Feb 2026 14:44:50 +0100 Subject: [PATCH 10/12] autoload InlineFragments --- composer.json | 1 + 1 file changed, 1 insertion(+) diff --git a/composer.json b/composer.json index 1955df20..04765be7 100644 --- a/composer.json +++ b/composer.json @@ -64,6 +64,7 @@ "psr-4": { "Spawnia\\Sailor\\CustomTypesSrc\\": "examples/custom-types/src/", "Spawnia\\Sailor\\CustomTypes\\": "examples/custom-types/expected/", + "Spawnia\\Sailor\\InlineFragments\\": "examples/inline-fragments/expected/", "Spawnia\\Sailor\\Input\\": "examples/input/expected/", "Spawnia\\Sailor\\PhpKeywords\\": "examples/php-keywords/expected/", "Spawnia\\Sailor\\Polymorphic\\": "examples/polymorphic/expected/", From 89e865923ca6aaa58334d212e23473ba1bfbed95 Mon Sep 17 00:00:00 2001 From: spawnia Date: Tue, 17 Feb 2026 14:53:10 +0100 Subject: [PATCH 11/12] Handle @skip/@include directives on parent inline fragments Extend the @skip/@include directive handling to also check parent inline fragments. Direct children of fragments with these directives are made nullable (since the server may omit them), while nested descendants preserve their original nullability. Refactor depth tracking: use unified $selectionNestingDepth counter and $inlineFragmentsByDepth map keyed by depth, eliminating redundant resets and improving code clarity. Deduplicate directive checking with new hasSkipOrIncludeDirective() helper method. Exception: __typename field is never unwrapped, as it's always available. Fixes: https://github.com/spawnia/sailor/pull/79#discussion_r2816881129 Co-Authored-By: Claude Haiku 4.5 --- ...lineFragmentWithDirectNonNullableField.php | 53 ++++++++ ...lineFragmentWithDirectNonNullableField.php | 49 ++++++++ ...hDirectNonNullableFieldErrorFreeResult.php | 18 +++ ...agmentWithDirectNonNullableFieldResult.php | 41 +++++++ .../Search/Article.php | 47 ++++++++ .../Search/Video.php | 38 ++++++ ...lineFragmentWithNestedNonNullableField.php | 56 +++++++++ ...lineFragmentWithNestedNonNullableField.php | 50 ++++++++ ...hNestedNonNullableFieldErrorFreeResult.php | 18 +++ ...agmentWithNestedNonNullableFieldResult.php | 41 +++++++ .../Search/Article.php | 47 ++++++++ .../Search/Content/ArticleContent.php | 46 +++++++ .../Search/Video.php | 38 ++++++ .../inline-fragments/src/SearchQuery.graphql | 18 +++ src/Codegen/OperationGenerator.php | 54 ++++++++- tests/Integration/InlineFragmentsTest.php | 113 ++++++++++++++++++ 16 files changed, 723 insertions(+), 4 deletions(-) create mode 100644 examples/inline-fragments/expected/Operations/InlineFragmentWithDirectNonNullableField.php create mode 100644 examples/inline-fragments/expected/Operations/InlineFragmentWithDirectNonNullableField/InlineFragmentWithDirectNonNullableField.php create mode 100644 examples/inline-fragments/expected/Operations/InlineFragmentWithDirectNonNullableField/InlineFragmentWithDirectNonNullableFieldErrorFreeResult.php create mode 100644 examples/inline-fragments/expected/Operations/InlineFragmentWithDirectNonNullableField/InlineFragmentWithDirectNonNullableFieldResult.php create mode 100644 examples/inline-fragments/expected/Operations/InlineFragmentWithDirectNonNullableField/Search/Article.php create mode 100644 examples/inline-fragments/expected/Operations/InlineFragmentWithDirectNonNullableField/Search/Video.php create mode 100644 examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField.php create mode 100644 examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField/InlineFragmentWithNestedNonNullableField.php create mode 100644 examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField/InlineFragmentWithNestedNonNullableFieldErrorFreeResult.php create mode 100644 examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField/InlineFragmentWithNestedNonNullableFieldResult.php create mode 100644 examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField/Search/Article.php create mode 100644 examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField/Search/Content/ArticleContent.php create mode 100644 examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField/Search/Video.php create mode 100644 tests/Integration/InlineFragmentsTest.php diff --git a/examples/inline-fragments/expected/Operations/InlineFragmentWithDirectNonNullableField.php b/examples/inline-fragments/expected/Operations/InlineFragmentWithDirectNonNullableField.php new file mode 100644 index 00000000..0bfccd65 --- /dev/null +++ b/examples/inline-fragments/expected/Operations/InlineFragmentWithDirectNonNullableField.php @@ -0,0 +1,53 @@ + + */ +class InlineFragmentWithDirectNonNullableField extends \Spawnia\Sailor\Operation +{ + /** + * @param bool $skip + */ + public static function execute( + $skip, + ): InlineFragmentWithDirectNonNullableField\InlineFragmentWithDirectNonNullableFieldResult { + return self::executeOperation( + $skip, + ); + } + + protected static function converters(): array + { + /** @var array|null $converters */ + static $converters; + + return $converters ??= [ + ['skip', new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\BooleanConverter)], + ]; + } + + public static function document(): string + { + return /* @lang GraphQL */ 'query InlineFragmentWithDirectNonNullableField($skip: Boolean!) { + __typename + search(query: "test") { + __typename + ... on Article @skip(if: $skip) { + title + } + } + }'; + } + + public static function endpoint(): string + { + return 'inline-fragments'; + } + + public static function config(): string + { + return \Safe\realpath(__DIR__ . '/../../sailor.php'); + } +} diff --git a/examples/inline-fragments/expected/Operations/InlineFragmentWithDirectNonNullableField/InlineFragmentWithDirectNonNullableField.php b/examples/inline-fragments/expected/Operations/InlineFragmentWithDirectNonNullableField/InlineFragmentWithDirectNonNullableField.php new file mode 100644 index 00000000..987b55c2 --- /dev/null +++ b/examples/inline-fragments/expected/Operations/InlineFragmentWithDirectNonNullableField/InlineFragmentWithDirectNonNullableField.php @@ -0,0 +1,49 @@ + $search + * @property string $__typename + */ +class InlineFragmentWithDirectNonNullableField extends \Spawnia\Sailor\ObjectLike +{ + /** + * @param array $search + */ + public static function make($search): self + { + $instance = new self; + + if ($search !== self::UNDEFINED) { + $instance->__set('search', $search); + } + $instance->__typename = 'Query'; + + return $instance; + } + + protected function converters(): array + { + /** @var array|null $converters */ + static $converters; + + return $converters ??= [ + 'search' => new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\ListConverter(new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\PolymorphicConverter([ + 'Article' => '\\Spawnia\\Sailor\\InlineFragments\\Operations\\InlineFragmentWithDirectNonNullableField\\Search\\Article', + 'Video' => '\\Spawnia\\Sailor\\InlineFragments\\Operations\\InlineFragmentWithDirectNonNullableField\\Search\\Video', + ])))), + '__typename' => new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\StringConverter), + ]; + } + + public static function endpoint(): string + { + return 'inline-fragments'; + } + + public static function config(): string + { + return \Safe\realpath(__DIR__ . '/../../../sailor.php'); + } +} diff --git a/examples/inline-fragments/expected/Operations/InlineFragmentWithDirectNonNullableField/InlineFragmentWithDirectNonNullableFieldErrorFreeResult.php b/examples/inline-fragments/expected/Operations/InlineFragmentWithDirectNonNullableField/InlineFragmentWithDirectNonNullableFieldErrorFreeResult.php new file mode 100644 index 00000000..1e0a7175 --- /dev/null +++ b/examples/inline-fragments/expected/Operations/InlineFragmentWithDirectNonNullableField/InlineFragmentWithDirectNonNullableFieldErrorFreeResult.php @@ -0,0 +1,18 @@ +data = InlineFragmentWithDirectNonNullableField::fromStdClass($data); + } + + /** + * Useful for instantiation of successful mocked results. + * + * @return static + */ + public static function fromData(InlineFragmentWithDirectNonNullableField $data): self + { + $instance = new static; + $instance->data = $data; + + return $instance; + } + + public function errorFree(): InlineFragmentWithDirectNonNullableFieldErrorFreeResult + { + return InlineFragmentWithDirectNonNullableFieldErrorFreeResult::fromResult($this); + } + + public static function endpoint(): string + { + return 'inline-fragments'; + } + + public static function config(): string + { + return \Safe\realpath(__DIR__ . '/../../../sailor.php'); + } +} diff --git a/examples/inline-fragments/expected/Operations/InlineFragmentWithDirectNonNullableField/Search/Article.php b/examples/inline-fragments/expected/Operations/InlineFragmentWithDirectNonNullableField/Search/Article.php new file mode 100644 index 00000000..0ef21c75 --- /dev/null +++ b/examples/inline-fragments/expected/Operations/InlineFragmentWithDirectNonNullableField/Search/Article.php @@ -0,0 +1,47 @@ +__typename = 'Article'; + if ($title !== self::UNDEFINED) { + $instance->__set('title', $title); + } + + return $instance; + } + + protected function converters(): array + { + /** @var array|null $converters */ + static $converters; + + return $converters ??= [ + '__typename' => new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\StringConverter), + 'title' => new \Spawnia\Sailor\Convert\NullConverter(new \Spawnia\Sailor\Convert\StringConverter), + ]; + } + + public static function endpoint(): string + { + return 'inline-fragments'; + } + + public static function config(): string + { + return \Safe\realpath(__DIR__ . '/../../../../sailor.php'); + } +} diff --git a/examples/inline-fragments/expected/Operations/InlineFragmentWithDirectNonNullableField/Search/Video.php b/examples/inline-fragments/expected/Operations/InlineFragmentWithDirectNonNullableField/Search/Video.php new file mode 100644 index 00000000..068699a0 --- /dev/null +++ b/examples/inline-fragments/expected/Operations/InlineFragmentWithDirectNonNullableField/Search/Video.php @@ -0,0 +1,38 @@ +__typename = 'Video'; + + return $instance; + } + + protected function converters(): array + { + /** @var array|null $converters */ + static $converters; + + return $converters ??= [ + '__typename' => new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\StringConverter), + ]; + } + + public static function endpoint(): string + { + return 'inline-fragments'; + } + + public static function config(): string + { + return \Safe\realpath(__DIR__ . '/../../../../sailor.php'); + } +} diff --git a/examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField.php b/examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField.php new file mode 100644 index 00000000..f5fbab62 --- /dev/null +++ b/examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField.php @@ -0,0 +1,56 @@ + + */ +class InlineFragmentWithNestedNonNullableField extends \Spawnia\Sailor\Operation +{ + /** + * @param bool $skip + */ + public static function execute( + $skip, + ): InlineFragmentWithNestedNonNullableField\InlineFragmentWithNestedNonNullableFieldResult { + return self::executeOperation( + $skip, + ); + } + + protected static function converters(): array + { + /** @var array|null $converters */ + static $converters; + + return $converters ??= [ + ['skip', new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\BooleanConverter)], + ]; + } + + public static function document(): string + { + return /* @lang GraphQL */ 'query InlineFragmentWithNestedNonNullableField($skip: Boolean!) { + __typename + search(query: "test") { + __typename + ... on Article @skip(if: $skip) { + content { + __typename + text + } + } + } + }'; + } + + public static function endpoint(): string + { + return 'inline-fragments'; + } + + public static function config(): string + { + return \Safe\realpath(__DIR__ . '/../../sailor.php'); + } +} diff --git a/examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField/InlineFragmentWithNestedNonNullableField.php b/examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField/InlineFragmentWithNestedNonNullableField.php new file mode 100644 index 00000000..a8aaecf3 --- /dev/null +++ b/examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField/InlineFragmentWithNestedNonNullableField.php @@ -0,0 +1,50 @@ +|null $search + */ +class InlineFragmentWithNestedNonNullableField extends \Spawnia\Sailor\ObjectLike +{ + /** + * @param array|null $search + */ + public static function make( + $search = 'Special default value that allows Sailor to differentiate between explicitly passing null and not passing a value at all.', + ): self { + $instance = new self; + + $instance->__typename = 'Query'; + if ($search !== self::UNDEFINED) { + $instance->__set('search', $search); + } + + return $instance; + } + + protected function converters(): array + { + /** @var array|null $converters */ + static $converters; + + return $converters ??= [ + '__typename' => new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\StringConverter), + 'search' => new \Spawnia\Sailor\Convert\NullConverter(new \Spawnia\Sailor\Convert\ListConverter(new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\PolymorphicConverter([ + 'Article' => '\\Spawnia\\Sailor\\InlineFragments\\Operations\\InlineFragmentWithNestedNonNullableField\\Search\\Article', + 'Video' => '\\Spawnia\\Sailor\\InlineFragments\\Operations\\InlineFragmentWithNestedNonNullableField\\Search\\Video', + ])))), + ]; + } + + public static function endpoint(): string + { + return 'inline-fragments'; + } + + public static function config(): string + { + return \Safe\realpath(__DIR__ . '/../../../sailor.php'); + } +} diff --git a/examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField/InlineFragmentWithNestedNonNullableFieldErrorFreeResult.php b/examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField/InlineFragmentWithNestedNonNullableFieldErrorFreeResult.php new file mode 100644 index 00000000..cb1c5cfb --- /dev/null +++ b/examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField/InlineFragmentWithNestedNonNullableFieldErrorFreeResult.php @@ -0,0 +1,18 @@ +data = InlineFragmentWithNestedNonNullableField::fromStdClass($data); + } + + /** + * Useful for instantiation of successful mocked results. + * + * @return static + */ + public static function fromData(InlineFragmentWithNestedNonNullableField $data): self + { + $instance = new static; + $instance->data = $data; + + return $instance; + } + + public function errorFree(): InlineFragmentWithNestedNonNullableFieldErrorFreeResult + { + return InlineFragmentWithNestedNonNullableFieldErrorFreeResult::fromResult($this); + } + + public static function endpoint(): string + { + return 'inline-fragments'; + } + + public static function config(): string + { + return \Safe\realpath(__DIR__ . '/../../../sailor.php'); + } +} diff --git a/examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField/Search/Article.php b/examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField/Search/Article.php new file mode 100644 index 00000000..3de83ed7 --- /dev/null +++ b/examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField/Search/Article.php @@ -0,0 +1,47 @@ +__typename = 'Article'; + if ($content !== self::UNDEFINED) { + $instance->__set('content', $content); + } + + return $instance; + } + + protected function converters(): array + { + /** @var array|null $converters */ + static $converters; + + return $converters ??= [ + '__typename' => new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\StringConverter), + 'content' => new \Spawnia\Sailor\Convert\NullConverter(new \Spawnia\Sailor\InlineFragments\Operations\InlineFragmentWithNestedNonNullableField\Search\Content\ArticleContent), + ]; + } + + public static function endpoint(): string + { + return 'inline-fragments'; + } + + public static function config(): string + { + return \Safe\realpath(__DIR__ . '/../../../../sailor.php'); + } +} diff --git a/examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField/Search/Content/ArticleContent.php b/examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField/Search/Content/ArticleContent.php new file mode 100644 index 00000000..76d8957e --- /dev/null +++ b/examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField/Search/Content/ArticleContent.php @@ -0,0 +1,46 @@ +__set('text', $text); + } + $instance->__typename = 'ArticleContent'; + + return $instance; + } + + protected function converters(): array + { + /** @var array|null $converters */ + static $converters; + + return $converters ??= [ + 'text' => new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\StringConverter), + '__typename' => new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\StringConverter), + ]; + } + + public static function endpoint(): string + { + return 'inline-fragments'; + } + + public static function config(): string + { + return \Safe\realpath(__DIR__ . '/../../../../../sailor.php'); + } +} diff --git a/examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField/Search/Video.php b/examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField/Search/Video.php new file mode 100644 index 00000000..2dd3f87c --- /dev/null +++ b/examples/inline-fragments/expected/Operations/InlineFragmentWithNestedNonNullableField/Search/Video.php @@ -0,0 +1,38 @@ +__typename = 'Video'; + + return $instance; + } + + protected function converters(): array + { + /** @var array|null $converters */ + static $converters; + + return $converters ??= [ + '__typename' => new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\StringConverter), + ]; + } + + public static function endpoint(): string + { + return 'inline-fragments'; + } + + public static function config(): string + { + return \Safe\realpath(__DIR__ . '/../../../../sailor.php'); + } +} diff --git a/examples/inline-fragments/src/SearchQuery.graphql b/examples/inline-fragments/src/SearchQuery.graphql index 8f68c860..c1a09a5e 100644 --- a/examples/inline-fragments/src/SearchQuery.graphql +++ b/examples/inline-fragments/src/SearchQuery.graphql @@ -16,3 +16,21 @@ query SearchQuery($query: String!) { } } } + +query InlineFragmentWithDirectNonNullableField($skip: Boolean!) { + search(query: "test") { + ... on Article @skip(if: $skip) { + title + } + } +} + +query InlineFragmentWithNestedNonNullableField($skip: Boolean!) { + search(query: "test") { + ... on Article @skip(if: $skip) { + content { + text + } + } + } +} diff --git a/src/Codegen/OperationGenerator.php b/src/Codegen/OperationGenerator.php index 6411562b..2f25b4ff 100644 --- a/src/Codegen/OperationGenerator.php +++ b/src/Codegen/OperationGenerator.php @@ -2,8 +2,10 @@ namespace Spawnia\Sailor\Codegen; +use GraphQL\Language\AST\DirectiveNode; use GraphQL\Language\AST\DocumentNode; use GraphQL\Language\AST\FieldNode; +use GraphQL\Language\AST\InlineFragmentNode; use GraphQL\Language\AST\NameNode; use GraphQL\Language\AST\NodeKind; use GraphQL\Language\AST\OperationDefinitionNode; @@ -54,6 +56,12 @@ public function __construct(Schema $schema, DocumentNode $document, EndpointConf /** @var array */ protected array $types; + /** Track nesting depth within selection sets */ + protected int $selectionNestingDepth = 0; + + /** @var array Inline fragments keyed by their nesting depth */ + protected array $inlineFragmentsByDepth = []; + /** @var array */ protected array $operationStorage = []; @@ -188,6 +196,16 @@ public function generate(): iterable ); }, ], + NodeKind::INLINE_FRAGMENT => [ + 'enter' => function (InlineFragmentNode $inlineFragment): void { + $this->inlineFragmentsByDepth[$this->selectionNestingDepth] = $inlineFragment; + ++$this->selectionNestingDepth; + }, + 'leave' => function (InlineFragmentNode $_): void { + --$this->selectionNestingDepth; + unset($this->inlineFragmentsByDepth[$this->selectionNestingDepth]); + }, + ], NodeKind::FIELD => [ 'enter' => function (FieldNode $field) use ($typeInfo): ?VisitorOperation { // We are only interested in the name that will come from the server @@ -199,11 +217,20 @@ public function generate(): iterable assert($type !== null, 'schema is validated'); // @skip and @include directives mean the server may omit the field, - // even if the schema type is non-null - if ($type instanceof NonNull && self::fieldHasSkipOrInclude($field)) { + // even if the schema type is non-null. Only unwrap for direct children of fragments with directives. + // Exception: __typename is always available and non-nullable + if ($type instanceof NonNull + && ( + self::fieldHasSkipOrInclude($field) + || $this->parentInlineFragmentHasSkipOrInclude() + ) + && $fieldName !== Introspection::TYPE_NAME_FIELD_NAME + ) { $type = $type->getWrappedType(); } + ++$this->selectionNestingDepth; + $namedType = Type::getNamedType($type); assert($namedType !== null, 'schema is validated'); // @phpstan-ignore function.alreadyNarrowedType, notIdentical.alwaysTrue (keep for safety across graphql-php versions) @@ -298,6 +325,8 @@ public function generate(): iterable return null; }, 'leave' => function (FieldNode $_) use ($typeInfo): void { + --$this->selectionNestingDepth; + $type = $typeInfo->getType(); assert($type !== null, 'schema is validated'); @@ -351,9 +380,10 @@ protected function currentNamespace(): string return implode('\\', $this->namespaceStack); } - protected static function fieldHasSkipOrInclude(FieldNode $field): bool + /** @param iterable $directives */ + protected static function hasSkipOrIncludeDirective(iterable $directives): bool { - foreach ($field->directives as $directive) { + foreach ($directives as $directive) { $name = $directive->name->value; if ($name === Directive::SKIP_NAME || $name === Directive::INCLUDE_NAME) { return true; @@ -362,4 +392,20 @@ protected static function fieldHasSkipOrInclude(FieldNode $field): bool return false; } + + protected static function fieldHasSkipOrInclude(FieldNode $field): bool + { + return self::hasSkipOrIncludeDirective($field->directives); + } + + protected function parentInlineFragmentHasSkipOrInclude(): bool + { + // Check if the direct parent inline fragment (at current depth) has @skip or @include + $parentFragment = $this->inlineFragmentsByDepth[$this->selectionNestingDepth - 1] ?? null; + if ($parentFragment === null) { + return false; + } + + return self::hasSkipOrIncludeDirective($parentFragment->directives); + } } diff --git a/tests/Integration/InlineFragmentsTest.php b/tests/Integration/InlineFragmentsTest.php new file mode 100644 index 00000000..51bad5f0 --- /dev/null +++ b/tests/Integration/InlineFragmentsTest.php @@ -0,0 +1,113 @@ + (object) [ + '__typename' => 'Query', + 'search' => [ + (object) [ + '__typename' => 'Article', + ], + ], + ], + ]); + + self::assertNotNull($result->data); + $article = $result->data->search[0]; + self::assertInstanceOf(InlineFragmentWithDirectNonNullableField\Search\Article::class, $article); + self::assertNull($article->title); + } + + public function testInlineFragmentWithDirectNonNullableFieldPresent(): void + { + $result = InlineFragmentWithDirectNonNullableField\InlineFragmentWithDirectNonNullableFieldResult::fromStdClass((object) [ + 'data' => (object) [ + '__typename' => 'Query', + 'search' => [ + (object) [ + '__typename' => 'Article', + 'title' => 'Test Article', + ], + ], + ], + ]); + + self::assertNotNull($result->data); + $article = $result->data->search[0]; + self::assertInstanceOf(InlineFragmentWithDirectNonNullableField\Search\Article::class, $article); + self::assertSame('Test Article', $article->title); + } + + public function testInlineFragmentWithNestedNonNullableFieldOmitted(): void + { + $result = InlineFragmentWithNestedNonNullableField\InlineFragmentWithNestedNonNullableFieldResult::fromStdClass((object) [ + 'data' => (object) [ + '__typename' => 'Query', + 'search' => [ + (object) [ + '__typename' => 'Article', + ], + ], + ], + ]); + + self::assertNotNull($result->data); + $article = $result->data->search[0]; + self::assertInstanceOf(InlineFragmentWithNestedNonNullableField\Search\Article::class, $article); + self::assertNull($article->content); + } + + public function testInlineFragmentWithNestedNonNullableFieldPresent(): void + { + $result = InlineFragmentWithNestedNonNullableField\InlineFragmentWithNestedNonNullableFieldResult::fromStdClass((object) [ + 'data' => (object) [ + '__typename' => 'Query', + 'search' => [ + (object) [ + '__typename' => 'Article', + 'content' => (object) [ + '__typename' => 'ArticleContent', + 'text' => 'Article content', + ], + ], + ], + ], + ]); + + self::assertNotNull($result->data); + $article = $result->data->search[0]; + self::assertInstanceOf(InlineFragmentWithNestedNonNullableField\Search\Article::class, $article); + self::assertNotNull($article->content); + self::assertSame('Article content', $article->content->text); + } + + public function testInlineFragmentWithNestedNonNullableFieldMissing(): void + { + $this->expectException(InvalidDataException::class); + $this->expectExceptionMessage('Missing field text'); + + InlineFragmentWithNestedNonNullableField\InlineFragmentWithNestedNonNullableFieldResult::fromStdClass((object) [ + 'data' => (object) [ + '__typename' => 'Query', + 'search' => [ + (object) [ + '__typename' => 'Article', + 'content' => (object) [ + '__typename' => 'ArticleContent', + ], + ], + ], + ], + ]); + } +} From 43d7e5d5821e9a925935ba9b0f2cb085da87f703 Mon Sep 17 00:00:00 2001 From: spawnia Date: Tue, 17 Feb 2026 14:57:07 +0100 Subject: [PATCH 12/12] Add null checks for search property in InlineFragmentsTest PHPStan correctly identified that the search property can be null, so we need to assert it's not null before accessing array offsets. Co-Authored-By: Claude Haiku 4.5 --- tests/Integration/InlineFragmentsTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Integration/InlineFragmentsTest.php b/tests/Integration/InlineFragmentsTest.php index 51bad5f0..08d02d4b 100644 --- a/tests/Integration/InlineFragmentsTest.php +++ b/tests/Integration/InlineFragmentsTest.php @@ -62,6 +62,7 @@ public function testInlineFragmentWithNestedNonNullableFieldOmitted(): void ]); self::assertNotNull($result->data); + self::assertNotNull($result->data->search); $article = $result->data->search[0]; self::assertInstanceOf(InlineFragmentWithNestedNonNullableField\Search\Article::class, $article); self::assertNull($article->content); @@ -85,6 +86,7 @@ public function testInlineFragmentWithNestedNonNullableFieldPresent(): void ]); self::assertNotNull($result->data); + self::assertNotNull($result->data->search); $article = $result->data->search[0]; self::assertInstanceOf(InlineFragmentWithNestedNonNullableField\Search\Article::class, $article); self::assertNotNull($article->content);