Undefined offset: 0 in PropertyHelper.php on line 81

[morozov]

This issue is similar to #254. I don't know whether it's the PHP_CodeSniffer that produces an invalid token or the Slevomat Coding Standard that expects the unavailable properties of the token, so I'll file it here.

These are the relevant dependency versions:

$ composer show # truncated

doctrine/coding-standard       7.0.2   The Doctrine Coding Standard is a ...
slevomat/coding-standard       6.1.3   Slevomat Coding Standard for PHP_C...
squizlabs/php_codesniffer      3.5.4   PHP_CodeSniffer tokenizes PHP, Jav...

Consider the following code example:

<?php

class Problem
{
    public function test()
    {
        if (true) {
            return;
        } else if (true) {
            return;
        } else { // A comment
            return;
        }
    }

    protected $property;
}

An attempt to fix this file make phpcs crash:

$ phpcbf -v Problem.php
Registering sniffs in the  standard... DONE (150 sniffs registered)
Creating file list... DONE (1 files in queue)
Changing into directory .
Processing Problem.php [PHP => 77 tokens in 17 lines]... DONE in 9ms (5 fixable violations)
        => Fixing file: 17/5 violations remaining [made 1 pass]... [PHP => 84 tokens in 18 lines]... 
Fatal error: Uncaught PHP_CodeSniffer\Exceptions\RuntimeException: Undefined offset: 0 in vendor/slevomat/coding-standard/SlevomatCodingStandard/Helpers/PropertyHelper.php on line 81 in vendor/squizlabs/php_codesniffer/src/Runner.php on line 606

PHP_CodeSniffer\Exceptions\RuntimeException: Undefined offset: 0 in vendor/slevomat/coding-standard/SlevomatCodingStandard/Helpers/PropertyHelper.php on line 81 in vendor/squizlabs/php_codesniffer/src/Runner.php on line 606

Call Stack:
    0.0001     470952   1. {main}() vendor/squizlabs/php_codesniffer/bin/phpcbf:0
    0.0028    1002816   2. PHP_CodeSniffer\Runner->runPHPCBF() vendor/squizlabs/php_codesniffer/bin/phpcbf:18
    0.0626    7448656   3. PHP_CodeSniffer\Runner->run() vendor/squizlabs/php_codesniffer/src/Runner.php:200
    0.0661    8052520   4. PHP_CodeSniffer\Runner->processFile() vendor/squizlabs/php_codesniffer/src/Runner.php:434
    0.0755    8576728   5. PHP_CodeSniffer\Reporter->cacheFileReport() vendor/squizlabs/php_codesniffer/src/Runner.php:656
    0.0755    8595448   6. PHP_CodeSniffer\Reports\Cbf->generateFileReport() vendor/squizlabs/php_codesniffer/src/Reporter.php:285
    0.0755    8578936   7. PHP_CodeSniffer\Fixer->fixFile() vendor/squizlabs/php_codesniffer/src/Reports/Cbf.php:49
    0.0923    8622448   8. PHP_CodeSniffer\Files\LocalFile->process() vendor/squizlabs/php_codesniffer/src/Fixer.php:174
    0.0923    8622448   9. PHP_CodeSniffer\Files\LocalFile->process() vendor/squizlabs/php_codesniffer/src/Files/LocalFile.php:91
    0.0993    8684560  10. SlevomatCodingStandard\Sniffs\TypeHints\PropertyTypeHintSniff->process() vendor/squizlabs/php_codesniffer/src/Files/File.php:496
    0.0993    8684560  11. SlevomatCodingStandard\Sniffs\TypeHints\PropertyTypeHintSniff->checkTypeHint() vendor/slevomat/coding-standard/SlevomatCodingStandard/Sniffs/TypeHints/PropertyTypeHintSniff.php:103
    0.0993    8684560  12. SlevomatCodingStandard\Helpers\PropertyHelper::getFullyQualifiedName() vendor/slevomat/coding-standard/SlevomatCodingStandard/Sniffs/TypeHints/PropertyTypeHintSniff.php:124
    0.0993    8685408  13. PHP_CodeSniffer\Runner->handleErrors() vendor/slevomat/coding-standard/SlevomatCodingStandard/Helpers/PropertyHelper.php:81

As far as I understand the failure, the code expects at least one condition available in the token but there is none in this case.

coding-standard/SlevomatCodingStandard/Helpers/PropertyHelper.php

Line 81 in c8f2e2b

$classPointer = array_reverse(array_keys($propertyToken['conditions']))[0];

I was able to reproduce the issue only by keeping the property declared after the method, the else if combination and the comment after the else.

[kukulich]

