-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[VarDumper] Support for ReflectionAttribute #38167
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
Conversation
1e313ff
to
6ac8ec3
Compare
/** | ||
* @param \Reflector|\ReflectionAttribute $c | ||
*/ | ||
private static function addMap(array &$a, $c, $map, $prefix = Caster::PREFIX_VIRTUAL) |
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 felt a bit odd that I needed to do this, so I've opened https://bugs.php.net/bug.php?id=80097 for clarification.
6ac8ec3
to
6225dc2
Compare
The segfault that be observed in https://travis-ci.org/github/symfony/symfony/jobs/726646852 (line 3655) appeared after my changes to the
But when I rerun that command a couple of times, it eventually works and the tests pass. I haven't been able to isolate the problem yet, but maybe this is an interesting find for @nikic or @beberlei that helps with stabilizing the reflection API for attributes. |
Do you have the stacktrace for the segfault? |
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.
Thanks for the PR.
This should target master to me, as I have this rule in mind:
- making existing code run on newest PHP versions should be considered as a bug fix
- making Symfony compatible with new features of the language should be considered as a new feature.
src/Symfony/Component/VarDumper/Tests/Caster/ReflectionCasterTest.php
Outdated
Show resolved
Hide resolved
I use the |
6225dc2
to
b61e5c4
Compare
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.
with minor comments
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.
with minor comments
b61e5c4
to
f2d674c
Compare
for |
f2d674c
to
34dbf01
Compare
Thank you @derrabus. |
VarDumper currently does not understand that certain reflection objects might have attributes attached to it. Dumping a
ReflectionAttribute
just yieldsReflectionAttribute {#4711}
which is not really helpful. This PR attempts to fix this.While working on this, I noticed that class constants (which can be reflected on since PHP 7.1) are just dumped as plain values, so I've also added a caster for
ReflectionClasConstant
as bonus.The full output for the
LotsOfAttributes
fixture class that is included with is PR looks like this: