diff --git a/README.md b/README.md index b38d0f3..1338efa 100644 --- a/README.md +++ b/README.md @@ -641,6 +641,7 @@ fn (int $key, Item $item, string $unused) => $item->ok(); // unused parameter $u ### requirePreviousExceptionPass - Detects forgotten exception pass-as-previous when re-throwing - Checks if caught exception can be passed as argument to the call (including constructor call) in `throw` node inside the catch block +- Accepts the caught exception also as an argument of a nested call, e.g. `throw new HttpException(401, 'Locked', MyException::fromPrevious($e))` - You may encounter false-positives in some edge-cases, where you do not want to pass exception as previous, feel free to ignore those ```php diff --git a/src/Rule/RequirePreviousExceptionPassRule.php b/src/Rule/RequirePreviousExceptionPassRule.php index 2d13d82..0651df9 100644 --- a/src/Rule/RequirePreviousExceptionPassRule.php +++ b/src/Rule/RequirePreviousExceptionPassRule.php @@ -106,27 +106,7 @@ private function processExceptionCreation( Scope $scope, ): array { - $passed = false; - - foreach ($node->getArgs() as $argument) { - if (!$argument->value instanceof Variable) { - continue; // support only simple variable pass - } - - $argumentVariableName = $argument->value->name; - - if (!is_string($argumentVariableName)) { - continue; - } - - if ($caughtExceptionVariableName === null) { - continue; - } - - if ($caughtExceptionVariableName === $argumentVariableName) { - $passed = true; - } - } + $passed = $caughtExceptionVariableName !== null && $this->isVariablePassed($node, $caughtExceptionVariableName); if (!$this->reportEvenIfExceptionIsNotAcceptableByRethrownOne) { $accepts = false; @@ -152,6 +132,30 @@ private function processExceptionCreation( return []; } + private function isVariablePassed( + CallLike $node, + string $variableName, + ): bool + { + if ($node->isFirstClassCallable()) { + return false; + } + + foreach ($node->getArgs() as $argument) { + $value = $argument->value; + + if ($value instanceof Variable && $value->name === $variableName) { + return true; + } + + if ($value instanceof CallLike && $this->isVariablePassed($value, $variableName)) { + return true; + } + } + + return false; + } + /** * @return list */ diff --git a/tests/Rule/data/RequirePreviousExceptionPassRule/with-accept-check.php b/tests/Rule/data/RequirePreviousExceptionPassRule/with-accept-check.php index 9aff2bd..e5c5af7 100644 --- a/tests/Rule/data/RequirePreviousExceptionPassRule/with-accept-check.php +++ b/tests/Rule/data/RequirePreviousExceptionPassRule/with-accept-check.php @@ -70,3 +70,23 @@ public static function createWithoutPrevious(): self } catch (LogicException|RuntimeException $e) { // Logic can never occur here, so Runtime must be passed throw MyException::createForSpecificPrevious(); // error: Exception $e not passed as previous to \RequirePreviousExceptionPassRule\With\MyException::createForSpecificPrevious() } + +try { +} catch (RuntimeException $e) { + throw new LogicException('Message', 0, MyException::createForAnyPrevious($e)); +} + +try { +} catch (RuntimeException $e) { + throw new LogicException('Message', 0, MyException::createForAnyPrevious(new RuntimeException('Inner', 0, $e))); +} + +try { +} catch (RuntimeException $e) { + throw new LogicException('Message', 0, MyException::createWithoutPrevious()); // error: Exception $e not passed as previous to new \LogicException('Message', 0, \RequirePreviousExceptionPassRule\With\MyException::createWithoutPrevious()) +} + +try { +} catch (RuntimeException $e) { + throw new LogicException('Message', 0, MyException::createForAnyPrevious(...)); // error: Exception $e not passed as previous to new \LogicException('Message', 0, \RequirePreviousExceptionPassRule\With\MyException::createForAnyPrevious(...)) +}