Re: [PHP-DEV] [RFC] Deprecate dynamic properties

This is only part of a thread. view whole thread
  115802
August 25, 2021 10:45 rowan.collins@gmail.com (Rowan Tommins)
On 25/08/2021 11:02, Nikita Popov wrote:
> I'd like to propose the deprecation of "dynamic properties", that is > properties that have not been declared in the class (stdClass and > __get/__set excluded, of course): > > https://wiki.php.net/rfc/deprecate_dynamic_properties
This is a bold move, and in principle seems sensible, although I'm slightly scared how many places will need fixing in legacy code bases. I have a couple of concerns with using stdClass as the opt-in mechanism: * The name of that class already leads to a lot of confusion about its purpose - it's not actually "standard" in any way, and new users seeing it as a base class are even more likely to mistake it as some kind of "universal ancestor". Would it be feasible to introduce an alias like "DynamicObject" which more clearly defines its role? * Adding a parent to an existing class isn't always possible, if it already inherits from something else. Perhaps the behaviour could also be available as a trait, which defined stub __get and __set methods, allowing for the replacement of the internal implementation as you've described? Regards, -- Rowan Tommins [IMSoP]
  115804
August 25, 2021 10:52 flaviohbatista@gmail.com (=?UTF-8?Q?Fl=C3=A1vio_Heleno?=)
On Wed, Aug 25, 2021 at 7:46 AM Rowan Tommins collins@gmail.com>
wrote:

> On 25/08/2021 11:02, Nikita Popov wrote: > > I'd like to propose the deprecation of "dynamic properties", that is > > properties that have not been declared in the class (stdClass and > > __get/__set excluded, of course): > > > > https://wiki.php.net/rfc/deprecate_dynamic_properties > > > This is a bold move, and in principle seems sensible, although I'm > slightly scared how many places will need fixing in legacy code bases. > > I have a couple of concerns with using stdClass as the opt-in mechanism: > > * The name of that class already leads to a lot of confusion about its > purpose - it's not actually "standard" in any way, and new users seeing > it as a base class are even more likely to mistake it as some kind of > "universal ancestor". Would it be feasible to introduce an alias like > "DynamicObject" which more clearly defines its role? > > * Adding a parent to an existing class isn't always possible, if it > already inherits from something else. Perhaps the behaviour could also > be available as a trait, which defined stub __get and __set methods, > allowing for the replacement of the internal implementation as you've > described? > > > Regards, > > -- > Rowan Tommins > [IMSoP] > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > > Although not having voting karma, I'm +1 on this.
I've always preferred explicit over implicit code as it makes the learning path to newcomers a lot easier to grasp. When we talk about legacy code, more frequently than not is that legacy code is unlikely to run on the latest version of PHP, thus I'm not entirely sure if that's a super strong argument against it, but I may be missing something. -- Atenciosamente, Flávio Heleno
  115806
August 25, 2021 11:05 office.hamzaahmad@gmail.com (Hamza Ahmad)
HI Nikita,

