-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
PHPCS 4.x | Tokenizer: fix stray "parenthesis_..." indexes being set
Commit 08824f3 added support for `T_USE` tokens for closures being parentheses owners. However, the net effect was that *every* `T_USE` token - including import/trait use statement `T_USE` tokens, would have the `parenthesis_owner`, `parenthesis_opener` and `parenthesis_closer` indexes set. For closure `use` statements, those were set correctly. However, for import/trait `use` statements, the `parenthesis_owner` would point to the `T_USE` keyword which doesn't have parentheses and the `parenthesis_opener` and `parenthesis_closer` indexes would be `null` in most cases, but they would still be set. Also, in some cases, the `parenthesis_opener` and `parenthesis_closer` indexes for import/trait `use` statements would incorrectly be set if to arbitrary, unrelated parenthesis, if no open curly or open square bracket was encountered between the `use` statement and the next set of parenthesis. That makes the `parenthesis_...` indexes harder and less intuitive to work with as any of the `parenthesis_...` indexes could be set, even when the token has no parenthesis. I've fixed this now by: * Not just resetting the `$openOwner` variable when we know there won't be any parenthesis, but by also resetting the `parenthesis_...` indexes of the incorrectly set owner. * Doing the reset on more tokens, including doing it on a `T_OPEN_USE_GROUP` token, as the `tokenize()` method has already run, so group use tokens have already been retokenized to their dedicated token. I've also added a few `continue` statements to skip the rest of the code within the loop when we know in advance none of the other conditions would match anyway. Includes adding dedicated unit tests verifying the correct setting of the `parenthesis_...` indexes.
- Loading branch information
Showing
3 changed files
with
245 additions
and
8 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| <?php | ||
|
|
||
| /* testUseImportSimple */ | ||
| use Imported\ClassName; | ||
|
|
||
| $a = (1 + 2); | ||
|
|
||
| /* testUseImportGroup */ | ||
| use Imported\Package\{ | ||
| Classname, | ||
| }; | ||
|
|
||
| $a = (1 + 2); | ||
|
|
||
| $closure = function($param) /* testClosureUse */ use ($var) {}; | ||
|
|
||
| class ClassUsingTrait { | ||
| /* testUseTrait */ | ||
| use MyTopLevelTrait { oldfunction as Foo; } | ||
|
|
||
| public function functionName() { | ||
| $closure = function($param) /* testClosureUseNestedInClass */ use ($var) {} ?> | ||
| <?php | ||
|
|
||
| $anon_class = new class($param) { | ||
| /* testUseTraitInNestedAnonClass */ | ||
| use MyNestedTrait { oldfunction as Foo; } | ||
| }; | ||
|
|
||
| return $anon_class('foo'); | ||
| } | ||
| } | ||
|
|
||
| // Intentional parse error. Live coding. This has to be the last test in the file. | ||
| /* testLiveCoding */ | ||
| use |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,189 @@ | ||
| <?php | ||
| /** | ||
| * Tests the adding of the "parenthesis" keys to closure use tokens. | ||
| * | ||
| * @author Juliette Reinders Folmer <[email protected]> | ||
| * @copyright 2020 Squiz Pty Ltd (ABN 77 084 670 600) | ||
| * @license https://github.com/squizlabs/PHP_CodeSniffer/blob/master/licence.txt BSD Licence | ||
| */ | ||
|
|
||
| namespace PHP_CodeSniffer\Tests\Core\Tokenizer; | ||
|
|
||
| use PHP_CodeSniffer\Tests\Core\AbstractMethodUnitTest; | ||
|
|
||
| class UseParenthesisOwnerTest extends AbstractMethodUnitTest | ||
| { | ||
|
|
||
|
|
||
| /** | ||
| * Test that a non-closure use token does not get assigned the parenthesis_... indexes. | ||
| * | ||
| * @param string $testMarker The comment which prefaces the target token in the test file. | ||
| * | ||
| * @dataProvider dataUseNotClosure | ||
| * @covers PHP_CodeSniffer\Tokenizers\Tokenizer::createTokenMap | ||
| * | ||
| * @return void | ||
| */ | ||
| public function testUseNotClosure($testMarker) | ||
| { | ||
| $tokens = self::$phpcsFile->getTokens(); | ||
|
|
||
| $use = $this->getTargetToken($testMarker, T_USE); | ||
| $this->assertArrayNotHasKey('parenthesis_owner', $tokens[$use]); | ||
| $this->assertArrayNotHasKey('parenthesis_opener', $tokens[$use]); | ||
| $this->assertArrayNotHasKey('parenthesis_closer', $tokens[$use]); | ||
|
|
||
| }//end testUseNotClosure() | ||
|
|
||
|
|
||
| /** | ||
| * Test that the next open/close parenthesis after a non-closure use token | ||
| * do not get assigned the use keyword as a parenthesis owner. | ||
| * | ||
| * @param string $testMarker The comment which prefaces the target token in the test file. | ||
| * @param int|null $expectedOwnerCode Optional. If an owner is expected for the parentheses, the token | ||
| * constant with is expected as the 'code'. | ||
| * If left at the default (null), the parentheses will be tested to | ||
| * *not* have an owner. | ||
| * | ||
| * @dataProvider dataUseNotClosure | ||
| * @covers PHP_CodeSniffer\Tokenizers\Tokenizer::createTokenMap | ||
| * | ||
| * @return void | ||
| */ | ||
| public function testUseNotClosureNextOpenClose($testMarker, $expectedOwnerCode=null) | ||
| { | ||
| $tokens = self::$phpcsFile->getTokens(); | ||
| $opener = $this->getTargetToken($testMarker, T_OPEN_PARENTHESIS); | ||
| $closer = $this->getTargetToken($testMarker, T_CLOSE_PARENTHESIS); | ||
|
|
||
| $this->assertArrayHasKey('parenthesis_opener', $tokens[$opener]); | ||
| $this->assertArrayHasKey('parenthesis_closer', $tokens[$opener]); | ||
| $this->assertSame($opener, $tokens[$opener]['parenthesis_opener']); | ||
| $this->assertSame($closer, $tokens[$opener]['parenthesis_closer']); | ||
|
|
||
| $this->assertArrayHasKey('parenthesis_opener', $tokens[$closer]); | ||
| $this->assertArrayHasKey('parenthesis_closer', $tokens[$closer]); | ||
| $this->assertSame($opener, $tokens[$closer]['parenthesis_opener']); | ||
| $this->assertSame($closer, $tokens[$closer]['parenthesis_closer']); | ||
|
|
||
| if ($expectedOwnerCode === null) { | ||
| $this->assertArrayNotHasKey('parenthesis_owner', $tokens[$opener]); | ||
| $this->assertArrayNotHasKey('parenthesis_owner', $tokens[$closer]); | ||
| } else { | ||
| $this->assertArrayHasKey('parenthesis_owner', $tokens[$opener]); | ||
| $this->assertArrayHasKey('parenthesis_owner', $tokens[$closer]); | ||
| $this->assertSame($expectedOwnerCode, $tokens[$tokens[$opener]['parenthesis_owner']]['code']); | ||
| $this->assertSame($expectedOwnerCode, $tokens[$tokens[$closer]['parenthesis_owner']]['code']); | ||
| } | ||
|
|
||
| }//end testUseNotClosureNextOpenClose() | ||
|
|
||
|
|
||
| /** | ||
| * Data provider. | ||
| * | ||
| * @see testUseNotClosure() | ||
| * @see testUseNotClosureNextOpenClose() | ||
| * | ||
| * @return array | ||
| */ | ||
| public function dataUseNotClosure() | ||
| { | ||
| return [ | ||
| ['/* testUseImportSimple */'], | ||
| ['/* testUseImportGroup */'], | ||
| [ | ||
| '/* testUseTrait */', | ||
| T_FUNCTION, | ||
| ], | ||
| ['/* testUseTraitInNestedAnonClass */'], | ||
| ]; | ||
|
|
||
| }//end dataUseNotClosure() | ||
|
|
||
|
|
||
| /** | ||
| * Test that a closure use token gets assigned a parenthesis owner, opener and closer; | ||
| * and that the opener/closer get the closure use token assigned as owner. | ||
| * | ||
| * @param string $testMarker The comment which prefaces the target token in the test file. | ||
| * | ||
| * @dataProvider dataClosureUse | ||
| * @covers PHP_CodeSniffer\Tokenizers\Tokenizer::createTokenMap | ||
| * | ||
| * @return void | ||
| */ | ||
| public function testClosureUse($testMarker) | ||
| { | ||
| $tokens = self::$phpcsFile->getTokens(); | ||
| $use = $this->getTargetToken($testMarker, T_USE); | ||
| $opener = $this->getTargetToken($testMarker, T_OPEN_PARENTHESIS); | ||
| $closer = $this->getTargetToken($testMarker, T_CLOSE_PARENTHESIS); | ||
|
|
||
| $this->assertArrayHasKey('parenthesis_owner', $tokens[$use]); | ||
| $this->assertArrayHasKey('parenthesis_opener', $tokens[$use]); | ||
| $this->assertArrayHasKey('parenthesis_closer', $tokens[$use]); | ||
| $this->assertSame($use, $tokens[$use]['parenthesis_owner']); | ||
| $this->assertSame($opener, $tokens[$use]['parenthesis_opener']); | ||
| $this->assertSame($closer, $tokens[$use]['parenthesis_closer']); | ||
|
|
||
| $this->assertArrayHasKey('parenthesis_owner', $tokens[$opener]); | ||
| $this->assertArrayHasKey('parenthesis_opener', $tokens[$opener]); | ||
| $this->assertArrayHasKey('parenthesis_closer', $tokens[$opener]); | ||
| $this->assertSame($use, $tokens[$opener]['parenthesis_owner']); | ||
| $this->assertSame($opener, $tokens[$opener]['parenthesis_opener']); | ||
| $this->assertSame($closer, $tokens[$opener]['parenthesis_closer']); | ||
|
|
||
| $this->assertArrayHasKey('parenthesis_owner', $tokens[$closer]); | ||
| $this->assertArrayHasKey('parenthesis_opener', $tokens[$closer]); | ||
| $this->assertArrayHasKey('parenthesis_closer', $tokens[$closer]); | ||
| $this->assertSame($use, $tokens[$closer]['parenthesis_owner']); | ||
| $this->assertSame($opener, $tokens[$closer]['parenthesis_opener']); | ||
| $this->assertSame($closer, $tokens[$closer]['parenthesis_closer']); | ||
|
|
||
| }//end testClosureUse() | ||
|
|
||
|
|
||
| /** | ||
| * Data provider. | ||
| * | ||
| * @see testClosureUse() | ||
| * | ||
| * @return array | ||
| */ | ||
| public function dataClosureUse() | ||
| { | ||
| return [ | ||
| ['/* testClosureUse */'], | ||
| ['/* testClosureUseNestedInClass */'], | ||
| ]; | ||
|
|
||
| }//end dataClosureUse() | ||
|
|
||
|
|
||
| /** | ||
| * Test (and document) the behaviour of the parentheses setting during live coding, when a | ||
| * `use` token is encountered at the very end of a file. | ||
| * | ||
| * @covers PHP_CodeSniffer\Tokenizers\Tokenizer::createTokenMap | ||
| * | ||
| * @return void | ||
| */ | ||
| public function testLiveCoding() | ||
| { | ||
| $tokens = self::$phpcsFile->getTokens(); | ||
| $use = $this->getTargetToken('/* testLiveCoding */', T_USE); | ||
|
|
||
| $this->assertArrayHasKey('parenthesis_owner', $tokens[$use]); | ||
| $this->assertArrayHasKey('parenthesis_opener', $tokens[$use]); | ||
| $this->assertArrayHasKey('parenthesis_closer', $tokens[$use]); | ||
| $this->assertSame($use, $tokens[$use]['parenthesis_owner']); | ||
| $this->assertNull($tokens[$use]['parenthesis_opener']); | ||
| $this->assertNull($tokens[$use]['parenthesis_closer']); | ||
|
|
||
| }//end testLiveCoding() | ||
|
|
||
|
|
||
| }//end class |