Skip to content

Commit

Permalink
Fix false positives about uninitialized properties
Browse files Browse the repository at this point in the history
  • Loading branch information
staabm authored Apr 4, 2024
1 parent e606fbe commit 677afdd
Show file tree
Hide file tree
Showing 4 changed files with 419 additions and 5 deletions.
20 changes: 15 additions & 5 deletions src/Node/ClassPropertiesNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -152,15 +152,15 @@ public function getUninitializedProperties(
return [$uninitializedProperties, [], []];
}

$methodsCalledFromConstructor = $this->getMethodsCalledFromConstructor($classReflection, $initialInitializedProperties, $initializedProperties, $constructors);
$prematureAccess = [];
$additionalAssigns = [];

$initializedInConstructor = [];
if ($classReflection->hasConstructor()) {
$initializedInConstructor = array_diff_key($uninitializedProperties, $this->collectUninitializedProperties([$classReflection->getConstructor()->getName()], $uninitializedProperties));
}

$methodsCalledFromConstructor = $this->getMethodsCalledFromConstructor($classReflection, $initialInitializedProperties, $initializedProperties, $constructors, $initializedInConstructor);
$prematureAccess = [];
$additionalAssigns = [];

foreach ($this->getPropertyUsages() as $usage) {
$fetch = $usage->getFetch();
if (!$fetch instanceof PropertyFetch) {
Expand Down Expand Up @@ -311,17 +311,21 @@ private function collectUninitializedProperties(array $constructors, array $unin
* @param string[] $methods
* @param array<string, TrinaryLogic> $initialInitializedProperties
* @param array<string, array<string, TrinaryLogic>> $initializedProperties
* @param array<string, ClassPropertyNode> $initializedInConstructorProperties
*
* @return array<string, array<string, TrinaryLogic>>
*/
private function getMethodsCalledFromConstructor(
ClassReflection $classReflection,
array $initialInitializedProperties,
array $initializedProperties,
array $methods,
array $initializedInConstructorProperties,
): array
{
$originalMap = $initializedProperties;
$originalMethods = $methods;

foreach ($this->methodCalls as $methodCall) {
$methodCallNode = $methodCall->getNode();
if ($methodCallNode instanceof Array_) {
Expand Down Expand Up @@ -353,6 +357,12 @@ private function getMethodsCalledFromConstructor(
continue;
}

if ($inMethod->getName() !== '__construct') {
foreach ($initializedInConstructorProperties as $propertyName => $propertyNode) {
$initializedProperties[$inMethod->getName()][$propertyName] = TrinaryLogic::createYes();
}
}

$methodName = $methodCallNode->name->toString();
if (array_key_exists($methodName, $initializedProperties)) {
foreach ($this->getInitializedProperties($callScope, $initializedProperties[$inMethod->getName()] ?? $initialInitializedProperties) as $propertyName => $isInitialized) {
Expand All @@ -375,7 +385,7 @@ private function getMethodsCalledFromConstructor(
return $initializedProperties;
}

return $this->getMethodsCalledFromConstructor($classReflection, $initialInitializedProperties, $initializedProperties, $methods);
return $this->getMethodsCalledFromConstructor($classReflection, $initialInitializedProperties, $initializedProperties, $methods, $initializedInConstructorProperties);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ protected function getRule(): Rule
self::getContainer(),
[
'MissingReadOnlyPropertyAssign\\TestCase::setUp',
'Bug10523\\Controller::init',
'Bug10523\\MultipleWrites::init',
'Bug10523\\SingleWriteInConstructorCalledMethod::init',
],
),
);
Expand Down Expand Up @@ -261,4 +264,27 @@ public function testAnonymousReadonlyClass(): void
]);
}

public function testBug10523(): void
{
if (PHP_VERSION_ID < 80100) {
$this->markTestSkipped('Test requires PHP 8.1.');
}

$this->analyse([__DIR__ . '/data/bug-10523.php'], [
[
'Readonly property Bug10523\MultipleWrites::$userAccount is already assigned.',
55,
],
]);
}

public function testBug10822(): void
{
if (PHP_VERSION_ID < 80100) {
$this->markTestSkipped('Test requires PHP 8.1.');
}

$this->analyse([__DIR__ . '/data/bug-10822.php'], []);
}

}
84 changes: 84 additions & 0 deletions tests/PHPStan/Rules/Properties/data/bug-10523.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
<?php // lint >= 8.1

namespace Bug10523;

final class Controller
{
private readonly B $userAccount;

public function __construct()
{
$this->userAccount = new B();
}

public function init(): void
{
$this->redirectIfNkdeCheckoutNotAllowed();
$this->redirectIfNoShoppingBasketPresent();
}

private function redirectIfNkdeCheckoutNotAllowed(): void
{

}

private function redirectIfNoShoppingBasketPresent(): void
{
$x = $this->userAccount;
}

}

class B {}

final class MultipleWrites
{
private readonly B $userAccount;

public function __construct()
{
$this->userAccount = new B();
}

public function init(): void
{
$this->redirectIfNkdeCheckoutNotAllowed();
$this->redirectIfNoShoppingBasketPresent();
}

private function redirectIfNkdeCheckoutNotAllowed(): void
{
}

private function redirectIfNoShoppingBasketPresent(): void
{
$this->userAccount = new B();
}

}


final class SingleWriteInConstructorCalledMethod
{
private readonly B $userAccount;

public function __construct()
{
}

public function init(): void
{
$this->redirectIfNkdeCheckoutNotAllowed();
$this->redirectIfNoShoppingBasketPresent();
}

private function redirectIfNkdeCheckoutNotAllowed(): void
{
}

private function redirectIfNoShoppingBasketPresent(): void
{
$this->userAccount = new B();
}

}
Loading

0 comments on commit 677afdd

Please sign in to comment.