Skip to content

Add ReturnTypeWillChange to ReflectionEmbeddedProperty#8742

Merged
greg0ire merged 1 commit intodoctrine:2.9.xfrom
derrabus:bugfix/return-type-will-change
Jun 5, 2021
Merged

Add ReturnTypeWillChange to ReflectionEmbeddedProperty#8742
greg0ire merged 1 commit intodoctrine:2.9.xfrom
derrabus:bugfix/return-type-will-change

Conversation

@derrabus
Copy link
Member

@derrabus derrabus commented Jun 5, 2021

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 ReflectionProperty 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 future 3.0 release could add the actual return types.

@derrabus
Copy link
Member Author

derrabus commented Jun 5, 2021

The PHP CodeSniffer errors seem to be false positives.

edit: Reported as slevomat/coding-standard#1233

@greg0ire
Copy link
Member

greg0ire commented Jun 5, 2021

@derrabus let's add an ignore rule with the link above then? Or is the plan to wait for an upstream fix?

@derrabus derrabus force-pushed the bugfix/return-type-will-change branch from 0f6f28b to 6432a3e Compare June 5, 2021 20:52
@derrabus
Copy link
Member Author

derrabus commented Jun 5, 2021

let's add an ignore rule with the link above then?

Done.

@greg0ire greg0ire merged commit b880809 into doctrine:2.9.x Jun 5, 2021
@greg0ire greg0ire added this to the 2.9.3 milestone Jun 5, 2021
@greg0ire
Copy link
Member

greg0ire commented Jun 5, 2021

Thanks @derrabus !

@derrabus derrabus deleted the bugfix/return-type-will-change branch June 5, 2021 22:03
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.

2 participants