Fixed bug #781 : Incorrect checking for PHP7 return types on multi-li…

…ne function declartions
squizlabs · Nov 22, 2015 · c1b0593 · c1b0593
1 parent 578a084
commit c1b0593
Show file tree

Hide file tree

Showing 6 changed files with 72 additions and 39 deletions.
diff --git a/CodeSniffer/Standards/PEAR/Sniffs/Functions/FunctionDeclarationSniff.php b/CodeSniffer/Standards/PEAR/Sniffs/Functions/FunctionDeclarationSniff.php
@@ -364,54 +364,43 @@ public function processMultiLineDeclaration(PHP_CodeSniffer_File $phpcsFile, $st
             }
         }//end for
 
-        if (isset($tokens[$stackPtr]['scope_opener']) === true) {
-            // The opening brace needs to be one space away
-            // from the closing parenthesis.
-            $next = $tokens[($closeBracket + 1)];
-            if ($next['code'] !== T_WHITESPACE) {
+        if (isset($tokens[$stackPtr]['scope_opener']) === false) {
+            return;
+        }
+
+        // The opening brace needs to be one space away from the closing parenthesis.
+        $opener = $tokens[$stackPtr]['scope_opener'];
+        if ($tokens[$opener]['line'] !== $tokens[$closeBracket]['line']) {
+            $error = 'The closing parenthesis and the opening brace of a multi-line function declaration must be on the same line';
+            $fix   = $phpcsFile->addFixableError($error, $opener, 'NewlineBeforeOpenBrace');
+            if ($fix === true) {
+                $prev = $phpcsFile->findPrevious(PHP_CodeSniffer_Tokens::$emptyTokens, ($opener - 1), $closeBracket, true);
+                $phpcsFile->fixer->beginChangeset();
+                $phpcsFile->fixer->addContent($prev, ' {');
+                $phpcsFile->fixer->replaceToken($opener, '');
+                $phpcsFile->fixer->endChangeset();
+            }
+        } else {
+            $prev = $tokens[($opener - 1)];
+            if ($prev['code'] !== T_WHITESPACE) {
                 $length = 0;
-            } else if ($next['content'] === $phpcsFile->eolChar) {
-                $length = -1;
             } else {
-                $length = strlen($next['content']);
+                $length = strlen($prev['content']);
             }
 
             if ($length !== 1) {
-                $data = array($length);
-                $code = 'SpaceBeforeOpenBrace';
-
-                $error = 'There must be a single space between the closing parenthesis and the opening brace of a multi-line function declaration; found ';
-                if ($length === -1) {
-                    $error .= 'newline';
-                    $code   = 'NewlineBeforeOpenBrace';
-                } else {
-                    $error .= '%s spaces';
-                }
-
-                $fix = $phpcsFile->addFixableError($error, ($closeBracket + 1), $code, $data);
+                $error = 'There must be a single space between the closing parenthesis and the opening brace of a multi-line function declaration; found %s spaces';
+                $fix   = $phpcsFile->addFixableError($error, ($opener - 1), 'SpaceBeforeOpenBrace', array($length));
                 if ($fix === true) {
                     if ($length === 0) {
-                        $phpcsFile->fixer->addContent($closeBracket, ' ');
+                        $phpcsFile->fixer->addContentBefore($opener, ' ');
                     } else {
-                        $phpcsFile->fixer->replaceToken(($closeBracket + 1), ' ');
+                        $phpcsFile->fixer->replaceToken(($opener - 1), ' ');
                     }
                 }
 
                 return;
             }//end if
-
-            // And just in case they do something funny before the brace...
-            $next = $phpcsFile->findNext(
-                T_WHITESPACE,
-                ($closeBracket + 1),
-                null,
-                true
-            );
-
-            if ($next !== false && $tokens[$next]['code'] !== T_OPEN_CURLY_BRACKET) {
-                $error = 'There must be a single space between the closing parenthesis and the opening brace of a multi-line function declaration';
-                $phpcsFile->addError($error, $next, 'NoSpaceBeforeOpenBrace');
-            }
         }//end if
 
     }//end processMultiLineDeclaration()

diff --git a/CodeSniffer/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.inc b/CodeSniffer/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.inc
@@ -178,3 +178,25 @@ blah
 $b = function &() {
     echo "hello";
 };
+
+function foo(
+    $param1,
+    $param2,
+    $param3,
+) : SomeClass {
+}
+
+function foo(
+    $param1,
+    $param2,
+    $param3,
+): SomeClass {
+}
+
+function foo(
+    $param1,
+    $param2,
+    $param3,
+): SomeClass // Comment here
+{
+}
diff --git a/CodeSniffer/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.inc.fixed b/CodeSniffer/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.inc.fixed
@@ -51,6 +51,7 @@ function getInstalledStandards(
     $includeGeneric=false,
     $standardsDir=''
 ) {
+
 }
 
 function &testFunction($arg1,
@@ -182,3 +183,25 @@ function blah()
 $b = function &() {
     echo "hello";
 };
+
+function foo(
+    $param1,
+    $param2,
+    $param3,
+) : SomeClass {
+}
+
+function foo(
+    $param1,
+    $param2,
+    $param3,
+): SomeClass {
+}
+
+function foo(
+    $param1,
+    $param2,
+    $param3,
+): SomeClass { // Comment here
+
+}
diff --git a/CodeSniffer/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.php b/CodeSniffer/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.php
@@ -51,9 +51,8 @@ public function getErrorList()
                 11  => 1,
                 14  => 1,
                 17  => 1,
-                27  => 1,
                 44  => 1,
-                51  => 1,
+                52  => 1,
                 61  => 2,
                 98  => 1,
                 110 => 2,
@@ -67,6 +66,7 @@ public function getErrorList()
                 167 => 2,
                 171 => 1,
                 173 => 1,
+                201 => 1,
                );
 
     }//end getErrorList()

diff --git a/CodeSniffer/Standards/Squiz/Tests/Functions/MultiLineFunctionDeclarationUnitTest.php b/CodeSniffer/Standards/Squiz/Tests/Functions/MultiLineFunctionDeclarationUnitTest.php
@@ -53,7 +53,6 @@ public function getErrorList($testFile='MultiLineFunctionDeclarationUnitTest.inc
                        12  => 1,
                        13  => 1,
                        16  => 1,
-                       33  => 1,
                        36  => 1,
                        43  => 2,
                        48  => 1,
@@ -74,7 +73,6 @@ public function getErrorList($testFile='MultiLineFunctionDeclarationUnitTest.inc
                        13 => 1,
                        16 => 1,
                        26 => 1,
-                       33 => 1,
                        36 => 1,
                        43 => 2,
                        48 => 1,

diff --git a/package.xml b/package.xml
@@ -73,6 +73,7 @@ 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 #781 : Incorrect checking for PHP7 return types on multi-line function declartions
   - 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