Dear Internals,
I believe we had a long enough and fruitful discussion period,
so I have just opened the vote at
https://wiki.php.net/rfc/write_once_properties
since I didn't want to add any significant change to the proposal
any more.
The vote will run for 2 weeks and it will be closed on 2020-03-31.
Regards:
Máté Kocsis
I believe we had a long enough and fruitful discussion period,
so I have just opened the vote at
https://wiki.php.net/rfc/write_once_properties
since I didn't want to add any significant change to the proposal
any more.
I'm not sure the RFC can be edited during vote, but the second code
block in the RFC is wrong - missing "<keyword>".
--
Aleksander Machniak
Kolab Groupware Developer [https://kolab.org]
Roundcube Webmail Developer [https://roundcube.net]
PGP: 19359DC1 # Blog: https://kolabian.wordpress.com
Hi Aleksander,
Thank you for the comment!
You are right, the <keyword> is missing from there.
I believe we can correct small typos/grammatical errors
as far as the contents of the RFC stays the same. That's
why I've just fixed the issue.
Cheers,
Máté
I believe we had a long enough and fruitful discussion period,
so I have just opened the vote at
https://wiki.php.net/rfc/write_once_properties
since I didn't want to add any significant change to the proposal
any more.The vote will run for 2 weeks and it will be closed on 2020-03-31.
Hi everyone,
I voted against the RFC because it has too many rough edges to me.
eg cloning doesn't work, the initialization needs special rules, etc.
I don't think these issues can nor should be figured out later on: they are
low-level conceptual issues IMHO.
We should not build on them before they're resolved (a land field of
special cases look like a big code smell to me).
I proposed an alternative behavior that removes all the issues.
I think it needs to be discussed more in-depth:
https://externals.io/message/108675#108753
Regards,
Nicolas
I don't think these issues can nor should be figured out later on: they
are low-level conceptual issues IMHO.
I don't agree. Initialization would for example 100% work, I only removed
it from the proposal at the end because
we'll have more freedom to add new language behaviour until we find a much
more useful use-case for default
values of "write-once" properties than what we currently have.
Speaking about cloning: I didn't want to propose a "workaround" in this
proposal (as I wrote in the RFC) because
tackling that problem in the same time would make the RFC much more
complex. And since we can divide the whole
problem into two smaller ones (because you can also create a new instance
instead of cloning), I chose this approach.
I proposed an alternative behavior that removes all the issues. I think it
needs to be discussed more in-depth:
https://externals.io/message/108675#108753
I'd like to reiterate my answer then: I think your idea and my proposal
doesn't try to solve the same problem.
My goal is to make sure that a property can't be messed with. No matter the
scope, no matter if it's some exotic
use-case (like calling the constructor twice) or a mistake of the author
herself/himself.
Your idea would however add a property-accessor-like feature:
public readonly $foo => read access from the public scope, write access
from everywhere else
protected readonly $foo => read access from the public and protected
scopes, write access from the private scope
It seems to me that it's basically a convention-over-configuration based
property-accessor variant where one can't give
for example read access to the public scope and write access to only the
private scope.
And I'm saying that you try to solve a different problem because we have
already discussed that property accessors
and "write-once" properties are not closely related to each other:
While "write-once properties" would solve a problem that is currently not
possible to do (immutability of properties),
I consider property accessors to be only a syntactic sugar to separate read
and write access to a property without
using getters and setters - at least it's true for the version you proposed.
Of course, not everyone considers property immutability a problem to be
worth to bother about (which I can not relate,
but can totally understand), it's also totally ok not to like the special
rules I added or the implementation itself, but please
don't promote your idea as a better version of "write-once" properties that
removes all its issues - simply because it's not
an answer to the same problem and in turn has other issues.
Máté
I'd like to reiterate my answer then: I think your idea and my proposal
doesn't try to solve the same problem.
Like you write in the RFC:
Although actually “write-once” properties and property accessors are
orthogonal to each other, it's arguable whether we still needed
“write-once” properties if we had property accessors.
Both proposals relate a lot to each other: it's one or another, both cannot
coexist: there is only one meaning for the "readonly" keyword once it's
bound to some interpretation.
once a “write-once” property is initialized, it can't be changed after
cloning
The solution could be to add support for either object initializers or
property mutation during the clone operation
I think this is my biggest issue: it is very common to write immutable VOs
with "withers" on them (see PSR-7).
Thus the RFC doesn't address the currently most common way of writing
immutable VOs I fear.
I would be very interested in having at least a draft of the possibilities
you have in mind because I don't get the suggestion you make in the RFC
right now.
Having a better clue about this might remove at least part of my concerns.
I think this sentence in the RFC sums up the "why" quite nicely:
The only problem with solely relying on property accessors is that they
can't prevent changes in the private/protected scope (depending on
visibility).
In short, the RFC builds on the idea that preventing changes to
private/protected scope is worth the listed limitations:
this RFC proposes to disable the property modifier in question for
[untyped properties]
Another restriction of “write-once” properties is that they can't have a
default value
“write-once” properties must not override regular properties
While "property accessors-like" would have none of them if I'm not wrong,
and would solve the "withers" use case.
That's where we have a different judgment IIUC: to me, visibility-bound
read-only access provides all the conceptual guarantees we need:
- as class authors: "don't mess up with my state, you, userland";
- and from userland: "I trust you to know how to deal with your internal
state, you, class author".
From a static analysis pov, all would be plain explicit and could be
verified automatically. Not internal-immutability of course.
Note that I'm just arguing that the extra guarantee you look for might not
be worth the downside. If there is a way to provide private-scope
guaranteed immutability without the listed drawbacks, my concerns would
void (thus my request for precisions about cloning.)
Nicolas
PS: the RFC doesn't mention anything about Reflection-based mutability. It
could be worth adding something since we talked about it on the list:
"ReflectionProperty::setReadOnly()"
Both proposals relate a lot to each other: it's one or another, both cannot
coexist: there is only one meaning for the "readonly" keyword once it's
bound to some interpretation.
You are right in the sense that the two proposals can't use the same
keyword.
However, I believe we both agree that the two features themselves are not
exclusive to each other (although some argue if it's worth to have both).
So I don't understand why you would insist on using the same keyword
a "write-once" property would have according to my proposal.
I mean, if your end goal is to offer a simpler variant of a property
accessor feature
then in case of a "write-once" property the syntax could be (copied from
someone's
tweet):
readonly public private $foo;
Not that I like this idea, but it could mean that the property can be read
in the public
scope, and written only once in the private scope. I'm sure there are
other, more
explicit ways to express the same intention.
I think it's clear after the prior example that there is a place for both
of our features.
I understand though that you don't feel the need to be that strict in the
private/protected
scope (while I would most probably vote against all variants of property
accessors).
What I'd like to achieve is to offer strong guarantees - actually, as
strong as a type-system
level one - about that my properties can't change over time, as well as
being able to reason
about the (non-internal) mutability of my properties just by looking at the
type.
At this point I'd like to repeat one of my previously mentioned arguments
for the feature:
if we have an immutable PSR-7 that is used all over the PHP ecosystem, and
we also have
lots of people who wish for more type-level strictness by using immutable
DTOs or VOs,
why don't we provide a proper way to achieve immutability? If there is such
a need
then I think we should react. Even more so, because the same feature is
available in other
languages where we usually copy from, so it wouldn't even be without
precedents.
(Just to be clear: I don't think that we have to copy everything from Java
or C#. :) )
Also, Marco was wondering somewhere on Twitter if it would be possible to
perform some
engine-level optimizations of "write-once" properties? I'm also not sure
but I'd
be very curious about its feasibility. Hopefully, Nikita or Dmitry can
answer it. :)
Thus the RFC doesn't address the currently most common way of writing
immutable VOs I fear.
Yes, VOs that currently use cloning for mutation has to change their code
to create
a new instance if they have "write-once" properties. At least it's the
situation
according to the current proposal. I'll copy-paste an earlier answer of
mine here
what ideas were brought up to solve the cloning issue:
... Actually, I even had a short discussion with Nikita about the topic. He
proposed the
following syntax (which is inspired by Rust):
public function withFoo(FooT $foo): static {
return new static { foo => $foo, ...$this };
}
My idea was very similar, but it affects cloning:
$self = clone $this with { foo => $foo, bar => $bar, ... }
Do you have any other ideas or comments about the two proposals? Another
reason
why I wanted to omit this from the current proposal is because that way the
RFC
would have included even more things to debate on and an even bigger
"attack surface".
We didn't want property initializers as proposed by the previous RFC, and
we'd probably
debate for quite some time on the clone syntax I proposed... Let's discuss
bigger
things separately instead as how we did in case of nullable types or the
void type.
(Please note that I wouldn't like to wait for one or more releases)
Note that I'm just arguing that the extra guarantee you look for might not
be worth the downside.
I understand your point of view. However, I'm not sure what you mean by the
downsides? I believe you might think about cloning for one, but what are the
others? Maybe that a "write-once" property must have a type and can't have
a default value?
I think the second one is not something that people in practice would have
to face.
We have constants for exactly the same purpose. This way, we can postpone
the decision about what to do with default values until it's necessary to
decide.
I also wasn't very happy to add a special rule like that but it was the most
responsible thing to do I think.
Besides, I'd call the requirement to have typed properties in order to be
eligible for
the "readonly" keyword a good thing. Having more types is good, right? :)
But half-joking aside, there is just no way to extend "write-once"
semantics to
untyped properties. If anyone can come up with a sane way then I'd would be
very
happy to push it. Of course, you can call this restriction a conceptual
issue, but
I think we stayed within the limitations of the current type-system to
achieve the
goal I set without adding more quirks and weird workarounds to the
language...
Yes, for the price of special casing this feature for typed properties...
the RFC doesn't mention anything about Reflection-based mutability
You are right, a mention could have been added to the RFC. If I'm allowed to
make a short clarification (like "you can't circumvent the property
modifier via
Reflection"), I'll do it.
Sorry for the lengthy response, but I really hope that I managed to clarify
the whys behind the proposal.
Máté
At this point I'd like to repeat one of my previously mentioned arguments
for the feature:
if we have an immutable PSR-7 that is used all over the PHP ecosystem, and
we also have
lots of people who wish for more type-level strictness by using immutable
DTOs or VOs,
why don't we provide a proper way to achieve immutability? If there is such
a need
then I think we should react. Even more so, because the same feature is
available in other
languages where we usually copy from, so it wouldn't even be without
precedents.
(Just to be clear: I don't think that we have to copy everything from Java
or C#. :) )
I don't have voting rights yet, but I would also vote no as is precisely because of the PSR-7 type case, along the same lines that Nicolas is arguing.
Currently, a PSR-7 implementation does something like this (assuming it were updated to typed properties):
class Response implements ResponseInterface {
private int $statusCode;
public function getStatusCode : string {
return $this->statusCode;
}
public function withStatusCode(int $code): self {
$new = clone($this):
$new->statusCode = $code;
return $new;
}
}
Now repeat that for about 10 different properties across 3 different objects, and sometimes with multiple variations on the with*() method.
Allowing the status code to be declared public-readonly would change it to this:
class Response implements ResponseInterface {
public readonly int $statusCode;
public function withStatusCode(int $code): self {
$new = new static(
$code,
$this->headers,
$this->protocolVersion,
$this->body
);
return $new;
}
}
And that's for Response, which is the simplest of the message objects in PSR-7.
That is, you get to skip the trivially easy method (a positive) in return for having a longer and more involved with*() method, which you already have more of to begin with (a negative), and for forcing you to have a constructor that takes every possible property (a negative).
That is not a net win.
Thinking about it, Uri is possibly an even better example as it has 8 properties:
class Uri implements UriInterface {
private string $scheme;
private string $authority;
private string $userInfo;
private string $host;
private int $port
private string $path;
private array $query;
private string $fragment;
// These methods could all go away, but they're the simple ones:
public function getScheme() {
return $this->scheme;
}
public function getAuthority();
public function getUserInfo();
public function getHost();
public function getPort();
public function getPath();
public function getQuery();
public function getFragment();
// Whereas these turn from this:
public function withScheme(string $scheme) {
$new = clone($this);
$new->scheme = $scheme;
return $new;
}
// In this:
public function withScheme(string $scheme) {
$new = static(
$scheme,
$this->authority,
$this->userInfo,
$this->host,
$this->port,
$this->path,
$this->query,
$this->fragment
);
return $new;
}
// And doing so for all of these methods, but slightly differently:
public function withUserInfo($user, $password = null);
public function withHost($host);
public function withPort($port);
public function withPath($path);
public function withQuery($query);
public function withFragment($fragment);
}
So switching it to use readonly properties as proposed here, would involve:
- More complex methods
- A mandatory 8 parameter constructor
- Avoiding the trivial methods in exchange for the mandatory methods being longer.
Honestly... that's not usable for value objects. I love value objects, I love immutable values, and I would find this property useless in its current form.
Also, I just realized... properties cannot be defined in interfaces. That means a PSR interface (or any other interface) could not require that they be defined, which means they cannot be made part of a contract. That renders readonly properties as defined here unusable by FIG entirely.
Requiring that properties be typed to be readonly is fine with me. But the way it renders cloning unusable for evolvable objects makes evolvable objects considerably harder to use and to standardize, not easier.
And this property co-existing with some other way of achieving the same goal (whether that's something like Nicolas's proposal or something entirely different) would only lead to confusion and people doing one, realizing they should have done the other, and having more BC breaks now in their own code.
I appreciate the work that Mate has put into this proposal, but as is I have to encourage people to vote no.
--Larry Garfield
Nicolas Grekas nicolas.grekas+php@gmail.com wrote:
(from the other thread)
there must be a way to work around the keyword -
either via reflection or another means.
Can you expand on why there 'must' be a way to work around this? Can
you provide some example code where not being able to change the value
is going to cause problems.
I voted against the RFC because it has too many rough edges to me.
eg cloning doesn't work, the initialization needs special rules, etc.
To be clear, cloning would work....but just not be useful. But being
required to write a function to copy an object, and overriding the
values doesn't seem like a terrible burden. And, because PHP has an
evolved type system, it is always going to have some 'special rules'
for certain things, just because of maintaining BC.
I don't think these issues can nor should be figured out later on: they are
low-level conceptual issues IMHO.
I can't see where the edge-cases are going to bite us. What's an
example of where they can't be figured out later? btw I totally agree
that having them now would be nicer...
cheers
Dan
Ack
Chiming in to express my disappointment that final wasn't a voting choice.
- It's already reserved, so we don't have to worry about a new keyword.
- Another very popular language that is similar to PHP already uses it (Java).
I voted no for a variety of reasons, which includes:
- It doesn't support default values, and doesn't defend that choice
well in my opinion. - I think clone should be able to change the value. It's similar to
a constructor, and in fact I'd call it a "copy constructor." - The property type is mandatory. I am less certain about this, but
do not like it.
I do appreciate the RFC, but think it needs a bit more work.
Hi Levi,
Thank you very much for your feedback! I'll try to answer some of your
concerns.
Chiming in to express my disappointment that final wasn't a voting choice.
When I started to draft the RFC, I realized that a final property modifier
that I wanted
to propose would be pretty much inconsistent with final's current behaviour
(since it
currently controls inheritance), that's why we shouldn't copy from Java in
this case.
Now, readonly is the most popular choice in the vote - which directly comes
from C#.
- It doesn't support default values, and doesn't defend that choice
well in my opinion.
I wouldn't have thought before that this omission would be a double edged
sword.. :)
But I admit, I didn't really provide a well-expressed reasoning about the
choice
in the RFC. That was a fault. However, we talked about the decision quite a
lot
in the very end of the discussion thread so you might find some answers
there
if you are curious about more info.
- I think clone should be able to change the value. It's similar to
a constructor, and in fact I'd call it a "copy constructor."
I agree, it would be perfect if clone worked smoothly. However, my
decision was to cut this aspect off from the current proposal since it's
not an
absolute prerequisite of "write-once" properties in my opinion. Plus,
I wanted to avoid the situation when the RFC grows so big that we miss finer
details out from the discussion, or when the whole feature gets rejected
because a smaller detail has the wrong syntax/behaviour... Now, it seems
that the whole feature can get rejected because some (IMO less important)
decisions were postponed. That's Catch-22?
Just one more note about this topic: clone + final doesn't work well
together even
in Java (
https://en.wikipedia.org/wiki/Clone_(Java_method)#clone()_and_final_fields),
so it's not a kind of a problem that other languages could easily overcome.
In turn,
I would absolutely like to offer a more ergonomic solution for PHP as soon
as possible.
Please, have a look at my previous response to Nicolas where I showed the
two alternatives that already came up. :) Any comments are welcome!
- The property type is mandatory. I am less certain about this, but
do not like it.
Yes, I see your point. Because of the limitations of the current type
system,
we had to choose: we either leave untyped properties out of the game, or
start to treat them as if they had the mixed type (so a readonly $foo
property
would be equivalent to readonly mixed $foo). It's just the smaller problem
with the latter solution that the mixed type only has a draft RFC (where
the debate
was mainly about if it should contain void or null...), but in my opinion,
the implicit
type conversion would be a mistake. This modifier shouldn't change
the initialization rules of the property. At least this is what I think
now, and
that's why we rather chose the tradeoff to eliminate untyped properties.
I do appreciate the RFC, but think it needs a bit more work.
I also agree that my proposal couldn't give a definitive answer to all the
possible
questions that came up. But we - those ones who took part in the discussion
basically agreed that the feature is useful in spite of these unknowns.
That's
why we decided that the proposal should only keep the most important things,
and carve off the rest - what's controversial or unknown yet (e.g. default
values
or "fixing" cloning).
The most advantageous consequence of being this conservative is that we
have more
time (thus we can get more feedback/freedom) until we fill in the missing
holes of the
feature. So we made the responsible choice by postponing not
clear/controversial
decisions (e.g. how to treat non-typed properties) instead of trying to
immediately
guess the use-cases or to find workarounds. Since it's much-much easier to
add support for a new thing than to revert existing rules, I believe we
made a
good decision. I think something similar happened when the parameter type
system
was extended with scalar, then nullable, and finally with the void type, or
when
parameter covariance initially only supported omitting the type and 2
releases later
your RFC made covariance and contravariance much more complete.
I hope I could give explanations to you why the RFC became what it is now,
and what our thought process was.
Cheers:
Máté
Dear Internals,
I've just closed the vote: The RFC is declined with 23 votes in favour and
23 against.
Regards:
Máté