Skip to content

Commit

Permalink
File::findStartOfStatement(): bug fix - don't give tokens within nest…
Browse files Browse the repository at this point in the history 
…ed parentheses the "match" treatment

The `File::findStartOfStatement()` method has special handling for `match` expressions to find the start of a statement, but that special handling would also kick in when the `$start` token is within a parenthesized expression within the `match`, while, in that case, the special handling is not needed and ends up returning an incorrect "start" pointer, in most cases, even a "start pointer" which is _after_ the token for which the start of statement is requested, which should never be possible.

Fixed now by changing the special handling for `match` expressions to only kick in when the `$start` pointer and the `match` pointer are at the same parentheses nesting level.

Includes unit tests.
Of the tests, the first array item/parameter was not affected by this bug, but subsequent array items/parameters were all affected by this bug.
  • Loading branch information
@jrfnl
jrfnl committed May 21, 2024
1 parent 467d284 commit b82438f
Show file tree
Hide file tree
Showing 3 changed files with 191 additions and 3 deletions.
13 changes: 10 additions & 3 deletions src/Files/File.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2438,9 +2438,16 @@ public function findStartOfStatement($start, $ignore=null)
if (empty($this->tokens[$start]['conditions']) === false) {
$conditions = $this->tokens[$start]['conditions'];
$lastConditionOwner = end($conditions);

if ($lastConditionOwner === T_MATCH) {
$matchExpression = key($conditions);
$matchExpression = key($conditions);

if ($lastConditionOwner === T_MATCH
// Check if the $start token is at the same parentheses nesting level as the match token.
&& ((empty($this->tokens[$matchExpression]['nested_parenthesis']) === true
&& empty($this->tokens[$start]['nested_parenthesis']) === true)
|| ((empty($this->tokens[$matchExpression]['nested_parenthesis']) === false
&& empty($this->tokens[$start]['nested_parenthesis']) === false)
&& $this->tokens[$matchExpression]['nested_parenthesis'] === $this->tokens[$start]['nested_parenthesis']))
) {
for ($prevMatch = $start; $prevMatch > $this->tokens[$matchExpression]['scope_opener']; $prevMatch--) {
if ($prevMatch !== $start
&& ($this->tokens[$prevMatch]['code'] === T_MATCH_ARROW
Expand Down
20 changes: 20 additions & 0 deletions tests/Core/File/FindStartOfStatementTest.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,3 +172,23 @@ match ($var) {
},
default => false
};

match ($var) {
/* test437NestedLongArrayWithinMatch */
'a' => array( 1, 2.5, $var),
/* test437NestedFunctionCallWithinMatch */
'b' => functionCall( 11, $var, 50.50),
/* test437NestedArrowFunctionWithinMatch */
'c' => fn($p1, /* test437FnSecondParamWithinMatch */ $p2) => $p1 + $p2,
default => false
};

callMe($paramA, match ($var) {
/* test437NestedLongArrayWithinNestedMatch */
'a' => array( 1, 2.5, $var),
/* test437NestedFunctionCallWithinNestedMatch */
'b' => functionCall( 11, $var, 50.50),
/* test437NestedArrowFunctionWithinNestedMatch */
'c' => fn($p1, /* test437FnSecondParamWithinNestedMatch */ $p2) => $p1 + $p2,
default => false
});
161 changes: 161 additions & 0 deletions tests/Core/File/FindStartOfStatementTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -719,4 +719,165 @@ public static function dataFindStartInsideClosedScopeNestedWithinMatch()
}//end dataFindStartInsideClosedScopeNestedWithinMatch()


/**
* Test finding the start of a statement for a token within a set of parentheses within a match expressions.
*
* @param string $testMarker The comment which prefaces the target token in the test file.
* @param int|string $target The token to search for after the test marker.
* @param int|string $expectedTarget Token code of the expected start of statement stack pointer.
*
* @link https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/437
*
* @dataProvider dataFindStartInsideParenthesesNestedWithinMatch
*
* @return void
*/
public function testFindStartInsideParenthesesNestedWithinMatch($testMarker, $target, $expectedTarget)
{
$testToken = $this->getTargetToken($testMarker, $target);
$expected = $this->getTargetToken($testMarker, $expectedTarget);

$found = self::$phpcsFile->findStartOfStatement($testToken);

$this->assertSame($expected, $found);

}//end testFindStartInsideParenthesesNestedWithinMatch()


/**
* Data provider.
*
* @return array<string, array<string, int|string>>
*/
public static function dataFindStartInsideParenthesesNestedWithinMatch()
{
return [
'Array item itself should be start for first array item' => [
'testMarker' => '/* test437NestedLongArrayWithinMatch */',
'targets' => T_LNUMBER,
'expectedTarget' => T_LNUMBER,
],
'Array item itself should be start for second array item' => [
'testMarker' => '/* test437NestedLongArrayWithinMatch */',
'targets' => T_DNUMBER,
'expectedTarget' => T_DNUMBER,
],
'Array item itself should be start for third array item' => [
'testMarker' => '/* test437NestedLongArrayWithinMatch */',
'targets' => T_VARIABLE,
'expectedTarget' => T_VARIABLE,
],

'Parameter itself should be start for first param passed to function call' => [
'testMarker' => '/* test437NestedFunctionCallWithinMatch */',
'targets' => T_LNUMBER,
'expectedTarget' => T_LNUMBER,
],
'Parameter itself should be start for second param passed to function call' => [
'testMarker' => '/* test437NestedFunctionCallWithinMatch */',
'targets' => T_VARIABLE,
'expectedTarget' => T_VARIABLE,
],
'Parameter itself should be start for third param passed to function call' => [
'testMarker' => '/* test437NestedFunctionCallWithinMatch */',
'targets' => T_DNUMBER,
'expectedTarget' => T_DNUMBER,
],

'Parameter itself should be start for first param declared in arrow function' => [
'testMarker' => '/* test437NestedArrowFunctionWithinMatch */',
'targets' => T_VARIABLE,
'expectedTarget' => T_VARIABLE,
],
'Parameter itself should be start for second param declared in arrow function' => [
'testMarker' => '/* test437FnSecondParamWithinMatch */',
'targets' => T_VARIABLE,
'expectedTarget' => T_VARIABLE,
],
];

}//end dataFindStartInsideParenthesesNestedWithinMatch()


/**
* Test finding the start of a statement for a token within a set of parentheses within a match expressions,
* which itself is nested within parentheses.
*
* @param string $testMarker The comment which prefaces the target token in the test file.
* @param int|string $target The token to search for after the test marker.
* @param int|string $expectedTarget Token code of the expected start of statement stack pointer.
*
* @link https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/437
*
* @dataProvider dataFindStartInsideParenthesesNestedWithinNestedMatch
*
* @return void
*/
public function testFindStartInsideParenthesesNestedWithinNestedMatch($testMarker, $target, $expectedTarget)
{
$testToken = $this->getTargetToken($testMarker, $target);
$expected = $this->getTargetToken($testMarker, $expectedTarget);

$found = self::$phpcsFile->findStartOfStatement($testToken);

$this->assertSame($expected, $found);

}//end testFindStartInsideParenthesesNestedWithinNestedMatch()


/**
* Data provider.
*
* @return array<string, array<string, int|string>>
*/
public static function dataFindStartInsideParenthesesNestedWithinNestedMatch()
{
return [
'Array item itself should be start for first array item' => [
'testMarker' => '/* test437NestedLongArrayWithinNestedMatch */',
'targets' => T_LNUMBER,
'expectedTarget' => T_LNUMBER,
],
'Array item itself should be start for second array item' => [
'testMarker' => '/* test437NestedLongArrayWithinNestedMatch */',
'targets' => T_DNUMBER,
'expectedTarget' => T_DNUMBER,
],
'Array item itself should be start for third array item' => [
'testMarker' => '/* test437NestedLongArrayWithinNestedMatch */',
'targets' => T_VARIABLE,
'expectedTarget' => T_VARIABLE,
],

'Parameter itself should be start for first param passed to function call' => [
'testMarker' => '/* test437NestedFunctionCallWithinNestedMatch */',
'targets' => T_LNUMBER,
'expectedTarget' => T_LNUMBER,
],
'Parameter itself should be start for second param passed to function call' => [
'testMarker' => '/* test437NestedFunctionCallWithinNestedMatch */',
'targets' => T_VARIABLE,
'expectedTarget' => T_VARIABLE,
],
'Parameter itself should be start for third param passed to function call' => [
'testMarker' => '/* test437NestedFunctionCallWithinNestedMatch */',
'targets' => T_DNUMBER,
'expectedTarget' => T_DNUMBER,
],

'Parameter itself should be start for first param declared in arrow function' => [
'testMarker' => '/* test437NestedArrowFunctionWithinNestedMatch */',
'targets' => T_VARIABLE,
'expectedTarget' => T_VARIABLE,
],
'Parameter itself should be start for second param declared in arrow function' => [
'testMarker' => '/* test437FnSecondParamWithinNestedMatch */',
'targets' => T_VARIABLE,
'expectedTarget' => T_VARIABLE,
],
];

}//end dataFindStartInsideParenthesesNestedWithinNestedMatch()


}//end class

0 comments on commit b82438f

Please sign in to comment.