Squiz.WhiteSpace.FunctionSpacing now applies beforeFirst and afterLast spacing rules to nested functions (ref #2406)

Author: gsherwood

package.xml

@@ -70,6 +70,9 @@ http://pear.php.net/dtd/package-2.0.xsd">
     -- The new error code is Squiz.PHP.DisallowMultipleAssignments.FoundInControlStructure
     -- All other multiple assignment cases use the existing error code Squiz.PHP.DisallowMultipleAssignments.Found
     -- Thanks to Juliette Reinders Folmer for the patch
+  - Squiz.WhiteSpace.FunctionSpacing now applies beforeFirst and afterLast spacing rules to nested functions
+    -- Previously, these rules only applied to the first and last function in a class, interface, or trait
+    -- These rules now apply to functions nested in any statement block, including other functions and conditions
   - Fixed bug #2391 : Sniff-specific ignore rules inside rulesets are filtering out too many files
     -- Thanks to Juliette Reinders Folmer and Willington Vega for the patch
   - Fixed bug #2478 : FunctionCommentThrowTag.WrongNumber when exception is thrown once but built conditionally

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

@@ -115,18 +115,12 @@ public function process(File $phpcsFile, $stackPtr)
         $ignore = (Tokens::$emptyTokens + Tokens::$methodPrefixes);
 
         $prev = $phpcsFile->findPrevious($ignore, ($stackPtr - 1), null, true);
-        if (isset($tokens[$prev]['scope_opener']) === true
-            && $tokens[$prev]['scope_opener'] === $prev
-            && isset(Tokens::$ooScopeTokens[$tokens[$tokens[$prev]['scope_condition']]['code']]) === true
-        ) {
+        if ($tokens[$prev]['code'] === T_OPEN_CURLY_BRACKET) {
             $isFirst = true;
         }
 
         $next = $phpcsFile->findNext($ignore, ($closer + 1), null, true);
-        if (isset($tokens[$next]['scope_closer']) === true
-            && $tokens[$next]['scope_closer'] === $next
-            && isset(Tokens::$ooScopeTokens[$tokens[$tokens[$next]['scope_condition']]['code']]) === true
-        ) {
+        if ($tokens[$next]['code'] === T_CLOSE_CURLY_BRACKET) {
             $isLast = true;
         }
 
@@ -202,12 +196,12 @@ public function process(File $phpcsFile, $stackPtr)
 
         $prevLineToken = null;
         for ($i = $stackPtr; $i >= 0; $i--) {
-            if (strpos($tokens[$i]['content'], $phpcsFile->eolChar) === false) {
+            if ($tokens[$i]['line'] === $tokens[$stackPtr]['line']) {
                 continue;
-            } else {
-                $prevLineToken = $i;
-                break;
             }
+
+            $prevLineToken = $i;
+            break;
         }
 
         if ($prevLineToken === null) {
@@ -220,6 +214,7 @@ public function process(File $phpcsFile, $stackPtr)
             $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
             ) {
@@ -243,18 +238,27 @@ public function process(File $phpcsFile, $stackPtr)
             $prevLine   = ($tokens[$prevContent]['line'] - 1);
             $i          = ($stackPtr - 1);
             $foundLines = 0;
-            while ($currentLine !== $prevLine && $currentLine > 1 && $i > 0) {
-                if (isset($tokens[$i]['scope_condition']) === true) {
-                    $scopeCondition = $tokens[$i]['scope_condition'];
-                    if ($tokens[$scopeCondition]['code'] === T_FUNCTION) {
-                        // Found a previous function.
-                        return;
-                    }
-                } else if ($tokens[$i]['code'] === T_FUNCTION) {
+
+            $stopAt = 0;
+            if (isset($tokens[$stackPtr]['conditions']) === true) {
+                $conditions = $tokens[$stackPtr]['conditions'];
+                $conditions = array_keys($conditions);
+                $stopAt     = array_pop($conditions);
+            }
+
+            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;

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

@@ -449,3 +449,34 @@ $anon_class = new class() {
     private function c(){}
 
 };
+
+// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacing 2
+// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingBeforeFirst 1
+// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingAfterLast 3
+
+class MyClass {
+    function a(){}
+    function b(){}
+    function c(){}
+}
+
+$bar = function () {
+    {
+        function forbiddenToo() {}
+    }
+};
+class Foo {
+    public function returnAnonymousClass() {
+        return new class() {
+            public function baz() {}
+        };
+    }
+
+    public function bar() {
+        function forbidden() {}
+    }
+}
+
+if (function_exists('foo') === false) {
+    function foo(){}
+}

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

@@ -500,3 +500,63 @@ $anon_class = new class() {
 
 
 };
+
+// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacing 2
+// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingBeforeFirst 1
+// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingAfterLast 3
+
+class MyClass {
+
+    function a(){}
+
+
+    function b(){}
+
+
+    function c(){}
+
+
+
+}
+
+$bar = function () {
+    {
+
+        function forbiddenToo() {}
+
+
+
+    }
+};
+class Foo {
+
+    public function returnAnonymousClass() {
+        return new class() {
+
+            public function baz() {}
+
+
+
+        };
+    }
+
+
+    public function bar() {
+
+        function forbidden() {}
+
+
+
+    }
+
+
+
+}
+
+if (function_exists('foo') === false) {
+
+    function foo(){}
+
+
+
+}

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

@@ -75,6 +75,16 @@ public function getErrorList($testFile='')
                 442 => 2,
                 444 => 1,
                 449 => 1,
+                458 => 2,
+                459 => 1,
+                460 => 1,
+                465 => 2,
+                469 => 1,
+                471 => 2,
+                473 => 1,
+                476 => 2,
+                477 => 1,
+                481 => 2,
             ];
 
         case 'FunctionSpacingUnitTest.2.inc':