[RFC] Readonly properties

Le 04/06/2021 à 17:34, Pierre a écrit :

Le 04/06/2021 à 17:19, Nikita Popov a écrit :

Hi internals,

I'd like to open the discussion on readonly properties:
https://wiki.php.net/rfc/readonly_properties_v2

This proposal is similar to the
https://wiki.php.net/rfc/write_once_properties RFC that has been
declined
previously. One significant difference is that the new RFC limits the
scope
of initializing assignments. I think a key mistake of the previous
RFC was
the confusing "write-once" framing, which is both technically correct
and
quite irrelevant.

Please see the rationale section (
https://wiki.php.net/rfc/readonly_properties_v2#rationale) for how this
proposal relates to other RFCs and alternatives.

Regards,
Nikita

Hello,

I like this RFC, but I have a question:

Specifying an explicit default value on readonly properties is also
not allowed:

Does that mean I wouldn't be allowed to write something such as:

<?php

class Point {

    public function __construct(

        public readonly int $x = 0,

        public readonly int $y = 0,

    ) {}

}

$foo = new Point(x: 12);

?>

Because the readonly+promoted+named combo is very powerful but if you
can't assign defaults it makes it much less interesting.

Regards,

--

Pierre

Please ignore my mail, the answer is just below...

Sorry for the noise.

--

Pierre

Le 04/06/2021 à 17:19, Nikita Popov a écrit :

Hi internals,

I'd like to open the discussion on readonly properties:
https://wiki.php.net/rfc/readonly_properties_v2

This proposal is similar to the
https://wiki.php.net/rfc/write_once_properties RFC that has been
declined
previously. One significant difference is that the new RFC limits the
scope
of initializing assignments. I think a key mistake of the previous RFC
was
the confusing "write-once" framing, which is both technically correct and
quite irrelevant.

Please see the rationale section (
https://wiki.php.net/rfc/readonly_properties_v2#rationale) for how this
proposal relates to other RFCs and alternatives.

Regards,
Nikita

Hello,

If I understood it well you count assignments, one note thought after
re-reading once again, you state:

This variant is not allowed, as the initializing assignment occurs from
outside the class:

Does this mean it'll prevent programmatic object hydration using the
scope-stealing closure pattern ? Such as:

<?php

class SomeClass
{
public readonly int $foo;
}

$bar = 12;

$object = (new
\ReflectionClass('SomeClass'))->newInstanceWithoutConstructor();

(\Closure::bind(
function (SomeClass $object) ($bar
$object->foo = $bar;
},
null,
SomeClass::class
))($object);

?>

I know this is a weird stuff to do I wouldn't advise to others myself,
but some hydration libraries such as ocramius/generated-hydrator do
leverage this mechanism to hydrate objects, and I sometime do it myself
in business code, sporadically, to hydrate objects from database without
needing to define a constructor for those value objects.

If it does prevent such code, will there be some additional mechanism to
create value objects with readonly properties without constructor ? Such
as an JavaScript object-like object instanciation mechanism ?

<?php

class SomeClass
{
public readonly int $foo;
}

$bar = 12;

$object = {
foo: $bar,
};

?>

I know this goes far outside of this RFC's own scope, but maybe this is
the occasion to open a discussion about this, this would play well with
readonly properties and various serializers or hydrators libraries.

Regards,

--

Pierre

Sounds like a revelant subject to be raised, indeed. But I honestly hope
that this awkward hack will not prevent us from getting read-only
attributes.

Le 04/06/2021 à 17:19, Nikita Popov a écrit :

Hi internals,

I'd like to open the discussion on readonly properties:https://wiki.php.net/rfc/readonly_properties_v2

This proposal is similar to thehttps://wiki.php.net/rfc/write_once_properties RFC that has been declined
previously. One significant difference is that the new RFC limits the scope
of initializing assignments. I think a key mistake of the previous RFC was
the confusing "write-once" framing, which is both technically correct and
quite irrelevant.

Please see the rationale section (https://wiki.php.net/rfc/readonly_properties_v2#rationale) for how this
proposal relates to other RFCs and alternatives.

Regards,
Nikita

Hello,

If I understood it well you count assignments, one note thought after
re-reading once again, you state:

This variant is not allowed, as the initializing assignment occurs from outside the class:

Does this mean it'll prevent programmatic object hydration using the
scope-stealing closure pattern ? Such as:

<?php

class SomeClass
{
public readonly int $foo;
}

$bar = 12;

$object = (new \ReflectionClass('SomeClass'))->newInstanceWithoutConstructor();

(\Closure::bind(
function (SomeClass $object) ($bar
$object->foo = $bar;
},
null,
SomeClass::class
))($object);

?>

I know this is a weird stuff to do I wouldn't advise to others myself, but
some hydration libraries such as ocramius/generated-hydrator do leverage
this mechanism to hydrate objects, and I sometime do it myself in business
code, sporadically, to hydrate objects from database without needing to
define a constructor for those value objects.

Yes, this is supported. What matters is the scope from which the
initialization occurs. If you rebind a closure to the scope of the class,
then you can perform any operations that would be legal inside that class,
including access to private properties and initialization of readonly
properties.

Regards,
Nikita

Hi internals,

I'd like to open the discussion on readonly properties:
https://wiki.php.net/rfc/readonly_properties_v2

This proposal is similar to the
https://wiki.php.net/rfc/write_once_properties RFC that has been declined
previously. One significant difference is that the new RFC limits the
scope
of initializing assignments. I think a key mistake of the previous RFC was
the confusing "write-once" framing, which is both technically correct and
quite irrelevant.

Please see the rationale section (
https://wiki.php.net/rfc/readonly_properties_v2#rationale) for how this
proposal relates to other RFCs and alternatives.

I have a question about Reflection support, the RFC states:

ReflectionProperty::setValue() can bypass the requirement that
initialization occurs from the scope where the property has been declared.
However, reflection cannot modify a readonly property that has already been
initialized.

Is there a reason why this is not possible? I am thinking about ORMs or
Deserializers here where a pattern would be:

class MyDataObject
{
public function __construct(
public readonly $foo
) {}
}

$dataObject = $reflectionClass->newInstanceWithoutConstructor();
$dataObject->getProperty('foo')->setValue($row['value']);

This example is compatible with the proposal. Calling
newInstanceWithoutConstructor() leaves the property uninitialized, and you
can initialize it through reflection (or closure rebinding) from any scope.
Hydration and deserialization only requires the ability to initialize, not
to change the value of properties past initialization.

Regards,
Nikita

Hi internals,

I'd like to open the discussion on readonly properties:
https://wiki.php.net/rfc/readonly_properties_v2

This proposal is similar to the
https://wiki.php.net/rfc/write_once_properties RFC that has been declined
previously. One significant difference is that the new RFC limits the scope
of initializing assignments. I think a key mistake of the previous RFC was
the confusing "write-once" framing, which is both technically correct and
quite irrelevant.

Please see the rationale section (
https://wiki.php.net/rfc/readonly_properties_v2#rationale) for how this
proposal relates to other RFCs and alternatives.

Regards,
Nikita

Thank you for the detailed analysis in the rationale section. I am, however, still skeptical of this approach, for a couple of reasons.

1. This does appear to address the "leakage" problem I noted in my earlier analysis around the start of the year, when considering the writeonce proposal[1][2]. That's great to see.

2. It doesn't address the larger question of cloning, however. The answer for now seems "maybe we'll get clone-with at some point", which would be a way around it, but there is no active RFC for that right now. I'm obviously very in favor of RFCs that complement each other to give more than the sum of their parts, but those RFCs need to be at least on the horizon to actually come together. Right now that looks like it won't happen this cycle. Absent clone-with, readonly would be effectively unusable in any evolvable object of any non-trivial complexity. It also wouldn't work with objects that need properties that are not constructor arguments, such as PSR-7 type objects. That's a no in my book.

Larry,

To address the cloning objection to raise to Nikita's RFC there is one solution you did not mention in your blog post[1. Rather than your clone-with using a syntax that does not exist anywhere else[2] in PHP we could instead use one similar to your preferred "clone-with" approach where a closure could be run in an internal context. This closure would give the best of both your clone-with and of __clone() approaches. Applying this approach to your example in [3] gives us:

$r6 = clone $r5 with function($obj) {
$obj->uri = new Uri('https://python.org/');
$obj->headers = ['host' => 'http://java.com/'];
$obj->version = 'the old one';
};

If PHP Internals were to approve the "Auto-capturing multi-statement closures" RFC[4] we could also have auto-capture in a place where it would be likely common:

$uri = new Uri('https://python.org/');
$headers = [host => 'http://java.com/'];
$version = 'the old one';
$r6 = clone $r5 with fn($obj) {
$obj->uri = $uri;
$obj->headers = $headers;
$obj->version = $version;
};

Another improvement, which I don't know if it is possible or even advisable, is if the closure could have automatic access to a $this variable for the instance of the cloned object then it becomes even simpler:

$uri = new Uri('https://python.org/');
$headers = [host: 'http://java.com/'];
$version = 'the old one';
$r6 = clone $r5 with fn() {
$this->uri = $uri;
$this->headers = $headers;
$this->version = $version;
};

Interestingly, using your example[2] again, if we were to adopt the same syntax for new statements we could obviate the need for tedious and verbose "wither" methods as well as the numerous internal clones and throwaway variables needed with withers called immediately after instantiation.

For example:

$r2 = new Request() with fn() {
$this->method = 'POST';
$this->uri = new Uri('https://php.net/');
$this->protocolVersion = '2.0';
$this->header = [ 'cache' => 'none' ];
};

Lastly, you proposed "with" as your keyword to identify the initialization values which I assume you picked it because of the "wither" method pattern, and my examples above followed suit.

However, given with is not currently a reserved word in PHP and using a closure inherently provides an existing keyword, PHP could leverage function and/or fn to indicate "with." (Note the following example do not include the two (2) enhancements mentioned above using fn and $this):

$r6 = clone $r5 function($obj) {...};

-Mike

[1] https://peakd.com/hive-168588/@crell/object-properties-and-immutability https://peakd.com/hive-168588/@crell/object-properties-and-immutability

[2] Even though I would very much like to see that syntax become a general purpose syntax for object instantiation, I think it would not be good to use it unless it were fully fleshed out for much greater utility.

[3] https://peakd.com/hive-168588/@crell/object-properties-part-2-examples https://peakd.com/hive-168588/@crell/object-properties-part-2-examples

[4] https://wiki.php.net/rfc/auto-capture-closure <https://wiki.php.net/rfc/auto-capture-closure

On Sat, Jun 5, 2021 at 6:51 PM Larry Garfield larry@garfieldtech.com
wrote:

Hi internals,

I'd like to open the discussion on readonly properties:
https://wiki.php.net/rfc/readonly_properties_v2

This proposal is similar to the
https://wiki.php.net/rfc/write_once_properties RFC that has been
declined
previously. One significant difference is that the new RFC limits the
scope
of initializing assignments. I think a key mistake of the previous RFC
was
the confusing "write-once" framing, which is both technically correct and
quite irrelevant.

Please see the rationale section (
https://wiki.php.net/rfc/readonly_properties_v2#rationale) for how this
proposal relates to other RFCs and alternatives.

Regards,
Nikita

Thank you for the detailed analysis in the rationale section. I am,
however, still skeptical of this approach, for a couple of reasons.

1. This does appear to address the "leakage" problem I noted in my earlier
   analysis around the start of the year, when considering the writeonce
   proposal[1][2]. That's great to see.

2. It doesn't address the larger question of cloning, however. The answer
   for now seems "maybe we'll get clone-with at some point", which would be a
   way around it, but there is no active RFC for that right now. I'm
   obviously very in favor of RFCs that complement each other to give more
   than the sum of their parts, but those RFCs need to be at least on the
   horizon to actually come together. Right now that looks like it won't
   happen this cycle. Absent clone-with, readonly would be effectively
   unusable in any evolvable object of any non-trivial complexity.

Compared to the general area of applicability of readonly properties, the
cases that require clone-with are rare. It's the intersection of
"not-really-immutable objects using wither evolution" and "has so many
properties that passing them to the constructor is infeasible". While this
intersection does include a couple of prominent examples, they are by no
means numerous. While it may be a problem worth solving, I personally
don't consider it neither particularly important nor time critical.

For what it's worth, I believe Mate wants to work on clone-with once this
RFC passes -- while there is no RFC for that, there is already an
implementation, though from a cursory look it would require some
adjustments to actually work with readonly properties.

It also wouldn't work with objects that need properties that are not
constructor arguments, such as PSR-7 type objects. That's a no in my book.

I don't understand what you mean here. Why would properties that are not
constructor arguments be a problem?

3. As noted in my previous analysis, even with clone-with, asymmetric
   visibility results in a nicer syntax when evolving objects that have any
   complexity to them. (See the examples in [2].)

I tend to agree. And I am not opposed to having asymmetric visibility as
well. For example, C# does both, and I think both do have value. "readonly"
is a very strong signal to the reader. Asymmetric visibility is not.
Asymmetric visibility (without proper readonly support) could mean that the
property is actually readonly, or it could mean that it's exactly what it
says on the tin: The property is internally mutable. I don't think these
two concepts should be conflated, though they certainly have overlap.

4. One detail left out of the rationale section is how much of a
   performance difference there is between readonly and implicit-accessors.
   It says the latter still has a performance hit, but not how much. How
   significant is it? If it's tiny, then frankly the biggest argument for
   readonly goes away, since implicit-accessors gives us 98% the same
   functionality in a more forward-compatible way without the cloning issues.
   If it's twice as slow, then having a separate keyword for a common case
   makes sense.

The performance difference shouldn't be large, maybe 10-15%. I don't
consider performance a big deciding factor here. When comparing to
accessors the relevant difference I see is that readonly properties cover a
large use case at a small fraction of language complexity.

5. I would have to experiment a bit with hydration, as others have noted,
   because unconventional object construction mechanisms are a critically
   important workflow. The RFC even notes in passing that serialization is
   possibly made complicated. Just how complicated? There's no mention of
   __deserialize() and how it would interact, but cases like that need to be
   very clearly handled and documented.

The RFC notes in passing that if readonly default values were allowed, it
could have impact on userland deserialization. As-is, I believe the
proposal doesn't present any problem for userland hydrators or
deserializers, or PHP's own unserialization mechanisms. This should get
mentioned in the proposal, though it will be along the lines of "yes,
everything works as expected".

6. I'm OK with the approach to constructor promotion and default values.
   That seems reasonable in context, especially if combined with
   New-in-initializers[3], which I am hoping you plan to finish this cycle. :-)

7. Though, it just occurred to me, this may result in issues around
   optional values.

class URL {
public function __construct(
public readonly string $scheme = 'https',
public readonly string $domain = '',
public readonly int $port = '443',
public readonly string $path = '',
) {}
}

Now, if you want to hydrate the object externally, you need to ensure
those properties are not set by the constructor. However, a promoted value
is always set by the constructor; either it has a default or it is
required. From previous replies it sounds like the workaround for that is
reflection and newWithoutConstructor(), but I'm not sure how I feel about
that, because that would also then preclude any other logic in the
constructor, as that would also get skipped. Perhaps this isn't as big of
an issue as I think it is, and if so I'd love to hear why, but it makes me
concerned.

Using newInstanceWithoutConstructor() is not a workaround, to the best of
my knowledge these things are always implemented this way. You let the
constructor initialize the properties, or you bypass the constructor and
manually initialize them. I don't really see how it could work in any other
way, unless you have some very specific contract between your constructor
and a hydrator library (in which case you should make that contract with a
method other than the constructor).

8. Although the RFC says it does not preclude accessors or explicit

asymmetric visibility in the future, and I absolutely believe that Nikita
is genuine about that, I am still concerned that should more robust
proposals come forward later, it will be met with "we don't need another
still-fancier syntax here, readonly is good enough." It's good to know
that C# manages to have both, but that doesn't mean the same logic would
apply in PHP, or to PHP voters, specifically.

To clarify, I think your argument here goes roughly like this (correct me
if I'm wrong): If we already have one of readonly properties or asymmetric
visibility, an argument could be made for not adding the other one because
it doesn't provide sufficient marginal value. So, if we assume that we can
only have one of these features, is readonly properties the right one to
add? Shouldn't we add asymmetric visibility instead, which can be used for
a wider range of use cases?

I think this is a legitimate concern to have, but (under the assumption
that we can have only one) it's not easy to say which would be preferable.
Yes, asymmetric visibility is more widely applicable, but at the same time
I would expect most applications to be bonafide readonly properties. So
while asymmetric visibility is more widely applicable, it's also imprecise
for the majority use case. Which is better? Of course, my assumption here
could also be wrong.

9. I know that the variance for properties in child classes was a source of

discussion, and the end result seems to be that readonly-ness is
invariant. However, that precludes having an easy way to have both a
mutable and immutable version of a class, easily. (If you wanted to have,
say, a read-only class most of the time for safety, but for writing you use
an alternate "open" version that can be updated and then persisted to the
database.) That's a style I've experimented with on and off for a while
and already don't have a great solution to, but it feels like readonly
would make that even harder. Again, I'd be very happy to hear alternatives
around that.

I don't believe you can't have an immutable class extend a mutable one or a
mutable extend an immutable one, both result in LSP violations (though not
on the type level). There was a lot of discussion around this when
DateTimeImmutable was introduced, which notably is not in an inheritance
relationship (in other direction) with DateTime, for good reason.

Depending on the answers to the above, I could be convinced of this as a
stop gap iff paired with clone-with in the same version. However, on its
own I think this is only a half-solution, and I'm not wild about a
half-solution for a version or two. That's why I prefer going straight to
asymmetric visibility, as that would cover mostly the same use case in a
single RFC while still being forward compatible; it would benefit from
clone-with, but still be usable without it.

Just to reiterate this point: Readonly properties is not a half solution to
which asymmetric visibility is the full solution. They are different
solutions to different albeit overlapping problems. I'm not proposing
readonly properties here because it's the only thing I can get into 8.1,
I'm proposing it because I think it's the more important problem to solve.

Regards,
Nikita

Hi Nikita,

Hi internals,

I'd like to open the discussion on readonly properties:
https://wiki.php.net/rfc/readonly_properties_v2

Very good work, thank you :)

This proposal is similar to the
https://wiki.php.net/rfc/write_once_properties RFC that has been declined
previously. One significant difference is that the new RFC limits the scope
of initializing assignments. I think a key mistake of the previous RFC was
the confusing "write-once" framing, which is both technically correct and
quite irrelevant.

Please see the rationale section (
https://wiki.php.net/rfc/readonly_properties_v2#rationale) for how this
proposal relates to other RFCs and alternatives.

About this paragraph in
https://wiki.php.net/rfc/readonly_properties_v2#rationale:

" The addition of readonly properties neither precludes nor
discourages the addition of asymmetric property visibility."

To me this sentence is the meaning of a readonly property, not an
immutable property, as in writeable once property, in constructor.

This RFC is perfect given its goal but the property attribute "readonly".

How would you define a property only readable from outside the scope
but writeable inside the scope of its class (->status f.e.)?

This is PHP and we always have our ways. That being said, the keyword
"readonly" is really about what is defined as "asymmetric property
visibility" in many other languages. Rust uses annotation, .net via
either readonly and they use an init setter to make it immutable in
v9 (public DateTime RecordedAt { get; init; }), other like java or
scala relies on setting a property getters only.

Given this, I wonder if it would not be easier to have actual per
property getter/setter as an addition or replacement to the current
(kind of horrible) get/set($name, $value). This would all cases in
one shot, a more complicated shot but much more powerful. For the
record, I am totally not a fan of the 'manual" get/setMyProperty ;)

In any case, I would at least put a second thought on the name "readonly".

ps: sorry if this question has been discussed in previous RFCs,
pointers appreciated as I did not find it :)

Best,

Pierre

Pierre and Mike:

"Asymmetric visibility" as we keep referring to it would mean the "implicit accessors only" version of this: https://wiki.php.net/rfc/property_accessors

That is, it would let you define public/private/protected for get and set operations on a property separately from each other.

There are three key differences between readonly and asymmetric visibility as described there:

• Asymmetric visibility would allow a property to be reassigned multiple times from within a class, readonly would allow writing to it only once when it's uninitialized. Whether one of those is too-tight or too-loose is a matter of opinion and context.

• Asymmetric visibility is deliberately a syntax that is forward-compatible with explicit accessor methods in the future. readonly would be a separate, independent feature/syntax.

• Asymmetric visibility, IMO, is a stand-alone useful feature. readonly would be most useful if combined with a separate clone-with operator (discussed elsewhere). Whether readonly is useful enough on its own without that to justify its passage without a clone-with RFC also under discussion is an open question, and largely what we've been debating.

Both approaches would eliminate most uses of __get/__set, which I think everyone agrees is a win.

(Nikita, I hope I represented that accurately.)

--Larry Garfield

I'd like to open the discussion on readonly properties:
https://wiki.php.net/rfc/readonly_properties_v2

This proposal is similar to the
https://wiki.php.net/rfc/write_once_properties RFC that has been
declined
previously. One significant difference is that the new RFC limits the
scope
of initializing assignments. I think a key mistake of the previous RFC
was
the confusing "write-once" framing, which is both technically correct and
quite irrelevant.

Please see the rationale section (
https://wiki.php.net/rfc/readonly_properties_v2#rationale) for how this
proposal relates to other RFCs and alternatives.

I plan to open voting on this RFC soon. I don't think there's anything
technical left to address here, the discussion mostly comes down to a value
judgement. I think everyone has made their position regarding that clear...

Actually, we talked off the list about a way to possibly make this work
with __clone():

We could allow __clone to have one argument, the object being cloned. And
when the signature declares this argument, then all readonly properties
would be set as uninitialized on $this.

A typical __clone function would look like this with readonly properties:
function __clone(object $original)
{
$this->readonlyProp = clone $original->readonlyProp;
}

That would turn my vote into a +1 if that could be made to work!

Cheers,
Nicolas