From fed53f07569c68a927b589f298dab590fd001b6e Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Fri, 4 Sep 2026 18:40:57 +0200 Subject: [PATCH 1/2] [DeadCode] Skip dead catch before catch-all Throwable/Exception in RemoveDeadCatchRector A catch block that only re-throws its own exception is not dead when a later catch of Throwable or Exception handles it differently. As those types catch anything, the current catch type cannot always be resolved to prove the parent relation, so treat them as catch-all explicitly. Fixes #9888 Claude-Session: https://claude.ai/code/session_011TpTyRop7qReoHY1MfLpqF --- ...d_catch_before_catch_all_exception.php.inc | 21 +++++++++++++++++++ .../Rector/TryCatch/RemoveDeadCatchRector.php | 10 ++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 rules-tests/DeadCode/Rector/TryCatch/RemoveDeadCatchRector/Fixture/skip_dead_catch_before_catch_all_exception.php.inc diff --git a/rules-tests/DeadCode/Rector/TryCatch/RemoveDeadCatchRector/Fixture/skip_dead_catch_before_catch_all_exception.php.inc b/rules-tests/DeadCode/Rector/TryCatch/RemoveDeadCatchRector/Fixture/skip_dead_catch_before_catch_all_exception.php.inc new file mode 100644 index 00000000000..4ff24697f91 --- /dev/null +++ b/rules-tests/DeadCode/Rector/TryCatch/RemoveDeadCatchRector/Fixture/skip_dead_catch_before_catch_all_exception.php.inc @@ -0,0 +1,21 @@ + diff --git a/rules/DeadCode/Rector/TryCatch/RemoveDeadCatchRector.php b/rules/DeadCode/Rector/TryCatch/RemoveDeadCatchRector.php index db8fb2aac7f..fd2c4f4325e 100644 --- a/rules/DeadCode/Rector/TryCatch/RemoveDeadCatchRector.php +++ b/rules/DeadCode/Rector/TryCatch/RemoveDeadCatchRector.php @@ -154,7 +154,15 @@ 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 + ); + + if (! $catchesAnything && ! $this->isObjectType($fullyQualified, new ObjectType($nextCatchType->toString()))) { continue; } From 8ea12e035eb58b547432d377f763061529d4b7d5 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Fri, 4 Sep 2026 22:04:51 +0200 Subject: [PATCH 2/2] [DeadCode] Skip dead catch when later catch type is unresolvable in RemoveDeadCatchRector An unresolvable next catch type may be a parent of the current caught exception, so removing the re-throwing catch could change behavior by routing the exception to a different handler. Claude-Session: https://claude.ai/code/session_01Bh57AAY3DpLKXGX5c7EETM --- .../drop_with_many_dead_catches.php.inc | 18 +++++++-------- ...ip_dead_catch_child_of_later_catch.php.inc | 19 +++++++++++++++ .../Rector/TryCatch/RemoveDeadCatchRector.php | 23 ++++++++++++++++++- 3 files changed, 50 insertions(+), 10 deletions(-) create mode 100644 rules-tests/DeadCode/Rector/TryCatch/RemoveDeadCatchRector/Fixture/skip_dead_catch_child_of_later_catch.php.inc diff --git a/rules-tests/DeadCode/Rector/TryCatch/RemoveDeadCatchRector/Fixture/drop_with_many_dead_catches.php.inc b/rules-tests/DeadCode/Rector/TryCatch/RemoveDeadCatchRector/Fixture/drop_with_many_dead_catches.php.inc index ff76fd20508..7fbcde67856 100644 --- a/rules-tests/DeadCode/Rector/TryCatch/RemoveDeadCatchRector/Fixture/drop_with_many_dead_catches.php.inc +++ b/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()); } } } diff --git a/rules-tests/DeadCode/Rector/TryCatch/RemoveDeadCatchRector/Fixture/skip_dead_catch_child_of_later_catch.php.inc b/rules-tests/DeadCode/Rector/TryCatch/RemoveDeadCatchRector/Fixture/skip_dead_catch_child_of_later_catch.php.inc new file mode 100644 index 00000000000..841054aadab --- /dev/null +++ b/rules-tests/DeadCode/Rector/TryCatch/RemoveDeadCatchRector/Fixture/skip_dead_catch_child_of_later_catch.php.inc @@ -0,0 +1,19 @@ + diff --git a/rules/DeadCode/Rector/TryCatch/RemoveDeadCatchRector.php b/rules/DeadCode/Rector/TryCatch/RemoveDeadCatchRector.php index fd2c4f4325e..070f4117e8f 100644 --- a/rules/DeadCode/Rector/TryCatch/RemoveDeadCatchRector.php +++ b/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( @@ -162,7 +168,9 @@ private function shouldSkipNextCatchClassParentWithSpecialTreatment( true ); - if (! $catchesAnything && ! $this->isObjectType($fullyQualified, new ObjectType($nextCatchType->toString()))) { + // 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; } @@ -174,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 */