From b19110febdaf1b0b554b88e7974118dd89ad6685 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Fri, 19 Jun 2026 16:23:19 +0200 Subject: [PATCH 1/2] [dead-code] Fix RemoveUnusedVariableAssignRector, allow for SplFileInfo as cleanup on purpose for gc --- .../null_reset_of_plain_object.php.inc | 29 +++++++++++++++++++ .../Fixture/skip_null_reset_of_object.php.inc | 23 +++++++++++++++ .../RemoveUnusedVariableAssignRector.php | 18 ++++++++++++ .../PromotedPropertyCandidateResolver.php | 1 + 4 files changed, 71 insertions(+) create mode 100644 rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/null_reset_of_plain_object.php.inc create mode 100644 rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/skip_null_reset_of_object.php.inc diff --git a/rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/null_reset_of_plain_object.php.inc b/rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/null_reset_of_plain_object.php.inc new file mode 100644 index 00000000000..c7ce8dfabaf --- /dev/null +++ b/rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/null_reset_of_plain_object.php.inc @@ -0,0 +1,29 @@ +name; + + $value = null; + } +} +?> +----- +name; + } +} +?> diff --git a/rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/skip_null_reset_of_object.php.inc b/rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/skip_null_reset_of_object.php.inc new file mode 100644 index 00000000000..dfb5e6b59e9 --- /dev/null +++ b/rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/skip_null_reset_of_object.php.inc @@ -0,0 +1,23 @@ +seek(0); + echo $file->current(); + + // Close the file + $file = null; + + return true; + } +} diff --git a/rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php b/rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php index 865e678dcba..0d09dcd41d4 100644 --- a/rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php +++ b/rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php @@ -28,6 +28,7 @@ use Rector\Php\ReservedKeywordAnalyzer; use Rector\PhpParser\Enum\NodeGroup; use Rector\PhpParser\Node\BetterNodeFinder; +use Rector\PhpParser\Node\Value\ValueResolver; use Rector\Rector\AbstractRector; use Rector\ValueObject\MethodName; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; @@ -45,6 +46,7 @@ public function __construct( private readonly BetterNodeFinder $betterNodeFinder, private readonly StmtsManipulator $stmtsManipulator, private readonly NoDiscardCallAnalyzer $noDiscardCallAnalyzer, + private readonly ValueResolver $valueResolver, ) { } @@ -116,6 +118,10 @@ public function refactor(Node $node): null|ClassMethod|Function_ continue; } + if ($this->isNullResetOfObject($assign)) { + continue; + } + if ($this->hasCallLikeInAssignExpr($assign)) { // clean safely $cleanAssignedExpr = $this->cleanCastedExpr($assign->expr); @@ -136,6 +142,18 @@ public function refactor(Node $node): null|ClassMethod|Function_ return null; } + private function isNullResetOfObject(Assign $assign): bool + { + // resetting a native filesystem object to null releases the held file handle, + // e.g. $file = null on a SplFileObject/SplFileInfo/RecursiveDirectoryIterator + if (! $this->valueResolver->isNull($assign->expr)) { + return false; + } + + return (new ObjectType('SplFileInfo'))->isSuperTypeOf($this->getType($assign->var)) + ->yes(); + } + private function isObjectWithDestructMethod(Expr $expr): bool { $exprType = $this->getType($expr); diff --git a/rules/Php80/NodeAnalyzer/PromotedPropertyCandidateResolver.php b/rules/Php80/NodeAnalyzer/PromotedPropertyCandidateResolver.php index 5956e0a715f..e6214f14438 100644 --- a/rules/Php80/NodeAnalyzer/PromotedPropertyCandidateResolver.php +++ b/rules/Php80/NodeAnalyzer/PromotedPropertyCandidateResolver.php @@ -5,6 +5,7 @@ namespace Rector\Php80\NodeAnalyzer; use PhpParser\Node; +use PhpParser\Node\Expr\ArrayDimFetch; use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Expr\Variable; From d5536354281dce9d4ca45895d163cb0c5e1cfa04 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Fri, 19 Jun 2026 16:27:38 +0200 Subject: [PATCH 2/2] [php 8.0] Skip merged property in PromotedPropertyCandidateResolver --- .../Fixture/skip_merged_property.php.inc | 21 ------ .../PromotedPropertyCandidateResolver.php | 64 +++++++++++++++++++ 2 files changed, 64 insertions(+), 21 deletions(-) diff --git a/rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Fixture/skip_merged_property.php.inc b/rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Fixture/skip_merged_property.php.inc index 080229a29e5..fb1bc335578 100644 --- a/rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Fixture/skip_merged_property.php.inc +++ b/rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Fixture/skip_merged_property.php.inc @@ -27,24 +27,3 @@ final class SkipMergedProperty } ?> ------ - $metadata - */ - public function __construct(private array $metadata) - { - if (!isset($this->metadata['permission'])) { - $this->metadata['permission'] = []; - } - - $permission = $this->metadata['permission']; - } -} - -?> diff --git a/rules/Php80/NodeAnalyzer/PromotedPropertyCandidateResolver.php b/rules/Php80/NodeAnalyzer/PromotedPropertyCandidateResolver.php index e6214f14438..1e47b42efe4 100644 --- a/rules/Php80/NodeAnalyzer/PromotedPropertyCandidateResolver.php +++ b/rules/Php80/NodeAnalyzer/PromotedPropertyCandidateResolver.php @@ -136,6 +136,15 @@ private function matchPropertyPromotionCandidate( continue; } + if ($this->isParamReadAfterPropertyMutation( + $constructClassMethod, + $assignStmtPosition, + $propertyName, + $matchedParam + )) { + continue; + } + return new PropertyPromotionCandidate( $property, $matchedParam, @@ -206,6 +215,61 @@ private function isParamUsedBeforeAssign(Variable $variable, array $firstParamAs return $firstVariablePosition < $variable->getStartTokenPos(); } + /** + * The rule rewrites later $param reads to $this->property. That is only safe while $this->property still + * equals the param. Once the property is mutated in the constructor, a following $param read would diverge, + * so the promotion must be skipped. + */ + private function isParamReadAfterPropertyMutation( + ClassMethod $constructClassMethod, + int $assignStmtPosition, + string $propertyName, + Param $matchedParam + ): bool { + $followingStmts = array_slice((array) $constructClassMethod->stmts, $assignStmtPosition + 1); + if ($followingStmts === []) { + return false; + } + + $firstMutationTokenPos = null; + foreach ($this->betterNodeFinder->findInstanceOf($followingStmts, Assign::class) as $assign) { + $assignVar = $assign->var; + while ($assignVar instanceof ArrayDimFetch) { + $assignVar = $assignVar->var; + } + + if (! $this->propertyFetchAnalyzer->isLocalPropertyFetchName($assignVar, $propertyName)) { + continue; + } + + $tokenPos = $assign->var->getStartTokenPos(); + if ($firstMutationTokenPos === null || $tokenPos < $firstMutationTokenPos) { + $firstMutationTokenPos = $tokenPos; + } + } + + if ($firstMutationTokenPos === null) { + return false; + } + + $paramName = $this->nodeNameResolver->getName($matchedParam); + + return $this->betterNodeFinder->findFirst($followingStmts, function (Node $node) use ( + $paramName, + $firstMutationTokenPos + ): bool { + if (! $node instanceof Variable) { + return false; + } + + if (! $this->nodeNameResolver->isName($node, $paramName)) { + return false; + } + + return $node->getStartTokenPos() > $firstMutationTokenPos; + }) instanceof Node; + } + /** * @param int[] $firstParamAsVariable */