Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Messenger] Removes deprecated call to ReflectionType::__toString() on MessengerPass#32398
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Tobion commentedJul 5, 2019
Should be fixed in 4.2 |
derrabus commentedJul 5, 2019
Is this the only place where we do that cast? |
| } | ||
| return [(string)$parameters[0]->getType()]; | ||
| return [$parameters[0]->getType()->getName()]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
it seemsgetName only exists on ReflectionNamedTypehttps://www.php.net/manual/en/reflectionnamedtype.getname.php andhttps://php.net/manual/en/reflectionparameter.gettype.php says it only returns ReflectionType. So getName would not be safe to call here.
But that looks like the php doc is wrong. When the param does not have a typegetType returns null. So getType always returns ReflectionNamedType|null, seehttps://3v4l.org/W1PUh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
That's because currently, ReflectionNamedType is the only subtype of ReflectionType
ref:phpstan/phpstan#1133 (comment)
volkyeth commentedJul 6, 2019
Not sure. That's the only one that triggered the Deprecation notice on my test suite, tough |
volkyeth commentedJul 6, 2019
you mean I should retarget this PR to the 4.2 branch?
Do you mean I should retarget this PR to the 4.2 branch or do another one for that one? |
xabbuh commentedJul 6, 2019
@brunowowk Yes, we always apply bugfixes to the lowest maintained branch where the bug occurs. From there we regularly merge branches up into all other still maintained branches. |
volkyeth commentedJul 6, 2019
Done ✌️ |
Tobion commentedJul 6, 2019
Thank you@brunowowk. |
…oString() on MessengerPass (brunowowk)This PR was merged into the 4.2 branch.Discussion----------[Messenger] Removes deprecated call to ReflectionType::__toString() on MessengerPassCloses#32397| Q | A| ------------- | ---| Branch? | 4.3| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#32397| License | MIT| Doc PR | -Removes deprecated call to ReflectionType::__toString() for 7.4 supportCommits-------0c52a53 Remove call to deprecated method
derrabus commentedJul 6, 2019
Congratulations on your first contribution. 😃 |
Closes#32397
Removes deprecated call to ReflectionType::__toString() for 7.4 support