Avoid PHP error on parse error in `PSR12.Functions.ReturnTypeDeclaration` by fredden · Pull Request #1354 · PHPCSStandards/PHP_CodeSniffer

fredden · 2025-12-29T15:49:53Z

Description

When attempting to process a file with a git merge conflict marker with the PSR12 standard, there is a PHP notice (with phpcs) or PHP Fatal error (with phpcbf) due to an unvalidated assumption in the PSR12.Functions.ReturnTypeDeclaration sniff that there will be a colon in the token array when File::getMethodProperties() detects a return type.

$ phpcs --version
PHP_CodeSniffer version 4.0.1 (stable) by Squiz and PHPCSStandards
$
$ phpcs --standard=PSR12 --basepath=. src/Standards/Generic/Tests/Arrays/DisallowLongArraySyntaxUnitTest.2.inc

FILE: src/Standards/Generic/Tests/Arrays/DisallowLongArraySyntaxUnitTest.2.inc
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 2 LINES
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  1 | ERROR | [x] Header blocks must be separated by a single blank line
  1 | ERROR | [ ] An error occurred during processing; checking has been aborted. The error message was: Undefined array key -1 in
    |       |     ...../vendor/squizlabs/php_codesniffer/src/Standards/PSR12/Sniffs/Functions/ReturnTypeDeclarationSniff.php on line 87
    |       |     The error originated in the PSR12.Functions.ReturnTypeDeclaration sniff on line 87.
 11 | ERROR | [ ] There must be a single space between the colon and type in a return type declaration
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Time: 35ms; Memory: 8MB
$
$ phpcbf --standard=PSR12 --basepath=. src/Standards/Generic/Tests/Arrays/DisallowLongArraySyntaxUnitTest.2.inc
PHP Fatal error:  Uncaught PHP_CodeSniffer\Exceptions\RuntimeException: Undefined array key -1 in ...../vendor/squizlabs/php_codesniffer/src/Standards/PSR12/Sniffs/Functions/ReturnTypeDeclarationSniff.php on line 87 in ...../vendor/squizlabs/php_codesniffer/src/Runner.php:543
Stack trace:
#0 ...../vendor/squizlabs/php_codesniffer/src/Standards/PSR12/Sniffs/Functions/ReturnTypeDeclarationSniff.php(87): PHP_CodeSniffer\Runner->handleErrors()
#1 ...../vendor/squizlabs/php_codesniffer/src/Files/File.php(497): PHP_CodeSniffer\Standards\PSR12\Sniffs\Functions\ReturnTypeDeclarationSniff->process()
#2 ...../vendor/squizlabs/php_codesniffer/src/Files/LocalFile.php(90): PHP_CodeSniffer\Files\File->process()
#3 ...../vendor/squizlabs/php_codesniffer/src/Fixer.php(181): PHP_CodeSniffer\Files\LocalFile->process()
#4 ...../vendor/squizlabs/php_codesniffer/src/Reports/Cbf.php(59): PHP_CodeSniffer\Fixer->fixFile()
#5 ...../vendor/squizlabs/php_codesniffer/src/Reporter.php(368): PHP_CodeSniffer\Reports\Cbf->generateFileReport()
#6 ...../vendor/squizlabs/php_codesniffer/src/Runner.php(627): PHP_CodeSniffer\Reporter->cacheFileReport()
#7 ...../vendor/squizlabs/php_codesniffer/src/Runner.php(390): PHP_CodeSniffer\Runner->processFile()
#8 ...../vendor/squizlabs/php_codesniffer/src/Runner.php(208): PHP_CodeSniffer\Runner->run()
#9 ...../vendor/squizlabs/php_codesniffer/bin/phpcbf(30): PHP_CodeSniffer\Runner->runPHPCBF()
#10 ...../vendor/bin/phpcbf(119): include('...')
#11 {main}
  thrown in ...../vendor/squizlabs/php_codesniffer/src/Runner.php on line 543
$

I initially opened #1350 to change the output of File::getMethodProperties(). While some discussion continues in that pull request, this pull request should unblock anyone attempting to use this sniff during a merge conflict.

Suggested changelog entry

Added defensive coding to PSR12.Functions.ReturnTypeDeclaration to avoid PHP errors.

Related issues/external references

Relates to #152

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
- This change is only breaking for integrators, not for external standards or end-users.
Documentation improvement

PR checklist

I have checked there is no other PR open for the same change.
I have read the Contribution Guidelines.
I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
I have added tests to cover my changes.
I have verified that the code complies with the projects coding standards.
[Required for new sniffs] I have added XML documentation for the sniff.
I have opened a sister-PR in the documentation repository to update the Wiki.

jrfnl

Thanks for setting this up @fredden !

I've left some questions in-line.

