Skip to content

Commit e163dbf

Browse files
andrewnicolsjrfnl
andauthored
PHP 8.5 | Tokenizer/PHP: properly fix "Using null as an array offset" deprecation (#1230)
If a slash/hash comment is on its own line, the new line token is merged into the comment token for PHP 8.0+, and the new line is skipped by setting it to `null`. The PHP Tokenizer did not take the possibility of a token being set to `null` into account when retokening a `?` to either `T_NULLABLE` or `T_INLINE_THEN`, leading to the PHP 8.5 deprecation notice. In that case, the sniff first looks forward to see if we can draw a conclusion based on the non-empty tokens following the `?` and if not, walks backward to look at the tokens before the `?`. The problem occurs when a `null` token exists in the sequence before the `?`. This `null` token will only have been injected with a specific code sequence: when there is a slash/hash comment followed by a new line in the token sequence before the `?` and there is no indentation/new line whitespace on the next line. Also see the detailed analysis by andrewnicols in PHPCSStandards/PHP_CodeSniffer 1216#issuecomment-3274198443 There are only two situations in which this causes the tokenizer to hit the PHP 8.5 "Using null as an array offset" deprecation notice: 1. If the `?` token is the last token in the file (live coding - the issue was discovered via a test related to live coding). 2. If there are tokens after the `?`, but not tokens which allows us to draw a conclusion (yet) and there is a slash/hash comment between the `?` and the previous non-empty token. This commit: 1. Adds dedicated tests for both situations. 2. Adds a new fix for situation [1] as if there are no tokens after a `?`, we cannot determine definitively whether the `?` is a nullable operator or an inline then. For BC, this should be tokenized as `T_INLINE_THEN`. 3. Makes a small tweak to the previously added fix which addressed situation [2]. Alternatively, we could consider switching to using the "$finalTokens" token stack to do the token walking instead, but as that is a bigger change and this part of the code has nowhere near sufficient tests, that change is left for later. Includes reverting the error silencing which was put in place temporarily to allow for the issue to be investigated. Fixes 1216 Ref: https://wiki.php.net/rfc/deprecations_php_8_5#deprecate_using_values_null_as_an_array_offset_and_when_calling_array_key_exists --------- Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
1 parent af1b003 commit e163dbf

File tree

5 files changed

+92
-15
lines changed

5 files changed

+92
-15
lines changed

src/Tokenizers/PHP.php

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2240,6 +2240,24 @@ protected function tokenize($string)
22402240
break;
22412241
}//end for
22422242

2243+
// Handle live coding/parse errors elegantly.
2244+
// If the "?" is the last non-empty token in the file, we cannot draw a definitive conclusion,
2245+
// so tokenize as T_INLINE_THEN.
2246+
if ($i === $numTokens) {
2247+
if (PHP_CODESNIFFER_VERBOSITY > 1) {
2248+
echo "\t\t* token $stackPtr at end of file changed from ? to T_INLINE_THEN".PHP_EOL;
2249+
}
2250+
2251+
$newToken['code'] = T_INLINE_THEN;
2252+
$newToken['type'] = 'T_INLINE_THEN';
2253+
2254+
$insideInlineIf[] = $stackPtr;
2255+
2256+
$finalTokens[$newStackPtr] = $newToken;
2257+
$newStackPtr++;
2258+
continue;
2259+
}
2260+
22432261
/*
22442262
* This can still be a nullable type or a ternary.
22452263
* Do additional checking.
@@ -2249,6 +2267,11 @@ protected function tokenize($string)
22492267
$lastSeenNonEmpty = null;
22502268

22512269
for ($i = ($stackPtr - 1); $i >= 0; $i--) {
2270+
if (isset($tokens[$i]) === false) {
2271+
// Ignore skipped tokens (related to PHP 8+ slash/hash comment vs new line retokenization).
2272+
continue;
2273+
}
2274+
22522275
if (is_array($tokens[$i]) === true) {
22532276
$tokenType = $tokens[$i][0];
22542277
} else {
@@ -2264,7 +2287,7 @@ protected function tokenize($string)
22642287
}
22652288

22662289
if ($prevNonEmpty === null
2267-
&& @isset(Tokens::$emptyTokens[$tokenType]) === false
2290+
&& isset(Tokens::$emptyTokens[$tokenType]) === false
22682291
) {
22692292
// Found the previous non-empty token.
22702293
if ($tokenType === ':' || $tokenType === ',' || $tokenType === T_ATTRIBUTE_END) {
@@ -2283,8 +2306,8 @@ protected function tokenize($string)
22832306

22842307
if ($tokenType === T_FUNCTION
22852308
|| $tokenType === T_FN
2286-
|| @isset(Tokens::$methodPrefixes[$tokenType]) === true
2287-
|| @isset(Tokens::$scopeModifiers[$tokenType]) === true
2309+
|| isset(Tokens::$methodPrefixes[$tokenType]) === true
2310+
|| isset(Tokens::$scopeModifiers[$tokenType]) === true
22882311
|| $tokenType === T_VAR
22892312
|| $tokenType === T_READONLY
22902313
) {
@@ -2307,7 +2330,7 @@ protected function tokenize($string)
23072330
break;
23082331
}
23092332

2310-
if (@isset(Tokens::$emptyTokens[$tokenType]) === false) {
2333+
if (isset(Tokens::$emptyTokens[$tokenType]) === false) {
23112334
$lastSeenNonEmpty = $tokenType;
23122335
}
23132336
}//end for
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?php
2+
3+
// This should be the only test in the file.
4+
// Ref: https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/1216
5+
6+
/* testLiveCoding */
7+
$ternary = true
8+
# This must be a slash or hash comment and the next line must **NOT** have any indentation for the PHP 8.5 deprecation notice to occur.
9+
? //comment.
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<?php
2+
/**
3+
* Tests the retokenization of ? to T_NULLABLE or T_INLINE_THEN.
4+
*
5+
* @copyright 2025 PHPCSStandards and contributors
6+
* @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/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+
/**
14+
* Tests the retokenization of ? to T_NULLABLE or T_INLINE_THEN.
15+
*
16+
* @covers PHP_CodeSniffer\Tokenizers\PHP::tokenize
17+
*/
18+
final class NullableVsInlineThenParseErrorTest extends AbstractTokenizerTestCase
19+
{
20+
21+
22+
/**
23+
* Verify that a "?" as the last functional token in a file (live coding) is tokenized as `T_INLINE_THEN`
24+
* as it cannot yet be determined what the token would be once the code is finalized.
25+
*
26+
* @return void
27+
*/
28+
public function testInlineThenAtEndOfFile()
29+
{
30+
$tokens = $this->phpcsFile->getTokens();
31+
$target = $this->getTargetToken('/* testLiveCoding */', [T_NULLABLE, T_INLINE_THEN]);
32+
$tokenArray = $tokens[$target];
33+
34+
$this->assertSame(T_INLINE_THEN, $tokenArray['code'], 'Token tokenized as '.$tokenArray['type'].', not T_INLINE_THEN (code)');
35+
$this->assertSame('T_INLINE_THEN', $tokenArray['type'], 'Token tokenized as '.$tokenArray['type'].', not T_INLINE_THEN (type)');
36+
37+
}//end testInlineThenAtEndOfFile()
38+
39+
40+
}//end class

