[RFC] Readonly Property Defaults

Hey Tim,

thanks for the feedback!

Hi

I would like to present a tiny, five-line-removal RFC allowing
default values on readonly properties.

Allowing default values on readonly class properties enables creating
a strict contract for implementation property values that will not
change at runtime -- as in, constant-like behaviour with contract. I
would love doing that.

Please find the RFC text here:
https://wiki.php.net/rfc/readonly_property_defaults

Thank you, the proposal makes sense to me. I have just some requests
for clarification:

Can you add a “clone-with” example to the “Interaction with clone”
section? I assume that “clone-with” will also just work for the same
reasons, but it makes sense to spell it out.

Spelled it out and added a test for it.

Can you spell out the unserialization behavior with an example.
“Unserialisation continues to use the existing object hydration
semantics and is not changed by this RFC” is not clear to me, because
from my mental model unserialization first does a
newInstanceWithoutConstructor() and then fills in the properties
based on the serialized data. Thus a readonly field with a default
value would already be initialized, which means that filling in the
serialized data would fail, which I think is not a situation we have
so far.

At the time __unserialize is called the object exists, and the
property is initialised with the default value. So it fails in the same
way a re-assignment in the constructor or reflection setValue() will
fail. So, I wouldn't say "not a situation we have so far", but agree
that spelling it out cannot be wrong. RFC section is updated, and your
PR comment example added as a test!

In the “Interaction with asymmetric visibility” section, the example
would be stronger with a public(set) instead of private(set),
because with private(set) the example would fail either way, since
the global scope has no access to the property.

You are right. Changed.

Minor polishing remark in “Interaction with traits”: The opening PHP
tag is missing in the example.

Fixed!

Best regards
Tim Düsterhus

--

Cheers
Nick

Hey Larry,

Hey Internals,

I would like to present a tiny, five-line-removal RFC allowing default
values on readonly properties.

Allowing default values on readonly class properties enables creating a
strict contract for implementation property values that will not change
at runtime -- as in, constant-like behaviour with contract. I would love
doing that.

Please find the RFC text here:
https://wiki.php.net/rfc/readonly_property_defaults
I had been planning to say "meh, doesn't seem like it would hurt anything," but as Tim pointed out with unserialize, it potentially could hurt something. That makes me skeptical.

I also want to call out, as the RFC notes, that this is already a solved problem with private(set). It's not quite the same as readonly, but for most practical purposes it's close enough and already addresses the use case this RFC.

Thanks for the feedback.

I have to push back. It is not a solved problem. In fact, private(set)
is not only "not quite the same". Both features solve fundamentally
different problems -- private(set) has a foot gun readonly does not
have.

Because this point comes up repeatedly here is a concrete example:

final class PipelineOne // private(set)

{
     public private(set)array $orderedPipelineSteps = [
         \Thing\StepDoSomethingTwo::class,
         \Thing\StepAtReportBeginning::class,
         \Thing\StepDoSomethingOne::class,
     ];public function describe(): array {\sort($this->orderedPipelineSteps);// whoopsie, should have copied first $ordered = $this->orderedPipelineSteps;

         // do stuff return $ordered;
     }
}

final readonly class PipelineTwo// readonly{
     public function __construct(public array $orderedPipelineSteps = [
             \Thing\StepDoSomethingTwo::class,
             \Thing\StepAtReportBeginning::class,
             \Thing\StepDoSomethingOne::class,
         ],
     ) {}public function describe(): array {\sort($this->orderedPipelineSteps);// whoopsie, should have copied first $ordered = $this->orderedPipelineSteps;

         // do stuff return $ordered;
     }
}

// ###################### private(set) ###################### $one = new 
PipelineOne();
\var_dump($one->describe());
// array(3) { // [0]=> // string(27) "Thing\StepAtReportBeginning" // 
good, now ordered // [1]=> // string(24) "Thing\StepDoSomethingOne" // 
[2]=> // string(24) "Thing\StepDoSomethingTwo" //} \var_dump($one->orderedPipelineSteps);
// array(3) { // [0]=> // string(27) "Thing\StepAtReportBeginning" // 
whoopsie, this should NOT be first! // [1]=> // string(24) 
"Thing\StepDoSomethingOne" // [2]=> // string(24) 
"Thing\StepDoSomethingTwo" //} // ###################### readonly 
###################### $two = new PipelineTwo();
\var_dump($two->describe());
//Fatal error: Uncaught Error: Cannot indirectly modify readonly 
property PipelineTwo::$orderedPipelineSteps

