Do not call Reflection*::setAccessible() #82
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR removes a deprecated call to ReflectionProperty::setAccessible() which is no longer needed as of PHP 8.1.0, where all reflection methods are invokable by default. The change helps future-proof the codebase against the planned deprecation in PHP 8.5.
- Removes redundant
setAccessible(true)call from reflection property access
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| $idProperty = $reflection->getProperty($this->userIdField); | ||
| $idProperty->setAccessible(true); | ||
|
|
||
| $value = $idProperty->getValue($user); |
There was a problem hiding this comment.
Removing setAccessible(true) may cause issues when accessing private or protected properties in PHP versions prior to 8.1. Consider checking the PHP version or handling potential ReflectionException when accessing non-public properties.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Will this cause any problems for older versions of PHP that may be using this library?
There was a problem hiding this comment.
According to https://github.com/Unleash/unleash-client-symfony/blob/main/composer.json#L12 this package requires at least PHP 8.2.
Do we still need to check for older versions?
There was a problem hiding this comment.
It's automatically transpiled for older versions, this runs on 7.3 as well.
|
This can't be merged, the code is transpiled all the way down to 7.3 where it would cause issues if merged. See for example the 7.3 branch. |
3 participants
Description
(https://www.php.net/manual/en/reflectionmethod.setaccessible.php and https://www.php.net/manual/en/reflectionproperty.setaccessible.php).
There're even plans to deprecate the method in PHP 8.5: https://wiki.php.net/rfc/deprecations_php_8_5#extreflection_deprecations
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: