From 379c931e57057acfc8ca9788366c255b20f8713f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 28 May 2026 20:12:50 +0000 Subject: [PATCH 1/2] Emit polymorphic payloads as unions of sealed arms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Inline fragments / fragment spreads on union and interface parents now register per-variant shapes on `PayloadShape` instead of folding every variant field into one unsealed shape with `?:` markers. The payload is emitted as `array{__typename: 'X', ...} | array{__typename: 'Y', ...}` arms — one per concrete type the parent can hold, with each arm sealed. Three reinforcing changes make this work: 1. Concrete-type fragments add a variant arm; interface/union fragments distribute their fields to every concrete implementor. Abstract type names never appear as their own arms since `__typename` only holds a concrete object type at runtime. 2. Even when the query only writes a fragment for one of several schema-known variants, all concrete types are still enumerated as arms (empty bodies for the unwritten ones). This keeps PHPStan's `__typename === 'X'` check from being always-true. 3. The variant getters drop the `array_key_exists` guards — PHPStan can narrow a union of sealed arms by `__typename` literal alone, so the variant constructor's required fields are provably present. `PayloadShape::addVariant()` resolves nested variants whose names match the destination, so an `... on Employee { role }` inside an `... on Person` correctly contributes `role` to the `Developer` / `Manager` arms but not to `RegularUser`. Conditional fragments (`@include`/`@skip`) demote their contributed variant fields to optional via `withFieldsOptional()`. --- README.md | 25 ++- examples/Generated/Query/Search/Data.php | 25 ++- .../Data/SearchResultItemConnection.php | 25 ++- .../Data/SearchResultItemConnection/Node.php | 69 ++----- src/Generator/DataClassGenerator.php | 54 +---- src/Planner/PayloadShape.php | 96 ++++++++- src/Planner/PayloadShapeBuilder.php | 162 +++++++++++++-- src/Planner/SelectionSetPlanner.php | 12 +- .../Generated/Query/Test/Data.php | 10 +- .../Generated/Query/Test/Data/Viewer.php | 46 +---- tests/Fragments/Generated/Query/Test/Data.php | 10 +- .../Generated/Query/Test/Data/Viewer.php | 42 +--- .../Generated/Query/Test/Data.php | 10 +- .../Generated/Query/Test/Data/Thing.php | 46 +---- .../Generated/Mutation/Test/Data.php | 13 +- .../Mutation/Test/Data/AddCountry.php | 49 +---- .../Generated/Query/Test/Data.php | 10 +- .../Generated/Query/Test/Data/Viewer.php | 42 +--- .../Generated/Query/Test/Data.php | 12 +- .../Generated/Query/Test/Data/Viewer.php | 52 +---- tests/Planner/PayloadShapeBuilderTest.php | 189 +++++++++++++----- .../Generated/Query/Test/Data.php | 9 +- .../Generated/Query/Test/Data/Order.php | 29 +-- 23 files changed, 576 insertions(+), 461 deletions(-) diff --git a/README.md b/README.md index c35596c..f98a6e0 100644 --- a/README.md +++ b/README.md @@ -331,11 +331,26 @@ final class Data * @param array{ * 'search': array{ * 'nodes': null|list, * ..., * }, diff --git a/examples/Generated/Query/Search/Data.php b/examples/Generated/Query/Search/Data.php index fc137ef..06d85c2 100644 --- a/examples/Generated/Query/Search/Data.php +++ b/examples/Generated/Query/Search/Data.php @@ -37,11 +37,26 @@ final class Data * @param array{ * 'search': array{ * 'nodes': null|list, * ..., * }, diff --git a/examples/Generated/Query/Search/Data/SearchResultItemConnection.php b/examples/Generated/Query/Search/Data/SearchResultItemConnection.php index 744c826..aad8aaa 100644 --- a/examples/Generated/Query/Search/Data/SearchResultItemConnection.php +++ b/examples/Generated/Query/Search/Data/SearchResultItemConnection.php @@ -32,11 +32,26 @@ final class SearchResultItemConnection /** * @param array{ * 'nodes': null|list, * ..., * } $data diff --git a/examples/Generated/Query/Search/Data/SearchResultItemConnection/Node.php b/examples/Generated/Query/Search/Data/SearchResultItemConnection/Node.php index 0082cae..a0427aa 100644 --- a/examples/Generated/Query/Search/Data/SearchResultItemConnection/Node.php +++ b/examples/Generated/Query/Search/Data/SearchResultItemConnection/Node.php @@ -26,25 +26,7 @@ final class Node } public ?AsIssue $asIssue { - get { - if (isset($this->asIssue)) { - return $this->asIssue; - } - - if ($this->data['__typename'] !== 'Issue') { - return $this->asIssue = null; - } - - if (! array_key_exists('number', $this->data)) { - return $this->asIssue = null; - } - - if (! array_key_exists('title', $this->data)) { - return $this->asIssue = null; - } - - return $this->asIssue = new AsIssue($this->data); - } + get => $this->asIssue ??= $this->data['__typename'] === 'Issue' ? new AsIssue($this->data) : null; } /** @@ -56,29 +38,7 @@ final class Node } public ?PullRequestInfo $pullRequestInfo { - get { - if (isset($this->pullRequestInfo)) { - return $this->pullRequestInfo; - } - - if ($this->data['__typename'] !== 'PullRequest') { - return $this->pullRequestInfo = null; - } - - if (! array_key_exists('number', $this->data)) { - return $this->pullRequestInfo = null; - } - - if (! array_key_exists('title', $this->data)) { - return $this->pullRequestInfo = null; - } - - if (! array_key_exists('merged', $this->data)) { - return $this->pullRequestInfo = null; - } - - return $this->pullRequestInfo = new PullRequestInfo($this->data); - } + get => $this->pullRequestInfo ??= $this->data['__typename'] === 'PullRequest' ? new PullRequestInfo($this->data) : null; } /** @@ -91,11 +51,26 @@ final class Node /** * @param array{ - * '__typename': string, - * 'merged'?: bool, - * 'number'?: int, - * 'title'?: string, - * ..., + * '__typename': 'App', + * }|array{ + * '__typename': 'Discussion', + * }|array{ + * '__typename': 'Issue', + * 'number': int, + * 'title': string, + * }|array{ + * '__typename': 'MarketplaceListing', + * }|array{ + * '__typename': 'Organization', + * }|array{ + * '__typename': 'PullRequest', + * 'merged': bool, + * 'number': int, + * 'title': string, + * }|array{ + * '__typename': 'Repository', + * }|array{ + * '__typename': 'User', * } $data */ public function __construct( diff --git a/src/Generator/DataClassGenerator.php b/src/Generator/DataClassGenerator.php index 0cfca79..7dcc4e1 100644 --- a/src/Generator/DataClassGenerator.php +++ b/src/Generator/DataClassGenerator.php @@ -800,51 +800,15 @@ function () use ($plan, $parentType, $nodesType, $fields, $isData, $isMutationDa return; } - // For fragments on concrete types when parent is interface/union - // We need to check both typename and required fields - if ($requiredFields === []) { - // No required fields to check, only typename - yield sprintf( - 'get => $this->%s ??= $this->data[\'__typename\'] === %s ? %s : null;', - $fieldName, - var_export($nakedFieldType->fragmentType->name(), true), - $construct, - ); - } else { - // Generate verbose getter with field checks for PHPStan type safety - yield 'get {'; - yield $generator->indent(function () use ($fieldName, $nakedFieldType, $requiredFields, $construct) { - yield sprintf('if (isset($this->%s)) {', $fieldName); - yield ' return $this->' . $fieldName . ';'; - yield '}'; - yield ''; - yield sprintf( - 'if ($this->data[\'__typename\'] !== %s) {', - var_export($nakedFieldType->fragmentType->name(), true), - ); - yield ' return $this->' . $fieldName . ' = null;'; - yield '}'; - - // Check all required fields exist - foreach ($requiredFields as $requiredField) { - yield ''; - yield sprintf( - 'if (! array_key_exists(%s, $this->data)) {', - var_export($requiredField, true), - ); - yield ' return $this->' . $fieldName . ' = null;'; - yield '}'; - } - - yield ''; - yield sprintf( - 'return $this->%s = %s;', - $fieldName, - $construct, - ); - }); - yield '}'; - } + // For fragments on concrete types when parent is interface/union, + // the union-of-sealed-arms payload shape lets PHPStan narrow + // by `__typename` alone — no `array_key_exists` guards needed. + yield sprintf( + 'get => $this->%s ??= $this->data[\'__typename\'] === %s ? %s : null;', + $fieldName, + var_export($nakedFieldType->fragmentType->name(), true), + $construct, + ); return; } diff --git a/src/Planner/PayloadShape.php b/src/Planner/PayloadShape.php index e655844..8ed9202 100644 --- a/src/Planner/PayloadShape.php +++ b/src/Planner/PayloadShape.php @@ -4,6 +4,7 @@ namespace Ruudk\GraphQLCodeGenerator\Planner; +use Ruudk\GraphQLCodeGenerator\Type\StringLiteralType; use Symfony\Component\TypeInfo\Type as SymfonyType; /** @@ -16,6 +17,16 @@ final class PayloadShape */ private array $shape = []; + /** + * Per-variant shapes for polymorphic selections (inline fragments on + * interface/union parents). Keyed by concrete type name; the stored + * `PayloadShape` only holds fields selected inside that variant's + * fragment, not the common fields from the parent selection set. + * + * @var array + */ + private array $variants = []; + public function addRequired(string $key, SymfonyType $type) : self { $this->shape[$key] = $type; @@ -33,11 +44,47 @@ public function addOptional(string $key, SymfonyType $type) : self return $this; } + public function addVariant(string $typeName, PayloadShape $variantShape) : self + { + // Always store an independent copy. The same source shape is reused + // when distributing an abstract fragment (e.g. `... on Person`) to + // every concrete implementor, and a shared reference would let one + // implementor's later additions leak into the others. + if ( ! isset($this->variants[$typeName])) { + $this->variants[$typeName] = new PayloadShape(); + } + + $this->variants[$typeName]->merge($variantShape); + + // If the source shape itself carried a nested variant whose name + // matches the destination — e.g. distributing an `... on Person` shape + // (which has a nested `... on Employee` variant for `Developer`) to + // the `Developer` implementor — collapse those nested fields into + // this destination directly. Without this, fields from nested inline + // fragments would only live inside an unreachable second-level + // variant and never surface in the emitted arm. + if (isset($variantShape->variants[$typeName])) { + $this->variants[$typeName]->merge($variantShape->variants[$typeName]); + } + + return $this; + } + public function has(string $key) : bool { return isset($this->shape[$key]); } + public function hasVariants() : bool + { + return $this->variants !== []; + } + + public function hasVariant(string $typeName) : bool + { + return isset($this->variants[$typeName]); + } + /** * Get the type for a specific key */ @@ -123,11 +170,58 @@ public function merge(PayloadShape $other, bool $asOptional = false) : self } } + foreach ($other->variants as $typeName => $variant) { + $this->addVariant($typeName, $asOptional ? $variant->withFieldsOptional() : $variant); + } + return $this; } + /** + * Return a copy with every direct field demoted to optional. Used when a + * conditional fragment (`@include`/`@skip`) contributes variant shapes — + * the variant arm exists, but each of its fields may or may not show up. + */ + private function withFieldsOptional() : self + { + $clone = new self(); + + foreach ($this->shape as $key => $value) { + $type = is_array($value) ? $value['type'] : $value; + $clone->addOptional($key, $type); + } + + foreach ($this->variants as $typeName => $variant) { + $clone->addVariant($typeName, $variant->withFieldsOptional()); + } + + return $clone; + } + public function toArrayShape() : SymfonyType { - return SymfonyType::arrayShape($this->shape, sealed: false); + if ($this->variants === []) { + return SymfonyType::arrayShape($this->shape, sealed: false); + } + + $arms = []; + + foreach ($this->variants as $typeName => $variant) { + $combined = $this->shape; + + foreach ($variant->shape as $key => $value) { + $combined[$key] = $value; + } + + $combined['__typename'] = new StringLiteralType($typeName); + + // Arms must be sealed: PHPStan's narrowing on `__typename` literal + // only preserves required fields when each arm is sealed. Unsealed + // arms collapse to common-only after narrowing, dropping the + // variant-specific fields the variant subclass constructor needs. + $arms[] = SymfonyType::arrayShape($combined, sealed: true); + } + + return count($arms) === 1 ? $arms[0] : SymfonyType::union(...$arms); } } diff --git a/src/Planner/PayloadShapeBuilder.php b/src/Planner/PayloadShapeBuilder.php index f565442..a79d29d 100644 --- a/src/Planner/PayloadShapeBuilder.php +++ b/src/Planner/PayloadShapeBuilder.php @@ -22,6 +22,7 @@ use GraphQL\Type\Definition\UnionType; use GraphQL\Type\Definition\WrappingType; use GraphQL\Type\Schema; +use Ruudk\GraphQLCodeGenerator\GraphQL\PossibleTypesFinder; use Ruudk\GraphQLCodeGenerator\TypeMapper; use Symfony\Component\TypeInfo\Type as SymfonyType; use Webmozart\Assert\Assert; @@ -34,6 +35,8 @@ */ final readonly class PayloadShapeBuilder { + private PossibleTypesFinder $possibleTypesFinder; + /** * @param array}> $fragmentDefinitions * @param array $fragmentTypes @@ -43,7 +46,9 @@ public function __construct( private TypeMapper $typeMapper, private array $fragmentDefinitions = [], private array $fragmentTypes = [], - ) {} + ) { + $this->possibleTypesFinder = new PossibleTypesFinder($this->schema); + } /** * Build a payload shape from a selection set @@ -123,11 +128,29 @@ private function processSelections( $this->processFragmentSpread($selection, $shape, $parentType, $visitedFragments); } } + + // When the parent is polymorphic and at least one concrete-type variant + // was registered, enumerate the schema's remaining possible types as + // empty variant arms. This keeps `__typename === 'X'` from being + // `always-true` when the client only wrote a fragment for one of + // several possible types: PHPStan sees the other typenames as live + // alternatives. + if ( + $shape->hasVariants() + && ($parentType instanceof UnionType || $parentType instanceof InterfaceType) + ) { + foreach ($this->possibleTypesFinder->find($parentType) as $possibleTypeName) { + if ( ! $shape->hasVariant($possibleTypeName)) { + $shape->addVariant($possibleTypeName, new PayloadShape()); + } + } + } } /** * Collect all field selections including from same-type fragments * @param array $visitedFragments + * @throws \GraphQL\Error\InvariantViolation * @return array> */ private function collectAllFieldSelections( @@ -179,11 +202,6 @@ private function collectAllFieldSelections( $fragmentType = $this->fragmentTypes[$fragmentName]; - // Skip if different type - if ($fragmentType->name() !== $parentType->name()) { - continue; - } - // Skip if parent is union/interface (needs special processing) $isUnionOrInterface = $parentType instanceof UnionType || $parentType instanceof InterfaceType; @@ -191,6 +209,21 @@ private function collectAllFieldSelections( continue; } + $isSameType = $fragmentType->name() === $parentType->name(); + + // Collect direct fields when the spread is either same-type, or + // the concrete parent satisfies the fragment's abstract type + // (i.e. it implements the interface or is a union member). For + // implementor spreads the abstract fragment's fields are present + // unconditionally on the parent at runtime, so its direct fields + // belong in the same group. + if ( + ! $isSameType + && ! ($parentType instanceof ObjectType && $this->parentSatisfiesAbstract($parentType, $fragmentType)) + ) { + continue; + } + // Collect fields from this fragment $fragmentDef = $this->fragmentDefinitions[$fragmentName][0]; $newVisited = [ @@ -344,17 +377,71 @@ private function processInlineFragment( $fragmentType = $this->schema->getType($fragmentTypeName); Assert::isInstanceOf($fragmentType, NamedType::class); - // Create a sub-shape for this fragment + $isSameType = $fragmentType->name() === $parentType->name(); + $parentIsPolymorphic = $parentType instanceof UnionType || $parentType instanceof InterfaceType; + + // Same-type fragment on union/interface spreads into common fields. + if ($isSameType && $parentIsPolymorphic) { + $this->processSelections($fragment->selectionSet, $fragmentType, $shape, $visitedFragments); + + return; + } + + // Polymorphic parent → variant arms keyed by concrete `__typename`. + if ($parentIsPolymorphic) { + $variantShape = new PayloadShape(); + $this->processSelections($fragment->selectionSet, $fragmentType, $variantShape, $visitedFragments); + $this->distributeVariant($shape, $fragmentType, $variantShape); + + return; + } + + // Concrete parent that satisfies the fragment's type condition: process + // the inline fragment's selection set against the concrete parent so + // nested abstract spreads resolve in concrete context. + if ( + $parentType instanceof ObjectType + && $this->parentSatisfiesAbstract($parentType, $fragmentType) + ) { + $this->processSelections($fragment->selectionSet, $parentType, $shape, $visitedFragments); + + return; + } + + // Fallback (concrete parent, different fragment type): merge with optional. $fragmentShape = new PayloadShape(); $this->processSelections($fragment->selectionSet, $fragmentType, $fragmentShape, $visitedFragments); - - // Determine if fields should be optional $isOptional = $this->shouldFieldsBeOptional($parentType, $fragmentType, false); - - // Merge the fragment fields into the main shape $shape->merge($fragmentShape, $isOptional); } + /** + * Add a fragment's fields to the appropriate variant arm(s). When the + * fragment type is an interface or union, the fields apply to every + * concrete implementor of that abstract type — we add the variant to each + * one. `__typename` only ever holds a concrete object type's name at + * runtime, so abstract type names never appear as their own arms. + * + * @throws \GraphQL\Error\InvariantViolation + */ + private function distributeVariant( + PayloadShape $shape, + NamedType $fragmentType, + PayloadShape $variantShape, + ) : void { + if ($fragmentType instanceof ObjectType) { + $shape->addVariant($fragmentType->name(), $variantShape); + + return; + } + + if ($fragmentType instanceof UnionType || $fragmentType instanceof InterfaceType) { + foreach ($this->possibleTypesFinder->find($fragmentType) as $concreteTypeName) { + $shape->addVariant($concreteTypeName, $variantShape); + } + } + } + /** * Process fragment spreads (for different-type or conditional fragments) * @param array $visitedFragments @@ -381,18 +468,43 @@ private function processFragmentSpread( $fragmentName => true, ]; - // Determine if fields should be optional $hasDirective = $this->hasConditionalDirectives($spread); $isSameType = $fragmentType->name() === $parentType->name(); + $parentIsPolymorphic = $parentType instanceof UnionType || $parentType instanceof InterfaceType; // For same-type interface/union fragments without directives, // merge directly to preserve field requiredness - if ($isSameType && ! $hasDirective && ($parentType instanceof UnionType || $parentType instanceof InterfaceType)) { + if ($isSameType && ! $hasDirective && $parentIsPolymorphic) { $this->processSelections($fragmentDef->selectionSet, $fragmentType, $shape, $newVisited); return; } + // Polymorphic parent → variant arms keyed by concrete `__typename`. + if ($parentIsPolymorphic && ! $hasDirective) { + $variantShape = new PayloadShape(); + $this->processSelections($fragmentDef->selectionSet, $fragmentType, $variantShape, $newVisited); + $this->distributeVariant($shape, $fragmentType, $variantShape); + + return; + } + + // Concrete parent that satisfies the fragment's type condition (parent + // implements an interface fragment, or is a member of a union + // fragment). The fragment's fields apply unconditionally — process its + // selection set with the concrete parent so that any deeper abstract + // spreads also resolve against the same concrete type. + if ( + ! $hasDirective + && $parentType instanceof ObjectType + && ! $isSameType + && $this->parentSatisfiesAbstract($parentType, $fragmentType) + ) { + $this->processSelections($fragmentDef->selectionSet, $parentType, $shape, $newVisited); + + return; + } + // Create sub-shape and merge with optionality $fragmentShape = new PayloadShape(); $this->processSelections($fragmentDef->selectionSet, $fragmentType, $fragmentShape, $newVisited); @@ -401,6 +513,30 @@ private function processFragmentSpread( $shape->merge($fragmentShape, $isOptional); } + /** + * Is the concrete object type a runtime member of the fragment's abstract + * type? True when `$parentType` implements an interface fragment, or is + * listed in a union fragment. + * + * @throws \GraphQL\Error\InvariantViolation + */ + private function parentSatisfiesAbstract(ObjectType $parentType, NamedType $fragmentType) : bool + { + if ($fragmentType instanceof InterfaceType) { + return $parentType->implementsInterface($fragmentType); + } + + if ($fragmentType instanceof UnionType) { + foreach ($fragmentType->getTypes() as $member) { + if ($member->name === $parentType->name()) { + return true; + } + } + } + + return false; + } + private function shouldFieldsBeOptional( NamedType $parentType, NamedType $fragmentType, diff --git a/src/Planner/SelectionSetPlanner.php b/src/Planner/SelectionSetPlanner.php index ae80931..a9e6fa6 100644 --- a/src/Planner/SelectionSetPlanner.php +++ b/src/Planner/SelectionSetPlanner.php @@ -597,8 +597,9 @@ private function processInlineFragment( $pathFields->addWithPrefix($context->path, $fieldName, $accessorType); $pathFields->merge($fragmentResult->pathFields); - // Merge inline fragment fields into parent payload as optional - $this->mergeInlineFragmentPayload($fragmentResult->payloadShape, $payloadShape); + // The parent payload already includes this variant — PayloadShapeBuilder + // registers it via `PayloadShape::addVariant()` while building the parent + // shape at the top of `planNamedTypeSelectionSet()`. } /** @@ -1108,13 +1109,6 @@ private function collectRequiredFieldsFromSelectionSet( } } - private function mergeInlineFragmentPayload( - PayloadShape $fragmentPayload, - PayloadShape $parentPayload, - ) : void { - $parentPayload->merge($fragmentPayload, asOptional: true); - } - // Utility methods private function isList(Type $fieldType) : bool diff --git a/tests/ExplicitTypename/Generated/Query/Test/Data.php b/tests/ExplicitTypename/Generated/Query/Test/Data.php index 0493864..520ff84 100644 --- a/tests/ExplicitTypename/Generated/Query/Test/Data.php +++ b/tests/ExplicitTypename/Generated/Query/Test/Data.php @@ -22,11 +22,13 @@ final class Data /** * @param array{ * 'viewer': array{ - * '__typename': string, - * 'login'?: string, + * '__typename': 'Application', + * 'name': string, + * 'url': string, + * }|array{ + * '__typename': 'User', + * 'login': string, * 'name': string, - * 'url'?: string, - * ..., * }, * ..., * } $data diff --git a/tests/ExplicitTypename/Generated/Query/Test/Data/Viewer.php b/tests/ExplicitTypename/Generated/Query/Test/Data/Viewer.php index 6624c8c..5ce29d0 100644 --- a/tests/ExplicitTypename/Generated/Query/Test/Data/Viewer.php +++ b/tests/ExplicitTypename/Generated/Query/Test/Data/Viewer.php @@ -16,25 +16,7 @@ final class Viewer } public ?AsApplication $asApplication { - get { - if (isset($this->asApplication)) { - return $this->asApplication; - } - - if ($this->data['__typename'] !== 'Application') { - return $this->asApplication = null; - } - - if (! array_key_exists('url', $this->data)) { - return $this->asApplication = null; - } - - if (! array_key_exists('name', $this->data)) { - return $this->asApplication = null; - } - - return $this->asApplication = new AsApplication($this->data); - } + get => $this->asApplication ??= $this->data['__typename'] === 'Application' ? new AsApplication($this->data) : null; } /** @@ -50,21 +32,7 @@ final class Viewer } public ?UserDetails $userDetails { - get { - if (isset($this->userDetails)) { - return $this->userDetails; - } - - if ($this->data['__typename'] !== 'User') { - return $this->userDetails = null; - } - - if (! array_key_exists('login', $this->data)) { - return $this->userDetails = null; - } - - return $this->userDetails = new UserDetails($this->data); - } + get => $this->userDetails ??= $this->data['__typename'] === 'User' ? new UserDetails($this->data) : null; } /** @@ -77,11 +45,13 @@ final class Viewer /** * @param array{ - * '__typename': string, - * 'login'?: string, + * '__typename': 'Application', + * 'name': string, + * 'url': string, + * }|array{ + * '__typename': 'User', + * 'login': string, * 'name': string, - * 'url'?: string, - * ..., * } $data */ public function __construct( diff --git a/tests/Fragments/Generated/Query/Test/Data.php b/tests/Fragments/Generated/Query/Test/Data.php index 3629b25..1e2df38 100644 --- a/tests/Fragments/Generated/Query/Test/Data.php +++ b/tests/Fragments/Generated/Query/Test/Data.php @@ -36,11 +36,13 @@ final class Data * ..., * }>, * 'viewer': array{ - * '__typename': string, - * 'login'?: string, + * '__typename': 'Application', + * 'name': string, + * 'url': string, + * }|array{ + * '__typename': 'User', + * 'login': string, * 'name': string, - * 'url'?: string, - * ..., * }, * ..., * } $data diff --git a/tests/Fragments/Generated/Query/Test/Data/Viewer.php b/tests/Fragments/Generated/Query/Test/Data/Viewer.php index 9b95b95..dbc42fe 100644 --- a/tests/Fragments/Generated/Query/Test/Data/Viewer.php +++ b/tests/Fragments/Generated/Query/Test/Data/Viewer.php @@ -13,21 +13,7 @@ final class Viewer { public ?ApplicationDetails $applicationDetails { - get { - if (isset($this->applicationDetails)) { - return $this->applicationDetails; - } - - if ($this->data['__typename'] !== 'Application') { - return $this->applicationDetails = null; - } - - if (! array_key_exists('url', $this->data)) { - return $this->applicationDetails = null; - } - - return $this->applicationDetails = new ApplicationDetails($this->data); - } + get => $this->applicationDetails ??= $this->data['__typename'] === 'Application' ? new ApplicationDetails($this->data) : null; } /** @@ -39,21 +25,7 @@ final class Viewer } public ?UserDetails $userDetails { - get { - if (isset($this->userDetails)) { - return $this->userDetails; - } - - if ($this->data['__typename'] !== 'User') { - return $this->userDetails = null; - } - - if (! array_key_exists('login', $this->data)) { - return $this->userDetails = null; - } - - return $this->userDetails = new UserDetails($this->data); - } + get => $this->userDetails ??= $this->data['__typename'] === 'User' ? new UserDetails($this->data) : null; } /** @@ -78,11 +50,13 @@ final class Viewer /** * @param array{ - * '__typename': string, - * 'login'?: string, + * '__typename': 'Application', + * 'name': string, + * 'url': string, + * }|array{ + * '__typename': 'User', + * 'login': string, * 'name': string, - * 'url'?: string, - * ..., * } $data */ public function __construct( diff --git a/tests/HooksInUnionVariant/Generated/Query/Test/Data.php b/tests/HooksInUnionVariant/Generated/Query/Test/Data.php index 03de083..a3ed415 100644 --- a/tests/HooksInUnionVariant/Generated/Query/Test/Data.php +++ b/tests/HooksInUnionVariant/Generated/Query/Test/Data.php @@ -26,11 +26,13 @@ final class Data /** * @param array{ * 'things': list, * ..., * } $data diff --git a/tests/HooksInUnionVariant/Generated/Query/Test/Data/Thing.php b/tests/HooksInUnionVariant/Generated/Query/Test/Data/Thing.php index 0d3e948..1c0f6d9 100644 --- a/tests/HooksInUnionVariant/Generated/Query/Test/Data/Thing.php +++ b/tests/HooksInUnionVariant/Generated/Query/Test/Data/Thing.php @@ -17,25 +17,7 @@ final class Thing } public ?AsVariantA $asVariantA { - get { - if (isset($this->asVariantA)) { - return $this->asVariantA; - } - - if ($this->data['__typename'] !== 'VariantA') { - return $this->asVariantA = null; - } - - if (! array_key_exists('id', $this->data)) { - return $this->asVariantA = null; - } - - if (! array_key_exists('realFieldA', $this->data)) { - return $this->asVariantA = null; - } - - return $this->asVariantA = new AsVariantA($this->data, $this->hooks); - } + get => $this->asVariantA ??= $this->data['__typename'] === 'VariantA' ? new AsVariantA($this->data, $this->hooks) : null; } /** @@ -47,21 +29,7 @@ final class Thing } public ?AsVariantB $asVariantB { - get { - if (isset($this->asVariantB)) { - return $this->asVariantB; - } - - if ($this->data['__typename'] !== 'VariantB') { - return $this->asVariantB = null; - } - - if (! array_key_exists('realFieldB', $this->data)) { - return $this->asVariantB = null; - } - - return $this->asVariantB = new AsVariantB($this->data); - } + get => $this->asVariantB ??= $this->data['__typename'] === 'VariantB' ? new AsVariantB($this->data) : null; } /** @@ -78,11 +46,13 @@ final class Thing /** * @param array{ - * '__typename': string, + * '__typename': 'VariantA', * 'id': string, - * 'realFieldA'?: string, - * 'realFieldB'?: string, - * ..., + * 'realFieldA': string, + * }|array{ + * '__typename': 'VariantB', + * 'id': string, + * 'realFieldB': string, * } $data * @param array{ * 'findUserById': FindUserByIdHook, diff --git a/tests/InlineFragmentTypename/Generated/Mutation/Test/Data.php b/tests/InlineFragmentTypename/Generated/Mutation/Test/Data.php index fcdf86b..cde372e 100644 --- a/tests/InlineFragmentTypename/Generated/Mutation/Test/Data.php +++ b/tests/InlineFragmentTypename/Generated/Mutation/Test/Data.php @@ -25,11 +25,14 @@ final class Data /** * @param array{ * 'addCountry': array{ - * '__typename': string, - * 'code'?: string, - * 'id'?: string, - * 'name'?: string, - * ..., + * '__typename': 'Country', + * 'id': string, + * 'name': string, + * }|array{ + * '__typename': 'SupportedCountryError', + * }|array{ + * '__typename': 'UnsupportedCountryError', + * 'code': string, * }, * ..., * } $data diff --git a/tests/InlineFragmentTypename/Generated/Mutation/Test/Data/AddCountry.php b/tests/InlineFragmentTypename/Generated/Mutation/Test/Data/AddCountry.php index 6e6c29b..26b685a 100644 --- a/tests/InlineFragmentTypename/Generated/Mutation/Test/Data/AddCountry.php +++ b/tests/InlineFragmentTypename/Generated/Mutation/Test/Data/AddCountry.php @@ -13,25 +13,7 @@ final class AddCountry { public ?AsCountry $asCountry { - get { - if (isset($this->asCountry)) { - return $this->asCountry; - } - - if ($this->data['__typename'] !== 'Country') { - return $this->asCountry = null; - } - - if (! array_key_exists('id', $this->data)) { - return $this->asCountry = null; - } - - if (! array_key_exists('name', $this->data)) { - return $this->asCountry = null; - } - - return $this->asCountry = new AsCountry($this->data); - } + get => $this->asCountry ??= $this->data['__typename'] === 'Country' ? new AsCountry($this->data) : null; } /** @@ -55,21 +37,7 @@ final class AddCountry } public ?AsUnsupportedCountryError $asUnsupportedCountryError { - get { - if (isset($this->asUnsupportedCountryError)) { - return $this->asUnsupportedCountryError; - } - - if ($this->data['__typename'] !== 'UnsupportedCountryError') { - return $this->asUnsupportedCountryError = null; - } - - if (! array_key_exists('code', $this->data)) { - return $this->asUnsupportedCountryError = null; - } - - return $this->asUnsupportedCountryError = new AsUnsupportedCountryError($this->data); - } + get => $this->asUnsupportedCountryError ??= $this->data['__typename'] === 'UnsupportedCountryError' ? new AsUnsupportedCountryError($this->data) : null; } /** @@ -82,11 +50,14 @@ final class AddCountry /** * @param array{ - * '__typename': string, - * 'code'?: string, - * 'id'?: string, - * 'name'?: string, - * ..., + * '__typename': 'Country', + * 'id': string, + * 'name': string, + * }|array{ + * '__typename': 'SupportedCountryError', + * }|array{ + * '__typename': 'UnsupportedCountryError', + * 'code': string, * } $data */ public function __construct( diff --git a/tests/InlineFragments/Generated/Query/Test/Data.php b/tests/InlineFragments/Generated/Query/Test/Data.php index 6043419..9ffd084 100644 --- a/tests/InlineFragments/Generated/Query/Test/Data.php +++ b/tests/InlineFragments/Generated/Query/Test/Data.php @@ -36,11 +36,13 @@ final class Data * ..., * }>, * 'viewer': array{ - * '__typename': string, - * 'login'?: string, + * '__typename': 'Application', + * 'name': string, + * 'url': string, + * }|array{ + * '__typename': 'User', + * 'login': string, * 'name': string, - * 'url'?: string, - * ..., * }, * ..., * } $data diff --git a/tests/InlineFragments/Generated/Query/Test/Data/Viewer.php b/tests/InlineFragments/Generated/Query/Test/Data/Viewer.php index 7fdd077..8b04af6 100644 --- a/tests/InlineFragments/Generated/Query/Test/Data/Viewer.php +++ b/tests/InlineFragments/Generated/Query/Test/Data/Viewer.php @@ -12,21 +12,7 @@ final class Viewer { public ?AsApplication $asApplication { - get { - if (isset($this->asApplication)) { - return $this->asApplication; - } - - if ($this->data['__typename'] !== 'Application') { - return $this->asApplication = null; - } - - if (! array_key_exists('url', $this->data)) { - return $this->asApplication = null; - } - - return $this->asApplication = new AsApplication($this->data); - } + get => $this->asApplication ??= $this->data['__typename'] === 'Application' ? new AsApplication($this->data) : null; } /** @@ -38,21 +24,7 @@ final class Viewer } public ?AsUser $asUser { - get { - if (isset($this->asUser)) { - return $this->asUser; - } - - if ($this->data['__typename'] !== 'User') { - return $this->asUser = null; - } - - if (! array_key_exists('login', $this->data)) { - return $this->asUser = null; - } - - return $this->asUser = new AsUser($this->data); - } + get => $this->asUser ??= $this->data['__typename'] === 'User' ? new AsUser($this->data) : null; } /** @@ -69,11 +41,13 @@ final class Viewer /** * @param array{ - * '__typename': string, - * 'login'?: string, + * '__typename': 'Application', + * 'name': string, + * 'url': string, + * }|array{ + * '__typename': 'User', + * 'login': string, * 'name': string, - * 'url'?: string, - * ..., * } $data */ public function __construct( diff --git a/tests/Optimization/Generated/Query/Test/Data.php b/tests/Optimization/Generated/Query/Test/Data.php index ef5a19f..8de0f8e 100644 --- a/tests/Optimization/Generated/Query/Test/Data.php +++ b/tests/Optimization/Generated/Query/Test/Data.php @@ -36,13 +36,17 @@ final class Data * ..., * }>, * 'viewer': array{ - * '__typename': string, + * '__typename': 'Application', * 'id': string, * 'idAlias': string, - * 'login'?: string, * 'name': string, - * 'url'?: string, - * ..., + * 'url': string, + * }|array{ + * '__typename': 'User', + * 'id': string, + * 'idAlias': string, + * 'login': string, + * 'name': string, * }, * ..., * } $data diff --git a/tests/Optimization/Generated/Query/Test/Data/Viewer.php b/tests/Optimization/Generated/Query/Test/Data/Viewer.php index 20b164a..3d95748 100644 --- a/tests/Optimization/Generated/Query/Test/Data/Viewer.php +++ b/tests/Optimization/Generated/Query/Test/Data/Viewer.php @@ -12,25 +12,7 @@ final class Viewer { public ?AppUrl $appUrl { - get { - if (isset($this->appUrl)) { - return $this->appUrl; - } - - if ($this->data['__typename'] !== 'Application') { - return $this->appUrl = null; - } - - if (! array_key_exists('url', $this->data)) { - return $this->appUrl = null; - } - - if (! array_key_exists('name', $this->data)) { - return $this->appUrl = null; - } - - return $this->appUrl = new AppUrl($this->data); - } + get => $this->appUrl ??= $this->data['__typename'] === 'Application' ? new AppUrl($this->data) : null; } /** @@ -42,25 +24,7 @@ final class Viewer } public ?AsUser $asUser { - get { - if (isset($this->asUser)) { - return $this->asUser; - } - - if ($this->data['__typename'] !== 'User') { - return $this->asUser = null; - } - - if (! array_key_exists('login', $this->data)) { - return $this->asUser = null; - } - - if (! array_key_exists('name', $this->data)) { - return $this->asUser = null; - } - - return $this->asUser = new AsUser($this->data); - } + get => $this->asUser ??= $this->data['__typename'] === 'User' ? new AsUser($this->data) : null; } /** @@ -85,13 +49,17 @@ final class Viewer /** * @param array{ - * '__typename': string, + * '__typename': 'Application', + * 'id': string, + * 'idAlias': string, + * 'name': string, + * 'url': string, + * }|array{ + * '__typename': 'User', * 'id': string, * 'idAlias': string, - * 'login'?: string, + * 'login': string, * 'name': string, - * 'url'?: string, - * ..., * } $data */ public function __construct( diff --git a/tests/Planner/PayloadShapeBuilderTest.php b/tests/Planner/PayloadShapeBuilderTest.php index 5c6f2e5..a0b79b2 100644 --- a/tests/Planner/PayloadShapeBuilderTest.php +++ b/tests/Planner/PayloadShapeBuilderTest.php @@ -368,11 +368,15 @@ public function testInlineFragmentsOnInterface() : void self::assertSame( <<<'PHPDOC' array{ - 'address'?: null|string, - 'email'?: string, + '__typename': 'AdminViewer', + 'email': string, + 'id': string, + 'role': string, + }|array{ + '__typename': 'UserViewer', + 'address': null|string, + 'email': string, 'id': string, - 'role'?: string, - ..., } PHPDOC, TypeDumper::dump($shape->toArrayShape()), @@ -639,14 +643,18 @@ public function testUnionWithFragmentSpreads() : void <<<'PHPDOC' array{ 'search': list, - ..., + }|array{ + '__typename': 'User', }>, ..., } @@ -782,10 +790,13 @@ public function testDeeplyNestedWithLists() : void <<<'PHPDOC' array{ 'search': list, ..., }>, - ..., + }|array{ + '__typename': 'User', + 'email': null|string, + 'id': string, + 'name': string, }>, 'transaction': null|array{ 'id': string, @@ -1195,12 +1210,13 @@ public function testComplexQueryWithAllFeatures() : void array{ 'id': string, 'search': list, - ..., + }|array{ + '__typename': 'User', + 'createdAt': scalar, + 'email': null|string, + 'id': string, + 'lastSeen': null|scalar, + 'name': string, }>, 'user': null|array{ 'email': null|string, @@ -1227,11 +1249,15 @@ public function testComplexQueryWithAllFeatures() : void ..., }, 'viewer': null|array{ - 'address'?: null|string, - 'email'?: string, + '__typename': 'AdminViewer', + 'email': string, + 'id': string, + 'role': string, + }|array{ + '__typename': 'UserViewer', + 'address': null|string, + 'email': string, 'id': string, - 'role'?: string, - ..., }, ..., } @@ -1281,16 +1307,30 @@ public function testInterfaceInheritance() : void <<<'PHPDOC' array{ 'actors': list, - 'lastName'?: string, - 'role'?: string, - 'teamSize'?: int, - ..., + 'languages': list, + 'lastName': string, + 'role': string, + }|array{ + '__typename': 'Manager', + 'createdAt': scalar, + 'department': null|string, + 'firstName': string, + 'id': string, + 'lastName': string, + 'role': string, + 'teamSize': int, + }|array{ + '__typename': 'RegularUser', + 'createdAt': scalar, + 'email': string, + 'firstName': string, + 'id': string, + 'lastName': string, }>, ..., } @@ -1369,21 +1409,37 @@ public function testNestedInterfaceInheritanceWithFragments() : void ), $queryType, ); - // All fields should be optional since they're type-specific + // Each concrete type gets its own sealed arm with the fields its + // ancestry contributes — interfaces in the fragment chain widen the + // arm rather than introducing arms of their own. self::assertSame( <<<'PHPDOC' array{ 'actors': list, - 'lastName'?: string, - 'role'?: string, - 'teamSize'?: int, - ..., + '__typename': 'Developer', + 'createdAt': scalar, + 'department': null|string, + 'firstName': string, + 'id': string, + 'languages': list, + 'lastName': string, + 'role': string, + }|array{ + '__typename': 'Manager', + 'createdAt': scalar, + 'department': null|string, + 'firstName': string, + 'id': string, + 'lastName': string, + 'role': string, + 'teamSize': int, + }|array{ + '__typename': 'RegularUser', + 'createdAt': scalar, + 'email': string, + 'firstName': string, + 'id': string, + 'lastName': string, }>, ..., } @@ -1424,13 +1480,24 @@ public function testMixedInterfaceAndDirectSelection() : void <<<'PHPDOC' array{ 'actors': list, ..., } @@ -1508,23 +1575,37 @@ public function testComplexInterfaceInheritanceWithConditionalFragments() : void ), $queryType, ); - // Fields with directives should be optional - // __typename is always present + // Fields contributed by a `@include`/`@skip` fragment are optional in + // every arm that receives them; fields selected directly inside an + // unconditional inline fragment stay required for that arm. self::assertSame( <<<'PHPDOC' array{ 'actors': list, 'lastName'?: string, 'role'?: string, + }|array{ + '__typename': 'Manager', + 'createdAt'?: scalar, + 'department'?: null|string, + 'firstName'?: string, + 'id'?: string, + 'lastName'?: string, + 'role'?: string, 'teamSize'?: int, - ..., + }|array{ + '__typename': 'RegularUser', + 'createdAt'?: scalar, + 'email': string, + 'firstName'?: string, + 'id'?: string, + 'lastName'?: string, }>, ..., } diff --git a/tests/QueryObjectTypename/Generated/Query/Test/Data.php b/tests/QueryObjectTypename/Generated/Query/Test/Data.php index 80b150d..e8d7486 100644 --- a/tests/QueryObjectTypename/Generated/Query/Test/Data.php +++ b/tests/QueryObjectTypename/Generated/Query/Test/Data.php @@ -27,13 +27,14 @@ final class Data /** * @param array{ * 'order': array{ - * '__typename': string, - * 'fxFee'?: null|array{ + * '__typename': 'MarketPlaceOrderItem', + * 'fxFee': null|array{ * '__typename': string, * ..., * }, - * 'id'?: string, - * ..., + * 'id': string, + * }|array{ + * '__typename': 'OtherOrderItem', * }, * 'supportedCountry': array{ * 'error': null|array{ diff --git a/tests/QueryObjectTypename/Generated/Query/Test/Data/Order.php b/tests/QueryObjectTypename/Generated/Query/Test/Data/Order.php index 611766a..10c4f5c 100644 --- a/tests/QueryObjectTypename/Generated/Query/Test/Data/Order.php +++ b/tests/QueryObjectTypename/Generated/Query/Test/Data/Order.php @@ -11,25 +11,7 @@ final class Order { public ?AsMarketPlaceOrderItem $asMarketPlaceOrderItem { - get { - if (isset($this->asMarketPlaceOrderItem)) { - return $this->asMarketPlaceOrderItem; - } - - if ($this->data['__typename'] !== 'MarketPlaceOrderItem') { - return $this->asMarketPlaceOrderItem = null; - } - - if (! array_key_exists('id', $this->data)) { - return $this->asMarketPlaceOrderItem = null; - } - - if (! array_key_exists('fxFee', $this->data)) { - return $this->asMarketPlaceOrderItem = null; - } - - return $this->asMarketPlaceOrderItem = new AsMarketPlaceOrderItem($this->data); - } + get => $this->asMarketPlaceOrderItem ??= $this->data['__typename'] === 'MarketPlaceOrderItem' ? new AsMarketPlaceOrderItem($this->data) : null; } /** @@ -42,13 +24,14 @@ final class Order /** * @param array{ - * '__typename': string, - * 'fxFee'?: null|array{ + * '__typename': 'MarketPlaceOrderItem', + * 'fxFee': null|array{ * '__typename': string, * ..., * }, - * 'id'?: string, - * ..., + * 'id': string, + * }|array{ + * '__typename': 'OtherOrderItem', * } $data */ public function __construct( From 818e6b96c03e30b7d2baa25ba53ef5b0ab7dc170 Mon Sep 17 00:00:00 2001 From: Ruud Kamphuis Date: Wed, 3 Jun 2026 12:00:34 +0200 Subject: [PATCH 2/2] Merge spread sub-selections into polymorphic-field selections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a parent's selection set contains both a polymorphic field (with inline-fragment arms) and a fragment spread that selects the same polymorphic field at its own path, PayloadShapeBuilder correctly includes the spread-contributed fields on every arm of the parent's typed shape. The recursive class generated for the polymorphic field must mirror that shape — otherwise its sealed arms reject the wider parent value (PHPStan: "Sealed array shape does not accept array with extra key ...") and `new Sub($this->data[$field])` fails. Restricting the enrichment to interface/union-typed fields preserves fragment-field isolation for concrete-typed fields, whose open shapes accept extras without any subclass mismatch. --- src/Planner/SelectionSetPlanner.php | 165 ++++++++++++++++-- .../Generated/Fragment/ContainerBase.php | 34 ++++ .../Generated/Fragment/ContainerBase/Item.php | 24 +++ .../Generated/Query/Test/Data.php | 52 ++++++ .../Generated/Query/Test/Data/Container.php | 40 +++++ .../Query/Test/Data/Container/Item.php | 56 ++++++ .../Test/Data/Container/Item/AsVariantA.php | 25 +++ .../Test/Data/Container/Item/AsVariantB.php | 25 +++ .../Generated/Query/Test/Error.php | 24 +++ .../Generated/Query/Test/TestQuery.php | 59 +++++++ .../Schema.graphql | 22 +++ ...SpreadAddsFieldsToPolymorphicFieldTest.php | 26 +++ .../Test.graphql | 20 +++ 13 files changed, 562 insertions(+), 10 deletions(-) create mode 100644 tests/SpreadAddsFieldsToPolymorphicField/Generated/Fragment/ContainerBase.php create mode 100644 tests/SpreadAddsFieldsToPolymorphicField/Generated/Fragment/ContainerBase/Item.php create mode 100644 tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Data.php create mode 100644 tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Data/Container.php create mode 100644 tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Data/Container/Item.php create mode 100644 tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Data/Container/Item/AsVariantA.php create mode 100644 tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Data/Container/Item/AsVariantB.php create mode 100644 tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Error.php create mode 100644 tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/TestQuery.php create mode 100644 tests/SpreadAddsFieldsToPolymorphicField/Schema.graphql create mode 100644 tests/SpreadAddsFieldsToPolymorphicField/SpreadAddsFieldsToPolymorphicFieldTest.php create mode 100644 tests/SpreadAddsFieldsToPolymorphicField/Test.graphql diff --git a/src/Planner/SelectionSetPlanner.php b/src/Planner/SelectionSetPlanner.php index a9e6fa6..15a12ea 100644 --- a/src/Planner/SelectionSetPlanner.php +++ b/src/Planner/SelectionSetPlanner.php @@ -666,36 +666,58 @@ private function processFragmentSpread( } /** - * Collect and merge field selections from direct selections only - * Fragment spread fields must remain isolated and are NOT collected here + * Collect and merge field selections from direct selections, enriching the + * sub-selection sets of POLYMORPHIC fields that are ALSO selected by same- + * type fragment spreads at this level. Only directly-selected field names + * surface as keys — spread-only fields stay isolated to the fragment + * accessor — and the enrichment is restricted to interface/union-typed + * fields, where the recursive class's sealed-arm shape otherwise rejects + * the spread-contributed extras (PHPStan: "Sealed array shape does not + * accept array with extra key ..."). Concrete-typed fields keep their + * isolation because their open shapes accept extras without complaint. + * + * @throws InvariantViolation * @return array */ private function collectMergedFieldSelections(SelectionSetNode $selectionSet, NamedType & Type $type) : array { $fieldsByName = []; - // Collect direct field selections ONLY - // Fragment spread fields should NOT be collected here - they must remain isolated - // and only accessible through fragment accessors foreach ($selectionSet->selections as $selection) { if ($selection instanceof FieldNode) { $fieldName = $selection->alias->value ?? $selection->name->value; + $fieldsByName[$fieldName] ??= []; + $fieldsByName[$fieldName][] = $selection; + } + } + + if ($type instanceof HasFieldsType) { + $spreadContributions = null; - if ( ! isset($fieldsByName[$fieldName])) { - $fieldsByName[$fieldName] = []; + foreach ($fieldsByName as $fieldName => &$selections) { + if ($fieldName === '__typename') { + continue; } - $fieldsByName[$fieldName][] = $selection; + if ( ! $this->fieldTypeIsPolymorphic($type, $fieldName)) { + continue; + } + + $spreadContributions ??= $this->collectSpreadContributedFieldSelections($selectionSet, $type, []); + + if (isset($spreadContributions[$fieldName])) { + $selections = [...$selections, ...$spreadContributions[$fieldName]]; + } } + + unset($selections); } - // Merge selections for fields that appear multiple times $mergedFields = []; foreach ($fieldsByName as $fieldName => $selections) { if (count($selections) === 1) { $mergedFields[$fieldName] = $selections[0]; } else { - // Merge the selection sets of fields that appear multiple times $mergedFields[$fieldName] = $this->mergeFieldNodes($selections); } } @@ -703,6 +725,129 @@ private function collectMergedFieldSelections(SelectionSetNode $selectionSet, Na return $mergedFields; } + private function fieldTypeIsPolymorphic(HasFieldsType $parent, string $fieldName) : bool + { + if ($fieldName === '__typename' || ! $parent->hasField($fieldName)) { + return false; + } + + $fieldType = $parent->getField($fieldName)->getType(); + + while ($fieldType instanceof WrappingType) { + $fieldType = $fieldType->getWrappedType(); + } + + return $fieldType instanceof InterfaceType || $fieldType instanceof UnionType; + } + + /** + * Walk unconditional same-type (or implementor) fragment spreads at this + * level and collect their direct field selections by name. Used only to + * enrich `collectMergedFieldSelections`' sub-selection merges — never to + * surface new public properties. + * + * @param array $visitedFragments + * @throws InvariantViolation + * @return array> + */ + private function collectSpreadContributedFieldSelections( + SelectionSetNode $selectionSet, + NamedType & Type $parent, + array $visitedFragments, + ) : array { + $fieldsByName = []; + + foreach ($selectionSet->selections as $selection) { + if ( ! $selection instanceof FragmentSpreadNode) { + continue; + } + + $fragmentName = $selection->name->value; + + if (isset($visitedFragments[$fragmentName])) { + continue; + } + + if ( ! isset($this->fragmentDefinitions[$fragmentName])) { + continue; + } + + if ($this->spreadHasConditionalDirective($selection)) { + continue; + } + + $fragmentType = $this->fragmentTypes[$fragmentName]; + $isSameType = $fragmentType->name() === $parent->name(); + $satisfies = ! $isSameType + && $parent instanceof ObjectType + && $this->parentSatisfiesAbstract($parent, $fragmentType); + + if ( ! $isSameType && ! $satisfies) { + continue; + } + + $fragmentDef = $this->fragmentDefinitions[$fragmentName][0]; + $newVisited = [ + ...$visitedFragments, + $fragmentName => true, + ]; + + foreach ($fragmentDef->selectionSet->selections as $sub) { + if ($sub instanceof FieldNode) { + $name = $sub->alias->value ?? $sub->name->value; + $fieldsByName[$name] ??= []; + $fieldsByName[$name][] = $sub; + } + } + + $nested = $this->collectSpreadContributedFieldSelections( + $fragmentDef->selectionSet, + $parent, + $newVisited, + ); + + foreach ($nested as $name => $selections) { + $fieldsByName[$name] ??= []; + $fieldsByName[$name] = [...$fieldsByName[$name], ...$selections]; + } + } + + return $fieldsByName; + } + + private function spreadHasConditionalDirective(FragmentSpreadNode $spread) : bool + { + foreach ($spread->directives as $directive) { + $name = $directive->name->value; + + if ($name === 'include' || $name === 'skip') { + return true; + } + } + + return false; + } + + /** + * @throws InvariantViolation + */ + private function parentSatisfiesAbstract(ObjectType $parent, NamedType $fragmentType) : bool + { + if ($fragmentType instanceof InterfaceType) { + return $parent->implementsInterface($fragmentType); + } + + if ($fragmentType instanceof UnionType) { + foreach ($fragmentType->getTypes() as $member) { + if ($member->name === $parent->name()) { + return true; + } + } + } + + return false; + } + /** * Merge multiple FieldNodes into one with combined selection set * @param list $nodes diff --git a/tests/SpreadAddsFieldsToPolymorphicField/Generated/Fragment/ContainerBase.php b/tests/SpreadAddsFieldsToPolymorphicField/Generated/Fragment/ContainerBase.php new file mode 100644 index 0000000..6ab0f77 --- /dev/null +++ b/tests/SpreadAddsFieldsToPolymorphicField/Generated/Fragment/ContainerBase.php @@ -0,0 +1,34 @@ + $this->id ??= $this->data['id']; + } + + public Item $item { + get => $this->item ??= new Item($this->data['item']); + } + + /** + * @param array{ + * 'id': string, + * 'item': array{ + * 'id': string, + * ..., + * }, + * ..., + * } $data + */ + public function __construct( + private readonly array $data, + ) {} +} diff --git a/tests/SpreadAddsFieldsToPolymorphicField/Generated/Fragment/ContainerBase/Item.php b/tests/SpreadAddsFieldsToPolymorphicField/Generated/Fragment/ContainerBase/Item.php new file mode 100644 index 0000000..64849b1 --- /dev/null +++ b/tests/SpreadAddsFieldsToPolymorphicField/Generated/Fragment/ContainerBase/Item.php @@ -0,0 +1,24 @@ + $this->id ??= $this->data['id']; + } + + /** + * @param array{ + * 'id': string, + * ..., + * } $data + */ + public function __construct( + private readonly array $data, + ) {} +} diff --git a/tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Data.php b/tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Data.php new file mode 100644 index 0000000..77106c6 --- /dev/null +++ b/tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Data.php @@ -0,0 +1,52 @@ + $this->container ??= new Container($this->data['container']); + } + + /** + * @var list + */ + public readonly array $errors; + + /** + * @param array{ + * 'container': array{ + * 'id': string, + * 'item': array{ + * '__typename': 'VariantA', + * 'id': string, + * 'valueA': string, + * }|array{ + * '__typename': 'VariantB', + * 'id': string, + * 'valueB': string, + * }, + * ..., + * }, + * ..., + * } $data + * @param list $errors + */ + public function __construct( + private readonly array $data, + array $errors, + ) { + $this->errors = array_map(fn(array $error) => new Error($error), $errors); + } +} diff --git a/tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Data/Container.php b/tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Data/Container.php new file mode 100644 index 0000000..d2efc0e --- /dev/null +++ b/tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Data/Container.php @@ -0,0 +1,40 @@ + $this->containerBase ??= new ContainerBase($this->data); + } + + public Item $item { + get => $this->item ??= new Item($this->data['item']); + } + + /** + * @param array{ + * 'id': string, + * 'item': array{ + * '__typename': 'VariantA', + * 'id': string, + * 'valueA': string, + * }|array{ + * '__typename': 'VariantB', + * 'id': string, + * 'valueB': string, + * }, + * ..., + * } $data + */ + public function __construct( + private readonly array $data, + ) {} +} diff --git a/tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Data/Container/Item.php b/tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Data/Container/Item.php new file mode 100644 index 0000000..4dae141 --- /dev/null +++ b/tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Data/Container/Item.php @@ -0,0 +1,56 @@ + $this->asVariantA ??= $this->data['__typename'] === 'VariantA' ? new AsVariantA($this->data) : null; + } + + /** + * @api + * @phpstan-assert-if-true !null $this->asVariantA + */ + public bool $isVariantA { + get => $this->isVariantA ??= $this->data['__typename'] === 'VariantA'; + } + + public ?AsVariantB $asVariantB { + get => $this->asVariantB ??= $this->data['__typename'] === 'VariantB' ? new AsVariantB($this->data) : null; + } + + /** + * @api + * @phpstan-assert-if-true !null $this->asVariantB + */ + public bool $isVariantB { + get => $this->isVariantB ??= $this->data['__typename'] === 'VariantB'; + } + + public string $id { + get => $this->id ??= $this->data['id']; + } + + /** + * @param array{ + * '__typename': 'VariantA', + * 'id': string, + * 'valueA': string, + * }|array{ + * '__typename': 'VariantB', + * 'id': string, + * 'valueB': string, + * } $data + */ + public function __construct( + private readonly array $data, + ) {} +} diff --git a/tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Data/Container/Item/AsVariantA.php b/tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Data/Container/Item/AsVariantA.php new file mode 100644 index 0000000..9bb272b --- /dev/null +++ b/tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Data/Container/Item/AsVariantA.php @@ -0,0 +1,25 @@ + $this->valueA ??= $this->data['valueA']; + } + + /** + * @param array{ + * '__typename': 'VariantA', + * 'valueA': string, + * ..., + * } $data + */ + public function __construct( + private readonly array $data, + ) {} +} diff --git a/tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Data/Container/Item/AsVariantB.php b/tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Data/Container/Item/AsVariantB.php new file mode 100644 index 0000000..2f38908 --- /dev/null +++ b/tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Data/Container/Item/AsVariantB.php @@ -0,0 +1,25 @@ + $this->valueB ??= $this->data['valueB']; + } + + /** + * @param array{ + * '__typename': 'VariantB', + * 'valueB': string, + * ..., + * } $data + */ + public function __construct( + private readonly array $data, + ) {} +} diff --git a/tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Error.php b/tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Error.php new file mode 100644 index 0000000..5459f18 --- /dev/null +++ b/tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/Error.php @@ -0,0 +1,24 @@ +message = $error['debugMessage'] ?? $error['message']; + } +} diff --git a/tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/TestQuery.php b/tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/TestQuery.php new file mode 100644 index 0000000..8c96533 --- /dev/null +++ b/tests/SpreadAddsFieldsToPolymorphicField/Generated/Query/Test/TestQuery.php @@ -0,0 +1,59 @@ +client->graphql( + self::OPERATION_DEFINITION, + [ + ], + self::OPERATION_NAME, + ); + + return new Data( + $data['data'] ?? [], // @phpstan-ignore argument.type + $data['errors'] ?? [] // @phpstan-ignore argument.type + ); + } +} diff --git a/tests/SpreadAddsFieldsToPolymorphicField/Schema.graphql b/tests/SpreadAddsFieldsToPolymorphicField/Schema.graphql new file mode 100644 index 0000000..c8b77c0 --- /dev/null +++ b/tests/SpreadAddsFieldsToPolymorphicField/Schema.graphql @@ -0,0 +1,22 @@ +type Query { + container: Container! +} + +type Container { + id: ID! + item: Item! +} + +interface Item { + id: ID! +} + +type VariantA implements Item { + id: ID! + valueA: String! +} + +type VariantB implements Item { + id: ID! + valueB: String! +} diff --git a/tests/SpreadAddsFieldsToPolymorphicField/SpreadAddsFieldsToPolymorphicFieldTest.php b/tests/SpreadAddsFieldsToPolymorphicField/SpreadAddsFieldsToPolymorphicFieldTest.php new file mode 100644 index 0000000..5b66aa9 --- /dev/null +++ b/tests/SpreadAddsFieldsToPolymorphicField/SpreadAddsFieldsToPolymorphicFieldTest.php @@ -0,0 +1,26 @@ +data['item'])` is rejected by PHPStan because the + * sealed arms don't accept the extra `id` key. + */ +final class SpreadAddsFieldsToPolymorphicFieldTest extends GraphQLTestCase +{ + public function testGenerate() : void + { + $this->assertActualMatchesExpected(); + } +} diff --git a/tests/SpreadAddsFieldsToPolymorphicField/Test.graphql b/tests/SpreadAddsFieldsToPolymorphicField/Test.graphql new file mode 100644 index 0000000..97eb39e --- /dev/null +++ b/tests/SpreadAddsFieldsToPolymorphicField/Test.graphql @@ -0,0 +1,20 @@ +query Test { + container { + ...ContainerBase + item { + ... on VariantA { + valueA + } + ... on VariantB { + valueB + } + } + } +} + +fragment ContainerBase on Container { + id + item { + id + } +}