#29: [New Rule] Exceptions must not be handled in the same function

Magento/Sniffs/Functions/ThrowCatchSniff.php

@@ -0,0 +1,95 @@
+<?php
+/**
+ * Copyright © Magento. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+namespace Magento\Sniffs\Functions;
+
+use function array_slice;
+use PHP_CodeSniffer\Sniffs\Sniff;
+use PHP_CodeSniffer\Files\File;
+
+/**
+ * Detects exceptions must not be handled in same function
+ */
+class ThrowCatchSniff implements Sniff
+{
+    /**
+     * String representation of warning.
+     *
+     * @var string
+     */
+    protected $warningMessage = 'Exceptions must not be handled in the same function where they are thrown.';
+
+    /**
+     * Warning violation code.
+     *
+     * @var string
+     */
+    protected $warningCode = 'ThrowCatch';
+
+    /**
+     * @inheritDoc
+     */
+    public function register()
+    {
+        return [T_FUNCTION, T_CLOSURE];
+    }
+
+    /**
+     * @inheritDoc
+     */
+    public function process(File $phpcsFile, $stackPtr)
+    {
+        $tokens = $phpcsFile->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);
+            }
+        }
+    }
+}

Magento/Tests/Functions/ThrowCatchUnitTest.inc

@@ -0,0 +1,160 @@
+<?php
+
+namespace Foo\Bar;
+
+use ArithmeticError;
+use BadFunctionCallException;
+use Exception;
+use LogicException;
+
+abstract class Calculator
+{
+    abstract public function complexNotRelevantToCheck(array $data);
+}
+
+class CustomFooException extends Exception {}
+
+class Foo extends Calculator
+{
+
+    /**
+     * @var CallulationService
+     */
+    private $calculationService;
+
+    public function __construct(CallulationService $calculationService)
+    {
+        $this->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
+    }
+};

Magento/Tests/Functions/ThrowCatchUnitTest.php

@@ -0,0 +1,37 @@
+<?php
+/**
+ * Copyright © Magento. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+namespace Magento\Tests\Functions;
+
+use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;
+
+/**
+ * Class ThrowCatchUnitTest
+ */
+class ThrowCatchUnitTest extends AbstractSniffUnitTest
+{
+    /**
+     * @inheritdoc
+     */
+    protected function getErrorList()
+    {
+        return [];
+    }
+
+    /**
+     * @inheritdoc
+     */
+    protected function getWarningList()
+    {
+        return [
+            41 => 1,
+            120 => 1,
+            126 => 1,
+            145 => 1,
+            156 => 1
+        ];
+    }
+}

Magento/ruleset.xml

@@ -72,6 +72,9 @@
     <rule ref="Magento.Exceptions.Namespace">
         <severity>8</severity>
     </rule>
+    <rule ref="Magento.Functions.ThrowCatch">
+        <severity>8</severity>
+    </rule>
     <rule ref="Magento.Functions.StaticFunction">
         <severity>6</severity>
     </rule>
@@ -203,10 +206,6 @@
         <severity>6</severity>
         <type>warning</type>
     </rule>
-    <rule ref="Squiz.Operators.ValidLogicalOperators">
-        <severity>6</severity>
-        <type>warning</type>
-    </rule>
     <rule ref="Squiz.Functions.GlobalFunction">
         <severity>8</severity>
     </rule>