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/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/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() 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()