tests/Core/Tokenizers/PHP/NullableVsInlineThenTest.inc

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,11 @@ $closure = function (
2222
/* testClosureParamTypeNullableInt */
2323
?Int $a,
2424
/* testClosureParamTypeNullableCallable */
25-
? Callable $b
25+
? Callable $b,
26+
/* testClosureParamTypeNullableStringWithAttributeAndSlashComment */
27+
#[AttributeForParam]
28+
// This must be a slash or hash comment and the next line must **NOT** have any indentation for the PHP 8.5 deprecation notice (issue PHPCSStandards/PHP_CodeSniffer#1216) to occur.
29+
?string $c
2630
/* testClosureReturnTypeNullableInt */
2731
) :?INT{};
2832

tests/Core/Tokenizers/PHP/NullableVsInlineThenTest.php

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,16 +50,17 @@ public function testNullable($testMarker)
5050
public static function dataNullable()
5151
{
5252
return [
53-
'property declaration, readonly, no visibility' => ['/* testNullableReadonlyOnly */'],
54-
'property declaration, private set' => ['/* testNullablePrivateSet */'],
55-
'property declaration, public and protected set' => ['/* testNullablePublicProtectedSet */'],
56-
'property declaration, final, no visibility' => ['/* testNullableFinalOnly */'],
57-
'property declaration, abstract, no visibility' => ['/* testNullableAbstractOnly */'],
58-
59-
'closure param type, nullable int' => ['/* testClosureParamTypeNullableInt */'],
60-
'closure param type, nullable callable' => ['/* testClosureParamTypeNullableCallable */'],
61-
'closure return type, nullable int' => ['/* testClosureReturnTypeNullableInt */'],
62-
'function return type, nullable callable' => ['/* testFunctionReturnTypeNullableCallable */'],
53+
'property declaration, readonly, no visibility' => ['/* testNullableReadonlyOnly */'],
54+
'property declaration, private set' => ['/* testNullablePrivateSet */'],
55+
'property declaration, public and protected set' => ['/* testNullablePublicProtectedSet */'],
56+
'property declaration, final, no visibility' => ['/* testNullableFinalOnly */'],
57+
'property declaration, abstract, no visibility' => ['/* testNullableAbstractOnly */'],
58+
59+
'closure param type, nullable int' => ['/* testClosureParamTypeNullableInt */'],
60+
'closure param type, nullable callable' => ['/* testClosureParamTypeNullableCallable */'],
61+
'closure param type, nullable string with comment, issue #1216' => ['/* testClosureParamTypeNullableStringWithAttributeAndSlashComment */'],
62+
'closure return type, nullable int' => ['/* testClosureReturnTypeNullableInt */'],
63+
'function return type, nullable callable' => ['/* testFunctionReturnTypeNullableCallable */'],
6364
];
6465

6566
}//end dataNullable()

0 commit comments

Comments
 (0)