Skip to content

Commit

Permalink
File::getMethodProperties(): bug fix - skip over closure use statements
Browse files Browse the repository at this point in the history
While working on something else, I came across this beauty of a bug for which I'm flabbergasted that it hasn't been reported before, though I can only presume that this means it has been a bug with little impact.

The short of it is that the `File::getMethodProperties()` method previously did not skip over closure `use` statements when walking the tokens to find the return type.

As closure `use` statements can only contain variables, the only tokens which could be encountered in a closure `use` statement, which could also be encountered in a return type declaration are the `T_STRING`, `T_SELF`, `T_PARENT` and the `T_STATIC` tokens in the case when the variable being imported is a class property, i.e. the property name part of `$this->name` or `self` in `self::$name`.

Either way, this patch fixes the issue.

Includes unit tests.
  • Loading branch information
jrfnl committed Mar 29, 2024
1 parent 846fbf0 commit 4343197
Show file tree
Hide file tree
Showing 3 changed files with 188 additions and 0 deletions.
14 changes: 14 additions & 0 deletions src/Files/File.php
Original file line number Diff line number Diff line change
Expand Up @@ -1757,6 +1757,20 @@ public function getMethodProperties($stackPtr)
break;
}

if ($this->tokens[$i]['code'] === T_USE) {
// Skip over closure use statements.
for ($j = ($i + 1); $j < $this->numTokens && isset(Tokens::$emptyTokens[$this->tokens[$j]['code']]) === true; $j++);
if ($this->tokens[$j]['code'] === T_OPEN_PARENTHESIS) {
if (isset($this->tokens[$j]['parenthesis_closer']) === false) {
// Live coding/parse error, stop parsing.
break;
}

$i = $this->tokens[$j]['parenthesis_closer'];
continue;
}
}

if ($this->tokens[$i]['code'] === T_NULLABLE) {
$nullableReturnType = true;
}
Expand Down
18 changes: 18 additions & 0 deletions tests/Core/File/GetMethodPropertiesTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,24 @@ $value = $obj->fn(true);
/* testFunctionDeclarationNestedInTernaryPHPCS2975 */
return (!$a ? [ new class { public function b(): c {} } ] : []);

/* testClosureWithUseNoReturnType */
$closure = function () use($a) /*comment*/ {};

/* testClosureWithUseNoReturnTypeUseProp */
$closure = function () use ($this->prop){};

/* testClosureWithUseWithReturnType */
$closure = function () use /*comment*/ ($a): Type {};

/* testClosureWithUseWithReturnTypeUseProp */
$closure = function () use ($this->prop): ?array {};

/* testClosureWithUseWithReturnTypeUseStaticPropWithSelf */
$closure = function() use(self::$prop) : int {};

/* testClosureWithUseWithReturnTypeUseStaticPropWithStatic */
$closure = function () use (static::$prop): false|null {};

/* testArrowFunctionLiveCoding */
// Intentional parse error. This has to be the last test in the file.
$fn = fn
156 changes: 156 additions & 0 deletions tests/Core/File/GetMethodPropertiesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1297,6 +1297,162 @@ public function testFunctionDeclarationNestedInTernaryPHPCS2975()
}//end testFunctionDeclarationNestedInTernaryPHPCS2975()


/**
* Test handling of closure declarations with a use variable import without a return type declaration.
*
* @return void
*/
public function testClosureWithUseNoReturnType()
{
// Offsets are relative to the T_CLOSURE token.
$expected = [
'scope' => 'public',
'scope_specified' => false,
'return_type' => '',
'return_type_token' => false,
'return_type_end_token' => false,
'nullable_return_type' => false,
'is_abstract' => false,
'is_final' => false,
'is_static' => false,
'has_body' => true,
];

$this->getMethodPropertiesTestHelper('/* '.__FUNCTION__.' */', $expected);

}//end testClosureWithUseNoReturnType()


/**
* Test handling of closure declarations with a use variable import without a return type declaration.
*
* @return void
*/
public function testClosureWithUseNoReturnTypeUseProp()
{
// Offsets are relative to the T_CLOSURE token.
$expected = [
'scope' => 'public',
'scope_specified' => false,
'return_type' => '',
'return_type_token' => false,
'return_type_end_token' => false,
'nullable_return_type' => false,
'is_abstract' => false,
'is_final' => false,
'is_static' => false,
'has_body' => true,
];

$this->getMethodPropertiesTestHelper('/* '.__FUNCTION__.' */', $expected);

}//end testClosureWithUseNoReturnTypeUseProp()


/**
* Test handling of closure declarations with a use variable import with a return type declaration.
*
* @return void
*/
public function testClosureWithUseWithReturnType()
{
// Offsets are relative to the T_CLOSURE token.
$expected = [
'scope' => 'public',
'scope_specified' => false,
'return_type' => 'Type',
'return_type_token' => 14,
'return_type_end_token' => 14,
'nullable_return_type' => false,
'is_abstract' => false,
'is_final' => false,
'is_static' => false,
'has_body' => true,
];

$this->getMethodPropertiesTestHelper('/* '.__FUNCTION__.' */', $expected);

}//end testClosureWithUseWithReturnType()


/**
* Test handling of closure declarations with a use variable import with a return type declaration.
*
* @return void
*/
public function testClosureWithUseWithReturnTypeUseProp()
{
// Offsets are relative to the T_CLOSURE token.
$expected = [
'scope' => 'public',
'scope_specified' => false,
'return_type' => '?array',
'return_type_token' => 15,
'return_type_end_token' => 15,
'nullable_return_type' => true,
'is_abstract' => false,
'is_final' => false,
'is_static' => false,
'has_body' => true,
];

$this->getMethodPropertiesTestHelper('/* '.__FUNCTION__.' */', $expected);

}//end testClosureWithUseWithReturnTypeUseProp()


/**
* Test handling of closure declarations with a use variable import with a return type declaration.
*
* @return void
*/
public function testClosureWithUseWithReturnTypeUseStaticPropWithSelf()
{
// Offsets are relative to the T_CLOSURE token.
$expected = [
'scope' => 'public',
'scope_specified' => false,
'return_type' => 'int',
'return_type_token' => 13,
'return_type_end_token' => 13,
'nullable_return_type' => false,
'is_abstract' => false,
'is_final' => false,
'is_static' => false,
'has_body' => true,
];

$this->getMethodPropertiesTestHelper('/* '.__FUNCTION__.' */', $expected);

}//end testClosureWithUseWithReturnTypeUseStaticPropWithSelf()


/**
* Test handling of closure declarations with a use variable import with a return type declaration.
*
* @return void
*/
public function testClosureWithUseWithReturnTypeUseStaticPropWithStatic()
{
// Offsets are relative to the T_CLOSURE token.
$expected = [
'scope' => 'public',
'scope_specified' => false,
'return_type' => 'false|null',
'return_type_token' => 14,
'return_type_end_token' => 16,
'nullable_return_type' => false,
'is_abstract' => false,
'is_final' => false,
'is_static' => false,
'has_body' => true,
];

$this->getMethodPropertiesTestHelper('/* '.__FUNCTION__.' */', $expected);

}//end testClosureWithUseWithReturnTypeUseStaticPropWithStatic()


/**
* Test helper.
*
Expand Down

0 comments on commit 4343197

Please sign in to comment.