Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:119037
MIME-Version: 1.0
References: <CAH5C8xUd4ueJ62pR6vT2haz9rFgrgspJQui1rPCqTneupD+tsQ@mail.gmail.com>
 <CADyq6sLFZpM1A+V-aa3dWjY1mL=65e-kxeiyBHABNPZR2GfpKQ@mail.gmail.com>
In-Reply-To: <CADyq6sLFZpM1A+V-aa3dWjY1mL=65e-kxeiyBHABNPZR2GfpKQ@mail.gmail.com>
Date: Sat, 26 Nov 2022 22:56:28 +0000
Message-ID: <CAFPFaMKc-PQD8YD6yDWPrMONcJdYqFCXgyJZuZZBVsT7VSy89A@mail.gmail.com>
To: Marco Pivetta <ocramius@gmail.com>
Cc: =?UTF-8?B?TcOhdMOpIEtvY3Npcw==?= <kocsismate90@gmail.com>, 
	PHP Internals List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000001e25cc05ee678dc0"
Subject: Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
From: george.banyard@gmail.com ("G. P. B.")

--0000000000001e25cc05ee678dc0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, 17 Nov 2022 at 14:47, Marco Pivetta <ocramius@gmail.com> wrote:

> Hey M=C3=A1t=C3=A9,
>
> On Tue, 15 Nov 2022 at 07:30, M=C3=A1t=C3=A9 Kocsis <kocsismate90@gmail.c=
om> wrote:
>
> > Hi Everyone,
> >
> > Following Nicolas' thread about "Issues with readonly classes" (
> > https://externals.io/message/118554), we created an RFC to fix two
> issues
> > with the readonly behavior: https://wiki.php.net/rfc/readonly_amendment=
s
> >
> >
> Since I was reviewing https://github.com/lcobucci/jwt/pull/979 today, I
> had
> to put the "why immutability is an LSP requirement" in examples.
>
> As explained in
> https://github.com/lcobucci/jwt/pull/979#discussion_r1025280053, a
> consumer
> relying on a `readonly` implementation of a class probably also expects
> replacement implementations as read-only.
> I am aware that we lack the `readonly` marker at interface level (would b=
e
> really neat), but the practical example is as follows:
>

>
> ```php
> <?php
>
> /* readonly */ class ImmutableCounter {
>     public function __construct(private readonly int $count) {}
>     public function add1(): self { return new self($this->count + 1); }
>     public function value(): int { return $this->count; }
> }
>
> // assuming the proposed RFC is in place, this is now possible:
> class MutableCounter extends ImmutableCounter {
>     public function __construct(private int $count) {}
>     public function add1(): self { return new self(++$this->count); }
>     public function value(): int { return $this->count; }
> }
>
> $counter1 =3D new ImmutableCounter(0);
> $counter2 =3D $counter1->add1();
> $counter3 =3D $counter2->add1();
>
> var_dump([$counter1->value(), $counter2->value(), $counter3->value()]);
>
> $mutableCounter1 =3D new MutableCounter(0);
> $mutableCounter2 =3D $mutableCounter1->add1();
> $mutableCounter3 =3D $mutableCounter2->add1();
>
> var_dump([$mutableCounter1->value(), $mutableCounter2->value(),
> $mutableCounter3->value()]);
> ```
>
> This prints ( https://3v4l.org/IDhRY )
>
> ```
> array(4) {
>   [0]=3D>
>   int(0)
>   [1]=3D>
>   int(1)
>   [2]=3D>
>   int(2)
> }
> array(4) {
>   [0]=3D>
>   int(1)
>   [1]=3D>
>   int(2)
>   [2]=3D>
>   int(2)
> }
> ```
>
> I took a simplified example on purpose, just to demonstrate the problem
> with `readonly` disappearing in child classes.
>
> That's really really confusing, buggy, and, from a consumer PoV, broken
> (LSP violation too, transitively).
>

Removing the readonly restriction for a child class is *not* an LSP
violation.
As all the invariants will be maintained in the child classes (i.e. all the
properties of the parent class remain readonly).
Moreover, a consumer of the base class can only rely on the API defined by
said class, as such it will not be aware of any mutable properties defined
by a child class.

LSP is really not about the class hierarchy/subtypes itself, but the
relation of input data and output data of the operations allowed on a type
(i.e. method calls or property access), and because readonly is not the
same as immutability there is no theoretical issue.

Now, I can agree that it can be slightly surprising that a class which has
a writeable property extends from one which only has readonly ones, but one
can already immitate this by declaring every property readonly but not the
class.

Best regards,

George P. Banyard

--0000000000001e25cc05ee678dc0--