Skip to content

Fix not null get collection key types #41582

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 8, 2021
Merged

Fix not null get collection key types #41582

merged 1 commit into from
Jun 8, 2021

Conversation

dragosprotung
Copy link
Contributor

Q A
Branch? 5.3
Bug fix? yes
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

Warning: Undefined array key 0 is generated because getCollectionKeyTypes never returns null so a check against empty array is needed.

@dragosprotung dragosprotung requested a review from dunglas as a code owner June 7, 2021 10:11
@carsonbot carsonbot added this to the 5.3 milestone Jun 7, 2021
@dragosprotung dragosprotung changed the title Fix not null get collection key types [Serializer] Fix not null get collection key types Jun 7, 2021
ogizanagi
ogizanagi approved these changes Jun 7, 2021
View reviewed changes
Copy link
Contributor

@ogizanagi ogizanagi left a comment

Choose a reason for hiding this comment

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

Was missed in #41463? LGTM

Could you add a test, though?

@carsonbot carsonbot changed the title [Serializer] Fix not null get collection key types Fix not null get collection key types Jun 7, 2021
@dragosprotung
Copy link
Contributor Author

I would add a test if I could even begin to know how to write a test for it. It's too deep into a private method and do not really know how to set it up

@nicolas-grekas
Copy link
Member

Maybe you can take inspiration from the tests added to #41463?

@dragosprotung
Copy link
Contributor Author

In #41463 there was no test added for the Serializer component after the fixes. The test was added for Validator, related to the changes made in Symfony\Component\Validator\Mapping\Loader\PropertyInfoLoader

@nicolas-grekas
Copy link
Member

Thank you @dragosprotung.

@nicolas-grekas nicolas-grekas merged commit 22fd960 into symfony:5.3 Jun 8, 2021
@dragosprotung dragosprotung deleted the fix/not-null-getCollectionKeyTypes branch June 8, 2021 14:42
@fabpot fabpot mentioned this pull request Jun 17, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ogizanagi ogizanagi ogizanagi approved these changes

@dunglas dunglas Awaiting requested review from dunglas dunglas is a code owner

Assignees
No one assigned
Labels
Bug Status: Reviewed
Projects
None yet
Milestone
5.3
Development

Successfully merging this pull request may close these issues.
4 participants
@dragosprotung @nicolas-grekas @ogizanagi @carsonbot