Skip to content

Commit

Permalink
Fix mis-identification of 'readonly' keyword
Browse files Browse the repository at this point in the history
PHP 8.2 introduces disjunctive normal form types, which use parentheses, which invalidates the previous "special casing" for function/method declarations and calls using the `readonly` keyword.

This commit fixes this.

Note: this does not (yet) add support for DNF types to the tokenizer or anywhere else in PHPCS, it only fixes the tokenization of `readonly`.

Includes additional tests.

Ref: php/php-src@08b7539

Co-authored-by: Dan Wallis <dan@wallis.nz>
  • Loading branch information
jrfnl and fredden committed Nov 9, 2023
1 parent 8f43103 commit 1858e46
Show file tree
Hide file tree
Showing 3 changed files with 163 additions and 6 deletions.
91 changes: 86 additions & 5 deletions src/Tokenizers/PHP.php
Original file line number Diff line number Diff line change
Expand Up @@ -1313,32 +1313,113 @@ protected function tokenize($string)
"readonly" keyword for PHP < 8.1
*/

if (PHP_VERSION_ID < 80100
&& $tokenIsArray === true
if ($tokenIsArray === true
&& strtolower($token[1]) === 'readonly'
&& isset($this->tstringContexts[$finalTokens[$lastNotEmptyToken]['code']]) === false
) {
// Get the next non-whitespace token.
for ($i = ($stackPtr + 1); $i < $numTokens; $i++) {
if (is_array($tokens[$i]) === false
|| $tokens[$i][0] !== T_WHITESPACE
|| isset(Util\Tokens::$emptyTokens[$tokens[$i][0]]) === false
) {
break;
}
}

$isReadonlyKeyword = false;

if (isset($tokens[$i]) === false
|| $tokens[$i] !== '('
) {
$isReadonlyKeyword = true;
} else if ($tokens[$i] === '(') {
/*
* Skip over tokens which can be used in type declarations.
* At this point, the only token types which need to be taken into consideration
* as potential type declarations are identifier names, T_ARRAY, T_CALLABLE and T_NS_SEPARATOR
* and the union/intersection/dnf parentheses.
*/

$foundDNFParens = 1;
$foundDNFPipe = 0;

for (++$i; $i < $numTokens; $i++) {
if (is_array($tokens[$i]) === true) {
$tokenType = $tokens[$i][0];
} else {
$tokenType = $tokens[$i];
}

if (isset(Util\Tokens::$emptyTokens[$tokenType]) === true) {
continue;
}

if ($tokenType === '|') {
++$foundDNFPipe;
continue;
}

if ($tokenType === ')') {
++$foundDNFParens;
continue;
}

if ($tokenType === '(') {
++$foundDNFParens;
continue;
}

if ($tokenType === T_STRING
|| $tokenType === T_NAME_FULLY_QUALIFIED
|| $tokenType === T_NAME_RELATIVE
|| $tokenType === T_NAME_QUALIFIED
|| $tokenType === T_ARRAY
|| $tokenType === T_NAMESPACE
|| $tokenType === T_NS_SEPARATOR
|| $tokenType === T_AMPERSAND_NOT_FOLLOWED_BY_VAR_OR_VARARG // PHP 8.0+.
|| $tokenType === '&' // PHP < 8.0.
) {
continue;
}

// Reached the next token after.
if (($foundDNFParens % 2) === 0
&& $foundDNFPipe >= 1
&& ($tokenType === T_VARIABLE
|| $tokenType === T_AMPERSAND_FOLLOWED_BY_VAR_OR_VARARG)
) {
$isReadonlyKeyword = true;
}

break;
}//end for
}//end if

if ($isReadonlyKeyword === true) {
$finalTokens[$newStackPtr] = [
'code' => T_READONLY,
'type' => 'T_READONLY',
'content' => $token[1],
];
$newStackPtr++;

continue;
}
if (PHP_CODESNIFFER_VERBOSITY > 1 && $type !== T_READONLY) {
echo "\t\t* token $stackPtr changed from $type to T_READONLY".PHP_EOL;
}
} else {
$finalTokens[$newStackPtr] = [
'code' => T_STRING,
'type' => 'T_STRING',
'content' => $token[1],
];
$newStackPtr++;

if (PHP_CODESNIFFER_VERBOSITY > 1 && $type !== T_STRING) {
echo "\t\t* token $stackPtr changed from $type to T_STRING".PHP_EOL;
}
}//end if

continue;
}//end if

