Allow hooks in `readonly` promoted properties

1 month ago by Nick — view source

unread

Dear Internals,

I am sending my first message here. Thank you all for your hard work!

PHP has evolved amazingly over the past few years; I love it.
Promoted properties allow us to write very neat value objects, with almost no boilerplate.
Now, with property hooks, we can eliminate even more boilerplate.

However, there is a bummer.
When we want to use a set hook to add a simple mapper or validator, we are have to deal with asymmetric visibility, and refactor the whole class.
Here in user land, I don't see a clear reason for that.
Ideally, I would simply like to handle what will be set without having to deal with the general visibility of the class or its properties.

Currently, instead of:

final readonly class Entry
{
    public function __construct(
        public string $word,
        public string $slug,
        public string $file,
        public string $path,
        public array $lemmas {
            set(array $value) => array_map(static function (Lemma|array $lemma): Lemma {
                return $lemma instanceof Lemma ? $lemma : new Lemma(...$lemma);
            }, $value);
        },
    ) {}
}

we must refactor our entire class and will arrive at:

final class Entry // no more readonly
{
    public function __construct(
        public readonly string $word, // added readonly
        public readonly string $slug, // added readonly
        public readonly string $file, // added readonly
        public readonly string $path, // added readonly
        private(set) array $lemmas {  // added visibility
            set(array $value) => array_map(static function (Lemma|array $lemma): Lemma {
                return $lemma instanceof Lemma ? $lemma : new Lemma(...$lemma);
            }, $value);
        },
    ) {}
}

The downsides are:

the class can no longer be readonly
each property must be set to readonly
hooked properties must use private(set)

There are many unrelated things we need to think about just to use a set hook.
The example object is tiny and already then feels so much denser.
Think of a bigger object if you like. For each added property, we once again must add the readonly keyword.
Unnecessary noise, IMHO.

Personally, this puts me off adopting property hooks -- because right now, a good old factory method is, subjectively, more clean/clear.

I believe promoted properties should allowed for readonly properties and in readonly classes.
This would help us to avoid the unnecessary boilerplate like outlined above.

That said, I would greatly appreciate if internals could explore to allow `readonly` for hooks in promoted properties in 8.5.

Cheers & thanks,
Nick

1 month ago by Larry Garfield — view source

unread

Dear Internals,

I am sending my first message here. Thank you all for your hard work!

Greetings.

I believe promoted properties should allowed for readonly properties
and in readonly classes. This would help us to avoid the unnecessary
boilerplate like outlined above.

That said, I would greatly appreciate if internals could explore to
allow readonly for hooks in promoted properties in 8.5.--Cheers &
thanks,Nick

This was discussed heavily in the design and discussion phase for hooks. The main problem is that the expectations for readonly and the expectations for hooks don't always align. For example, if a virtual property has a get-hook, can it be readonly? We cannot guarantee that the property will always return the same value, but that is rather the expectation of readonly.

Given how large and complex the RFC was already, we collectively decided to punt on that question until later. Ilija and I did tepidly propose loosening the restriction slightly:

https://wiki.php.net/rfc/readonly_hooks

Though we've not gotten back to it as we've both been busy and there hasn't been a ton of calls for it, and it likely still needs to be tweaked some.

Another possibility might be to have the readonly marker on a class just skip applying to virtual properties. So you cannot specify it explicitly, but a class-level readonly no longer causes a conflict. I don't know how viable that is off hand.

--Larry Garfield

1 month ago by Nick — view source

unread

Hey Larry,

Thanks for your answer.

This was discussed heavily in the design and discussion phase for hooks. The main problem is that the expectations for readonly and the expectations for hooks don't always align. For example, if a virtual property has a get-hook, can it be readonly? We cannot guarantee that the property will always return the same value, but that is rather the expectation of readonly.

For me, readonly is relevant for set operations. It gives me certainty that a value can only ever be set once. I don’t see how it must be related to retrieving a value.

Given how large and complex the RFC was already, we collectively decided to punt on that question until later. Ilija and I did tepidly propose loosening the restriction slightly:

https://wiki.php.net/rfc/readonly_hooks

Though we've not gotten back to it as we've both been busy and there hasn't been a ton of calls for it, and it likely still needs to be tweaked some.

Yeah, so here I am, with a call after the feature was in the wild for a while. Hoping that others might chime in with their own experiences and opinions.

I wasn’t aware of this follow up RFC exists. Excited to see it!

