Re: [RFC] Asymmetric Visibility, with readonly

> On 1 Dec 2022, at 08:38, Stephen Reay <php-lists@koalephant.com> wrote: > > > >> On 30 Nov 2022, at 22:09, Larry Garfield <larry@garfieldtech.com> wrote: >> >> On Tue, Nov 29, 2022, at 11:25 PM, Stephen Reay wrote: >> >>> Hi Larry, >>> >>> Thank you for clarifying the setter behaviour in more explicit terms, >>> but I have to say I’m quite disappointed in this continued “use the >>> logic of readonly to apply to something that is explicitly not >>> readonly” - this is even more stark now that you’ve explicitly made >>> them mutually exclusive behaviours. >>> >>> I’m generally very in favour of maintaining consistency, but this seems >>> like it’s using technical consistency as an excuse to justify >>> unintuitive behaviour that breaks consistency in another, much more >>> obvious way. >>> >>> >>> Can you explain why it makes more sense to maintain consistency with >>> “readonly” than it does to maintain consistency with the existing >>> “__set()” behaviour for properties, particularly now that you’ve >>> indicated these features (asymmetric visibility and readonly) are >>> mutually exclusive? >>> >>> While it’s stated multiple times that “readonly” introduced a limited >>> form of asymmetric visibility, and thus this is a continuation, in >>> terms of intuitiveness, the existing __set() rules are very easy to >>> comprehend even with readonly: >>> >>> - if the property is declared as public, __set() is never called; if >>> it’s declared as protected, __set is called when the property is >>> accessed from outside that class or it’s hierarchy. Yes, I know that >>> readonly imposes an implicit visibility difference - but that is >>> essentially an implementation detail, from the point of view of the >>> userland developer, it’s not a clear statement of intended behaviour on >>> their part, expressed through the code as written. >>> >>> For example, with public readonly int $foo it’s quite >>> obvious why >>> __set() isn’t called, using the exiting well-understood logic: it’s a >>> public property. PHP applies a kind of asymmetric visibility to the >>> property behind the scenes, but that isn’t what the developer declared, >>> it’s the implementation. This behaviour matches that of regular, >>> non-readonly fields: when the field is declared public (or has implicit >>> public visibility) __set() is never called. >>> >>> If we make that field protected, __set() will be called when the >>> property is written to from outside the class, regardless of whether >>> it’s readonly or not. >>> >>> >>> What you’re proposing changes that, in a way that is completely >>> unintuitive: when attempting to *write* data to a property that is >>> marked as protected(set), the __set() method will not be called. >>> >>> >>> So please, can you explain to me why consistency with an implementation >>> detail of readonly properties is more important than consistency with >>> declared developer intention for regular properties via the magic >>> setter method? >> >> There's a couple of reasons. >> >> One, and arguably the most important, readonly and aviz being incompatible is, hopefully, a >> temporary situation. There's some fiddly bits to work out design-wise, and based on earlier >> comments in the thread we're going to punt on that for now to avoid that dragging down the >> whole RFC. I believe we absolutely should allow them together in the future (maybe in a later 8.3 >> RFC, maybe a future version, TBD), which means ensuring, now, that they are compatible in the >> future. This approach involves the fewest future BC breaks. >> >> Second, I wouldn't call the current behavior of readonly a mere implementation detail. >> It's weird and unexpected, I'd agree, but only as a side effect of previous design >> decisions, some of which are even older than readonly. But it's an observed behavior that code >> can rely on, and in some cases does. For example: >> >> >> https://peakd.com/hive-168588/@crell/php-tricks-lazy-public-readonly-properties >> >> The "unset a declared property to force it through __get once" is a trick that >> some ORMs use extensively. readonly just inherited that, leading to the current behavior: __set >> depends on the write/set visibility of the property and its settedness. This RFC doesn't >> change anything there. >> >> The alternative would be to have __set called always for a non-public-set property. >> However, that is a place for bugs, as you then can't not have a back-door way to publicly set a >> property even if it's declared private(set). (Or, you have to be very careful in your __set to >> avoid it.) That is both inconsistent with the language today, and error prone. >> >> Finally, we're planning to work in the near-future on property hooks (aka property >> accessors), which would allow per-property custom set routines. That would largely remove the issue >> entirely, as the use of __set would go way down and you'd basically never have to use it with a >> declared property, fancy tricks or no, so this issue would never come up at all. >> >> --Larry Garfield >> >> -- >> PHP Internals - PHP Runtime Development Mailing List >> To unsubscribe, visit: https://www.php.net/unsub.php > > Hi Larry, > > > I think there must be some confusion somewhere, based on some of your comments. > > I’m not suggesting that the “unset to force a **public** property to go through > getter/setter methods” logic should be specifically different. > > I’m suggesting that when the decision is made to call __set or not, the properties **set** > visibility is what should be considered, not it’s **get** visibility. > > Your own comment even describes this behaviour: "leading to the current behavior: __set > depends on the write/set visibility of the property" > > But your RFC says that __set will depend on the **read/get** visibility of the property. > > >> you then can't not have a back-door way to publicly set a property even if it's >> declared private(set). (Or, you have to be very careful in your __set to avoid it.) That is both >> inconsistent with the language today, and error prone. > > If a developer adds a _set() method that can write to a private(set) property, I would expect > that is working exactly as desired, exactly as it does **now** where it’s just a “protected” > property. > > >> we're planning to work in the near-future on property hooks > > That’s great, but I don’t think weird unintuitive behaviour should be added because “well > we’re gonna try and solve this through something else later” > > > > Cheers > > Stephen > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > Apologies, I meant “where it’s just a private property” rather than “just a protected property” in the second to last sentence.