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

[PHP7] Consider to deprecate yii\base\Object as object might become soft-reserved #7936

Closed
suralc opened this Issue Mar 31, 2015 · 43 comments

Comments

Projects
None yet
10 participants
@suralc
Contributor

suralc commented Mar 31, 2015

PHP 7 will soft reserve object as a possible typehint for a 7.x release (or may never use it at all). Deprecating yii\base\Object in favor of yii\base\BaseObject (or a better name) early (in 2.1) might reduce some pain once installations are upgraded if php should decide to use the object name.

I'm not sure if the patch below is the best way to go forward as the deprecation notice is only triggered if parent::__construct() is called in all child classes. However triggering the deprecation message on autoload makes it harder to locate the class using the old inheritance chain.
The deprecation is based on the fact that php in the future might use object; causing the class declaration to cause a fatal error.

Possible patch (only targets the framework itself not extensions, no tests for the deprecation notice).:
master...suralc:deprecate-php7

I did not create this as a pull-request as the vote is not yet formally closed. The vote on soft-reserving object sits at 38:10 in favor so it is likely going to pass.

@suralc suralc changed the title from Deprecate yii\base\Object as object is soft-reserved in php7 to Consider to deprecate yii\base\Object as object might become soft-reserved in php7 Mar 31, 2015

@samdark samdark modified the milestone: 2.1.x Mar 31, 2015

@cebe cebe added this to the 2.1.x milestone Mar 31, 2015

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Mar 31, 2015

Member

Thanks for pointing this out, need to check it.

Member

cebe commented Mar 31, 2015

Thanks for pointing this out, need to check it.

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Apr 2, 2015

Member

Will the name be case sensitive or not?

Member

qiangxue commented Apr 2, 2015

Will the name be case sensitive or not?

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Apr 2, 2015

Member

I guess both will be reserved. We'll see when it will be accepted and someone will create a patch.

Member

samdark commented Apr 2, 2015

I guess both will be reserved. We'll see when it will be accepted and someone will create a patch.

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Apr 2, 2015

Member

Class names are case insensitive so it would not matter in which case we write it.

Member

cebe commented Apr 2, 2015

Class names are case insensitive so it would not matter in which case we write it.

@suralc

This comment has been minimized.

Show comment
Hide comment
@suralc

suralc Apr 7, 2015

Contributor

@cebe The voting has been closed:

[...] the following typenames are now reserved:
Resource
Object
Mixed
Numeric

http://marc.info/?l=php-internals&m=142833998201071&w=2

Contributor

suralc commented Apr 7, 2015

@cebe The voting has been closed:

[...] the following typenames are now reserved:
Resource
Object
Mixed
Numeric

http://marc.info/?l=php-internals&m=142833998201071&w=2

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Apr 7, 2015

Member

Thanks for the info, have already seen it. This is causing much trouble. Yii 2.0.x will not run at all on php7 because of this... Not sure how to solve the issue yet. As far as I see, there is no way without breaking BC. Renaming the base class of all things is a huge thing...

ref: https://twitter.com/cebe_cc/status/585573046808879104

Member

cebe commented Apr 7, 2015

Thanks for the info, have already seen it. This is causing much trouble. Yii 2.0.x will not run at all on php7 because of this... Not sure how to solve the issue yet. As far as I see, there is no way without breaking BC. Renaming the base class of all things is a huge thing...

ref: https://twitter.com/cebe_cc/status/585573046808879104

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Apr 7, 2015

Member

maybe something like this could work:

<?php

namespace yii\base;

class BaseObject { /* current implementation */ }

if (version_compare(PHP_VERSION, '7.0.0', '<')) {
    // ensure code works on php < 7.0.0 to not break BC
    class_alias('yii\base\BaseObject', 'yii\base\Object', false);
}
Member

cebe commented Apr 7, 2015

maybe something like this could work:

<?php

namespace yii\base;

class BaseObject { /* current implementation */ }

if (version_compare(PHP_VERSION, '7.0.0', '<')) {
    // ensure code works on php < 7.0.0 to not break BC
    class_alias('yii\base\BaseObject', 'yii\base\Object', false);
}
@suralc

This comment has been minimized.

Show comment
Hide comment
@suralc

suralc Apr 7, 2015

Contributor

You don't need class_alias (could be nicer with deprecation notices, though). If the class isn't used it won't be autoloaded and won't cause harm (Classes relying on Object will fatal either way, missing class or forbidden name). You could just have Object extend BaseObject and trigger a deprecation notice in it's constructor

Contributor

suralc commented Apr 7, 2015

You don't need class_alias (could be nicer with deprecation notices, though). If the class isn't used it won't be autoloaded and won't cause harm (Classes relying on Object will fatal either way, missing class or forbidden name). You could just have Object extend BaseObject and trigger a deprecation notice in it's constructor

@suralc

This comment has been minimized.

Show comment
Hide comment
@suralc

suralc Apr 7, 2015