Limiting it to backed properties feels sensible. I guess it still would solve the majority of use-cases already. Especially around dumb value objects.

I would like to add, personally, I don’t believe the above is dumb:

class Dumb {
    public readonly int $value { get => $this->value * random_int(1, 100); }
}

I think it is a legitimate use-case.
Why wouldn’t a readonly property allow us to format, cast or however mutate a “only once written value” on consumption? It will not change the underlying value.
If it makes things easier for us, allow it. It’s not like this is some hidden implicit behaviour. We consciously must add the extra code, hence expect the output to be changed accordingly.

So, I would love to see this RFC to be implemented.
Maybe you want to move it to discussion? Then my separate thread here would be obsolete.

Another possibility might be to have the readonly marker on a class just skip applying to virtual properties. So you cannot specify it explicitly, but a class-level readonly no longer causes a conflict. I don't know how viable that is off hand.

I don’t have a strong opinion on virtual properties. At first glance this feels too implicit to me.
Given their complexity, rather not allow it for virtual properties (for now). We can use getter methods for that.
The follow up RFC is good as is and a low-hanging fruit with high positive impact, if you ask me.

Cheers & thanks,
Nick

1 month ago by Larry Garfield — view source

unread

I would like to add, personally, I don’t believe the above is dumb:
class Dumb {
public readonly int $value { get => $this->value * random_int(1, 100); }
}
I think it is a legitimate use-case.
Why wouldn’t a readonly property allow us to format, cast or however
mutate a “only once written value” on consumption? It will not change
the underlying value.
If it makes things easier for us, allow it. It’s not like this is some
hidden implicit behaviour. We consciously must add the extra code,
hence expect the output to be changed accordingly.

It's about expectation setting. If you see a property marked readonly, it's reasonable to expect this to be true:

$foo->bar == $foo->bar;

For a traditional field (pre-hooks), this would be trivially true. With hooks, it may or may not be. Saying "well, that assumption doesn't hold anymore, deal" is certainly an option, but it's not an option we wanted to pursue as part of the larger RFC. But that is certainly a direction we could take.

So, I would love to see this RFC to be implemented.
Maybe you want to move it to discussion? Then my separate thread here
would be obsolete.

I believe at the moment that RFC text is all there is. :-) I don't know that it's worth opening a discussion without at least a mostly-done implementation. Also, Ilija is rather busy on other tasks at the moment, as am I. (Unless someone else wants to jump in to implement it, which would be fine.)

--Larry Garfield

1 month ago by Nick — view source

unread

Hey all,

It's about expectation setting. If you see a property marked readonly, it's reasonable to expect this to be true:

$foo->bar == $foo->bar;

For a traditional field (pre-hooks), this would be trivially true. With hooks, it may or may not be. Saying "well, that assumption doesn't hold anymore, deal" is certainly an option, but it's not an option we wanted to pursue as part of the larger RFC. But that is certainly a direction we could take.

Larry, I understand now that you in fact explicitly talk about random_int().
Previously, I did not. I was more on the “manipulating in general” meta level.

Fair. If someone really wants to add random_int(): "well, that assumption doesn't hold anymore, deal” from my side.

So, I would love to see this RFC to be implemented.
Maybe you want to move it to discussion? Then my separate thread here
would be obsolete.

I believe at the moment that RFC text is all there is. :-) I don't know that it's worth opening a discussion without at least a mostly-done implementation. Also, Ilija is rather busy on other tasks at the moment, as am I. (Unless someone else wants to jump in to implement it, which would be fine.)

People often say “you can just do things”. So I did, and tried to contribute the code for your existing RFC text:

https://github.com/php/php-src/pull/18757

Can it really be such a little change? I’d appreciate feedback from people more experienced than I am. Thanks!

Cheers,
Nick

1 month ago by tim@bastelstu.be — view source

unread

Am 2025-06-04 14:19, schrieb Nick:

Fair. If someone really wants to add random_int(): "well, that
assumption doesn't hold anymore, deal” from my side.

Semantically once you involved inheritance it isn't that easy. It is
allowed to override an “unhooked” property with a hooked property and in
the “Readonly Amendments” RFC we already decided that inheriting from a
readonly class by a non-readonly class should not be valid.

So if you would be allowed to override a readonly unhooked property with
a hooked property that has a get hook that is not a pure function, you
would make the property effectively mutable, which is something that
users of the class can't expect. It violates the history property of the
Liskov substitution principle.

