[dead-code] Fix RemoveUnusedVariableAssignRector, allow for SplFileInfo as cleanup on purpose for gc (#8054)

* [dead-code] Fix RemoveUnusedVariableAssignRector, allow for SplFileInfo as cleanup on purpose for gc

* [php 8.0] Skip merged property in PromotedPropertyCandidateResolver

Author: TomasVotruba

rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/null_reset_of_plain_object.php.inc

@@ -0,0 +1,29 @@
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedVariableAssignRector\Fixture;
+
+final class NullResetOfPlainObject
+{
+    public function run(): void
+    {
+        $value = new \stdClass();
+        echo $value->name;
+
+        $value = null;
+    }
+}
+?>
+-----
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedVariableAssignRector\Fixture;
+
+final class NullResetOfPlainObject
+{
+    public function run(): void
+    {
+        $value = new \stdClass();
+        echo $value->name;
+    }
+}
+?>

rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/skip_null_reset_of_object.php.inc

@@ -0,0 +1,23 @@
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\Assign\RemoveUnusedVariableAssignRector\Fixture;
+
+final class SkipNullResetOfObject
+{
+    public function process(string $path): bool
+    {
+        try {
+            $file = new \SplFileObject($path);
+        } catch (\Exception $e) {
+            return false;
+        }
+
+        $file->seek(0);
+        echo $file->current();
+
+        // Close the file
+        $file = null;
+
+        return true;
+    }
+}

rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Fixture/skip_merged_property.php.inc

@@ -27,24 +27,3 @@ final class SkipMergedProperty
 }
 
 ?>
------
-<?php declare(strict_types = 1);
-
-namespace Rector\Tests\Php80\Rector\Class_\ClassPropertyAssignToConstructorPromotionRector\Fixture;
-
-final class SkipMergedProperty
-{
-    /**
-     * @param array<string, mixed> $metadata
-     */
-    public function __construct(private array $metadata)
-    {
-        if (!isset($this->metadata['permission'])) {
-            $this->metadata['permission'] = [];
-        }
-
-        $permission = $this->metadata['permission'];
-    }
-}
-
-?>

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);

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;
@@ -135,6 +136,15 @@ private function matchPropertyPromotionCandidate(
                 continue;
             }
 
+            if ($this->isParamReadAfterPropertyMutation(
+                $constructClassMethod,
+                $assignStmtPosition,
+                $propertyName,
+                $matchedParam
+            )) {
+                continue;
+            }
+
             return new PropertyPromotionCandidate(
                 $property,
                 $matchedParam,
@@ -205,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
      */