-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[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
nicolas-grekas
left a comment
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
nicolas-grekas
left a comment
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
ReflectionAttributejust 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
ReflectionClasConstantas bonus.The full output for the
LotsOfAttributesfixture class that is included with is PR looks like this:Details