Support for PHP 8.1

[derrabus]

Q	A
Documentation	no
Bugfix	yes
BC Break	no
New Feature	no
RFC	no
QA	no

Description

PHP 8.1 will introduce a process to add return types to internal methods, see https://wiki.php.net/rfc/internal_method_return_types

This library extends various reflection classes and overrides public methods that are affected by this change. PHP 8.1 triggers deprecation warnings unless we declare that a future version of this library will add these return types as well. We can do so by adding the ReturnTypeWillChange attribute to those methods.

This PR suggests to do just that because adding the actual return types would break code that extends our classes. A potential version 5 of this library could then add the actual return types.

There is one problematic case though: MethodReflection::getPrototype(). That method as it is currently implemented will return an array or string, depending on a parameter passed to it. The parent method ReflectionMethod::getPrototype() however will return a ReflectionMethod instance.

My recommended course of action would be to introduce two new methods getPrototypeAsString() and getPrototypeAsArray() and deprecate calling MethodReflection::getPrototype() with the PROTOTYPE_AS_ARRAY/PROTOTYPE_AS_STRING parameter.

If you agree with that, I could work on a follow-up PR.

[derrabus]

Thanks for approving the CI run. Two static analysis jobs are failing:

• The PHP CodeSniffer version used here apparently does not know how to handle attributes and complains about "Perl-style comments" and an unused import ReturnTypeWillChange.
• Psalm complains about an undefined class ReturnTypeWillChange. We could either suppress that error or add symfony/polyfill-php81 to require-dev to make the class known.

Please advice how I may proceed.

[Ocramius]

ReturnTypeWillChange could either be stubbed, or we can wait to run psalm on 8.1 (after this summer, when the first RC are verifiable)

…

On Sat, Jun 5, 2021, 15:12 Alexander M. Turek ***@***.***> wrote: Thanks for approving the CI run. Two static analysis jobs are failing: - The PHP CodeSniffer version used here apparently does not know how to handle attributes and complains about "Perl-style comments" and an unused import ReturnTypeWillChange. - Psalm complains about an undefined class ReturnTypeWillChange. We could either suppress that error or add symfony/polyfill-php81 to require-dev to make the class known. Please advice how I may proceed. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#86 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABFVEDI7HFFJ3BDMFLHALTTRIPEXANCNFSM46EKGUVQ> .

[derrabus]

symfony/polyfill-php81 contains a stub, so I've added that package.

[Ocramius]

No, stubbed as in stubbed for psalm, not further deps (and complexity) for runtime users. A polyfill is absolutely to be avoided here.

…

On Sun, Jun 6, 2021, 01:26 Alexander M. Turek ***@***.***> wrote: symfony/polyfill-php81 contains a stub, so I've added that package. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#86 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABFVECDRSQNXXCUA5GYYNLTRKXBZANCNFSM46EKGUVQ> .

[Ocramius]

Nvm, I see it is require-dev: will probably still need to be extracted to a standalone file 🤔

…

On Sun, Jun 6, 2021, 01:28 Marco Pivetta ***@***.***> wrote: No, stubbed as in stubbed for psalm, not further deps (and complexity) for runtime users. A polyfill is absolutely to be avoided here. On Sun, Jun 6, 2021, 01:26 Alexander M. Turek ***@***.***> wrote: > symfony/polyfill-php81 contains a stub, so I've added that package. > > — > You are receiving this because you commented. > Reply to this email directly, view it on GitHub > <#86 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AABFVECDRSQNXXCUA5GYYNLTRKXBZANCNFSM46EKGUVQ> > . >

[Ocramius]

Marking as blocked until we test also on 8.1.x (later this year)

[derrabus]

Sure, whenever you're ready.

[derrabus]

I had to bump PHP for the PHP CodeSniffer job to 8.0 because it apparently does not understand attributes when run on PHP 7.4. 😢

[derrabus]

Hello again. I've updated the PR to include a green CI for PHP 8.1. Please let me know if I can do anything else to help.

[derrabus]

Hello again. This PR is one of the last blockers for Symfony's PHP 8.1 CI. Before I implement workarounds into our CI to make it pass despite the deprecation warnings triggered by laminas-code, I'd like to ask if there's anything I can do to move this PR forward.

[Ocramius]

We'll wait for the next PHP RC before moving forward. Before PHP 8.1 compatibility is released, we need to add intersection types too.

…

On Sun, 29 Aug 2021, 23:16 Alexander M. Turek, ***@***.***> wrote: Hello again. This PR is one of the last blockers for Symfony's PHP 8.1 CI. Before I implement workarounds into our CI to make it pass despite the deprecation warnings triggered by laminas-code, I'd like to ask if there's anything I can do to move this PR forward. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#86 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABFVEGLNUFOBMTWXEHGDU3T7KPSZANCNFSM46EKGUVQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[derrabus]

All right. Thanks for the heads-up.

[alexander-schranz]

Tested sulu with symfony 5.3 against this PR and did work. Thx for all working on this 👍

[Ocramius]

🚢

Thanks @derrabus!

[derrabus]

Thank you for the review & merge, @Ocramius!

[racile]

symfony/symfony#44285

[Ocramius]

@racile see the related milestone: https://github.com/laminas/laminas-code/milestone/19

Likely done around end of year, I'd say, but no promises.