Skip to content

Commit 97e340e

Browse files
jrfnljrfnl
committed
Ruleset: handle invalid sniffs more graciously
As things were, a ruleset loading an invalid sniff - a sniff which doesn't implement the `Sniff` interface and is missing either the `register()` or `process()` method, or both -, would result in fatal errors which are unfriendly to the end-user. Now, while this will hopefully be a very rare occurrence and should no longer be possible as of PHPCS 4.0, which intends to remove support for sniffs not implementing the `Sniff` interface, I still believe it prudent to show more user-friendly and more informative error messages to the end-user until that time. This commit implements this and executes step 2 to address issue 694.. Includes tests. Output of commands involving various invalid sniffs **before** this PR: ``` $ phpcs -ps . --standard=./tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterTest.xml Fatal error: Uncaught Error: Call to undefined method TestStandard\Sniffs\InvalidSniffError\NoImplementsNoRegisterSniff::register() in path/to/PHP_CodeSniffer/src/Ruleset.php:1505 Stack trace: thrown in path/to/PHP_CodeSniffer/src/Ruleset.php on line 1505 ``` ``` $ phpcs -ps . --standard=./tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoProcessTest.xml PHP_CodeSniffer version 3.11.1 (stable) by Squiz and PHPCSStandards Fatal error: Uncaught Error: Call to undefined method TestStandard\Sniffs\InvalidSniffError\NoImplementsNoProcessSniff::process() in path/to/PHP_CodeSniffer/src/Files/File.php:519 Stack trace: thrown in path/to/PHP_CodeSniffer/src/Files/File.php on line 519 ``` ``` $ phpcs -ps . --standard=./tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterOrProcessTest.xml PHP_CodeSniffer version 3.11.1 (stable) by Squiz and PHPCSStandards Fatal error: Uncaught Error: Call to undefined method TestStandard\Sniffs\InvalidSniffError\NoImplementsNoRegisterOrProcessSniff::register() in path/to/PHP_CodeSniffer/src/Ruleset.php:1505 Stack trace: thrown in path/to/PHP_CodeSniffer/src/Ruleset.php on line 1505 ``` Output of commands involving various invalid sniffs **after** this PR: ``` $ phpcs -ps . --standard=./tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterTest.xml ERROR: Sniff class TestStandard\Sniffs\InvalidSniffError\NoImplementsNoRegisterSniff is missing required method register(). ``` ``` $ phpcs -ps . --standard=./tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoProcessTest.xml ERROR: Sniff class TestStandard\Sniffs\InvalidSniffError\NoImplementsNoProcessSniff is missing required method process(). ``` ``` $ phpcs -ps . --standard=./tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterOrProcessTest.xml ERROR: Sniff class TestStandard\Sniffs\InvalidSniffError\NoImplementsNoRegisterOrProcessSniff is missing required method register(). ERROR: Sniff class TestStandard\Sniffs\InvalidSniffError\NoImplementsNoRegisterOrProcessSniff is missing required method process(). ```
1 parent c6a579e commit 97e340e

10 files changed

+172
-0
lines changed

‎src/Ruleset.php‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1409,6 +1409,24 @@ public function registerSniffs($files, $restrictions, $exclusions)
14091409
continue;
14101410
}
14111411

1412+
if ($reflection->implementsInterface('PHP_CodeSniffer\Sniffs\Sniff') === false) {
1413+
// Skip classes which don't implement the register() or process() methods.
1414+
if (method_exists($className, 'register') === false
1415+
|| method_exists($className, 'process') === false
1416+
) {
1417+
$errorMsg = 'Sniff class %s is missing required method %s().';
1418+
if (method_exists($className, 'register') === false) {
1419+
$this->msgCache->add(sprintf($errorMsg, $className, 'register'), MessageCollector::ERROR);
1420+
}
1421+
1422+
if (method_exists($className, 'process') === false) {
1423+
$this->msgCache->add(sprintf($errorMsg, $className, 'process'), MessageCollector::ERROR);
1424+
}
1425+
1426+
continue;
1427+
}
1428+
}//end if
1429+
14121430
$listeners[$className] = $className;
14131431

