Re: [PHP-DEV] [RFC] [VOTE] Immutable/final/readonly properties
On Tue, Mar 17, 2020, at 1:07 PM, Máté Kocsis wrote: > 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 --
Re: [PHP-DEV] [RFC] [VOTE] Immutable/final/readonly properties
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é
Re: [PHP-DEV] [RFC] [VOTE] Immutable/final/readonly properties
Chiming in to express my disappointment that `final` wasn't a voting choice. 1. It's already reserved, so we don't have to worry about a new keyword. 2. 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. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [VOTE] Immutable/final/readonly properties
> > 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
Re: [PHP-DEV] [RFC] [VOTE] Immutable/final/readonly properties
Nicolas Grekas 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 workbut 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 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [VOTE] Immutable/final/readonly properties
> 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()"
Re: [PHP-DEV] [RFC] [VOTE] Immutable/final/readonly properties
> 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é
Re: [PHP-DEV] [RFC] [VOTE] Immutable/final/readonly properties
> 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
Re: [PHP-DEV] [RFC] [VOTE] Immutable/final/readonly properties
Hi Aleksander, Thank you for the comment! You are right, the 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é
Re: [PHP-DEV] [RFC] [VOTE] Immutable/final/readonly properties
On 17.03.2020 11:12, Máté Kocsis wrote: > 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 "". -- Aleksander Machniak Kolab Groupware Developer[https://kolab.org] Roundcube Webmail Developer [https://roundcube.net] PGP: 19359DC1 # Blog: https://kolabian.wordpress.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php