Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
Add Yaml::PARSE_EXCEPTION_ON_DUPLICATE to throw exceptions on duplicates#19529
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
linaori commentedAug 4, 2016
I'd love this feature. In a few big applications with 25+ developers, mistakes like this happen every once in a while. I love to see them crash instead of silently continuing. |
GuilhemN commentedAug 4, 2016
As proposed by other people, i would prefer deprecate this and throw an exception in symfony 4.0. |
nicolas-grekas commentedAug 7, 2016
@alexpott OK to replace this flag by a deprecation? |
| if (!isset($output[$key])) { | ||
| $output[$key] =$value; | ||
| }else { | ||
| @trigger_error(sprintf('Duplicate key "%s" detected whilst parsing YAML. Silent handling of duplicates in YAML is deprecated since version 3.3 and will cause an exception in 4.0.',$key),E_USER_DEPRECATED); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Should besince 3.2, right ?
Also should be:
- and will cause an exception+ and will throw an exception
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We should probably also indicates the thrown exception will be an instance ofSymfony\Component\Yaml\Exception\ParseException
alexpott commentedAug 8, 2016
The failure in fabbot.io is on a line not changed by this patch - I guess src/Symfony/Component/Yaml/Tests/Fixtures/YtsSpecificationExamples.yml does not comply with the standard. |
fabpot commentedAug 8, 2016
👍 |
| * @see http://yaml.org/spec/1.1/#id932806 | ||
| */ | ||
| publicfunctiontestMappingDuplicateKeyBlock() | ||
| publicfunctiontestLegacyMappingDuplicateKeyBlock() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
missing@group legacy annotation (and no need to rename the test case then)
same below
xabbuh commentedAug 8, 2016
Please also update the upgrade files for 3.2 and 4.0. |
UPGRADE-4.0.md Outdated
| * The`!!php/object` tag to indicate dumped PHP objects was removed in favor of | ||
| the`!php/object` tag. | ||
| * Support for silently ignoring duplicate keys in YAML has been deprecated and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
In 4.0 it will be longer deprecated.
fabpot commentedAug 9, 2016
Thank you@alexpott. |
…ions on duplicates (Alex Pott)This PR was squashed before being merged into the 3.2-dev branch (closes#19529).Discussion----------Add Yaml::PARSE_EXCEPTION_ON_DUPLICATE to throw exceptions on duplicates| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#19526| License | MIT| Doc PR |Commits-------cb362f2 Add Yaml::PARSE_EXCEPTION_ON_DUPLICATE to throw exceptions on duplicates
stof commentedSep 7, 2016
IIRC, the Yaml spec explicitly says that parsers should use the first key when finding duplicates (and we made a bugfix in the past to respect it instead of using the last one). So this deprecation is a violation of the spec |
GuilhemN commentedSep 7, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@stof from what i see, the spec says that it should be considered as an error:
And:
|
ivangretsky commentedJul 28, 2017
Good day! |
xabbuh commentedJul 28, 2017
@ivangretsky There is no flag controlling this. The parser just triggers a deprecation when detecting duplicated keys on 3.2 or higher and will throw an exception starting with Symfony 4.0. |
Uh oh!
There was an error while loading.Please reload this page.