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

This is only part of a thread. view whole thread
  116382
November 15, 2021 17:20 larry@garfieldtech.com ("Larry Garfield")
On Mon, Nov 15, 2021, at 10:52 AM, Rowan Tommins wrote:
> On 15/11/2021 16:23, Andreas Heigl wrote: >> One thing, that can verify the correct working of properties, whether >> that is dynamic or static ones, is testing. > > > Can it? I can't actually see how that would work, without also having a > way to detect when dynamic properties were accessed, which brings us > full circle. Also: > > >> So the mistakes-part would be easy to handle. > > If you are working with a million lines of code going back 20 years, > "write tests for everything" is not "easy to handle"; it's a long-term > ambition which you chip away at when priorities allow. > > "Logging all deprecations and warnings on a production workload", > however, *is* easy to handle. Diagnostic messages in logs are the *only* > way this behaviour will be spotted in old code. > > >> What I am still missing is the differentiation between "everything is >> strict and you have to explicitly opt-in to make it dynamic" and an >> "everything is dynamic and you can use a marker to mark this >> explicitly an intended behaviour". That would allow users to mark a >> class explicitly to use dynamic features even though it would make no >> difference code-wise. > > > I'm not sure what you mean by that. Do you mean, create the attribute, > but don't actually do anything with it? Would the plan be to deprecate > later? Never remove at all?
A possible idea to help make this transition (which I do support) more gradual: Instead of an "allow" attribute, introduce a boolean flag attribute. #[DynamicProperties(true)] class Beep {} The attribute marks whether dynamic properties are enabled or disabled via boolean. If disabled, then they're an error if used. 8.2: Introduce the attribute, with a default of TRUE. Exactly zero code breaks, but people can start adding the attribute now. That means people who want to lock-out dynamic properties can do so starting now, and those cases that do need them (or can't easily get away from them) can be flagged far in advance. 8.something: Change the default to FALSE. Using dynamic properties when false throws a deprecation, not an error. People have had some number of years to add the attribute either direction if desired. 9.0: Change the deprecation to an error, if the attribute is set false (which by now is the default) and dynamic properties are used. Sometime in the future: Setting the attribute to true triggers a deprecation. Sometime in the future: Remove dynamic properties entirely, and the attribute along with it. People can use __get/__set if they really need. This still gets us to the same place in the end, but introduces another stage along the way to make the transition more gradual. We can debate which version the default should flip in, I don't much mind. That could even wait for 9 if we want to be really really gradual about it. Meanwhile, IDEs and code templates can start including the attribute set to false by default on any new classes that get written, just like they have done for years with strict types. Nikita, would that be feasible? Matthew, Derick, would that be satisfactory? --Larry Garfield
  116427
November 16, 2021 22:10 pollita@php.net (Sara Golemon)
On Mon, Nov 15, 2021 at 11:21 AM Larry Garfield <larry@garfieldtech.com>
wrote:

> A possible idea to help make this transition (which I do support) more > gradual: > > Instead of an "allow" attribute, introduce a boolean flag attribute. > > #[DynamicProperties(true)] > class Beep {} > > The attribute marks whether dynamic properties are enabled or disabled via > boolean. If disabled, then they're an error if used. > > 8.2: Introduce the attribute, with a default of TRUE. Exactly zero code > breaks, but people can start adding the attribute now. That means people > who want to lock-out dynamic properties can do so starting now, and those > cases that do need them (or can't easily get away from them) can be flagged > far in advance. > 8.something: Change the default to FALSE. Using dynamic properties when > false throws a deprecation, not an error. People have had some number of > years to add the attribute either direction if desired. >
This is exactly what Nikita is proposing, except that instead of the attribute being introduced in PHP 8.2, he's proposing to introduce it EARLIER. Roughly PHP 2 sort of timeframe. This is because attributes are forward compatible by design and developers can already start adding #[AllowDynamicProperties] to their code NOW. Even their code that was written to run cleanly on PHP 5.6 because users are terrible at upgrading their servers despite a 2x performance increase. The point is, we don't need 8.2 to be a soft-launch before deprecation because precisely nobody is prevented from adding this attribute preemptively. -Sara
  116431
