Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix return type comments #229

Merged
merged 6 commits into from
Feb 10, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 83 additions & 8 deletions Joomla/Sniffs/Commenting/FunctionCommentSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,18 @@ protected function processReturn(PHP_CodeSniffer_File $phpcsFile, $stackPtr, $co
}
else
{
// Support both a return type and a description.
preg_match('`^((?:\|?(?:array\([^\)]*\)|[\\\\a-z0-9\[\]]+))*)( .*)?`i', $content, $returnParts);

if (isset($returnParts[1]) === false)
{
return;
}

$returnType = $returnParts[1];

// Check return type (can have multiple return types separated by '|').
$typeNames = explode('|', $content);
$typeNames = explode('|', $returnType);
$suggestedNames = array();

foreach ($typeNames as $i => $typeName)
Expand All @@ -98,19 +108,27 @@ protected function processReturn(PHP_CodeSniffer_File $phpcsFile, $stackPtr, $co

$suggestedType = implode('|', $suggestedNames);

if ($content !== $suggestedType)
if ($returnType !== $suggestedType)
{
$error = 'Expected "%s" but found "%s" for function return type';
$data = array(
$suggestedType,
$content
$returnType
);

$fix = $phpcsFile->addFixableError($error, $return, 'InvalidReturn', $data);

if ($fix === true)
{
$phpcsFile->fixer->replaceToken(($return + 2), $suggestedType);
$replacement = $suggestedType;

if (empty($returnParts[2]) === false)
{
$replacement .= $returnParts[2];
}

$phpcsFile->fixer->replaceToken(($return + 2), $replacement);
unset($replacement);
}
}

Expand All @@ -119,14 +137,71 @@ protected function processReturn(PHP_CodeSniffer_File $phpcsFile, $stackPtr, $co
* somewhere in the function that returns something.
* Skip this check for mixed return types.
*/
if (!in_array($content, array('void', 'mixed')))
if ($returnType === 'void')
{
if (isset($tokens[$stackPtr]['scope_closer']) === true)
{
$endToken = $tokens[$stackPtr]['scope_closer'];
$returnToken = $phpcsFile->findNext(array(T_RETURN, T_YIELD), $stackPtr, $endToken);
$endToken = $tokens[$stackPtr]['scope_closer'];

for ($returnToken = $stackPtr; $returnToken < $endToken; $returnToken++)
{
if ($tokens[$returnToken]['code'] === T_CLOSURE
|| $tokens[$returnToken]['code'] === T_ANON_CLASS
)
{
$returnToken = $tokens[$returnToken]['scope_closer'];
continue;
}

if ($tokens[$returnToken]['code'] === T_RETURN
|| $tokens[$returnToken]['code'] === T_YIELD
|| $tokens[$returnToken]['code'] === T_YIELD_FROM
)
{
break;
}
}

if ($returnToken !== $endToken)
{
// If the function is not returning anything, just exiting, then there is no problem.
$semicolon = $phpcsFile->findNext(T_WHITESPACE, ($returnToken + 1), null, true);

if ($tokens[$semicolon]['code'] !== T_SEMICOLON)
{
$error = 'Function return type is void, but function contains return statement';
$phpcsFile->addError($error, $return, 'InvalidReturnVoid');
}
}
}//end if
}
elseif ($returnType !== 'mixed' && in_array('void', $typeNames, true) === false)
{
// 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'];

for ($returnToken = $stackPtr; $returnToken < $endToken; $returnToken++)
{
if ($tokens[$returnToken]['code'] === T_CLOSURE
|| $tokens[$returnToken]['code'] === T_ANON_CLASS
)
{
$returnToken = $tokens[$returnToken]['scope_closer'];
continue;
}

if ($tokens[$returnToken]['code'] === T_RETURN
|| $tokens[$returnToken]['code'] === T_YIELD
|| $tokens[$returnToken]['code'] === T_YIELD_FROM
)
{
break;
}
}

if ($returnToken === false)
if ($returnToken === $endToken)
{
$error = 'Function return type is not void, but function has no return statement';
$phpcsFile->addError($error, $return, 'InvalidNoReturn');
Expand Down
53 changes: 53 additions & 0 deletions Joomla/Tests/Commenting/FunctionCommentUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -197,4 +197,57 @@ class MyClass
{
return 2;
}

/**
* Get the details list of keys for a table.
*
* @param array $keys An array of objects that comprise the keys for the table.
*
* @return array The lookup array. array({key name} => array(object, ...))
*
* @since 1.0
*/
protected function getKeyLookup($keys)
{
// First pass, create a lookup of the keys.
$lookup = array();

return $lookup;
}

/**
* Test function ArrayWithClosure
*
* @return array
*/
function returnArrayWithClosure()
{
function ()
{
return;
};

return array();
}

/**
* Test function ArrayWithAnonymousClass
*
* @return array
*/
function returnArrayWithAnonymousClass()
{
new class
{
/**
* @return void
*/
public function test()
{
return;
}
};

return array();
}
}
53 changes: 53 additions & 0 deletions Joomla/Tests/Commenting/FunctionCommentUnitTest.inc.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -197,4 +197,57 @@ class MyClass
{
return 2;
}

/**
* Get the details list of keys for a table.
*
* @param array $keys An array of objects that comprise the keys for the table.
*
* @return array The lookup array. array({key name} => array(object, ...))
*
* @since 1.0
*/
protected function getKeyLookup($keys)
{
// First pass, create a lookup of the keys.
$lookup = array();

return $lookup;
}

/**
* Test function ArrayWithClosure
*
* @return array
*/
function returnArrayWithClosure()
{
function ()
{
return;
};

return array();
}

/**
* Test function ArrayWithAnonymousClass
*
* @return array
*/
function returnArrayWithAnonymousClass()
{
new class
{
/**
* @return void
*/
public function test()
{
return;
}
};

return array();
}
}