On Tue, Jun 29, 2021 at 9:14 AM Nicolas Grekas <nicolas.grekas+...@gmail.com> wrote:
> Le lun. 28 juin 2021 à 18:22, Larry Garfield <la...@garfieldtech.com> a >>> écrit : >>> >>>> On Mon, Jun 28, 2021, at 11:17 AM, Nicolas Grekas wrote: >>>> > > > I'd like to open the discussion on readonly properties: >>>> > > > https://wiki.php.net/rfc/readonly_properties_v2 >>>> > > > >>>> > > > This proposal is similar to the >>>> > > > https://wiki.php.net/rfc/write_once_properties RFC that has been >>>> > > declined >>>> > > > previously. One significant difference is that the new RFC limits >>>> the >>>> > > scope >>>> > > > of initializing assignments. I think a key mistake of the >>>> previous RFC >>>> > > was >>>> > > > the confusing "write-once" framing, which is both technically >>>> correct and >>>> > > > quite irrelevant. >>>> > > > >>>> > > > Please see the rationale section ( >>>> > > > https://wiki.php.net/rfc/readonly_properties_v2#rationale) for >>>> how this >>>> > > > proposal relates to other RFCs and alternatives. >>>> > > > >>>> > > >>>> > > I plan to open voting on this RFC soon. I don't think there's >>>> anything >>>> > > technical left to address here, the discussion mostly comes down to >>>> a value >>>> > > judgement. I think everyone has made their position regarding that >>>> clear... >>>> > > >>>> > >>>> > Actually, we talked off the list about a way to possibly make this >>>> work >>>> > with __clone(): >>>> > >>>> > We could allow __clone to have one argument, the object being cloned. >>>> And >>>> > when the signature declares this argument, then all readonly >>>> properties >>>> > would be set as uninitialized on $this. >>>> > >>>> > A typical __clone function would look like this with readonly >>>> properties: >>>> > function __clone(object $original) >>>> > { >>>> > $this->readonlyProp = clone $original->readonlyProp; >>>> > } >>>> > >>>> > That would turn my vote into a +1 if that could be made to work! >>>> >>>> That sounds like it would support deep cloning, but not with-er >>>> methods. There's no way to provide a changed value. It also would mean a >>>> lot of work on larger objects to transfer across all the properties. I >>>> don't really see what this would add. >>>> >>> >>> Can you elaborate about the lack of support for withers? Having some >>> work to do doesn't look like an issue to me, especially when there is no >>> alternative to compare that too. >>> >> >> I sent that too fast, I agree about withers... :) >> I'm looking for a way to +1 that RFC... >> Any other idea? >> > > And I was too fast agreeing that my proposal to pass the original object > as argument was incompatible with withers. > > I also think that not being compatible with deep cloning is a major issue. > Past trivial cases, the use cases I can think of where I would use readonly > require deep cloning. See eg the Symfony Request object where query params, > headers, and a few others expose state as public objects. > > Here is some code that just works with them: > class C > { > public readonly string $foo; > public readonly string $bar; > > private $skipWhenCloning; > > public function withFoo($foo) > { > $this->skipWhenCloning = 'foo'; > $clone = clone $this; > $clone->foo = $foo; > } > > public function withBar($bar) > { > $this->skipWhenCloning = 'bar'; > $clone = clone $this; > $clone->bar = $bar; > } > > public function __clone(self $original) > { > foreach ($this as $k => $v) { > if ($k !== $this->skipWhenCloning) { > $this->$k = $original->$k; > } > } > $this->skipWhenCloning = $original->skipWhenCloning = null; > } > } > > You might not like the boilerplate, but that just works. > > Can this be considered Nikita? > Well, it's a nifty hack :) I don't think this is the solution we want to encourage though. It requires you pass extra information through a side-channel -- I think I'd rather not use readonly than write that code. Continuing along the same line, one could extend that to "clone with argument" and do something like this: public function __clone(self $original, array $replacements = []) { foreach ($original as $k => $v) { $this->$k = $replacements[$k] ?? $original->$k; } } and then do "clone $this(['bar' => $bar])". In any case, I don't want to include changes to cloning in this proposal -- the topic is related, but also orthogonal to readonly properties. Unfortunately, it will not be possible to get cloning changes into PHP 8.1 anymore, due to feature freeze. It's okay to vote against this if cloning is a deal breaker. In that case I'll probably either work on cloning before re-proposing this, or pivot to asymmetric visibility -- it's not my first preference, but it may be the more pragmatic choice. Cloning is definitely the weak point of this proposal. Regards, Nikita
