Implement ReflectionProperty::is{Readable,Writable}() #16209
Conversation
|
|
||
| public function isReadable(?string $scope = 'static'): bool {} | ||
|
|
||
| public function isWritable(object $object, ?string $scope = 'static'): bool {} |
There was a problem hiding this comment.
I think that object should be a nullable optional second parameter, because a property might be static. Also non-readonly properties can be checked without an object.
| --FILE-- | ||
| <?php | ||
|
|
||
| class A { |
There was a problem hiding this comment.
Why this test does not check protected properties from parent/child classes?
There was a problem hiding this comment.
Because the impl uses the same code for checking scope as read, so it's somewhat redundant.
There was a problem hiding this comment.
Tests should not assume any implementation details, because implementation might change later, but the tests should detect regression no matter how things are implemented.
| --FILE-- | ||
| <?php | ||
|
|
||
| class A {} |
There was a problem hiding this comment.
I would have added some tests for traits. For example, a test ensuring that protected properties from a shared trait are not accessible (https://3v4l.org/hM5TM). Also a trait must not be accepted as a scope.
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
| @@ -569,6 +569,10 @@ public function hasHook(PropertyHookType $type): bool {} | |||
| public function getHook(PropertyHookType $type): ?ReflectionMethod {} | |||
|
|
|||
| public function isFinal(): bool {} | |||
|
|
|||
| public function isReadable(?string $scope = 'static'): bool {} | |||
There was a problem hiding this comment.
Thinking about this some more, it would likely be beneficial to also accept the object here and check it for IS_UNDEF.
There was a problem hiding this comment.
What IS_UNDEF means in userland?
There was a problem hiding this comment.
unset() properties or typed properties that have not been assigned to.
There was a problem hiding this comment.
Then yes, it would be great to have an object parameter here.
Fixes GH-15309
Fixes GH-16175
Targetting 8.4 because of GH-16175, I'm aware this will probably not make it.