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

update php/object tag syntax #16952

Closed
wants to merge 3 commits into from
Closed

update php/object tag syntax #16952

wants to merge 3 commits into from

Conversation

koenpunt
Copy link

[Yaml] deprecate old php/object syntax

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? yes
Tests pass? yes
Fixed tickets #16877
License MIT

the old syntax (double exclamation mark) is to represent native YAML
types, a single exclamation mark represent a local tag.
an E_USER_DEPRECATED error is triggered for old tags.

the old syntax (double exclamation mark) is to represent native YAML
types, a single exclamation mark represent a local tag.
an E_USER_DEPRECATED error is triggered for old tags.
fixes #16877
@xabbuh
Copy link
Member

xabbuh commented Dec 10, 2015

Actually, I would consider this a bugfix which means that the changes here (except for the deprecation trigger) should be done in 2.3.

@@ -477,8 +477,11 @@ private static function evaluateScalar($scalar, $references = array())
case 0 === strpos($scalar, '! '):
return (int) self::parseScalar(substr($scalar, 2));
case 0 === strpos($scalar, '!!php/object:'):
trigger_error('!!php/object is deprecated use !php/object instead', E_USER_DEPRECATED);
$scalar = substr($scalar, 1);
Copy link
Member

Choose a reason for hiding this comment

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

I would add a comment saying the fallthrough is intended

@stof
Copy link
Member

stof commented Dec 10, 2015

I agree with @xabbuh that we should consider the proper syntax as a bugfix. The deprecation should then be added in master only

@@ -477,8 +477,11 @@ private static function evaluateScalar($scalar, $references = array())
case 0 === strpos($scalar, '! '):
return (int) self::parseScalar(substr($scalar, 2));
case 0 === strpos($scalar, '!!php/object:'):
trigger_error('!!php/object is deprecated use !php/object instead', E_USER_DEPRECATED);
Copy link
Member

Choose a reason for hiding this comment

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

missing silencing (which makes the testsuite fail btw)

update assertions
@koenpunt
Copy link
Author

Oh well, there's a difference in running the Yaml suite and running the full suite...

@xabbuh xabbuh added the Yaml label Jan 4, 2016
@xabbuh
Copy link
Member

xabbuh commented Jan 4, 2016

@koenpunt Can you only add the deprecation here and create a new pull request for the bugfix based on the 2.3 branch?

@@ -477,8 +477,11 @@ private static function evaluateScalar($scalar, $references = array())
case 0 === strpos($scalar, '! '):
return (int) self::parseScalar(substr($scalar, 2));
case 0 === strpos($scalar, '!!php/object:'):
@trigger_error('!!php/object is deprecated since version 3.0 and will be removed in 4.0. Use !php/object instead.', E_USER_DEPRECATED);
Copy link
Member

Choose a reason for hiding this comment

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

The version this will be deprecated in is 3.1 (not 3.0).

@xabbuh
Copy link
Member

xabbuh commented Jan 20, 2016

closing in favour of #17461 and #17462

@xabbuh xabbuh closed this Jan 20, 2016
@koenpunt
Copy link
Author

👌

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.

None yet

4 participants