[Rector] Skip used after TryCatch on RemoveUnusedVariableInCatchRector (

#4459) * [Rector] Skip used after TryCatch on RemoveUnusedVariableInCatchRector * clean up * clean up * move check variable used next to StmtsManipulator * clean up * clean up
rectorphp · Jul 9, 2023 · 1408914 · 1408914
1 parent 68d092c
commit 1408914
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 31 deletions.
diff --git a/...sts/Php80/Rector/Catch_/RemoveUnusedVariableInCatchRector/Fixture/skip_used_later.php.inc b/...sts/Php80/Rector/Catch_/RemoveUnusedVariableInCatchRector/Fixture/skip_used_later.php.inc
@@ -2,30 +2,14 @@
 
 namespace Rector\Tests\Php80\Rector\Catch_\RemoveUnusedVariableInCatchRector\Fixture;
 
-final class Fixture
+final class SkipUsedLater
 {
     public function run()
     {
         try {
-        } catch (Throwable $notUsedThrowable) {
+        } catch (Throwable $usedLater) {
         }
-    }
-}
-
-?>
------
-<?php
-
-namespace Rector\Tests\Php80\Rector\Catch_\RemoveUnusedVariableInCatchRector\Fixture;
 
-final class Fixture
-{
-    public function run()
-    {
-        try {
-        } catch (Throwable) {
-        }
+        $this->process($usedLater);
     }
 }
-
-?>
diff --git a/rules/Php80/Rector/Catch_/RemoveUnusedVariableInCatchRector.php b/rules/Php80/Rector/Catch_/RemoveUnusedVariableInCatchRector.php
@@ -6,7 +6,9 @@
 
 use PhpParser\Node;
 use PhpParser\Node\Expr\Variable;
-use PhpParser\Node\Stmt\Catch_;
+use PhpParser\Node\Stmt\TryCatch;
+use Rector\Core\Contract\PhpParser\Node\StmtsAwareInterface;
+use Rector\Core\NodeManipulator\StmtsManipulator;
 use Rector\Core\Rector\AbstractRector;
 use Rector\Core\ValueObject\PhpVersionFeature;
 use Rector\VersionBonding\Contract\MinPhpVersionInterface;
@@ -20,6 +22,11 @@
  */
 final class RemoveUnusedVariableInCatchRector extends AbstractRector implements MinPhpVersionInterface
 {
+    public function __construct(
+        private readonly StmtsManipulator $stmtsManipulator
+    ) {
+    }
+
     public function getRuleDefinition(): RuleDefinition
     {
         return new RuleDefinition('Remove unused variable in catch()', [
@@ -56,28 +63,44 @@ public function run()
      */
     public function getNodeTypes(): array
     {
-        return [Catch_::class];
+        return [StmtsAwareInterface::class];
     }
 
     /**
-     * @param Catch_ $node
+     * @param StmtsAwareInterface $node
      */
     public function refactor(Node $node): ?Node
     {
-        $caughtVar = $node->var;
-        if (! $caughtVar instanceof Variable) {
+        if ($node->stmts === null) {
             return null;
         }
 
-        /** @var string $variableName */
-        $variableName = $this->getName($caughtVar);
+        foreach ($node->stmts as $key => $stmt) {
+            if (! $stmt instanceof TryCatch) {
+                continue;
+            }
 
-        $isVariableUsed = (bool) $this->betterNodeFinder->findVariableOfName($node->stmts, $variableName);
-        if ($isVariableUsed) {
-            return null;
-        }
+            foreach ($stmt->catches as $catch) {
+                $caughtVar = $catch->var;
+                if (! $caughtVar instanceof Variable) {
+                    continue;
+                }
 
-        $node->var = null;
+                /** @var string $variableName */
+                $variableName = $this->getName($caughtVar);
+
+                $isVariableUsed = (bool) $this->betterNodeFinder->findVariableOfName($catch->stmts, $variableName);
+                if ($isVariableUsed) {
+                    continue;
+                }
+
+                if ($this->stmtsManipulator->isVariableUsedInNextStmt($node, $key + 1, $variableName)) {
+                    continue;
+                }
+
+                $catch->var = null;
+            }
+        }
 
         return $node;
     }

diff --git a/src/NodeManipulator/StmtsManipulator.php b/src/NodeManipulator/StmtsManipulator.php
@@ -8,13 +8,16 @@
 use PhpParser\Node\Stmt;
 use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Expression;
+use Rector\Core\Contract\PhpParser\Node\StmtsAwareInterface;
 use Rector\Core\PhpParser\Comparing\NodeComparator;
+use Rector\Core\PhpParser\Node\BetterNodeFinder;
 use Rector\PhpDocParser\NodeTraverser\SimpleCallableNodeTraverser;
 
 final class StmtsManipulator
 {
     public function __construct(
         private readonly SimpleCallableNodeTraverser $simpleCallableNodeTraverser,
+        private readonly BetterNodeFinder $betterNodeFinder,
         private readonly NodeComparator $nodeComparator
     ) {
     }
@@ -57,4 +60,32 @@ function (Node $node) use (&$stmts) {
 
         return $stmts;
     }
+
+    public function isVariableUsedInNextStmt(
+        StmtsAwareInterface $stmtsAware,
+        int $jumpToKey,
+        string $variableName
+    ): bool {
+        if ($stmtsAware->stmts === null) {
+            return false;
+        }
+
+        $totalKeys = array_key_last($stmtsAware->stmts);
+        for ($key = $jumpToKey; $key <= $totalKeys; ++$key) {
+            if (! isset($stmtsAware->stmts[$key])) {
+                continue;
+            }
+
+            $isVariableUsed = (bool) $this->betterNodeFinder->findVariableOfName(
+                $stmtsAware->stmts[$key],
+                $variableName
+            );
+
+            if ($isVariableUsed) {
+                return true;
+            }
+        }
+
+        return false;
+    }
 }