From ff62a6e2fce5432d77420e31ac7b0a7c2f6026b3 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 15 Feb 2025 01:02:30 +0100 Subject: [PATCH] Squiz/ClassDeclaration: allow for function attributes The `Squiz.Classes.ClassDeclaration` sniff checks the number of blank lines _after_ a class closing brace to be exactly one blank line. To prevent conflicts with other sniffs checking blank lines before function declarations, it bows out when the next thing after the class declaration is a function declaration and defers to the function declaration sniff to handle the "blank lines before function" check. See squizlabs/PHP_CodeSniffer 2033 for historic context. However, the above "bowing out" breaks when a function after a class has PHP 8.0+ attributes attached to it and the sniff would still the `NewlinesAfterCloseBrace` error. Fixed now. Includes tests. Note: the new "errors" caused by the new tests are for the `MultipleClasses` code, which is unrelated to this fix and valid. --- .../Sniffs/Classes/ClassDeclarationSniff.php | 25 ++++++++++++++++--- .../Classes/ClassDeclarationUnitTest.inc | 22 ++++++++++++++++ .../ClassDeclarationUnitTest.inc.fixed | 22 ++++++++++++++++ .../Classes/ClassDeclarationUnitTest.php | 2 ++ 4 files changed, 67 insertions(+), 4 deletions(-) diff --git a/src/Standards/Squiz/Sniffs/Classes/ClassDeclarationSniff.php b/src/Standards/Squiz/Sniffs/Classes/ClassDeclarationSniff.php index 3f5e1fbc79..9df6b39392 100644 --- a/src/Standards/Squiz/Sniffs/Classes/ClassDeclarationSniff.php +++ b/src/Standards/Squiz/Sniffs/Classes/ClassDeclarationSniff.php @@ -167,11 +167,28 @@ public function processClose(File $phpcsFile, $stackPtr) }//end if if ($difference !== -1 && $difference !== 1) { - if ($tokens[$nextContent]['code'] === T_DOC_COMMENT_OPEN_TAG) { - $next = $phpcsFile->findNext(T_WHITESPACE, ($tokens[$nextContent]['comment_closer'] + 1), null, true); - if ($next !== false && $tokens[$next]['code'] === T_FUNCTION) { - return; + for ($nextSignificant = $nextContent; $nextSignificant < $phpcsFile->numTokens; $nextSignificant++) { + if ($tokens[$nextSignificant]['code'] === T_WHITESPACE) { + continue; } + + if ($tokens[$nextSignificant]['code'] === T_DOC_COMMENT_OPEN_TAG) { + $nextSignificant = $tokens[$nextSignificant]['comment_closer']; + continue; + } + + if ($tokens[$nextSignificant]['code'] === T_ATTRIBUTE + && isset($tokens[$nextSignificant]['attribute_closer']) === true + ) { + $nextSignificant = $tokens[$nextSignificant]['attribute_closer']; + continue; + } + + break; + } + + if ($tokens[$nextSignificant]['code'] === T_FUNCTION) { + return; } $error = 'Closing brace of a %s must be followed by a single blank line; found %s'; diff --git a/src/Standards/Squiz/Tests/Classes/ClassDeclarationUnitTest.inc b/src/Standards/Squiz/Tests/Classes/ClassDeclarationUnitTest.inc index 2af42d3721..25d8d30211 100644 --- a/src/Standards/Squiz/Tests/Classes/ClassDeclarationUnitTest.inc +++ b/src/Standards/Squiz/Tests/Classes/ClassDeclarationUnitTest.inc @@ -128,3 +128,25 @@ class Test readonly class Test { } + +class TooMuchSpacingBelowClassButShouldNotBeFlaggedWhenNextThingIsFunctionWithAttribute +{ + var $x; +} + + +#[AttributesShouldBeJumpedOver] +function ThisIsFineAndHasAttribute() {} + +class TooMuchSpacingBelowClassButShouldNotBeFlaggedWhenNextThingIsFunctionWithDocblockAndAttribute +{ + var $x; +} + + +/** + * No error. + */ +#[AttributesShouldBeJumpedOver] +#[ASecondAttributeShouldBeJumpedOverToo]#[AndAThirdAsWell] +function ThisIsFineAndHasDocblockAndAttribute() {} diff --git a/src/Standards/Squiz/Tests/Classes/ClassDeclarationUnitTest.inc.fixed b/src/Standards/Squiz/Tests/Classes/ClassDeclarationUnitTest.inc.fixed index 5d01b68e0a..bf042f932d 100644 --- a/src/Standards/Squiz/Tests/Classes/ClassDeclarationUnitTest.inc.fixed +++ b/src/Standards/Squiz/Tests/Classes/ClassDeclarationUnitTest.inc.fixed @@ -138,3 +138,25 @@ readonly class Test readonly class Test { } + +class TooMuchSpacingBelowClassButShouldNotBeFlaggedWhenNextThingIsFunctionWithAttribute +{ + var $x; +} + + +#[AttributesShouldBeJumpedOver] +function ThisIsFineAndHasAttribute() {} + +class TooMuchSpacingBelowClassButShouldNotBeFlaggedWhenNextThingIsFunctionWithDocblockAndAttribute +{ + var $x; +} + + +/** + * No error. + */ +#[AttributesShouldBeJumpedOver] +#[ASecondAttributeShouldBeJumpedOverToo]#[AndAThirdAsWell] +function ThisIsFineAndHasDocblockAndAttribute() {} diff --git a/src/Standards/Squiz/Tests/Classes/ClassDeclarationUnitTest.php b/src/Standards/Squiz/Tests/Classes/ClassDeclarationUnitTest.php index 635fd5d10a..be3fd59970 100644 --- a/src/Standards/Squiz/Tests/Classes/ClassDeclarationUnitTest.php +++ b/src/Standards/Squiz/Tests/Classes/ClassDeclarationUnitTest.php @@ -68,6 +68,8 @@ public function getErrorList() 121 => 1, 124 => 2, 128 => 2, + 132 => 1, + 141 => 1, ]; }//end getErrorList()