Add ReflectionProperty::getMangledName()

[alexandre-daubois]

Fix #18605

Good news is that we'll be able to leverage this in the next version of Symfony.

[iluuu1994]

Thank you! The implementation looks correct. You may want to avoid printing the mangled name directly in the test, given that it will make the file appear as a binary file. Maybe print the hex value instead, or replace the special chars (i.e. \0 to \\0).

[alexandre-daubois]

replace the special chars (i.e. \0 to \0)

Nice idea! PR updated, thanks!

[TimWolla]

FWIW: The test format also supports the %0 placeholder for this purpose:

php-src/run-tests.php

Line 2886 in 4a98b36

'%0' => '\x00',

[alexandre-daubois]

I tried to swap for %0 but the tests fail. I can try to spend more time on it if you think it should really be used 🙂 Both ways suit me

[iluuu1994]

Maybe you forgot to switch to EXPECTF?

[alexandre-daubois]

Exactly this 🤦 Thank you!

[iluuu1994]

Thanks! LGTM. Waiting on @DanielEScherzer's review as reflection maintainer.

[DanielEScherzer]

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

[alexandre-daubois]

Good catch, ref->prop is null when dealing with dynamic properties. Given they are always public, we can directly return their unmangled name.

[DanielEScherzer]

prop2 never gets used or read, is that intentional?

[DanielEScherzer]

I don't see any placeholders here

Suggested change

	--EXPECTF--
	--EXPECT--

[DanielEScherzer]

Suggested change

	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?

[DanielEScherzer]

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