Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties

This is only part of a thread. view whole thread
  116371
November 15, 2021 13:35 kontakt@beberlei.de (Benjamin Eberlei)
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 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 account > > 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 introduce > > BC breaks and deprecations. > > > > I am all for improving the language and making it more feature rich, but > > 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 = $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-language-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°22'59.5" E 08°23'58" | > | https://andreas.heigl.org | > +---------------------------------------------------------------------+ > | https://hei.gl/appointmentwithandreas | > +---------------------------------------------------------------------+ >