Skip to content

Commit

Permalink
Evaluate uninitialized properties based on each execution end in cons…
Browse files Browse the repository at this point in the history
…tructor

Closes phpstan/phpstan#7649
  • Loading branch information
ondrejmirtes committed Jul 5, 2023
1 parent 2d2cae1 commit b2d2a9d
Show file tree
Hide file tree
Showing 9 changed files with 203 additions and 5 deletions.
8 changes: 7 additions & 1 deletion src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,7 @@ private function processStmtNode(
$this->processAttributeGroups($stmt->attrGroups, $classScope, $classStatementsGatherer);

$this->processStmtNodes($stmt, $stmt->stmts, $classScope, $classStatementsGatherer, $context);
$nodeCallback(new ClassPropertiesNode($stmt, $this->readWritePropertiesExtensionProvider, $classStatementsGatherer->getProperties(), $classStatementsGatherer->getPropertyUsages(), $classStatementsGatherer->getMethodCalls()), $classScope);
$nodeCallback(new ClassPropertiesNode($stmt, $this->readWritePropertiesExtensionProvider, $classStatementsGatherer->getProperties(), $classStatementsGatherer->getPropertyUsages(), $classStatementsGatherer->getMethodCalls(), $classStatementsGatherer->getReturnStatementsNodes()), $classScope);
$nodeCallback(new ClassMethodsNode($stmt, $classStatementsGatherer->getMethods(), $classStatementsGatherer->getMethodCalls()), $classScope);
$nodeCallback(new ClassConstantsNode($stmt, $classStatementsGatherer->getConstants(), $classStatementsGatherer->getConstantFetches()), $classScope);
$classReflection->evictPrivateSymbols();
Expand Down Expand Up @@ -3814,6 +3814,9 @@ private function processAssignVar(
} else {
if ($var instanceof PropertyFetch || $var instanceof StaticPropertyFetch) {
$nodeCallback(new PropertyAssignNode($var, $assignedPropertyExpr, $isAssignOp), $scope);
if ($var instanceof PropertyFetch && $var->name instanceof Node\Identifier && !$isAssignOp) {
$scope = $scope->assignInitializedProperty($scope->getType($var->var), $var->name->toString());
}
}
$scope = $scope->assignExpression(
$var,
Expand All @@ -3835,6 +3838,9 @@ private function processAssignVar(
} else {
if ($var instanceof PropertyFetch || $var instanceof StaticPropertyFetch) {
$nodeCallback(new PropertyAssignNode($var, $assignedPropertyExpr, $isAssignOp), $scope);
if ($var instanceof PropertyFetch && $var->name instanceof Node\Identifier && !$isAssignOp) {
$scope = $scope->assignInitializedProperty($scope->getType($var->var), $var->name->toString());
}
}
}

Expand Down
59 changes: 55 additions & 4 deletions src/Node/ClassPropertiesNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@
use PHPStan\Rules\Properties\ReadWritePropertiesExtensionProvider;
use PHPStan\ShouldNotHappenException;
use PHPStan\TrinaryLogic;
use PHPStan\Type\NeverType;
use PHPStan\Type\TypeUtils;
use function array_key_exists;
use function array_keys;
use function in_array;
use function strtolower;

/** @api */
class ClassPropertiesNode extends NodeAbstract implements VirtualNode
Expand All @@ -34,13 +36,15 @@ class ClassPropertiesNode extends NodeAbstract implements VirtualNode
* @param ClassPropertyNode[] $properties
* @param array<int, PropertyRead|PropertyWrite> $propertyUsages
* @param array<int, MethodCall> $methodCalls
* @param array<string, MethodReturnStatementsNode> $returnStatementNodes
*/
public function __construct(
private ClassLike $class,
private ReadWritePropertiesExtensionProvider $readWritePropertiesExtensionProvider,
private array $properties,
private array $propertyUsages,
private array $methodCalls,
private array $returnStatementNodes,
)
{
parent::__construct($class->getAttributes());
Expand Down Expand Up @@ -191,10 +195,6 @@ public function getUninitializedProperties(
}

if ($usage instanceof PropertyWrite) {
if (array_key_exists($propertyName, $uninitializedProperties)) {
unset($uninitializedProperties[$propertyName]);
}

if (array_key_exists($propertyName, $initializedPropertiesMap)) {
$hasInitialization = $initializedPropertiesMap[$propertyName]->or($usageScope->hasExpressionType(new PropertyInitializationExpr($propertyName)));
if (!$hasInitialization->no() && !$usage->isPromotedPropertyWrite()) {
Expand All @@ -217,6 +217,57 @@ public function getUninitializedProperties(
}
}

foreach (array_keys($methodsCalledFromConstructor) as $constructor) {
$lowerConstructorName = strtolower($constructor);
if (!array_key_exists($lowerConstructorName, $this->returnStatementNodes)) {
continue;
}

$returnStatementsNode = $this->returnStatementNodes[$lowerConstructorName];
$methodScope = null;
foreach ($returnStatementsNode->getExecutionEnds() as $executionEnd) {
$statementResult = $executionEnd->getStatementResult();
$endNode = $executionEnd->getNode();
if ($statementResult->isAlwaysTerminating()) {
if ($endNode instanceof Node\Stmt\Throw_) {
continue;
}
if ($endNode instanceof Node\Stmt\Expression) {
$exprType = $statementResult->getScope()->getType($endNode->expr);
if ($exprType instanceof NeverType && $exprType->isExplicit()) {
continue;
}
}
}
if ($methodScope === null) {
$methodScope = $statementResult->getScope();
continue;
}

$methodScope = $methodScope->mergeWith($statementResult->getScope());
}

foreach ($returnStatementsNode->getReturnStatements() as $returnStatement) {
if ($methodScope === null) {
$methodScope = $returnStatement->getScope();
continue;
}
$methodScope = $methodScope->mergeWith($returnStatement->getScope());
}

if ($methodScope === null) {
continue;
}

foreach (array_keys($uninitializedProperties) as $propertyName) {
if (!$methodScope->hasExpressionType(new PropertyInitializationExpr($propertyName))->yes()) {
continue;
}

unset($uninitializedProperties[$propertyName]);
}
}

return [
$uninitializedProperties,
$prematureAccess,
Expand Down
16 changes: 16 additions & 0 deletions src/Node/ClassStatementsGatherer.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use PHPStan\Type\TypeUtils;
use function count;
use function in_array;
use function strtolower;

class ClassStatementsGatherer
{
Expand Down Expand Up @@ -50,6 +51,9 @@ class ClassStatementsGatherer
/** @var ClassConstantFetch[] */
private array $constantFetches = [];

/** @var array<string, MethodReturnStatementsNode> */
private array $returnStatementNodes = [];

/**
* @param callable(Node $node, Scope $scope): void $nodeCallback
*/
Expand Down Expand Up @@ -109,6 +113,14 @@ public function getConstantFetches(): array
return $this->constantFetches;
}

/**
* @return array<string, MethodReturnStatementsNode>
*/
public function getReturnStatementsNodes(): array
{
return $this->returnStatementNodes;
}

public function __invoke(Node $node, Scope $scope): void
{
$nodeCallback = $this->nodeCallback;
Expand Down Expand Up @@ -151,6 +163,10 @@ private function gatherNodes(Node $node, Scope $scope): void
$this->methodCalls[] = new \PHPStan\Node\Method\MethodCall($node->getOriginalNode(), $scope);
return;
}
if ($node instanceof MethodReturnStatementsNode) {
$this->returnStatementNodes[strtolower($node->getMethodName())] = $node;
return;
}
if (
$node instanceof Expr\FuncCall
&& $node->name instanceof Node\Name
Expand Down
5 changes: 5 additions & 0 deletions src/Node/MethodReturnStatementsNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ public function hasNativeReturnTypehint(): bool
return $this->classMethod->returnType !== null;
}

public function getMethodName(): string
{
return $this->classMethod->name->toString();
}

public function getYieldStatements(): array
{
return $this->yieldStatements;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ public function testRule(): void
'Class MissingReadOnlyPropertyAssignPhpDoc\BarDoubleAssignInSetter has an uninitialized @readonly property $foo. Assign it in the constructor.',
57,
],
[
'Class MissingReadOnlyPropertyAssignPhpDoc\AssignOp has an uninitialized @readonly property $foo. Assign it in the constructor.',
85,
],
[
'Access to an uninitialized @readonly property MissingReadOnlyPropertyAssignPhpDoc\AssignOp::$foo.',
92,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ public function testRule(): void
'Class MissingReadOnlyPropertyAssign\BarDoubleAssignInSetter has an uninitialized readonly property $foo. Assign it in the constructor.',
53,
],
[
'Class MissingReadOnlyPropertyAssign\AssignOp has an uninitialized readonly property $foo. Assign it in the constructor.',
79,
],
[
'Access to an uninitialized readonly property MissingReadOnlyPropertyAssign\AssignOp::$foo.',
85,
Expand Down Expand Up @@ -195,4 +199,18 @@ public function testBug7198(): void
$this->analyse([__DIR__ . '/data/bug-7198.php'], []);
}

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

$this->analyse([__DIR__ . '/data/bug-7649.php'], [
[
'Class Bug7649\Foo has an uninitialized readonly property $bar. Assign it in the constructor.',
7,
],
]);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,18 @@ public function testRule(): void
'Access to an uninitialized property UninitializedProperty\InitializedInPublicSetterNonFinalClass::$foo.',
278,
],*/
[
'Class UninitializedProperty\SometimesInitializedInPrivateSetter has an uninitialized property $foo. Give it default value or assign it in the constructor.',
286,
],
[
'Access to an uninitialized property UninitializedProperty\SometimesInitializedInPrivateSetter::$foo.',
303,
],
[
'Class UninitializedProperty\EarlyReturn has an uninitialized property $foo. Give it default value or assign it in the constructor.',
372,
],
]);
}

Expand Down
16 changes: 16 additions & 0 deletions tests/PHPStan/Rules/Properties/data/bug-7649.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

namespace Bug7649;

class Foo
{
public readonly string $bar;

public function __construct(bool $flag)
{
if ($flag) {
$this->bar = 'baz';
} else {
}
}
}
74 changes: 74 additions & 0 deletions tests/PHPStan/Rules/Properties/data/uninitialized-property.php
Original file line number Diff line number Diff line change
Expand Up @@ -332,3 +332,77 @@ public function doSomething()
}

}

class ThrowInConstructor1
{

private int $foo;

public function __construct()
{
if (rand(0, 1)) {
$this->foo = 1;
return;
}

throw new \Exception;
}

}

class ThrowInConstructor2
{

private int $foo;

public function __construct()
{
if (rand(0, 1)) {
throw new \Exception;
}

$this->foo = 1;
}

}

class EarlyReturn
{

private int $foo;

public function __construct()
{
if (rand(0, 1)) {
return;
}

$this->foo = 1;
}

}

class NeverInConstructor
{

private int $foo;

public function __construct()
{
if (rand(0, 1)) {
$this->foo = 1;
return;
}

$this->returnNever();
}

/**
* @return never
*/
private function returnNever()
{
throw new \Exception();
}

}

0 comments on commit b2d2a9d

Please sign in to comment.