[Serializer] Add types to private and internal properties #45721
Conversation
|
Hey! I think @mtarld has recently worked with this code. Maybe they can help review this? Cheers! Carsonbot |
src/Symfony/Component/Serializer/CacheWarmer/CompiledClassMetadataCacheWarmer.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Serializer/Normalizer/ObjectNormalizer.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Friendly ping @derrabus
I'm just wondering about those array-key and array<Foo>: don't we prefer using Foo[]? /cc @wouterj since you're on a similar topic these days :)
For the record, I'm not sold on blindly using readonly on private properties: the keyword breaks legit use cases (lazy proxies) for little to no practical benefits.
I agree about I'm not sure about I prefer |
|
Friendly ping @derrabus ) |
bbaabc3 to
63bb48e
Compare
63bb48e to
7ff739a
Compare
|
Thank you @derrabus. |
|
Thank you for completing it. |
| * @param array<DecoderInterface> $decoders | ||
| */ | ||
| public function __construct( | ||
| private readonly array $decoders = [] |
There was a problem hiding this comment.
I think I've said this before, but strictly speaking this is a BC break: the $decoders property has no longer a default value (the default value is given to the parameter only).
This creates issues when overriding the constructor without calling the parent as the property becomes uninitialized: https://3v4l.org/DId0c
Is this something we consider too much of an edge case, or would this be a reason against transforming everything to CPP?
There was a problem hiding this comment.
This has already beaten us at least once so this is a valid concern to me.
This PR adds types to private and
@internalproperties of the serializer component. This time, I've also applied CPP andreadonlyflags where possible. If this makes the review too hard or there are other reasons not to do that, please tell me and I'll skip it in future PRs.