Skip to content
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

Do not use single quotes in enum type definition #1561

Merged
merged 1 commit into from
Oct 16, 2024

Conversation

bobvandevijver
Copy link
Contributor

@bobvandevijver bobvandevijver commented Oct 11, 2024

Q A
Bug fix? Inconsistency fix
New feature? no
Doc updated yes
BC breaks? no
Deprecations? yes
Tests pass? yes
Fixed tickets -
License MIT

When enum support was added, it was chosen to use quotes around the type in its type annotation, for example enum<'Color'>. However, when compared with the other type annotations such as array and iterable, those quotes are inconsistent.

This PR makes the enum type annotation behaviour similar to the others and deprecated passing a direct string. This deprecation is only triggered during deserialization, as the serialization path has never used the type parameter.

I found this while using NelmioApiDocBundle, see nelmio/NelmioApiDocBundle#2327. I am also making an update for that bundle so it can support both syntaxes.

This makes its behaviour similar to the other typed types, such as array and iterable.
@@ -12,32 +12,34 @@
class ObjectWithEnums
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just thinking out loud - how it is going to work with Attributes? I guess we might miss some test cases for it but it might be used by some users as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe it is just missing tests for Attributes on this particular level. I've said goodbye to annotations quite a while back, so I hit this while using Attributes and verified that for me this solution works.

Copy link
Collaborator

@goetas goetas left a comment

Choose a reason for hiding this comment

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

looks good. tbh i forgot that it would work without quotes

@scyzoryck scyzoryck merged commit 25b5ebe into schmittjoh:master Oct 16, 2024
18 checks passed
@scyzoryck
Copy link
Collaborator

scyzoryck commented Oct 16, 2024

I will release it on Monday. Thanks for contribution!

@bobvandevijver bobvandevijver deleted the generic-enum-interface branch October 16, 2024 09:50
DjordyKoert added a commit to nelmio/NelmioApiDocBundle that referenced this pull request Oct 31, 2024
| Q | A |

|---------------|---------------------------------------------------------------------------------------------------------------------------|
| Bug fix? | yes |
| New feature? | no <!-- please update src/**/CHANGELOG.md files --> |
| Deprecations? | no <!-- please update UPGRADE-*.md and
src/**/CHANGELOG.md files --> |
| Issues | Fix #2327 <!-- prefix each issue number with "Fix #", no need
to create an issue if none exists, explain below instead --> |

JMS implemented the enum type annotations inconsistently compared to the
other type annotations. I've created
schmittjoh/serializer#1561 to fix that (which
has been merged and released with 3.31.0), and this PR adds
compatibility for both methods.

---------

Co-authored-by: Djordy Koert <djordy.koert@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants