Re: [RFC] Asymmetric Visibility, with readonly

On Wed, Nov 30, 2022, at 7:38 PM, Stephen Reay wrote: >>> 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. I think we're talking past each other a bit. :-) The logic described in the RFC is, to the best of out knowledge, what already happens in 8.1/8.2 with readonly. Whether that is good or bad, obvious or intuitive, it's what PHP already does, for better or worse. We view readonly as, effectively, a shorthand for "private(set) write-once" (which is what it is), and want to ensure that future RFCs can do that explicitly so that we can allow public protected(set) readonly in the future. For that to be possible, not changing the existing behavior is a necessity. Changing the behavior described in the RFC right now is a BC break. That's true whether the logic described in the RFC is good or not, because that's how it already works with readonly. Is it weird that __set depends in part on read visibility? Kinda, yeah. But that's already the behavior in 8.1/8.2. We're not changing anything, and this RFC is not the place to break that kind of BC. If someone can demonstrate that the logic described there is not what actually happens now, please let us know, because the goal is to not change any behavior in this regard. Effectively, that whole section is descriptive of PHP today and a comment "and we're not breaking it." --Larry Garfield