Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:116371
MIME-Version: 1.0
References: <CAF+90c9_qu=PiTS81J2mesPfJdz7n5nSi3qoLNLjhEGN-aTytA@mail.gmail.com>
 <CAF+90c9XwAwbA60NQgnu5btiuE_Y9n8eASnVi8TH65jK3A2C8w@mail.gmail.com>
 <alpine.DEB.2.23.453.2111150948120.4221@singlemalt.home.derickrethans.nl> <371ca983-2b07-ae39-3629-49cf7ff8ee64@heigl.org>
In-Reply-To: <371ca983-2b07-ae39-3629-49cf7ff8ee64@heigl.org>
Date: Mon, 15 Nov 2021 14:35:35 +0100
Message-ID: <CAEPJdum6eU+xdZrojybF-FY2Hb6eHtxBifBhJPLmE1wn8MDqJg@mail.gmail.com>
To: Andreas Heigl <andreas@heigl.org>
Cc: PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000de4b5205d0d3e292"
Subject: Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
From: kontakt@beberlei.de (Benjamin Eberlei)

--000000000000de4b5205d0d3e292
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, Nov 15, 2021 at 1:52 PM Andreas Heigl <andreas@heigl.org> wrote:

> Hea all.
>
> On 15.11.21 10:52, Derick Rethans wrote:
> > Dear Internals,
> >
> > On Wed, 10 Nov 2021, Nikita Popov wrote:
> >
> >> On Wed, Aug 25, 2021 at 12:02 PM Nikita Popov <nikita.ppv@gmail.com>
> wrote:
> >>
> >>> 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 warnings with a three-line diff), but may
> >>> have a big impact on legacy code that doesn't declare properties at
> >>> all.
> >>>
> >>
> >> I plan to open voting on this RFC soon. Most of the feedback was
> >> positive, apart from the initial choice of opt-int mechanism, and that
> >> part should be addressed by the switch to the
> >> #[AllowDynamicProperties] attribute.
> >
> > The voting is now open, but I think one thing was not taken into accoun=
t
> > here, the many small changes that push work to maintainers of Open
> > Source library and CI related tools.
> >
> > In the last few years, the release cadence of PHP has increased, which
> > is great for new features. It however has not been helpful to introduce
> > many small deprecations and BC breaks in every single release.
> >
> > This invariably is making maintainers of Open Source anxious, and
> > frustrated as so much work is need to keep things up to date. I know
> > they are *deprecations*, and applications can turn these off, but that'=
s
> > not the case for maintainers of libraries.
> >
> > Before we introduce many more of this into PHP 8.2, I think it would be
> > wise to figure out a way how to:
> >
> > - improve the langauge with new features
> > - keep maintenance cost for open source library and CI tools much lower
> > - come up with a set of guidelines for when it is necessary to introduc=
e
> >    BC breaks and deprecations.
> >
> > I am all for improving the language and making it more feature rich, bu=
t
> > we have not spend enough time considering the impacts to the full
> > ecosystem.
> >
> > I have therefore voted "no" on this RFC, and I hope you will too.
> >
> > cheers,
> > Derick
>
> After some thoughs on this RFC I have reverted my original vote and
> voted "No" due to several reasons.
>
> For one thing it is not clear to me what the benefits are. Yes: The
> language evolution RFC talks about "Forbidding dynamic object
> properties" but it also specifies that "there is also a lot of old code
> that does not declare properties, so this needs to be opt-in"[1].
>
> And as far as I can see from the PR associated with this RFC it will not
> make life easier for the internals team. It is not like there will be
> hundreds of lines code less to maintain. On the contrary. There is more
> code and more logic to maintain [2].
>

This RFCs goal is not to have less code to maintain, but to fix a nasty
class of errors in user errors where they accidently write/read to a
dynamic property due to a typo, instead of accessing the declared one.

True this is a mistake of the RFC not to highlight more.


> So when the only reason for the change is that one line in the RFC ("In
> modern code, this is rarely done intentionally"[3]) then that is not
> enough of a reasoning for me for such a code change that requires a lot
> of existing code to change.
>
> Those that want a cleaner code can already use static code analysis to
> find such issues (if not, I'm sure that there will be some analyzers
> around before PHP8.2 will be around) or write appropriate tests to make
> sure that they do not use undeclared properties.
>

Code that intentionally or unintentionally uses dynamic properties often
does not write each propery explicitly:

$object->$columnName =3D $value;

This cannot be detected by static analysers.

For the case where you explitly write a property name, While static
analysis and IDEs do help detecing these as problems, this class of bugs
happens because you are *not* using an IDE but a text editor like
Vim/Notepad++ where you maybe add a typo to a property name while writing
code.


> While I personally would really like to deprecate dynamic properties I
> believe that it is the wrong thing to do for the language. At least
> given the presented arguments why we should do it.
>
> Cheers
>
> Andreas
>
> PS: Am I the only one missing whether this is a 2/3 or a 50%+1 vote in
> the RFC?
>
>
>
> [1]
>
> https://github.com/nikic/php-rfcs/blob/language-evolution/rfcs/0000-langu=
age-evolution.md#forbidding-dynamic-object-properties
> [2] https://github.com/php/php-src/pull/7571/files
> [3] https://wiki.php.net/rfc/deprecate_dynamic_properties
>
> --
>                                                                ,,,
>                                                               (o o)
> +---------------------------------------------------------ooO-(_)-Ooo-+
> | Andreas Heigl                                                       |
> | mailto:andreas@heigl.org                  N 50=C2=B022'59.5" E 08=C2=B0=
23'58" |
> | https://andreas.heigl.org                                           |
> +---------------------------------------------------------------------+
> | https://hei.gl/appointmentwithandreas                               |
> +---------------------------------------------------------------------+
>

--000000000000de4b5205d0d3e292--