Remove removeNode() from MultiExceptionCatchRector (#4063)

rules-tests/Php71/Rector/TryCatch/MultiExceptionCatchRector/Fixture/fixture.php.inc

@@ -4,26 +4,6 @@ namespace Rector\Tests\Php71\Rector\TryCatch\MultiExceptionCatchRector\Fixture;
 
 function multiExceptionCatch()
 {
-    try {
-        // Some code...
-    } catch (ExceptionType1 $e) {
-        // Code to handle the exception
-    } catch (ExceptionType2 $e) {
-        // Code to handle the exception
-    } catch (Exception $e) {
-        // ...
-    }
-
-
-    try {
-        // Some code...
-    } catch (ExceptionType1 $e) {
-        // Code to handle the exception
-    } catch (ExceptionType2 $e) {
-        $differentContent = 'hey';
-    }
-
-
     try {
         // Some code...
     } catch (ExceptionType1 $e) {
@@ -41,24 +21,6 @@ namespace Rector\Tests\Php71\Rector\TryCatch\MultiExceptionCatchRector\Fixture;
 
 function multiExceptionCatch()
 {
-    try {
-        // Some code...
-    } catch (ExceptionType1|ExceptionType2 $e) {
-        // Code to handle the exception
-    } catch (Exception $e) {
-        // ...
-    }
-
-
-    try {
-        // Some code...
-    } catch (ExceptionType1 $e) {
-        // Code to handle the exception
-    } catch (ExceptionType2 $e) {
-        $differentContent = 'hey';
-    }
-
-
     try {
         // Some code...
     } catch (ExceptionType1|ExceptionType2 $e) {

rules-tests/Php71/Rector/TryCatch/MultiExceptionCatchRector/Fixture/merge_commented_same.php.inc

@@ -0,0 +1,41 @@
+<?php
+
+namespace Rector\Tests\Php71\Rector\TryCatch\MultiExceptionCatchRector\Fixture;
+
+final class MergeCommentedSame
+{
+    public function run()
+    {
+        try {
+            // Some code...
+        } catch (ExceptionType1 $e) {
+            // Code to handle the exception
+        } catch (ExceptionType2 $e) {
+            // Code to handle the exception
+        } catch (Exception $e) {
+            // ...
+        }
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\Php71\Rector\TryCatch\MultiExceptionCatchRector\Fixture;
+
+final class MergeCommentedSame
+{
+    public function run()
+    {
+        try {
+            // Some code...
+        } catch (ExceptionType1|ExceptionType2 $e) {
+            // Code to handle the exception
+        } catch (Exception $e) {
+            // ...
+        }
+    }
+}
+
+?>

rules-tests/Php71/Rector/TryCatch/MultiExceptionCatchRector/Fixture/multiple_items.php.inc

@@ -0,0 +1,43 @@
+<?php
+
+namespace Rector\Tests\Php71\Rector\TryCatch\MultiExceptionCatchRector\Fixture;
+
+final class MultipleItems
+{
+    public function run()
+    {
+        try {
+            // Some code...
+        } catch (ExceptionType1 $e) {
+            // Code to handle the exception
+        } catch (ExceptionType2 $e) {
+            // Code to handle the exception
+        } catch (ExceptionType3 $e) {
+            // Code to handle the exception
+        } finally {
+            // nothing
+        }
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\Php71\Rector\TryCatch\MultiExceptionCatchRector\Fixture;
+
+final class MultipleItems
+{
+    public function run()
+    {
+        try {
+            // Some code...
+        } catch (ExceptionType1|ExceptionType2|ExceptionType3 $e) {
+            // Code to handle the exception
+        } finally {
+            // nothing
+        }
+    }
+}
+
+?>

rules-tests/Php71/Rector/TryCatch/MultiExceptionCatchRector/Fixture/skip_different_contents.php.inc

@@ -0,0 +1,17 @@
+<?php
+
+namespace Rector\Tests\Php71\Rector\TryCatch\MultiExceptionCatchRector\Fixture;
+
+final class SkipDifferentContents
+{
+    public function run()
+    {
+        try {
+            // Some code...
+        } catch (ExceptionType1 $e) {
+            // Code to handle the exception
+        } catch (ExceptionType2 $e) {
+            $differentContent = 'hey';
+        }
+    }
+}

rules/Php71/Rector/TryCatch/MultiExceptionCatchRector.php

@@ -5,8 +5,6 @@
 namespace Rector\Php71\Rector\TryCatch;
 
 use PhpParser\Node;
-use PhpParser\Node\Name;
-use PhpParser\Node\Stmt\Catch_;
 use PhpParser\Node\Stmt\TryCatch;
 use Rector\Core\Contract\PhpParser\NodePrinterInterface;
 use Rector\Core\Rector\AbstractRector;
@@ -72,66 +70,36 @@ public function refactor(Node $node): ?Node
             return null;
         }
 
-        $catchKeysByContent = $this->collectCatchKeysByContent($node);
-        $hasRemovedCatch = false;
-        /** @var Catch_[] $catchKeys */
-        foreach ($catchKeysByContent as $catchKeys) {
-            // no duplicates
-            $count = count($catchKeys);
-            if ($count < 2) {
-                continue;
-            }
+        $printedCatches = [];
+        $hasChanged = false;
+
+        foreach ($node->catches as $key => $catch) {
+            $currentPrintedCatch = $this->nodePrinter->print($catch->stmts);
 
-            $collectedTypes = $this->collectTypesFromCatchedByIds($catchKeys);
+            // already duplicated catch → remove it and join the type
+            if (in_array($currentPrintedCatch, $printedCatches, true)) {
+                // merge type to existing type
+                $existingCatchKey = array_search($currentPrintedCatch, $printedCatches, true);
+                $node->catches[$existingCatchKey]->types[] = $catch->types[0];
 
-            /** @var Catch_ $firstCatch */
-            $firstCatch = array_shift($catchKeys);
-            $firstCatch->types = $collectedTypes;
+                unset($node->catches[$key]);
+                $hasChanged = true;
 
-            foreach ($catchKeys as $catchKey) {
-                $this->removeNode($catchKey);
-                $hasRemovedCatch = true;
+                continue;
             }
+
+            $printedCatches[$key] = $currentPrintedCatch;
         }
 
-        if (! $hasRemovedCatch) {
-            return null;
+        if ($hasChanged) {
+            return $node;
         }
 
-        return $node;
+        return null;
     }
 
     public function provideMinPhpVersion(): int
     {
         return PhpVersionFeature::MULTI_EXCEPTION_CATCH;
     }
-
-    /**
-     * @return array<string, Catch_[]>
-     */
-    private function collectCatchKeysByContent(TryCatch $tryCatch): array
-    {
-        $catchKeysByContent = [];
-        foreach ($tryCatch->catches as $catch) {
-            $catchContent = $this->nodePrinter->print($catch->stmts);
-            $catchKeysByContent[$catchContent][] = $catch;
-        }
-
-        return $catchKeysByContent;
-    }
-
-    /**
-     * @param Catch_[] $catches
-     * @return Name[]
-     */
-    private function collectTypesFromCatchedByIds(array $catches): array
-    {
-        $collectedTypes = [];
-
-        foreach ($catches as $catch) {
-            $collectedTypes = array_merge($collectedTypes, $catch->types);
-        }
-
-        return $collectedTypes;
-    }
 }