[CodingStyle] Handle Change Exception variable usage after try catch …

…on CatchExceptionNameMatchingTypeRector (#360) * [CodingStyle] Handle Change Exception variable usage after try catch * [ci-review] Rector Rectify * stop change next after re-assign * fix * fixture usage in parent of try catch * Fixed 🎉 * cs fix * phpstan * final touch: fixture ensure always change next, stop after re-assign * [ci-review] Rector Rectify * stop on parent is FunctionLike * [ci-review] Rector Rectify * [ci-review] Rector Rectify * allow re-assign same variable * final touch Co-authored-by: GitHub Action <action@github.com>
rectorphp · Jul 2, 2021 · ecfaf21 · ecfaf21
1 parent 171417d
commit ecfaf21
Show file tree

Hide file tree

Showing 6 changed files with 231 additions and 1 deletion.
diff --git a/packages/BetterPhpDocParser/PhpDocInfo/PhpDocInfo.php b/packages/BetterPhpDocParser/PhpDocInfo/PhpDocInfo.php
@@ -285,7 +285,9 @@ public function findByAnnotationClass(string $desiredClass): array
     public function removeByType(string $typeToRemove): void
     {
         $phpDocNodeTraverser = new PhpDocNodeTraverser();
-        $phpDocNodeTraverser->traverseWithCallable($this->phpDocNode, '', function (Node $node) use ($typeToRemove) {
+        $phpDocNodeTraverser->traverseWithCallable($this->phpDocNode, '', function (Node $node) use (
+            $typeToRemove
+        ): ?int {
             if (! is_a($node, $typeToRemove, true)) {
                 return null;
             }

diff --git a/...ctor/Catch_/CatchExceptionNameMatchingTypeRector/Fixture/change_variable_next_try.php.inc b/...ctor/Catch_/CatchExceptionNameMatchingTypeRector/Fixture/change_variable_next_try.php.inc
@@ -0,0 +1,37 @@
+<?php
+
+namespace Rector\Tests\CodingStyle\Rector\Catch_\CatchExceptionNameMatchingTypeRector\Fixture;
+
+class ChangeVariableNextTry
+{
+    public function run()
+    {
+        try {
+        } catch (SomeException $typoException) {
+        }
+
+        $this->verify($typoException);
+        $this->verify2($typoException);
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\CodingStyle\Rector\Catch_\CatchExceptionNameMatchingTypeRector\Fixture;
+
+class ChangeVariableNextTry
+{
+    public function run()
+    {
+        try {
+        } catch (SomeException $someException) {
+        }
+
+        $this->verify($someException);
+        $this->verify2($someException);
+    }
+}
+
+?>
diff --git a/...tor/Catch_/CatchExceptionNameMatchingTypeRector/Fixture/change_variable_next_try2.php.inc b/...tor/Catch_/CatchExceptionNameMatchingTypeRector/Fixture/change_variable_next_try2.php.inc
@@ -0,0 +1,45 @@
+<?php
+
+namespace Rector\Tests\CodingStyle\Rector\Catch_\CatchExceptionNameMatchingTypeRector\Fixture;
+
+class ChangeVariableNextTry2
+{
+    public function run()
+    {
+        if (rand(0, 1)) {
+            try {
+            } catch (SomeException $typoException) {
+            }
+        }
+
+        if (isset($typoException)) {
+            $this->verify($typoException);
+            $this->verify2($typoException);
+        }
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\CodingStyle\Rector\Catch_\CatchExceptionNameMatchingTypeRector\Fixture;
+
+class ChangeVariableNextTry2
+{
+    public function run()
+    {
+        if (rand(0, 1)) {
+            try {
+            } catch (SomeException $someException) {
+            }
+        }
+
+        if (isset($someException)) {
+            $this->verify($someException);
+            $this->verify2($someException);
+        }
+    }
+}
+
+?>
diff --git a/...tchExceptionNameMatchingTypeRector/Fixture/change_variable_next_try_reassign_same.php.inc b/...tchExceptionNameMatchingTypeRector/Fixture/change_variable_next_try_reassign_same.php.inc
@@ -0,0 +1,35 @@
+<?php
+
+namespace Rector\Tests\CodingStyle\Rector\Catch_\CatchExceptionNameMatchingTypeRector\Fixture;
+
+class ChangeVariableNextTryReassignSameVariable
+{
+    public function run()
+    {
+        try {
+        } catch (SomeException $typoException) {
+        }
+
+        $typoException = $typoException;
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\CodingStyle\Rector\Catch_\CatchExceptionNameMatchingTypeRector\Fixture;
+
+class ChangeVariableNextTryReassignSameVariable
+{
+    public function run()
+    {
+        try {
+        } catch (SomeException $someException) {
+        }
+
+        $someException = $someException;
+    }
+}
+
+?>
diff --git a/...nNameMatchingTypeRector/Fixture/donot_change_variable_next_try_reassign_different.php.inc b/...nNameMatchingTypeRector/Fixture/donot_change_variable_next_try_reassign_different.php.inc
@@ -0,0 +1,41 @@
+<?php
+
+namespace Rector\Tests\CodingStyle\Rector\Catch_\CatchExceptionNameMatchingTypeRector\Fixture;
+
+class DonotChangeVariableNextTryReassignDifferent
+{
+    public function run()
+    {
+        try {
+        } catch (SomeException $typoException) {
+        }
+
+        $this->verify($typoException);
+
+        $typoException = 'test';
+        $this->verify($typoException);
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\CodingStyle\Rector\Catch_\CatchExceptionNameMatchingTypeRector\Fixture;
+
+class DonotChangeVariableNextTryReassignDifferent
+{
+    public function run()
+    {
+        try {
+        } catch (SomeException $someException) {
+        }
+
+        $this->verify($someException);
+
+        $typoException = 'test';
+        $this->verify($typoException);
+    }
+}
+
+?>
diff --git a/rules/CodingStyle/Rector/Catch_/CatchExceptionNameMatchingTypeRector.php b/rules/CodingStyle/Rector/Catch_/CatchExceptionNameMatchingTypeRector.php
@@ -6,9 +6,13 @@
 
 use Nette\Utils\Strings;
 use PhpParser\Node;
+use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Expr\Variable;
+use PhpParser\Node\FunctionLike;
 use PhpParser\Node\Stmt\Catch_;
+use PhpParser\Node\Stmt\TryCatch;
 use Rector\Core\Rector\AbstractRector;
+use Rector\NodeTypeResolver\Node\AttributeKey;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
@@ -140,5 +144,71 @@ private function renameVariableInStmts(Catch_ $catch, string $oldVariableName, s
 
             $node->name = $newVariableName;
         });
+
+        /** @var TryCatch $tryCatch */
+        $tryCatch = $catch->getAttribute(AttributeKey::PARENT_NODE);
+        $next = $tryCatch->getAttribute(AttributeKey::NEXT_NODE);
+
+        $this->replaceNextUsageVariable($tryCatch, $next, $oldVariableName, $newVariableName);
+    }
+
+    private function replaceNextUsageVariable(
+        Node $currentNode,
+        ?Node $nextNode,
+        string $oldVariableName,
+        string $newVariableName
+    ): void {
+        if (! $nextNode instanceof Node) {
+            $parent = $currentNode->getAttribute(AttributeKey::PARENT_NODE);
+            if (! $parent instanceof Node) {
+                return;
+            }
+            if ($parent instanceof FunctionLike) {
+                return;
+            }
+            $nextNode = $parent->getAttribute(AttributeKey::NEXT_NODE);
+            $this->replaceNextUsageVariable($parent, $nextNode, $oldVariableName, $newVariableName);
+
+            return;
+        }
+
+        /** @var Variable[] $variables */
+        $variables = $this->betterNodeFinder->find($nextNode, function (Node $node) use ($oldVariableName): bool {
+            if (! $node instanceof Variable) {
+                return false;
+            }
+
+            return $this->nodeNameResolver->isName($node, $oldVariableName);
+        });
+
+        $processRenameVariables = $this->processRenameVariable($variables, $oldVariableName, $newVariableName);
+        if (! $processRenameVariables) {
+            return;
+        }
+
+        $currentNode = $nextNode;
+        $nextNode = $nextNode->getAttribute(AttributeKey::NEXT_NODE);
+        $this->replaceNextUsageVariable($currentNode, $nextNode, $oldVariableName, $newVariableName);
+    }
+
+    /**
+     * @param Variable[] $variables
+     */
+    private function processRenameVariable(array $variables, string $oldVariableName, string $newVariableName): bool
+    {
+        foreach ($variables as $variable) {
+            $parent = $variable->getAttribute(AttributeKey::PARENT_NODE);
+            if ($parent instanceof Assign && $this->nodeComparator->areNodesEqual(
+                $parent->var,
+                $variable
+            ) && $this->nodeNameResolver->isName($parent->var, $oldVariableName)
+            && ! $this->nodeComparator->areNodesEqual($parent->expr, $variable)
+            ) {
+                return false;
+            }
+            $variable->name = $newVariableName;
+        }
+
+        return true;
     }
 }