From 9b837dd7b95822c3d82b2c746354d2512896248f Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 5 Jun 2024 17:39:47 +0200 Subject: [PATCH 1/5] Common::getSniffCode(): add tests Add initial set of tests for the `Common::getSniffCode()` method. Related to 146 Related to [review comment in PR 446](https://github.com/PHPCSStandards/PHP_CodeSniffer/pull/446#discussion_r1573947430). --- tests/Core/Util/Common/GetSniffCodeTest.php | 80 +++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 tests/Core/Util/Common/GetSniffCodeTest.php diff --git a/tests/Core/Util/Common/GetSniffCodeTest.php b/tests/Core/Util/Common/GetSniffCodeTest.php new file mode 100644 index 0000000000..738b6f55dd --- /dev/null +++ b/tests/Core/Util/Common/GetSniffCodeTest.php @@ -0,0 +1,80 @@ + + * @copyright 2024 PHPCSStandards and contributors + * @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence + */ + +namespace PHP_CodeSniffer\Tests\Core\Util\Common; + +use PHP_CodeSniffer\Util\Common; +use PHPUnit\Framework\TestCase; + +/** + * Tests for the \PHP_CodeSniffer\Util\Common::getSniffCode() method. + * + * @covers \PHP_CodeSniffer\Util\Common::getSniffCode + */ +final class GetSniffCodeTest extends TestCase +{ + + + /** + * Test transforming a sniff class name to a sniff code. + * + * @param string $fqnClass A fully qualified sniff class name. + * @param string $expected Expected function output. + * + * @dataProvider dataGetSniffCode + * + * @return void + */ + public function testGetSniffCode($fqnClass, $expected) + { + $this->assertSame($expected, Common::getSniffCode($fqnClass)); + + }//end testGetSniffCode() + + + /** + * Data provider. + * + * @see testGetSniffCode() + * + * @return array> + */ + public static function dataGetSniffCode() + { + return [ + 'PHPCS native sniff' => [ + 'fqnClass' => 'PHP_CodeSniffer\\Standards\\Generic\\Sniffs\\Arrays\\ArrayIndentSniff', + 'expected' => 'Generic.Arrays.ArrayIndent', + ], + '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' => [ + 'fqnClass' => 'MyStandard\\Sniffs\\PHP\\MyNameSniff', + 'expected' => 'MyStandard.PHP.MyName', + ], + 'Test in external standard without namespace prefix' => [ + 'fqnClass' => 'MyStandard\\Tests\\PHP\\MyNameSniff', + 'expected' => 'MyStandard.PHP.MyName', + ], + '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' => [ + 'fqnClass' => 'Vendor\\Package\\MyStandard\\Tests\\Category\\AnalyzeMeUnitTest', + 'expected' => 'MyStandard.Category.AnalyzeMe', + ], + ]; + + }//end dataGetSniffCode() + + +}//end class From 5fac55450b8cf34d30b291fd0aa6dfe949983de4 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 5 Jun 2024 18:02:12 +0200 Subject: [PATCH 2/5] Common::getSniffCode(): throw exception on invalid input [1] Previously, if an empty string was passed, the `Common::getSniffCode()` method would return `..`, which is just confusing (and incorrect). This commit changes the behaviour to throw an `InvalidArgumentException` instead. Includes making a potentially superfluous function call to the method conditionally (as it could hit the new exception). Includes test. --- src/Files/File.php | 10 +++-- src/Util/Common.php | 7 ++++ tests/Core/Util/Common/GetSniffCodeTest.php | 45 +++++++++++++++++++++ 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/src/Files/File.php b/src/Files/File.php index 3e1409c58a..29b15c0e08 100644 --- a/src/Files/File.php +++ b/src/Files/File.php @@ -873,9 +873,13 @@ protected function addMessage($error, $message, $line, $column, $code, $data, $s $parts = explode('.', $code); if ($parts[0] === 'Internal') { // An internal message. - $listenerCode = Common::getSniffCode($this->activeListener); - $sniffCode = $code; - $checkCodes = [$sniffCode]; + $listenerCode = ''; + if ($this->activeListener !== '') { + $listenerCode = Common::getSniffCode($this->activeListener); + } + + $sniffCode = $code; + $checkCodes = [$sniffCode]; } else { if ($parts[0] !== $code) { // The full message code has been passed in. diff --git a/src/Util/Common.php b/src/Util/Common.php index ca00d23898..ee1824297a 100644 --- a/src/Util/Common.php +++ b/src/Util/Common.php @@ -9,6 +9,7 @@ namespace PHP_CodeSniffer\Util; +use InvalidArgumentException; use Phar; class Common @@ -529,9 +530,15 @@ public static function suggestType($varType) * @param string $sniffClass The fully qualified sniff class name. * * @return string + * + * @throws \InvalidArgumentException When $sniffClass is not a non-empty string. */ public static function getSniffCode($sniffClass) { + if (is_string($sniffClass) === false || $sniffClass === '') { + throw new InvalidArgumentException('The $sniffClass parameter must be a non-empty string'); + } + $parts = explode('\\', $sniffClass); $sniff = array_pop($parts); diff --git a/tests/Core/Util/Common/GetSniffCodeTest.php b/tests/Core/Util/Common/GetSniffCodeTest.php index 738b6f55dd..70b6f97755 100644 --- a/tests/Core/Util/Common/GetSniffCodeTest.php +++ b/tests/Core/Util/Common/GetSniffCodeTest.php @@ -21,6 +21,51 @@ final class GetSniffCodeTest extends TestCase { + /** + * Test receiving an expected exception when the $sniffClass parameter is not passed a string value or is passed an empty string. + * + * @param string $input NOT a fully qualified sniff class name. + * + * @dataProvider dataGetSniffCodeThrowsExceptionOnInvalidInput + * + * @return void + */ + public function testGetSniffCodeThrowsExceptionOnInvalidInput($input) + { + $exception = 'InvalidArgumentException'; + $message = 'The $sniffClass parameter must be a non-empty string'; + + if (method_exists($this, 'expectException') === true) { + // PHPUnit 5+. + $this->expectException($exception); + $this->expectExceptionMessage($message); + } else { + // PHPUnit 4. + $this->setExpectedException($exception, $message); + } + + Common::getSniffCode($input); + + }//end testGetSniffCodeThrowsExceptionOnInvalidInput() + + + /** + * Data provider. + * + * @see testGetSniffCodeThrowsExceptionOnInvalidInput() + * + * @return array> + */ + public static function dataGetSniffCodeThrowsExceptionOnInvalidInput() + { + return [ + 'Class name is not a string' => [true], + 'Class name is empty' => [''], + ]; + + }//end dataGetSniffCodeThrowsExceptionOnInvalidInput() + + /** * Test transforming a sniff class name to a sniff code. * From 1719a66a82f9464af806a525f6b27ed28bb4352d Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 5 Jun 2024 18:20:20 +0200 Subject: [PATCH 3/5] Common::getSniffCode(): throw exception on invalid input [2a] Previously, if an invalid (incomplete) class name was passed, the `Common::getSniffCode()` method would return a garbled name, like `.Qualified.C`, which is just confusing. This commit changes the behaviour to throw an `InvalidArgumentException` instead. Includes test. --- src/Util/Common.php | 7 ++++ tests/Core/Util/Common/GetSniffCodeTest.php | 46 +++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/src/Util/Common.php b/src/Util/Common.php index ee1824297a..c993aa6a13 100644 --- a/src/Util/Common.php +++ b/src/Util/Common.php @@ -532,6 +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. */ public static function getSniffCode($sniffClass) { @@ -540,6 +541,12 @@ public static function getSniffCode($sniffClass) } $parts = explode('\\', $sniffClass); + if (count($parts) < 4) { + throw new InvalidArgumentException( + 'The $sniffClass parameter was not passed a fully qualified sniff class name. Received: '.$sniffClass + ); + } + $sniff = array_pop($parts); if (substr($sniff, -5) === 'Sniff') { diff --git a/tests/Core/Util/Common/GetSniffCodeTest.php b/tests/Core/Util/Common/GetSniffCodeTest.php index 70b6f97755..49740634d0 100644 --- a/tests/Core/Util/Common/GetSniffCodeTest.php +++ b/tests/Core/Util/Common/GetSniffCodeTest.php @@ -66,6 +66,52 @@ public static function dataGetSniffCodeThrowsExceptionOnInvalidInput() }//end dataGetSniffCodeThrowsExceptionOnInvalidInput() + /** + * Test receiving an expected exception when the $sniffClass parameter is not passed a value which + * could be a fully qualified sniff(test) class name. + * + * @param string $input String input which can not be a fully qualified sniff(test) class name. + * + * @dataProvider dataGetSniffCodeThrowsExceptionOnInputWhichIsNotASniffTestClass + * + * @return void + */ + public function testGetSniffCodeThrowsExceptionOnInputWhichIsNotASniffTestClass($input) + { + $exception = 'InvalidArgumentException'; + $message = 'The $sniffClass parameter was not passed a fully qualified sniff class name. Received:'; + + if (method_exists($this, 'expectException') === true) { + // PHPUnit 5+. + $this->expectException($exception); + $this->expectExceptionMessage($message); + } else { + // PHPUnit 4. + $this->setExpectedException($exception, $message); + } + + Common::getSniffCode($input); + + }//end testGetSniffCodeThrowsExceptionOnInputWhichIsNotASniffTestClass() + + + /** + * Data provider. + * + * @see testGetSniffCodeThrowsExceptionOnInputWhichIsNotASniffTestClass() + * + * @return array> + */ + public static function dataGetSniffCodeThrowsExceptionOnInputWhichIsNotASniffTestClass() + { + return [ + 'Unqualified class name' => ['ClassName'], + 'Fully qualified class name, not enough parts' => ['Fully\\Qualified\\ClassName'], + ]; + + }//end dataGetSniffCodeThrowsExceptionOnInputWhichIsNotASniffTestClass() + + /** * Test transforming a sniff class name to a sniff code. * From 2aac9ade167cde2eebf4106a165b3ddedd0f26ae Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 5 Jun 2024 18:27:17 +0200 Subject: [PATCH 4/5] Common::getSniffCode(): throw exception on invalid input [2b] Previously, if an invalid class name was passed, which didn't end on `Sniff` or `UnitTest`, the `Common::getSniffCode()` method would return a garbled name, like `Fully.Qualified.C`, which is just confusing. This commit changes the behaviour to throw an `InvalidArgumentException` instead. Includes test. --- src/Util/Common.php | 8 ++++++-- tests/Core/Util/Common/GetSniffCodeTest.php | 7 ++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/Util/Common.php b/src/Util/Common.php index c993aa6a13..2428f78d7d 100644 --- a/src/Util/Common.php +++ b/src/Util/Common.php @@ -543,7 +543,7 @@ public static function getSniffCode($sniffClass) $parts = explode('\\', $sniffClass); if (count($parts) < 4) { throw new InvalidArgumentException( - 'The $sniffClass parameter was not passed a fully qualified sniff class name. Received: '.$sniffClass + 'The $sniffClass parameter was not passed a fully qualified sniff(test) class name. Received: '.$sniffClass ); } @@ -552,9 +552,13 @@ public static function getSniffCode($sniffClass) if (substr($sniff, -5) === 'Sniff') { // Sniff class name. $sniff = substr($sniff, 0, -5); - } else { + } else if (substr($sniff, -8) === 'UnitTest') { // Unit test class name. $sniff = substr($sniff, 0, -8); + } else { + throw new InvalidArgumentException( + 'The $sniffClass parameter was not passed a fully qualified sniff(test) class name. Received: '.$sniffClass + ); } $category = array_pop($parts); diff --git a/tests/Core/Util/Common/GetSniffCodeTest.php b/tests/Core/Util/Common/GetSniffCodeTest.php index 49740634d0..d27593416d 100644 --- a/tests/Core/Util/Common/GetSniffCodeTest.php +++ b/tests/Core/Util/Common/GetSniffCodeTest.php @@ -79,7 +79,7 @@ public static function dataGetSniffCodeThrowsExceptionOnInvalidInput() public function testGetSniffCodeThrowsExceptionOnInputWhichIsNotASniffTestClass($input) { $exception = 'InvalidArgumentException'; - $message = 'The $sniffClass parameter was not passed a fully qualified sniff class name. Received:'; + $message = 'The $sniffClass parameter was not passed a fully qualified sniff(test) class name. Received:'; if (method_exists($this, 'expectException') === true) { // PHPUnit 5+. @@ -105,8 +105,9 @@ 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' => ['ClassName'], + 'Fully qualified class name, not enough parts' => ['Fully\\Qualified\\ClassName'], + 'Fully qualified class name, doesn\'t end on Sniff or UnitTest' => ['Fully\\Sniffs\\Qualified\\ClassName'], ]; }//end dataGetSniffCodeThrowsExceptionOnInputWhichIsNotASniffTestClass() From 7449b299075f07664ad0f5c1cfc2d6a7b3567c59 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 5 Jun 2024 18:37:58 +0200 Subject: [PATCH 5/5] Common::getSniffCode(): minor simplification * Remove the use of `array_pop()` in favour of directly referencing the required "parts" by their index in the array. * Remove the unused `$sniffDir` variable. * Remove the unnecessary `$code` variable. Related to [review comment in PR 446](https://github.com/PHPCSStandards/PHP_CodeSniffer/pull/446#discussion_r1573947430). --- src/Util/Common.php | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/Util/Common.php b/src/Util/Common.php index 2428f78d7d..af4aba69fd 100644 --- a/src/Util/Common.php +++ b/src/Util/Common.php @@ -540,14 +540,15 @@ public static function getSniffCode($sniffClass) throw new InvalidArgumentException('The $sniffClass parameter must be a non-empty string'); } - $parts = explode('\\', $sniffClass); - if (count($parts) < 4) { + $parts = explode('\\', $sniffClass); + $partsCount = count($parts); + if ($partsCount < 4) { throw new InvalidArgumentException( 'The $sniffClass parameter was not passed a fully qualified sniff(test) class name. Received: '.$sniffClass ); } - $sniff = array_pop($parts); + $sniff = $parts[($partsCount - 1)]; if (substr($sniff, -5) === 'Sniff') { // Sniff class name. @@ -561,11 +562,9 @@ public static function getSniffCode($sniffClass) ); } - $category = array_pop($parts); - $sniffDir = array_pop($parts); - $standard = array_pop($parts); - $code = $standard.'.'.$category.'.'.$sniff; - return $code; + $standard = $parts[($partsCount - 4)]; + $category = $parts[($partsCount - 2)]; + return $standard.'.'.$category.'.'.$sniff; }//end getSniffCode()