From fa0c4c5e13cef00fc801ca0b03b96f87389b9b89 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 27 Jul 2026 11:24:45 +0200 Subject: [PATCH 1/3] [DeadCode] Use PHPStan flow analysis for property initialization in RemoveDefaultValueFromAssignedPropertyRector Decorate every ClassMethod with the scope merged from all its execution ends and return statements, so rules can ask PHPStan whether a property is initialized on every path out of the constructor. This replaces the hand-rolled "any return bails out" heuristic and covers if/elseif/else, switch, loops, try/catch and closures for free. --- .../assign_after_throwing_branch.php.inc | 43 ++++++++++++ .../Fixture/assign_in_if_elseif_else.php.inc | 43 ++++++++++++ .../assign_in_switch_with_default.php.inc | 43 ++++++++++++ .../Fixture/return_after_assign.php.inc | 43 ++++++++++++ .../Fixture/skip_assign_in_foreach.php.inc | 18 +++++ .../skip_assign_inside_closure.php.inc | 19 ++++++ .../Fixture/skip_assign_only_in_try.php.inc | 18 +++++ .../Fixture/skip_assign_reads_default.php.inc | 13 ++++ .../skip_assign_via_offset_access.php.inc | 16 +++++ ...DefaultValueFromAssignedPropertyRector.php | 55 +++++++++++++-- src/NodeTypeResolver/Node/AttributeKey.php | 6 ++ .../Scope/PHPStanNodeScopeResolver.php | 67 +++++++++++++++++++ 12 files changed, 379 insertions(+), 5 deletions(-) create mode 100644 rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/assign_after_throwing_branch.php.inc create mode 100644 rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/assign_in_if_elseif_else.php.inc create mode 100644 rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/assign_in_switch_with_default.php.inc create mode 100644 rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/return_after_assign.php.inc create mode 100644 rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/skip_assign_in_foreach.php.inc create mode 100644 rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/skip_assign_inside_closure.php.inc create mode 100644 rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/skip_assign_only_in_try.php.inc create mode 100644 rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/skip_assign_reads_default.php.inc create mode 100644 rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/skip_assign_via_offset_access.php.inc diff --git a/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/assign_after_throwing_branch.php.inc b/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/assign_after_throwing_branch.php.inc new file mode 100644 index 00000000000..936a4cef48a --- /dev/null +++ b/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/assign_after_throwing_branch.php.inc @@ -0,0 +1,43 @@ +name = $name; + } +} + +?> +----- +name = $name; + } +} + +?> diff --git a/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/assign_in_if_elseif_else.php.inc b/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/assign_in_if_elseif_else.php.inc new file mode 100644 index 00000000000..4fe52fd6a18 --- /dev/null +++ b/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/assign_in_if_elseif_else.php.inc @@ -0,0 +1,43 @@ +name = 'one'; + } elseif ($type === 2) { + $this->name = 'two'; + } else { + $this->name = 'many'; + } + } +} + +?> +----- +name = 'one'; + } elseif ($type === 2) { + $this->name = 'two'; + } else { + $this->name = 'many'; + } + } +} + +?> diff --git a/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/assign_in_switch_with_default.php.inc b/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/assign_in_switch_with_default.php.inc new file mode 100644 index 00000000000..53441210a9b --- /dev/null +++ b/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/assign_in_switch_with_default.php.inc @@ -0,0 +1,43 @@ +name = 'one'; + break; + default: + $this->name = 'many'; + } + } +} + +?> +----- +name = 'one'; + break; + default: + $this->name = 'many'; + } + } +} + +?> diff --git a/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/return_after_assign.php.inc b/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/return_after_assign.php.inc new file mode 100644 index 00000000000..31df9613088 --- /dev/null +++ b/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/return_after_assign.php.inc @@ -0,0 +1,43 @@ +name = $name; + + if ($shouldReturn) { + return; + } + + $this->name = strtoupper($name); + } +} + +?> +----- +name = $name; + + if ($shouldReturn) { + return; + } + + $this->name = strtoupper($name); + } +} + +?> diff --git a/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/skip_assign_in_foreach.php.inc b/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/skip_assign_in_foreach.php.inc new file mode 100644 index 00000000000..f9109c26264 --- /dev/null +++ b/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/skip_assign_in_foreach.php.inc @@ -0,0 +1,18 @@ +name = $name; + } + } +} diff --git a/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/skip_assign_inside_closure.php.inc b/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/skip_assign_inside_closure.php.inc new file mode 100644 index 00000000000..f92434bbc0a --- /dev/null +++ b/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/skip_assign_inside_closure.php.inc @@ -0,0 +1,19 @@ +setName = function () use ($name): void { + $this->name = $name; + }; + } +} diff --git a/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/skip_assign_only_in_try.php.inc b/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/skip_assign_only_in_try.php.inc new file mode 100644 index 00000000000..abe68815c82 --- /dev/null +++ b/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/skip_assign_only_in_try.php.inc @@ -0,0 +1,18 @@ +name = json_encode($name, JSON_THROW_ON_ERROR); + } catch (Throwable) { + } + } +} diff --git a/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/skip_assign_reads_default.php.inc b/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/skip_assign_reads_default.php.inc new file mode 100644 index 00000000000..341a0733b59 --- /dev/null +++ b/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/skip_assign_reads_default.php.inc @@ -0,0 +1,13 @@ +count = $this->count + $extra; + } +} diff --git a/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/skip_assign_via_offset_access.php.inc b/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/skip_assign_via_offset_access.php.inc new file mode 100644 index 00000000000..30569d913e7 --- /dev/null +++ b/rules-tests/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector/Fixture/skip_assign_via_offset_access.php.inc @@ -0,0 +1,16 @@ + + */ + private array $cachedByName = []; + + public function __construct(string $name) + { + $this->cachedByName[$name] = true; + } +} diff --git a/rules/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector.php b/rules/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector.php index 6dd4b633d5a..1ec0cb85744 100644 --- a/rules/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector.php +++ b/rules/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector.php @@ -6,9 +6,14 @@ use PhpParser\Node; use PhpParser\Node\Expr; +use PhpParser\Node\Expr\ArrayDimFetch; +use PhpParser\Node\Expr\Assign; use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassMethod; -use PhpParser\Node\Stmt\Return_; +use PHPStan\Analyser\Scope; +use PHPStan\Node\Expr\PropertyInitializationExpr; +use Rector\NodeAnalyzer\PropertyFetchAnalyzer; +use Rector\NodeTypeResolver\Node\AttributeKey; use Rector\PhpParser\Node\BetterNodeFinder; use Rector\Rector\AbstractRector; use Rector\TypeDeclaration\AlreadyAssignDetector\ConstructorAssignDetector; @@ -23,7 +28,8 @@ final class RemoveDefaultValueFromAssignedPropertyRector extends AbstractRector { public function __construct( private readonly ConstructorAssignDetector $constructorAssignDetector, - private readonly BetterNodeFinder $betterNodeFinder + private readonly BetterNodeFinder $betterNodeFinder, + private readonly PropertyFetchAnalyzer $propertyFetchAnalyzer ) { } @@ -79,8 +85,9 @@ public function refactor(Node $node): ?Node return null; } - // early return can skip the assign, so the default value is still needed - if ($this->betterNodeFinder->hasInstancesOfInFunctionLikeScoped($constructClassMethod, Return_::class)) { + // the scope of every constructor exit point, as resolved by PHPStan flow analysis + $executionEndScope = $constructClassMethod->getAttribute(AttributeKey::EXECUTION_END_SCOPE); + if (! $executionEndScope instanceof Scope) { return null; } @@ -102,7 +109,20 @@ public function refactor(Node $node): ?Node } $propertyName = $this->getName($propertyProperty); - if (! $this->constructorAssignDetector->isPropertyAssigned($node, $propertyName)) { + + // guards against an assign that reads the default value first, e.g. $this->value = $this->value + 1 + if (! $this->constructorAssignDetector->isPropertyAssignedConditionally($node, $propertyName)) { + continue; + } + + // the property must be initialized on every path out of the constructor, + // otherwise removing the default value leaves it uninitialized + if (! $executionEndScope->hasExpressionType(new PropertyInitializationExpr($propertyName))->yes()) { + continue; + } + + // $this->value['key'] = ... writes to the default value, it does not replace it + if ($this->isAssignedViaOffsetAccess($constructClassMethod, $propertyName)) { continue; } @@ -117,4 +137,29 @@ public function refactor(Node $node): ?Node return null; } + + private function isAssignedViaOffsetAccess(ClassMethod $constructClassMethod, string $propertyName): bool + { + $assign = $this->betterNodeFinder->findFirstInFunctionLikeScoped( + $constructClassMethod, + function (Node $node) use ($propertyName): bool { + if (! $node instanceof Assign) { + return false; + } + + $assignVar = $node->var; + if (! $assignVar instanceof ArrayDimFetch) { + return false; + } + + while ($assignVar instanceof ArrayDimFetch) { + $assignVar = $assignVar->var; + } + + return $this->propertyFetchAnalyzer->isLocalPropertyFetchName($assignVar, $propertyName); + } + ); + + return $assign instanceof Assign; + } } diff --git a/src/NodeTypeResolver/Node/AttributeKey.php b/src/NodeTypeResolver/Node/AttributeKey.php index 33f40638ebe..4ce8d0f44d2 100644 --- a/src/NodeTypeResolver/Node/AttributeKey.php +++ b/src/NodeTypeResolver/Node/AttributeKey.php @@ -176,4 +176,10 @@ final class AttributeKey public const string IS_IN_TRY_BLOCK = 'is_in_try_block'; public const string NEWLINE_ON_FLUENT_CALL = 'newline_on_fluent_call'; + + /** + * Scope merged from every execution end and return statement of a class method, + * so it describes what is certain once the method has finished + */ + public const string EXECUTION_END_SCOPE = 'execution_end_scope'; } diff --git a/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php b/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php index 7c73de9d1ab..9f7e69db459 100644 --- a/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php +++ b/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php @@ -87,11 +87,13 @@ use PHPStan\Analyser\Fiber\FiberScope; use PHPStan\Analyser\MutatingScope; use PHPStan\Analyser\NodeScopeResolver; +use PHPStan\Analyser\Scope; use PHPStan\Analyser\ScopeContext; use PHPStan\Analyser\UndefinedVariableException; use PHPStan\Node\FunctionCallableNode; use PHPStan\Node\InstantiationCallableNode; use PHPStan\Node\MethodCallableNode; +use PHPStan\Node\MethodReturnStatementsNode; use PHPStan\Node\Printer\Printer; use PHPStan\Node\StaticMethodCallableNode; use PHPStan\Node\UnreachableStatementNode; @@ -153,8 +155,17 @@ public function processNodes( $scope = $formerMutatingScope ?? $this->scopeFactory->createFromFile($filePath); + /** + * PHPStan emits the MethodReturnStatementsNode without the ClassMethod it belongs to, + * but with its attributes copied over, so the start file position pairs them back together + * + * @var array + */ + $classMethodsByStartFilePos = []; + $nodeCallback = function (Node $node, MutatingScope $mutatingScope) use ( &$nodeCallback, + &$classMethodsByStartFilePos, $filePath, ): void { if ($mutatingScope instanceof FiberScope) { @@ -194,11 +205,21 @@ public function processNodes( return; } + // emitted once the whole class method body is resolved, so the class method is already known + if ($node instanceof MethodReturnStatementsNode) { + $this->processMethodReturnStatementsNode($node, $classMethodsByStartFilePos); + return; + } + // init current Node set Attribute // not a VirtualNode, then set scope attribute // do not return early, as its properties will be checked next if (! $node instanceof VirtualNode) { $node->setAttribute(AttributeKey::SCOPE, $mutatingScope); + + if ($node instanceof ClassMethod && $node->getStartFilePos() >= 0) { + $classMethodsByStartFilePos[$node->getStartFilePos()] = $node; + } } // handle unwrapped stmts @@ -609,6 +630,52 @@ private function processUnreachableStatementNode( ); } + /** + * @param array $classMethodsByStartFilePos + */ + private function processMethodReturnStatementsNode( + MethodReturnStatementsNode $methodReturnStatementsNode, + array $classMethodsByStartFilePos + ): void { + $classMethod = $classMethodsByStartFilePos[$methodReturnStatementsNode->getStartFilePos()] ?? null; + if (! $classMethod instanceof ClassMethod) { + return; + } + + $executionEndScope = null; + + foreach ($methodReturnStatementsNode->getExecutionEnds() as $executionEndNode) { + $executionEndScope = $this->mergeScopes( + $executionEndScope, + $executionEndNode->getStatementResult() + ->getScope() + ); + } + + foreach ($methodReturnStatementsNode->getReturnStatements() as $returnStatement) { + $executionEndScope = $this->mergeScopes($executionEndScope, $returnStatement->getScope()); + } + + if (! $executionEndScope instanceof MutatingScope) { + return; + } + + $classMethod->setAttribute(AttributeKey::EXECUTION_END_SCOPE, $executionEndScope); + } + + private function mergeScopes(?MutatingScope $mutatingScope, Scope $scope): ?MutatingScope + { + if (! $scope instanceof MutatingScope) { + return $mutatingScope; + } + + if (! $mutatingScope instanceof MutatingScope) { + return $scope; + } + + return $mutatingScope->mergeWith($scope); + } + /** * @param callable(Node $node, MutatingScope $scope): void $nodeCallback */ From c19cbd96eb460ff7ddbf557c138fdbfc54dce209 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 27 Jul 2026 11:40:11 +0200 Subject: [PATCH 2/3] Extract AlwaysInitializedPropertyAnalyzer and resolve the exit point scope lazily The scope resolver now only stores the raw MethodReturnStatementsNode on the ClassMethod. Merging the exit point scopes happens in the new analyzer, on demand, so methods no rule asks about cost nothing. --- ...DefaultValueFromAssignedPropertyRector.php | 18 ++--- .../AlwaysInitializedPropertyAnalyzer.php | 73 +++++++++++++++++++ src/NodeTypeResolver/Node/AttributeKey.php | 6 +- .../Scope/PHPStanNodeScopeResolver.php | 35 +-------- .../AlwaysInitializedPropertyAnalyzerTest.php | 72 ++++++++++++++++++ .../AssignInForeach.php | 20 +++++ .../DirectAssign.php | 17 +++++ .../AlwaysInitializedProperty/EarlyReturn.php | 19 +++++ .../EveryBranchAssign.php | 21 ++++++ 9 files changed, 234 insertions(+), 47 deletions(-) create mode 100644 src/NodeAnalyzer/AlwaysInitializedPropertyAnalyzer.php create mode 100644 tests/NodeAnalyzer/AlwaysInitializedPropertyAnalyzerTest.php create mode 100644 tests/NodeAnalyzer/Source/AlwaysInitializedProperty/AssignInForeach.php create mode 100644 tests/NodeAnalyzer/Source/AlwaysInitializedProperty/DirectAssign.php create mode 100644 tests/NodeAnalyzer/Source/AlwaysInitializedProperty/EarlyReturn.php create mode 100644 tests/NodeAnalyzer/Source/AlwaysInitializedProperty/EveryBranchAssign.php diff --git a/rules/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector.php b/rules/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector.php index 1ec0cb85744..e4fc84c0e5c 100644 --- a/rules/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector.php +++ b/rules/DeadCode/Rector/Property/RemoveDefaultValueFromAssignedPropertyRector.php @@ -10,10 +10,8 @@ use PhpParser\Node\Expr\Assign; use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassMethod; -use PHPStan\Analyser\Scope; -use PHPStan\Node\Expr\PropertyInitializationExpr; +use Rector\NodeAnalyzer\AlwaysInitializedPropertyAnalyzer; use Rector\NodeAnalyzer\PropertyFetchAnalyzer; -use Rector\NodeTypeResolver\Node\AttributeKey; use Rector\PhpParser\Node\BetterNodeFinder; use Rector\Rector\AbstractRector; use Rector\TypeDeclaration\AlreadyAssignDetector\ConstructorAssignDetector; @@ -29,7 +27,8 @@ final class RemoveDefaultValueFromAssignedPropertyRector extends AbstractRector public function __construct( private readonly ConstructorAssignDetector $constructorAssignDetector, private readonly BetterNodeFinder $betterNodeFinder, - private readonly PropertyFetchAnalyzer $propertyFetchAnalyzer + private readonly PropertyFetchAnalyzer $propertyFetchAnalyzer, + private readonly AlwaysInitializedPropertyAnalyzer $alwaysInitializedPropertyAnalyzer ) { } @@ -85,12 +84,6 @@ public function refactor(Node $node): ?Node return null; } - // the scope of every constructor exit point, as resolved by PHPStan flow analysis - $executionEndScope = $constructClassMethod->getAttribute(AttributeKey::EXECUTION_END_SCOPE); - if (! $executionEndScope instanceof Scope) { - return null; - } - $hasChanged = false; foreach ($node->getProperties() as $property) { @@ -117,7 +110,10 @@ public function refactor(Node $node): ?Node // the property must be initialized on every path out of the constructor, // otherwise removing the default value leaves it uninitialized - if (! $executionEndScope->hasExpressionType(new PropertyInitializationExpr($propertyName))->yes()) { + if (! $this->alwaysInitializedPropertyAnalyzer->isAlwaysInitialized( + $constructClassMethod, + $propertyName + )) { continue; } diff --git a/src/NodeAnalyzer/AlwaysInitializedPropertyAnalyzer.php b/src/NodeAnalyzer/AlwaysInitializedPropertyAnalyzer.php new file mode 100644 index 00000000000..e3d8d06e197 --- /dev/null +++ b/src/NodeAnalyzer/AlwaysInitializedPropertyAnalyzer.php @@ -0,0 +1,73 @@ +resolveExecutionEndScope($classMethod); + if (! $executionEndScope instanceof Scope) { + return false; + } + + return $executionEndScope->hasExpressionType(new PropertyInitializationExpr($propertyName)) + ->yes(); + } + + /** + * Scope merged from every execution end and return statement, so it describes + * what is certain once the class method has finished, no matter which path was taken + */ + private function resolveExecutionEndScope(ClassMethod $classMethod): ?MutatingScope + { + $methodReturnStatementsNode = $classMethod->getAttribute(AttributeKey::METHOD_RETURN_STATEMENTS_NODE); + if (! $methodReturnStatementsNode instanceof MethodReturnStatementsNode) { + return null; + } + + $executionEndScope = null; + + foreach ($methodReturnStatementsNode->getExecutionEnds() as $executionEndNode) { + $executionEndScope = $this->mergeScopes( + $executionEndScope, + $executionEndNode->getStatementResult() + ->getScope() + ); + } + + foreach ($methodReturnStatementsNode->getReturnStatements() as $returnStatement) { + $executionEndScope = $this->mergeScopes($executionEndScope, $returnStatement->getScope()); + } + + return $executionEndScope; + } + + private function mergeScopes(?MutatingScope $mutatingScope, Scope $scope): ?MutatingScope + { + if (! $scope instanceof MutatingScope) { + return $mutatingScope; + } + + if (! $mutatingScope instanceof MutatingScope) { + return $scope; + } + + return $mutatingScope->mergeWith($scope); + } +} diff --git a/src/NodeTypeResolver/Node/AttributeKey.php b/src/NodeTypeResolver/Node/AttributeKey.php index 4ce8d0f44d2..01fb961a9c8 100644 --- a/src/NodeTypeResolver/Node/AttributeKey.php +++ b/src/NodeTypeResolver/Node/AttributeKey.php @@ -178,8 +178,8 @@ final class AttributeKey public const string NEWLINE_ON_FLUENT_CALL = 'newline_on_fluent_call'; /** - * Scope merged from every execution end and return statement of a class method, - * so it describes what is certain once the method has finished + * PHPStan virtual node that holds every exit point of a class method, + * @see \Rector\NodeAnalyzer\AlwaysInitializedPropertyAnalyzer to read it */ - public const string EXECUTION_END_SCOPE = 'execution_end_scope'; + public const string METHOD_RETURN_STATEMENTS_NODE = 'method_return_statements_node'; } diff --git a/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php b/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php index 9f7e69db459..362a7bb8582 100644 --- a/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php +++ b/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php @@ -87,7 +87,6 @@ use PHPStan\Analyser\Fiber\FiberScope; use PHPStan\Analyser\MutatingScope; use PHPStan\Analyser\NodeScopeResolver; -use PHPStan\Analyser\Scope; use PHPStan\Analyser\ScopeContext; use PHPStan\Analyser\UndefinedVariableException; use PHPStan\Node\FunctionCallableNode; @@ -642,38 +641,8 @@ private function processMethodReturnStatementsNode( return; } - $executionEndScope = null; - - foreach ($methodReturnStatementsNode->getExecutionEnds() as $executionEndNode) { - $executionEndScope = $this->mergeScopes( - $executionEndScope, - $executionEndNode->getStatementResult() - ->getScope() - ); - } - - foreach ($methodReturnStatementsNode->getReturnStatements() as $returnStatement) { - $executionEndScope = $this->mergeScopes($executionEndScope, $returnStatement->getScope()); - } - - if (! $executionEndScope instanceof MutatingScope) { - return; - } - - $classMethod->setAttribute(AttributeKey::EXECUTION_END_SCOPE, $executionEndScope); - } - - private function mergeScopes(?MutatingScope $mutatingScope, Scope $scope): ?MutatingScope - { - if (! $scope instanceof MutatingScope) { - return $mutatingScope; - } - - if (! $mutatingScope instanceof MutatingScope) { - return $scope; - } - - return $mutatingScope->mergeWith($scope); + // kept raw on purpose, merging the exit point scopes is only worth it once a rule asks for it + $classMethod->setAttribute(AttributeKey::METHOD_RETURN_STATEMENTS_NODE, $methodReturnStatementsNode); } /** diff --git a/tests/NodeAnalyzer/AlwaysInitializedPropertyAnalyzerTest.php b/tests/NodeAnalyzer/AlwaysInitializedPropertyAnalyzerTest.php new file mode 100644 index 00000000000..5b9eeb588cc --- /dev/null +++ b/tests/NodeAnalyzer/AlwaysInitializedPropertyAnalyzerTest.php @@ -0,0 +1,72 @@ +alwaysInitializedPropertyAnalyzer = $this->make(AlwaysInitializedPropertyAnalyzer::class); + $this->testingParser = $this->make(TestingParser::class); + $this->betterNodeFinder = $this->make(BetterNodeFinder::class); + } + + #[DataProvider('provideData')] + public function test(string $filePath, string $propertyName, bool $expectedIsAlwaysInitialized): void + { + $stmts = $this->testingParser->parseFileToDecoratedNodes($filePath); + + $classMethod = $this->betterNodeFinder->findFirstInstanceOf($stmts, ClassMethod::class); + $this->assertInstanceOf(ClassMethod::class, $classMethod); + + $this->assertSame( + $expectedIsAlwaysInitialized, + $this->alwaysInitializedPropertyAnalyzer->isAlwaysInitialized($classMethod, $propertyName) + ); + } + + /** + * @return iterable + */ + public static function provideData(): iterable + { + $sourceDirectory = __DIR__ . '/Source/AlwaysInitializedProperty'; + + yield [$sourceDirectory . '/DirectAssign.php', 'name', true]; + + // never touched in the constructor + yield [$sourceDirectory . '/DirectAssign.php', 'surname', false]; + + // the early return skips the assign + yield [$sourceDirectory . '/EarlyReturn.php', 'name', false]; + + yield [$sourceDirectory . '/EveryBranchAssign.php', 'name', true]; + + // the loop body might never run + yield [$sourceDirectory . '/AssignInForeach.php', 'name', false]; + } + + public function testUnresolvedClassMethodIsNotInitialized(): void + { + $classMethod = new ClassMethod('__construct'); + + $this->assertFalse($this->alwaysInitializedPropertyAnalyzer->isAlwaysInitialized($classMethod, 'name')); + } +} diff --git a/tests/NodeAnalyzer/Source/AlwaysInitializedProperty/AssignInForeach.php b/tests/NodeAnalyzer/Source/AlwaysInitializedProperty/AssignInForeach.php new file mode 100644 index 00000000000..84780c878ae --- /dev/null +++ b/tests/NodeAnalyzer/Source/AlwaysInitializedProperty/AssignInForeach.php @@ -0,0 +1,20 @@ +name = $name; + } + } +} diff --git a/tests/NodeAnalyzer/Source/AlwaysInitializedProperty/DirectAssign.php b/tests/NodeAnalyzer/Source/AlwaysInitializedProperty/DirectAssign.php new file mode 100644 index 00000000000..f4165571b28 --- /dev/null +++ b/tests/NodeAnalyzer/Source/AlwaysInitializedProperty/DirectAssign.php @@ -0,0 +1,17 @@ +name = $name; + } +} diff --git a/tests/NodeAnalyzer/Source/AlwaysInitializedProperty/EarlyReturn.php b/tests/NodeAnalyzer/Source/AlwaysInitializedProperty/EarlyReturn.php new file mode 100644 index 00000000000..0297e96a2fb --- /dev/null +++ b/tests/NodeAnalyzer/Source/AlwaysInitializedProperty/EarlyReturn.php @@ -0,0 +1,19 @@ +name = $name; + } +} diff --git a/tests/NodeAnalyzer/Source/AlwaysInitializedProperty/EveryBranchAssign.php b/tests/NodeAnalyzer/Source/AlwaysInitializedProperty/EveryBranchAssign.php new file mode 100644 index 00000000000..1c0c9f2f507 --- /dev/null +++ b/tests/NodeAnalyzer/Source/AlwaysInitializedProperty/EveryBranchAssign.php @@ -0,0 +1,21 @@ +name = 'one'; + } elseif ($type === 2) { + $this->name = 'two'; + } else { + $this->name = 'many'; + } + } +} From 87f25306d028f47fc84a3e8d3a30c5ac5529d563 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 27 Jul 2026 11:43:57 +0200 Subject: [PATCH 3/3] Decorate only class methods that can assign a property, to bound the retained scopes --- .../Scope/PHPStanNodeScopeResolver.php | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php b/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php index 362a7bb8582..d7f3220c799 100644 --- a/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php +++ b/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php @@ -83,6 +83,7 @@ use PhpParser\Node\Stmt\Unset_; use PhpParser\Node\Stmt\While_; use PhpParser\Node\UnionType; +use PhpParser\NodeFinder; use PhpParser\NodeTraverser; use PHPStan\Analyser\Fiber\FiberScope; use PHPStan\Analyser\MutatingScope; @@ -641,10 +642,35 @@ private function processMethodReturnStatementsNode( return; } + // the node keeps every exit point scope alive, so only pay for it where a property can be initialized + if (! $this->hasPropertyAssign($classMethod)) { + return; + } + // kept raw on purpose, merging the exit point scopes is only worth it once a rule asks for it $classMethod->setAttribute(AttributeKey::METHOD_RETURN_STATEMENTS_NODE, $methodReturnStatementsNode); } + private function hasPropertyAssign(ClassMethod $classMethod): bool + { + $nodeFinder = new NodeFinder(); + + $assign = $nodeFinder->findFirst((array) $classMethod->stmts, static function (Node $node): bool { + if (! $node instanceof Assign) { + return false; + } + + $assignVar = $node->var; + while ($assignVar instanceof ArrayDimFetch) { + $assignVar = $assignVar->var; + } + + return $assignVar instanceof PropertyFetch; + }); + + return $assign instanceof Assign; + } + /** * @param callable(Node $node, MutatingScope $scope): void $nodeCallback */