Squiz/FunctionSpacing: bug fix for attribute handling

The `Squiz.WhiteSpace.FunctionSpacing` sniff is supposed to check the number of blank lines before (and after) a function declaration, while taking docblocks and attributes attached to the function into account. I.e. if there is a docblock and/or attributes, the blank lines _above_ those will be checked.

Additionally, the sniff has a separate error code `BeforeFirst` for the number of blank lines before a first function in a class and the expected number of blank lines for the first function in a class may also be configured to be different than the "normal" number of blank lines before a function/between methods.

Now, while the sniff does take attributes into account, it does so incorrectly:
1. It doesn't allow for attributes _before_ a docblock, even though this is perfectly valid PHP and the attributes still belong with the function.
2. It doesn't allow for multiple attributes on the same line. Again, this is perfectly valid PHP.

The effect of this bug can be seen by checking the new tests without the fix:
1. The "Correct" functions will also be flagged as incorrect as the sniff gets confused by multiple attributes on the same line.
2. The set of tests with the attributes before the docblock will be flagged with the `Before` error code instead of `BeforeFirst` as the functions will not be recognized as the first in class due to the attributes before the docblock.
3. The fixer will make incorrect fixes.

Fixed now by removing the presumption about the _order_ of docblocks vs attributes and stabilizing the "current line" determination.

Includes tests.

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

@@ -116,14 +116,23 @@ public function process(File $phpcsFile, $stackPtr)
 
         $prev = $phpcsFile->findPrevious($ignore, ($stackPtr - 1), null, true);
 
-        while ($tokens[$prev]['code'] === T_ATTRIBUTE_END) {
-            // Skip past function attributes.
-            $prev = $phpcsFile->findPrevious($ignore, ($tokens[$prev]['attribute_opener'] - 1), null, true);
-        }
+        // Skip past function docblocks and attributes.
+        for ($prev; $prev > 0; $prev--) {
+            if ($tokens[$prev]['code'] === T_WHITESPACE) {
+                continue;
+            }
+
+            if ($tokens[$prev]['code'] === T_DOC_COMMENT_CLOSE_TAG) {
+                $prev = $tokens[$prev]['comment_opener'];
+                continue;
+            }
 
-        if ($tokens[$prev]['code'] === T_DOC_COMMENT_CLOSE_TAG) {
-            // Skip past function docblocks.
-            $prev = $phpcsFile->findPrevious($ignore, ($tokens[$prev]['comment_opener'] - 1), null, true);
+            if ($tokens[$prev]['code'] === T_ATTRIBUTE_END) {
+                $prev = $tokens[$prev]['attribute_opener'];
+                continue;
+            }
+
+            break;
         }
 
         if ($tokens[$prev]['code'] === T_OPEN_CURLY_BRACKET) {
@@ -253,20 +262,30 @@ public function process(File $phpcsFile, $stackPtr)
                 return;
             }
 
-            while ($tokens[$prevContent]['code'] === T_ATTRIBUTE_END
-                && $tokens[$prevContent]['line'] === ($currentLine - 1)
-            ) {
-                // Account for function attributes.
-                $currentLine = $tokens[$tokens[$prevContent]['attribute_opener']]['line'];
-                $prevContent = $phpcsFile->findPrevious(T_WHITESPACE, ($tokens[$prevContent]['attribute_opener'] - 1), null, true);
-            }
+            // 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)
-            ) {
-                // Account for function comments.
-                $prevContent = $phpcsFile->findPrevious(T_WHITESPACE, ($tokens[$prevContent]['comment_opener'] - 1), null, true);
-            }
+                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

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

