Skip to content

Commit

Permalink
Fix readonly property assigned in if-else
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Jul 2, 2023
1 parent 185b964 commit ce3c164
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 13 deletions.
20 changes: 15 additions & 5 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
29 changes: 21 additions & 8 deletions src/Node/ClassPropertiesNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -108,6 +109,9 @@ public function getUninitializedProperties(
if ($property->getDefault() !== null) {
continue;
}
if ($property->isPromoted()) {
continue;
}
$properties[$property->getName()] = $property;
}

Expand Down Expand Up @@ -136,6 +140,7 @@ public function getUninitializedProperties(
$prematureAccess = [];
$additionalAssigns = [];
$originalProperties = $properties;

foreach ($this->getPropertyUsages() as $usage) {
$fetch = $usage->getFetch();
if (!$fetch instanceof PropertyFetch) {
Expand Down Expand Up @@ -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],
];
}
}

Expand Down
34 changes: 34 additions & 0 deletions src/Node/Expr/PropertyInitializationExpr.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php declare(strict_types = 1);

namespace PHPStan\Node\Expr;

use PhpParser\Node\Expr;
use PHPStan\Node\VirtualNode;

class PropertyInitializationExpr extends Expr implements VirtualNode
{

public function __construct(private string $propertyName)
{
parent::__construct([]);
}

public function getPropertyName(): string
{
return $this->propertyName;
}

public function getType(): string
{
return 'PHPStan_Node_PropertyInitializationExpr';
}

/**
* @return string[]
*/
public function getSubNodeNames(): array
{
return [];
}

}
6 changes: 6 additions & 0 deletions src/Node/Printer/Printer.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
],
]);
}

}
32 changes: 32 additions & 0 deletions tests/PHPStan/Rules/Properties/data/bug-6402.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php // lint >= 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;
}
}
}

0 comments on commit ce3c164

Please sign in to comment.