What if you consider this instead? Rather allowing STD class as an
exception to dynamic properties, why not introduce STDAbstract?
```php
interface STDClassInterface
{
public function __get(string $name) : mixed;
public function __set(string $name, mixed $value = null) : mixed;
/* * other magic methods */
};
abstract class STDClassAbstract implements STDClassInterface {/* *
interface methods */}
Class STDClass extends SDTClassAbstract{}
``

Best
Hamza

On 8/25/21, Rowan Tommins collins@gmail.com> wrote:
> On 25/08/2021 11:02, Nikita Popov wrote: >> I'd like to propose the deprecation of "dynamic properties", that is >> properties that have not been declared in the class (stdClass and >> __get/__set excluded, of course): >> >> https://wiki.php.net/rfc/deprecate_dynamic_properties > > > This is a bold move, and in principle seems sensible, although I'm > slightly scared how many places will need fixing in legacy code bases. > > I have a couple of concerns with using stdClass as the opt-in mechanism: > > * The name of that class already leads to a lot of confusion about its > purpose - it's not actually "standard" in any way, and new users seeing > it as a base class are even more likely to mistake it as some kind of > "universal ancestor". Would it be feasible to introduce an alias like > "DynamicObject" which more clearly defines its role? > > * Adding a parent to an existing class isn't always possible, if it > already inherits from something else. Perhaps the behaviour could also > be available as a trait, which defined stub __get and __set methods, > allowing for the replacement of the internal implementation as you've > described? > > > Regards, > > -- > Rowan Tommins > [IMSoP] > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > >
  115809
August 25, 2021 12:35 a.leathley@gmx.net (Andreas Leathley)
On 25.08.21 12:45, Rowan Tommins wrote:
> * Adding a parent to an existing class isn't always possible, if it > already inherits from something else. Perhaps the behaviour could also > be available as a trait, which defined stub __get and __set methods, > allowing for the replacement of the internal implementation as you've > described? > Couldn't this be easily done in userland with the ArrayLikeObject (an
example in the RFC) or something similar, just done as a trait? Maybe having an example trait implementation that basically "imitates" stdClass in terms of dynamic properties would be helpful to show in the RFC, to highlight a possible resolution path for legacy code with this problem. This could be an example implementation: trait DynamicPropertiesTrait {     private array $dynamicProperties = [];     public function &__get(string $name): mixed { return $this->dynamicProperties[$name]; }     public function __isset(string $name): bool { return isset($this->dynamicProperties[$name]); }     public function __set(string $name, mixed $value): void { $this->dynamicProperties[$name] = $value; }     public function __unset(string $name): void { unset($this->dynamicProperties[$name]); } }
  115810
August 25, 2021 12:45 nikita.ppv@gmail.com (Nikita Popov)
On Wed, Aug 25, 2021 at 12:45 PM Rowan Tommins collins@gmail.com>
wrote:

> On 25/08/2021 11:02, Nikita Popov wrote: > > I'd like to propose the deprecation of "dynamic properties", that is > > properties that have not been declared in the class (stdClass and > > __get/__set excluded, of course): > > > > https://wiki.php.net/rfc/deprecate_dynamic_properties > > > This is a bold move, and in principle seems sensible, although I'm > slightly scared how many places will need fixing in legacy code bases. > > I have a couple of concerns with using stdClass as the opt-in mechanism: > > * The name of that class already leads to a lot of confusion about its > purpose - it's not actually "standard" in any way, and new users seeing > it as a base class are even more likely to mistake it as some kind of > "universal ancestor". Would it be feasible to introduce an alias like > "DynamicObject" which more clearly defines its role? > > * Adding a parent to an existing class isn't always possible, if it > already inherits from something else. Perhaps the behaviour could also > be available as a trait, which defined stub __get and __set methods, > allowing for the replacement of the internal implementation as you've > described? >
So, a few thoughts: First of all, I should say that "extends stdClass" is not so much an explicit escape hatch I built into this, it's more something that arises naturally: We obviously need to keep support for dynamic properties on stdClass, and if we do so, I would expect that to apply to subclasses as well. As such, I believe that the "extends stdClass" escape hatch is something that's going to work anyway -- the only question would be whether we want to provide additional opt-ins in the form of interfaces/traits/attributes/etc. Second, I consider "extends stdClass" to be something of a last-ditch option. If you encounter a dynamic property deprecation warning, you should generally resolve it in some other way, and only fall back to "extends stdClass" as the final option. All that said, yes, we could add a trait. It would basically be ArrayLikeObject from the RFC with "class" replaced by "trait". However, I'm not sure this is really worthwhile. The nice thing about extending stdClass is that it a) gives you much more efficient property access than __get/__set and b) matches current behavior. Implementing such a trait, even if bundled, doesn't really give you any advantages over implemening __get/__set yourself. It would not make use of an optimized implementation (or at least, the implementation would still be calling real __get/__set methods) and the behavior would be limited to what the trait provides. A custom implementation is not significantly harder (the minimal implementation is five lines of code) but gives you more control, e.g. you might want just the minimal implementation, or you might want to also support Traversable, have a custom __debugInfo(), etc. My preliminary position would be that if a) you can't avoid dynamic properties in any other way and b) you can't extend stdClass either, then you should go with c) implementing __get/__set yourself, as appropriate for the given use-case. Regards, Nikita
  115811
August 25, 2021 13:51 rowan.collins@gmail.com (Rowan Tommins)
On 25/08/2021 13:45, Nikita Popov wrote:

> We obviously need to keep support for dynamic properties on stdClass, > and if we do so, I would expect that to apply to subclasses as well.
Does that actually follow, though? Right now, there is no advantage to extending stdClass, so no reason to expect existing code to do so, and no reason for people doing so to expect it to affect behaviour.
> Second, I consider "extends stdClass" to be something of a last-ditch > option. If you encounter a dynamic property deprecation warning, you > should generally resolve it in some other way, and only fall back to > "extends stdClass" as the final option.
That's a reasonable argument in terms of the multiple inheritance case. My concern about the name remains though: people already do get confused by the name "stdClass", because it's not in any way "standard", and tells you nothing about what it does. Reading "class Foo extends stdClass" gives the reader no clues what magic behaviour is being inherited; "class Foo extends DynamicObject" would be much more clear. Similarly, "$foo = new DynamicObject; $foo->bar = 42;" is clearer than "$foo = new stdClass; $foo->bar = 42;" Regards, -- Rowan Tommins [IMSoP]
  115812
August 25, 2021 14:00 nikita.ppv@gmail.com (Nikita Popov)
On Wed, Aug 25, 2021 at 3:51 PM Rowan Tommins collins@gmail.com>
wrote:

> On 25/08/2021 13:45, Nikita Popov wrote: > > > We obviously need to keep support for dynamic properties on stdClass, > > and if we do so, I would expect that to apply to subclasses as well. > > Does that actually follow, though? Right now, there is no advantage to > extending stdClass, so no reason to expect existing code to do so, and > no reason for people doing so to expect it to affect behaviour. >
Isn't this a direct consequence of LSP? We can't just drop behavior when extending. Only way for this not to follow would be if we disallowed extending from stdClass entirely, which we presumably don't want to do.
> Second, I consider "extends stdClass" to be something of a last-ditch > > option. If you encounter a dynamic property deprecation warning, you > > should generally resolve it in some other way, and only fall back to > > "extends stdClass" as the final option. > > > That's a reasonable argument in terms of the multiple inheritance case. > > My concern about the name remains though: people already do get confused > by the name "stdClass", because it's not in any way "standard", and > tells you nothing about what it does. > > Reading "class Foo extends stdClass" gives the reader no clues what > magic behaviour is being inherited; "class Foo extends DynamicObject" > would be much more clear. Similarly, "$foo = new DynamicObject; > $foo->bar = 42;" is clearer than "$foo = new stdClass; $foo->bar = 42;" >
Yes, I can see the argument for aliasing stdClass to something more meaningful, even independently of this RFC. I'm not sure it will have a big impact for this use-case though, because using the new name would require polyfilling it, so I'd expect people would stick to the old name in the foreseeable future... Regards, Nikita
  115818
August 25, 2021 14:49 rowan.collins@gmail.com (Rowan Tommins)
On 25/08/2021 15:00, Nikita Popov wrote:
> I'm not sure it will have a big impact for this use-case though, > because using the new name would require polyfilling it, so I'd expect > people would stick to the old name in the foreseeable future...
Not necessarily - it depends what versions you need to support, and what your migration strategy is. Since it's just a deprecation notice initially, you could support 8.1 and 8.2 on the same code base with no changes at all - that's what deprecation notices are for, to give a lead time before changes are mandatory. Once the code base no longer needed to support 8.1, "extends DynamicObject" could be added. For many code bases, that will be long before the feature is actually removed - for a private project, maybe as soon as it's on 8.2 in production. Regards, -- Rowan Tommins [IMSoP]
  115813
August 25, 2021 14:04 chasepeeler@gmail.com (Chase Peeler)
On Wed, Aug 25, 2021 at 9:51 AM Rowan Tommins collins@gmail.com>
wrote:

> On 25/08/2021 13:45, Nikita Popov wrote: > > > We obviously need to keep support for dynamic properties on stdClass, > > and if we do so, I would expect that to apply to subclasses as well. > > Does that actually follow, though? Right now, there is no advantage to > extending stdClass, so no reason to expect existing code to do so, and > no reason for people doing so to expect it to affect behaviour. > > > > Second, I consider "extends stdClass" to be something of a last-ditch > > option. If you encounter a dynamic property deprecation warning, you > > should generally resolve it in some other way, and only fall back to > > "extends stdClass" as the final option. > > > That's a reasonable argument in terms of the multiple inheritance case. > > My concern about the name remains though: people already do get confused > by the name "stdClass", because it's not in any way "standard", and > tells you nothing about what it does. > > Reading "class Foo extends stdClass" gives the reader no clues what > magic behaviour is being inherited; "class Foo extends DynamicObject" > would be much more clear. Similarly, "$foo = new DynamicObject; > $foo->bar = 42;" is clearer than "$foo = new stdClass; $foo->bar = 42;" > > Regards, > > -- > Rowan Tommins > [IMSoP] > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > > Please don't do this. Call it bad coding practices or not, but this was
something I've considered a feature of PHP and have actually built things around it. It's not something that can be easily refactored since it was part of the design. -- Chase Peeler chasepeeler@gmail.com
  115836
August 25, 2021 20:56 ramsey@php.net (Ben Ramsey)
--9T0kEoBqncr6QLdj4opjFb23AIhnDfYXp
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: quoted-printable

Chase Peeler wrote on 8/25/21 09:04:
> Please don't do this. Call it bad coding practices or not, but this was=
> something I've considered a feature of PHP and have actually built thin= gs
> around it. It's not something that can be easily refactored since it wa= s
> part of the design.
Since the stdClass behavior is retained in this proposal, could you change your base classes to `extend stdClass`? Cheers, Ben --9T0kEoBqncr6QLdj4opjFb23AIhnDfYXp--
  115840
August 25, 2021 22:37 mike@newclarity.net (Mike Schinkel)
> On Aug 25, 2021, at 10:04 AM, Chase Peeler <chasepeeler@gmail.com> wrote: > > On Wed, Aug 25, 2021 at 9:51 AM Rowan Tommins collins@gmail.com> > wrote: > >> On 25/08/2021 13:45, Nikita Popov wrote: >> >>> We obviously need to keep support for dynamic properties on stdClass, >>> and if we do so, I would expect that to apply to subclasses as well. >> >> Does that actually follow, though? Right now, there is no advantage to >> extending stdClass, so no reason to expect existing code to do so, and >> no reason for people doing so to expect it to affect behaviour. >> >> >>> Second, I consider "extends stdClass" to be something of a last-ditch >>> option. If you encounter a dynamic property deprecation warning, you >>> should generally resolve it in some other way, and only fall back to >>> "extends stdClass" as the final option. >> >> >> That's a reasonable argument in terms of the multiple inheritance case. >> >> My concern about the name remains though: people already do get confused >> by the name "stdClass", because it's not in any way "standard", and >> tells you nothing about what it does. >> >> Reading "class Foo extends stdClass" gives the reader no clues what >> magic behaviour is being inherited; "class Foo extends DynamicObject" >> would be much more clear. Similarly, "$foo = new DynamicObject; >> $foo->bar = 42;" is clearer than "$foo = new stdClass; $foo->bar = 42;" >> >> Regards, >> >> -- >> Rowan Tommins >> [IMSoP] >> >> -- >> PHP Internals - PHP Runtime Development Mailing List >> To unsubscribe, visit: https://www.php.net/unsub.php >> >> > Please don't do this. Call it bad coding practices or not, but this was > something I've considered a feature of PHP and have actually built things > around it. It's not something that can be easily refactored since it was > part of the design.
While I tend to really object to BC breakage, when I saw this proposal I was all for it because it disallows a practice that I have seen used all too often in cases where there was no good reason to not declare the properties, and I know that to be fact because the cases I am referring to are ones where I refactored to use declared properties. That said, I'd be really interested in seeing use-cases where having dynamic properties is essential to an architecture and where it could not be easily refactored. I cannot envision any, but I am sure that I am just limited by the extent of my vision and so would like to know what those use-cases would be. --------- Speaking of, it would seem like if dynamic properties were to be deprecated PHP should also add a function that would allow registering a callable as global hook to be called every time ANY object has a property dynamically accessed or assigned — conceptually like how spl_autoload_register() works — so that developers could run their programs to see what properties are used. The hook could collect class names, property names and data types to help developers who need to update their code discover the information required for their class declarations. Because doing it manually is a real PITA. As would be adding __get()/__set() to every class when you have lots of classes and you'd need to delete all that code later anyway. -Mike
  115814
August 25, 2021 14:08 pierre-php@processus.org (Pierre)
Le 25/08/2021 à 15:51, Rowan Tommins a écrit :
> On 25/08/2021 13:45, Nikita Popov wrote: > >> We obviously need to keep support for dynamic properties on stdClass, >> and if we do so, I would expect that to apply to subclasses as well. > > Does that actually follow, though? Right now, there is no advantage to > extending stdClass, so no reason to expect existing code to do so, and > no reason for people doing so to expect it to affect behaviour. > > >> Second, I consider "extends stdClass" to be something of a last-ditch >> option. If you encounter a dynamic property deprecation warning, you >> should generally resolve it in some other way, and only fall back to >> "extends stdClass" as the final option. > > > That's a reasonable argument in terms of the multiple inheritance case. > > My concern about the name remains though: people already do get > confused by the name "stdClass", because it's not in any way > "standard", and tells you nothing about what it does. > > Reading "class Foo extends stdClass" gives the reader no clues what > magic behaviour is being inherited; "class Foo extends DynamicObject" > would be much more clear. Similarly, "$foo = new DynamicObject; > $foo->bar = 42;" is clearer than "$foo = new stdClass; $foo->bar = 42;" > > Regards, > Hello,
And why not adding an extra keyword, such as: ``` ``` Regards, -- Pierre
  115815
August 25, 2021 14:18 internals@lists.php.net ("Levi Morrison via internals")
> And why not adding an extra keyword, such as: > > ``` > dynamic class Foo { } // Dynamic allowed > class Bar {} // Dynamic disallowed > ?> > ```
I am opposed. IMO it's not worth adding anything to the language to preserve this existing behavior other than allowing `__get`/`__set`, etc, which already exist for this purpose. Additionally, there are technical benefits to removing dynamic properties altogether, so I don't think we should "half" remove it. See https://wiki.php.net/rfc/deprecate_dynamic_properties#internal_impact.