Skip to content

feat: support phpdocumentor/reflection-docblock 6 and type-resolver 2 #840

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
oojacoboo merged 2 commits into from
Oct 15, 2026
+67 −6
Merged

feat: support phpdocumentor/reflection-docblock 6 and type-resolver 2 #840

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8e5a39d
feat: support phpdocumentor/reflection-docblock 6 and type-resolver 2
timtheisinger Oct 13, 2026
9b800ec
fix: do not treat generic iterables as raw iterators
timtheisinger Oct 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
"composer-runtime-api": "^2.0",
"php": ">=8.2",
"ext-json": "*",
"phpdocumentor/reflection-docblock": "^5.4",
"phpdocumentor/type-resolver": "^1.7",
"phpdocumentor/reflection-docblock": "^5.4 || ^6.0",
"phpdocumentor/type-resolver": "^1.7 || ^2.0",
"psr/container": "^1.1 || ^2",
"psr/http-factory": "^1",
"psr/http-message": "^1.0.1 || ^2.0",
Expand Down
11 changes: 11 additions & 0 deletions phpstan.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,17 @@ parameters:
reportUnmatchedIgnoredErrors: false
treatPhpDocTypesAsCertain: false
ignoreErrors:
# Collection exists in phpdocumentor/type-resolver 1 only, Generic in phpdocumentor/type-resolver 2 only
-
message: '#phpDocumentor\\Reflection\\(Types\\Collection|PseudoTypes\\Generic)#'
identifier: class.notFound
path: src/Mappers/Parameters/TypeHandler.php
-
message: '#phpDocumentor\\Reflection\\PseudoTypes\\Generic#'
identifier: class.notFound
paths:
- src/Mappers/Root/BaseTypeMapper.php
- src/Mappers/Root/IteratorTypeMapper.php
- "#PHPDoc tag \\@throws with type Psr\\\\Container\\\\ContainerExceptionInterface is not subtype of Throwable#"
- "#Parameter .* of class ReflectionMethod constructor expects string(\\|null)?, object\\|string given.#"
- "#PHPDoc tag @throws with type Psr\\\\SimpleCache\\\\InvalidArgumentException is not subtype of Throwable#"
Expand Down
4 changes: 3 additions & 1 deletion src/Mappers/Parameters/TypeHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use phpDocumentor\Reflection\DocBlock\Tags\Return_;
use phpDocumentor\Reflection\DocBlock\Tags\Var_;
use phpDocumentor\Reflection\Fqsen;
use phpDocumentor\Reflection\PseudoTypes\Generic;
use phpDocumentor\Reflection\Type;
use phpDocumentor\Reflection\TypeResolver as PhpDocumentorTypeResolver;
use phpDocumentor\Reflection\Types\Array_;
Expand Down Expand Up @@ -453,7 +454,8 @@ private function mapType(
// Try to match generic phpdoc-provided iterables with non-generic return-type-provided iterables
// Example: (return type `\ArrayObject`, phpdoc `\ArrayObject<string, TestObject>`)
|| ($innerType instanceof Object_
&& $docBlockType instanceof Collection
// `Collection` with phpdocumentor/type-resolver 1, `Generic` with phpdocumentor/type-resolver 2
&& ($docBlockType instanceof Collection || $docBlockType instanceof Generic)
&& (string) $innerType->getFqsen() === (string) $docBlockType->getFqsen()
)
) {
Expand Down
12 changes: 11 additions & 1 deletion src/Mappers/Root/BaseTypeMapper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use GraphQL\Type\Definition\Type as GraphQLType;
use GraphQL\Upload\UploadType;
use phpDocumentor\Reflection\DocBlock;
use phpDocumentor\Reflection\PseudoTypes\Generic;
use phpDocumentor\Reflection\Type;
use phpDocumentor\Reflection\Types\AbstractList;
use phpDocumentor\Reflection\Types\Array_;
Expand All @@ -36,6 +37,7 @@
use TheCodingMachine\GraphQLite\Types\DateTimeType;
use TheCodingMachine\GraphQLite\Types\ID;

use function end;
use function ltrim;

/**
Expand Down Expand Up @@ -65,6 +67,14 @@ public function toGraphQLOutputType(Type $type, OutputType|null $subType, Reflec

return GraphQLType::listOf($innerType);
}
// phpdocumentor/type-resolver 2 represents generic iterables like `\ArrayObject<int, User>` as `Generic`
// (an `Object_`) instead of `Collection` (an `AbstractList`).
if ($type instanceof Generic && $type->getTypes() !== []) {
$genericTypes = $type->getTypes();
$innerType = $this->topRootTypeMapper->toGraphQLOutputType(end($genericTypes), $subType, $reflector, $docBlockObj);

@oojacoboo oojacoboo Oct 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we only pass the last type of the $type->getTypes() for the OutputType? Does this mean support for union and intersection types aren't properly supported here?

@timtheisinger timtheisinger Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're working with lists here and want to get the type of items in that list. For all common PHP list types like ArrayObject or Iterator (not true for Generator) this should be last the type parameter. This behavior is also exactly what getValueType did in type-resolver v1.
See: https://github.com/phpDocumentor/TypeResolver/blob/1.12.0/src/TypeResolver.php#L457-L466
https://github.com/phpDocumentor/TypeResolver/blob/1.12.0/src/Types/Collection.php#L40
It reverses the params and the last one lands in valueType.

I'm not sure exactly what you mean by missing support for unions/intersections. For example if we have a \ArrayObject<int, A&B>, the whole intersection A&B will be the value type, as before. The tests cover this already as far as I can see.

@oojacoboo oojacoboo Oct 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timtheisinger what does getTypes return for a phpdoc like:

@return \ArrayObject<int, A&B>|bool

@timtheisinger timtheisinger Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure but getTypes() on the outer type isn't called. \ArrayObject<int, A&B>|bool is a Compound, and only its \ArrayObject<int, A&B> part reaches this branch, with getTypes() = [int, A&B].

I found another problem though. Generic is an Object_, unlike Collection, so IteratorTypeMapper treated it as a raw iterator when looking at \ArrayObject|User[]. \ArrayObject<int, A>|bool then mapped to [A!]! and silently dropped bool, where type-resolver 1 threw. Fixed this and added tests.
I compared these cases:

Docblock type-resolver 1 type-resolver 2 (current PR)
\ArrayObject<int, A>|bool error: unions only allowed between objects [A!]!, bool silently dropped
\ArrayObject<int, A>|B error [A!]!, B dropped
\ArrayObject<int, A>|B[] error [A!]!, B[] dropped
\ArrayObject<int, A&B> error: can't handle A&B error, but misleading text ("ArrayObject is iterable…")
\ArrayObject<int, A|B>, …|null works works, same result

@oojacoboo oojacoboo Oct 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timtheisinger The errors are preferred in these cases, as it let's the developer know that it's not supported. Silently dropping is misleading.

@oojacoboo oojacoboo Oct 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we can improve the error handling around this as well, so that it's more descriptive of the actual issue, that's a plus

@timtheisinger timtheisinger Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The table I added was the behavior before my newest commit (my bad). It now errors instead of dropping, same as before.

If we can improve the error handling around this as well, so that it's more descriptive of the actual issue, that's a plus

I just did a minimal fix and didn't look into that. I'd rather keep this PR minimal though if that's ok.

@oojacoboo oojacoboo Oct 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timtheisinger I've read over your tests and see now - thanks

@timtheisinger timtheisinger Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! Do you have an idea when the next release might be come out?

@oojacoboo oojacoboo Oct 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have some pretty large changes that recently got merged. I'd like to give it a little time to see if there are any bug reports. It's a breaking release. Soon though.

You're welcome to target dev-master in the meantime.


return GraphQLType::listOf($innerType);
}
if ($type instanceof Object_) {
$className = ltrim((string) $type->getFqsen(), '\\');

Expand Down Expand Up @@ -92,7 +102,7 @@ public function toGraphQLInputType(Type $type, InputType|null $subType, string $

return GraphQLType::listOf($innerType);
}
if ($type instanceof Object_) {
if ($type instanceof Object_ && ! $type instanceof Generic) {
$className = ltrim((string) $type->getFqsen(), '\\');

return $this->recursiveTypeMapper->mapClassToInputType($className);
Expand Down
6 changes: 4 additions & 2 deletions src/Mappers/Root/IteratorTypeMapper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Iterator;
use IteratorAggregate;
use phpDocumentor\Reflection\DocBlock;
use phpDocumentor\Reflection\PseudoTypes\Generic;
use phpDocumentor\Reflection\Type;
use phpDocumentor\Reflection\Types\Array_;
use phpDocumentor\Reflection\Types\Compound;
Expand Down Expand Up @@ -44,7 +45,8 @@ public function toGraphQLOutputType(Type $type, OutputType|null $subType, Reflec
try {
return $this->next->toGraphQLOutputType($type, $subType, $reflector, $docBlockObj);
} catch (CannotMapTypeException $e) {
if ($type instanceof Object_) {
// A `Generic` like `\ArrayObject<int, User>` already provides its item type
if ($type instanceof Object_ && ! $type instanceof Generic) {
/** @var class-string<object> $fqcn */
$fqcn = (string) $type->getFqsen();
$refClass = new ReflectionClass($fqcn);
Expand Down Expand Up @@ -207,7 +209,7 @@ private function toGraphQLType(Compound $type, Closure $topToGraphQLType, bool $
private function splitIteratorFromOtherTypes(array &$types): Type|null
{
foreach ($types as $key => $singleDocBlockType) {
if (! ($singleDocBlockType instanceof Object_)) {
if (! ($singleDocBlockType instanceof Object_) || $singleDocBlockType instanceof Generic) {
continue;
}

Expand Down
33 changes: 33 additions & 0 deletions tests/Mappers/Root/IteratorTypeMapperTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@
use GraphQL\Type\Definition\ListOfType;
use GraphQL\Type\Definition\NonNull;
use phpDocumentor\Reflection\DocBlock;
use PHPUnit\Framework\Attributes\DataProvider;
use ReflectionMethod;
use TheCodingMachine\GraphQLite\AbstractQueryProvider;
use TheCodingMachine\GraphQLite\Fixtures\TestObject;
use TheCodingMachine\GraphQLite\Fixtures\TestObject2;
use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeException;

class IteratorTypeMapperTest extends AbstractQueryProvider
Expand Down Expand Up @@ -50,4 +52,35 @@ public function testIterableWithTwoArrays(): void
$this->expectExceptionMessage('"\ArrayObject" is iterable. Please provide a more specific type. For instance: \ArrayObject|User[].');
$result = $typeMapper->toGraphQLOutputType(self::resolveType('ArrayObject|array<int>|array<string>'), null, new ReflectionMethod(__CLASS__, 'testInputIterator'), new DocBlock());
}

#[DataProvider('unmappableGenericIterablesProvider')]
public function testUnmappableGenericIterable(string $phpdocType, string $expectedMessage): void
{
$typeMapper = $this->getRootTypeMapper();

$this->expectException(CannotMapTypeException::class);
$this->expectExceptionMessage($expectedMessage);
$typeMapper->toGraphQLOutputType(self::resolveType($phpdocType), null, new ReflectionMethod(__CLASS__, 'testUnmappableGenericIterable'), new DocBlock());
}

public static function unmappableGenericIterablesProvider(): iterable
{
// A generic iterable already defines its item type, so other union members are not used as its item type
yield 'generic iterable or bool' => [
\sprintf('\ArrayObject<int, %s>|bool', TestObject::class),
'in GraphQL, you can only use union types between objects. These types cannot be used in union types: [TestObject!]!, Boolean!',
];
yield 'generic iterable or object' => [
\sprintf('\ArrayObject<int, %s>|%s', TestObject::class, TestObject2::class),
'in GraphQL, you can only use union types between objects. These types cannot be used in union types: [TestObject!]!, TestObject2!',
];
yield 'generic iterable or array' => [
\sprintf('\ArrayObject<int, %s>|%s[]', TestObject::class, TestObject2::class),
'in GraphQL, you can only use union types between objects. These types cannot be used in union types: [TestObject!]!, [TestObject2!]!',
];
yield 'generic iterable with unmappable item type' => [
\sprintf('\ArrayObject<int, %s&%s>', TestObject::class, TestObject2::class),
\sprintf("don't know how to handle type \\%s&\\%s", TestObject::class, TestObject2::class),
];
}
}
3 changes: 3 additions & 0 deletions website/docs/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ sidebar_label: Changelog
`withAddedDescriptionItem()`. `QueryFieldDescriptor::withAddedDescriptionLines()` is deprecated in favor of
it.
([#838](https://github.com/thecodingmachine/graphqlite/pull/838)) @oojacoboo
- Supports `phpdocumentor/reflection-docblock` 6 and `phpdocumentor/type-resolver` 2 alongside 5 and 1, so GraphQLite
installs next to packages that already require them. Generic iterables such as `\ArrayObject<int, User>` map to
lists with both type-resolver versions. @timtheisinger

### Bug Fixes

Expand Down
Loading