Add ReflectionProperty::getMangledName()
#18980
Conversation
ef72bbf to
b6188b6
Compare
9022201 to
5dd71d0
Compare
Nice idea! PR updated, thanks! |
|
FWIW: The test format also supports the Line 2886 in 4a98b36 |
|
I tried to swap for |
|
Maybe you forgot to switch to EXPECTF? |
5dd71d0 to
dd598de
Compare
|
Exactly this 🤦 Thank you! |
|
Thanks! LGTM. Waiting on @DanielEScherzer's review as reflection maintainer. |
There was a problem hiding this comment.
Looks good, but there are a few edge cases that should probably be tested
- Dynamic class properties
- Class properties that override parent properties (both for public and protected, and both for maintaining the same visibility and increasing it (for protected->public))
- using ReflectionProperty from a specific instance, rather than from a class - the instance might have a dynamic property that the class doesn't
- and more specifically, ReflectionProperty from an instance of stdClass
dd598de to
8043a9d
Compare
|
Good catch, |
8043a9d to
9118590
Compare
9118590 to
298dcc1
Compare
| echo "=== Testing stdClass with dynamic properties ===\n"; | ||
| $stdObj = new stdClass(); | ||
| $stdObj->prop1 = 'value1'; | ||
| $stdObj->prop2 = 'value2'; |
There was a problem hiding this comment.
prop2 never gets used or read, is that intentional?
| } | ||
|
|
||
| ?> | ||
| --EXPECTF-- |
There was a problem hiding this comment.
I don't see any placeholders here
| --EXPECTF-- | |
| --EXPECT-- |
| $obj = new $class(); | ||
| $array = (array) $obj; | ||
|
|
||
| echo "Class: $class, Property: $property\n"; |
There was a problem hiding this comment.
| echo "Class: $class, Property: $property\n"; | |
| echo "Class: $class, Property: \$$property\n"; |
because some of the property names are also visibility indicators, and the output is confusing in that case - is it a property named "public", or one that is public?
| testProperty('ChildClass', 'public'); | ||
| testProperty('ChildClass', 'protected'); | ||
| testProperty('ChildClass', 'childProp'); | ||
| testProperty('ChildClass', 'private'); |
There was a problem hiding this comment.
can you also test with a private property on the parent class that does not get shadowed with a property of the same name in the child class? In both instance and class based creation of ReflectionProperty the constructor should fail, but its good to confirm so that if it ever changes we have a test that gets updated
Fix #18605
Good news is that we'll be able to leverage this in the next version of Symfony.