Repository navigation
ReflectionMethod constructor should not find private parent method #9470
Description
Description
Original issue: #9409
Description
Related to #9405 and https://bugs.php.net/bug.php?id=64592
It might not be important, but I think this should be fixed at some point, so I've created a new issue. I can also provide PR if you want.
- Private methods are accessible through child using
ReflectionMethod:
The following code: https://3v4l.org/Bso8B
<?php
class A
{
private $privateProperty = 1;
private function privateMethod() {}
}
class B extends A {}
try {
echo (string) new ReflectionMethod('B', 'privateMethod');
} catch(Throwable $e){
echo (string) $e;
}
try {
echo (string) new ReflectionProperty('B', 'privateProperty');
} catch(Throwable $e){
echo (string) $e;
}Resulted in this output:
Method [ <user, inherits A> private method privateMethod ] {
@@ /in/Bso8B 6 - 6
}
ReflectionException: Property B::$privateProperty does not exist in /in/Bso8B:16
Stack trace:
#0 /in/Bso8B(16): ReflectionProperty->__construct('B', 'privateProperty')
#1 {main}
But I expected this output instead:
ReflectionException: Method B::privateMethod() does not exist in /in/Bso8B:11
Stack trace:
#0 /in/Bso8B(11): ReflectionMethod->__construct('B', 'privateMethod2')
#1 {main}ReflectionException: Property B::$privateProperty does not exist in /in/Bso8B:16
Stack trace:
#0 /in/Bso8B(16): ReflectionProperty->__construct('B', 'privateProperty')
#1 {main}
PHP Version
8.0 - 8.2
Operating System
No response
Reacted by KapitanOczywisty, Nikita Popov, Michael Voříšek and Gina Peter Banyard
Activity
usarise commentedon Jun 9, 2023
I'm at least the second person who didn't immediately realize it was a language change and not a bug,
maybe add information about it to the UPGRADE -> Backward Incompatible Changes?
iluuu1994 commentedon Jun 22, 2023
@usarise Yes, I usually add the note just before merging (because rebasing loses it, due to our git merge policy). That's why it's not in the PR diff.
jrfnl commentedon Aug 28, 2023
@iluuu1994 While the commit includes it in
NEWS, this change is still not mentioned inUPGRADING. This change could be considered breaking for certain projects, so I imagine raising visibility of the changed behaviour would be good.I've already ran into one project which is affected by this in their test suite (Composer).
Their use-case: create a mock of a certain class, then access a private method on the original class for test purposes.
This is now broken on PHP 8.3 and I currently can't think of a straight-forward work-around for it either.
https://3v4l.org/8YcIk/rfc#vgit.master
iluuu1994 commentedon Aug 28, 2023
@jrfnl Thanks for the heads-up. We usually only add bugs to NEWS, but given that it broke your use case I will add it to UPGRADING.
jrfnl commentedon Aug 28, 2023
@iluuu1994 Thanks!
usarise commentedon Aug 28, 2023
@jrfnl straight-forward work-around:
replace
with
jrfnl commentedon Aug 28, 2023
Thanks @usarise !
iluuu1994 commentedon Sep 1, 2023
@usarise @jrfnl @sebastianbergmann FYI, we've discussed this briefly and decided to revert this change for now, given that this seems to break more code than I expected.
jrfnl commentedon Sep 1, 2023
@iluuu1994 Thanks for letting us know.
fix ReflectionMethod with php8.3 in MicroKernelTrait
fix ReflectionMethod with php8.3 in MicroKernelTrait
fix ReflectionMethod with php8.3 in MicroKernelTrait
iluuu1994 commentedon Sep 5, 2023
FYI #12127. At least this behavior is tested now. 🙂
sebastianbergmann commentedon Nov 7, 2023
AFAICS, the NEWS entry was not reverted and is still there: https://github.com/php/php-src/blob/85cb0816615140afdff89e3bb37f5bcb6bca586b/NEWS