Skip to content

Commit 44fba8d

Browse files
jrfnljrfnl
committed
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.
1 parent 014c3d0 commit 44fba8d

File tree

3 files changed

+245
-8
lines changed

3 files changed

+245
-8
lines changed

‎src/Tokenizers/Tokenizer.php‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -651,6 +651,8 @@ private function createTokenMap()
651651
if (PHP_CODESNIFFER_VERBOSITY > 1) {
652652
Common::printStatusMessage("=> Found parenthesis owner at $i", (count($openers) + 1));
653653
}
654+
655+
continue;
654656
} else if ($this->tokens[$i]['code'] === T_OPEN_PARENTHESIS) {
655657
$openers[] = $i;
656658
$this->tokens[$i]['parenthesis_opener'] = $i;
@@ -665,6 +667,8 @@ private function createTokenMap()
665667
} else if (PHP_CODESNIFFER_VERBOSITY > 1) {
666668
Common::printStatusMessage("=> Found unowned parenthesis opener at $i", count($openers));
667669
}
670+
671+
continue;
668672
} else if ($this->tokens[$i]['code'] === T_CLOSE_PARENTHESIS) {
669673
// Did we set an owner for this set of parenthesis?
670674
$numOpeners = count($openers);
@@ -721,6 +725,22 @@ private function createTokenMap()
721725
echo "=> Found unowned attribute closer at $i for $opener".PHP_EOL;
722726
}
723727
}//end if
728+
729+
continue;
730+
} else if ($openOwner !== null
731+
&& ($this->tokens[$i]['code'] === T_OPEN_CURLY_BRACKET
732+
|| $this->tokens[$i]['code'] === T_OPEN_SQUARE_BRACKET
733+
|| $this->tokens[$i]['code'] === T_OPEN_USE_GROUP
734+
|| $this->tokens[$i]['code'] === T_SEMICOLON
735+
|| $this->tokens[$i]['code'] === T_CLOSE_TAG)
736+
) {
737+
// This must have been a T_USE for an import/trait use statement, not a closure.
738+
unset(
739+
$this->tokens[$openOwner]['parenthesis_opener'],
740+
$this->tokens[$openOwner]['parenthesis_closer'],
741+
$this->tokens[$openOwner]['parenthesis_owner']
742+
);
743+
$openOwner = null;
724744
}//end if
725745

726746
/*
@@ -734,10 +754,6 @@ private function createTokenMap()
734754
if (PHP_CODESNIFFER_VERBOSITY > 1) {
735755
Common::printStatusMessage("=> Found square bracket opener at $i", (count($squareOpeners) + count($curlyOpeners)));
736756
}
737-
738-
if ($openOwner !== null) {
739-
$openOwner = null;
740-
}
741757
break;
742758
case T_OPEN_CURLY_BRACKET:
743759
if (isset($this->tokens[$i]['scope_closer']) === false) {
@@ -747,10 +763,6 @@ private function createTokenMap()
747763
Common::printStatusMessage("=> Found curly bracket opener at $i", (count($squareOpeners) + count($curlyOpeners)));
748764
}
749765
}
750-
751-
if ($openOwner !== null) {
752-
$openOwner = null;
753-
}
754766
break;
755767
case T_CLOSE_SQUARE_BRACKET:
756768
if (empty($squareOpeners) === false) {

‎tests/Core/Tokenizer/UseParenthesisOwnerTest.inc‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php
2+
3+
/* testUseImportSimple */
4+
use Imported\ClassName;
5+
6+
$a = (1 + 2);
7+
8+
/* testUseImportGroup */
9+
use Imported\Package\{
10+
Classname,
11+
};
12+
13+
$a = (1 + 2);
14+
15+
$closure = function($param) /* testClosureUse */ use ($var) {};
16+
17+
class ClassUsingTrait {
18+
/* testUseTrait */
19+
use MyTopLevelTrait { oldfunction as Foo; }
20+
21+
public function functionName() {
22+
$closure = function($param) /* testClosureUseNestedInClass */ use ($var) {} ?>
23+
<?php
24+
25+
$anon_class = new class($param) {
26+
/* testUseTraitInNestedAnonClass */
27+
use MyNestedTrait { oldfunction as Foo; }
28+
};
29+
30+
return $anon_class('foo');
31+
}
32+
}
33+
34+
// Intentional parse error. Live coding. This has to be the last test in the file.
35+
/* testLiveCoding */
36+
use

