Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:116362
MIME-Version: 1.0
References: <CAF+90c9_qu=PiTS81J2mesPfJdz7n5nSi3qoLNLjhEGN-aTytA@mail.gmail.com>
 <CAOWwgp=MhJDzc2qmZ+_iPJ11mzZqHOk_FHAHpGTxFBWp7q_RYQ@mail.gmail.com>
In-Reply-To: <CAOWwgp=MhJDzc2qmZ+_iPJ11mzZqHOk_FHAHpGTxFBWp7q_RYQ@mail.gmail.com>
Date: Mon, 15 Nov 2021 12:23:56 +0100
Message-ID: <CAEPJdunmqg8Q5ZwZqS2mJm9kts5SV96HXj+AoyKEQB-sf5NA_A@mail.gmail.com>
To: Nicolas Grekas <nicolas.grekas+php@gmail.com>
Cc: Nikita Popov <nikita.ppv@gmail.com>, PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000000adfa405d0d20c93"
Subject: Re: [PHP-DEV] [RFC] Deprecate dynamic properties
From: kontakt@beberlei.de (Benjamin Eberlei)

--0000000000000adfa405d0d20c93
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, Nov 15, 2021 at 11:26 AM Nicolas Grekas <
nicolas.grekas+php@gmail.com> wrote:

> Hi Nikita, hi everybody,
>
> Le mer. 25 ao=C3=BBt 2021 =C3=A0 12:03, Nikita Popov <nikita.ppv@gmail.co=
m> 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 mode=
rn
> > code (e.g. in Symfony I could fix the vast majority of deprecation
> warnings
> > 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.
>

Its not so true from my POV that static analysis can avoid having this
deprecation:

1. static analysis does not work for dynamic assignments,

$object =3D new SomeDataObject();
$row =3D $pdo->fetch();
foreach ($row as $column =3D> $value) {
    $object->$column =3D $value;
}

arguably this is one of the important use cases this deprecation fixes.

A second example of this is when doing deserialization into an object from
JSON or XML:

$object =3D new SomeDataObject();
$objectPayload =3D json_decode($input, true);
foreach ($objectPayload as $prop =3D> $value) {
    $object->$prop =3D $value;
}

This doesn't apply sole to user input where maybe more validation of input
is necessary, but also for mapping config files to an object.

All this kind of generic code cannot be statically analysed, but this
deprecation and removal has the most value in exactly that use-case.


>
> 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
>

--0000000000000adfa405d0d20c93--