@@ -593,3 +593,102 @@ function a() {
  */
 function b() {
 }
+
+
+// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingBeforeFirst 1
+// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingAfterLast 0
+
+class DocblockFollowedByAttributesCorrectSpacing {
+
+    /**
+     * No error.
+     */
+    #[AttributesShouldBeJumpedOver]
+    #[
+        ASecondAttributeShouldBeJumpedOverToo
+    ]#[AndAThirdAsWell]
+    function FirstFunction()
+    {
+        // Code
+    }
+}
+
+class DocblockFollowedByAttributesTooMuchSpacing {
+
+
+
+    /**
+     * Docblock.
+     */
+    #[AttributesShouldBeJumpedOver]
+    #[
+        ASecondAttributeShouldBeJumpedOverToo
+    ]#[AndAThirdAsWell]
+    function FirstFunction()
+    {
+        // Code
+    }
+}
+
+class DocblockFollowedByAttributesTooLittleSpacing {
+    /**
+     * Docblock.
+     */
+    #[AttributesShouldBeJumpedOver]
+    #[
+        ASecondAttributeShouldBeJumpedOverToo
+    ]#[AndAThirdAsWell]
+    function FirstFunction()
+    {
+        // Code
+    }
+}
+
+class DocblockPrecededByAttributesCorrectSpacing {
+
+    #[AttributesShouldBeJumpedOver]
+    #[
+        ASecondAttributeShouldBeJumpedOverToo
+    ]#[AndAThirdAsWell]
+    /**
+     * No error.
+     */
+    function FirstFunction()
+    {
+        // Code
+    }
+}
+
+class DocblockPrecededByAttributesTooMuchSpacing {
+
+
+    #[AttributesShouldBeJumpedOver]
+    #[
+        ASecondAttributeShouldBeJumpedOverToo
+    ]#[AndAThirdAsWell]
+    /**
+     * Docblock.
+     */
+    function FirstFunction()
+    {
+        // Code
+    }
+}
+
+class DocblockPrecededByAttributesTooLittleSpacing {
+    #[AttributesShouldBeJumpedOver]
+    #[
+        ASecondAttributeShouldBeJumpedOverToo
+    ]#[AndAThirdAsWell]
+    /**
+     * Docblock.
+     */
+    function FirstFunction()
+    {
+        // Code
+    }
+}
+
+// Reset properties to their default value.
+// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingBeforeFirst 2
+// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingAfterLast 2

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

@@ -685,3 +685,101 @@ function a() {
  */
 function b() {
 }
+
+
+// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingBeforeFirst 1
+// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingAfterLast 0
+
+class DocblockFollowedByAttributesCorrectSpacing {
+
+    /**
+     * No error.
+     */
+    #[AttributesShouldBeJumpedOver]
+    #[
+        ASecondAttributeShouldBeJumpedOverToo
+    ]#[AndAThirdAsWell]
+    function FirstFunction()
+    {
+        // Code
+    }
+}
+
+class DocblockFollowedByAttributesTooMuchSpacing {
+
+    /**
+     * Docblock.
+     */
+    #[AttributesShouldBeJumpedOver]
+    #[
+        ASecondAttributeShouldBeJumpedOverToo
+    ]#[AndAThirdAsWell]
+    function FirstFunction()
+    {
+        // Code
+    }
+}
+
+class DocblockFollowedByAttributesTooLittleSpacing {
+
+    /**
+     * Docblock.
+     */
+    #[AttributesShouldBeJumpedOver]
+    #[
+        ASecondAttributeShouldBeJumpedOverToo
+    ]#[AndAThirdAsWell]
+    function FirstFunction()
+    {
+        // Code
+    }
+}
+
+class DocblockPrecededByAttributesCorrectSpacing {
+
+    #[AttributesShouldBeJumpedOver]
+    #[
+        ASecondAttributeShouldBeJumpedOverToo
+    ]#[AndAThirdAsWell]
+    /**
+     * No error.
+     */
+    function FirstFunction()
+    {
+        // Code
+    }
+}
+
+class DocblockPrecededByAttributesTooMuchSpacing {
+
+    #[AttributesShouldBeJumpedOver]
+    #[
+        ASecondAttributeShouldBeJumpedOverToo
+    ]#[AndAThirdAsWell]
+    /**
+     * Docblock.
+     */
+    function FirstFunction()
+    {
+        // Code
+    }
+}
+
+class DocblockPrecededByAttributesTooLittleSpacing {
+
+    #[AttributesShouldBeJumpedOver]
+    #[
+        ASecondAttributeShouldBeJumpedOverToo
+    ]#[AndAThirdAsWell]
+    /**
+     * Docblock.
+     */
+    function FirstFunction()
+    {
+        // Code
+    }
+}
+
+// Reset properties to their default value.
+// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingBeforeFirst 2
+// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingAfterLast 2

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

@@ -103,6 +103,10 @@ public function getErrorList($testFile='')
                 580 => 2,
                 583 => 3,
                 591 => 1,
+                627 => 1,
+                641 => 1,
+                672 => 1,
+                686 => 1,
             ];
 
         case 'FunctionSpacingUnitTest.2.inc':