I also wonder (but haven't looked in detail yet) whether the code block on (new) line 70-90 could be simplified a little what with the $colon variable being set and known ahead of that code block.

Let me know what you think.

src/Standards/PSR12/Tests/Functions/ReturnTypeDeclarationUnitTest.2.inc

src/Standards/PSR12/Sniffs/Functions/ReturnTypeDeclarationSniff.php

fredden · 2025-12-30T16:32:41Z

I also wonder (but haven't looked in detail yet) whether the code block on (new) line 70-90 could be simplified a little what with the $colon variable being set and known ahead of that code block.

I rewrote the block with this in mind, but don't find it more legible than the original. There is precedent in this sniff to check the 'code' rather than the pointer for the closing parenthesis in the following block (new line 92).

- if ($tokens[($colon - 1)]['code'] !== T_CLOSE_PARENTHESIS) {
+ if (($colon - 1) !== $tokens[$stackPtr]['parenthesis_closer']) {

 if ($tokens[($returnType - 1)]['code'] !== T_WHITESPACE
     || $tokens[($returnType - 1)]['content'] !== ' '
-    || $tokens[($returnType - 2)]['code'] !== T_COLON
+    || ($returnType - 2) !== $colon
 ) {

I'm happy to adjust according to your steer if you'd like.

jrfnl · 2025-12-30T18:17:22Z

I also wonder (but haven't looked in detail yet) whether the code block on (new) line 70-90 could be simplified a little what with the $colon variable being set and known ahead of that code block.

I rewrote the block with this in mind, but don't find it more legible than the original. There is precedent in this sniff to check the 'code' rather than the pointer for the closing parenthesis in the following block (new line 92).

@fredden Thanks for looking into this. I agree that the first one doesn't improve readability, though the second may and that condition also has a duplicate in two more places in that block. Not a blocker either way. Whatever you decide will work.

jrfnl

@fredden Thanks for making those updates! Looking good now. Just one nitpick left (and a non-blocking question).

src/Standards/PSR12/Sniffs/Functions/ReturnTypeDeclarationSniff.php

jrfnl · 2026-01-10T01:32:50Z

src/Standards/PSR12/Sniffs/Functions/ReturnTypeDeclarationSniff.php

+                    for ($i = ($returnType - 1); $i > $colon; $i--) {
+                        $phpcsFile->fixer->replaceToken($i, '');
+                    }


Just curious: any particular reason to walk backwards instead of forwards ?

I don't know why this is going down versus up. There might have been a good reason, but I can't think of it currently. Would you prefer it to go up instead?

Because PHP evaluates `false - 1` to `(int) -1`, the array look-up was looking for `$tokens[-1]` which does not exist. This code change avoids that case by detecting the parse error / live coding situation early. Includes test.

github-actions bot added Type: bug Standard: PSR12 labels Dec 29, 2025

fredden mentioned this pull request Dec 29, 2025

Handle parse error in File::getMethodProperties() #1350

Draft

12 tasks

jrfnl reviewed Dec 30, 2025

View reviewed changes

jrfnl added the Status: awaiting feedback label Dec 30, 2025

jrfnl added this to the 4.0.2 milestone Dec 30, 2025

fredden mentioned this pull request Dec 30, 2025

Fix PHP Notice in PSR12.Functions.ReturnTypeDeclaration #161

Closed

11 tasks

fredden force-pushed the feature/fixer-conflict/PSR12.Functions.ReturnTypeDeclaration branch from c8e974c to 4c89af7 Compare January 2, 2026 17:30

fredden requested a review from jrfnl January 2, 2026 19:01

github-actions bot removed the Status: awaiting feedback label Jan 2, 2026

jrfnl reviewed Jan 10, 2026

View reviewed changes

fredden added 3 commits January 15, 2026 22:35

Rename test file to allow for more test files

4081e9e

Avoid fixer conflict on parse error

ddefbcb

Because PHP evaluates `false - 1` to `(int) -1`, the array look-up was looking for `$tokens[-1]` which does not exist. This code change avoids that case by detecting the parse error / live coding situation early. Includes test.

Avoid multiple loops when fixing whitespace error

124b61f

fredden force-pushed the feature/fixer-conflict/PSR12.Functions.ReturnTypeDeclaration branch from 4c89af7 to 124b61f Compare January 15, 2026 22:36

jrfnl approved these changes Jan 19, 2026

View reviewed changes

jrfnl merged commit 7f4fd39 into PHPCSStandards:4.x Jan 19, 2026
53 checks passed

fredden deleted the feature/fixer-conflict/PSR12.Functions.ReturnTypeDeclaration branch January 19, 2026 22:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Avoid PHP error on parse error in `PSR12.Functions.ReturnTypeDeclaration`#1354

Avoid PHP error on parse error in `PSR12.Functions.ReturnTypeDeclaration`#1354
jrfnl merged 3 commits intoPHPCSStandards:4.xfrom
fredden:feature/fixer-conflict/PSR12.Functions.ReturnTypeDeclaration

fredden commented Dec 29, 2025

Uh oh!

jrfnl left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fredden commented Dec 30, 2025

Uh oh!

jrfnl commented Dec 30, 2025

Uh oh!

jrfnl left a comment

Uh oh!

Uh oh!

jrfnl Jan 10, 2026

Uh oh!

fredden Jan 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

fredden commented Dec 29, 2025

Description

Suggested changelog entry

Related issues/external references

Types of changes

PR checklist

Uh oh!

jrfnl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fredden commented Dec 30, 2025

Uh oh!

jrfnl commented Dec 30, 2025

Uh oh!

jrfnl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jrfnl Jan 10, 2026

Choose a reason for hiding this comment

Uh oh!

fredden Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants