Set properties to readonly if possible #5547
Conversation
derrabus
force-pushed
the
improvement/readonly
branch
from
c599c96 to
04d2ce3
Compare
| @@ -16,19 +16,15 @@ | |||
| */ | |||
| final class ArrayResult implements Result | |||
| { | |||
| private int $columnCount = 0; | |||
| private int $num = 0; | |||
| private readonly int $columnCount; | |||
There was a problem hiding this comment.
readonly properties must not have a default. The old default was moved to the constructor logic.
| @@ -33,7 +33,7 @@ final class Connection implements ConnectionInterface | |||
| * | |||
| * @param resource $connection | |||
| */ | |||
| public function __construct(private $connection) | |||
| public function __construct(private readonly mixed $connection) | |||
There was a problem hiding this comment.
Only typed properties may be flagged readonly and there is no native resource type. These are known limitations of the PHP engine. This is why I'm adding a mixed type here. Since the doc block is more precise, this change should not make a difference for static analysis.
There was a problem hiding this comment.
I don't really like using the mixed native type where the argument is of a more specific type but given that it's the limitation of the type system, it should be okay.
|
This is also implemented to a certain extent in #5546. But I don't mind merging this PR first and then rebasing the rest of the changes. |
|
We can also merge the automated changes first. This should make this PR easier to review because I've made some changes an automated tool would probably not make. |
derrabus
force-pushed
the
improvement/readonly
branch
from
04d2ce3 to
1481f22
Compare
Summary
This PR flags all properties as
readonlywhen I found it safe to do so:privateor it or its class is flagged as@internal.My main motivation is to make clear that we consider those properties as immutable. My expectation is that this makes the code easier to comprehend and analyze.