Making this legal might also inhibit engine optimization. Currently if
you know that a property is readonly you can theoretically optimize:

 if ($object->foo !== null) {
     do_something($object->foo);
 }

into:

 if (($foo = $object->foo) !== null) {
     do_something($foo);
 }

to avoid reading $object->foo twice, which for example would need to
check visibility twice.

I believe at the moment that RFC text is all there is. :-) I don't
know that it's worth opening a discussion without at least a
mostly-done implementation. Also, Ilija is rather busy on other tasks
at the moment, as am I. (Unless someone else wants to jump in to
implement it, which would be fine.)

People often say “you can just do things”. So I did, and tried to
contribute the code for your existing RFC text:

https://github.com/php/php-src/pull/18757

Can it really be such a little change? I’d appreciate feedback from
people more experienced than I am. Thanks!

Your test cases really only scratch the surface of what should be
tested. You are basically just verifying that it compiles. In fact you
are not even testing that reassigning the property is disallowed,
because the test fails due to a visibility error. In fact it appears
that the readonly check comes before the visibility check, which would
imply that the readonly doesn't have an effect: https://3v4l.org/nqgpL

Best regards
Tim Düsterhus

1 month ago by Nick — view source

unread

Hey Tim,

Hi

Am 2025-06-04 14:19, schrieb Nick:

Fair. If someone really wants to add random_int(): "well, that assumption doesn't hold anymore, deal” from my side.

Semantically once you involved inheritance it isn't that easy. It is allowed to override an “unhooked” property with a hooked property and in the “Readonly Amendments” RFC we already decided that inheriting from a readonly class by a non-readonly class should not be valid.

So if you would be allowed to override a readonly unhooked property with a hooked property that has a get hook that is not a pure function, you would make the property effectively mutable, which is something that users of the class can't expect. It violates the history property of the Liskov substitution principle.

Making this legal might also inhibit engine optimization. Currently if you know that a property is readonly you can theoretically optimize:

if ($object->foo !== null) {
do_something($object->foo);
}

into:

if (($foo = $object->foo) !== null) {
do_something($foo);
}

to avoid reading $object->foo twice, which for example would need to check visibility twice.

I believe at the moment that RFC text is all there is. :-) I don't know that it's worth opening a discussion without at least a mostly-done implementation. Also, Ilija is rather busy on other tasks at the moment, as am I. (Unless someone else wants to jump in to implement it, which would be fine.)
People often say “you can just do things”. So I did, and tried to contribute the code for your existing RFC text:
https://github.com/php/php-src/pull/18757
Can it really be such a little change? I’d appreciate feedback from people more experienced than I am. Thanks!

Your test cases really only scratch the surface of what should be tested. You are basically just verifying that it compiles. In fact you are not even testing that reassigning the property is disallowed, because the test fails due to a visibility error. In fact it appears that the readonly check comes before the visibility check, which would imply that the readonly doesn't have an effect: https://3v4l.org/nqgpL

https://github.com/php/php-src/pull/18757/files#diff-fdc22521b074231ac6de0df97aeb23a4759ad9b668d3b9ed3fedf8727f5795a4R21

It checks re-assignment, and expects:

Cannot modify protected(set) readonly property Test1::$prop from global scope

It’s tested for readonly props, readonly class and promoted properties.

Correct me, but this is what you are talking about, isn't it? Anything else you are missing?

1 month ago by tim@bastelstu.be — view source

unread

Am 2025-06-04 15:39, schrieb Nick:

Your test cases really only scratch the surface of what should be
tested. You are basically just verifying that it compiles. In fact you
are not even testing that reassigning the property is disallowed,
because the test fails due to a visibility error. In fact it appears
that the readonly check comes before the visibility check, which
would imply that the readonly doesn't have an effect:
https://3v4l.org/nqgpL

https://github.com/php/php-src/pull/18757/files#diff-fdc22521b074231ac6de0df97aeb23a4759ad9b668d3b9ed3fedf8727f5795a4R21

It checks re-assignment, and expects:

Cannot modify protected(set) readonly property Test1::$prop from
global scope

It’s tested for readonly props, readonly class and promoted properties.

Correct me, but this is what you are talking about, isn't it? Anything
else you are missing?

Your link to GitHub is dead (probably due to the force-pushes), but the
tests in the current version of the PR resolved my question by
attempting to re-assign from within the class (such that visibility is
not the cause of the error).

