Skip to content

Commit bfdc6ea

Browse files
jrfnljrfnl
authored
Merge pull request #826 from PHPCSStandards/feature/squiz-functionspacing-improve-attribute-handling
Squiz/FunctionSpacing: bug fix for attribute handling
2 parents dc3a639 + 04bf4c7 commit bfdc6ea

File tree

4 files changed

+330
-52
lines changed

4 files changed

+330
-52
lines changed

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

Lines changed: 52 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -116,14 +116,36 @@ public function process(File $phpcsFile, $stackPtr)
116116

117117
$prev = $phpcsFile->findPrevious($ignore, ($stackPtr - 1), null, true);
118118

119-
while ($tokens[$prev]['code'] === T_ATTRIBUTE_END) {
120-
// Skip past function attributes.
121-
$prev = $phpcsFile->findPrevious($ignore, ($tokens[$prev]['attribute_opener'] - 1), null, true);
119+
$startOfDeclarationLine = $phpcsFile->findNext(T_WHITESPACE, ($prev + 1), null, true);
120+
for ($i = $startOfDeclarationLine; $i >= 0; $i--) {
121+
if ($tokens[$i]['line'] === $tokens[$startOfDeclarationLine]['line']) {
122+
$startOfDeclarationLine = $i;
123+
continue;
124+
}
125+
126+
break;
122127
}
123128

124-
if ($tokens[$prev]['code'] === T_DOC_COMMENT_CLOSE_TAG) {
125-
// Skip past function docblocks.
126-
$prev = $phpcsFile->findPrevious($ignore, ($tokens[$prev]['comment_opener'] - 1), null, true);
129+
// Skip past function docblocks and attributes.
130+
$prev = $startOfDeclarationLine;
131+
if ($startOfDeclarationLine > 0) {
132+
for ($prev = ($startOfDeclarationLine - 1); $prev > 0; $prev--) {
133+
if ($tokens[$prev]['code'] === T_WHITESPACE) {
134+
continue;
135+
}
136+
137+
if ($tokens[$prev]['code'] === T_DOC_COMMENT_CLOSE_TAG) {
138+
$prev = $tokens[$prev]['comment_opener'];
139+
continue;
140+
}
141+
142+
if ($tokens[$prev]['code'] === T_ATTRIBUTE_END) {
143+
$prev = $tokens[$prev]['attribute_opener'];
144+
continue;
145+
}
146+
147+
break;
148+
}
127149
}
128150

129151
if ($tokens[$prev]['code'] === T_OPEN_CURLY_BRACKET) {
@@ -224,9 +246,11 @@ public function process(File $phpcsFile, $stackPtr)
224246
before the function.
225247
*/
226248

249+
$startOfPreamble = $phpcsFile->findNext(T_WHITESPACE, ($prev + 1), null, true);
250+
227251
$prevLineToken = null;
228-
for ($i = $stackPtr; $i >= 0; $i--) {
229-
if ($tokens[$i]['line'] === $tokens[$stackPtr]['line']) {
252+
for ($i = $startOfPreamble; $i >= 0; $i--) {
253+
if ($tokens[$i]['line'] === $tokens[$startOfPreamble]['line']) {
230254
continue;
231255
}
232256

@@ -241,33 +265,15 @@ public function process(File $phpcsFile, $stackPtr)
241265
$prevContent = 0;
242266
$prevLineToken = 0;
243267
} else {
244-
$currentLine = $tokens[$stackPtr]['line'];
245-
246-
$prevContent = $phpcsFile->findPrevious(T_WHITESPACE, $prevLineToken, null, true);
247-
248-
if ($tokens[$prevContent]['code'] === T_COMMENT
249-
|| isset(Tokens::$phpcsCommentTokens[$tokens[$prevContent]['code']]) === true
268+
$firstBefore = $phpcsFile->findPrevious(T_WHITESPACE, ($startOfDeclarationLine - 1), null, true);
269+
if ($tokens[$firstBefore]['code'] === T_COMMENT
270+
|| isset(Tokens::$phpcsCommentTokens[$tokens[$firstBefore]['code']]) === true
250271
) {
251272
// Ignore comments as they can have different spacing rules, and this
252273
// isn't a proper function comment anyway.
253274
return;
254275
}
255276

256-
while ($tokens[$prevContent]['code'] === T_ATTRIBUTE_END
257-
&& $tokens[$prevContent]['line'] === ($currentLine - 1)
258-
) {
259-
// Account for function attributes.
260-
$currentLine = $tokens[$tokens[$prevContent]['attribute_opener']]['line'];
261-
$prevContent = $phpcsFile->findPrevious(T_WHITESPACE, ($tokens[$prevContent]['attribute_opener'] - 1), null, true);
262-
}
263-
264-
if ($tokens[$prevContent]['code'] === T_DOC_COMMENT_CLOSE_TAG
265-
&& $tokens[$prevContent]['line'] === ($currentLine - 1)
266-
) {
267-
// Account for function comments.
268-
$prevContent = $phpcsFile->findPrevious(T_WHITESPACE, ($tokens[$prevContent]['comment_opener'] - 1), null, true);
269-
}
270-
271277
// Before we throw an error, check that we are not throwing an error
272278
// for another function. We don't want to error for no blank lines after
273279
// the previous function and no blank lines before this one as well.
@@ -278,38 +284,33 @@ public function process(File $phpcsFile, $stackPtr)
278284
$stopAt = array_pop($conditions);
279285
}
280286

281-
$prevLineToken = $prevContent;
282-
$prevLine = ($tokens[$prevContent]['line'] - 1);
283-
$i = ($stackPtr - 1);
284-
$foundLines = 0;
287+
$currentLine = $tokens[$startOfPreamble]['line'];
288+
$prevContent = $prev;
289+
$prevLine = ($tokens[$prevContent]['line'] - 1);
290+
$foundLines = ($currentLine - $tokens[$prevContent]['line'] - 1);
291+
292+
for ($i = $prevContent; $i > $stopAt; $i--) {
293+
if ($tokens[$i]['code'] === T_CLOSE_CURLY_BRACKET) {
294+
if (isset($tokens[$i]['scope_condition']) === true
295+
&& $tokens[$tokens[$i]['scope_condition']]['code'] === T_FUNCTION
296+
) {
297+
// Found a previous function.
298+
return;
299+
} else {
300+
break;
301+
}
302+
}
285303

286-
while ($currentLine !== $prevLine && $currentLine > 1 && $i > $stopAt) {
287304
if ($tokens[$i]['code'] === T_FUNCTION) {
288305
// Found another interface or abstract function.
289306
return;
290307
}
291308

292-
if ($tokens[$i]['code'] === T_CLOSE_CURLY_BRACKET
293-
&& $tokens[$tokens[$i]['scope_condition']]['code'] === T_FUNCTION
294-
) {
295-
// Found a previous function.
296-
return;
297-
}
298-
299309
$currentLine = $tokens[$i]['line'];
300310
if ($currentLine === $prevLine) {
301311
break;
302312
}
303-
304-
if ($tokens[($i - 1)]['line'] < $currentLine && $tokens[($i + 1)]['line'] > $currentLine) {
305-
// This token is on a line by itself. If it is whitespace, the line is empty.
306-
if ($tokens[$i]['code'] === T_WHITESPACE) {
307-
$foundLines++;
308-
}
309-
}
310-
311-
$i--;
312-
}//end while
313+
}//end for
313314
}//end if
314315

315316
$requiredSpacing = $this->spacing;

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

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,3 +593,136 @@ function a() {
593593
*/
594594
function b() {
595595
}
596+
597+
598+
// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingBeforeFirst 1
599+
// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingAfterLast 0
600+
601+
class DocblockFollowedByAttributesCorrectSpacing {
602+
603+
/**
604+
* No error.
605+
*/
606+
#[AttributesShouldBeJumpedOver]
607+
#[
608+
ASecondAttributeShouldBeJumpedOverToo
609+
]#[AndAThirdAsWell]
610+
function FirstFunction()
611+
{
612+
// Code
613+
}
614+
}
615+
616+
class DocblockFollowedByAttributesTooMuchSpacing {
617+
618+
619+
620+
/**
621+
* Docblock.
622+
*/
623+
#[AttributesShouldBeJumpedOver]
624+
#[
625+
ASecondAttributeShouldBeJumpedOverToo
626+
]#[AndAThirdAsWell]
627+
function FirstFunction()
628+
{
629+
// Code
630+
}
631+
}
632+
633+
class DocblockFollowedByAttributesTooLittleSpacing {
634+
/**
635+
* Docblock.
636+
*/
637+
#[AttributesShouldBeJumpedOver]
638+
#[
639+
ASecondAttributeShouldBeJumpedOverToo
640+
]#[AndAThirdAsWell]
641+
function FirstFunction()
642+
{
643+
// Code
644+
}
645+
}
646+
647+
class DocblockPrecededByAttributesCorrectSpacing {
648+
649+
#[AttributesShouldBeJumpedOver]
650+
#[
651+
ASecondAttributeShouldBeJumpedOverToo
652+
]#[AndAThirdAsWell]
653+
/**
654+
* No error.
655+
*/
656+
function FirstFunction()
657+
{
658+
// Code
659+
}
660+
}
661+
662+
class DocblockPrecededByAttributesTooMuchSpacing {
663+
664+
665+
#[AttributesShouldBeJumpedOver]
666+
#[
667+
ASecondAttributeShouldBeJumpedOverToo
668+
]#[AndAThirdAsWell]
669+
/**
670+
* Docblock.
671+
*/
672+
function FirstFunction()
673+
{
674+
// Code
675+
}
676+
}
677+
678+
class DocblockPrecededByAttributesTooLittleSpacing {
679+
#[AttributesShouldBeJumpedOver]
680+
#[
681+
ASecondAttributeShouldBeJumpedOverToo
682+
]#[AndAThirdAsWell]
683+
/**
684+
* Docblock.
685+
*/
686+
function FirstFunction()
687+
{
688+
// Code
689+
}
690+
}
691+
692+
// Reset properties to their default value.
693+
// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingBeforeFirst 2
694+
// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingAfterLast 2
695+
696+
class SilenceBeforeErrorIfPreviousThingWasAFunctionBug
697+
{
698+
/**
699+
* Docblock.
700+
*/
701+
702+
#[ReturnTypeWillChange]
703+
704+
705+
706+
707+
708+
#[
709+
710+
AnotherAttribute
711+
712+
]#[AndAThirdAsWell]
713+
714+
public function blankLineDetectionA()
715+
{
716+
717+
}//end blankLineDetectionA()
718+
719+
/**
720+
* Docblock.
721+
*/
722+
#[ReturnTypeWillChange]
723+
724+
public function blankLineDetectionB()
725+
{
726+
727+
}//end blankLineDetectionB()
728+
}//end class

0 commit comments

Comments
 (0)