Skip to content

Commit 53306fa

Browse files
committed
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.
1 parent 36d731e commit 53306fa

File tree

2 files changed

+16
-7
lines changed

2 files changed

+16
-7
lines changed

src/Runner.php

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
namespace PHP_CodeSniffer;
1414

1515
use Exception;
16+
use InvalidArgumentException;
1617
use PHP_CodeSniffer\Exceptions\DeepExitException;
1718
use PHP_CodeSniffer\Exceptions\RuntimeException;
1819
use PHP_CodeSniffer\Files\DummyFile;
@@ -688,16 +689,23 @@ public function processFile($file)
688689
}
689690

690691
if (empty($sniffStack) === false) {
691-
if (empty($nextStack) === false
692-
&& isset($nextStack['class']) === true
693-
&& substr($nextStack['class'], -5) === 'Sniff'
694-
) {
695-
$sniffCode = Common::getSniffCode($nextStack['class']);
696-
} else {
692+
$sniffCode = '';
693+
try {
694+
if (empty($nextStack) === false
695+
&& isset($nextStack['class']) === true
696+
&& substr($nextStack['class'], -5) === 'Sniff'
697+
) {
698+
$sniffCode = 'the '.Common::getSniffCode($nextStack['class']).' sniff';
699+
}
700+
} catch (InvalidArgumentException $e) {
701+
// Sniff code could not be determined. This may be an abstract sniff class.
702+
}
703+
704+
if ($sniffCode === '') {
697705
$sniffCode = substr(strrchr(str_replace('\\', '/', $sniffStack['file']), '/'), 1);
698706
}
699707

700-
$error .= sprintf(PHP_EOL.'The error originated in the %s sniff on line %s.', $sniffCode, $sniffStack['line']);
708+
$error .= sprintf(PHP_EOL.'The error originated in %s on line %s.', $sniffCode, $sniffStack['line']);
701709
}
702710

703711
$file->addErrorOnLine($error, 1, 'Internal.Exception');

tests/Core/Util/Common/GetSniffCodeTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ public static function dataGetSniffCodeThrowsExceptionOnInputWhichIsNotASniffTes
108108
'Unqualified class name' => ['ClassName'],
109109
'Fully qualified class name, not enough parts' => ['Fully\\Qualified\\ClassName'],
110110
'Fully qualified class name, doesn\'t end on Sniff or UnitTest' => ['Fully\\Sniffs\\Qualified\\ClassName'],
111+
'Fully qualified class name, ends on Sniff, but isn\'t' => ['Fully\\Sniffs\\AbstractSomethingSniff'],
111112
];
112113

113114
}//end dataGetSniffCodeThrowsExceptionOnInputWhichIsNotASniffTestClass()

0 commit comments

Comments
 (0)