Add native types to reflection property classes #9631
Conversation
| use ValueError; | ||
|
|
||
| use function array_map; | ||
| use function get_class; | ||
| use function is_array; | ||
|
|
||
| class ReflectionEnumProperty extends ReflectionProperty | ||
| final class ReflectionEnumProperty extends ReflectionProperty |
There was a problem hiding this comment.
I've finalized the classes because they extend built-in classes. If the signatures change upstream, maintaining BC for child classes is going to be a nightmare.
There was a problem hiding this comment.
Should we at some point generate a list of finalized classes, then add an @final annotation on every such class on 2.x?
derrabus
force-pushed
the
types/reflection-properties
branch
from
ecf2474 to
0bbf53b
Compare
| @@ -68,7 +53,7 @@ public function getValue($object = null) | |||
| * @param object $object | |||
| * @param int|string|int[]|string[]|null $value | |||
| */ | |||
| public function setValue($object, $value = null): void | |||
| public function setValue(mixed $object, mixed $value = null): void | |||
There was a problem hiding this comment.
Doesn't mixed $value break with the getValue method return type?
There was a problem hiding this comment.
setValue() is inherited from ReflectionProperty which declares mixed as parameter type already. I'm only allowed to make parameter types wider when overriding and it can't get wider than mixed. So mixed is actually the only possible native type here.
There was a problem hiding this comment.
Then I would expect getValue() to return mixed too. https://www.php.net/manual/en/reflectionproperty.getvalue.php
If any value is set that is not part of int|string|array|null, then we'll have a TypeError.
There was a problem hiding this comment.
If any value is set that is not part of
int|string|array|null, then we'll have a TypeError.
No, $enum->value would crash before that. Our implementation of getValue() can only return int|string|array|null, we can be sure about that.
Note, I'm declaring the parameter type as mixed because of a technical limitation which is PHP not supporting generics natively. The doc block is more precise in that regard.
derrabus
force-pushed
the
types/reflection-properties
branch
from
0bbf53b to
d9d2bda
Compare
| use ValueError; | ||
|
|
||
| use function array_map; | ||
| use function get_class; | ||
| use function is_array; | ||
|
|
||
| class ReflectionEnumProperty extends ReflectionProperty | ||
| final class ReflectionEnumProperty extends ReflectionProperty |
There was a problem hiding this comment.
Should we at some point generate a list of finalized classes, then add an @final annotation on every such class on 2.x?
No description provided.