Skip to content
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

[Yaml] fix PHP const mapping keys using the inline notation #35318

Merged
merged 1 commit into from
Jan 13, 2020

Conversation

xabbuh
Copy link
Member

@xabbuh xabbuh commented Jan 13, 2020

Q A
Branch? 3.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #35179
License MIT
Doc PR

@fabpot
Copy link
Member

fabpot commented Jan 13, 2020

Thank you @xabbuh.

fabpot added a commit that referenced this pull request Jan 13, 2020
…n (xabbuh)

This PR was merged into the 3.4 branch.

Discussion
----------

[Yaml] fix PHP const mapping keys using the inline notation

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #35179
| License       | MIT
| Doc PR        |

Commits
-------

45461c7 fix PHP const mapping keys using the inline notation
@fabpot fabpot merged commit 45461c7 into symfony:3.4 Jan 13, 2020
@xabbuh xabbuh deleted the issue-35179 branch January 14, 2020 05:21
@rhift
Copy link

rhift commented Jan 20, 2020

hey @xabbuh will this make its way into the YAML component? Sorry, I'm not familiar with the release process. Thanks for fixing it!

@xabbuh
Copy link
Member Author

xabbuh commented Jan 20, 2020

@rhift Yes, this change will be part of the next patch releases for 3.4, 4.3, 4.4 and 5.0 as well as future minor versions.

@@ -504,6 +504,11 @@ private static function parseMapping($mapping, $flags, &$i = 0, $references = []
$isKeyQuoted = \in_array($mapping[$i], ['"', "'"], true);
$key = self::parseScalar($mapping, $flags, [':', ' '], $i, false, [], true);

if ('!php/const' === $key) {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we check for self::$constantSupport first? /cc @xabbuh

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is fine because evaluateScalar() (which is called now) does the check.

@rhift
Copy link

rhift commented Apr 28, 2020

Hi there, thanks for this!

I can get globally defined constants to work great, but I can't find a way to get class constants to work. Is this intended behavior?

I've tried the following.

!php/const \PATH\TO\Class::CONSTANT: "value"
!php/const "\PATH\TO\Class::CONSTANT": "value"

The scope resolution operator (::) is trimmed and the constant cannot be found.

Being able to use class constants as keys was what I was hoping for when submitting #35179.

Any help would be appreciated. Thank you. =)

@xabbuh
Copy link
Member Author

xabbuh commented Apr 29, 2020

@rhift If I don't miss anything, we have these cases covered with tests. Could you open a new issue please with some code that allows to reproduce so that we can see whether there is another bug or if there is something else going wrong?

@rhift
Copy link

rhift commented Apr 29, 2020

@xabbuh ok, thanks! I opened #36624.

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