> On 30 Nov 2022, at 22:09, Larry Garfield <la...@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
