10000 [Serializer] Add support for variadic arguments in the GetSetNormalizer by stof · Pull Request #15426 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Serializer] Add support for variadic arguments in the GetSetNormalizer #15426

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 9, 2015

Conversation

stof
Copy link
Member
@stof stof commented Aug 1, 2015
Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

There were 2 broken cases:

  • when the value was passed, the array was passed as argument, becoming the first value of the variadic array. The array needs to be spread into multiple arguments when calling the method
  • when the value was missing, the code would throw a ReflectionException, similar to the issue reported in [Routing] Annotated routes with a variadic parameter #13690, because a variadic argument is optional but does not have a default value

if (method_exists($constructorParameter, 'isVariadic') && $constructorParameter->isVariadic()) {
if (isset($normalizedData[$paramName])) {
if (!is_array($normalizedData[$paramName])) {
throw new RuntimeException(sprintf('Cannot create an instance of %s from serialized data because the variadic parameter %s can only accept an array.', $class, $constructorParameter->name));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add a test for this?

@Tobion
Copy link
Contributor
Tobion commented Aug 2, 2015

👍

@dunglas
Copy link
Member
dunglas commented Aug 7, 2015

This patch must also be applied to AbstractNormalizer::instantiateObject for 2.7+ and to PropertyNormalizer in 2.3.

👍

@stof
Copy link
Member Author
stof commented Aug 7, 2015

there is no PropertyNormalizer in 2.3

@aitboudad
Copy link
Contributor

👍

@fabpot
Copy link
Member
fabpot commented Aug 9, 2015

Thank you @stof.

@fabpot fabpot merged commit 704760b into symfony:2.3 Aug 9, 2015
fabpot added a commit that referenced this pull request Aug 9, 2015
…SetNormalizer (stof)

This PR was merged into the 2.3 branch.

Discussion
----------

[Serializer] Add support for variadic arguments in the GetSetNormalizer

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

There were 2 broken cases:

- when the value was passed, the array was passed as argument, becoming the first value of the variadic array. The array needs to be spread into multiple arguments when calling the method
- when the value was missing, the code would throw a ReflectionException, similar to the issue reported in #13690, because a variadic argument is optional but does not have a default value

Commits
-------

704760b Add support for variadic arguments in the GetSetNormalizer
@fabpot
Copy link
Member
fabpot commented Aug 9, 2015

@stof Can you merge 2.3 into 2.7?

@stof stof deleted the fix_variadic_arguments branch August 26, 2015 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants
0