From b7d9f2e6fe085ccf0064d769d935d1beb709fc8a Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Wed, 26 Feb 2025 11:39:10 -0300 Subject: [PATCH 1/5] Tests/Tokenizer: use markers for the `testSwitchDefault()` test Using markers instead of offsets help stabilize the tests. --- ...seScopeMapDefaultKeywordConditionsTest.inc | 7 +- ...seScopeMapDefaultKeywordConditionsTest.php | 100 +++++++++--------- 2 files changed, 56 insertions(+), 51 deletions(-) diff --git a/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.inc b/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.inc index cad89eec05..c98f518cb9 100644 --- a/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.inc +++ b/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.inc @@ -28,8 +28,10 @@ function switchWithDefaultAndCurlies($i) { case 2: return 2; /* testSimpleSwitchDefaultWithCurlies */ - default: { + default: /* testSimpleSwitchDefaultWithCurliesScopeOpener */ { + /* testSimpleSwitchDefaultWithCurliesConditionStop */ return 'default'; + /* testSimpleSwitchDefaultWithCurliesScopeCloser */ } } } @@ -60,6 +62,7 @@ function matchWithDefaultInSwitch() { /* testMatchDefaultNestedInSwitchDefault */ default, => 'default', }; + /* testSwitchDefaultCloserMarker */ break; } } @@ -97,6 +100,7 @@ function switchAndDefaultSharingScopeCloser($i) { /* testSwitchAndDefaultSharingScopeCloser */ default: echo 'one'; + /* testSwitchAndDefaultSharingScopeCloserScopeCloser */ endswitch; } @@ -111,6 +115,7 @@ function switchDefaultNestedIfWithAndWithoutBraces($i, $foo, $baz) { else { echo 'else'; } + /* testSwitchDefaultNestedIfWithAndWithoutBracesScopeCloser */ break; } } diff --git a/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php b/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php index 1c370a7063..b70ff9c4c3 100644 --- a/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php +++ b/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php @@ -123,30 +123,30 @@ public static function dataMatchDefault() * Note: Cases and default structures within a switch control structure *do* get case/default scope * conditions. * - * @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 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. - * @param bool $sharedScopeCloser Whether to skip checking for the `scope_condition` of the - * scope closer. Needed when the default and switch - * structures share a scope closer. See - * https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/810. + * @param string $testMarker The comment prefacing the target token. + * @param string $openerMarker The comment prefacing the scope opener token. + * @param string $closerMarker The comment prefacing the scope closer token. + * @param string|null $conditionStopMarker The expected offset in relation to the testMarker, after which tokens stop + * having T_DEFAULT as a scope condition. + * @param string $testContent The token content to look for. + * @param bool $sharedScopeCloser Whether to skip checking for the `scope_condition` of the + * scope closer. Needed when the default and switch + * structures share a scope closer. See + * https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/810. * * @dataProvider dataSwitchDefault * @covers PHP_CodeSniffer\Tokenizers\Tokenizer::recurseScopeMap * * @return void */ - public function testSwitchDefault($testMarker, $openerOffset, $closerOffset, $conditionStop=null, $testContent='default', $sharedScopeCloser=false) + public function testSwitchDefault($testMarker, $openerMarker, $closerMarker, $conditionStopMarker=null, $testContent='default', $sharedScopeCloser=false) { $tokens = $this->phpcsFile->getTokens(); $token = $this->getTargetToken($testMarker, [T_MATCH_DEFAULT, T_DEFAULT, T_STRING], $testContent); $tokenArray = $tokens[$token]; - $expectedScopeOpener = ($token + $openerOffset); - $expectedScopeCloser = ($token + $closerOffset); + $expectedScopeOpener = $this->getTargetToken($openerMarker, [T_COLON, T_OPEN_CURLY_BRACKET, T_SEMICOLON]); + $expectedScopeCloser = $this->getTargetToken($closerMarker, [T_BREAK, T_CLOSE_CURLY_BRACKET, T_RETURN, T_ENDSWITCH]); // Make sure we're looking at the right token. $this->assertSame( @@ -178,44 +178,44 @@ public function testSwitchDefault($testMarker, $openerOffset, $closerOffset, $co $this->assertSame( $expectedScopeOpener, $tokenArray['scope_opener'], - sprintf('Scope opener of the T_DEFAULT token incorrect. Marker: %s.', $testMarker) + sprintf('Scope opener of the T_DEFAULT token incorrect. Marker: %s.', $openerMarker) ); $this->assertSame( $expectedScopeCloser, $tokenArray['scope_closer'], - sprintf('Scope closer of the T_DEFAULT token incorrect. Marker: %s.', $testMarker) + sprintf('Scope closer of the T_DEFAULT token incorrect. Marker: %s.', $closerMarker) ); $opener = $tokenArray['scope_opener']; $this->assertArrayHasKey( 'scope_condition', $tokens[$opener], - sprintf('Opener scope condition is not set. Marker: %s.', $testMarker) + sprintf('Opener scope condition is not set. Marker: %s.', $openerMarker) ); $this->assertArrayHasKey( 'scope_opener', $tokens[$opener], - sprintf('Opener scope opener is not set. Marker: %s.', $testMarker) + sprintf('Opener scope opener is not set. Marker: %s.', $openerMarker) ); $this->assertArrayHasKey( 'scope_closer', $tokens[$opener], - sprintf('Opener scope closer is not set. Marker: %s.', $testMarker) + sprintf('Opener scope closer is not set. Marker: %s.', $openerMarker) ); $this->assertSame( $token, $tokens[$opener]['scope_condition'], - sprintf('Opener scope condition is not the T_DEFAULT token. Marker: %s.', $testMarker) + sprintf('Opener scope condition is not the T_DEFAULT token. Marker: %s.', $openerMarker) ); $this->assertSame( $expectedScopeOpener, $tokens[$opener]['scope_opener'], - sprintf('T_DEFAULT opener scope opener token incorrect. Marker: %s.', $testMarker) + sprintf('T_DEFAULT opener scope opener token incorrect. Marker: %s.', $openerMarker) ); $this->assertSame( $expectedScopeCloser, $tokens[$opener]['scope_closer'], - sprintf('T_DEFAULT opener scope closer token incorrect. Marker: %s.', $testMarker) + sprintf('T_DEFAULT opener scope closer token incorrect. Marker: %s.', $openerMarker) ); $closer = $expectedScopeCloser; @@ -225,39 +225,39 @@ public function testSwitchDefault($testMarker, $openerOffset, $closerOffset, $co $this->assertArrayHasKey( 'scope_condition', $tokens[$closer], - sprintf('Closer scope condition is not set. Marker: %s.', $testMarker) + sprintf('Closer scope condition is not set. Marker: %s.', $closerMarker) ); $this->assertArrayHasKey( 'scope_opener', $tokens[$closer], - sprintf('Closer scope opener is not set. Marker: %s.', $testMarker) + sprintf('Closer scope opener is not set. Marker: %s.', $closerMarker) ); $this->assertArrayHasKey( 'scope_closer', $tokens[$closer], - sprintf('Closer scope closer is not set. Marker: %s.', $testMarker) + sprintf('Closer scope closer is not set. Marker: %s.', $closerMarker) ); $this->assertSame( $token, $tokens[$closer]['scope_condition'], - sprintf('Closer scope condition is not the T_DEFAULT token. Marker: %s.', $testMarker) + sprintf('Closer scope condition is not the T_DEFAULT token. Marker: %s.', $closerMarker) ); $this->assertSame( $expectedScopeOpener, $tokens[$closer]['scope_opener'], - sprintf('T_DEFAULT closer scope opener token incorrect. Marker: %s.', $testMarker) + sprintf('T_DEFAULT closer scope opener token incorrect. Marker: %s.', $closerMarker) ); $this->assertSame( $expectedScopeCloser, $tokens[$closer]['scope_closer'], - sprintf('T_DEFAULT closer scope closer token incorrect. Marker: %s.', $testMarker) + sprintf('T_DEFAULT closer scope closer token incorrect. Marker: %s.', $closerMarker) ); }//end if if (($opener + 1) !== $closer) { $end = $closer; - if (isset($conditionStop) === true) { - $end = ($token + $conditionStop + 1); + if (isset($conditionStopMarker) === true) { + $end = ($this->getTargetToken($conditionStopMarker, [T_RETURN]) + 1); } for ($i = ($opener + 1); $i < $end; $i++) { @@ -283,47 +283,47 @@ public static function dataSwitchDefault() { return [ 'simple_switch_default' => [ - 'testMarker' => '/* testSimpleSwitchDefault */', - 'openerOffset' => 1, - 'closerOffset' => 4, + 'testMarker' => '/* testSimpleSwitchDefault */', + 'testOpenerMarker' => '/* testSimpleSwitchDefault */', + 'testCloserMarker' => '/* testSimpleSwitchDefault */', ], 'simple_switch_default_with_curlies' => [ // For a default structure with curly braces, the scope opener // will be the open curly and the closer the close curly. // However, scope conditions will not be set for open to close, // but only for the open token up to the "break/return/continue" etc. - 'testMarker' => '/* testSimpleSwitchDefaultWithCurlies */', - 'openerOffset' => 3, - 'closerOffset' => 12, - 'conditionStop' => 6, + 'testMarker' => '/* testSimpleSwitchDefaultWithCurlies */', + 'testOpenerMarker' => '/* testSimpleSwitchDefaultWithCurliesScopeOpener */', + 'testCloserMarker' => '/* testSimpleSwitchDefaultWithCurliesScopeCloser */', + 'conditionStopMarker' => '/* testSimpleSwitchDefaultWithCurliesConditionStop */', ], 'switch_default_toplevel' => [ - 'testMarker' => '/* testSwitchDefault */', - 'openerOffset' => 1, - 'closerOffset' => 43, + 'testMarker' => '/* testSwitchDefault */', + 'testOpenerMarker' => '/* testSwitchDefault */', + 'testCloserMarker' => '/* testSwitchDefaultCloserMarker */', ], 'switch_default_nested_in_match_case' => [ - 'testMarker' => '/* testSwitchDefaultNestedInMatchCase */', - 'openerOffset' => 1, - 'closerOffset' => 20, + 'testMarker' => '/* testSwitchDefaultNestedInMatchCase */', + 'testOpenerMarker' => '/* testSwitchDefaultNestedInMatchCase */', + 'testCloserMarker' => '/* testSwitchDefaultNestedInMatchCase */', ], 'switch_default_nested_in_match_default' => [ - 'testMarker' => '/* testSwitchDefaultNestedInMatchDefault */', - 'openerOffset' => 1, - 'closerOffset' => 18, + 'testMarker' => '/* testSwitchDefaultNestedInMatchDefault */', + 'testOpenerMarker' => '/* testSwitchDefaultNestedInMatchDefault */', + 'testCloserMarker' => '/* testSwitchDefaultNestedInMatchDefault */', ], 'switch_and_default_sharing_scope_closer' => [ 'testMarker' => '/* testSwitchAndDefaultSharingScopeCloser */', - 'openerOffset' => 1, - 'closerOffset' => 10, + 'testOpenerMarker' => '/* testSwitchAndDefaultSharingScopeCloser */', + 'testCloserMarker' => '/* testSwitchAndDefaultSharingScopeCloserScopeCloser */', 'conditionStop' => null, 'testContent' => 'default', 'sharedScopeCloser' => true, ], 'switch_and_default_with_nested_if_with_and_without_braces' => [ - 'testMarker' => '/* testSwitchDefaultNestedIfWithAndWithoutBraces */', - 'openerOffset' => 1, - 'closerOffset' => 48, + 'testMarker' => '/* testSwitchDefaultNestedIfWithAndWithoutBraces */', + 'testOpenerMarker' => '/* testSwitchDefaultNestedIfWithAndWithoutBraces */', + 'testCloserMarker' => '/* testSwitchDefaultNestedIfWithAndWithoutBracesScopeCloser */', ], ]; From 4eab727000fc1ab2997a848f6569280cf5c89169 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Mon, 31 Mar 2025 16:19:48 -0300 Subject: [PATCH 2/5] Fix: keys in sub-arrays in data provider methods should match the parameter names of the test method --- ...seScopeMapDefaultKeywordConditionsTest.php | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php b/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php index b70ff9c4c3..0c67de2c42 100644 --- a/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php +++ b/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php @@ -283,9 +283,9 @@ public static function dataSwitchDefault() { return [ 'simple_switch_default' => [ - 'testMarker' => '/* testSimpleSwitchDefault */', - 'testOpenerMarker' => '/* testSimpleSwitchDefault */', - 'testCloserMarker' => '/* testSimpleSwitchDefault */', + 'testMarker' => '/* testSimpleSwitchDefault */', + 'openerMarker' => '/* testSimpleSwitchDefault */', + 'closerMarker' => '/* testSimpleSwitchDefault */', ], 'simple_switch_default_with_curlies' => [ // For a default structure with curly braces, the scope opener @@ -293,37 +293,37 @@ public static function dataSwitchDefault() // However, scope conditions will not be set for open to close, // but only for the open token up to the "break/return/continue" etc. 'testMarker' => '/* testSimpleSwitchDefaultWithCurlies */', - 'testOpenerMarker' => '/* testSimpleSwitchDefaultWithCurliesScopeOpener */', - 'testCloserMarker' => '/* testSimpleSwitchDefaultWithCurliesScopeCloser */', + 'openerMarker' => '/* testSimpleSwitchDefaultWithCurliesScopeOpener */', + 'closerMarker' => '/* testSimpleSwitchDefaultWithCurliesScopeCloser */', 'conditionStopMarker' => '/* testSimpleSwitchDefaultWithCurliesConditionStop */', ], 'switch_default_toplevel' => [ - 'testMarker' => '/* testSwitchDefault */', - 'testOpenerMarker' => '/* testSwitchDefault */', - 'testCloserMarker' => '/* testSwitchDefaultCloserMarker */', + 'testMarker' => '/* testSwitchDefault */', + 'openerMarker' => '/* testSwitchDefault */', + 'closerMarker' => '/* testSwitchDefaultCloserMarker */', ], 'switch_default_nested_in_match_case' => [ - 'testMarker' => '/* testSwitchDefaultNestedInMatchCase */', - 'testOpenerMarker' => '/* testSwitchDefaultNestedInMatchCase */', - 'testCloserMarker' => '/* testSwitchDefaultNestedInMatchCase */', + 'testMarker' => '/* testSwitchDefaultNestedInMatchCase */', + 'openerMarker' => '/* testSwitchDefaultNestedInMatchCase */', + 'closerMarker' => '/* testSwitchDefaultNestedInMatchCase */', ], 'switch_default_nested_in_match_default' => [ - 'testMarker' => '/* testSwitchDefaultNestedInMatchDefault */', - 'testOpenerMarker' => '/* testSwitchDefaultNestedInMatchDefault */', - 'testCloserMarker' => '/* testSwitchDefaultNestedInMatchDefault */', + 'testMarker' => '/* testSwitchDefaultNestedInMatchDefault */', + 'openerMarker' => '/* testSwitchDefaultNestedInMatchDefault */', + 'closerMarker' => '/* testSwitchDefaultNestedInMatchDefault */', ], 'switch_and_default_sharing_scope_closer' => [ 'testMarker' => '/* testSwitchAndDefaultSharingScopeCloser */', - 'testOpenerMarker' => '/* testSwitchAndDefaultSharingScopeCloser */', - 'testCloserMarker' => '/* testSwitchAndDefaultSharingScopeCloserScopeCloser */', + 'openerMarker' => '/* testSwitchAndDefaultSharingScopeCloser */', + 'closerMarker' => '/* testSwitchAndDefaultSharingScopeCloserScopeCloser */', 'conditionStop' => null, 'testContent' => 'default', 'sharedScopeCloser' => true, ], 'switch_and_default_with_nested_if_with_and_without_braces' => [ - 'testMarker' => '/* testSwitchDefaultNestedIfWithAndWithoutBraces */', - 'testOpenerMarker' => '/* testSwitchDefaultNestedIfWithAndWithoutBraces */', - 'testCloserMarker' => '/* testSwitchDefaultNestedIfWithAndWithoutBracesScopeCloser */', + 'testMarker' => '/* testSwitchDefaultNestedIfWithAndWithoutBraces */', + 'openerMarker' => '/* testSwitchDefaultNestedIfWithAndWithoutBraces */', + 'closerMarker' => '/* testSwitchDefaultNestedIfWithAndWithoutBracesScopeCloser */', ], ]; From c4cfc92d71d3380b082d768d10952f0f0e7d9a23 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Fri, 4 Apr 2025 10:52:23 -0300 Subject: [PATCH 3/5] Fix: marker passed to the error message of two assertions --- .../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 0c67de2c42..332418750d 100644 --- a/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php +++ b/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php @@ -178,12 +178,12 @@ public function testSwitchDefault($testMarker, $openerMarker, $closerMarker, $co $this->assertSame( $expectedScopeOpener, $tokenArray['scope_opener'], - sprintf('Scope opener of the T_DEFAULT token incorrect. Marker: %s.', $openerMarker) + sprintf('Scope opener of the T_DEFAULT token incorrect. Marker: %s.', $testMarker) ); $this->assertSame( $expectedScopeCloser, $tokenArray['scope_closer'], - sprintf('Scope closer of the T_DEFAULT token incorrect. Marker: %s.', $closerMarker) + sprintf('Scope closer of the T_DEFAULT token incorrect. Marker: %s.', $testMarker) ); $opener = $tokenArray['scope_opener']; From 7a19f8a22a292dad64462f5533c54ec7eaa5a9fe Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Mon, 7 Apr 2025 09:45:10 -0300 Subject: [PATCH 4/5] Include all possible token types when looking for the condition stop token --- .../RecurseScopeMapDefaultKeywordConditionsTest.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php b/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php index 332418750d..0591f3c69a 100644 --- a/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php +++ b/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php @@ -257,7 +257,15 @@ public function testSwitchDefault($testMarker, $openerMarker, $closerMarker, $co if (($opener + 1) !== $closer) { $end = $closer; if (isset($conditionStopMarker) === true) { - $end = ($this->getTargetToken($conditionStopMarker, [T_RETURN]) + 1); + $tokenTypes = [ + T_BREAK, + T_CONTINUE, + T_EXIT, + T_GOTO, + T_RETURN, + T_THROW, + ]; + $end = ($this->getTargetToken($conditionStopMarker, $tokenTypes) + 1); } for ($i = ($opener + 1); $i < $end; $i++) { @@ -267,7 +275,7 @@ public function testSwitchDefault($testMarker, $openerMarker, $closerMarker, $co sprintf('T_DEFAULT condition not added for token belonging to the T_DEFAULT structure. Marker: %s.', $testMarker) ); } - } + }//end if }//end testSwitchDefault() From 386f8fa917217f8403da20176aa4ae7c8a5cb8bf Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Mon, 7 Apr 2025 11:43:01 -0300 Subject: [PATCH 5/5] Move list of condition stop tokens to a class property --- ...seScopeMapDefaultKeywordConditionsTest.php | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php b/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php index 0591f3c69a..8cce3bc9e4 100644 --- a/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php +++ b/tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php @@ -14,6 +14,20 @@ final class RecurseScopeMapDefaultKeywordConditionsTest extends AbstractTokenizerTestCase { + /** + * Condition stop tokens when `default` is used with curlies. + * + * @var array + */ + protected $conditionStopTokens = [ + T_BREAK, + T_CONTINUE, + T_EXIT, + T_GOTO, + T_RETURN, + T_THROW, + ]; + /** * Test that match "default" tokens does not get scope indexes. @@ -257,15 +271,7 @@ public function testSwitchDefault($testMarker, $openerMarker, $closerMarker, $co if (($opener + 1) !== $closer) { $end = $closer; if (isset($conditionStopMarker) === true) { - $tokenTypes = [ - T_BREAK, - T_CONTINUE, - T_EXIT, - T_GOTO, - T_RETURN, - T_THROW, - ]; - $end = ($this->getTargetToken($conditionStopMarker, $tokenTypes) + 1); + $end = ( $this->getTargetToken($conditionStopMarker, $this->conditionStopTokens) + 1); } for ($i = ($opener + 1); $i < $end; $i++) {