Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:116360
MIME-Version: 1.0
References: <CAF+90c9_qu=PiTS81J2mesPfJdz7n5nSi3qoLNLjhEGN-aTytA@mail.gmail.com>
In-Reply-To: <CAF+90c9_qu=PiTS81J2mesPfJdz7n5nSi3qoLNLjhEGN-aTytA@mail.gmail.com>
Date: Mon, 15 Nov 2021 11:26:20 +0100
Message-ID: <CAOWwgp=MhJDzc2qmZ+_iPJ11mzZqHOk_FHAHpGTxFBWp7q_RYQ@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000015ee1d05d0d13e86"
Subject: Re: [PHP-DEV] [RFC] Deprecate dynamic properties
From: nicolas.grekas+php@gmail.com (Nicolas Grekas)

--00000000000015ee1d05d0d13e86
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Nikita, hi everybody,

Le mer. 25 ao=C3=BBt 2021 =C3=A0 12:03, Nikita Popov <nikita.ppv@gmail.com>=
 a =C3=A9crit :

> Hi internals,
>
> 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 has been discussed in various forms in the past, e.g. in
> https://wiki.php.net/rfc/locked-classes as a class modifier and
> https://wiki.php.net/rfc/namespace_scoped_declares /
>
> https://github.com/nikic/php-rfcs/blob/language-evolution/rfcs/0000-langu=
age-evolution.md
> as a declare directive.
>
> This RFC takes the more direct route of deprecating this functionality
> entirely. I expect that this will have relatively little impact on modern
> code (e.g. in Symfony I could fix the vast majority of deprecation warnin=
gs
> with a three-line diff), but may have a big impact on legacy code that
> doesn't declare properties at all.
>

Thanks for the RFC, it makes sense to me and I support the move.

Since Symfony is mentioned in the RFC, I thought you might want to know
about this PR, that removes dynamic properties from the Symfony codebase:
https://github.com/symfony/symfony/pull/44037/files

What Nikita describes in the RFC is correct: declaring+unsetting the
"groups" property works.
There's just one more thing I had to do; I also had to replace two calls to
property_exists:

-        if (!property_exists($this, 'groups')) {
+        if (!isset(((array) $this)['groups'])) {

The rest are test cases where we've been lazily accepting fixtures with
undeclared properties. No big deal, and I'm happy the engine might soon
help us get a bit stricter in this regard.

I read that some think that this PR is not required because static
analysers (SA) can report when a dynamic property is used. Although that's
correct, I think it would be detrimental to PHP as a language if SA tools
(or any tools actually) were a requirement to code in the safe-n-modern
way. You should not have to install any complex safeguarding tooling
infrastructure to start coding; both for newcomers, but also for
no-so-new-comers.

About the discussion related to deprecations. I've yet to see a better
reporting system than the current one.
It's true that too many userland error handlers are throwing instead of
collecting/logging/skipping deprecations.
But these can be fixed (and many are being fixed these days, which is nice!=
)

Cheers,
Nicolas

--00000000000015ee1d05d0d13e86--