With private(set) accidental mutation is legal. With readonly,
modification after initialisation is illegal. Both are great and useful
features, but private(set) only controls visibility not mutation. As
the above example proves, readonly is strict when it comes to mutation
and would have prevented the bug while private(set) happily ignores it
because mutation happens from within the class. With readonly it would
have been surfaced that I did:

\sort($this->orderedPipelineSteps); // whoopsie, mutated

$ordered = $this->orderedPipelineSteps;

instead of:

$ordered = $this->orderedPipelineSteps;

\sort($ordered);

This alone is reason enough to use readonly over private(set); less
verbosity is a cherry on top.

Note: only to make the example copy & work without the PR branch I used
constructor property promotion here. With the RFC, PipelineTwo
can declare the property default directly, without allowing outside
modification.

--

Cheers
Nick

(sorry for the double post; for some reason Thunderbird messed up
formatting on web in the last attempt)

Hey Larry,

Hey Internals,

I would like to present a tiny, five-line-removal RFC allowing default
values on readonly properties.

Allowing default values on readonly class properties enables creating a
strict contract for implementation property values that will not change
at runtime -- as in, constant-like behaviour with contract. I would love
doing that.

Please find the RFC text here:
https://wiki.php.net/rfc/readonly_property_defaults
I had been planning to say "meh, doesn't seem like it would hurt
anything," but as Tim pointed out with unserialize, it potentially could
hurt something.  That makes me skeptical.

I also want to call out, as the RFC notes, that this is already a
solved problem with private(set).  It's not quite the same as readonly,
but for most practical purposes it's close enough and already addresses
the use case this RFC.

Thanks for the feedback.

I have to push back. It is not a solved problem. In fact, private(set)
is not only "not quite the same". Both features solve fundamentally
different problems -- private(set) has a foot gun readonly does not
have.

Because this point comes up repeatedly here is a concrete example:

final class PipelineOne // private(set)

{
     public private(set) array $orderedPipelineSteps = [
         \Thing\StepDoSomethingTwo::class,
         \Thing\StepAtReportBeginning::class,
         \Thing\StepDoSomethingOne::class,
     ];

     public function describe(): array
     {
         \sort($this->orderedPipelineSteps); // whoopsie, should have 
copied first

         $ordered = $this->orderedPipelineSteps;

         // do stuff

         return $ordered;
     }
}

final readonly class PipelineTwo // readonly
{
     public function __construct(
         public array $orderedPipelineSteps = [
             \Thing\StepDoSomethingTwo::class,
             \Thing\StepAtReportBeginning::class,
             \Thing\StepDoSomethingOne::class,
         ],
     ) {}

     public function describe(): array
     {
         \sort($this->orderedPipelineSteps); // whoopsie, should have 
copied first

         $ordered = $this->orderedPipelineSteps;

         // do stuff

         return $ordered;
     }
}

// ###################### private(set) ######################
$one = new PipelineOne();
\var_dump($one->describe());
// array(3) {
//  [0]=>
//  string(27) "Thing\StepAtReportBeginning" // good, now ordered
//  [1]=>
//  string(24) "Thing\StepDoSomethingOne"
//  [2]=>
//  string(24) "Thing\StepDoSomethingTwo"
//}
\var_dump($one->orderedPipelineSteps);
// array(3) {
//  [0]=>
//  string(27) "Thing\StepAtReportBeginning" // whoopsie, this should 
NOT be first!
//  [1]=>
//  string(24) "Thing\StepDoSomethingOne"
//  [2]=>
//  string(24) "Thing\StepDoSomethingTwo"
//}
// ###################### readonly ######################
$two = new PipelineTwo();
\var_dump($two->describe());
//Fatal error: Uncaught Error: Cannot indirectly modify readonly 
property PipelineTwo::$orderedPipelineSteps

With private(set) accidental mutation is legal. With readonly,
modification after initialisation is illegal. Both are great and useful
features, but private(set) only controls visibility not mutation. As
the above example proves, readonly is strict when it comes to mutation
and would have prevented the bug while private(set) happily ignores it
because mutation happens from within the class. With readonly it would
have been surfaced that I did:

\sort($this->orderedPipelineSteps); // whoopsie, mutated

$ordered = $this->orderedPipelineSteps;

instead of:

$ordered = $this->orderedPipelineSteps;

\sort($ordered);

This alone is reason enough to use readonly over private(set); less
verbosity is a cherry on top.

Note: only to make the example copy & work without the PR branch I used
constructor property promotion here. With the RFC, PipelineTwo can
declare the property default directly, without allowing outside
modification.

--

Cheers
Nick