Merge pull request #8 from dmeybohm/fix-func-return-comments

Fix function return sniff
zumba · Jan 9, 2018 · c6772c3 · c6772c3
2 parents 48dd0a2 + c0cb02a
commit c6772c3
Show file tree

Hide file tree

Showing 4 changed files with 94 additions and 28 deletions.
diff --git a/Zumba/Sniffs/Commenting/FunctionCommentSniff.php b/Zumba/Sniffs/Commenting/FunctionCommentSniff.php
@@ -327,21 +327,12 @@ protected function processReturn($commentStart, $commentEnd)
                     // no return statement in the function.
                     if ($content === 'void') {
                         if (isset($tokens[$this->_functionToken]['scope_closer']) === true) {
-                            $endToken    = $tokens[$this->_functionToken]['scope_closer'];
-                            $returnToken = $this->currentFile->findNext(T_RETURN, $this->_functionToken, $endToken);
-                            if ($returnToken !== false) {
-                                // If the function is not returning anything, just
-                                // exiting, then there is no problem.
-                                $semicolon = $this->currentFile->findNext(T_WHITESPACE, ($returnToken + 1), null, true);
-                                if ($tokens[$semicolon]['code'] !== T_SEMICOLON) {
-                                    $error = 'Function return type is void, but function contains return statement';
-                                    $this->currentFile->addError($error, $errorPos, 'InvalidReturnVoid');
-                                }
-                            }
+                            $endToken = $tokens[$this->_functionToken]['scope_closer'];
+                            $this->ensureNoReturnStatementsReturnAValue($tokens, $endToken);
                         }
-                    } else if ($content !== 'mixed') {
-                        // If return type is not void, there needs to be a
-                        // returns statement somewhere in the function that
+                    } else if ($content !== 'mixed' && in_array("\\Generator", $suggestedNames, true) === false) {
+                        // If return type is not void, and the function is not a generator,
+                        // there needs to be a returns statement somewhere in the function that
                         // returns something.
                         if (isset($tokens[$this->_functionToken]['scope_closer']) === true) {
                             $endToken    = $tokens[$this->_functionToken]['scope_closer'];
@@ -549,7 +540,76 @@ protected function processParams($commentStart, $commentEnd)
 
     }//end processParams()
 
+    /**
+     * Ensure no return statements within a function return a value.
+     *
+     * Also detect return statements within closures, and ignore those.
+     *
+     * @param array $tokens
+     * @param int $endToken
+     * @return void
+     */
+    protected function ensureNoReturnStatementsReturnAValue($tokens, $endToken)
+    {
+        $startToken = $this->_functionToken;
+        while (true) {
+            $returnToken = $this->currentFile->findNext(T_RETURN, $startToken, $endToken);
+            if ($returnToken === false || $returnToken === $endToken) {
+                break;
+            }
+            // If the function is not returning anything, just
+            // exiting, then there is no problem.
+            $semicolon = $this->currentFile->findNext(T_WHITESPACE, ($returnToken + 1), null, true);
+            if ($tokens[$semicolon]['code'] !== T_SEMICOLON) {
+                if (!$this->returnIsWithinClosure($tokens, $this->_functionToken, $returnToken)) {
+                    $error = 'Function return type is void, but function contains return statement';
+                    $this->currentFile->addError($error, $returnToken, 'InvalidReturnVoid');
+                }
+            }
+            $startToken = $semicolon;
+        }
+    }//end ensureNoReturnStatementsReturnAValue()
 
-}//end class
+    /**
+     * Search through the function to determine if the return is within a closure.
+     *
+     * @param array $tokens
+     * @param int $startFunctionToken
+     * @param int $returnToken
+     * @return boolean
+     */
+    protected function returnIsWithinClosure($tokens, $startFunctionToken, $returnToken)
+    {
+        //
+        // We scan backwards in the function looking for curly braces, and keep a count of how many we've seen.
+        // If we see a curly bracket open followed by a function token, then the return is within a function.
+        // If the curly bracket count is zero, then we hit a closure that preceded the return statement but was
+        // not surrounding it.
+        //
+        $openBraceCount = 0;
+        $types = array(T_OPEN_CURLY_BRACKET, T_CLOSE_CURLY_BRACKET, T_CLOSURE);
+        $endToken = $returnToken;
+        while (true) {
+            $tokenPos = $this->currentFile->findPrevious($types, $endToken, $startFunctionToken);
+            if ($tokenPos === false) {
+                break;
+            }
+            switch ($tokens[$tokenPos]['code']) {
+                case T_OPEN_CURLY_BRACKET:
+                    $openBraceCount--;
+                    break;
+                case T_CLOSE_CURLY_BRACKET:
+                    $openBraceCount++;
+                    break;
+                case T_CLOSURE:
+                    if ($openBraceCount < 0) {
+                        return true;
+                    }
+                    break;
+            }
+            $endToken = $tokenPos - 1;
+        }
+        return false;
+    }//end returnIsWithinClosure()
 
-?>
+}//end class
diff --git a/test/data/closure-return.php b/test/data/closure-return.php
@@ -17,6 +17,6 @@ function foobar() {
 --------------------------------------------------------------------------------
 FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
 --------------------------------------------------------------------------------
- 6 | ERROR | Function return type is void, but function contains return
-   |       | statement
+ 12 | ERROR | Function return type is void, but function contains return
+    |       | statement
 --------------------------------------------------------------------------------
diff --git a/test/data/generator.php b/test/data/generator.php
@@ -12,20 +12,19 @@ function generatorByItself() {
 /**
  * Short function description
  *
- * @return \Generator|boolean
+ * @return \Generator|boolean[]
  */
 function generatorWithBoolean() {
 	yield true;
 }
 
+/**
+ * Short function description
+ * 
+ * @return mixed
+ */
+function generatorWithMixedReturn() {
+    yield 'something';
+}
 ?>
 --EXPECT--
-
---------------------------------------------------------------------------------
-FOUND 2 ERROR(S) AFFECTING 2 LINE(S)
---------------------------------------------------------------------------------
-  6 | ERROR | Function return type is not void, but function has no return
-    |       | statement
- 15 | ERROR | Function return type is not void, but function has no return
-    |       | statement
---------------------------------------------------------------------------------
diff --git a/test/data/multireturn.php b/test/data/multireturn.php
@@ -13,3 +13,10 @@ function foobar() {
 }
 ?>
 --EXPECT--
+
+--------------------------------------------------------------------------------
+FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
+--------------------------------------------------------------------------------
+ 12 | ERROR | Function return type is void, but function contains return
+    |       | statement
+--------------------------------------------------------------------------------