Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Closed
alexpott wants to merge13 commits intosymfony:masterfromalexpott:yaml-duplicate-flag

Conversation

@alexpott
Copy link
Contributor

@alexpottalexpott commentedAug 4, 2016
edited
Loading

QA
Branch?master
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#19526
LicenseMIT
Doc PR

ivangretsky reacted with thumbs up emoji
@linaori
Copy link
Contributor

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
Copy link
Contributor

As proposed by other people, i would prefer deprecate this and throw an exception in symfony 4.0.

nicolas-grekas, linaori, and sstok reacted with thumbs up emoji

@nicolas-grekas
Copy link
Member

@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);
Copy link
Contributor

@ogizanagiogizanagiAug 8, 2016
edited
Loading

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

Copy link
Contributor

@ogizanagiogizanagiAug 8, 2016
edited
Loading

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
Copy link
ContributorAuthor

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
Copy link
Member

👍

* @see http://yaml.org/spec/1.1/#id932806
*/
publicfunctiontestMappingDuplicateKeyBlock()
publicfunctiontestLegacyMappingDuplicateKeyBlock()

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
Copy link
Member

Please also update the upgrade files for 3.2 and 4.0.

* 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
Copy link
Member

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
Copy link
Member

Thank you@alexpott.

@fabpotfabpot closed thisAug 9, 2016
fabpot added a commit that referenced this pull requestAug 9, 2016
…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
Copy link
Member

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
Copy link
Contributor

GuilhemN commentedSep 7, 2016
edited
Loading

@stof from what i see, the spec says that it should be considered as an error:

If both notations are used as keys in the same mapping, only a YAML processor which recognizes integer formats would correctly flag the duplicate key as an error.

And:

Technically, YAML therefore complies with the JSON spec, choosing to treat duplicates as an error. In practice, since JSON is silent on the semantics of such duplicates, the only portable JSON files are those with unique keys, which are therefore valid YAML files.

@fabpotfabpot mentioned this pull requestOct 27, 2016
@ivangretsky
Copy link

Good day!
Was looking for exact same feature. It is said to be in the master, but I cannotfind it. Am I missing something?

@xabbuh
Copy link
Member

@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.

ivangretsky reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

11 participants

@alexpott@linaori@GuilhemN@nicolas-grekas@fabpot@xabbuh@stof@ivangretsky@ogizanagi@javiereguiluz@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp