Skip to content

[Yaml] throw meaningful exception for complex mappings #22058

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

Closed
wants to merge 1 commit into from

Conversation

xabbuh
Copy link
Member

@xabbuh xabbuh commented Mar 19, 2017

Q A
Branch? master
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #20579
License MIT
Doc PR

@xabbuh
Copy link
Member Author

xabbuh commented Mar 19, 2017

In fact, I think we need to deprecate the current behaviour in 3.3 first and merge this one for 4.0 (see #22059 for the deprecation).

@@ -144,6 +144,10 @@ public function parse($value, $flags = 0)
$values['value'] = $matches['value'];
}

if (isset($values['value'][1]) && '?' === $values['value'][0] && ' ' === $values['value'][1]) {
throw new ParseException('The parser does not support complex mappings.', $this->currentLineNb + 1, $this->currentLine);
Copy link
Member

Choose a reason for hiding this comment

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

Complex mappings are not supported. ? To match the style of other error messages (e.g. 'Multiple documents are not supported.')

Copy link
Member

@javiereguiluz javiereguiluz left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

👍

@xabbuh
Copy link
Member Author

xabbuh commented Mar 20, 2017

Closing here for now. To be reopened when we start the development of 4.0.

@xabbuh xabbuh closed this Mar 20, 2017
@xabbuh
Copy link
Member Author

xabbuh commented May 21, 2017

For the records: This change is part of #22770.

@xabbuh xabbuh deleted the issue-20579 branch May 21, 2017 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants