Skip to content

Commit 1792d99

Browse files
committed
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.
1 parent f281e46 commit 1792d99

File tree

4 files changed

+57
-3
lines changed

4 files changed

+57
-3
lines changed

src/Standards/Squiz/Sniffs/WhiteSpace/FunctionSpacingSniff.php

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,15 +127,18 @@ public function process(File $phpcsFile, $stackPtr)
127127
}
128128

129129
// Skip past function docblocks and attributes.
130-
$prev = $startOfDeclarationLine;
130+
// Only the first docblock is a function docblock. Other docblocks should be disregarded.
131+
$prev = $startOfDeclarationLine;
132+
$seenDocblock = false;
131133
if ($startOfDeclarationLine > 0) {
132134
for ($prev = ($startOfDeclarationLine - 1); $prev > 0; $prev--) {
133135
if ($tokens[$prev]['code'] === T_WHITESPACE) {
134136
continue;
135137
}
136138

137-
if ($tokens[$prev]['code'] === T_DOC_COMMENT_CLOSE_TAG) {
138-
$prev = $tokens[$prev]['comment_opener'];
139+
if ($seenDocblock === false && $tokens[$prev]['code'] === T_DOC_COMMENT_CLOSE_TAG) {
140+
$prev = $tokens[$prev]['comment_opener'];
141+
$seenDocblock = true;
139142
continue;
140143
}
141144

src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.1.inc

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -726,3 +726,27 @@ class SilenceBeforeErrorIfPreviousThingWasAFunctionBug
726726

727727
}//end blankLineDetectionB()
728728
}//end class
729+
730+
// Issue #945.
731+
class OnlyAcceptFirstDocblockAsBelongingWithFunction {
732+
/**
733+
* Superfluous docblock
734+
*/
735+
736+
737+
/**
738+
* Function docblock
739+
*/
740+
function correctSpacing($x) {}
741+
742+
743+
/**
744+
* Superfluous docblock
745+
*/
746+
/**
747+
* Function docblock
748+
*/
749+
function incorrectSpacing($x) {}
750+
751+
752+
}

src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.1.inc.fixed

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -822,3 +822,29 @@ class SilenceBeforeErrorIfPreviousThingWasAFunctionBug
822822

823823

824824
}//end class
825+
826+
// Issue #945.
827+
class OnlyAcceptFirstDocblockAsBelongingWithFunction {
828+
/**
829+
* Superfluous docblock
830+
*/
831+
832+
833+
/**
834+
* Function docblock
835+
*/
836+
function correctSpacing($x) {}
837+
838+
839+
/**
840+
* Superfluous docblock
841+
*/
842+
843+
844+
/**
845+
* Function docblock
846+
*/
847+
function incorrectSpacing($x) {}
848+
849+
850+
}

src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ public function getErrorList($testFile='')
110110
714 => 1,
111111
717 => 1,
112112
727 => 1,
113+
749 => 1,
113114
];
114115

115116
case 'FunctionSpacingUnitTest.2.inc':

0 commit comments

Comments
 (0)