Skip to content

Commit 5e4b32b

Browse files
jrfnljrfnl
committed
Tokenizer/PHP: fix tokens missing/file content being removed by attribute retokenization
This commit fixes the issue detailed in ticket 1279, where in case of an unfinished (parse error) attribute, the contents on the same line as the attribute opener would be removed from the token stream when PHPCS is run on PHP < 8.0. On PHP < 8.0, attributes tokenize as comments and the Tokenizer reparses the contents of the comment to make it a usable token stream. As things were, if the attribute closer could not be found, the `PHP::parsePhpAttribute()` method would return `null` instead of returning the array of (reparsed) tokens which were on the same line as the attribute opener, which meant the "comment content"/attribute content was effectively removed from the token stream. The fix includes changing the return type of the (private) `PHP::parsePhpAttribute()` method from `array|null` to `array`, which also allows for simplifying some code handling the return value of this method. Includes additional tests and improvements to the pre-existing parse error test. Fixes 1279
1 parent a0c8869 commit 5e4b32b

8 files changed

+304
-26
lines changed

‎src/Tokenizers/PHP.php‎

Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1446,15 +1446,11 @@ protected function tokenize($string)
14461446
&& strpos($token[1], '#[') === 0
14471447
) {
14481448
$subTokens = $this->parsePhpAttribute($tokens, $stackPtr);
1449-
if ($subTokens !== null) {
1450-
array_splice($tokens, $stackPtr, 1, $subTokens);
1451-
$numTokens = count($tokens);
1449+
array_splice($tokens, $stackPtr, 1, $subTokens);
1450+
$numTokens = count($tokens);
14521451

1453-
$tokenIsArray = true;
1454-
$token = $tokens[$stackPtr];
1455-
} else {
1456-
$token[0] = T_ATTRIBUTE;
1457-
}
1452+
$tokenIsArray = true;
1453+
$token = $tokens[$stackPtr];
14581454
}
14591455

14601456
if ($tokenIsArray === true
@@ -4105,11 +4101,10 @@ private function findCloser(array &$tokens, $start, $openerTokens, $closerChar)
41054101
* @param array $tokens The original array of tokens (as returned by token_get_all).
41064102
* @param int $stackPtr The current position in token array.
41074103
*
4108-
* @return array|null The array of parsed attribute tokens
4104+
* @return array The array of parsed attribute tokens
41094105
*/
41104106
private function parsePhpAttribute(array &$tokens, $stackPtr)
41114107
{
4112-
41134108
$token = $tokens[$stackPtr];
41144109

41154110
$commentBody = substr($token[1], 2);
@@ -4121,18 +4116,24 @@ private function parsePhpAttribute(array &$tokens, $stackPtr)
41214116
&& strpos($subToken[1], '#[') === 0
41224117
) {
41234118
$reparsed = $this->parsePhpAttribute($subTokens, $i);
4124-
if ($reparsed !== null) {
4125-
array_splice($subTokens, $i, 1, $reparsed);
4126-
} else {
4127-
$subToken[0] = T_ATTRIBUTE;
4128-
}
4119+
array_splice($subTokens, $i, 1, $reparsed);
41294120
}
41304121
}
41314122

41324123
array_splice($subTokens, 0, 1, [[T_ATTRIBUTE, '#[']]);
41334124

41344125
// Go looking for the close bracket.
41354126
$bracketCloser = $this->findCloser($subTokens, 1, '[', ']');
4127+
4128+
/*
4129+
* No closer bracket found, this might be a multi-line attribute,
4130+
* but it could also be an unfinished attribute (parse error).
4131+
*
4132+
* If it is a multi-line attribute, we need to grab a larger part of the code.
4133+
* If it is a parse error, we need to stick with only handling the line
4134+
* containing the attribute opener.
4135+
*/
4136+
41364137
if (PHP_VERSION_ID < 80000 && $bracketCloser === null) {
41374138
foreach (array_slice($tokens, ($stackPtr + 1)) as $token) {
41384139
if (is_array($token) === true) {
@@ -4142,20 +4143,17 @@ private function parsePhpAttribute(array &$tokens, $stackPtr)
41424143
}
41434144
}
41444145

4145-
$subTokens = @token_get_all('<?php '.$commentBody);
4146-
array_splice($subTokens, 0, 1, [[T_ATTRIBUTE, '#[']]);
4146+
$newSubTokens = @token_get_all('<?php '.$commentBody);
4147+
array_splice($newSubTokens, 0, 1, [[T_ATTRIBUTE, '#[']]);
41474148

4148-
$bracketCloser = $this->findCloser($subTokens, 1, '[', ']');
4149+
$bracketCloser = $this->findCloser($newSubTokens, 1, '[', ']');
41494150
if ($bracketCloser !== null) {
4150-
array_splice($tokens, ($stackPtr + 1), count($tokens), array_slice($subTokens, ($bracketCloser + 1)));
4151-
$subTokens = array_slice($subTokens, 0, ($bracketCloser + 1));
4151+
// We found the closer, overwrite the original $subTokens array.
4152+
array_splice($tokens, ($stackPtr + 1), count($tokens), array_slice($newSubTokens, ($bracketCloser + 1)));
4153+
$subTokens = array_slice($newSubTokens, 0, ($bracketCloser + 1));
41524154
}
41534155
}
41544156