Contributor

Also (thus the soft reserving in the initial post): (and yes, the rfc is ambigous in my oppinion)

To be clear, this RFC is about declaring these names as reserved, not
about actually /using/ them. There's no language in the RFC about
actually throwing errors or other code-breaking behavior. Therefore,
even should all of them pass, it does NOT mean that your code using
these names suddenly stops working on PHP7. It means that you have
been warned that using the names of intrinsics is a bad idea, and that
the language reserves the /right/ to make doing so an error at some
point in the future.

http://marc.info/?l=php-internals&m=142654982915943&w=2

Deprecation as soon as possible and add a notice to the docs that using Object over BaseObject is bad practice should be enough for now:

Possible route:

  1. Introduce BaseObject in 2.0.x and have Object extend BaseObject (add a note to docs)
  2. Deprecate Object in 2.1 (and make sure it will never be loaded by default or by some user site optimization based on classmaps) make sure to trigger E_USER_DEPRECATED(?).
  3. Everyone using Object has been warned and does so at their own risk. If their code fails at some point it's not the frameworks fault.
Contributor

suralc commented Apr 7, 2015

Also (thus the soft reserving in the initial post): (and yes, the rfc is ambigous in my oppinion)

To be clear, this RFC is about declaring these names as reserved, not
about actually /using/ them. There's no language in the RFC about
actually throwing errors or other code-breaking behavior. Therefore,
even should all of them pass, it does NOT mean that your code using
these names suddenly stops working on PHP7. It means that you have
been warned that using the names of intrinsics is a bad idea, and that
the language reserves the /right/ to make doing so an error at some
point in the future.

http://marc.info/?l=php-internals&m=142654982915943&w=2

Deprecation as soon as possible and add a notice to the docs that using Object over BaseObject is bad practice should be enough for now:

Possible route:

  1. Introduce BaseObject in 2.0.x and have Object extend BaseObject (add a note to docs)
  2. Deprecate Object in 2.1 (and make sure it will never be loaded by default or by some user site optimization based on classmaps) make sure to trigger E_USER_DEPRECATED(?).
  3. Everyone using Object has been warned and does so at their own risk. If their code fails at some point it's not the frameworks fault.
@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Apr 7, 2015

Member

okay, thats weird, imo it should add some implementation that stops using the words as class names.
The RFC itself is very clear about this:

This breaks any and all cases where these new reserved words are used in class, interface or trait names. It does not break any usages as function or method names or as class constants.

Member

cebe commented Apr 7, 2015

okay, thats weird, imo it should add some implementation that stops using the words as class names.
The RFC itself is very clear about this:

This breaks any and all cases where these new reserved words are used in class, interface or trait names. It does not break any usages as function or method names or as class constants.

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Apr 7, 2015

Member

You don't need class_alias (could be nicer with deprecation notices, though). If the class isn't used it won't be autoloaded. You could just have Object extend BaseObject and trigger a deprecation notice in it's constructor

tests like $var instanceof \yii\base\Object will fail if there is no alias. Also code in user applications extending from Object will break.

Member

cebe commented Apr 7, 2015

You don't need class_alias (could be nicer with deprecation notices, though). If the class isn't used it won't be autoloaded. You could just have Object extend BaseObject and trigger a deprecation notice in it's constructor

tests like $var instanceof \yii\base\Object will fail if there is no alias. Also code in user applications extending from Object will break.

@suralc

This comment has been minimized.

Show comment
Hide comment
@suralc

suralc Apr 7, 2015

Contributor

You won't be able to hint against yii\base\object once the name is used, if it is handled like the scalar type names.

Some idea when and if object breaks code would be helpful.

edit: I missed a point in the text that was located here earlier, so I removed it.

Contributor

suralc commented Apr 7, 2015

You won't be able to hint against yii\base\object once the name is used, if it is handled like the scalar type names.

Some idea when and if object breaks code would be helpful.

edit: I missed a point in the text that was located here earlier, so I removed it.

@cebe cebe self-assigned this Apr 7, 2015

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Apr 7, 2015

Member

You won't be able to hint against yii\base\object once the name is used, if it is handled like the scalar type names.

thats clear, but we can do this to keep BC on PHP versions <7.0.0

Member

cebe commented Apr 7, 2015

You won't be able to hint against yii\base\object once the name is used, if it is handled like the scalar type names.

thats clear, but we can do this to keep BC on PHP versions <7.0.0

@cebe cebe added the php7 label Apr 8, 2015

@cebe cebe modified the milestones: 2.0.5, 2.1.x Apr 8, 2015

@klimov-paul

This comment has been minimized.

Show comment
Hide comment
@klimov-paul

klimov-paul Apr 24, 2015

Member

This could be solved without BC break.
We need to rename yii\base\Object class to yii\base\BaseObject, adjusting any internal inheritance. So yii\base\Component extends yii\base\BaseObject.
Then we simply re-create yii\base\Object in following way:

namespace yii\base;

class Object extends BaseObject
{
    // empty
}

After that framework code will become compatibale with PHP 7, while any developer, which uses ealier version and yii\base\Object will never notice anything.

Member

klimov-paul commented Apr 24, 2015

This could be solved without BC break.
We need to rename yii\base\Object class to yii\base\BaseObject, adjusting any internal inheritance. So yii\base\Component extends yii\base\BaseObject.
Then we simply re-create yii\base\Object in following way:

namespace yii\base;

class Object extends BaseObject
{
    // empty
}

After that framework code will become compatibale with PHP 7, while any developer, which uses ealier version and yii\base\Object will never notice anything.

@klimov-paul

This comment has been minimized.

Show comment
Hide comment
@klimov-paul

klimov-paul Apr 24, 2015

Member

Although, no.
This would not work for checks like:

if ($object instanceof 'yii\base\Object')

If developer relies on such code anywhere it will break.

Member

klimov-paul commented Apr 24, 2015

Although, no.
This would not work for checks like:

if ($object instanceof 'yii\base\Object')

If developer relies on such code anywhere it will break.

@samdark samdark changed the title from Consider to deprecate yii\base\Object as object might become soft-reserved in php7 to [PHP7] Consider to deprecate yii\base\Object as object might become soft-reserved May 29, 2015

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Jun 18, 2015

Member

https://github.com/php/php-src/blob/PHP-7.0.0/UPGRADING

  Furthermore the following class, interface and trait names are now reserved
  for future use, but do not yet throw an error when used:

      resource
      object
      mixed
      numeric

So this one would not be a problem for PHP7 release.

Member

samdark commented Jun 18, 2015

https://github.com/php/php-src/blob/PHP-7.0.0/UPGRADING

  Furthermore the following class, interface and trait names are now reserved
  for future use, but do not yet throw an error when used:

      resource
      object
      mixed
      numeric

So this one would not be a problem for PHP7 release.

@klimov-paul

This comment has been minimized.

Show comment
Hide comment
@klimov-paul

klimov-paul Jun 18, 2015

Member

Still it may become a problem at PHP 7.1 or other future releases.

Member

klimov-paul commented Jun 18, 2015

Still it may become a problem at PHP 7.1 or other future releases.

@tecnologiaterabyte

This comment has been minimized.

Show comment
Hide comment
@tecnologiaterabyte

tecnologiaterabyte Jun 18, 2015

Should consider taking a clean compatible version php 7 or higher warning of possible incompatibilities changes in php code, and could apply all patches delayed for fear bc break, and maintain other yii2 version with less than 7 php for a while.

tecnologiaterabyte commented Jun 18, 2015

Should consider taking a clean compatible version php 7 or higher warning of possible incompatibilities changes in php code, and could apply all patches delayed for fear bc break, and maintain other yii2 version with less than 7 php for a while.

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Jun 18, 2015

Member

for now there is no need for action anymore as yii will still run on php7. we can consider a change in 2.1 to get rid of Object.

Member

cebe commented Jun 18, 2015

for now there is no need for action anymore as yii will still run on php7. we can consider a change in 2.1 to get rid of Object.

@cebe cebe added this to the 2.1.x milestone Jun 18, 2015

cebe added a commit to cebe/yii2 that referenced this issue Jul 18, 2017

cebe added a commit to cebe/yii2 that referenced this issue Jul 18, 2017

cebe added a commit to cebe/yii2 that referenced this issue Jul 18, 2017

cebe added a commit to cebe/yii2 that referenced this issue Jul 18, 2017

cebe added a commit to cebe/yii2 that referenced this issue Jul 18, 2017

cebe added a commit that referenced this issue Jul 19, 2017

@cebe cebe closed this in 393fc27 Jul 19, 2017

macklus added a commit to macklus/yii2-holded that referenced this issue Dec 11, 2017

* Change yii\base\Object to yii\base\BaseObject
Since 2.0.13, the class name `Object` is invalid since PHP 7.2. More info at: yiisoft/yii2#7936 (comment)

tsingsun pushed a commit to tsingsun/yii2 that referenced this issue Jan 9, 2018

tsingsun pushed a commit to tsingsun/yii2 that referenced this issue Jan 9, 2018

tsingsun pushed a commit to tsingsun/yii2 that referenced this issue Jan 9, 2018

Mylistryx pushed a commit to Mylistryx/yii2-asset-converter that referenced this issue Feb 2, 2018

Mylistryx
yiisoft/yii2#7936
use BaseObject instead Object for PHP 7.2 compatibility

mahkali added a commit to mahkali/yii2-cart that referenced this issue Feb 16, 2018

mahkali added a commit to mahkali/yii2-cart that referenced this issue Feb 16, 2018

@netbrothers-tr netbrothers-tr referenced this issue Sep 10, 2018

Closed

Ensure PHP 7.2 compatibility #1

1 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment