From be51e064f717e618ef00894d7525d2cd7868c3a2 Mon Sep 17 00:00:00 2001 From: roettigl Date: Thu, 1 Aug 2019 16:38:48 +0200 Subject: [PATCH 1/4] MC-18816: Impelement an better check for ThrowCatchSniff --- .../Sniffs/Exceptions/ThrowCatchSniff.php | 90 ++++++++++++------- .../Tests/Exceptions/ThrowCatchUnitTest.1.inc | 18 ++++ ...hUnitTest.inc => ThrowCatchUnitTest.2.inc} | 0 .../Tests/Exceptions/ThrowCatchUnitTest.php | 8 +- 4 files changed, 82 insertions(+), 34 deletions(-) create mode 100644 Magento2/Tests/Exceptions/ThrowCatchUnitTest.1.inc rename Magento2/Tests/Exceptions/{ThrowCatchUnitTest.inc => ThrowCatchUnitTest.2.inc} (100%) diff --git a/Magento2/Sniffs/Exceptions/ThrowCatchSniff.php b/Magento2/Sniffs/Exceptions/ThrowCatchSniff.php index 50c84307..7ce1ae90 100644 --- a/Magento2/Sniffs/Exceptions/ThrowCatchSniff.php +++ b/Magento2/Sniffs/Exceptions/ThrowCatchSniff.php @@ -3,6 +3,7 @@ * Copyright © Magento. All rights reserved. * See COPYING.txt for license details. */ + namespace Magento2\Sniffs\Exceptions; use function array_slice; @@ -33,7 +34,7 @@ class ThrowCatchSniff implements Sniff */ public function register() { - return [T_FUNCTION, T_CLOSURE]; + return [T_TRY]; } /** @@ -42,53 +43,76 @@ public function register() public function process(File $phpcsFile, $stackPtr) { $tokens = $phpcsFile->getTokens(); - if (!isset($tokens[$stackPtr]['scope_closer'])) { - // Probably an interface method no check - return; - } + $endOfStatement = $phpcsFile->findEndOfStatement($stackPtr); + + $throwClassNames = []; + $searchForNextThrow = $stackPtr; - $closeBrace = $tokens[$stackPtr]['scope_closer']; - $throwTags = []; - $catchTags = []; + // search for all throws + do { + $throwTag = $phpcsFile->findNext(T_THROW, $searchForNextThrow, $endOfStatement); - for ($i = $stackPtr; $i < $closeBrace; $i++) { - $token = $tokens[$i]; - if ($token['code'] === T_CATCH) { - $catchTags[] = $token; - } - if ($token['code'] === T_THROW) { - $throwTags[] = $i; + if ($throwTag === false) { + break; } - } - if (count($catchTags) === 0 || count($throwTags) === 0) { - // No catch or throw found no check - return; + $throwClassNames[] = $this->getFullClassName($tokens, $throwTag + 1); + + $searchForNextThrow = $throwTag + 1; + } while ($throwTag !== false); + + if (empty($throwClassNames)) { + return; // is not relevant not throw in try found. } $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']; - } + // TRY statements need to check until the end of all CATCH statements. + do { + $nextToken = $phpcsFile->findNext(T_WHITESPACE, ($endOfStatement + 1), null, true); + if ($tokens[$nextToken]['code'] === T_CATCH) { + $endOfStatement = $tokens[$nextToken]['scope_closer']; + $catchClassNames[$nextToken] = $this->getFullClassName($tokens, $nextToken + 1); + } else { + break; + } + } while (isset($tokens[$nextToken]['scope_closer']) === true); - // find all relevant classes in throws - foreach ($throwTags as $throwTag) { - $match = $phpcsFile->findNext(T_STRING, $throwTag); - $throwClassNames[] = $tokens[$match]['content']; + if (empty($catchClassName)) { + return; // is not relevant no catch found } $throwClassNames = array_flip($throwClassNames); foreach ($catchClassNames as $match => $catchClassName) { - if (array_key_exists($catchClassName, $throwClassNames)) { + if (isset($throwClassNames[$catchClassName])) { $phpcsFile->addWarning($this->warningMessage, $match, $this->warningCode); } } } + + /** + * Get the full class name with namespace. + * + * @param array $tokens + * @param int $startPos + * @return string + */ + private function getFullClassName(array $tokens, int $startPos) + { + $fullName = ""; + $endOfClassName = [T_SEMICOLON => 0, T_CLOSE_PARENTHESIS => 0]; + for ($i = $startPos; $i <= count($tokens); $i++) { + $type = $tokens[$i]['code']; + + if ($type === T_STRING || $type === T_NS_SEPARATOR) { + $fullName .= $tokens[$i]['content']; + } + + if (array_key_exists($type, $endOfClassName)) { + break; // line end each + } + } + + return $fullName; + } } diff --git a/Magento2/Tests/Exceptions/ThrowCatchUnitTest.1.inc b/Magento2/Tests/Exceptions/ThrowCatchUnitTest.1.inc new file mode 100644 index 00000000..5ed95f47 --- /dev/null +++ b/Magento2/Tests/Exceptions/ThrowCatchUnitTest.1.inc @@ -0,0 +1,18 @@ + 1, 120 => 1, From 73ad14a7a59fbb3eb0bd03873bf8b535a8c38f31 Mon Sep 17 00:00:00 2001 From: roettigl Date: Thu, 1 Aug 2019 17:01:02 +0200 Subject: [PATCH 2/4] MC-18816: Impelement an better check for ThrowCatchSniff --- Magento2/Sniffs/Exceptions/ThrowCatchSniff.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Magento2/Sniffs/Exceptions/ThrowCatchSniff.php b/Magento2/Sniffs/Exceptions/ThrowCatchSniff.php index 7ce1ae90..19b87bd2 100644 --- a/Magento2/Sniffs/Exceptions/ThrowCatchSniff.php +++ b/Magento2/Sniffs/Exceptions/ThrowCatchSniff.php @@ -78,7 +78,7 @@ public function process(File $phpcsFile, $stackPtr) } } while (isset($tokens[$nextToken]['scope_closer']) === true); - if (empty($catchClassName)) { + if (empty($catchClassNames)) { return; // is not relevant no catch found } @@ -101,7 +101,9 @@ private function getFullClassName(array $tokens, int $startPos) { $fullName = ""; $endOfClassName = [T_SEMICOLON => 0, T_CLOSE_PARENTHESIS => 0]; - for ($i = $startPos; $i <= count($tokens); $i++) { + + $tokenCount = count($tokens); + for ($i = $startPos; $i <= $tokenCount; $i++) { $type = $tokens[$i]['code']; if ($type === T_STRING || $type === T_NS_SEPARATOR) { From 67063d3fc049767e4455a7dd710c5e3644d03c68 Mon Sep 17 00:00:00 2001 From: roettigl Date: Thu, 1 Aug 2019 17:19:17 +0200 Subject: [PATCH 3/4] MC-18816: Impelement an better check for ThrowCatchSniff --- Magento2/Sniffs/Exceptions/ThrowCatchSniff.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Magento2/Sniffs/Exceptions/ThrowCatchSniff.php b/Magento2/Sniffs/Exceptions/ThrowCatchSniff.php index 19b87bd2..7468857d 100644 --- a/Magento2/Sniffs/Exceptions/ThrowCatchSniff.php +++ b/Magento2/Sniffs/Exceptions/ThrowCatchSniff.php @@ -97,7 +97,7 @@ public function process(File $phpcsFile, $stackPtr) * @param int $startPos * @return string */ - private function getFullClassName(array $tokens, int $startPos) + private function getFullClassName(array $tokens, $startPos) { $fullName = ""; $endOfClassName = [T_SEMICOLON => 0, T_CLOSE_PARENTHESIS => 0]; From 15857340abac6e4628e99464817e3040493729ee Mon Sep 17 00:00:00 2001 From: Lena Orobei Date: Thu, 1 Aug 2019 15:20:28 -0500 Subject: [PATCH 4/4] Update ThrowCatchUnitTest.1.inc --- Magento2/Tests/Exceptions/ThrowCatchUnitTest.1.inc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Magento2/Tests/Exceptions/ThrowCatchUnitTest.1.inc b/Magento2/Tests/Exceptions/ThrowCatchUnitTest.1.inc index 5ed95f47..24b18f02 100644 --- a/Magento2/Tests/Exceptions/ThrowCatchUnitTest.1.inc +++ b/Magento2/Tests/Exceptions/ThrowCatchUnitTest.1.inc @@ -1,6 +1,6 @@