Skip to content

Commit

Permalink
Function return types are now tokenized as T_RETURN_TYPE in all cases…
Browse files Browse the repository at this point in the history 
… + the return value of File::getMethodProperties() now contains a return_type array index with a cleaned up return type (ref squizlabs#1527)
  • Loading branch information
@gsherwood
gsherwood committed Apr 17, 2018
1 parent 4270c75 commit 1f60f4f
Show file tree
Hide file tree
Showing 6 changed files with 624 additions and 54 deletions.
13 changes: 13 additions & 0 deletions package.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ http://pear.php.net/dtd/package-2.0.xsd">
-- This stops sniffs (especially commenting sniffs) from generating a large number of false errors when ignoring
-- Any custom sniffs that are using the T_PHPCS_* tokens to detect annotations may need to be changed to ignore them
-- Check $phpcsFile->config->annotations to see if annotations are enabled and ignore when false
- Function return types are now tokenized as T_RETURN_TYPE in all cases
-- Previously, only simple types were tokenized in this way and namespaces were ignored
-- Custom sniffs looking for T_NS_SEPERATOR inside return types will now need to look for T_RETURN_TYPE and check the value directly
- You can now use fully or partially qualified class names for custom reports instead of absolute file paths
-- To support this, you must specify an autoload file in your ruleset.xml file and use it to register an autoloader
-- Your autoloader will need to load your custom report class when requested
Expand All @@ -60,6 +63,10 @@ http://pear.php.net/dtd/package-2.0.xsd">
- Invalid installed_paths values are now ignored instead of causing a fatal error
- Improved testability of custom rulesets by allowing the installed standards to be overridden
-- Thanks to Timo Schinkel for the patch
- The return value of File::getMethodProperties() now contains a "return_type" array index
-- This contains the return type of the function or closer, or a blank string if not specified
-- If the return type contains namespace information, it will be cleaned of whitespace and comments
-- To access the original return value string, use the main tokens array
- The key used for caching PHPCS runs now includes all set config values
-- This fixes a problem where changing config values (e.g., via --runtime-set) used an incorrect cache file
- The "Function opening brace placement" metric has been separated into function and closure metrics in the info report
Expand Down Expand Up @@ -211,6 +218,8 @@ http://pear.php.net/dtd/package-2.0.xsd">
<file baseinstalldir="" name="GetMemberPropertiesTest.php" role="test" />
<file baseinstalldir="" name="GetMethodParametersTest.inc" role="test" />
<file baseinstalldir="" name="GetMethodParametersTest.php" role="test" />
<file baseinstalldir="" name="GetMethodPropertiesTest.inc" role="test" />
<file baseinstalldir="" name="GetMethodPropertiesTest.php" role="test" />
<file baseinstalldir="" name="IsReferenceTest.inc" role="test" />
<file baseinstalldir="" name="IsReferenceTest.php" role="test" />
</dir>
Expand Down Expand Up @@ -1723,6 +1732,8 @@ http://pear.php.net/dtd/package-2.0.xsd">
<install as="CodeSniffer/Core/File/FindImplementedInterfaceNamesTest.inc" name="tests/Core/File/FindImplementedInterfaceNamesTest.inc" />
<install as="CodeSniffer/Core/File/GetMethodParametersTest.php" name="tests/Core/File/GetMethodParametersTest.php" />
<install as="CodeSniffer/Core/File/GetMethodParametersTest.inc" name="tests/Core/File/GetMethodParametersTest.inc" />
<install as="CodeSniffer/Core/File/GetMethodPropertiesTest.php" name="tests/Core/File/GetMethodPropertiesTest.php" />
<install as="CodeSniffer/Core/File/GetMethodPropertiesTest.inc" name="tests/Core/File/GetMethodPropertiesTest.inc" />
<install as="CodeSniffer/Core/File/IsReferenceTest.php" name="tests/Core/File/IsReferenceTest.php" />
<install as="CodeSniffer/Core/File/IsReferenceTest.inc" name="tests/Core/File/IsReferenceTest.inc" />
<install as="CodeSniffer/Standards/AllSniffs.php" name="tests/Standards/AllSniffs.php" />
Expand Down Expand Up @@ -1752,6 +1763,8 @@ http://pear.php.net/dtd/package-2.0.xsd">
<install as="CodeSniffer/Core/File/FindImplementedInterfaceNamesTest.inc" name="tests/Core/File/FindImplementedInterfaceNamesTest.inc" />
<install as="CodeSniffer/Core/File/GetMethodParametersTest.php" name="tests/Core/File/GetMethodParametersTest.php" />
<install as="CodeSniffer/Core/File/GetMethodParametersTest.inc" name="tests/Core/File/GetMethodParametersTest.inc" />
<install as="CodeSniffer/Core/File/GetMethodPropertiesTest.php" name="tests/Core/File/GetMethodPropertiesTest.php" />
<install as="CodeSniffer/Core/File/GetMethodPropertiesTest.inc" name="tests/Core/File/GetMethodPropertiesTest.inc" />
<install as="CodeSniffer/Core/File/IsReferenceTest.php" name="tests/Core/File/IsReferenceTest.php" />
<install as="CodeSniffer/Core/File/IsReferenceTest.inc" name="tests/Core/File/IsReferenceTest.inc" />
<install as="CodeSniffer/Standards/AllSniffs.php" name="tests/Standards/AllSniffs.php" />
Expand Down
30 changes: 30 additions & 0 deletions src/Files/File.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1428,6 +1428,7 @@ public function getMethodParameters($stackPtr)
* <code>
* array(
* 'scope' => 'public', // public protected or protected
* 'return_type' => '', // the return type of the method.
* 'scope_specified' => true, // true is scope keyword was found.
* 'is_abstract' => false, // true if the abstract keyword was found.
* 'is_final' => false, // true if the final keyword was found.
Expand Down Expand Up @@ -1476,6 +1477,7 @@ public function getMethodProperties($stackPtr)
$isAbstract = false;
$isFinal = false;
$isStatic = false;
$returnType = '';

for ($i = ($stackPtr - 1); $i > 0; $i--) {
if (isset($valid[$this->tokens[$i]['code']]) === false) {
Expand Down Expand Up @@ -1507,8 +1509,36 @@ public function getMethodProperties($stackPtr)
}//end switch
}//end for

if (isset($this->tokens[$stackPtr]['parenthesis_closer']) === true) {
$scopeOpener = null;
if (isset($this->tokens[$stackPtr]['scope_opener']) === true) {
$scopeOpener = $this->tokens[$stackPtr]['scope_opener'];
}

for ($i = $this->tokens[$stackPtr]['parenthesis_closer']; $i < $this->numTokens; $i++) {
if (($scopeOpener === null && $this->tokens[$i]['code'] === T_SEMICOLON)
|| ($scopeOpener !== null && $i === $scopeOpener)
) {
// Didn't find anything.
break;
}

while ($this->tokens[$i]['code'] === T_RETURN_TYPE) {
$returnType .= $this->tokens[$i]['content'];
$i++;
}
}
}

if ($returnType !== '') {
// Cleanup.
$returnType = preg_replace('/\s+/', '', $returnType);
$returnType = preg_replace('/\/\*.*?\*\//', '', $returnType);
}

return [
'scope' => $scope,
'return_type' => $returnType,
'scope_specified' => $scopeSpecified,
'is_abstract' => $isAbstract,
'is_final' => $isFinal,
Expand Down
151 changes: 97 additions & 54 deletions src/Tokenizers/PHP.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1017,7 +1017,8 @@ protected function tokenize($string)
so go forward and change the token type before it is processed.
*/

if ($tokenIsArray === true && $token[0] === T_FUNCTION
if ($tokenIsArray === true
&& $token[0] === T_FUNCTION
&& $finalTokens[$lastNotEmptyToken]['code'] !== T_USE
) {
for ($x = ($stackPtr + 1); $x < $numTokens; $x++) {
Expand All @@ -1032,7 +1033,101 @@ protected function tokenize($string)
if ($x < $numTokens && is_array($tokens[$x]) === true) {
$tokens[$x][0] = T_STRING;
}
}

// Go looking for the colon to start the return type hint.
// Start by finding the closing parenthesis of the function.
$parenthesisStack = [];
$parenthesisCloser = false;
for ($x = ($stackPtr + 1); $x < $numTokens; $x++) {
if (is_array($tokens[$x]) === false && $tokens[$x] === '(') {
$parenthesisStack[] = $x;
} else if (is_array($tokens[$x]) === false && $tokens[$x] === ')') {
array_pop($parenthesisStack);
if (empty($parenthesisStack) === true) {
$parenthesisCloser = $x;
break;
}
}
}

if ($parenthesisCloser !== false) {
for ($x = ($parenthesisCloser + 1); $x < $numTokens; $x++) {
if (is_array($tokens[$x]) === false
|| isset(Util\Tokens::$emptyTokens[$tokens[$x][0]]) === false
) {
// Non-empty content.
if (is_array($tokens[$x]) === true && $tokens[$x][0] === T_USE) {
// Search ahead for the closing parenthesis.
for ($x = ($x + 1); $x < $numTokens; $x++) {
if (is_array($tokens[$x]) === false && $tokens[$x] === ')') {
continue(2);
}
}
}

break;
}
}

if (is_array($tokens[$x]) === false && $tokens[$x] === ':') {
$allowed = [
T_STRING => T_STRING,
T_ARRAY => T_ARRAY,
T_ARRAY_HINT => T_ARRAY_HINT,
T_CALLABLE => T_CALLABLE,
T_SELF => T_SELF,
T_PARENT => T_PARENT,
T_NS_SEPARATOR => T_NS_SEPARATOR,
];

$allowed += Util\Tokens::$emptyTokens;

$typeHintStart = null;
$typeHintEnd = null;

for ($x = ($x + 1); $x < $numTokens; $x++) {
if ($typeHintStart === null
&& is_array($tokens[$x]) === true
&& isset(Util\Tokens::$emptyTokens[$tokens[$x][0]]) === true
) {
// We haven't found the start of the type hint yet.
continue;
}

if (is_array($tokens[$x]) === true
&& isset($allowed[$tokens[$x][0]]) === true
) {
if ($typeHintStart === null) {
$typeHintStart = $x;
}

if (isset(Util\Tokens::$emptyTokens[$tokens[$x][0]]) === false) {
$typeHintEnd = $x;
}

continue;
}

break;
}//end for

if ($typeHintStart !== null) {
$tokens[$typeHintStart][0] = T_RETURN_TYPE;

for ($i = ($typeHintStart + 1); $i <= $typeHintEnd; $i++) {
if (PHP_CODESNIFFER_VERBOSITY > 1) {
$type = Util\Tokens::tokenName($tokens[$i][0]);
$content = Util\Common::prepareForOutput($tokens[$i][1]);
echo "\t\t* token $i merged into token $typeHintStart (T_RETURN_TYPE); was: $type => $content".PHP_EOL;
}

$tokens[$typeHintStart][1] .= $tokens[$i][1];
$tokens[$i] = null;
}
}
}//end if
}//end if
}//end if

/*
Before PHP 7, the <=> operator was tokenized as
Expand Down Expand Up @@ -1439,60 +1534,8 @@ protected function processAdditional()
}
}
}

$tokenAfterReturnTypeHint = $this->tokens[$i]['scope_opener'];
} else if (isset($this->tokens[$i]['parenthesis_closer']) === true) {
$tokenAfterReturnTypeHint = null;
for ($x = ($this->tokens[$i]['parenthesis_closer'] + 1); $x < $numTokens; $x++) {
if ($this->tokens[$x]['code'] === T_SEMICOLON) {
$tokenAfterReturnTypeHint = $x;
break;
}
}

if ($tokenAfterReturnTypeHint === null) {
// Probably a syntax error.
continue;
}
} else {
// Probably a syntax error.
continue;
}//end if

/*
Detect function return values and assign them
a special token, because PHP doesn't.
*/

for ($x = ($tokenAfterReturnTypeHint - 1); $x > $i; $x--) {
if (isset(Util\Tokens::$emptyTokens[$this->tokens[$x]['code']]) === false) {
if (in_array($this->tokens[$x]['code'], [T_STRING, T_ARRAY, T_ARRAY_HINT, T_CALLABLE, T_SELF, T_PARENT], true) === true) {
if (PHP_CODESNIFFER_VERBOSITY > 1) {
$line = $this->tokens[$x]['line'];
$type = $this->tokens[$x]['type'];
echo "\t* token $x on line $line changed from $type to T_RETURN_TYPE".PHP_EOL;
}

$this->tokens[$x]['code'] = T_RETURN_TYPE;
$this->tokens[$x]['type'] = 'T_RETURN_TYPE';

if (array_key_exists('parenthesis_opener', $this->tokens[$x]) === true) {
unset($this->tokens[$x]['parenthesis_opener']);
}

if (array_key_exists('parenthesis_closer', $this->tokens[$x]) === true) {
unset($this->tokens[$x]['parenthesis_closer']);
}

if (array_key_exists('parenthesis_owner', $this->tokens[$x]) === true) {
unset($this->tokens[$x]['parenthesis_owner']);
}
}//end if

break;
}//end if
}//end for

continue;
} else if ($this->tokens[$i]['code'] === T_CLASS && isset($this->tokens[$i]['scope_opener']) === true) {
/*
Expand Down
2 changes: 2 additions & 0 deletions tests/Core/AllTests.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
require_once 'File/FindImplementedInterfaceNamesTest.php';
require_once 'File/GetMemberPropertiesTest.php';
require_once 'File/GetMethodParametersTest.php';
require_once 'File/GetMethodPropertiesTest.php';
require_once 'File/IsReferenceTest.php';

class AllTests
Expand Down Expand Up @@ -52,6 +53,7 @@ public static function suite()
$suite->addTestSuite('PHP_CodeSniffer\Tests\Core\File\FindImplementedInterfaceNamesTest');
$suite->addTestSuite('PHP_CodeSniffer\Tests\Core\File\GetMemberPropertiesTest');
$suite->addTestSuite('PHP_CodeSniffer\Tests\Core\File\GetMethodParametersTest');
$suite->addTestSuite('PHP_CodeSniffer\Tests\Core\File\GetMethodPropertiesTest');
$suite->addTestSuite('PHP_CodeSniffer\Tests\Core\File\IsReferenceTest');
return $suite;

Expand Down
48 changes: 48 additions & 0 deletions tests/Core/File/GetMethodPropertiesTest.inc
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php
/* phpcs:ignoreFile */

/* testBasicFunction */
function myFunction() {}

/* testReturnFunction */
function myFunction(array ...$arrays): array
{
return array_map(/* testNestedClosure */function(array $array): int {
return array_sum($array);
}, $arrays);
}

class MyClass {
/* testBasicMethod */
function myFunction() {}

/* testPrivateStaticMethod */
private static function myFunction() {}

/* testFinalMethod */
final public function myFunction() {}

/* testProtectedReturnMethod */
protected function myFunction() : int {}

/* testPublicReturnMethod */
public function myFunction(): array {}

/* testReturnNamespace */
function myFunction(): \MyNamespace\MyClass {}

/* testReturnMultilineNamespace */
function myFunction(): \MyNamespace /** comment *\/ comment */
\MyClass /* comment */
\Foo {}
}

abstract class MyClass
{
/* testAbstractMethod */
abstract function myFunction();

/* testAbstractReturnMethod */
abstract protected function myFunction(): bool;
}

Loading

0 comments on commit 1f60f4f

Please sign in to comment.