Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix parsing nullable collections #168

Merged
merged 2 commits into from
Jul 29, 2022
Merged

Conversation

hemberger
Copy link
Contributor

Fixes #163.
Fixes phpDocumentor/phpDocumentor#3290.

Shorthand nullable type syntax is now supported for collection types
(e.g. ?array<int>).

This also prevents a misuse of the shorthand nullable type syntax with
compound types, which is illegal with PHP types. This is documented in
the Union Types 2.0 RFC (https://wiki.php.net/rfc/union_types_v2) by
N. Popov:

Union types and the ?T nullable type notation cannot be mixed.
Writing ?T1|T2, T1|?T2 or ?(T1|T2) is not supported and T1|T2|null
needs to be used instead.

Fixes phpDocumentor#163.
Fixes phpDocumentor/phpDocumentor#3290.

Shorthand nullable type syntax is now supported for collection types
(e.g. `?array<int>`).

This also prevents a misuse of the shorthand nullable type syntax with
compound types, which is illegal with PHP types. This is documented in
the Union Types 2.0 RFC (https://wiki.php.net/rfc/union_types_v2) by
N. Popov:

> Union types and the ?T nullable type notation cannot be mixed.
> Writing ?T1|T2, T1|?T2 or ?(T1|T2) is not supported and T1|T2|null
> needs to be used instead.
$resolvedType = $fixture->resolve('?string|null|?boolean');

$this->assertSame('?string|null|?bool', (string) $resolvedType);
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if this behavior should change. I do understand that PHP doesn't allow this format and that it looks more than ugly. But this case has been added because we found some repositories using this notation.

As this library is used to parse types in docblocks and native PHP, I think we should keep this format supported.
It could result in a deprecation that we will remove in the future. But for now, this is a backward compatibility break which we cannot do with the number of consuming projects for this library.

Do you think it is possible to revert this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback! I did not fully appreciate the need for backwards compatibility, so I have restored support for shorthand nullable syntax in compound types as requested. Please let me know if this solution is not satisfactory.

While PHP types do not support shorthand nullable syntax in compound
types (e.g. `?string|int`), it has been requested to retain support
for this in phpdoc types for backwards compatibility reasons.
@jaapio jaapio merged commit 9aaa423 into phpDocumentor:1.x Jul 29, 2022
@hemberger hemberger deleted the issue-163 branch July 29, 2022 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error parsing nullable arrays in phpdoc Nullable collections throw RuntimeException
2 participants