/*
Expand Down
36 changes: 36 additions & 0 deletions tests/Core/Tokenizer/BackfillReadonlyTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,42 @@ echo ClassName::READONLY;
/* testReadonlyUsedAsFunctionCallWithSpaceBetweenKeywordAndParens */
$var = readonly /* comment */ ();

// These test cases are inspired by
// https://github.com/php/php-src/commit/08b75395838b4b42a41e3c70684fa6c6b113eee0
class ReadonlyWithDisjunctiveNormalForm
{
/* testReadonlyPropertyDNFTypeUnqualified */
readonly (B&C)|A $h;

/* testReadonlyPropertyDNFTypeFullyQualified */
public readonly (\Fully\Qualified\B&\Full\C)|\Foo\Bar $j;

/* testReadonlyPropertyDNFTypePartiallyQualified */
protected readonly (Partially\Qualified&C)|A $l;

/* testReadonlyPropertyDNFTypeRelativeName */
private readonly (namespace\Relative&C)|A $n;

/* testReadonlyPropertyDNFTypeMultipleSets */
private readonly (A&C)|(B&C)|(C&D) $m;

/* testReadonlyPropertyDNFTypeWithArray */
private readonly (B & C)|array $o;

/* testReadonlyPropertyDNFTypeWithSpacesAndComments */
private readonly ( B & C /*something*/) | A $q;

public function __construct(
/* testReadonlyConstructorPropertyPromotionWithDNF */
private readonly (B&C)|A $b1,
/* testReadonlyConstructorPropertyPromotionWithDNFAndRefence */
readonly (B&C)|A &$b2,
) {}

/* testReadonlyUsedAsMethodNameWithDNFParam */
public function readonly (A&B $param): void {}
}

/* testParseErrorLiveCoding */
// This must be the last test in the file.
readonly
42 changes: 41 additions & 1 deletion tests/Core/Tokenizer/BackfillReadonlyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,39 @@ public function dataReadonly()
'readonly',
],
[
'/* testReadonlyUsedAsFunctionCallWithSpaceBetweenKeywordAndParens */',
'/* testReadonlyPropertyDNFTypeUnqualified */',
'readonly',
],
[
'/* testReadonlyPropertyDNFTypeFullyQualified */',
'readonly',
],
[
'/* testReadonlyPropertyDNFTypePartiallyQualified */',
'readonly',
],
[
'/* testReadonlyPropertyDNFTypeRelativeName */',
'readonly',
],
[
'/* testReadonlyPropertyDNFTypeMultipleSets */',
'readonly',
],
[
'/* testReadonlyPropertyDNFTypeWithArray */',
'readonly',
],
[
'/* testReadonlyPropertyDNFTypeWithSpacesAndComments */',
'readonly',
],
[
'/* testReadonlyConstructorPropertyPromotionWithDNF */',
'readonly',
],
[
'/* testReadonlyConstructorPropertyPromotionWithDNFAndRefence */',
'readonly',
],
[
Expand Down Expand Up @@ -252,6 +284,14 @@ public function dataNotReadonly()
'/* testClassConstantFetchWithReadonlyAsConstantName */',
'READONLY',
],
[
'/* testReadonlyUsedAsFunctionCallWithSpaceBetweenKeywordAndParens */',
'readonly',
],
[
'/* testReadonlyUsedAsMethodNameWithDNFParam */',
'readonly',
],
];

}//end dataNotReadonly()
Expand Down

0 comments on commit 1858e46

Please sign in to comment.