Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:115810
MIME-Version: 1.0
References: <CAF+90c9_qu=PiTS81J2mesPfJdz7n5nSi3qoLNLjhEGN-aTytA@mail.gmail.com>
 <df1992f6-fef0-8f62-95d3-dcbc4a5b86d1@gmail.com>
In-Reply-To: <df1992f6-fef0-8f62-95d3-dcbc4a5b86d1@gmail.com>
Date: Wed, 25 Aug 2021 14:45:35 +0200
Message-ID: <CAF+90c_o1eRxPdJBrOai0Vnmsc=-MYpaWq0wY9zNuL_Vc5jm8Q@mail.gmail.com>
To: Rowan Tommins <rowan.collins@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000061099805ca61a153"
Subject: Re: [PHP-DEV] [RFC] Deprecate dynamic properties
From: nikita.ppv@gmail.com (Nikita Popov)

--00000000000061099805ca61a153
Content-Type: text/plain; charset="UTF-8"

On Wed, Aug 25, 2021 at 12:45 PM Rowan Tommins <rowan.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

--00000000000061099805ca61a153--