diff --git a/Magento/Sniffs/Exceptions/ThrowCatchSniff.php b/Magento/Sniffs/Exceptions/ThrowCatchSniff.php new file mode 100644 index 00000000..b9ebf0d4 --- /dev/null +++ b/Magento/Sniffs/Exceptions/ThrowCatchSniff.php @@ -0,0 +1,95 @@ +getTokens(); + if (!isset($tokens[$stackPtr]['scope_closer'])) { + // Probably an interface method no check + return; + } + + $closeBrace = $tokens[$stackPtr]['scope_closer']; + $throwTags = []; + $catchTags = []; + + for ($i = $stackPtr; $i < $closeBrace; $i++) { + $token = $tokens[$i]; + if ($token['code'] === T_CATCH) { + $catchTags[] = $token; + } + if ($token['code'] === T_THROW) { + $throwTags[] = $i; + } + } + + if (count($catchTags) === 0 || count($throwTags) === 0) { + // No catch or throw found no check + return; + } + + $catchClassNames = []; + $throwClassNames = []; + + // find all relevant classes in catch + foreach ($catchTags as $catchTag) { + $start = $catchTag['parenthesis_opener']; + $end = $catchTag['parenthesis_closer']; + + $match = $phpcsFile->findNext(T_STRING, $start, $end); + $catchClassNames[$match] = $tokens[$match]['content']; + } + + // find all relevant classes in throws + foreach ($throwTags as $throwTag) { + $match = $phpcsFile->findNext(T_STRING, $throwTag); + $throwClassNames[] = $tokens[$match]['content']; + } + + $throwClassNames = array_flip($throwClassNames); + foreach ($catchClassNames as $match => $catchClassName) { + if (array_key_exists($catchClassName, $throwClassNames)) { + $phpcsFile->addWarning($this->warningMessage, $match, $this->warningCode); + } + } + } +} diff --git a/Magento/Tests/Exceptions/ThrowCatchUnitTest.inc b/Magento/Tests/Exceptions/ThrowCatchUnitTest.inc new file mode 100644 index 00000000..2c6e81d0 --- /dev/null +++ b/Magento/Tests/Exceptions/ThrowCatchUnitTest.inc @@ -0,0 +1,160 @@ +calculationService = $calculationService; + } + + /** + * @param array $data + * @return array|mixed + */ + private function vaildateFunction(array $data) + { + try { + if (!isset($data['formData'])) { + throw new CustomFooException('Bar is not set.'); + } + return $data['formData']; + } catch (CustomFooException $exception) { + // Rule find: Exceptions MUST NOT handled in same function + // handle $exception code + } + + return []; + } + + /** + * @param array $data + * @return int + */ + public function complexNotRelevantToCheck(array $data) + { + $result = $this->vaildateFunction($data); + $previous = 1; + + foreach ($result as $number) { + if ($number === 0) { + throw new LogicException('Cant not deviated by null'); + } + + $previous /= $number; + + // more complex code + + if ($previous === 0) { + throw new ArithmeticError('Calculation error'); + } + + // more complex code + + $result = $this->calculationService->call($previous); + + if ($result === false) { + throw new BadFunctionCallException('Cant not reach calculationService'); + } + } + + return (int)$previous; + } + + /** + * @param array $data + * @return int + * @throws BadFunctionCallException + */ + public function complexDivisionFunction(array $data) + { + try { + try { + + $result = $this->vaildateFunction($data); + $previous = 1; + + foreach ($result as $number) { + if ($number === 0) { + throw new LogicException('Cant not deviated by null'); + } + + $previous /= $number; + + // more complex code + + if ($previous === 0) { + throw new ArithmeticError('Calculation error'); + } + + // more complex code + + $result = $this->calculationService->call($previous); + + if ($result === false) { + throw new BadFunctionCallException('Cant not reach calculationService'); + } + } + + return (int)$previous; + + } catch (LogicException $logicException) { + // Rule find: Exceptions MUST NOT handled in same function + // handle $exception code + } catch (CustomFooException $exception) { + // handle $exception code + } + } catch (ArithmeticError $arithmeticError) { + // Rule find: Exceptions MUST NOT handled in same function + // handle $exception code + } + + return 0; + } +} + + +// bad logic function in .phtml + +function formatFunction(array $data) +{ + try { + if (!isset($data['formData'])) { + throw new CustomFooException('Bar is not set.'); + } + return $data['formData']; + } catch (CustomFooException $exception) { + // Rule find: Exceptions MUST NOT handled in same function + // handle $exception code + } + + return []; +} + +$d = function ($data) { + try { + throw new CustomFooException('Bar is not set.'); + } catch (CustomFooException $exception) { + // Rule find: Exceptions MUST NOT handled in same function + // handle $exception code + } +}; diff --git a/Magento/Tests/Exceptions/ThrowCatchUnitTest.php b/Magento/Tests/Exceptions/ThrowCatchUnitTest.php new file mode 100644 index 00000000..07ff18ea --- /dev/null +++ b/Magento/Tests/Exceptions/ThrowCatchUnitTest.php @@ -0,0 +1,37 @@ + 1, + 120 => 1, + 126 => 1, + 145 => 1, + 156 => 1 + ]; + } +} diff --git a/Magento/ruleset.xml b/Magento/ruleset.xml index 40336962..2ce4fff1 100644 --- a/Magento/ruleset.xml +++ b/Magento/ruleset.xml @@ -142,6 +142,10 @@ 8 warning + + 8 + warning +