Skip to content

Commit

Permalink
Tests/Tokenizer: fix bug in the default keyword tests (#850)
Browse files Browse the repository at this point in the history
This commit fixes a bug in `RecurseScopeMapDefaultKeywordConditionsTest
::testSwitchDefault()`. Among other things, this method checks whether
tokens within the scope of a `default` have `T_DEFAULT` set as the scope
condition.

But the code would never check if the scope condition is set for the
tokens within the scope of a `T_DEFAULT` token when `$conditionStop` is
not `null`. `$conditionStop` is used when `T_DEFAULT` uses curlies to
open and close the scope. In those cases, scope conditions are not set
all the way until the scope closer. Instead, they are set only until
just before a T_BREAK|T_RETURN|T_CONTINUE.

`simple_switch_default_with_curlies` test case is the only one that
currently sets `$conditionStop` to something other than `null`. But it
passes an offset value of the condition stop while the code was
expecting an absolute value of the index of the token where the scope
condition stops being set. Passing an offset meant that when `$end` was
set to be equal to `$conditionStop` it would always be less than `$i`
and the assertion inside the `for` loop would never run:

https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/be74da1a2727948c35e8862cc378fcf9e9345b42/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php#L160-L170

Now the code calculates the correct value for `$end` when
`$conditionStop` is not null. It was also necessary to update the value
for `$conditionStop` in `simple_switch_default_with_curlies` as it was
pointing to the wrong token.

This uncovered an inconsistency in the tokenizer that might need to be
addressed in a separate issue. When `T_DEFAULT` doesn't use curlies the
tokenizer sets `scope_condition` for all the tokens until the token just
before `T_BREAK|T_RETURN|T_CONTINUE`. But when there are curlies,
`scope_condition` is set including for `T_BREAK|T_RETURN|T_CONTINUE`. It
needs to be determined if this behavior is intentional or not.
  • Loading branch information
rodrigoprimo authored Mar 7, 2025
1 parent 8adbcb2 commit dc3a639
Showing 1 changed file with 3 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ public static function dataMatchDefault()
* @param string $testMarker The comment prefacing the target token.
* @param int $openerOffset The expected offset of the scope opener in relation to the testMarker.
* @param int $closerOffset The expected offset of the scope closer in relation to the testMarker.
* @param int|null $conditionStop The expected offset at which tokens stop having T_DEFAULT as a scope condition.
* @param int|null $conditionStop The expected offset in relation to the testMarker, at which tokens stop
* having T_DEFAULT as a scope condition.
* @param string $testContent The token content to look for.
*
* @dataProvider dataSwitchDefault
Expand Down Expand Up @@ -158,7 +159,7 @@ public function testSwitchDefault($testMarker, $openerOffset, $closerOffset, $co
if (($opener + 1) !== $closer) {
$end = $closer;
if (isset($conditionStop) === true) {
$end = $conditionStop;
$end = ($token + $conditionStop);
}

for ($i = ($opener + 1); $i < $end; $i++) {
Expand Down

0 comments on commit dc3a639

Please sign in to comment.