diff --git a/coder_sniffer/Drupal/Sniffs/Commenting/FunctionCommentSniff.php b/coder_sniffer/Drupal/Sniffs/Commenting/FunctionCommentSniff.php index 72eff15..c9e241c 100644 --- a/coder_sniffer/Drupal/Sniffs/Commenting/FunctionCommentSniff.php +++ b/coder_sniffer/Drupal/Sniffs/Commenting/FunctionCommentSniff.php @@ -221,7 +221,17 @@ class Drupal_Sniffs_Commenting_FunctionCommentSniff implements PHP_CodeSniffer_S // Check return type (can be multiple, separated by '|'). $typeNames = explode('|', $type); $suggestedNames = array(); + $hasNull = false; + $hasMultiple = false; + if (count($typeNames) > 0) { + $hasMultiple = true; + } + foreach ($typeNames as $i => $typeName) { + if (strtolower($typeName) === 'null') { + $hasNull = true; + } + $suggestedName = $this->suggestType($typeName); if (in_array($suggestedName, $suggestedNames) === false) { $suggestedNames[] = $suggestedName; @@ -251,19 +261,42 @@ class Drupal_Sniffs_Commenting_FunctionCommentSniff implements PHP_CodeSniffer_S // If return type is not void, there needs to be a return statement // somewhere in the function that returns something. if (isset($tokens[$stackPtr]['scope_closer']) === true) { - $endToken = $tokens[$stackPtr]['scope_closer']; - $returnToken = $phpcsFile->findNext(T_RETURN, $stackPtr, $endToken); - if ($returnToken === false) { - $error = '@return doc comment specified, but function has no return statement'; - $phpcsFile->addError($error, $return, 'InvalidNoReturn'); - } else { - $semicolon = $phpcsFile->findNext(T_WHITESPACE, ($returnToken + 1), null, true); - if ($tokens[$semicolon]['code'] === T_SEMICOLON) { - $error = 'Function return type is not void, but function is returning void here'; - $phpcsFile->addError($error, $returnToken, 'InvalidReturnNotVoid'); - } + $endToken = $tokens[$stackPtr]['scope_closer']; + $foundReturnToken = false; + $searchStart = $stackPtr; + $foundNonVoidReturn = false; + do { + $returnToken = $phpcsFile->findNext(T_RETURN, $searchStart, $endToken); + if ($returnToken === false && $foundReturnToken === false) { + $error = '@return doc comment specified, but function has no return statement'; + $phpcsFile->addError($error, $return, 'InvalidNoReturn'); + } else { + // Check for return token as the last loop after the last return + // in the function will enter this else condition + // but without the returnToken. + if ($returnToken !== false) { + $foundReturnToken = true; + $semicolon = $phpcsFile->findNext(T_WHITESPACE, ($returnToken + 1), null, true); + if ($tokens[$semicolon]['code'] === T_SEMICOLON) { + // Void return is allowed if the @return type has null in it. + if ($hasNull === false) { + $error = 'Function return type is not void, but function is returning void here'; + $phpcsFile->addError($error, $returnToken, 'InvalidReturnNotVoid'); + } + } else { + $foundNonVoidReturn = true; + }//end if + + $searchStart = ($returnToken + 1); + }//end if + }//end if + } while ($returnToken !== false); + + if ($foundNonVoidReturn === false && $foundReturnToken === true) { + $error = 'Function return type is not void, but function does not have a non-void return statement'; + $phpcsFile->addError($error, $return, 'InvalidReturnNotVoid'); } - } + }//end if }//end if }//end if diff --git a/coder_sniffer/Drupal/Test/Commenting/FunctionCommentUnitTest.inc b/coder_sniffer/Drupal/Test/Commenting/FunctionCommentUnitTest.inc index 81a809c..b439ceb 100644 --- a/coder_sniffer/Drupal/Test/Commenting/FunctionCommentUnitTest.inc +++ b/coder_sniffer/Drupal/Test/Commenting/FunctionCommentUnitTest.inc @@ -258,3 +258,36 @@ function test21($arg1, $arg2, $arg3) { */ function test22($arg1, $arg2, $arg3, $arg4, $arg5, $arg6) { } + +/** + * Void returns allowed when null is given in @return. + * + * When null is a potential return value it should be allowed to have return + * statements with void return as well. + * + * @return bool|null + * This implies that the return value can be NULL, a boolean, or empty. + */ +function test23() { + if (1 == 2) { + return; + } + return TRUE; +} + +/** + * Even when null is given in @return there must be at least 1 valid return. + * + * When null is a potential return value it should be allowed to have return + * statements with void return. This does not mean that all returns can be void. + * There must be at least one non-void return. + * + * @return bool|null + * This implies that the return value can be NULL, a boolean, or empty. + */ +function test24() { + if (1 == 2) { + return; + } + return; +} diff --git a/coder_sniffer/Drupal/Test/Commenting/FunctionCommentUnitTest.inc.fixed b/coder_sniffer/Drupal/Test/Commenting/FunctionCommentUnitTest.inc.fixed index 01a2908..c47ea1f 100644 --- a/coder_sniffer/Drupal/Test/Commenting/FunctionCommentUnitTest.inc.fixed +++ b/coder_sniffer/Drupal/Test/Commenting/FunctionCommentUnitTest.inc.fixed @@ -270,3 +270,36 @@ function test21($arg1, $arg2, $arg3) { */ function test22($arg1, $arg2, $arg3, $arg4, $arg5, $arg6) { } + +/** + * Void returns allowed when null is given in @return. + * + * When null is a potential return value it should be allowed to have return + * statements with void return as well. + * + * @return bool|null + * This implies that the return value can be NULL, a boolean, or empty. + */ +function test23() { + if (1 == 2) { + return; + } + return TRUE; +} + +/** + * Even when null is given in @return there must be at least 1 valid return. + * + * When null is a potential return value it should be allowed to have return + * statements with void return. This does not mean that all returns can be void. + * There must be at least one non-void return. + * + * @return bool|null + * This implies that the return value can be NULL, a boolean, or empty. + */ +function test24() { + if (1 == 2) { + return; + } + return; +} diff --git a/coder_sniffer/Drupal/Test/Commenting/FunctionCommentUnitTest.php b/coder_sniffer/Drupal/Test/Commenting/FunctionCommentUnitTest.php index dbd3f4e..7d437c7 100644 --- a/coder_sniffer/Drupal/Test/Commenting/FunctionCommentUnitTest.php +++ b/coder_sniffer/Drupal/Test/Commenting/FunctionCommentUnitTest.php @@ -44,6 +44,7 @@ class Drupal_Sniffs_Commenting_FunctionCommentUnitTest extends CoderSniffUnitTes 252 => 1, 254 => 1, 256 => 1, + 285 => 1, ); }//end getErrorList() diff --git a/coder_sniffer/Drupal/Test/good/good.php b/coder_sniffer/Drupal/Test/good/good.php index dadd14e..021d505 100644 --- a/coder_sniffer/Drupal/Test/good/good.php +++ b/coder_sniffer/Drupal/Test/good/good.php @@ -1315,3 +1315,22 @@ $access = $mapper->hasSchema() && $mapper->hasTranslatable() && $source_language_access; + +/** + * Return with optional null and function with some void returns. + * + * When a function has an optional null as a return value then it should not + * result in an error when there is a void return in the function. + * + * @param string $arg + * An argument. + * + * @return int|null + * Return has an optional null. + */ +function test16($arg) { + if ($arg === 1) { + return; + } + return 1; +}