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 <la...@garfieldtech.com> 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
