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_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-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 db8fb2aac7f..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( @@ -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 */