From 53306fa3f6e85c0711a1506df6f09b580f4c40dc Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 3 Oct 2024 21:07:27 +0200 Subject: [PATCH] Runner::processFile()/error handling: add more defensive coding Follow up to squizlabs/PHP_CodeSniffer 3716 and PR 524. PR squizlabs/PHP_CodeSniffer 3716 improved the information provided in the error messages for `Internal.Exception`s with additional information about the source of the problem. This code uses the `Common::getSniffCode()` method to transform a sniff class name into a sniff code. PR 524 changes how the `Common::getSniffCode()` method handles invalid input, i.e. it will now throw an exception when a class is passed which isn't a sniff (or sniff test). In the context of this error handling, however, this new exception can easily be encountered when an `Abstract...Sniff` class contains the code which triggered the error - which would trigger the exception as it is not a sniff class. This commit now adds some additional defensive coding to ensure that a) the error message will still be informative, while b) making sure that any exception which may be thrown by the `Common::getSniffCode()` method will be caught and and won't cause an uncaught `InvalidArgumentException` in the error handling code to stop execution of the script. Includes a test documenting the behaviour of the `Common::getSniffCode()` method for abstract sniff classes. --- src/Runner.php | 22 ++++++++++++++------- tests/Core/Util/Common/GetSniffCodeTest.php | 1 + 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/Runner.php b/src/Runner.php index cfd844732f..20c2eddf44 100644 --- a/src/Runner.php +++ b/src/Runner.php @@ -13,6 +13,7 @@ namespace PHP_CodeSniffer; use Exception; +use InvalidArgumentException; use PHP_CodeSniffer\Exceptions\DeepExitException; use PHP_CodeSniffer\Exceptions\RuntimeException; use PHP_CodeSniffer\Files\DummyFile; @@ -688,16 +689,23 @@ public function processFile($file) } if (empty($sniffStack) === false) { - if (empty($nextStack) === false - && isset($nextStack['class']) === true - && substr($nextStack['class'], -5) === 'Sniff' - ) { - $sniffCode = Common::getSniffCode($nextStack['class']); - } else { + $sniffCode = ''; + try { + if (empty($nextStack) === false + && isset($nextStack['class']) === true + && substr($nextStack['class'], -5) === 'Sniff' + ) { + $sniffCode = 'the '.Common::getSniffCode($nextStack['class']).' sniff'; + } + } catch (InvalidArgumentException $e) { + // Sniff code could not be determined. This may be an abstract sniff class. + } + + if ($sniffCode === '') { $sniffCode = substr(strrchr(str_replace('\\', '/', $sniffStack['file']), '/'), 1); } - $error .= sprintf(PHP_EOL.'The error originated in the %s sniff on line %s.', $sniffCode, $sniffStack['line']); + $error .= sprintf(PHP_EOL.'The error originated in %s on line %s.', $sniffCode, $sniffStack['line']); } $file->addErrorOnLine($error, 1, 'Internal.Exception'); diff --git a/tests/Core/Util/Common/GetSniffCodeTest.php b/tests/Core/Util/Common/GetSniffCodeTest.php index d27593416d..7cf9b16edf 100644 --- a/tests/Core/Util/Common/GetSniffCodeTest.php +++ b/tests/Core/Util/Common/GetSniffCodeTest.php @@ -108,6 +108,7 @@ public static function dataGetSniffCodeThrowsExceptionOnInputWhichIsNotASniffTes 'Unqualified class name' => ['ClassName'], 'Fully qualified class name, not enough parts' => ['Fully\\Qualified\\ClassName'], 'Fully qualified class name, doesn\'t end on Sniff or UnitTest' => ['Fully\\Sniffs\\Qualified\\ClassName'], + 'Fully qualified class name, ends on Sniff, but isn\'t' => ['Fully\\Sniffs\\AbstractSomethingSniff'], ]; }//end dataGetSniffCodeThrowsExceptionOnInputWhichIsNotASniffTestClass()