[DX] Check only for current scope in RemoveUnusedVariableInCatchRecto…

…r, as… magic exception use is bad practise and should be avoided (#4449)
rectorphp · Jul 9, 2023 · cfc88dc · cfc88dc
1 parent ce03029
commit cfc88dc
Show file tree

Hide file tree

Showing 5 changed files with 10 additions and 71 deletions.
diff --git a/phpstan.neon b/phpstan.neon
@@ -699,3 +699,6 @@ parameters:
                 - src/NodeManipulator/PropertyManipulator.php
 
         - '#Fetching class constant class of deprecated class Rector\\Core\\Configuration\\Parameter\\ParameterProvider#'
+
+        # handle in next PR
+        - '#Call to deprecated method findFirstNext\(\) of class Rector\\Core\\PhpParser\\Node\\BetterNodeFinder#'
diff --git a/...emoveUnusedVariableInCatchRector/Fixture/skip_when_variable_is_used_outside_catch.php.inc b/...emoveUnusedVariableInCatchRector/Fixture/skip_when_variable_is_used_outside_catch.php.inc
diff --git a/rules/Php80/Rector/Catch_/RemoveUnusedVariableInCatchRector.php b/rules/Php80/Rector/Catch_/RemoveUnusedVariableInCatchRector.php
@@ -9,7 +9,6 @@
 use PhpParser\Node\Stmt\Catch_;
 use Rector\Core\Rector\AbstractRector;
 use Rector\Core\ValueObject\PhpVersionFeature;
-use Rector\DeadCode\NodeAnalyzer\ExprUsedInNodeAnalyzer;
 use Rector\VersionBonding\Contract\MinPhpVersionInterface;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
@@ -21,11 +20,6 @@
  */
 final class RemoveUnusedVariableInCatchRector extends AbstractRector implements MinPhpVersionInterface
 {
-    public function __construct(
-        private readonly ExprUsedInNodeAnalyzer $exprUsedInNodeAnalyzer
-    ) {
-    }
-
     public function getRuleDefinition(): RuleDefinition
     {
         return new RuleDefinition('Remove unused variable in catch()', [
@@ -75,11 +69,11 @@ public function refactor(Node $node): ?Node
             return null;
         }
 
-        if ($this->isVariableUsedInStmts($node->stmts, $caughtVar)) {
-            return null;
-        }
+        /** @var string $variableName */
+        $variableName = $this->getName($caughtVar);
 
-        if ($this->isVariableUsedNext($node, $caughtVar)) {
+        $isVariableUsed = (bool) $this->betterNodeFinder->findVariableOfName($node->stmts, $variableName);
+        if ($isVariableUsed) {
             return null;
         }
 
@@ -92,23 +86,4 @@ public function provideMinPhpVersion(): int
     {
         return PhpVersionFeature::NON_CAPTURING_CATCH;
     }
-
-    /**
-     * @param Node[] $nodes
-     */
-    private function isVariableUsedInStmts(array $nodes, Variable $variable): bool
-    {
-        return (bool) $this->betterNodeFinder->findFirst(
-            $nodes,
-            fn (Node $node): bool => $this->exprUsedInNodeAnalyzer->isUsed($node, $variable)
-        );
-    }
-
-    private function isVariableUsedNext(Catch_ $catch, Variable $variable): bool
-    {
-        return (bool) $this->betterNodeFinder->findFirstNext(
-            $catch,
-            fn (Node $node): bool => $this->exprUsedInNodeAnalyzer->isUsed($node, $variable)
-        );
-    }
 }
diff --git a/src/Application/ChangedNodeScopeRefresher.php b/src/Application/ChangedNodeScopeRefresher.php
@@ -29,7 +29,6 @@
 use Rector\Core\NodeAnalyzer\ScopeAnalyzer;
 use Rector\Core\Provider\CurrentFileProvider;
 use Rector\Core\ValueObject\Application\File;
-use Rector\NodeTypeResolver\Node\AttributeKey;
 use Rector\NodeTypeResolver\PHPStan\Scope\PHPStanNodeScopeResolver;
 
 /**
@@ -60,22 +59,7 @@ public function refresh(Node $node, ?MutatingScope $mutatingScope, ?string $file
         $mutatingScope = $this->scopeAnalyzer->resolveScope($node, $filePath, $mutatingScope);
 
         if (! $mutatingScope instanceof MutatingScope) {
-            /**
-             * @var Node $parentNode
-             *
-             * $parentNode is always a Node when $mutatingScope is null, as checked in previous
-             *
-             *      $this->scopeAnalyzer->resolveScope()
-             *
-             *  which verify if no parent and no scope, it resolve Scope from File
-             */
-            $parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
-
-            $errorMessage = sprintf(
-                'Node "%s" with parent of "%s" is missing scope required for scope refresh.',
-                $node::class,
-                $parentNode::class
-            );
+            $errorMessage = sprintf('Node "%s" with is missing scope required for scope refresh', $node::class);
 
             throw new ShouldNotHappenException($errorMessage);
         }

diff --git a/src/PhpParser/Node/BetterNodeFinder.php b/src/PhpParser/Node/BetterNodeFinder.php
@@ -166,6 +166,8 @@ public function findFirst(Node | array $nodes, callable $filter): ?Node
     }
 
     /**
+     * @deprecated Use node traversing instead
+     *
      * @param callable(Node $node): bool $filter
      */
     public function findFirstNext(Node $node, callable $filter): ?Node