From ce3c1641926e8abf7ce6dca055717e1657deb792 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 2 Jul 2023 12:58:47 +0200 Subject: [PATCH] Fix readonly property assigned in if-else --- src/Analyser/NodeScopeResolver.php | 20 ++++++++--- src/Node/ClassPropertiesNode.php | 29 +++++++++++----- src/Node/Expr/PropertyInitializationExpr.php | 34 +++++++++++++++++++ src/Node/Printer/Printer.php | 6 ++++ .../MissingReadOnlyPropertyAssignRuleTest.php | 14 ++++++++ .../Rules/Properties/data/bug-6402.php | 32 +++++++++++++++++ 6 files changed, 122 insertions(+), 13 deletions(-) create mode 100644 src/Node/Expr/PropertyInitializationExpr.php create mode 100644 tests/PHPStan/Rules/Properties/data/bug-6402.php diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index de9b31741a..de96aed5e9 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -77,6 +77,7 @@ use PHPStan\Node\Expr\GetIterableValueTypeExpr; use PHPStan\Node\Expr\GetOffsetValueTypeExpr; use PHPStan\Node\Expr\OriginalPropertyTypeExpr; +use PHPStan\Node\Expr\PropertyInitializationExpr; use PHPStan\Node\Expr\SetOffsetValueTypeExpr; use PHPStan\Node\FinallyExitPointsNode; use PHPStan\Node\FunctionCallableNode; @@ -3835,11 +3836,20 @@ static function (): void { $scope = $scope->assignExpression($var, $assignedExprType, $scope->getNativeType($assignedExpr)); } $declaringClass = $propertyReflection->getDeclaringClass(); - if ( - $declaringClass->hasNativeProperty($propertyName) - && !$declaringClass->getNativeProperty($propertyName)->getNativeType()->accepts($assignedExprType, true)->yes() - ) { - $throwPoints[] = ThrowPoint::createExplicit($scope, new ObjectType(TypeError::class), $assignedExpr, false); + if ($declaringClass->hasNativeProperty($propertyName)) { + $nativeProperty = $declaringClass->getNativeProperty($propertyName); + if ( + !$nativeProperty->getNativeType()->accepts($assignedExprType, true)->yes() + ) { + $throwPoints[] = ThrowPoint::createExplicit($scope, new ObjectType(TypeError::class), $assignedExpr, false); + } + if ( + $enterExpressionAssign + && $scope->isInClass() + && $scope->getClassReflection()->getName() === $declaringClass->getName() + ) { + $scope = $scope->assignExpression(new PropertyInitializationExpr($propertyName), new MixedType(), new MixedType()); + } } } else { // fallback diff --git a/src/Node/ClassPropertiesNode.php b/src/Node/ClassPropertiesNode.php index b371482e3e..194fd87959 100644 --- a/src/Node/ClassPropertiesNode.php +++ b/src/Node/ClassPropertiesNode.php @@ -11,6 +11,7 @@ use PhpParser\Node\Stmt\ClassLike; use PhpParser\NodeAbstract; use PHPStan\Analyser\Scope; +use PHPStan\Node\Expr\PropertyInitializationExpr; use PHPStan\Node\Method\MethodCall; use PHPStan\Node\Property\PropertyRead; use PHPStan\Node\Property\PropertyWrite; @@ -108,6 +109,9 @@ public function getUninitializedProperties( if ($property->getDefault() !== null) { continue; } + if ($property->isPromoted()) { + continue; + } $properties[$property->getName()] = $property; } @@ -136,6 +140,7 @@ public function getUninitializedProperties( $prematureAccess = []; $additionalAssigns = []; $originalProperties = $properties; + foreach ($this->getPropertyUsages() as $usage) { $fetch = $usage->getFetch(); if (!$fetch instanceof PropertyFetch) { @@ -173,19 +178,27 @@ public function getUninitializedProperties( if ($usage instanceof PropertyWrite) { if (array_key_exists($propertyName, $properties)) { unset($properties[$propertyName]); - } elseif (array_key_exists($propertyName, $originalProperties)) { - $additionalAssigns[] = [ + } + + if (array_key_exists($propertyName, $originalProperties)) { + $hasInitialization = $usageScope->hasExpressionType(new PropertyInitializationExpr($propertyName)); + if (!$hasInitialization->no()) { + $additionalAssigns[] = [ + $propertyName, + $fetch->getLine(), + $originalProperties[$propertyName], + ]; + } + } + } elseif (array_key_exists($propertyName, $originalProperties)) { + $hasInitialization = $usageScope->hasExpressionType(new PropertyInitializationExpr($propertyName)); + if (!$hasInitialization->yes()) { + $prematureAccess[] = [ $propertyName, $fetch->getLine(), $originalProperties[$propertyName], ]; } - } elseif (array_key_exists($propertyName, $properties)) { - $prematureAccess[] = [ - $propertyName, - $fetch->getLine(), - $properties[$propertyName], - ]; } } diff --git a/src/Node/Expr/PropertyInitializationExpr.php b/src/Node/Expr/PropertyInitializationExpr.php new file mode 100644 index 0000000000..942fa08d5c --- /dev/null +++ b/src/Node/Expr/PropertyInitializationExpr.php @@ -0,0 +1,34 @@ +propertyName; + } + + public function getType(): string + { + return 'PHPStan_Node_PropertyInitializationExpr'; + } + + /** + * @return string[] + */ + public function getSubNodeNames(): array + { + return []; + } + +} diff --git a/src/Node/Printer/Printer.php b/src/Node/Printer/Printer.php index e35f5d8190..a0e5c662a9 100644 --- a/src/Node/Printer/Printer.php +++ b/src/Node/Printer/Printer.php @@ -8,6 +8,7 @@ use PHPStan\Node\Expr\GetIterableValueTypeExpr; use PHPStan\Node\Expr\GetOffsetValueTypeExpr; use PHPStan\Node\Expr\OriginalPropertyTypeExpr; +use PHPStan\Node\Expr\PropertyInitializationExpr; use PHPStan\Node\Expr\SetOffsetValueTypeExpr; use PHPStan\Node\Expr\TypeExpr; use PHPStan\Type\VerbosityLevel; @@ -51,4 +52,9 @@ protected function pPHPStan_Node_AlwaysRememberedExpr(AlwaysRememberedExpr $expr return sprintf('__phpstanRembered(%s)', $this->p($expr->getExpr())); } + protected function pPHPStan_Node_PropertyInitializationExpr(PropertyInitializationExpr $expr): string // phpcs:ignore + { + return sprintf('__phpstanPropertyInitialization(%s)', $expr->getPropertyName()); + } + } diff --git a/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php b/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php index 902e7cd03e..b9e155d837 100644 --- a/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php +++ b/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php @@ -156,4 +156,18 @@ public function testBug8563(): void $this->analyse([__DIR__ . '/data/bug-8563.php'], []); } + public function testBug6402(): void + { + if (PHP_VERSION_ID < 80100) { + $this->markTestSkipped('Test requires PHP 8.1.'); + } + + $this->analyse([__DIR__ . '/data/bug-6402.php'], [ + [ + 'Access to an uninitialized readonly property Bug6402\SomeModel2::$views.', + 28, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Properties/data/bug-6402.php b/tests/PHPStan/Rules/Properties/data/bug-6402.php new file mode 100644 index 0000000000..16f9c8c2aa --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/bug-6402.php @@ -0,0 +1,32 @@ += 8.1 + +namespace Bug6402; + +class SomeModel +{ + public readonly ?int $views; + + public function __construct(string $mode, int $views) + { + if ($mode === 'mode1') { + $this->views = $views; + } else { + $this->views = null; + } + } +} + +class SomeModel2 +{ + public readonly ?int $views; + + public function __construct(string $mode, int $views) + { + if ($mode === 'mode1') { + $this->views = $views; + } else { + echo $this->views; + $this->views = null; + } + } +}