4155-
if ($bracketCloser === null) {
4156-
return null;
4157-
}
4158-
41594157
return $subTokens;
41604158

41614159
}//end parsePhpAttribute()

‎tests/Core/Tokenizers/PHP/AttributesParseError1Test.php‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
* Tests the support of PHP 8 attributes
44
*
55
* @author Alessandro Chitolina <[email protected]>
6-
* @copyright 2019 Squiz Pty Ltd (ABN 77 084 670 600)
6+
* @author Juliette Reinders Folmer <[email protected]>
7+
* @copyright 2019-2023 Squiz Pty Ltd (ABN 77 084 670 600)
8+
* @copyright 2023 PHPCSStandards and contributors
79
* @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/HEAD/licence.txt BSD Licence
810
*/
911

@@ -16,7 +18,8 @@ final class AttributesParseError1Test extends AbstractTokenizerTestCase
1618

1719

1820
/**
19-
* Test that invalid attribute (or comment starting with #[ and without ]) are parsed correctly.
21+
* Test that invalid attribute (or comment starting with #[ and without ]) are parsed correctly
22+
* and that tokens "within" the attribute are not removed.
2023
*
2124
* @covers PHP_CodeSniffer\Tokenizers\PHP::tokenize
2225
* @covers PHP_CodeSniffer\Tokenizers\PHP::findCloser
@@ -33,6 +36,30 @@ public function testInvalidAttribute()
3336
$this->assertArrayHasKey('attribute_closer', $tokens[$attribute]);
3437
$this->assertNull($tokens[$attribute]['attribute_closer']);
3538

39+
$expectedTokenCodes = [
40+
'T_ATTRIBUTE',
41+
'T_STRING',
42+
'T_WHITESPACE',
43+
'T_FUNCTION',
44+
];
45+
$length = count($expectedTokenCodes);
46+
47+
$map = array_map(
48+
function ($token) {
49+
if ($token['code'] === T_ATTRIBUTE) {
50+
$this->assertArrayHasKey('attribute_closer', $token);
51+
$this->assertNull($token['attribute_closer']);
52+
} else {
53+
$this->assertArrayNotHasKey('attribute_closer', $token);
54+
}
55+
56+
return $token['type'];
57+
},
58+
array_slice($tokens, $attribute, $length)
59+
);
60+
61+
$this->assertSame($expectedTokenCodes, $map);
62+
3663
}//end testInvalidAttribute()
3764

3865

‎tests/Core/Tokenizers/PHP/AttributesParseError2Test.inc‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
// Intentional parse error.
4+
// This must be the only test in the file.
5+
6+
/* testLiveCoding */
7+
#[AttributeName(10)
8+
function hasUnfinishedAttribute() {}

‎tests/Core/Tokenizers/PHP/AttributesParseError2Test.php‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
<?php
2+
/**
3+
* Tests the support of PHP 8 attributes
4+
*
5+
* @copyright 2025 PHPCSStandards and contributors
6+
* @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/HEAD/licence.txt BSD Licence
7+
*/
8+
9+
namespace PHP_CodeSniffer\Tests\Core\Tokenizers\PHP;
10+
11+
use PHP_CodeSniffer\Tests\Core\Tokenizers\AbstractTokenizerTestCase;
12+
13+
final class AttributesParseError2Test extends AbstractTokenizerTestCase
14+
{
15+
16+
17+
/**
18+
* Test that invalid attribute (or comment starting with #[ and without ]) are parsed correctly
19+
* and that tokens "within" the attribute are not removed.
20+
*
21+
* @covers PHP_CodeSniffer\Tokenizers\PHP::tokenize
22+
* @covers PHP_CodeSniffer\Tokenizers\PHP::findCloser
23+
* @covers PHP_CodeSniffer\Tokenizers\PHP::parsePhpAttribute
24+
*
25+
* @return void
26+
*/
27+
public function testInvalidAttribute()
28+
{
29+
$tokens = $this->phpcsFile->getTokens();
30+
31+
$attribute = $this->getTargetToken('/* testLiveCoding */', T_ATTRIBUTE);
32+
33+
$this->assertArrayHasKey('attribute_closer', $tokens[$attribute]);
34+
$this->assertNull($tokens[$attribute]['attribute_closer']);
35+
36+
$expectedTokenCodes = [
37+
'T_ATTRIBUTE',
38+
'T_STRING',
39+
'T_OPEN_PARENTHESIS',
40+
'T_LNUMBER',
41+
'T_CLOSE_PARENTHESIS',
42+
'T_WHITESPACE',
43+
'T_FUNCTION',
44+
];
45+
46+
$length = count($expectedTokenCodes);
47+
48+
$map = array_map(
49+
function ($token) {
50+
if ($token['code'] === T_ATTRIBUTE) {
51+
$this->assertArrayHasKey('attribute_closer', $token);
52+
$this->assertNull($token['attribute_closer']);
53+
} else {
54+
$this->assertArrayNotHasKey('attribute_closer', $token);
55+
}
56+
57+
return $token['type'];
58+
},
59+
array_slice($tokens, $attribute, $length)
60+
);
61+
62+
$this->assertSame($expectedTokenCodes, $map);
63+
64+
}//end testInvalidAttribute()
65+
66+
67+
}//end class

