From d9a3d1cf991b1b4a71bf19c70bdf8a37713871db Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 13 Feb 2025 09:14:17 +0100 Subject: [PATCH] Squiz/ValidClassName: bug fix - improve comment handling Noticed while working on something else. If there would be a comment between the OO keyword and the declared name, the sniff could throw false positives with unhelpful error messages, like: ``` 193 | ERROR | Class name "/*comment*/" is not in PascalCase format (Squiz.Classes.ValidClassName.NotCamelCaps) 194 | ERROR | Trait name "//comment" is not in PascalCase format (Squiz.Classes.ValidClassName.NotCamelCaps) 196 | ERROR | Interface name "// phpcs:ignore Stnd.Cat.SniffName -- just testing" is not in PascalCase format | | (Squiz.Classes.ValidClassName.NotCamelCaps) 199 | ERROR | Class name "CommentsShouldBeIgnoredValid/*comment*/" is not in PascalCase format (Squiz.Classes.ValidClassName.NotCamelCaps) 200 | ERROR | Interface name "annotations_should_be_ignored_InvalidName" is not in PascalCase format (Squiz.Classes.ValidClassName.NotCamelCaps) ``` Fixed now by: 1. Ignoring any comments between the OO keyword and the name. 2. Not including comments directly following a name in the name to be evaluated. Includes tests. Includes minor error message precision fix - the error will now be thrown on the name which is being flagged as invalid, not on the OO keyword. --- .../Squiz/Sniffs/Classes/ValidClassNameSniff.php | 7 ++++--- .../Squiz/Tests/Classes/ValidClassNameUnitTest.inc | 10 ++++++++++ .../Squiz/Tests/Classes/ValidClassNameUnitTest.php | 3 +++ 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/Standards/Squiz/Sniffs/Classes/ValidClassNameSniff.php b/src/Standards/Squiz/Sniffs/Classes/ValidClassNameSniff.php index a8975353fd..5f4f261fc3 100644 --- a/src/Standards/Squiz/Sniffs/Classes/ValidClassNameSniff.php +++ b/src/Standards/Squiz/Sniffs/Classes/ValidClassNameSniff.php @@ -12,6 +12,7 @@ use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Sniffs\Sniff; use PHP_CodeSniffer\Util\Common; +use PHP_CodeSniffer\Util\Tokens; class ValidClassNameSniff implements Sniff { @@ -58,8 +59,8 @@ public function process(File $phpcsFile, $stackPtr) // simply look for the first T_STRING because a class name // starting with the number will be multiple tokens. $opener = $tokens[$stackPtr]['scope_opener']; - $nameStart = $phpcsFile->findNext(T_WHITESPACE, ($stackPtr + 1), $opener, true); - $nameEnd = $phpcsFile->findNext([T_WHITESPACE, T_COLON], $nameStart, $opener); + $nameStart = $phpcsFile->findNext(Tokens::$emptyTokens, ($stackPtr + 1), $opener, true); + $nameEnd = $phpcsFile->findNext((Tokens::$emptyTokens + [T_COLON => T_COLON]), $nameStart, $opener); if ($nameEnd === false) { $name = $tokens[$nameStart]['content']; } else { @@ -75,7 +76,7 @@ public function process(File $phpcsFile, $stackPtr) $type, $name, ]; - $phpcsFile->addError($error, $stackPtr, 'NotCamelCaps', $data); + $phpcsFile->addError($error, $nameStart, 'NotCamelCaps', $data); $phpcsFile->recordMetric($stackPtr, 'PascalCase class name', 'no'); } else { $phpcsFile->recordMetric($stackPtr, 'PascalCase class name', 'yes'); diff --git a/src/Standards/Squiz/Tests/Classes/ValidClassNameUnitTest.inc b/src/Standards/Squiz/Tests/Classes/ValidClassNameUnitTest.inc index 3fe39435b4..7ec00650be 100644 --- a/src/Standards/Squiz/Tests/Classes/ValidClassNameUnitTest.inc +++ b/src/Standards/Squiz/Tests/Classes/ValidClassNameUnitTest.inc @@ -189,3 +189,13 @@ $foo = new class( } ) extends DateTime { }; + +class /*comment*/ CommentsShouldBeIgnoredValidName {} +trait //comment + commentsshouldbeignoredInvalidName {} +interface // phpcs:ignore Stnd.Cat.SniffName -- just testing + annotationshouldbeignored_InvalidName {} + +class CommentsShouldBeIgnoredValid/*comment*/ {} +interface annotations_should_be_ignored_InvalidName// phpcs:ignore Stnd.Cat.SniffName -- just testing +{} diff --git a/src/Standards/Squiz/Tests/Classes/ValidClassNameUnitTest.php b/src/Standards/Squiz/Tests/Classes/ValidClassNameUnitTest.php index 8e505a4188..d23816591b 100644 --- a/src/Standards/Squiz/Tests/Classes/ValidClassNameUnitTest.php +++ b/src/Standards/Squiz/Tests/Classes/ValidClassNameUnitTest.php @@ -57,6 +57,9 @@ public function getErrorList() 150 => 1, 151 => 1, 156 => 1, + 195 => 1, + 197 => 1, + 200 => 1, ]; }//end getErrorList()