Skip to content

Commit b65945b

Browse files
committed
PHP 8.5 | Tokenizer/PHP: properly fix "Using null as an array offset" deprecation
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. Fixes 1216
1 parent c314822 commit b65945b

File tree

5 files changed

+88
-14
lines changed

5 files changed

+88
-14
lines changed

src/Tokenizers/PHP.php

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2244,6 +2244,24 @@ protected function tokenize($string)
22442244
break;
22452245
}//end for
22462246

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

22552273
for ($i = ($stackPtr - 1); $i >= 0; $i--) {
2274+
if (isset($tokens[$i]) === false) {
2275+
// Ignore skipped tokens (related to PHP 8+ slash/hash comment vs new line retokenization).
2276+
continue;
2277+
}
2278+
22562279
if (is_array($tokens[$i]) === true) {
22572280
$tokenType = $tokens[$i][0];
2258-
} else if ($tokens[$i] === null) {
2259-
// Ignore skipped tokens.
2260-
continue;
22612281
} else {
22622282
$tokenType = $tokens[$i];
22632283
}
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)