14141432
if (PHP_CODESNIFFER_VERBOSITY > 2) {

‎tests/Core/Ruleset/Fixtures/TestStandard/Sniffs/InvalidSniffError/NoImplementsNoProcessSniff.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
/**
3+
* Test fixture.
4+
*
5+
* @see \PHP_CodeSniffer\Tests\Core\Ruleset\RegisterSniffsRejectsInvalidSniffTest
6+
*/
7+
8+
namespace Fixtures\TestStandard\Sniffs\InvalidSniffError;
9+
10+
final class NoImplementsNoProcessSniff
11+
{
12+
13+
public function register()
14+
{
15+
return [T_OPEN_TAG];
16+
}
17+
}

‎tests/Core/Ruleset/Fixtures/TestStandard/Sniffs/InvalidSniffError/NoImplementsNoRegisterOrProcessSniff.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?php
2+
/**
3+
* Test fixture.
4+
*
5+
* @see \PHP_CodeSniffer\Tests\Core\Ruleset\RegisterSniffsRejectsInvalidSniffTest
6+
*/
7+
8+
namespace Fixtures\TestStandard\Sniffs\InvalidSniffError;
9+
10+
final class NoImplementsNoRegisterOrProcessSniff
11+
{
12+
}

‎tests/Core/Ruleset/Fixtures/TestStandard/Sniffs/InvalidSniffError/NoImplementsNoRegisterSniff.php‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
/**
3+
* Test fixture.
4+
*
5+
* @see \PHP_CodeSniffer\Tests\Core\Ruleset\RegisterSniffsRejectsInvalidSniffTest
6+
*/
7+
8+
namespace Fixtures\TestStandard\Sniffs\InvalidSniffError;
9+
10+
use PHP_CodeSniffer\Files\File;
11+
12+
final class NoImplementsNoRegisterSniff
13+
{
14+
15+
public function process(File $phpcsFile, $stackPtr)
16+
{
17+
// Do something.
18+
}
19+
}

‎tests/Core/Ruleset/ProcessRulesetAutoExpandSniffsDirectoryTest.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
<rule ref="TestStandard">
77
<exclude name="TestStandard.InvalidSniffs"/>
8+
<exclude name="TestStandard.InvalidSniffError"/>
89
</rule>
910

1011
</ruleset>

‎tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoProcessTest.xml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?xml version="1.0"?>
2+
<ruleset xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" name="RegisterSniffsRejectsInvalidSniffTest" xsi:noNamespaceSchemaLocation="https://raw.githubusercontent.com/PHPCSStandards/PHP_CodeSniffer/master/phpcs.xsd">
3+
4+
<rule ref="./tests/Core/Ruleset/Fixtures/TestStandard/Sniffs/InvalidSniffError/NoImplementsNoProcessSniff.php"/>
5+
6+
<!-- Prevent a "no sniff were registered" error. -->
7+
<rule ref="Generic.PHP.BacktickOperator"/>
8+
</ruleset>

‎tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterOrProcessTest.xml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?xml version="1.0"?>
2+
<ruleset xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" name="RegisterSniffsRejectsInvalidSniffTest" xsi:noNamespaceSchemaLocation="https://raw.githubusercontent.com/PHPCSStandards/PHP_CodeSniffer/master/phpcs.xsd">
3+
4+
<rule ref="./tests/Core/Ruleset/Fixtures/TestStandard/Sniffs/InvalidSniffError/NoImplementsNoRegisterOrProcessSniff.php"/>
5+
6+
<!-- Prevent a "no sniff were registered" error. -->
7+
<rule ref="Generic.PHP.BacktickOperator"/>
8+
</ruleset>

‎tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterTest.xml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?xml version="1.0"?>
2+
<ruleset xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" name="RegisterSniffsRejectsInvalidSniffTest" xsi:noNamespaceSchemaLocation="https://raw.githubusercontent.com/PHPCSStandards/PHP_CodeSniffer/master/phpcs.xsd">
3+
4+
<rule ref="./tests/Core/Ruleset/Fixtures/TestStandard/Sniffs/InvalidSniffError/NoImplementsNoRegisterSniff.php"/>
5+
6+
<!-- Prevent a "no sniff were registered" error. -->
7+
<rule ref="Generic.PHP.BacktickOperator"/>
8+
</ruleset>

‎tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffTest.php‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
<?php
2+
/**
3+
* Tests that invalid sniffs will be rejected with an informative error message.
4+
*
5+
* @author Juliette Reinders Folmer <[email protected]>
6+
* @copyright 2025 PHPCSStandards and contributors
7+
* @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
8+
*/
9+
10+
namespace PHP_CodeSniffer\Tests\Core\Ruleset;
11+
12+
use PHP_CodeSniffer\Ruleset;
13+
use PHP_CodeSniffer\Tests\ConfigDouble;
14+
use PHP_CodeSniffer\Tests\Core\Ruleset\AbstractRulesetTestCase;
15+
16+
/**
17+
* Tests that invalid sniffs will be rejected with an informative error message.
18+
*
19+
* @covers \PHP_CodeSniffer\Ruleset::registerSniffs
20+
*/
21+
final class RegisterSniffsRejectsInvalidSniffTest extends AbstractRulesetTestCase
22+
{
23+
24+
25+
/**
26+
* Verify that an error is thrown if an invalid sniff class is loaded.
27+
*
28+
* @param string $standard The standard to use for the test.
29+
* @param string $methodName The name of the missing method.
30+
*
31+
* @dataProvider dataExceptionIsThrownOnMissingInterfaceMethod
32+
*
33+
* @return void
34+
*/
35+
public function testExceptionIsThrownOnMissingInterfaceMethod($standard, $methodName)
36+
{
37+
// Set up the ruleset.
38+
$standard = __DIR__.'/'.$standard;
39+
$config = new ConfigDouble(["--standard=$standard"]);
40+
41+
$regex = "`(^|\R)ERROR: Sniff class \S+Sniff is missing required method $methodName\(\)\.\R`";
42+
$this->expectRuntimeExceptionRegex($regex);
43+
44+
new Ruleset($config);
45+
46+
}//end testExceptionIsThrownOnMissingInterfaceMethod()
47+
48+
49+
/**
50+
* Data provider.
51+
*
52+
* @see testExceptionIsThrownOnMissingInterfaceMethod()
53+
*
54+
* @return array<string, array<string, string>>
55+
*/
56+
public static function dataExceptionIsThrownOnMissingInterfaceMethod()
57+
{
58+
return [
59+
'Missing register() method' => [
60+
'standard' => 'RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterTest.xml',
61+
'methodName' => 'register',
62+
],
63+
'Missing process() method' => [
64+
'standard' => 'RegisterSniffsRejectsInvalidSniffNoImplementsNoProcessTest.xml',
65+
'methodName' => 'process',
66+
],
67+
'Missing both, checking register() method' => [
68+
'standard' => 'RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterOrProcessTest.xml',
69+
'methodName' => 'register',
70+
],
71+
'Missing both, checking process() method' => [
72+
'standard' => 'RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterOrProcessTest.xml',
73+
'methodName' => 'process',
74+
],
75+
];
76+
77+
}//end dataExceptionIsThrownOnMissingInterfaceMethod()
78+
79+
80+
}//end class

‎tests/Core/Ruleset/ShowSniffDeprecationsTest.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
<rule ref="TestStandard">
77
<exclude name="TestStandard.DeprecatedInvalid"/>
88
<exclude name="TestStandard.InvalidSniffs"/>
9+
<exclude name="TestStandard.InvalidSniffError"/>
910
</rule>
1011

1112
</ruleset>

0 commit comments

Comments
 (0)