‎tests/Core/Tokenizer/UseParenthesisOwnerTest.php‎

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
<?php
2+
/**
3+
* Tests the adding of the "parenthesis" keys to closure use tokens.
4+
*
5+
* @author Juliette Reinders Folmer <phpcs_nospam@adviesenzo.nl>
6+
* @copyright 2020 Squiz Pty Ltd (ABN 77 084 670 600)
7+
* @license https://github.com/squizlabs/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
8+
*/
9+
10+
namespace PHP_CodeSniffer\Tests\Core\Tokenizer;
11+
12+
use PHP_CodeSniffer\Tests\Core\AbstractMethodUnitTest;
13+
14+
class UseParenthesisOwnerTest extends AbstractMethodUnitTest
15+
{
16+
17+
18+
/**
19+
* Test that a non-closure use token does not get assigned the parenthesis_... indexes.
20+
*
21+
* @param string $testMarker The comment which prefaces the target token in the test file.
22+
*
23+
* @dataProvider dataUseNotClosure
24+
* @covers PHP_CodeSniffer\Tokenizers\Tokenizer::createTokenMap
25+
*
26+
* @return void
27+
*/
28+
public function testUseNotClosure($testMarker)
29+
{
30+
$tokens = self::$phpcsFile->getTokens();
31+
32+
$use = $this->getTargetToken($testMarker, T_USE);
33+
$this->assertArrayNotHasKey('parenthesis_owner', $tokens[$use]);
34+
$this->assertArrayNotHasKey('parenthesis_opener', $tokens[$use]);
35+
$this->assertArrayNotHasKey('parenthesis_closer', $tokens[$use]);
36+
37+
}//end testUseNotClosure()
38+
39+
40+
/**
41+
* Test that the next open/close parenthesis after a non-closure use token
42+
* do not get assigned the use keyword as a parenthesis owner.
43+
*
44+
* @param string $testMarker The comment which prefaces the target token in the test file.
45+
* @param int|null $expectedOwnerCode Optional. If an owner is expected for the parentheses, the token
46+
* constant with is expected as the 'code'.
47+
* If left at the default (null), the parentheses will be tested to
48+
* *not* have an owner.
49+
*
50+
* @dataProvider dataUseNotClosure
51+
* @covers PHP_CodeSniffer\Tokenizers\Tokenizer::createTokenMap
52+
*
53+
* @return void
54+
*/
55+
public function testUseNotClosureNextOpenClose($testMarker, $expectedOwnerCode=null)
56+
{
57+
$tokens = self::$phpcsFile->getTokens();
58+
$opener = $this->getTargetToken($testMarker, T_OPEN_PARENTHESIS);
59+
$closer = $this->getTargetToken($testMarker, T_CLOSE_PARENTHESIS);
60+
61+
$this->assertArrayHasKey('parenthesis_opener', $tokens[$opener]);
62+
$this->assertArrayHasKey('parenthesis_closer', $tokens[$opener]);
63+
$this->assertSame($opener, $tokens[$opener]['parenthesis_opener']);
64+
$this->assertSame($closer, $tokens[$opener]['parenthesis_closer']);
65+
66+
$this->assertArrayHasKey('parenthesis_opener', $tokens[$closer]);
67+
$this->assertArrayHasKey('parenthesis_closer', $tokens[$closer]);
68+
$this->assertSame($opener, $tokens[$closer]['parenthesis_opener']);
69+
$this->assertSame($closer, $tokens[$closer]['parenthesis_closer']);
70+
71+
if ($expectedOwnerCode === null) {
72+
$this->assertArrayNotHasKey('parenthesis_owner', $tokens[$opener]);
73+
$this->assertArrayNotHasKey('parenthesis_owner', $tokens[$closer]);
74+
} else {
75+
$this->assertArrayHasKey('parenthesis_owner', $tokens[$opener]);
76+
$this->assertArrayHasKey('parenthesis_owner', $tokens[$closer]);
77+
$this->assertSame($expectedOwnerCode, $tokens[$tokens[$opener]['parenthesis_owner']]['code']);
78+
$this->assertSame($expectedOwnerCode, $tokens[$tokens[$closer]['parenthesis_owner']]['code']);
79+
}
80+
81+
}//end testUseNotClosureNextOpenClose()
82+
83+
84+
/**
85+
* Data provider.
86+
*
87+
* @see testUseNotClosure()
88+
* @see testUseNotClosureNextOpenClose()
89+
*
90+
* @return array
91+
*/
92+
public function dataUseNotClosure()
93+
{
94+
return [
95+
['/* testUseImportSimple */'],
96+
['/* testUseImportGroup */'],
97+
[
98+
'/* testUseTrait */',
99+
T_FUNCTION,
100+
],
101+
['/* testUseTraitInNestedAnonClass */'],
102+
];
103+
104+
}//end dataUseNotClosure()
105+
106+
107+
/**
108+
* Test that a closure use token gets assigned a parenthesis owner, opener and closer;
109+
* and that the opener/closer get the closure use token assigned as owner.
110+
*
111+
* @param string $testMarker The comment which prefaces the target token in the test file.
112+
*
113+
* @dataProvider dataClosureUse
114+
* @covers PHP_CodeSniffer\Tokenizers\Tokenizer::createTokenMap
115+
*
116+
* @return void
117+
*/
118+
public function testClosureUse($testMarker)
119+
{
120+
$tokens = self::$phpcsFile->getTokens();
121+
$use = $this->getTargetToken($testMarker, T_USE);
122+
$opener = $this->getTargetToken($testMarker, T_OPEN_PARENTHESIS);
123+
$closer = $this->getTargetToken($testMarker, T_CLOSE_PARENTHESIS);
124+
125+
$this->assertArrayHasKey('parenthesis_owner', $tokens[$use]);
126+
$this->assertArrayHasKey('parenthesis_opener', $tokens[$use]);
127+
$this->assertArrayHasKey('parenthesis_closer', $tokens[$use]);
128+
$this->assertSame($use, $tokens[$use]['parenthesis_owner']);
129+
$this->assertSame($opener, $tokens[$use]['parenthesis_opener']);
130+
$this->assertSame($closer, $tokens[$use]['parenthesis_closer']);
131+
132+
$this->assertArrayHasKey('parenthesis_owner', $tokens[$opener]);
133+
$this->assertArrayHasKey('parenthesis_opener', $tokens[$opener]);
134+
$this->assertArrayHasKey('parenthesis_closer', $tokens[$opener]);
135+
$this->assertSame($use, $tokens[$opener]['parenthesis_owner']);
136+
$this->assertSame($opener, $tokens[$opener]['parenthesis_opener']);
137+
$this->assertSame($closer, $tokens[$opener]['parenthesis_closer']);
138+
139+
$this->assertArrayHasKey('parenthesis_owner', $tokens[$closer]);
140+
$this->assertArrayHasKey('parenthesis_opener', $tokens[$closer]);
141+
$this->assertArrayHasKey('parenthesis_closer', $tokens[$closer]);
142+
$this->assertSame($use, $tokens[$closer]['parenthesis_owner']);
143+
$this->assertSame($opener, $tokens[$closer]['parenthesis_opener']);
144+
$this->assertSame($closer, $tokens[$closer]['parenthesis_closer']);
145+
146+
}//end testClosureUse()
147+
148+
149+
/**
150+
* Data provider.
151+
*
152+
* @see testClosureUse()
153+
*
154+
* @return array
155+
*/
156+
public function dataClosureUse()
157+
{
158+
return [
159+
['/* testClosureUse */'],
160+
['/* testClosureUseNestedInClass */'],
161+
];
162+
163+
}//end dataClosureUse()
164+
165+
166+
/**
167+
* Test (and document) the behaviour of the parentheses setting during live coding, when a
168+
* `use` token is encountered at the very end of a file.
169+
*
170+
* @covers PHP_CodeSniffer\Tokenizers\Tokenizer::createTokenMap
171+
*
172+
* @return void
173+
*/
174+
public function testLiveCoding()
175+
{
176+
$tokens = self::$phpcsFile->getTokens();
177+
$use = $this->getTargetToken('/* testLiveCoding */', T_USE);
178+
179+
$this->assertArrayHasKey('parenthesis_owner', $tokens[$use]);
180+
$this->assertArrayHasKey('parenthesis_opener', $tokens[$use]);
181+
$this->assertArrayHasKey('parenthesis_closer', $tokens[$use]);
182+
$this->assertSame($use, $tokens[$use]['parenthesis_owner']);
183+
$this->assertNull($tokens[$use]['parenthesis_opener']);
184+
$this->assertNull($tokens[$use]['parenthesis_closer']);
185+
186+
}//end testLiveCoding()
187+
188+
189+
}//end class

0 commit comments

Comments
 (0)