November 16, 2021 23:55 larry@garfieldtech.com ("Larry Garfield")
On Tue, Nov 16, 2021, at 4:10 PM, Sara Golemon wrote:
> On Mon, Nov 15, 2021 at 11:21 AM Larry Garfield <larry@garfieldtech.com> > wrote: > >> A possible idea to help make this transition (which I do support) more >> gradual: >> >> Instead of an "allow" attribute, introduce a boolean flag attribute. >> >> #[DynamicProperties(true)] >> class Beep {} >> >> The attribute marks whether dynamic properties are enabled or disabled via >> boolean. If disabled, then they're an error if used. >> >> 8.2: Introduce the attribute, with a default of TRUE. Exactly zero code >> breaks, but people can start adding the attribute now. That means people >> who want to lock-out dynamic properties can do so starting now, and those >> cases that do need them (or can't easily get away from them) can be flagged >> far in advance. >> 8.something: Change the default to FALSE. Using dynamic properties when >> false throws a deprecation, not an error. People have had some number of >> years to add the attribute either direction if desired. >> > > This is exactly what Nikita is proposing, except that instead of the > attribute being introduced in PHP 8.2, he's proposing to introduce it > EARLIER. Roughly PHP 2 sort of timeframe. > > This is because attributes are forward compatible by design and developers > can already start adding #[AllowDynamicProperties] to their code NOW. Even > their code that was written to run cleanly on PHP 5.6 because users are > terrible at upgrading their servers despite a 2x performance increase. > > > The point is, we don't need 8.2 to be a soft-launch before deprecation > because precisely nobody is prevented from adding this attribute > preemptively. > > -Sara
It's not *quite* the same, although your point about preemptive attributes is valid. 1. If we adopt the RFC right now as-is, the market has ~12 months to add the attribute. If we instead have a default-true flag that changes to default false in the future, it means at minimum 24 months in which to add the attribute to opt-in to dynamic properties. 2. The RFC as-is has no way for developers to opt-in to actively rejecting dynamic properties. They'll get a deprecation, but... we're shouting from the rooftops that deprecations shouldn't be a big red warning, so if you want a big red warning you can't get that until PHP 9. With the flag attribute, developers could opt into "please slap me across the face if I use dynamic properties by accident" in ~12 months when 8.2 ships, rather than waiting 36-48 months until the likely PHP 9 release. So it gives the nitpickers what they want sooner, and gives the old-codies more time to get their ducks in a row. --Larry Garfield
  116433
November 17, 2021 00:45 claude.pache@gmail.com (Claude Pache)
> Le 17 nov. 2021 à 00:56, Larry Garfield <larry@garfieldtech.com> a écrit : > > They'll get a deprecation, but... we're shouting from the rooftops that deprecations shouldn't be a big red warning, so if you want a big red warning you can't get that until PHP 9.
Deprecation notices *are* big red warnings. They mean: the feature *will* be removed or altered in the next major version. The beauty of deprecation notices is that they warn you about future breakage without actually breaking anything. There is no need to invent some new sophisticated way, but only to learn to use correctly the existing one. —Claude
  116434
November 17, 2021 01:12 pollita@php.net (Sara Golemon)
On Tue, Nov 16, 2021 at 5:55 PM Larry Garfield <larry@garfieldtech.com>
wrote:

> 1. If we adopt the RFC right now as-is, the market has ~12 months to add > the attribute. If we instead have a default-true flag that changes to > default false in the future, it means at minimum 24 months in which to add > the attribute to opt-in to dynamic properties. > > Okay sure, but that's an argument about lead time, not about
implementation. If we agree on targeting 9.0 for this becoming an error (and I think we do), then the argument that's left is whether users get notified as early as 8.2, or if they only get notified as of 8.3. Personally, I want to give users MORE lead time, but having the deprecation warnings come up sooner, rather than later. Assuming 9.0 comes out in late 2025 (guesstimate based on the cadence of 7.x), then including the deprecation with 8.2 (released in late 2022) will give users three years to attach an attribute where needed. If we delay the deprecation notice until 8.3 (released in late 2023), then they only have two years. I know this is the opposite end of lead time than you're talking about (you want max lead time before a deprecation notice even shows up), and here's why you're wrong: Most users don't pay attention to internals. That extra year you give them until notices show up will be wasted in ignorance as they don't know they have any work to do at all. All you will have done is robbed them of a year to implement the escape hatch (though let's be honest, they don't even need ONE year to do it). The only developers paying attention to internals@ traffic are the ones who have literally already added this attribute to their code bases in anticipation of this change.
> 2. The RFC as-is has no way for developers to opt-in to actively rejecting > dynamic properties. They'll get a deprecation, but... we're shouting from > the rooftops that deprecations shouldn't be a big red warning, so if you > want a big red warning you can't get that until PHP 9. With the flag > attribute, developers could opt into "please slap me across the face if I > use dynamic properties by accident" in ~12 months when 8.2 ships, rather > than waiting 36-48 months until the likely PHP 9 release. > > Your premise is that, since deprecation warnings will be ignored, we should
increase the verbosity level of the warning, but only for people who clearly know that a problem exists and can opt in to it? I'm sorry, but I don't track the logic of that. -Sara