From 467d284cc36af8606cc8621bbf374e2b12501b39 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 19 May 2024 18:10:11 +0200 Subject: [PATCH 1/4] File::findStartOfStatement(): bug fix - don't give nested scopes the "match" treatment The only "scopes" which can be nested within a `match` expression are closures, anonymous classes and other `match` expressions. 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 another scope nested within the `match`, while, in that case, the special handling is not needed and ends up resulting in an incorrect "start" pointer being returned, 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 `match` expression is the _deepest_ nested scope. Includes unit tests. --- src/Files/File.php | 79 ++++++++++--------- tests/Core/File/FindStartOfStatementTest.inc | 10 +++ tests/Core/File/FindStartOfStatementTest.php | 82 ++++++++++++++++++++ 3 files changed, 134 insertions(+), 37 deletions(-) diff --git a/src/Files/File.php b/src/Files/File.php index c4547ac054..f22ec1d73d 100644 --- a/src/Files/File.php +++ b/src/Files/File.php @@ -2435,51 +2435,56 @@ public function findStartOfStatement($start, $ignore=null) // If the start token is inside the case part of a match expression, // find the start of the condition. If it's in the statement part, find // the token that comes after the match arrow. - $matchExpression = $this->getCondition($start, T_MATCH); - if ($matchExpression !== false) { - for ($prevMatch = $start; $prevMatch > $this->tokens[$matchExpression]['scope_opener']; $prevMatch--) { - if ($prevMatch !== $start - && ($this->tokens[$prevMatch]['code'] === T_MATCH_ARROW - || $this->tokens[$prevMatch]['code'] === T_COMMA) - ) { - break; - } - - // Skip nested statements. - if (isset($this->tokens[$prevMatch]['bracket_opener']) === true - && $prevMatch === $this->tokens[$prevMatch]['bracket_closer'] - ) { - $prevMatch = $this->tokens[$prevMatch]['bracket_opener']; - } else if (isset($this->tokens[$prevMatch]['parenthesis_opener']) === true - && $prevMatch === $this->tokens[$prevMatch]['parenthesis_closer'] - ) { - $prevMatch = $this->tokens[$prevMatch]['parenthesis_opener']; - } - } + if (empty($this->tokens[$start]['conditions']) === false) { + $conditions = $this->tokens[$start]['conditions']; + $lastConditionOwner = end($conditions); + + if ($lastConditionOwner === T_MATCH) { + $matchExpression = key($conditions); + for ($prevMatch = $start; $prevMatch > $this->tokens[$matchExpression]['scope_opener']; $prevMatch--) { + if ($prevMatch !== $start + && ($this->tokens[$prevMatch]['code'] === T_MATCH_ARROW + || $this->tokens[$prevMatch]['code'] === T_COMMA) + ) { + break; + } - if ($prevMatch <= $this->tokens[$matchExpression]['scope_opener']) { - // We're before the arrow in the first case. - $next = $this->findNext(Tokens::$emptyTokens, ($this->tokens[$matchExpression]['scope_opener'] + 1), null, true); - if ($next === false) { - return $start; + // Skip nested statements. + if (isset($this->tokens[$prevMatch]['bracket_opener']) === true + && $prevMatch === $this->tokens[$prevMatch]['bracket_closer'] + ) { + $prevMatch = $this->tokens[$prevMatch]['bracket_opener']; + } else if (isset($this->tokens[$prevMatch]['parenthesis_opener']) === true + && $prevMatch === $this->tokens[$prevMatch]['parenthesis_closer'] + ) { + $prevMatch = $this->tokens[$prevMatch]['parenthesis_opener']; + } } - return $next; - } - - if ($this->tokens[$prevMatch]['code'] === T_COMMA) { - // We're before the arrow, but not in the first case. - $prevMatchArrow = $this->findPrevious(T_MATCH_ARROW, ($prevMatch - 1), $this->tokens[$matchExpression]['scope_opener']); - if ($prevMatchArrow === false) { + if ($prevMatch <= $this->tokens[$matchExpression]['scope_opener']) { // We're before the arrow in the first case. $next = $this->findNext(Tokens::$emptyTokens, ($this->tokens[$matchExpression]['scope_opener'] + 1), null, true); + if ($next === false) { + return $start; + } + return $next; } - $end = $this->findEndOfStatement($prevMatchArrow); - $next = $this->findNext(Tokens::$emptyTokens, ($end + 1), null, true); - return $next; - } + if ($this->tokens[$prevMatch]['code'] === T_COMMA) { + // We're before the arrow, but not in the first case. + $prevMatchArrow = $this->findPrevious(T_MATCH_ARROW, ($prevMatch - 1), $this->tokens[$matchExpression]['scope_opener']); + if ($prevMatchArrow === false) { + // We're before the arrow in the first case. + $next = $this->findNext(Tokens::$emptyTokens, ($this->tokens[$matchExpression]['scope_opener'] + 1), null, true); + return $next; + } + + $end = $this->findEndOfStatement($prevMatchArrow); + $next = $this->findNext(Tokens::$emptyTokens, ($end + 1), null, true); + return $next; + } + }//end if }//end if $lastNotEmpty = $start; diff --git a/tests/Core/File/FindStartOfStatementTest.inc b/tests/Core/File/FindStartOfStatementTest.inc index 148d8103a5..fc33f34084 100644 --- a/tests/Core/File/FindStartOfStatementTest.inc +++ b/tests/Core/File/FindStartOfStatementTest.inc @@ -162,3 +162,13 @@ switch ($foo) { /* testInsideDefaultContinueStatement */ continue $var; } + +match ($var) { + true => + /* test437ClosureDeclaration */ + function ($var) { + /* test437EchoNestedWithinClosureWithinMatch */ + echo $var, 'text', PHP_EOL; + }, + default => false +}; diff --git a/tests/Core/File/FindStartOfStatementTest.php b/tests/Core/File/FindStartOfStatementTest.php index c674a602d1..39823db895 100644 --- a/tests/Core/File/FindStartOfStatementTest.php +++ b/tests/Core/File/FindStartOfStatementTest.php @@ -637,4 +637,86 @@ public static function dataFindStartInsideSwitchCaseDefaultStatements() }//end dataFindStartInsideSwitchCaseDefaultStatements() + /** + * Test finding the start of a statement inside a closed scope nested 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 dataFindStartInsideClosedScopeNestedWithinMatch + * + * @return void + */ + public function testFindStartInsideClosedScopeNestedWithinMatch($testMarker, $target, $expectedTarget) + { + $testToken = $this->getTargetToken($testMarker, $target); + $expected = $this->getTargetToken($testMarker, $expectedTarget); + + $found = self::$phpcsFile->findStartOfStatement($testToken); + + $this->assertSame($expected, $found); + + }//end testFindStartInsideClosedScopeNestedWithinMatch() + + + /** + * Data provider. + * + * @return array> + */ + public static function dataFindStartInsideClosedScopeNestedWithinMatch() + { + return [ + // These were already working correctly. + 'Closure function keyword should be start of closure - closure keyword' => [ + 'testMarker' => '/* test437ClosureDeclaration */', + 'targets' => T_CLOSURE, + 'expectedTarget' => T_CLOSURE, + ], + 'Open curly is a statement/expression opener - open curly' => [ + 'testMarker' => '/* test437ClosureDeclaration */', + 'targets' => T_OPEN_CURLY_BRACKET, + 'expectedTarget' => T_OPEN_CURLY_BRACKET, + ], + + 'Echo should be start for expression - echo keyword' => [ + 'testMarker' => '/* test437EchoNestedWithinClosureWithinMatch */', + 'targets' => T_ECHO, + 'expectedTarget' => T_ECHO, + ], + 'Echo should be start for expression - variable' => [ + 'testMarker' => '/* test437EchoNestedWithinClosureWithinMatch */', + 'targets' => T_VARIABLE, + 'expectedTarget' => T_ECHO, + ], + 'Echo should be start for expression - comma' => [ + 'testMarker' => '/* test437EchoNestedWithinClosureWithinMatch */', + 'targets' => T_COMMA, + 'expectedTarget' => T_ECHO, + ], + + // These were not working correctly and would previously return the close curly of the match expression. + 'First token after comma in echo expression should be start for expression - text string' => [ + 'testMarker' => '/* test437EchoNestedWithinClosureWithinMatch */', + 'targets' => T_CONSTANT_ENCAPSED_STRING, + 'expectedTarget' => T_CONSTANT_ENCAPSED_STRING, + ], + 'First token after comma in echo expression - PHP_EOL constant' => [ + 'testMarker' => '/* test437EchoNestedWithinClosureWithinMatch */', + 'targets' => T_STRING, + 'expectedTarget' => T_STRING, + ], + 'First token after comma in echo expression - semicolon' => [ + 'testMarker' => '/* test437EchoNestedWithinClosureWithinMatch */', + 'targets' => T_SEMICOLON, + 'expectedTarget' => T_STRING, + ], + ]; + + }//end dataFindStartInsideClosedScopeNestedWithinMatch() + + }//end class From b82438f2e1199fb29f4825782dad686168f70352 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 19 May 2024 19:02:33 +0200 Subject: [PATCH 2/4] File::findStartOfStatement(): bug fix - don't give tokens within nested 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. --- src/Files/File.php | 13 +- tests/Core/File/FindStartOfStatementTest.inc | 20 +++ tests/Core/File/FindStartOfStatementTest.php | 161 +++++++++++++++++++ 3 files changed, 191 insertions(+), 3 deletions(-) diff --git a/src/Files/File.php b/src/Files/File.php index f22ec1d73d..66f551a4b6 100644 --- a/src/Files/File.php +++ b/src/Files/File.php @@ -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 diff --git a/tests/Core/File/FindStartOfStatementTest.inc b/tests/Core/File/FindStartOfStatementTest.inc index fc33f34084..1de91f3900 100644 --- a/tests/Core/File/FindStartOfStatementTest.inc +++ b/tests/Core/File/FindStartOfStatementTest.inc @@ -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 +}); diff --git a/tests/Core/File/FindStartOfStatementTest.php b/tests/Core/File/FindStartOfStatementTest.php index 39823db895..5243b13261 100644 --- a/tests/Core/File/FindStartOfStatementTest.php +++ b/tests/Core/File/FindStartOfStatementTest.php @@ -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> + */ + 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> + */ + 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 From d3abcd6c7c83b7cb398f324812c8d1bee3d57bb3 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 19 May 2024 20:43:52 +0200 Subject: [PATCH 3/4] File::findStartOfStatement(): bug fix - don't confuse comma's in short arrays with comma's belonging to a match expression The `File::findStartOfStatement()` method has special handling for `match` expressions to find the start of a statement, but that special handling did not correctly take the potential of comma's in nested short arrays into account. This would lead to any array item after the first getting the wrong return value. As there is currently isn't a "nested_brackets" index in the token array (also see the proposal for this in 12), there is no information available within the token array to prevent the special handling for `match` expressions from kicking in. So, we need to skip _out_ of the special handling if it is detected that the `$start` token is within a short array. In practice, this means we cannot `break` on a `T_COMMA`, as at that point we don't know if it is a comma from within a nested short array. Instead, we have to walk back to the last `T_MATCH_ARROW` to be sure and break out off the special handling if we encounter a `[`/`bracket opener` with a `bracket_closer` which is beyond the `$start` pointer. With these changes, the `$prevMatch` token will now always either be the `match` scope opener or a `T_MATCH_ARROW`. With this knowledge, we can now simplify the special handling for tokens which _do_ belong to the match expression itself a little. Includes unit tests. Of the tests, the first array item was not affected by this bug, but subsequent array items were all affected by this bug. --- src/Files/File.php | 67 ++++++++++++++------ tests/Core/File/FindStartOfStatementTest.inc | 6 ++ tests/Core/File/FindStartOfStatementTest.php | 53 ++++++++++++++++ 3 files changed, 105 insertions(+), 21 deletions(-) diff --git a/src/Files/File.php b/src/Files/File.php index 66f551a4b6..3e1409c58a 100644 --- a/src/Files/File.php +++ b/src/Files/File.php @@ -2448,49 +2448,74 @@ public function findStartOfStatement($start, $ignore=null) && empty($this->tokens[$start]['nested_parenthesis']) === false) && $this->tokens[$matchExpression]['nested_parenthesis'] === $this->tokens[$start]['nested_parenthesis'])) ) { + // Walk back to the previous match arrow (if it exists). + $lastComma = null; + $inNestedExpression = false; for ($prevMatch = $start; $prevMatch > $this->tokens[$matchExpression]['scope_opener']; $prevMatch--) { - if ($prevMatch !== $start - && ($this->tokens[$prevMatch]['code'] === T_MATCH_ARROW - || $this->tokens[$prevMatch]['code'] === T_COMMA) - ) { + if ($prevMatch !== $start && $this->tokens[$prevMatch]['code'] === T_MATCH_ARROW) { break; } + if ($prevMatch !== $start && $this->tokens[$prevMatch]['code'] === T_COMMA) { + $lastComma = $prevMatch; + continue; + } + // Skip nested statements. if (isset($this->tokens[$prevMatch]['bracket_opener']) === true && $prevMatch === $this->tokens[$prevMatch]['bracket_closer'] ) { $prevMatch = $this->tokens[$prevMatch]['bracket_opener']; - } else if (isset($this->tokens[$prevMatch]['parenthesis_opener']) === true + continue; + } + + if (isset($this->tokens[$prevMatch]['parenthesis_opener']) === true && $prevMatch === $this->tokens[$prevMatch]['parenthesis_closer'] ) { $prevMatch = $this->tokens[$prevMatch]['parenthesis_opener']; + continue; } - } - if ($prevMatch <= $this->tokens[$matchExpression]['scope_opener']) { - // We're before the arrow in the first case. - $next = $this->findNext(Tokens::$emptyTokens, ($this->tokens[$matchExpression]['scope_opener'] + 1), null, true); - if ($next === false) { - return $start; + // Stop if we're _within_ a nested short array statement, which may contain comma's too. + // No need to deal with parentheses, those are handled above via the `nested_parenthesis` checks. + if (isset($this->tokens[$prevMatch]['bracket_opener']) === true + && $this->tokens[$prevMatch]['bracket_closer'] > $start + ) { + $inNestedExpression = true; + break; } + }//end for - return $next; - } - - if ($this->tokens[$prevMatch]['code'] === T_COMMA) { - // We're before the arrow, but not in the first case. - $prevMatchArrow = $this->findPrevious(T_MATCH_ARROW, ($prevMatch - 1), $this->tokens[$matchExpression]['scope_opener']); - if ($prevMatchArrow === false) { + if ($inNestedExpression === false) { + // $prevMatch will now either be the scope opener or a match arrow. + // If it is the scope opener, go the first non-empty token after. $start will have been part of the first condition. + if ($prevMatch <= $this->tokens[$matchExpression]['scope_opener']) { // We're before the arrow in the first case. $next = $this->findNext(Tokens::$emptyTokens, ($this->tokens[$matchExpression]['scope_opener'] + 1), null, true); + if ($next === false) { + // Shouldn't be possible. + return $start; + } + return $next; } - $end = $this->findEndOfStatement($prevMatchArrow); - $next = $this->findNext(Tokens::$emptyTokens, ($end + 1), null, true); + // Okay, so we found a match arrow. + // If $start was part of the "next" condition, the last comma will be set. + // Otherwise, $start must have been part of a return expression. + if (isset($lastComma) === true && $lastComma > $prevMatch) { + $prevMatch = $lastComma; + } + + // In both cases, go to the first non-empty token after. + $next = $this->findNext(Tokens::$emptyTokens, ($prevMatch + 1), null, true); + if ($next === false) { + // Shouldn't be possible. + return $start; + } + return $next; - } + }//end if }//end if }//end if diff --git a/tests/Core/File/FindStartOfStatementTest.inc b/tests/Core/File/FindStartOfStatementTest.inc index 1de91f3900..3835d557b6 100644 --- a/tests/Core/File/FindStartOfStatementTest.inc +++ b/tests/Core/File/FindStartOfStatementTest.inc @@ -192,3 +192,9 @@ callMe($paramA, match ($var) { 'c' => fn($p1, /* test437FnSecondParamWithinNestedMatch */ $p2) => $p1 + $p2, default => false }); + +match ($var) { + /* test437NestedShortArrayWithinMatch */ + 'a' => [ 1, 2.5, $var], + default => false +}; diff --git a/tests/Core/File/FindStartOfStatementTest.php b/tests/Core/File/FindStartOfStatementTest.php index 5243b13261..659d0df466 100644 --- a/tests/Core/File/FindStartOfStatementTest.php +++ b/tests/Core/File/FindStartOfStatementTest.php @@ -880,4 +880,57 @@ public static function dataFindStartInsideParenthesesNestedWithinNestedMatch() }//end dataFindStartInsideParenthesesNestedWithinNestedMatch() + /** + * Test finding the start of a statement for a token within a short array 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 dataFindStartInsideShortArrayNestedWithinMatch + * + * @return void + */ + public function testFindStartInsideShortArrayNestedWithinMatch($testMarker, $target, $expectedTarget) + { + $testToken = $this->getTargetToken($testMarker, $target); + $expected = $this->getTargetToken($testMarker, $expectedTarget); + + $found = self::$phpcsFile->findStartOfStatement($testToken); + + $this->assertSame($expected, $found); + + }//end testFindStartInsideShortArrayNestedWithinMatch() + + + /** + * Data provider. + * + * @return array> + */ + public static function dataFindStartInsideShortArrayNestedWithinMatch() + { + return [ + 'Array item itself should be start for first array item' => [ + 'testMarker' => '/* test437NestedShortArrayWithinMatch */', + 'targets' => T_LNUMBER, + 'expectedTarget' => T_LNUMBER, + ], + 'Array item itself should be start for second array item' => [ + 'testMarker' => '/* test437NestedShortArrayWithinMatch */', + 'targets' => T_DNUMBER, + 'expectedTarget' => T_DNUMBER, + ], + 'Array item itself should be start for third array item' => [ + 'testMarker' => '/* test437NestedShortArrayWithinMatch */', + 'targets' => T_VARIABLE, + 'expectedTarget' => T_VARIABLE, + ], + ]; + + }//end dataFindStartInsideShortArrayNestedWithinMatch() + + }//end class From 28c376eff93fccd05a253fa0fa20df54b872e677 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 19 May 2024 20:53:50 +0200 Subject: [PATCH 4/4] Generic/ScopeIndent: add tests for issues 110 and 437 Both the mentioned issues are fixed by the improvements to the `File::findStartOfStatement()` method in this same PR. Fixes 110 Fixes 437 Fixes squizlabs/PHP_CodeSniffer 3875 --- .../WhiteSpace/ScopeIndentUnitTest.1.inc | 35 +++++++++++++++++++ .../ScopeIndentUnitTest.1.inc.fixed | 35 +++++++++++++++++++ .../WhiteSpace/ScopeIndentUnitTest.2.inc | 35 +++++++++++++++++++ .../ScopeIndentUnitTest.2.inc.fixed | 35 +++++++++++++++++++ .../Tests/WhiteSpace/ScopeIndentUnitTest.php | 8 ++--- 5 files changed, 144 insertions(+), 4 deletions(-) diff --git a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc index 4061aff567..bab866e00a 100644 --- a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc +++ b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc @@ -1579,6 +1579,41 @@ foo(function ($foo) { ]; }); +// Issue #110. +echo match (1) { + 0 => match (2) { + 2 => match (3) { + 3 => 3, + default => -1, + }, + }, + 1 => match (2) { + 1 => match (3) { + 3 => 3, + default => -1, + }, + 2 => match (3) { + 3 => 3, + default => -1, + }, + }, +}; + +// Issue #437. +match (true) { + default => [ + 'unrelated' => '', + 'example' => array_filter( + array_map( + function () { + return null; + }, + [] + ) + ) + ] +}; + /* ADD NEW TESTS ABOVE THIS LINE AND MAKE SURE THAT THE 1 (space-based) AND 2 (tab-based) FILES ARE IN SYNC! */ ?> diff --git a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc.fixed b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc.fixed index 7b5efea36a..dbbfa71c40 100644 --- a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc.fixed +++ b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc.fixed @@ -1579,6 +1579,41 @@ foo(function ($foo) { ]; }); +// Issue #110. +echo match (1) { + 0 => match (2) { + 2 => match (3) { + 3 => 3, + default => -1, + }, + }, + 1 => match (2) { + 1 => match (3) { + 3 => 3, + default => -1, + }, + 2 => match (3) { + 3 => 3, + default => -1, + }, + }, +}; + +// Issue #437. +match (true) { + default => [ + 'unrelated' => '', + 'example' => array_filter( + array_map( + function () { + return null; + }, + [] + ) + ) + ] +}; + /* ADD NEW TESTS ABOVE THIS LINE AND MAKE SURE THAT THE 1 (space-based) AND 2 (tab-based) FILES ARE IN SYNC! */ ?> diff --git a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc index e7253141d4..de344f9f6e 100644 --- a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc +++ b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc @@ -1579,6 +1579,41 @@ foo(function ($foo) { ]; }); +// Issue #110. +echo match (1) { + 0 => match (2) { + 2 => match (3) { + 3 => 3, + default => -1, + }, + }, + 1 => match (2) { + 1 => match (3) { + 3 => 3, + default => -1, + }, + 2 => match (3) { + 3 => 3, + default => -1, + }, + }, +}; + +// Issue #437. +match (true) { + default => [ + 'unrelated' => '', + 'example' => array_filter( + array_map( + function () { + return null; + }, + [] + ) + ) + ] +}; + /* ADD NEW TESTS ABOVE THIS LINE AND MAKE SURE THAT THE 1 (space-based) AND 2 (tab-based) FILES ARE IN SYNC! */ ?> diff --git a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc.fixed b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc.fixed index 57caa29175..3cf7fb61a6 100644 --- a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc.fixed +++ b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc.fixed @@ -1579,6 +1579,41 @@ foo(function ($foo) { ]; }); +// Issue #110. +echo match (1) { + 0 => match (2) { + 2 => match (3) { + 3 => 3, + default => -1, + }, + }, + 1 => match (2) { + 1 => match (3) { + 3 => 3, + default => -1, + }, + 2 => match (3) { + 3 => 3, + default => -1, + }, + }, +}; + +// Issue #437. +match (true) { + default => [ + 'unrelated' => '', + 'example' => array_filter( + array_map( + function () { + return null; + }, + [] + ) + ) + ] +}; + /* ADD NEW TESTS ABOVE THIS LINE AND MAKE SURE THAT THE 1 (space-based) AND 2 (tab-based) FILES ARE IN SYNC! */ ?> diff --git a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.php b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.php index 2533b434c0..bb1a5d0ed4 100644 --- a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.php +++ b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.php @@ -192,10 +192,10 @@ public function getErrorList($testFile='') 1527 => 1, 1529 => 1, 1530 => 1, - 1590 => 1, - 1591 => 1, - 1592 => 1, - 1593 => 1, + 1625 => 1, + 1626 => 1, + 1627 => 1, + 1628 => 1, ]; }//end getErrorList()