Skip to content

[Serializer] Construct annotations using named arguments #40710

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

Conversation

derrabus
Copy link
Member

@derrabus derrabus commented Apr 5, 2021

Q A
Branch? 5.x
Bug fix? no
New feature? no
Deprecations? yes
Tickets N/A
License MIT
Doc PR Not needed

This is the same as #40266, but applied to the serializer annotations.

This PR proposes to bump the doctrine/annotations library to 1.12 to gain access to its emulation layer for named arguments. Furthermore, constructing any of the serializer's annotation classes the old way by passing an array of parameters is deprecated.

Reasons for this change

The constructors of our annotation classes have become unnecessarily complicated because we have to support two ways of calling them:

  • An array of parameters, passed as first argument, because that's the default behavior doctrine/annotations.
  • A set of named arguments because that's how PHP 8 attributes work.

Since we can now tell the Doctrine annotation reader to use named arguments as well, we can simplify the constructors of our annotations significantly.

Drawback

After this change, there is no easy way anymore to construct instances of most of the annotation classes directly on PHP 7. The PR has been built under the assumption that instances of this class are usually created using either Doctrine annotations or a PHP 8 attribute. Thus, most applications should be unaffected by this change.

@derrabus derrabus requested a review from dunglas as a code owner April 5, 2021 14:53
@derrabus derrabus added this to the 5.x milestone Apr 5, 2021
@derrabus derrabus force-pushed the deprecation/serializer-annotation-array-constructors branch 2 times, most recently from 2fc3fd2 to b992301 Compare April 5, 2021 17:02
@derrabus derrabus force-pushed the deprecation/serializer-annotation-array-constructors branch from b992301 to c116662 Compare April 5, 2021 17:03
@carsonbot

This comment has been minimized.

@fabpot
Copy link
Member

fabpot commented Apr 8, 2021

Thank you @derrabus.

@fabpot fabpot merged commit 30b73c7 into symfony:5.x Apr 8, 2021
@derrabus derrabus deleted the deprecation/serializer-annotation-array-constructors branch April 8, 2021 21:47
@fabpot fabpot mentioned this pull request Apr 18, 2021
chalasr added a commit that referenced this pull request May 9, 2021
This PR was merged into the 5.3-dev branch.

Discussion
----------

[Serializer] Fix duplicated legacy test

| Q             | A
| ------------- | ---
| Branch?       | 5.x
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Probably accidentally kept in #40710

Commits
-------

19e5650 Fix duplicated legacy test
hultberg pushed a commit to hultberg/symfony that referenced this pull request Sep 17, 2021
…arguments (derrabus)

This PR was merged into the 5.3-dev branch.

Discussion
----------

[Serializer] Construct annotations using named arguments

| Q             | A
| ------------- | ---
| Branch?       | 5.x
| Bug fix?      | no
| New feature?  | no
| Deprecations? | yes
| Tickets       | N/A
| License       | MIT
| Doc PR        | Not needed

This is the same as symfony#40266, but applied to the serializer annotations.

This PR proposes to bump the `doctrine/annotations` library to 1.12 to gain access to its emulation layer for named arguments. Furthermore, constructing any of the serializer's annotation classes the old way by passing an array of parameters is deprecated.

### Reasons for this change

The constructors of our annotation classes have become unnecessarily complicated because we have to support two ways of calling them:
* An array of parameters, passed as first argument, because that's the default behavior `doctrine/annotations`.
* A set of named arguments because that's how PHP 8 attributes work.

Since we can now tell the Doctrine annotation reader to use named arguments as well, we can simplify the constructors of our annotations significantly.

### Drawback

After this change, there is no easy way anymore to construct instances of most of the annotation classes directly on PHP 7. The PR has been built under the assumption that instances of this class are usually created using either Doctrine annotations or a PHP 8 attribute. Thus, most applications should be unaffected by this change.

Commits
-------

c116662 [Serializer] Construct annotations using named arguments
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.

3 participants