diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index ef55f3683fc..e6119d7332d 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -25,6 +25,7 @@ use PHPStan\Analyser\Traverser\TransformStaticTypeTraverser; use PHPStan\DependencyInjection\Container; use PHPStan\Node\Expr\AlwaysRememberedExpr; +use PHPStan\Node\Expr\CloneReinitializationExpr; use PHPStan\Node\Expr\GetIterableKeyTypeExpr; use PHPStan\Node\Expr\IntertwinedVariableByReferenceWithExpr; use PHPStan\Node\Expr\OriginalForeachKeyExpr; @@ -2893,7 +2894,18 @@ public function assignInitializedProperty(Type $fetchedOnType, string $propertyN return $this; } - return $this->assignExpression(new PropertyInitializationExpr($propertyName), new MixedType(), new MixedType()); + $scope = $this->assignExpression(new PropertyInitializationExpr($propertyName), new MixedType(), new MixedType()); + + $function = $scope->getFunction(); + if ( + $function instanceof MethodReflection + && strtolower($function->getName()) === '__clone' + && $scope->phpVersion->supportsReadonlyPropertyReinitializationOnClone() + ) { + $scope = $scope->assignExpression(new CloneReinitializationExpr($propertyName), new MixedType(), new MixedType()); + } + + return $scope; } public function invalidateExpression(Expr $expressionToInvalidate, bool $requireMoreCharacters = false, ?ClassReflection $invalidatingClass = null): self diff --git a/src/Node/Expr/CloneReinitializationExpr.php b/src/Node/Expr/CloneReinitializationExpr.php new file mode 100644 index 00000000000..4bac55ec156 --- /dev/null +++ b/src/Node/Expr/CloneReinitializationExpr.php @@ -0,0 +1,48 @@ +propertyName; + } + + #[Override] + public function getType(): string + { + return 'PHPStan_Node_CloneReinitializationExpr'; + } + + /** + * @return string[] + */ + #[Override] + public function getSubNodeNames(): array + { + return []; + } + +} diff --git a/src/Node/Printer/Printer.php b/src/Node/Printer/Printer.php index 71d6b61c8e0..7409140e8c2 100644 --- a/src/Node/Printer/Printer.php +++ b/src/Node/Printer/Printer.php @@ -7,6 +7,7 @@ use PHPStan\Node\BooleanAndNode; use PHPStan\Node\BooleanOrNode; use PHPStan\Node\Expr\AlwaysRememberedExpr; +use PHPStan\Node\Expr\CloneReinitializationExpr; use PHPStan\Node\Expr\ExistingArrayDimFetch; use PHPStan\Node\Expr\ForeachValueByRefExpr; use PHPStan\Node\Expr\GetIterableKeyTypeExpr; @@ -104,6 +105,11 @@ protected function pPHPStan_Node_PropertyInitializationExpr(PropertyInitializati return sprintf('__phpstanPropertyInitialization(%s)', $expr->getPropertyName()); } + protected function pPHPStan_Node_CloneReinitializationExpr(CloneReinitializationExpr $expr): string // phpcs:ignore + { + return sprintf('__phpstanCloneReinitialization(%s)', $expr->getPropertyName()); + } + protected function pPHPStan_Node_ForeachValueByRefExpr(ForeachValueByRefExpr $expr): string // phpcs:ignore { return sprintf('__phpstanForeachValueByRef(%s)', $this->p($expr->getExpr())); diff --git a/src/Php/PhpVersion.php b/src/Php/PhpVersion.php index 9b198d567f5..0deb9f74cf8 100644 --- a/src/Php/PhpVersion.php +++ b/src/Php/PhpVersion.php @@ -342,6 +342,11 @@ public function supportsReadOnlyAnonymousClasses(): bool return $this->versionId >= 80300; } + public function supportsReadonlyPropertyReinitializationOnClone(): bool + { + return $this->versionId >= 80300; + } + public function supportsNeverReturnTypeInArrowFunction(): bool { return $this->versionId >= 80200; diff --git a/src/Rules/Properties/ReadOnlyPropertyAssignRule.php b/src/Rules/Properties/ReadOnlyPropertyAssignRule.php index bf078a3839a..b12eb2c89b1 100644 --- a/src/Rules/Properties/ReadOnlyPropertyAssignRule.php +++ b/src/Rules/Properties/ReadOnlyPropertyAssignRule.php @@ -6,6 +6,7 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\RegisteredRule; +use PHPStan\Node\Expr\CloneReinitializationExpr; use PHPStan\Node\Expr\SetOffsetValueTypeExpr; use PHPStan\Node\Expr\UnsetOffsetExpr; use PHPStan\Node\PropertyAssignNode; @@ -96,15 +97,26 @@ public function processNode(Node $node, Scope $scope): array throw new ShouldNotHappenException(); } + $methodName = $scopeMethod->getName(); + $inClone = $this->phpVersion->supportsReadonlyPropertyReinitializationOnClone() && strtolower($methodName) === '__clone'; 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' + || $inClone ) { 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())) ->line($propertyFetch->name->getStartLine()) ->identifier('property.readOnlyAssignNotOnThis') ->build(); + } elseif ( + $inClone + && !$scope->hasExpressionType(new CloneReinitializationExpr($propertyReflection->getName()))->no() + ) { + $errors[] = RuleErrorBuilder::message(sprintf('Readonly property %s::$%s is already assigned.', $declaringClass->getDisplayName(), $propertyReflection->getName())) + ->line($propertyFetch->name->getStartLine()) + ->identifier('assign.readOnlyProperty') + ->build(); } continue; diff --git a/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php b/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php index b12e6365cf5..d062d09af14 100644 --- a/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php +++ b/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php @@ -191,6 +191,64 @@ public function testCloneWith(): void $this->analyse([__DIR__ . '/data/readonly-property-assign-clone-with.php'], []); } + #[RequiresPhp('>= 8.1.0')] + public function testBug11495(): void + { + if (PHP_VERSION_ID < 80300) { + $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, + ], + [ + 'Readonly property Bug11495\DoubleAssign::$foo is assigned outside of the constructor.', + 40, + ], + [ + 'Readonly property Bug11495\DoubleAssign::$foo is assigned outside of the constructor.', + 41, + ], + [ + 'Readonly property Bug11495\BranchedAssign::$foo is assigned outside of the constructor.', + 57, + ], + [ + 'Readonly property Bug11495\BranchedAssign::$foo is assigned outside of the constructor.', + 59, + ], + [ + 'Readonly property Bug11495\ConditionalThenAssign::$foo is assigned outside of the constructor.', + 76, + ], + [ + 'Readonly property Bug11495\ConditionalThenAssign::$foo is assigned outside of the constructor.', + 78, + ], + ]; + } else { + $errors = [ + [ + 'Readonly property Bug11495\HelloWorld::$foo is not assigned on $this.', + 20, + ], + [ + 'Readonly property Bug11495\DoubleAssign::$foo is already assigned.', + 41, + ], + [ + 'Readonly property Bug11495\ConditionalThenAssign::$foo is already assigned.', + 78, + ], + ]; + } + + $this->analyse([__DIR__ . '/data/bug-11495.php'], $errors); + } + #[RequiresPhp('>= 8.4.0')] public function testBug12871(): void { 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 00000000000..2f27c28392e --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/bug-11495.php @@ -0,0 +1,80 @@ += 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'; + + $s = new self(); + $s->foo = 'baz'; + } + + public function getFoo(): string + { + return $this->foo; + } +} + +class DoubleAssign +{ + private readonly string $foo; + + public function __construct() + { + $this->foo = 'bar'; + } + + public function __clone() + { + $this->foo = 'baz'; + $this->foo = 'qux'; + } +} + +class BranchedAssign +{ + private readonly string $foo; + + public function __construct() + { + $this->foo = 'bar'; + } + + public function __clone() + { + if (rand(0, 1)) { + $this->foo = 'a'; + } else { + $this->foo = 'b'; + } + } +} + +class ConditionalThenAssign +{ + private readonly string $foo; + + public function __construct() + { + $this->foo = 'bar'; + } + + public function __clone() + { + if (rand(0, 1)) { + $this->foo = 'a'; + } + $this->foo = 'b'; + } +}