[Form][PhpUnitBridge] Remove usage of noop ReflectionProperty::setAccessible()
#61207
Conversation
OskarStark
changed the title
ReflectionProperty::setAccessible()ReflectionProperty::setAccessible()
carsonbot
changed the title
ReflectionProperty::setAccessible()ReflectionProperty::setAccessible()
|
|
Refactors like this one generally target the feature branch (7.4 at this time). Only bug fixes are pushed to 7.3 (and below), so given And indeed, you can safely ignore fabbot recommendations and Windows failures, which are not related to your pull request. |
Hey Alexandre, thanks for the input! Before I change the target branch, here my rational why I believe it should be in 7.3. This RFC (vote pending) seeks to deprecate the usage in PHP 8.5. If the vote goes through, what I believe based on the internals list discussion, it would unneccessarily spam depcrecation warnings. I agree, it's not a bug, but I think it's a worthy "fix" to have in all supported versions. Wdyt? |
|
Ah yes, I missed this one. Makes sense then 👍 If these lines are also present in earlier versions, it might be a good idea to even target 6.4, so it is also compatible with upcoming 8.5 and doesn't trigger deprecations on release. |
NickSdot
force-pushed
the
chore-setAccessible
branch
from
072c2b0 to
5623395
Compare
|
Done. 6.4 had a few more. Updated and target branch changed! Edit: I am a bit confused about the test suite here. 😅 Again not related? If there is anything I must do, please let me know. |
nicolas-grekas
force-pushed
the
chore-setAccessible
branch
from
e3fdd2b to
5f69f0f
Compare
|
Thank you @NickSdot. |
| @@ -86,7 +86,6 @@ public function startTest(Test $test): void | |||
| private function addCoversForClassToAnnotationCache(Test $test, array $covers): void | |||
| { | |||
| $r = new \ReflectionProperty(TestUtil::class, 'annotationCache'); | |||
| $r->setAccessible(true); | |||
There was a problem hiding this comment.
I think we need to revert the changes made to the PHPUnit bridge as it supports PHP 7.
There was a problem hiding this comment.
Why does/would the PhpUnit bridge in Symfony 6.4, which requires PHP 8.1, need to support PHP 7?
There was a problem hiding this comment.
We have to indeed, my bad for missing that!
The bridge on 7.4 is the one that's bumped to PHP >= 8.1
There was a problem hiding this comment.
So back to where we been yesterday? I target 7.4 for the full changes, and 6.4 for everything but bridge?
As of PHP 8.1.0, calling this method has no effect; all properties are accessible by default. Symfony 7.x requires PHP 8.2+.
https://www.php.net/manual/en/reflectionproperty.setaccessible.php
Notes:
Component/ErrorHandler/Internal/TentativeTypes.phpis auto-generated, didn't touch it.targeted 7.3 because support for 7.2 ends in a few days.