[DeadCode] Skip dead catch before catch-all Throwable/Exception in RemoveDeadCatchRector

rules-tests/DeadCode/Rector/TryCatch/RemoveDeadCatchRector/Fixture/drop_with_many_dead_catches.php.inc

@@ -8,12 +8,12 @@ class DropWithManyDeadCatches
     {
         try {
             // some code
-        } catch (SomeException $exception) {
-            throw new InvalidArgumentException($exception->getMessage());
-        } catch (DeadException $exception) {
+        } catch (\RangeException $exception) {
+            throw new \InvalidArgumentException($exception->getMessage());
+        } catch (\DomainException $exception) {
             throw $exception;
-        } catch (RuntimeException $exception) {
-            throw new InvalidArgumentException($exception->getMessage());
+        } catch (\RuntimeException $exception) {
+            throw new \InvalidArgumentException($exception->getMessage());
         } catch (\Throwable $throwable) {
             throw $throwable;
         }
@@ -32,10 +32,10 @@ class DropWithManyDeadCatches
     {
         try {
             // some code
-        } catch (SomeException $exception) {
-            throw new InvalidArgumentException($exception->getMessage());
-        } catch (RuntimeException $exception) {
-            throw new InvalidArgumentException($exception->getMessage());
+        } catch (\RangeException $exception) {
+            throw new \InvalidArgumentException($exception->getMessage());
+        } catch (\RuntimeException $exception) {
+            throw new \InvalidArgumentException($exception->getMessage());
         }
     }
 }

rules-tests/DeadCode/Rector/TryCatch/RemoveDeadCatchRector/Fixture/skip_dead_catch_before_catch_all_exception.php.inc

@@ -0,0 +1,21 @@
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\TryCatch\RemoveDeadCatchRector\Fixture;
+
+class SkipDeadCatchBeforeCatchAllException
+{
+    public function run()
+    {
+        try {
+            // some code
+        } catch (ValidationException $exception) {
+            throw $exception;
+        } catch (OAuthServerException $exception) {
+            echo 'oauth';
+        } catch (\Exception $exception) {
+            echo 'general';
+        }
+    }
+}
+
+?>

rules-tests/DeadCode/Rector/TryCatch/RemoveDeadCatchRector/Fixture/skip_dead_catch_child_of_later_catch.php.inc

@@ -0,0 +1,19 @@
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\TryCatch\RemoveDeadCatchRector\Fixture;
+
+class SkipDeadCatchChildOfLaterCatch
+{
+    public function run()
+    {
+        try {
+            // some code
+        } catch (\App\ChildException $exception) {
+            throw $exception;
+        } catch (\App\ParentException $exception) {
+            echo 'parent';
+        }
+    }
+}
+
+?>

rules/DeadCode/Rector/TryCatch/RemoveDeadCatchRector.php

@@ -12,6 +12,7 @@
 use PhpParser\Node\Stmt\Expression;
 use PhpParser\Node\Stmt\Nop;
 use PhpParser\Node\Stmt\TryCatch;
+use PHPStan\Reflection\ReflectionProvider;
 use PHPStan\Type\ObjectType;
 use Rector\Rector\AbstractRector;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
@@ -22,6 +23,11 @@
  */
 final class RemoveDeadCatchRector extends AbstractRector
 {
+    public function __construct(
+        private readonly ReflectionProvider $reflectionProvider
+    ) {
+    }
+
     public function getRuleDefinition(): RuleDefinition
     {
         return new RuleDefinition('Remove dead catches', [new CodeSample(
@@ -154,7 +160,17 @@ private function shouldSkipNextCatchClassParentWithSpecialTreatment(
                 return true;
             }
 
-            if (! $this->isObjectType($fullyQualified, new ObjectType($nextCatchType->toString()))) {
+            // Throwable/Exception catch anything, so the current catch is never dead against them,
+            // even when the current type cannot be resolved to prove the parent relation
+            $catchesAnything = in_array(
+                ltrim($nextCatchType->toString(), '\\'),
+                ['Throwable', 'Exception'],
+                true
+            );
+
+            // only skip when the next catch is provably unrelated; an unresolvable next type
+            // might be a parent of the current one, so removing the catch would change behavior
+            if (! $catchesAnything && $this->isProvablyUnrelated($fullyQualified, $nextCatchType)) {
                 continue;
             }
 
@@ -166,6 +182,19 @@ private function shouldSkipNextCatchClassParentWithSpecialTreatment(
         return false;
     }
 
+    private function isProvablyUnrelated(FullyQualified $current, FullyQualified $next): bool
+    {
+        if (! $this->reflectionProvider->hasClass($current->toString())) {
+            return false;
+        }
+
+        if (! $this->reflectionProvider->hasClass($next->toString())) {
+            return false;
+        }
+
+        return ! $this->isObjectType($current, new ObjectType($next->toString()));
+    }
+
     /**
      * @param Stmt[] $stmts
      */