On Sat, Sep 10, 2022, at 4:34 PM, Nicolas Grekas wrote:
> Hello Larry,
>
>
>> Regarding your main question: I understand your problem with readonly
>> > classes, and I'd be happy if we found a solution which fits your
>> use-cases
>> > and keeps consistency for the engine at the same time. To give you more
>> > context about the inheritance related restriction in the RFC: I went with
>> > forbidding the extension of readonly classes by non-readonly ones so that
>> > the invariants of the parent (no dynamic or mutable properties are
>> allowed)
>> > don't occasionally get violated by the child. However, I cannot think
>> about
>> > a proper scenario now where this would cause any problems... I'm
>> wondering
>> > if anyone could come up with one?
>>
>> I don't have a real-world example, but a general pattern. The advantage
>> of a readonly object (either conventionally like PSR-7 or enforced by the
>> `readonly` keyword) is that you can store a reference to it without having
>> to worry about it changing out from under you. That means you can code on
>> the assumption that any data derived from that object is also not subject
>> to change.
>>
>> If you accept a readonly object of type Foo, you would write code around
>> it on that assumption.
>>
>> If you're then passed an object of type Bar extends Foo, which has an
>> extra non-readonly property, that assumption would now be broken. Whether
>> that counts as a Liskov violation is... a bit squishy, I think.
>>
>> Where that might be a problem is a case like this:
>>
>> readonly class Person {
>> public function __construct(public string $first, public string $last) {}
>>
>> public function fullName() {
>> return "$this->first $this->last";
>> }
>> }
>>
>> class FancyPerson extends Person {
>> public string $middle = '';
>>
>> public function setMiddle(string $mid) {
>> $this->middle = $mid;
>> }
>>
>> public function fullName() {
>> return "$this->first $this-middle $this->last";
>> }
>> }
>>
>> class Display {
>> public function __construct(protected Person $person) {}
>>
>> public function hello() {
>> return "Hello " . $this->person->fullName();
>> }
>> }
>>
>> Now, Display assumes Person is readonly, and thus fullName() is
>> idempotent, and thus Display::hello() is idempotent. But if you pass a
>> FancyPerson to Display, then fullName() is not safely idempotent (it may
>> change out from under you) and thus hello() is no longer idempotent.
>>
>> Whether or not that problem actually exists in practice is another
>> question. And to be fair, the same risk exists for conventionally-readonly
>> classes today (PSR-7, PSR-13, etc.), unless they're made final. Arguably
>> the safety signaling of a lexically readonly class is stronger than for a
>> conventionally readonly class, so one would expect that to not be broken.
>> But that's the case where a non-readonly child of a readonly parent can
>> cause problems.
>>
>
> Thanks for constructing this example, that's good food for thoughts.
> Unfortunately, The code following readonly child class shows that this
> safety you describe doesn't exist:
>
> readonly class FancyPerson extends Person {
> private readonly stdClass $middle;
>
> public function setMiddle(string $mid) {
> $this->middle ??= new stdClass;
> $this->middle->name = $mid;
> }
>
> public function fullName() {
> return "$this->first $this-middle $this->last";
> }
> }
Hm. I seem to recall during the discussion of readonly classes someone saying
that object properties of a readonly class had to also be readonly classes,
which would render the above code a compile error. However, I just checked and
that is not in the RFC. Was it removed? Am I imagining things? Anyone else
know what I'm talking about? :-)
--Larry Garfield
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
