From 3fc2d5babd98b5a17f46f3bf7caf7833fb66f304 Mon Sep 17 00:00:00 2001 From: Takuya Aramaki Date: Sun, 18 Aug 2024 05:04:18 +0900 Subject: [PATCH 1/2] Allow reinitialization of a readonly property in `__clone` since PHP8.3 Closes phpstan/phpstan#11495 --- src/Php/PhpVersion.php | 5 ++++ .../Properties/ReadOnlyPropertyAssignRule.php | 8 +++++-- .../ReadOnlyPropertyAssignRuleTest.php | 19 +++++++++++++++ .../Rules/Properties/data/bug-11495.php | 24 +++++++++++++++++++ 4 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 tests/PHPStan/Rules/Properties/data/bug-11495.php diff --git a/src/Php/PhpVersion.php b/src/Php/PhpVersion.php index aaa08cf178..c226e2185b 100644 --- a/src/Php/PhpVersion.php +++ b/src/Php/PhpVersion.php @@ -327,4 +327,9 @@ public function hasDateTimeExceptions(): bool return $this->versionId >= 80300; } + public function supportsReadonlyPropertyReinitializationOnClone(): bool + { + return $this->versionId >= 80300; + } + } diff --git a/src/Rules/Properties/ReadOnlyPropertyAssignRule.php b/src/Rules/Properties/ReadOnlyPropertyAssignRule.php index 47d132b639..cf71693b90 100644 --- a/src/Rules/Properties/ReadOnlyPropertyAssignRule.php +++ b/src/Rules/Properties/ReadOnlyPropertyAssignRule.php @@ -5,6 +5,7 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; use PHPStan\Node\PropertyAssignNode; +use PHPStan\Php\PhpVersion; use PHPStan\Reflection\ConstructorsHelper; use PHPStan\Reflection\MethodReflection; use PHPStan\Rules\Rule; @@ -24,6 +25,7 @@ class ReadOnlyPropertyAssignRule implements Rule public function __construct( private PropertyReflectionFinder $propertyReflectionFinder, private ConstructorsHelper $constructorsHelper, + private PhpVersion $phpVersion, ) { } @@ -76,9 +78,11 @@ public function processNode(Node $node, Scope $scope): array throw new ShouldNotHappenException(); } + $methodName = $scopeMethod->getName(); if ( - in_array($scopeMethod->getName(), $this->constructorsHelper->getConstructors($scopeClassReflection), true) - || strtolower($scopeMethod->getName()) === '__unserialize' + in_array($methodName, $this->constructorsHelper->getConstructors($scopeClassReflection), true) + || strtolower($methodName) === '__unserialize' + || ($this->phpVersion->supportsReadonlyPropertyReinitializationOnClone() && strtolower($methodName) === '__clone') ) { if (TypeUtils::findThisType($scope->getType($propertyFetch->var)) === null) { $errors[] = RuleErrorBuilder::message(sprintf('Readonly property %s::$%s is not assigned on $this.', $declaringClass->getDisplayName(), $propertyReflection->getName())) diff --git a/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php b/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php index 90da9c44ec..a0e67f0f4c 100644 --- a/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php +++ b/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php @@ -2,6 +2,7 @@ namespace PHPStan\Rules\Properties; +use PHPStan\Php\PhpVersion; use PHPStan\Reflection\ConstructorsHelper; use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; @@ -23,6 +24,7 @@ protected function getRule(): Rule 'ReadonlyPropertyAssign\\TestCase::setUp', ], ), + new PhpVersion(PHP_VERSION_ID), ); } @@ -154,4 +156,21 @@ public function testBug6773(): void ]); } + public function testBug11495(): void + { + if (PHP_VERSION_ID < 80100) { + $this->markTestSkipped('Test requires PHP 8.1.'); + } + + $errors = []; + if (PHP_VERSION_ID < 80300) { + $errors[] = [ + 'Readonly property Bug11495\HelloWorld::$foo is assigned outside of the constructor.', + 17, + ]; + } + + $this->analyse([__DIR__ . '/data/bug-11495.php'], $errors); + } + } diff --git a/tests/PHPStan/Rules/Properties/data/bug-11495.php b/tests/PHPStan/Rules/Properties/data/bug-11495.php new file mode 100644 index 0000000000..670128dd6a --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/bug-11495.php @@ -0,0 +1,24 @@ += 8.1 +declare(strict_types = 1); + +namespace Bug11495; + +class HelloWorld +{ + private readonly string $foo; + + public function __construct() + { + $this->foo = 'bar'; + } + + public function __clone() + { + $this->foo = 'baz'; + } + + public function getFoo(): string + { + return $this->foo; + } +} From 9dbcae6a5db599464dcad63e355478a0f4055d88 Mon Sep 17 00:00:00 2001 From: Takuya Aramaki Date: Fri, 23 Aug 2024 00:58:22 +0900 Subject: [PATCH 2/2] Add more test for readonly property assignment --- .../ReadOnlyPropertyAssignRuleTest.php | 20 +++++++++++++++---- .../Rules/Properties/data/bug-11495.php | 3 +++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php b/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php index a0e67f0f4c..22b769b7cd 100644 --- a/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php +++ b/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php @@ -162,11 +162,23 @@ public function testBug11495(): void $this->markTestSkipped('Test requires PHP 8.1.'); } - $errors = []; if (PHP_VERSION_ID < 80300) { - $errors[] = [ - 'Readonly property Bug11495\HelloWorld::$foo is assigned outside of the constructor.', - 17, + $errors = [ + [ + 'Readonly property Bug11495\HelloWorld::$foo is assigned outside of the constructor.', + 17, + ], + [ + 'Readonly property Bug11495\HelloWorld::$foo is assigned outside of the constructor.', + 20, + ], + ]; + } else { + $errors = [ + [ + 'Readonly property Bug11495\HelloWorld::$foo is not assigned on $this.', + 20, + ], ]; } diff --git a/tests/PHPStan/Rules/Properties/data/bug-11495.php b/tests/PHPStan/Rules/Properties/data/bug-11495.php index 670128dd6a..fa125f26cf 100644 --- a/tests/PHPStan/Rules/Properties/data/bug-11495.php +++ b/tests/PHPStan/Rules/Properties/data/bug-11495.php @@ -15,6 +15,9 @@ public function __construct() public function __clone() { $this->foo = 'baz'; + + $s = new self(); + $s->foo = 'baz'; } public function getFoo(): string