[Serializer] Supports nested abstract items #27827
Conversation
fb2a316 to
2f7230e
Compare
|
This PR works well and fix my problem, thanks. |
| * Get the types for the given class and attribute. | ||
| * | ||
| * @param string $currentClass | ||
| * @param string $attribute |
There was a problem hiding this comment.
should be removed, we don't duplicate the signature when not adding more than what it already tells.
There was a problem hiding this comment.
Yeah, not sure why I've done that 😄
| * @param string $currentClass | ||
| * @param string $attribute | ||
| * | ||
| * @return null|Type[] |
| } | ||
| } | ||
|
|
||
| return null; |
There was a problem hiding this comment.
imo it's better to say "returning null" rather than not returning anything (void)
There was a problem hiding this comment.
I'd argue it is worth being explicit as well.
| * Get the types for the given class and attribute. | ||
| * | ||
| * @param string $currentClass | ||
| * @param string $attribute |
There was a problem hiding this comment.
Yeah, not sure why I've done that 😄
| } | ||
| } | ||
|
|
||
| return null; |
There was a problem hiding this comment.
I'd argue it is worth being explicit as well.
2f7230e to
43dc167
Compare
| @@ -357,6 +357,20 @@ private function validateAndDenormalize(string $currentClass, string $attribute, | |||
| throw new NotNormalizableValueException(sprintf('The type of the "%s" attribute for class "%s" must be one of "%s" ("%s" given).', $attribute, $currentClass, implode('", "', array_keys($expectedTypes)), gettype($data))); | |||
| } | |||
|
|
|||
| /** | |||
| * Get the types for the given class and attribute. | |||
There was a problem hiding this comment.
Gets, but I think you can remove the description as it does not add anything that is not already part of the signature.
43dc167 to
dcb93b8
Compare
|
@fabpot made the change you were asking for. Can you review again? |
| /** | ||
| * @return Type[]|null | ||
| */ | ||
| protected function getTypes(string $currentClass, string $attribute) |
There was a problem hiding this comment.
Can you make it @internal? The proliferation of public methods in this class make it very hard to maintain (we should split it in several classes/traits instead).
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| protected function getTypes(string $currentClass, string $attribute) |
|
rebase needed |
3c242a1 to
e84e467
Compare
e84e467 to
ec4b04b
Compare
|
Updated. No need to add a protected method in the end, it's all private 💃 Status: Needs Review |
|
Thank you @sroze. |
This PR was merged into the 4.1 branch. Discussion ---------- [Serializer] Supports nested abstract items | Q | A | ------------- | --- | Branch? | 4.1 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #27816, #27641 | License | MIT | Doc PR | ø This adds support for nested "abstract" objects. Commits ------- ec4b04b Supports nested "abstract" object while serializing and de-serializing
This adds support for nested "abstract" objects.