@morozov it’s caused probably by “else if”. Use “elseif” and everything should be fine I think.

There’s also sniff in PHPCS itself that can report “else if” usage.

[morozov]

@kukulich thank you for the suggestion. I just tried it on the real file (4800 LOC, proprietary codebase), and the crash is still reproducible. If you want, I can post another PoC that doesn't include else if later.

Regardless of whether there is a workaround, I believe the issue is worth fixing or at least some better research. I'm trying to apply the coding standard to a huge legacy old codebase with thousands of violations in some files.

Getting the fixer crash in the middle of a multi-hour long process, excluding the problematic files and starting over is a bit unproductive (I found a couple more issues like this in the OOTB standards as well).

[kukulich]

@morozov It looks I cannot reproduce it on PHP 7.4. Do you use PHP 7.2 or lower?

Btw it you use AlphabeticallySortedUsesSniff and other sniffs that add a lot of use statements, it's better to disable AlphabeticallySortedUsesSniff for the first run. It can speed up fixer very much.

[morozov]

Do you use PHP 7.2 or lower?

I used PHP 7.4.2 and it's also reproducible with 7.3.14.

Btw it you use AlphabeticallySortedUsesSniff and other sniffs that add a lot of use statements, it's better to disable AlphabeticallySortedUsesSniff for the first run. It can speed up fixer very much.

Thank you for the suggestion. I was thinking of applying the fixes in parts.

[kukulich]

@morozov Hmm, it may be bug in PHPCS that's caused by some combination of sniffs and fixes :(

The method PropertyHelper::getFullyQualifiedName() is only for nicer errors so it's useless for the fixer. You can add return 'whatever'; to the top of the method as a workaround.

[morozov]

The method PropertyHelper::getFullyQualifiedName() is only for nicer errors so it's useless for the fixer.

According to the stack trace, it's called from within the Slevomat Coding Standard:

    0.0923    8622448   9. PHP_CodeSniffer\Files\LocalFile->process() vendor/squizlabs/php_codesniffer/src/Files/LocalFile.php:91
    0.0993    8684560  10. SlevomatCodingStandard\Sniffs\TypeHints\PropertyTypeHintSniff->process() vendor/squizlabs/php_codesniffer/src/Files/File.php:496
    0.0993    8684560  11. SlevomatCodingStandard\Sniffs\TypeHints\PropertyTypeHintSniff->checkTypeHint() vendor/slevomat/coding-standard/SlevomatCodingStandard/Sniffs/TypeHints/PropertyTypeHintSniff.php:103
    0.0993    8684560  12. SlevomatCodingStandard\Helpers\PropertyHelper::getFullyQualifiedName() vendor/slevomat/coding-standard/SlevomatCodingStandard/Sniffs/TypeHints/PropertyTypeHintSniff.php:124

You can add return 'whatever'; to the top of the method as a workaround.

It didn't help. The issue is reproducible without the else if even on the same code sample:

<?php

class Problem
{
    public function test()
    {
        return 1;

        if (true) {
            return;
        } elseif (true) {
            return;
        } else { // A comment
            return;
        }
    }

    protected $property;
}

[kukulich]

@morozov I'm not sure if you understood me well.

PropertyTypeHintSniff, line 124 calls PropertyHelper::getFullyQualifiedName() but for the fixer the return value of PropertyHelper::getFullyQualifiedName() is useless so you can change the return value of PropertyHelper::getFullyQualifiedName() to anything (eg. return 'whatever';) to skip the error as a workaround.

[morozov]

@kukulich ah… I see. Makes sense. So far, I ended up manually moving the comment and it also enabled fixing the entire file:

diff --git a/Problem.php b/Problem.php
index f56c9e0bf52..7a73c72b353 100644
--- a/Problem.php
+++ b/Problem.php
@@ -8,7 +8,8 @@ class Problem
             return;
         } else if (true) {
             return;
-        } else { // A comment
+        } else {
+            // A comment
             return;
         }
     }

[kukulich]

Inline comments should be also fixed by #890

[morozov]

PHPCBF no longer crashes fixing the example in the description as well as the real 4800-LOC file. However, the result of fixing the example has some misalignment:

diff --git a/Problem.php b/Problem.php
index 41ba5980f5e..b1b698f2d0c 100644
--- a/Problem.php
+++ b/Problem.php
@@ -6,11 +6,14 @@ class Problem
     {
         if (true) {
             return;
-        } elseif (true) {
-            return;
-        } else { // A comment
+        }
+
+        if (true) {
             return;
         }
+
+    // A comment
+        return;
     }
 
     protected $property;

Is it something that could be addressed here or we’ll need a separate issue for that?

[kukulich]

@morozov Try dev-master please.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.