From e07a290cbf83dbdb446fb84db5e0599d8e666477 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 15 Nov 2024 11:26:53 +0100 Subject: [PATCH 1/2] Revert "Common::getSniffCode(): be more lenient about sniffs not following naming conventions" This reverts commit 71b452422dbeb2dd44ce7daad8d1fcc14094bcfc. Includes further enhancements to strictly apply the naming conventions by: 1. Making sure that the FQN contains a `Sniffs` or `Tests` sub-namespace in the correct position. 2. Making sure that the category is not called `Sniffs`. 3. Making sure that a class ends on `Sniff` or `UnitTest`, and that that is a suffix, not the sole name of the class. Includes updating the tests. Related to 689 Related to 6 --- src/Runner.php | 1 - src/Util/Common.php | 30 +++++----- tests/Core/Util/Common/GetSniffCodeTest.php | 63 +++++---------------- 3 files changed, 31 insertions(+), 63 deletions(-) diff --git a/src/Runner.php b/src/Runner.php index 88f0740a7e..ca1d1476a7 100644 --- a/src/Runner.php +++ b/src/Runner.php @@ -643,7 +643,6 @@ public function processFile($file) try { if (empty($nextStack) === false && isset($nextStack['class']) === true - && substr($nextStack['class'], -5) === 'Sniff' ) { $sniffCode = 'the '.Common::getSniffCode($nextStack['class']).' sniff'; } diff --git a/src/Util/Common.php b/src/Util/Common.php index cb6965f62c..5ca34ded59 100644 --- a/src/Util/Common.php +++ b/src/Util/Common.php @@ -532,7 +532,7 @@ public static function suggestType($varType) * @return string * * @throws \InvalidArgumentException When $sniffClass is not a non-empty string. - * @throws \InvalidArgumentException When $sniffClass is not a FQN for a sniff(test) class. + * @throws \InvalidArgumentException When $sniffClass is not a valid FQN for a sniff(test) class. */ public static function getSniffCode($sniffClass) { @@ -542,12 +542,22 @@ public static function getSniffCode($sniffClass) $parts = explode('\\', $sniffClass); $partsCount = count($parts); - $sniff = $parts[($partsCount - 1)]; + if ($partsCount < 4 + || ($parts[($partsCount - 3)] !== 'Sniffs' + && $parts[($partsCount - 3)] !== 'Tests') + || $parts[($partsCount - 2)] === 'Sniffs' + ) { + throw new InvalidArgumentException( + 'The $sniffClass parameter was not passed a fully qualified sniff(test) class name. Received: '.$sniffClass + ); + } + + $sniff = $parts[($partsCount - 1)]; - if (substr($sniff, -5) === 'Sniff') { + if ($sniff !== 'Sniff' && substr($sniff, -5) === 'Sniff') { // Sniff class name. $sniff = substr($sniff, 0, -5); - } else if (substr($sniff, -8) === 'UnitTest') { + } else if ($sniff !== 'UnitTest' && substr($sniff, -8) === 'UnitTest') { // Unit test class name. $sniff = substr($sniff, 0, -8); } else { @@ -556,16 +566,8 @@ public static function getSniffCode($sniffClass) ); } - $standard = ''; - if (isset($parts[($partsCount - 4)]) === true) { - $standard = $parts[($partsCount - 4)]; - } - - $category = ''; - if (isset($parts[($partsCount - 2)]) === true) { - $category = $parts[($partsCount - 2)]; - } - + $standard = $parts[($partsCount - 4)]; + $category = $parts[($partsCount - 2)]; return $standard.'.'.$category.'.'.$sniff; }//end getSniffCode() diff --git a/tests/Core/Util/Common/GetSniffCodeTest.php b/tests/Core/Util/Common/GetSniffCodeTest.php index 444ea5eb0d..4b4083dedf 100644 --- a/tests/Core/Util/Common/GetSniffCodeTest.php +++ b/tests/Core/Util/Common/GetSniffCodeTest.php @@ -105,9 +105,16 @@ public function testGetSniffCodeThrowsExceptionOnInputWhichIsNotASniffTestClass( public static function dataGetSniffCodeThrowsExceptionOnInputWhichIsNotASniffTestClass() { return [ - 'Unqualified class name' => ['ClassName'], - 'Fully qualified class name, not enough parts' => ['Fully\\Qualified\\ClassName'], + 'Unqualified class name' => ['ClassNameSniff'], + 'Fully qualified sniff class name, not enough parts [1]' => ['Fully\\Qualified\\ClassNameSniff'], + 'Fully qualified sniff class name, not enough parts [2]' => ['CompanyName\\CheckMeSniff'], + 'Fully qualified test class name, not enough parts' => ['Fully\\Qualified\\ClassNameUnitTest'], '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'], + 'Fully qualified class name, last part *is* Sniff' => ['CompanyName\\Sniffs\\Category\\Sniff'], + 'Fully qualified class name, last part *is* UnitTest' => ['CompanyName\\Tests\\Category\\UnitTest'], + 'Fully qualified class name, no Sniffs or Tests leaf' => ['CompanyName\\CustomSniffs\\Whatever\\CheckMeSniff'], + 'Fully qualified class name, category called Sniffs' => ['CompanyName\\Sniffs\\Sniffs\\InvalidCategorySniff'], ]; }//end dataGetSniffCodeThrowsExceptionOnInputWhichIsNotASniffTestClass() @@ -140,70 +147,30 @@ public function testGetSniffCode($fqnClass, $expected) public static function dataGetSniffCode() { return [ - 'PHPCS native sniff' => [ + 'PHPCS native sniff' => [ 'fqnClass' => 'PHP_CodeSniffer\\Standards\\Generic\\Sniffs\\Arrays\\ArrayIndentSniff', 'expected' => 'Generic.Arrays.ArrayIndent', ], - 'Class is a PHPCS native test class' => [ + 'Class is a PHPCS native test class' => [ 'fqnClass' => 'PHP_CodeSniffer\\Standards\\Generic\\Tests\\Arrays\\ArrayIndentUnitTest', 'expected' => 'Generic.Arrays.ArrayIndent', ], - 'Sniff in external standard without namespace prefix' => [ + 'Sniff in external standard without namespace prefix' => [ 'fqnClass' => 'MyStandard\\Sniffs\\PHP\\MyNameSniff', 'expected' => 'MyStandard.PHP.MyName', ], - 'Test in external standard without namespace prefix' => [ + 'Test in external standard without namespace prefix' => [ 'fqnClass' => 'MyStandard\\Tests\\PHP\\MyNameUnitTest', 'expected' => 'MyStandard.PHP.MyName', ], - 'Sniff in external standard with namespace prefix' => [ + 'Sniff in external standard with namespace prefix' => [ 'fqnClass' => 'Vendor\\Package\\MyStandard\\Sniffs\\Category\\AnalyzeMeSniff', 'expected' => 'MyStandard.Category.AnalyzeMe', ], - 'Test in external standard with namespace prefix' => [ + 'Test in external standard with namespace prefix' => [ 'fqnClass' => 'Vendor\\Package\\MyStandard\\Tests\\Category\\AnalyzeMeUnitTest', 'expected' => 'MyStandard.Category.AnalyzeMe', ], - - /* - * These are not valid sniff codes and is an undesirable result, but can't be helped - * as changing this would be a BC-break. - * Supporting these to allow for tags directly including sniff files. - * See: https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/675 - */ - - 'Fully qualified class name, ends on Sniff, but isn\'t' => [ - 'fqnClass' => 'Fully\\Sniffs\\AbstractSomethingSniff', - 'expected' => '.Sniffs.AbstractSomething', - ], - 'Sniff provided via file include and doesn\'t comply with naming conventions [1]' => [ - 'fqnClass' => 'CheckMeSniff', - 'expected' => '..CheckMe', - ], - 'Sniff provided via file include and doesn\'t comply with naming conventions [2]' => [ - 'fqnClass' => 'CompanyName\\CheckMeSniff', - 'expected' => '.CompanyName.CheckMe', - ], - 'Sniff provided via file include and doesn\'t comply with naming conventions [3]' => [ - 'fqnClass' => 'CompanyName\\Sniffs\\CheckMeSniff', - 'expected' => '.Sniffs.CheckMe', - ], - 'Sniff provided via file include and doesn\'t comply with naming conventions [4]' => [ - 'fqnClass' => 'CompanyName\\CustomSniffs\\Whatever\\CheckMeSniff', - 'expected' => 'CompanyName.Whatever.CheckMe', - ], - 'Sniff provided via file include and doesn\'t comply with naming conventions [5]' => [ - 'fqnClass' => 'CompanyName\\Sniffs\\Category\\Sniff', - 'expected' => 'CompanyName.Category.', - ], - 'Sniff provided via file include and doesn\'t comply with naming conventions [6]' => [ - 'fqnClass' => 'CompanyName\\Tests\\Category\\UnitTest', - 'expected' => 'CompanyName.Category.', - ], - 'Sniff provided via file include and doesn\'t comply with naming conventions [7]' => [ - 'fqnClass' => 'Sniffs\\Category\\NamedSniff', - 'expected' => '.Category.Named', - ], ]; }//end dataGetSniffCode() From c2636f5ca165b3ec63809689d11d1304490f10fd Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 6 Feb 2025 22:25:40 +0100 Subject: [PATCH 2/2] Ruleset: remove support for sniffs not following the naming conventions The [About Standards for PHP_CodeSniffer](https://github.com/PHPCSStandards/PHP_CodeSniffer/wiki/About-Standards-for-PHP_CodeSniffer) wiki page outlines exactly what the naming conventions are. This PR changes the previously added Ruleset deprecation notice to an error and starts rejecting sniffs which do not comply with the naming conventions. Includes updated tests. Closes 689 Related to 6 --- src/Ruleset.php | 25 ++++---- ...ateTokenListenersNamingConventionsTest.php | 62 ++++++++----------- 2 files changed, 36 insertions(+), 51 deletions(-) diff --git a/src/Ruleset.php b/src/Ruleset.php index 7a25bf31ca..00fb5ef455 100644 --- a/src/Ruleset.php +++ b/src/Ruleset.php @@ -11,6 +11,7 @@ namespace PHP_CodeSniffer; +use InvalidArgumentException; use PHP_CodeSniffer\Exceptions\RuntimeException; use PHP_CodeSniffer\Sniffs\DeprecatedSniff; use PHP_CodeSniffer\Util\Common; @@ -1461,23 +1462,19 @@ public function populateTokenListeners() $this->tokenListeners = []; foreach ($this->sniffs as $sniffClass => $sniffObject) { - $this->sniffs[$sniffClass] = null; - $this->sniffs[$sniffClass] = new $sniffClass(); - - $sniffCode = Common::getSniffCode($sniffClass); - - if (substr($sniffCode, 0, 1) === '.' - || substr($sniffCode, -1) === '.' - || strpos($sniffCode, '..') !== false - || preg_match('`(^|\.)Sniffs\.`', $sniffCode) === 1 - || preg_match('`[^\s\.-]+\\\\Sniffs\\\\[^\s\.-]+\\\\[^\s\.-]+Sniff`', $sniffClass) !== 1 - ) { - $message = "The sniff $sniffClass does not comply with the PHP_CodeSniffer naming conventions."; - $message .= ' This will no longer be supported in PHPCS 4.0.'.PHP_EOL; + try { + $sniffCode = Common::getSniffCode($sniffClass); + } catch (InvalidArgumentException $e) { + $message = "The sniff $sniffClass does not comply with the PHP_CodeSniffer naming conventions.".PHP_EOL; $message .= 'Contact the sniff author to fix the sniff.'; - $this->msgCache->add($message, MessageCollector::DEPRECATED); + $this->msgCache->add($message, MessageCollector::ERROR); + + // Unregister the sniff. + unset($this->sniffs[$sniffClass]); + continue; } + $this->sniffs[$sniffClass] = new $sniffClass(); $this->sniffCodes[$sniffCode] = $sniffClass; $isDeprecated = false; diff --git a/tests/Core/Ruleset/PopulateTokenListenersNamingConventionsTest.php b/tests/Core/Ruleset/PopulateTokenListenersNamingConventionsTest.php index d93840521b..d1f6660854 100644 --- a/tests/Core/Ruleset/PopulateTokenListenersNamingConventionsTest.php +++ b/tests/Core/Ruleset/PopulateTokenListenersNamingConventionsTest.php @@ -11,14 +11,14 @@ use PHP_CodeSniffer\Ruleset; use PHP_CodeSniffer\Tests\ConfigDouble; -use PHPUnit\Framework\TestCase; +use PHP_CodeSniffer\Tests\Core\Ruleset\AbstractRulesetTestCase; /** * Test handling of sniffs not following the PHPCS naming conventions in the Ruleset::populateTokenListeners() method. * * @covers \PHP_CodeSniffer\Ruleset::populateTokenListeners */ -final class PopulateTokenListenersNamingConventionsTest extends TestCase +final class PopulateTokenListenersNamingConventionsTest extends AbstractRulesetTestCase { @@ -26,12 +26,31 @@ final class PopulateTokenListenersNamingConventionsTest extends TestCase * Verify a warning is shown for sniffs not complying with the PHPCS naming conventions. * * Including sniffs which do not comply with the PHPCS naming conventions is soft deprecated since - * PHPCS 3.12.0, hard deprecated since PHPCS 3.13.0 and support will be removed in PHPCS 4.0.0. + * PHPCS 3.12.0, hard deprecated since PHPCS 3.13.0 and support has been removed in PHPCS 4.0.0. * * @return void */ public function testBrokenNamingConventions() { + $expectedMessage = 'ERROR: The sniff BrokenNamingConventions\\Sniffs\\MissingCategoryDirSniff does not comply'; + $expectedMessage .= ' with the PHP_CodeSniffer naming conventions.'.PHP_EOL; + $expectedMessage .= 'Contact the sniff author to fix the sniff.'.PHP_EOL; + $expectedMessage .= 'ERROR: The sniff NoNamespaceSniff does not comply with the PHP_CodeSniffer naming conventions.'.PHP_EOL; + $expectedMessage .= 'Contact the sniff author to fix the sniff.'.PHP_EOL; + $expectedMessage .= 'ERROR: The sniff Sniffs\PartialNamespaceSniff does not comply with the PHP_CodeSniffer naming conventions.'.PHP_EOL; + $expectedMessage .= 'Contact the sniff author to fix the sniff.'.PHP_EOL; + $expectedMessage .= 'ERROR: The sniff BrokenNamingConventions\Sniffs\Category\Sniff does not comply'; + $expectedMessage .= ' with the PHP_CodeSniffer naming conventions.'.PHP_EOL; + $expectedMessage .= 'Contact the sniff author to fix the sniff.'.PHP_EOL; + $expectedMessage .= 'ERROR: The sniff BrokenNamingConventions\Sniffs\Sniffs\CategoryCalledSniffsSniff does not'; + $expectedMessage .= ' comply with the PHP_CodeSniffer naming conventions.'.PHP_EOL; + $expectedMessage .= 'Contact the sniff author to fix the sniff.'.PHP_EOL; + $expectedMessage .= 'ERROR: The sniff BrokenNamingConventions\Sniffs\Category\SubDir\TooDeeplyNestedSniff'; + $expectedMessage .= ' does not comply with the PHP_CodeSniffer naming conventions.'.PHP_EOL; + $expectedMessage .= 'Contact the sniff author to fix the sniff.'.PHP_EOL.PHP_EOL; + + $this->expectRuntimeExceptionMessage($expectedMessage); + // Set up the ruleset. $standard = __DIR__.'/PopulateTokenListenersNamingConventionsTest.xml'; $config = new ConfigDouble(["--standard=$standard"]); @@ -39,42 +58,11 @@ public function testBrokenNamingConventions() // The "Generic.PHP.BacktickOperator" sniff is the only valid sniff. $expectedSniffCodes = [ - '..NoNamespace' => 'NoNamespaceSniff', - '.Sniffs.MissingCategoryDir' => 'BrokenNamingConventions\\Sniffs\\MissingCategoryDirSniff', - '.Sniffs.PartialNamespace' => 'Sniffs\\PartialNamespaceSniff', - 'BrokenNamingConventions.Category.' => 'BrokenNamingConventions\\Sniffs\\Category\\Sniff', - 'BrokenNamingConventions.Sniffs.CategoryCalledSniffs' => 'BrokenNamingConventions\\Sniffs\\Sniffs\\CategoryCalledSniffsSniff', - 'Generic.PHP.BacktickOperator' => 'PHP_CodeSniffer\\Standards\\Generic\\Sniffs\\PHP\\BacktickOperatorSniff', - 'Sniffs.SubDir.TooDeeplyNested' => 'BrokenNamingConventions\\Sniffs\\Category\\SubDir\\TooDeeplyNestedSniff', + 'Generic.PHP.BacktickOperator' => 'PHP_CodeSniffer\\Standards\\Generic\\Sniffs\\PHP\\BacktickOperatorSniff', ]; - // Sort the value to make the tests stable as different OSes will read directories - // in a different order and the order is not relevant for these tests. Just the values. - $actual = $ruleset->sniffCodes; - ksort($actual); - - $this->assertSame($expectedSniffCodes, $actual, 'Registered sniffs do not match expectation'); - - $expectedMessage = 'DEPRECATED: The sniff BrokenNamingConventions\\Sniffs\\MissingCategoryDirSniff does not comply'; - $expectedMessage .= ' with the PHP_CodeSniffer naming conventions. This will no longer be supported in PHPCS 4.0.'.PHP_EOL; - $expectedMessage .= 'Contact the sniff author to fix the sniff.'.PHP_EOL; - $expectedMessage .= 'DEPRECATED: The sniff NoNamespaceSniff does not comply with the PHP_CodeSniffer naming conventions.'; - $expectedMessage .= ' This will no longer be supported in PHPCS 4.0.'.PHP_EOL; - $expectedMessage .= 'Contact the sniff author to fix the sniff.'.PHP_EOL; - $expectedMessage .= 'DEPRECATED: The sniff Sniffs\\PartialNamespaceSniff does not comply with the PHP_CodeSniffer naming conventions.'; - $expectedMessage .= ' This will no longer be supported in PHPCS 4.0.'.PHP_EOL; - $expectedMessage .= 'Contact the sniff author to fix the sniff.'.PHP_EOL; - $expectedMessage .= 'DEPRECATED: The sniff BrokenNamingConventions\\Sniffs\\Category\\Sniff does not comply'; - $expectedMessage .= ' with the PHP_CodeSniffer naming conventions. This will no longer be supported in PHPCS 4.0.'.PHP_EOL; - $expectedMessage .= 'Contact the sniff author to fix the sniff.'.PHP_EOL; - $expectedMessage .= 'DEPRECATED: The sniff BrokenNamingConventions\\Sniffs\\Sniffs\\CategoryCalledSniffsSniff does not'; - $expectedMessage .= ' comply with the PHP_CodeSniffer naming conventions. This will no longer be supported in PHPCS 4.0.'.PHP_EOL; - $expectedMessage .= 'Contact the sniff author to fix the sniff.'.PHP_EOL; - $expectedMessage .= 'DEPRECATED: The sniff BrokenNamingConventions\\Sniffs\\Category\\SubDir\\TooDeeplyNestedSniff'; - $expectedMessage .= ' does not comply with the PHP_CodeSniffer naming conventions. This will no longer be supported in PHPCS 4.0.'.PHP_EOL; - $expectedMessage .= 'Contact the sniff author to fix the sniff.'.PHP_EOL.PHP_EOL; - - $this->expectOutputString($expectedMessage); + // This assertion will only take effect for PHPUnit 10+. + $this->assertSame($expectedSniffCodes, $ruleset->sniffCodes, 'Registered sniffs do not match expectation'); }//end testBrokenNamingConventions()