Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:118565
Feedback-ID: i8414410d:Fastmail
User-Agent: Cyrus-JMAP/3.7.0-alpha0-927-gf4c98c8499-fm-20220826.002-gf4c98c84
Mime-Version: 1.0
Message-ID: <c20b9aae-fbbf-484e-b73b-b370e4783ada@www.fastmail.com>
In-Reply-To: 
 <CAH5C8xU=eukkqn+PToG4-JT0vqjChhkY=kd_4t_ki9mXu1UMqQ@mail.gmail.com>
References: 
 <CAOWwgpk7uAkZD_CH5inbm8Ng8iVL5SZsz59Mpzgpc_iG42ei8Q@mail.gmail.com>
 <CAH5C8xU=eukkqn+PToG4-JT0vqjChhkY=kd_4t_ki9mXu1UMqQ@mail.gmail.com>
Date: Mon, 05 Sep 2022 09:40:26 -0500
To: "php internals" <internals@lists.php.net>
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Re: Issues with readonly classes
From: larry@garfieldtech.com ("Larry Garfield")

On Mon, Sep 5, 2022, at 4:01 AM, M=C3=A1t=C3=A9 Kocsis wrote:

> 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-c=
ases
> 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 w=
ith
> forbidding the extension of readonly classes by non-readonly ones so t=
hat
> the invariants of the parent (no dynamic or mutable properties are all=
owed)
> 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 wonde=
ring
> 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 t=
he `readonly` keyword) is that you can store a reference to it without h=
aving 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 n=
ot 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 ex=
tra 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";=20
  }
}

class FancyPerson extends Person {
  public string $middle =3D '';

  public function setMiddle(string $mid) {=20
    $this->middle =3D $mid;=20
  }

  public function fullName() {=20
    return "$this->first $this-middle $this->last";=20
  }
}

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 idempote=
nt, and thus Display::hello() is idempotent.  But if you pass a FancyPer=
son 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 quest=
ion.  And to be fair, the same risk exists for conventionally-readonly c=
lasses 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 bro=
ken.  But that's the case where a non-readonly child of a readonly paren=
t can cause problems.

Personally I'm undecided at the moment whether or not it should be allow=
ed.  I'm sympathetic to the "it's easier to disallow and allow later tha=
n vice versa" argument, but still not sure where I stand.  The above at =
least gives a concrete example of where the problem would be.

--Larry Garfield