Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:115848
MIME-Version: 1.0
References: <CAF+90c9_qu=PiTS81J2mesPfJdz7n5nSi3qoLNLjhEGN-aTytA@mail.gmail.com>
 <CAESVnVqMfxSZxG1+k+_o+HZ1B7QbH=wDeMTQm8R-Ok_bEyqSRA@mail.gmail.com>
In-Reply-To: <CAESVnVqMfxSZxG1+k+_o+HZ1B7QbH=wDeMTQm8R-Ok_bEyqSRA@mail.gmail.com>
Date: Thu, 26 Aug 2021 10:34:13 +0200
Message-ID: <CAF+90c-J7skECnr7TE5f5twBNRArhAh3xuKVjdGqq3HsOZrnog@mail.gmail.com>
To: Sara Golemon <pollita@php.net>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000425f2d05ca723c2f"
Subject: Re: [PHP-DEV] [RFC] Deprecate dynamic properties
From: nikita.ppv@gmail.com (Nikita Popov)

--000000000000425f2d05ca723c2f
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Aug 25, 2021 at 5:28 PM Sara Golemon <pollita@php.net> wrote:

> On Wed, Aug 25, 2021 at 5:03 AM Nikita Popov <nikita.ppv@gmail.com> 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 RFC offers `extends stdClass` as a way to opt-in to the use of
> dynamic properties.
> >
> This makes the assumption that existing uses of dynamic properties are al=
l
> root classes. I don't think that assumption can be made.
>
> > Some people have suggested that we could use a magic marker
> > interface (implements SupportsDynamicProperties),
> > an attribute (#[SupportsDynamicProperties])
> > or a trait (use DynamicProperties;) instead.
> >
> My gut-check says an Attribute works well enough.  This will unlock the
> class (disable deprecation warning) in 8.2+ and be ignored in earlier
> releases (8.0 and 8.1 would need an auto-loaded polyfill userspace
> attribute obviously, but that's a tractable problem).
>
> #[SupportsDynamicProperties]
> class Foo { ... }
>

The crux of the issue is what our end goal is:

1. Require users to explicitly annotate classes that use dynamic
properties, but otherwise keep dynamic properties as a fully supported part
of the core language.
2. Remove support for dynamic properties entirely. The core language only
has declared properties and __get/__set, and nothing else. stdClass is just
a very efficient implementation of __get/__set.

The RFC is currently written under the assumption that the end goal is (2).
To support that, I believe that "extends stdClass" and "use
DynamicProperties" are the only viable approaches. The former, because it
inherits stdClass behavior, the latter because it's literally __get/__set.
An attribute would require retaining the capability to have arbitrary
classes with dynamic properties.

Now, if the actual goal is (1) instead, then I fully agree that using a
#[SupportsDynamicProperties] attribute is the ideal solution. It can be
easily applied anywhere and has no BC concerns. Heck, someone who just
doesn't care could easily slap it on their whole codebase without spending
brain cycles on more appropriate solutions.

I do think that just (1) by itself would already be very worthwhile. If
that's all we can reasonably target, then I'd be fine with the
#[SupportsDynamicProperties] solution. Though if (2) is viable without too
many complications, then I think that would be the better final state to
reach.


> > The Locked classes RFC took an alternative approach to this problem
> space:
> > Rather than deprecating/removing dynamic properties and providing an
> opt-in
> > for specific classes, it instead allowed marking specific classes as
> locked in
> > order to forbid creation of dynamic properties on them.
> >
> > I don't believe that this is the right strategy, because in contemporar=
y
> code,
> > classes being =E2=80=9Clocked=E2=80=9D is the default state, while clas=
ses that require
> dynamic
> > properties are a rare exception. Additionally, this requires that class
> owners
> > (which may be 3rd party packages) consistently add the =E2=80=9Clocked=
=E2=80=9D keyword
> > to be effective.
> >
> I struggle here, because the opt-in approach is the most BC, but I
> actually think you're right.  I think[citation needed] that dynamic props
> are probably rare enough that as long as the escape hatch is clear, we ca=
n
> be a little more bold about nudging the ecosystem forward.  I will counte=
r
> however, that the same issue with 3rd party libraries applies to opt-out =
as
> opt-in.  A third party library that's undermaintained (and uses dynamic
> props) won't be able to be used out of the box once we apply this.  I don=
't
> think that's enough to scare us off, it just means that the opt-in side o=
f
> that argument cancels out.
>

I think the argument isn't quite symmetric: A 3rd-party library in
maintenance-only mode has essentially no incentive to go out of their way
and add a "locked" keyword/attribute to their classes. Adding some missing
property declarations to keep working on new PHP versions is a different
matter.

Regards,
Nikita

--000000000000425f2d05ca723c2f--