‎tests/Core/Tokenizers/PHP/AttributesParseError3Test.inc‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
3+
// Intentional parse error.
4+
// This must be the only test in the file.
5+
6+
class LiveCoding {
7+
/* testLiveCoding */
8+
#[AttributeName(10), SecondAttribute(
9+
public final function hasUnfinishedAttribute() {}
10+
}

‎tests/Core/Tokenizers/PHP/AttributesParseError3Test.php‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
<?php
2+
/**
3+
* Tests the support of PHP 8 attributes
4+
*
5+
* @copyright 2025 PHPCSStandards and contributors
6+
* @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/HEAD/licence.txt BSD Licence
7+
*/
8+
9+
namespace PHP_CodeSniffer\Tests\Core\Tokenizers\PHP;
10+
11+
use PHP_CodeSniffer\Tests\Core\Tokenizers\AbstractTokenizerTestCase;
12+
13+
final class AttributesParseError3Test extends AbstractTokenizerTestCase
14+
{
15+
16+
17+
/**
18+
* Test that invalid attribute (or comment starting with #[ and without ]) are parsed correctly
19+
* and that tokens "within" the attribute are not removed.
20+
*
21+
* @covers PHP_CodeSniffer\Tokenizers\PHP::tokenize
22+
* @covers PHP_CodeSniffer\Tokenizers\PHP::findCloser
23+
* @covers PHP_CodeSniffer\Tokenizers\PHP::parsePhpAttribute
24+
*
25+
* @return void
26+
*/
27+
public function testInvalidAttribute()
28+
{
29+
$tokens = $this->phpcsFile->getTokens();
30+
31+
$attribute = $this->getTargetToken('/* testLiveCoding */', T_ATTRIBUTE);
32+
33+
$this->assertArrayHasKey('attribute_closer', $tokens[$attribute]);
34+
$this->assertNull($tokens[$attribute]['attribute_closer']);
35+
36+
$expectedTokenCodes = [
37+
'T_ATTRIBUTE',
38+
'T_STRING',
39+
'T_OPEN_PARENTHESIS',
40+
'T_LNUMBER',
41+
'T_CLOSE_PARENTHESIS',
42+
'T_COMMA',
43+
'T_WHITESPACE',
44+
'T_STRING',
45+
'T_OPEN_PARENTHESIS',
46+
'T_WHITESPACE',
47+
'T_WHITESPACE',
48+
'T_PUBLIC',
49+
'T_WHITESPACE',
50+
'T_FINAL',
51+
'T_WHITESPACE',
52+
'T_FUNCTION',
53+
];
54+
55+
$length = count($expectedTokenCodes);
56+
57+
$map = array_map(
58+
function ($token) {
59+
if ($token['code'] === T_ATTRIBUTE) {
60+
$this->assertArrayHasKey('attribute_closer', $token);
61+
$this->assertNull($token['attribute_closer']);
62+
} else {
63+
$this->assertArrayNotHasKey('attribute_closer', $token);
64+
}
65+
66+
return $token['type'];
67+
},
68+
array_slice($tokens, $attribute, $length)
69+
);
70+
71+
$this->assertSame($expectedTokenCodes, $map);
72+
73+
}//end testInvalidAttribute()
74+
75+
76+
}//end class

‎tests/Core/Tokenizers/PHP/AttributesParseError4Test.inc‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
// Intentional parse error.
4+
// This must be the only test in the file.
5+
6+
/* testLiveCoding */
7+
#[ClosedAttribute] #[UnfinishedAttribute
8+
function hasUnfinishedAttribute() {}

0 commit comments

Comments
 (0)