Re: [RFC] [Discussion] Readonly class amendments

On Thu, 17 Nov 2022 at 14:47, Marco Pivetta <ocramius@gmail.com> wrote: > Hey Máté, > > On Tue, 15 Nov 2022 at 07:30, Máté Kocsis <kocsismate90@gmail.com> 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_amendments > > > > > 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 be > 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 = new ImmutableCounter(0); > $counter2 = $counter1->add1(); > $counter3 = $counter2->add1(); > > var_dump([$counter1->value(), $counter2->value(), $counter3->value()]); > > $mutableCounter1 = new MutableCounter(0); > $mutableCounter2 = $mutableCounter1->add1(); > $mutableCounter3 = $mutableCounter2->add1(); > > var_dump([$mutableCounter1->value(), $mutableCounter2->value(), > $mutableCounter3->value()]); > ``` > > This prints ( https://3v4l.org/IDhRY ) > > ``` > array(4) { > [0]=> > int(0) > [1]=> > int(1) > [2]=> > int(2) > } > array(4) { > [0]=> > int(1) > [1]=> > int(2) > [2]=> > 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