[Yaml] Throw on duplicate key even when value is NULL #57261
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
73fc4ff to
94d306a
Compare
xabbuh
approved these changes
May 31, 2024
94d306a to
0287305
Compare
|
IIUC, currently, this file is valid and has the "expected" behavior. So, throwing an exception now is a BC break that we cannot merge in 5.4. I would vote to do it in 7.2 only and add a note in the CHANGELOG file. |
0287305 to
f7cf88c
Compare
|
The base was changed to 7.2 and I've updated the changelog file. Thank you! |
fabpot
approved these changes
Jun 2, 2024
0f44bec to
f9df19b
Compare
|
Thank you @olsavmic. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Duplicate keys are not valid by definition in YAML. The current implementation contains a bug that allows a key to be defined multiple times when the value is not set.
Extends symfony/yaml@8094454
to throw when a key is set twice in YAML without a value.
It may be technically a breaking change (as it suddenly makes some yaml-like files invalid), even though I'd classify it as a bugfix (as by definition, such files were not valid).
If we classify it as a bug, we should probably backport the fix to the oldest maintained version.