Best regards
Tim Düsterhus

1 month ago by Nick — view source

unread

Hey Tim,

Semantically once you involved inheritance it isn't that easy. It is allowed to override an “unhooked” property with a hooked property and in the “Readonly Amendments” RFC we already decided that inheriting from a readonly class by a non-readonly class should not be valid.

I added tests for this. The behaviour you expect seems confirmed.

So if you would be allowed to override a readonly unhooked property with a hooked property that has a get hook that is not a pure function, you would make the property effectively mutable, which is something that users of the class can't expect. It violates the history property of the Liskov substitution principle.

Making this legal might also inhibit engine optimization. Currently if you know that a property is readonly you can theoretically optimize:

if ($object->foo !== null) {
do_something($object->foo);
}

into:

if (($foo = $object->foo) !== null) {
do_something($foo);
}

to avoid reading $object->foo twice, which for example would need to check visibility twice.

Added Zend/tests/property_hooks/readonly_property_backed_inheritance_3.phpt

Personally, I’d argue nothing unexpected happens here and everything is fair game.
If we overwrite parents from a child it will naturally result in a changed get hook result.
This, however, does not mean the actual class state has changed.

Opinions?

People often say “you can just do things”. So I did, and tried to contribute the code for your existing RFC text:
https://github.com/php/php-src/pull/18757
Can it really be such a little change? I’d appreciate feedback from people more experienced than I am. Thanks!

Your test cases really only scratch the surface of what should be tested. You are basically just verifying that it compiles. In fact you are not even testing that reassigning the property is disallowed, because the test fails due to a visibility error. In fact it appears that the readonly check comes before the visibility check, which would imply that the readonly doesn't have an effect: https://3v4l.org/nqgpL

You are right. I edited the tests accordingly.

—

Cheers,
Nick

1 month ago by tim@bastelstu.be — view source

unread

Am 2025-06-05 08:04, schrieb Nick:

Semantically once you involved inheritance it isn't that easy. It is
allowed to override an “unhooked” property with a hooked property and
in the “Readonly Amendments” RFC we already decided that inheriting
from a readonly class by a non-readonly class should not be valid.

I added tests for this. The behaviour you expect seems confirmed.

I think you might have misunderstood me there. I was not specifically
discussing the inheritance of readonly and non-readonly classes, but the
user’s expectations for the semantics of readonly classes. This
paragraph was intended as an introduction for the following paragraphs.

To be clear: The tests do not confirm my expected behavior, since I
expect a readonly property to never change after successfully reading
it. But that's something for the official RFC discussion.

So if you would be allowed to override a readonly unhooked property
with a hooked property that has a get hook that is not a pure
function, you would make the property effectively mutable, which is
something that users of the class can't expect. It violates the
history property of the Liskov substitution principle.

Making this legal might also inhibit engine optimization. Currently if
you know that a property is readonly you can theoretically optimize:

if ($object->foo !== null) {
do_something($object->foo);
}

into:

if (($foo = $object->foo) !== null) {
do_something($foo);
}

to avoid reading $object->foo twice, which for example would need to
check visibility twice.

Added
Zend/tests/property_hooks/readonly_property_backed_inheritance_3.phpt

Personally, I’d argue nothing unexpected happens here and everything is
fair game.
If we overwrite parents from a child it will naturally result in a
changed get hook result.
This, however, does not mean the actual class state has changed.

I'd argue that the user is not interested in the “actual class state”,
but what they can observe from interacting with the class. I'll probably
discuss this in more detail in the official RFC discussion thread.

Best regards
Tim Düsterhus

1 month ago by Marc Bennewitz — view source

unread

Hi Larry,

I would like to add, personally, I don’t believe the above is dumb:
class Dumb {
public readonly int $value { get => $this->value * random_int(1, 100); }
}
I think it is a legitimate use-case.
Why wouldn’t a readonly property allow us to format, cast or however
mutate a “only once written value” on consumption? It will not change
the underlying value.
If it makes things easier for us, allow it. It’s not like this is some
hidden implicit behaviour. We consciously must add the extra code,
hence expect the output to be changed accordingly.
It's about expectation setting. If you see a property marked readonly, it's reasonable to expect this to be true:
$foo->bar == $foo->bar;

