On Fri, May 31, 2024, at 8:59 PM, Larry Garfield wrote:
> On Fri, May 31, 2024, at 5:45 PM, Claude Pache wrote:
>>> Le 31 mai 2024 à 18:08, Larry Garfield <[email protected]> a écrit :
>>>
>>> However, this also brings up another interesting issue: readonly properties (in 8.3) DO
>>> allow redeclaration, essentially adjusting the property scope (the class that declares it) to make
>>> the visibility check pass.. That is, the definition of the class it is private to changes, which is
>>> different from how inheritance works elsewhere. When the parent writes to the same property, a
>>> special check is needed to verify the two properties are related. All that special casing
>>> effectively means that readonly in 8.4 wouldn't really be "write once +
>>> private(set)", but "write once + private(set) - final", which is... just kinda
>>> screwy. That means our options are:
>>>
>>> * A BC break on readonly (not allowing it to be overridden)
>>> * Make readonly an exception to the implicit final.
>>> * Just don't allow readonly with aviz after all.
>>
>> Another possible option is:
>>
>> * Make readonly be protected(set) by default.
>>
>> That would weaken the originally intended semantics of readonly, but in
>> a compatible and acceptable way?
>>
>> —Claude
>
> Only sort of compatible. It would allow readonly props to be
> redefined, and wouldn't break any existing code, I think... but it
> would mean any time you use readonly, suddenly a child class can come
> along and mess with it out from under you.
>
> In practice that's likely not a common concern, but it is a behavior
> change. I think it's possible (I need to confirm with Ilija), if we
> want that slight BC break, but I don't know if we do.
>
> --Larry Garfield
Ilija and I have been discussing this issue over the last few days. We agree that
private(set) should imply final, as that eliminates a bunch of issues both
implementation-wise and expectation-wise. However, that causes an issue for readonly.
The root issue is that if we say "readonly int $x is really just
private(set) readonly int $x", that runs into the issue of "whelp,
you've just made readonly always final, which is a BC break." So that's no good.
We see a couple of ways to resolve this, presented below in our order of preference.
1. Disallow readonly with aviz => No BC break, and we don't need to define readonly in terms
of private(set). The only really useful combination anyway would be readonly
protected(set), in which case the protected(set) is doing 90% of the work already.
There's few cases where the readonly is truly necessary at that point. Any other oddball edge
cases could be handled by a custom hook.
2. Make readonly implicitly protected(set) unless you explicitly specify
private(set) => Would have the most consistent result, and this is probably the
cleanest in the engine, as readonly private(set) would mean exactly what it says on the
tin, with no inconsistency of "well it's kinda sorta private(set)" as
readonly has now. However, this would be an expectation change, as suddenly all
readonly properties could be written to from a child class. That may be good in some cases but
it's possible some objects could have unexpected behavior if they didn't expect to be
extended. (No existing code will break, but you could now do things to it in a child class that the
author didn't anticipate.)
3. You can't mix readonly with private(set), but can use other
visibilities => No BC break, and we don't need to define readonly in terms of
private(set). However, it means the implicit private(set) of
readonly and an explicit private(set) behave differently (one is final, one is not).
It also unclear if a readonly property can be overridden with readonly
protected(set) only, or also readonly private(set). If the latter, does it
become implicitly final at that point?
4. readonly behaves differently for an explicit (final) and implicit (not-final)
private(set) => No BC break, but it's kinda weird and non-obvious to explain.
It also has the same non-obvious inheritance questions as option 3.
We consider only the first two to be really viable. For simplicity, we'd favor doing option 1,
and if desired option 2 could be presented in the future as its own RFC as that is technically a
behavior change, not just addition, so deserves careful consideration. However, if there is a clear
consensus to go with option 2 now, we're open to that.
--Larry Garfield