From 2ab89749e50e8854b03df047b52959d9ff4fd424 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Wed, 26 Feb 2025 11:20:52 -0300 Subject: [PATCH 1/2] Tests/Tokenizer: fix bug in the `default` keyword tests 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. --- .../Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php b/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php index 064ce8f407..5bb0b662aa 100644 --- a/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php +++ b/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php @@ -158,7 +158,7 @@ public function testSwitchDefault($testMarker, $openerOffset, $closerOffset, $co if (($opener + 1) !== $closer) { $end = $closer; if (isset($conditionStop) === true) { - $end = $conditionStop; + $end = ($opener + $conditionStop); } for ($i = ($opener + 1); $i < $end; $i++) { @@ -196,7 +196,7 @@ public static function dataSwitchDefault() 'testMarker' => '/* testSimpleSwitchDefaultWithCurlies */', 'openerOffset' => 3, 'closerOffset' => 12, - 'conditionStop' => 6, + 'conditionStop' => 3, ], 'switch_default_toplevel' => [ 'testMarker' => '/* testSwitchDefault */', From 01f5b86abad6d8bf335316a53bd29468c310e9e9 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Thu, 6 Mar 2025 17:32:48 -0300 Subject: [PATCH 2/2] Apply suggestions from code review --- .../RecurseScopeMapDefaultKeywordConditionsTest.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php b/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php index 5bb0b662aa..b8ef8f689f 100644 --- a/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php +++ b/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php @@ -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 @@ -158,7 +159,7 @@ public function testSwitchDefault($testMarker, $openerOffset, $closerOffset, $co if (($opener + 1) !== $closer) { $end = $closer; if (isset($conditionStop) === true) { - $end = ($opener + $conditionStop); + $end = ($token + $conditionStop); } for ($i = ($opener + 1); $i < $end; $i++) { @@ -196,7 +197,7 @@ public static function dataSwitchDefault() 'testMarker' => '/* testSimpleSwitchDefaultWithCurlies */', 'openerOffset' => 3, 'closerOffset' => 12, - 'conditionStop' => 3, + 'conditionStop' => 6, ], 'switch_default_toplevel' => [ 'testMarker' => '/* testSwitchDefault */',