Fixed bug #782 : Conditional function declarations cause fixing conflicts in Squiz standard

Author: gsherwood

CodeSniffer/Standards/Squiz/Sniffs/ControlStructures/ControlSignatureSniff.php

@@ -172,18 +172,12 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr)
                 break;
             }//end for
 
-            $found = ($tokens[$next]['line'] - $tokens[$opener]['line']);
-            if ($found !== 1) {
-                $error = 'Expected 1 newline after opening brace; %s found';
-                $data  = array($found);
-                $fix   = $phpcsFile->addFixableError($error, $opener, 'NewlineAfterOpenBrace', $data);
+            if ($tokens[$next]['line'] === $tokens[$opener]['line']) {
+                $error = 'Newline required after opening brace';
+                $fix   = $phpcsFile->addFixableError($error, $opener, 'NewlineAfterOpenBrace');
                 if ($fix === true) {
                     $phpcsFile->fixer->beginChangeset();
                     for ($i = ($opener + 1); $i < $next; $i++) {
-                        if ($found > 0 && $tokens[$i]['line'] === $tokens[$next]['line']) {
-                            break;
-                        }
-
                         if (trim($tokens[$i]['content']) !== '') {
                             break;
                         }

CodeSniffer/Standards/Squiz/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php

@@ -134,7 +134,18 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr)
             }
         }
 
-        if ($tokens[$firstContent]['line'] >= ($tokens[$scopeOpener]['line'] + 2)) {
+        // We ignore spacing for some structures that tend to have their own rules.
+        $ignore = array(
+                   T_FUNCTION             => true,
+                   T_CLASS                => true,
+                   T_INTERFACE            => true,
+                   T_TRAIT                => true,
+                   T_DOC_COMMENT_OPEN_TAG => true,
+                  );
+
+        if (isset($ignore[$tokens[$firstContent]['code']]) === false
+            && $tokens[$firstContent]['line'] >= ($tokens[$scopeOpener]['line'] + 2)
+        ) {
             $error = 'Blank line found at start of control structure';
             $fix   = $phpcsFile->addFixableError($error, $scopeOpener, 'SpacingAfterOpen');
 
@@ -159,7 +170,21 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr)
                 true
             );
 
-            if ($tokens[$lastContent]['line'] <= ($tokens[$scopeCloser]['line'] - 2)) {
+            $lastNonEmptyContent = $phpcsFile->findPrevious(
+                PHP_CodeSniffer_Tokens::$emptyTokens,
+                ($scopeCloser - 1),
+                null,
+                true
+            );
+
+            $checkToken = $lastContent;
+            if (isset($tokens[$lastNonEmptyContent]['scope_condition']) === true) {
+                $checkToken = $tokens[$lastNonEmptyContent]['scope_condition'];
+            }
+
+            if (isset($ignore[$tokens[$checkToken]['code']]) === false
+                && $tokens[$lastContent]['line'] <= ($tokens[$scopeCloser]['line'] - 2)
+            ) {
                 $errorToken = $scopeCloser;
                 for ($i = ($scopeCloser - 1); $i > $lastContent; $i--) {
                     if ($tokens[$i]['line'] < $tokens[$scopeCloser]['line']) {

CodeSniffer/Standards/Squiz/Tests/ControlStructures/ControlSignatureUnitTest.inc.fixed

@@ -120,10 +120,12 @@ echo $i;
 } while ($i > 0);
 
 if ($i === 0) {
+
     $i = 1
 }
 
 if ($a) {
+
 } elseif ($b) {
 }
 
@@ -157,6 +159,7 @@ try {
 }
 
 switch ($foo) {
+
     case 'bar':
     break;
 

CodeSniffer/Standards/Squiz/Tests/ControlStructures/ControlSignatureUnitTest.js.fixed

@@ -120,6 +120,7 @@ i = 0;
 } while (i > 0);
 
 if (i === 0) {
+
     i = 1
 }
 

CodeSniffer/Standards/Squiz/Tests/ControlStructures/ControlSignatureUnitTest.php

@@ -65,17 +65,15 @@ public function getErrorList($testFile='ControlSignatureUnitTest.inc')
                    99  => 1,
                    108 => 1,
                    112 => 1,
-                   115 => 1,
                   );
 
         if ($testFile === 'ControlSignatureUnitTest.inc') {
-            $errors[120] = 1;
             $errors[122] = 1;
             $errors[130] = 2;
             $errors[134] = 1;
             $errors[150] = 1;
             $errors[153] = 1;
-            $errors[158] = 2;
+            $errors[158] = 1;
             $errors[165] = 1;
             $errors[170] = 2;
             $errors[186] = 2;

CodeSniffer/Standards/Squiz/Tests/WhiteSpace/ControlStructureSpacingUnitTest.inc

@@ -197,3 +197,24 @@ try {
 catch (Exception $e) {
     // Something
 }
+
+if ($foo) {
+
+
+    /**
+     * Comment
+     */
+    function foo() {
+        // Code here
+    }
+
+
+    /**
+     * Comment
+     */
+    class bar() {
+
+    }//end class
+
+
+}

CodeSniffer/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.inc

@@ -280,3 +280,19 @@ class MyClass
 function test() {
 
 }//end test()
+
+
+if ($foo) {
+    /**
+     * Comment
+     */
+    function foo() {
+        // Code here
+    }
+    /**
+     * Comment
+     */
+    function bar() {
+        // Code here
+    }
+}

CodeSniffer/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.inc.fixed

@@ -302,3 +302,25 @@ class MyClass
 function test() {
 
 }//end test()
+
+
+if ($foo) {
+
+
+    /**
+     * Comment
+     */
+    function foo() {
+        // Code here
+    }
+
+
+    /**
+     * Comment
+     */
+    function bar() {
+        // Code here
+    }
+
+
+}

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

@@ -65,6 +65,9 @@ public function getErrorList()
                 252 => 1,
                 275 => 1,
                 276 => 1,
+                289 => 1,
+                291 => 1,
+                297 => 1,
                );
 
     }//end getErrorList()

package.xml

@@ -73,6 +73,9 @@ http://pear.php.net/dtd/package-2.0.xsd">
   - Fixed bug #769 : Incorrect detection of variable reference operator when used with short array syntax
     -- Thanks to Klaus Purer for the patch
   - Fixed bug #772 : Syntax error when using PHPCBF on alternative style foreach loops
+  - Fixed bug #782 : Conditional function declarations cause fixing conflicts in Squiz standard
+    -- Squiz.ControlStructures.ControlSignature no longer enforces a single newline after open brace
+    -- Squiz.WhiteSpace.ControlStructureSpacing can be used to checl spacing at the start/end of control structures
  </notes>
  <contents>
   <dir name="/">