For a traditional field (pre-hooks), this would be trivially true. With hooks, it may or may not be. Saying "well, that assumption doesn't hold anymore, deal" is certainly an option, but it's not an option we wanted to pursue as part of the larger RFC. But that is certainly a direction we could take.

I recently run into this limitation as well and I was under the
impression, that a get property hook allows for replacing getter methods
which isn't fully the case as seen here.

So, I would love to see this RFC to be implemented.
Maybe you want to move it to discussion? Then my separate thread here
would be obsolete.
I believe at the moment that RFC text is all there is. :-) I don't know that it's worth opening a discussion without at least a mostly-done implementation. Also, Ilija is rather busy on other tasks at the moment, as am I. (Unless someone else wants to jump in to implement it, which would be fine.)

I wanted to create an abstract class with a specific property defined to
be readonly as it's supposed to be used as value object. Than in the
implementation I wont to lazy load one of the expensive property values to.

This RFC would resolve that limitation for me :+1 but I still don't get
the logic behind it.

Especially why the following is allowed from the RFC:

readonlyclass LazyProduct{
public Category$category {
get =>$this->category ??= $this->dbApi->loadCategory($this->categoryId);
}
}

but this isn't:

readonlyclass Random{
public int$value {
get => random_int(PHP_INT_MIN, PHP_INT_MAX);
}
}

While considering someone re-writes the above Random class just to make
it work with a backed property:

readonlyclass Random{
public int$value {
get => ($this->value ?? 1) * random_int(PHP_INT_MIN, PHP_INT_MAX);
}
}

From the RFC text I would expect this to be allowed but it doesn't help
anyone. Your described expectation isn't true anymore, the class author
has more work and now it stores a needless value in memory.

Additionally, your expectation isn't true anyway as you noted in your RFC:

On the other hand, there is no shortage of dumb things that people
can do with PHP already. The exact same silliness could be implemented
using |__get|, for instance. ...

--Larry Garfield

Thanks,
Marc

1 month ago by Nick — view source

unread

Hey internals,

I believe at the moment that RFC text is all there is. :-) I don't know that it's worth opening a discussion without at least a mostly-done implementation. Also, Ilija is rather busy on other tasks at the moment, as am I. (Unless someone else wants to jump in to implement it, which would be fine.)

People often say “you can just do things”. So I did, and tried to contribute the code for your existing RFC text:

https://github.com/php/php-src/pull/18757

Would this be expected to work? Can interface properties be declared readonly?
interface Cleaned
{
public readonly string $clean { get; } // has readonly
}

class Something implements Cleaned
{
public function __construct(
public readonly string $clean {
get => trim($this->clean);
}
) {}
}

$u = new Something(' Yoda ');
var_dump($u->clean);
In my current implementation it would throw:

Fatal error: Hooked virtual properties cannot be readonly in

I’d appreciate input here.
Thanks in advance!

Cheers,
Nick

1 month ago by Larry Garfield — view source

unread

Hey internals,

I believe at the moment that RFC text is all there is. :-) I don't know that it's worth opening a discussion without at least a mostly-done implementation. Also, Ilija is rather busy on other tasks at the moment, as am I. (Unless someone else wants to jump in to implement it, which would be fine.)

People often say “you can just do things”. So I did, and tried to contribute the code for your existing RFC text:

https://github.com/php/php-src/pull/18757

Would this be expected to work? Can interface properties be declared readonly?
interface Cleaned
{
public readonly string $clean { get; } // has readonly
}

Interface properties cannot be declared readonly today:

https://3v4l.org/cXgR0

Which I think is correct behavior.

Make sure to include a test based on the "lazy product" example from the RFC text. That's the main sort of use case I'd expect we'd want to enable. :-)

--Larry Garfield

1 month ago by Nick — view source

unread

Hey Larry,

Would this be expected to work? Can interface properties be declared readonly?
interface Cleaned
{
public readonly string $clean { get; } // has readonly
}

Interface properties cannot be declared readonly today:

https://3v4l.org/cXgR0

Which I think is correct behavior.

Noted, thanks.

Make sure to include a test based on the "lazy product" example from the RFC text. That's the main sort of use case I'd expect we'd want to enable. :-)

Added readonly_lazy.phpt

—

Cheers,
Nick

Allow hooks in `readonly` promoted properties

That said, I would greatly appreciate if internals could explore to allow readonly for hooks in promoted properties in 8.5.

That said, I would greatly appreciate if internals could explore to allow `readonly` for hooks in promoted properties in 8.5.