From 1792d9970f01693d365587aca3f18565a5486f18 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 8 Apr 2025 17:54:14 +0200 Subject: [PATCH] Squiz/FunctionSpacing: fix regression - only look at first docblock To determine whether there are the correct number of lines before the function declaration, the sniff skips over attributes and docblocks to find the first bit of code which doesn't belong with the function declaration. While the change in PR 826 improved this for attributes _before_ the docblock, it didn't take multiple docblocks into account and would skip over more than just the first docblock. Fixed now. Includes tests. --- .../WhiteSpace/FunctionSpacingSniff.php | 9 ++++--- .../WhiteSpace/FunctionSpacingUnitTest.1.inc | 24 +++++++++++++++++ .../FunctionSpacingUnitTest.1.inc.fixed | 26 +++++++++++++++++++ .../WhiteSpace/FunctionSpacingUnitTest.php | 1 + 4 files changed, 57 insertions(+), 3 deletions(-) diff --git a/src/Standards/Squiz/Sniffs/WhiteSpace/FunctionSpacingSniff.php b/src/Standards/Squiz/Sniffs/WhiteSpace/FunctionSpacingSniff.php index a213a94af1..8e25717e18 100644 --- a/src/Standards/Squiz/Sniffs/WhiteSpace/FunctionSpacingSniff.php +++ b/src/Standards/Squiz/Sniffs/WhiteSpace/FunctionSpacingSniff.php @@ -127,15 +127,18 @@ public function process(File $phpcsFile, $stackPtr) } // Skip past function docblocks and attributes. - $prev = $startOfDeclarationLine; + // Only the first docblock is a function docblock. Other docblocks should be disregarded. + $prev = $startOfDeclarationLine; + $seenDocblock = false; if ($startOfDeclarationLine > 0) { for ($prev = ($startOfDeclarationLine - 1); $prev > 0; $prev--) { if ($tokens[$prev]['code'] === T_WHITESPACE) { continue; } - if ($tokens[$prev]['code'] === T_DOC_COMMENT_CLOSE_TAG) { - $prev = $tokens[$prev]['comment_opener']; + if ($seenDocblock === false && $tokens[$prev]['code'] === T_DOC_COMMENT_CLOSE_TAG) { + $prev = $tokens[$prev]['comment_opener']; + $seenDocblock = true; continue; } diff --git a/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.1.inc b/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.1.inc index c516775cc6..83600e8738 100644 --- a/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.1.inc +++ b/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.1.inc @@ -726,3 +726,27 @@ class SilenceBeforeErrorIfPreviousThingWasAFunctionBug }//end blankLineDetectionB() }//end class + +// Issue #945. +class OnlyAcceptFirstDocblockAsBelongingWithFunction { + /** + * Superfluous docblock + */ + + + /** + * Function docblock + */ + function correctSpacing($x) {} + + + /** + * Superfluous docblock + */ + /** + * Function docblock + */ + function incorrectSpacing($x) {} + + +} diff --git a/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.1.inc.fixed b/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.1.inc.fixed index 289b2b6840..91444e7ca9 100644 --- a/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.1.inc.fixed +++ b/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.1.inc.fixed @@ -822,3 +822,29 @@ class SilenceBeforeErrorIfPreviousThingWasAFunctionBug }//end class + +// Issue #945. +class OnlyAcceptFirstDocblockAsBelongingWithFunction { + /** + * Superfluous docblock + */ + + + /** + * Function docblock + */ + function correctSpacing($x) {} + + + /** + * Superfluous docblock + */ + + + /** + * Function docblock + */ + function incorrectSpacing($x) {} + + +} diff --git a/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.php b/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.php index 7bc264a67b..a963a9a4b0 100644 --- a/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.php +++ b/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.php @@ -110,6 +110,7 @@ public function getErrorList($testFile='') 714 => 1, 717 => 1, 727 => 1, + 749 => 1, ]; case 'FunctionSpacingUnitTest.2.inc':