From 3c01b3e10a5a23f78248a38a07016fc702b83962 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 16 Nov 2024 01:09:25 +0100 Subject: [PATCH] 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(). ``` --- src/Ruleset.php | 18 +++++ .../NoImplementsNoProcessSniff.php | 17 ++++ .../NoImplementsNoRegisterOrProcessSniff.php | 12 +++ .../NoImplementsNoRegisterSniff.php | 19 +++++ ...ssRulesetAutoExpandSniffsDirectoryTest.xml | 1 + ...sInvalidSniffNoImplementsNoProcessTest.xml | 8 ++ ...iffNoImplementsNoRegisterOrProcessTest.xml | 8 ++ ...InvalidSniffNoImplementsNoRegisterTest.xml | 8 ++ .../RegisterSniffsRejectsInvalidSniffTest.php | 80 +++++++++++++++++++ .../Ruleset/ShowSniffDeprecationsTest.xml | 1 + 10 files changed, 172 insertions(+) create mode 100644 tests/Core/Ruleset/Fixtures/TestStandard/Sniffs/InvalidSniffError/NoImplementsNoProcessSniff.php create mode 100644 tests/Core/Ruleset/Fixtures/TestStandard/Sniffs/InvalidSniffError/NoImplementsNoRegisterOrProcessSniff.php create mode 100644 tests/Core/Ruleset/Fixtures/TestStandard/Sniffs/InvalidSniffError/NoImplementsNoRegisterSniff.php create mode 100644 tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoProcessTest.xml create mode 100644 tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterOrProcessTest.xml create mode 100644 tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterTest.xml create mode 100644 tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffTest.php diff --git a/src/Ruleset.php b/src/Ruleset.php index 9a9f959163..b6279bf9f3 100644 --- a/src/Ruleset.php +++ b/src/Ruleset.php @@ -1409,6 +1409,24 @@ public function registerSniffs($files, $restrictions, $exclusions) continue; } + if ($reflection->implementsInterface('PHP_CodeSniffer\Sniffs\Sniff') === false) { + // Skip classes which don't implement the register() or process() methods. + if (method_exists($className, 'register') === false + || method_exists($className, 'process') === false + ) { + $errorMsg = 'Sniff class %s is missing required method %s().'; + if (method_exists($className, 'register') === false) { + $this->msgCache->add(sprintf($errorMsg, $className, 'register'), MessageCollector::ERROR); + } + + if (method_exists($className, 'process') === false) { + $this->msgCache->add(sprintf($errorMsg, $className, 'process'), MessageCollector::ERROR); + } + + continue; + } + }//end if + $listeners[$className] = $className; if (PHP_CODESNIFFER_VERBOSITY > 2) { diff --git a/tests/Core/Ruleset/Fixtures/TestStandard/Sniffs/InvalidSniffError/NoImplementsNoProcessSniff.php b/tests/Core/Ruleset/Fixtures/TestStandard/Sniffs/InvalidSniffError/NoImplementsNoProcessSniff.php new file mode 100644 index 0000000000..d6aa4ee6a8 --- /dev/null +++ b/tests/Core/Ruleset/Fixtures/TestStandard/Sniffs/InvalidSniffError/NoImplementsNoProcessSniff.php @@ -0,0 +1,17 @@ + + diff --git a/tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoProcessTest.xml b/tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoProcessTest.xml new file mode 100644 index 0000000000..6b70d32d85 --- /dev/null +++ b/tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoProcessTest.xml @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterOrProcessTest.xml b/tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterOrProcessTest.xml new file mode 100644 index 0000000000..06e2028375 --- /dev/null +++ b/tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterOrProcessTest.xml @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterTest.xml b/tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterTest.xml new file mode 100644 index 0000000000..9e0913f9cc --- /dev/null +++ b/tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterTest.xml @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffTest.php b/tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffTest.php new file mode 100644 index 0000000000..1efe5cdba0 --- /dev/null +++ b/tests/Core/Ruleset/RegisterSniffsRejectsInvalidSniffTest.php @@ -0,0 +1,80 @@ + + * @copyright 2025 PHPCSStandards and contributors + * @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence + */ + +namespace PHP_CodeSniffer\Tests\Core\Ruleset; + +use PHP_CodeSniffer\Ruleset; +use PHP_CodeSniffer\Tests\ConfigDouble; +use PHP_CodeSniffer\Tests\Core\Ruleset\AbstractRulesetTestCase; + +/** + * Tests that invalid sniffs will be rejected with an informative error message. + * + * @covers \PHP_CodeSniffer\Ruleset::registerSniffs + */ +final class RegisterSniffsRejectsInvalidSniffTest extends AbstractRulesetTestCase +{ + + + /** + * Verify that an error is thrown if an invalid sniff class is loaded. + * + * @param string $standard The standard to use for the test. + * @param string $methodName The name of the missing method. + * + * @dataProvider dataExceptionIsThrownOnMissingInterfaceMethod + * + * @return void + */ + public function testExceptionIsThrownOnMissingInterfaceMethod($standard, $methodName) + { + // Set up the ruleset. + $standard = __DIR__.'/'.$standard; + $config = new ConfigDouble(["--standard=$standard"]); + + $regex = "`(^|\R)ERROR: Sniff class \S+Sniff is missing required method $methodName\(\)\.\R`"; + $this->expectRuntimeExceptionRegex($regex); + + new Ruleset($config); + + }//end testExceptionIsThrownOnMissingInterfaceMethod() + + + /** + * Data provider. + * + * @see testExceptionIsThrownOnMissingInterfaceMethod() + * + * @return array> + */ + public static function dataExceptionIsThrownOnMissingInterfaceMethod() + { + return [ + 'Missing register() method' => [ + 'standard' => 'RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterTest.xml', + 'methodName' => 'register', + ], + 'Missing process() method' => [ + 'standard' => 'RegisterSniffsRejectsInvalidSniffNoImplementsNoProcessTest.xml', + 'methodName' => 'process', + ], + 'Missing both, checking register() method' => [ + 'standard' => 'RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterOrProcessTest.xml', + 'methodName' => 'register', + ], + 'Missing both, checking process() method' => [ + 'standard' => 'RegisterSniffsRejectsInvalidSniffNoImplementsNoRegisterOrProcessTest.xml', + 'methodName' => 'process', + ], + ]; + + }//end dataExceptionIsThrownOnMissingInterfaceMethod() + + +}//end class diff --git a/tests/Core/Ruleset/ShowSniffDeprecationsTest.xml b/tests/Core/Ruleset/ShowSniffDeprecationsTest.xml index 802bd3c0a9..ab632b1959 100644 --- a/tests/Core/Ruleset/ShowSniffDeprecationsTest.xml +++ b/tests/Core/Ruleset/ShowSniffDeprecationsTest.xml @@ -6,6 +6,7 @@ +