Squiz/FunctionSpacing: iterate on previous fix for attributes

Turns out the "silence the "before" error if the thing before was another function" functionality still wasn't working correctly.

And if there is a blank line between the first thing before the first "preamble" for the function and the declaration itself, the sniff would also disregard the "preamble" and insert the blank lines straight above the function declaration.

Fixed now.

Includes tests proving the bug.

Includes one minor change in the existing tests, where the recognition of "the thing before is also a function" no longer works.
This is a specific edge case when there are two functions declared on the same line and, in my opinion, this tiny regression is a valid trade-off for correctly handling functions with attributes, which is much more commonly found in code.

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

@@ -116,23 +116,36 @@ public function process(File $phpcsFile, $stackPtr)
 
         $prev = $phpcsFile->findPrevious($ignore, ($stackPtr - 1), null, true);
 
-        // Skip past function docblocks and attributes.
-        for ($prev; $prev > 0; $prev--) {
-            if ($tokens[$prev]['code'] === T_WHITESPACE) {
+        $startOfDeclarationLine = $phpcsFile->findNext(T_WHITESPACE, ($prev + 1), null, true);
+        for ($i = $startOfDeclarationLine; $i >= 0; $i--) {
+            if ($tokens[$i]['line'] === $tokens[$startOfDeclarationLine]['line']) {
+                $startOfDeclarationLine = $i;
                 continue;
             }
 
-            if ($tokens[$prev]['code'] === T_DOC_COMMENT_CLOSE_TAG) {
-                $prev = $tokens[$prev]['comment_opener'];
-                continue;
-            }
+            break;
+        }
 
-            if ($tokens[$prev]['code'] === T_ATTRIBUTE_END) {
-                $prev = $tokens[$prev]['attribute_opener'];
-                continue;
-            }
+        // Skip past function docblocks and attributes.
+        $prev = $startOfDeclarationLine;
+        if ($startOfDeclarationLine > 0) {
+            for ($prev = ($startOfDeclarationLine - 1); $prev > 0; $prev--) {
+                if ($tokens[$prev]['code'] === T_WHITESPACE) {
+                    continue;
+                }
 
-            break;
+                if ($tokens[$prev]['code'] === T_DOC_COMMENT_CLOSE_TAG) {
+                    $prev = $tokens[$prev]['comment_opener'];
+                    continue;
+                }
+
+                if ($tokens[$prev]['code'] === T_ATTRIBUTE_END) {
+                    $prev = $tokens[$prev]['attribute_opener'];
+                    continue;
+                }
+
+                break;
+            }
         }
 
         if ($tokens[$prev]['code'] === T_OPEN_CURLY_BRACKET) {
@@ -233,9 +246,11 @@ public function process(File $phpcsFile, $stackPtr)
             before the function.
         */
 
+        $startOfPreamble = $phpcsFile->findNext(T_WHITESPACE, ($prev + 1), null, true);
+
         $prevLineToken = null;
-        for ($i = $stackPtr; $i >= 0; $i--) {
-            if ($tokens[$i]['line'] === $tokens[$stackPtr]['line']) {
+        for ($i = $startOfPreamble; $i >= 0; $i--) {
+            if ($tokens[$i]['line'] === $tokens[$startOfPreamble]['line']) {
                 continue;
             }
 
@@ -250,43 +265,15 @@ public function process(File $phpcsFile, $stackPtr)
             $prevContent   = 0;
             $prevLineToken = 0;
         } else {
-            $currentLine = $tokens[$stackPtr]['line'];
-
-            $prevContent = $phpcsFile->findPrevious(T_WHITESPACE, $prevLineToken, null, true);
-
-            if ($tokens[$prevContent]['code'] === T_COMMENT
-                || isset(Tokens::$phpcsCommentTokens[$tokens[$prevContent]['code']]) === true
+            $firstBefore = $phpcsFile->findPrevious(T_WHITESPACE, ($startOfDeclarationLine - 1), null, true);
+            if ($tokens[$firstBefore]['code'] === T_COMMENT
+                || isset(Tokens::$phpcsCommentTokens[$tokens[$firstBefore]['code']]) === true
             ) {
                 // Ignore comments as they can have different spacing rules, and this
                 // isn't a proper function comment anyway.
                 return;
             }
 
-            // Skip past function docblocks and attributes.
-            for ($prevContent; $prevContent > 0; $prevContent--) {
-                if ($tokens[$prevContent]['code'] === T_WHITESPACE) {
-                    continue;
-                }
-
-                if ($tokens[$prevContent]['code'] === T_DOC_COMMENT_CLOSE_TAG
-                    && $tokens[$prevContent]['line'] >= ($currentLine - 1)
-                ) {
-                    $currentLine = $tokens[$tokens[$prevContent]['comment_opener']]['line'];
-                    $prevContent = $tokens[$prevContent]['comment_opener'];
-                    continue;
-                }
-
-                if ($tokens[$prevContent]['code'] === T_ATTRIBUTE_END
-                    && $tokens[$prevContent]['line'] >= ($currentLine - 1)
-                ) {
-                    $currentLine = $tokens[$tokens[$prevContent]['attribute_opener']]['line'];
-                    $prevContent = $tokens[$prevContent]['attribute_opener'];
-                    continue;
-                }
-
-                break;
-            }//end for
-
             // Before we throw an error, check that we are not throwing an error
             // for another function. We don't want to error for no blank lines after
             // the previous function and no blank lines before this one as well.
@@ -297,38 +284,33 @@ public function process(File $phpcsFile, $stackPtr)
                 $stopAt     = array_pop($conditions);
             }
 
-            $prevLineToken = $prevContent;
-            $prevLine      = ($tokens[$prevContent]['line'] - 1);
-            $i          = ($stackPtr - 1);
-            $foundLines = 0;
+            $currentLine = $tokens[$startOfPreamble]['line'];
+            $prevContent = $prev;
+            $prevLine    = ($tokens[$prevContent]['line'] - 1);
+            $foundLines  = ($currentLine - $tokens[$prevContent]['line'] - 1);
+
+            for ($i = $prevContent; $i > $stopAt; $i--) {
+                if ($tokens[$i]['code'] === T_CLOSE_CURLY_BRACKET) {
+                    if (isset($tokens[$i]['scope_condition']) === true
+                        && $tokens[$tokens[$i]['scope_condition']]['code'] === T_FUNCTION
+                    ) {
+                        // Found a previous function.
+                        return;
+                    } else {
+                        break;
+                    }
+                }
 
-            while ($currentLine !== $prevLine && $currentLine > 1 && $i > $stopAt) {
                 if ($tokens[$i]['code'] === T_FUNCTION) {
                     // Found another interface or abstract function.
                     return;
                 }
 
-                if ($tokens[$i]['code'] === T_CLOSE_CURLY_BRACKET
-                    && $tokens[$tokens[$i]['scope_condition']]['code'] === T_FUNCTION
-                ) {
-                    // Found a previous function.
-                    return;
-                }
-
                 $currentLine = $tokens[$i]['line'];
                 if ($currentLine === $prevLine) {
                     break;
                 }
-
-                if ($tokens[($i - 1)]['line'] < $currentLine && $tokens[($i + 1)]['line'] > $currentLine) {
-                    // This token is on a line by itself. If it is whitespace, the line is empty.
-                    if ($tokens[$i]['code'] === T_WHITESPACE) {
-                        $foundLines++;
-                    }
-                }
-
-                $i--;
-            }//end while
+            }//end for
         }//end if
 
         $requiredSpacing = $this->spacing;

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

@@ -692,3 +692,37 @@ class DocblockPrecededByAttributesTooLittleSpacing {
 // Reset properties to their default value.
 // phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingBeforeFirst 2
 // phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingAfterLast 2
+
+class SilenceBeforeErrorIfPreviousThingWasAFunctionBug
+{
+    /**
+     * Docblock.
+     */
+
+    #[ReturnTypeWillChange]
+
+
+
+
+
+    #[
+
+        AnotherAttribute
+
+    ]#[AndAThirdAsWell]
+
+    public function blankLineDetectionA()
+    {
+
+    }//end blankLineDetectionA()
+
+    /**
+     * Docblock.
+     */
+    #[ReturnTypeWillChange]
+
+    public function blankLineDetectionB()
+    {
+
+    }//end blankLineDetectionB()
+}//end class

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

@@ -783,3 +783,42 @@ class DocblockPrecededByAttributesTooLittleSpacing {
 // Reset properties to their default value.
 // phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingBeforeFirst 2
 // phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingAfterLast 2
+
+class SilenceBeforeErrorIfPreviousThingWasAFunctionBug
+{
+
+
+    /**
+     * Docblock.
+     */
+
+    #[ReturnTypeWillChange]
+
+
+
+
+
+    #[
+
+        AnotherAttribute
+
+    ]#[AndAThirdAsWell]
+
+    public function blankLineDetectionA()
+    {
+
+    }//end blankLineDetectionA()
+
+
+    /**
+     * Docblock.
+     */
+    #[ReturnTypeWillChange]
+
+    public function blankLineDetectionB()
+    {
+
+    }//end blankLineDetectionB()
+
+
+}//end class

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

@@ -101,12 +101,15 @@ public function getErrorList($testFile='')
                 560 => 1,
                 566 => 1,
                 580 => 2,
-                583 => 3,
+                583 => 4,
                 591 => 1,
                 627 => 1,
                 641 => 1,
                 672 => 1,
                 686 => 1,
+                714 => 1,
+                717 => 1,
+                727 => 1,
             ];
 
         case 'FunctionSpacingUnitTest.2.inc':