[PropertyInfo] Make ReflectionExtractor compatible with ReflectionType changes in PHP 7.1 #19853
Conversation
teohhanhui
commented
Sep 5, 2016
teohhanhui
commented
| Q | A |
|---|---|
| Branch? | 2.8 |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #19677 (comment) |
| License | MIT |
| Doc PR | N/A |
| @@ -231,7 +231,7 @@ private function extractFromAccessor($class, $property) | |||
| */ | |||
| private function extractFromReflectionType(\ReflectionType $reflectionType) | |||
| { | |||
| $phpTypeOrClass = (string) $reflectionType; | |||
| $phpTypeOrClass = method_exists($reflectionType, 'getName') ? $reflectionType->getName() : (string) $reflectionType; | |||
There was a problem hiding this comment.
\ReflectionType does't have a getName() method according to the docs: http://php.net/manual/en/class.reflectiontype.php. Will this be added in the future perhaps?
There was a problem hiding this comment.
@because ReflectionType::__toString has changed in PHP 7.1 and will change in the future (it is not properly part of the BC promise). As advised in the PHP 7.1 UPGRADING file, we need to use ReflectionNamedType::getName going forward.
There was a problem hiding this comment.
ReflectionType::__toString() will now return the type name with a leading
"?" if it is nullable. To retrieve the type name without leading "?" the new
ReflectionNamedType::getName() method can be used.
https://github.com/php/php-src/blob/9650b8e2412a919df8c5c626a1def937bc6d77d5/UPGRADING#L127-L129
There was a problem hiding this comment.
In case of the toString, shouldn't the ? be stripped?
There was a problem hiding this comment.
If there is a getName method, then we won't be using __toString, and if we are using __toString that means it's PHP <= 7.0 so there is no ? prepended anyway. Stripping of characters is only a temporary solution anyway, as the __toString is likely to keep changing, judging from the mailing list discussion.
teohhanhui
force-pushed
the
reflectiontype_7.1
branch
from
07b1622
to
4a041e9
Compare
|
Fabbot error is a false positive (it doesn't support PHP 7.1 yet). 👍 but I propose to wait and see what happens to this feature before the release of 7.1 to merge the PR. |
|
If we want people to test Symfony with PHP 7.1, we should provide the compatibility. It's already in RC now, so things shouldn't change as much. |
|
@teohhanhui see #19817 and related failure |
|
Thank you @teohhanhui. |
…flectionType changes in PHP 7.1 (teohhanhui) This PR was merged into the 2.8 branch. Discussion ---------- [PropertyInfo] Make ReflectionExtractor compatible with ReflectionType changes in PHP 7.1 | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #19677 (comment) | License | MIT | Doc PR | N/A Commits ------- 4a041e9 Make ReflectionExtractor compatible with ReflectionType changes in PHP 7.1
