Failures in child processes are not handled by the runner

[morozov]

Unfortunately, I don't know how to reproduce this issue with the OOTB standards, but here's the minimal required setup:

cat > composer.json << 'EOF'
{
    "require-dev": {
        "doctrine/coding-standard": "^4.0"
    }
}
EOF

cat > phpcs.xml.dist << 'EOF'
<?xml version="1.0"?>
<ruleset>
    <rule ref="Doctrine"/>
</ruleset>
EOF

cat > PhpCodeSnifferFailure.php << 'EOF'
<?php

namespace Doctrine\DBAL;

class PhpCodeSnifferFailure
{
    public function foo()
    {
        return [
        <<<HERE
HERE
            ,
        ];
    }
}
EOF

composer install

vendor/bin/phpcs --version
# PHP_CodeSniffer version 3.3.1 (stable) by Squiz (http://www.squiz.net)

vendor/bin/phpcbf --parallel=2 PhpCodeSnifferFailure.php

The run results in the following error:

1. PHP_CodeSniffer\Exceptions\RuntimeException: Undefined index: scope_closer in vendor/slevomat/coding-standard/SlevomatCodingStandard/Helpers/UseStatementHelper.php on line 156 in /home/morozov/t1/vendor/squizlabs/php_codesniffer/src/Runner.php on line 562

   This error happens in the child process outside of the PHP_CodeSniffer codebase because the T_CLASS token doesn't have the scope_closer attribute defined.

2. PHP_CodeSniffer\Exceptions\RuntimeException: Undefined variable: childOutput in vendor/squizlabs/php_codesniffer/src/Runner.php on line 705 in vendor/squizlabs/php_codesniffer/src/Runner.php on line 562

   This error happens in PHP_CodeSniffer itself. The parent creates an empty shared file before running the child:

   PHP_CodeSniffer/src/Runner.php

   Line 418 in 4725c01

   $childOutFilename = tempnam(sys_get_temp_dir(), 'phpcs-child');

   but the child dies before writing its contents due to the error above. However, the parent only checks if the file exists:

   PHP_CodeSniffer/src/Runner.php

   Line 677 in 4725c01

   if (file_exists($procData['out']) === true) {

Besides the bug in the runner, is the fact that a T_CLASS token doesn't have scope_closer also a bug?

[gsherwood]

A RuntimeException being thrown would end the run if you were not running with the parallel option as well, so you shouldn't expect a clean shutdown or valid report in this case.

The real error here is the T_CLASS missing the scope_closer. But in my testing, PHPCS tokenizes the sample code and finds the scope_closer correctly. I don't have time to debug this to see if there is a problem with the custom sniff, but I'd look there first to see what's going on.

[gsherwood]

You might also want to try running phpcbf with the -vvv option, which will show you what changes the fixer is making. That might help you track down a sniff that is fixing the file incorrectly.

[morozov]

@gsherwood how do you store the tokens produced from a given file? IIRC, there was an issue where PHPCS had different logic in debug mode.

[gsherwood]

how do you store the tokens produced from a given file? IIRC, there was an issue where PHPCS had different logic in debug mode.

I'm not sure what you mean, and I'm not sure what different logic you are referring to.

[morozov]

But in my testing, PHPCS tokenizes the sample code and finds the scope_closer correctly.

How can I produce a list of PHPCS tokens from a file to share it or see if there's the needed attribute? When debugging the custom sniff, I can see that there's no closer in the token.

I'm not sure what you mean, and I'm not sure what different logic you are referring to.

I'm referring to #903 (comment).

[gsherwood]

I'm referring to #903 (comment).

That's just the ScopeIndent sniff running in debug mode. You aren't using that special debug flag for it, so that's not going to be the issue.

How can I produce a list of PHPCS tokens from a file to share it or see if there's the needed attribute?

Try running phpcs --report=diff -vvv .... That will give you the tokenizer output for the original file, all the fixes being made, and the contents of the fixed file after each pass. That output will show what is going wrong. Note this is PHPCS and not PHPCBF.

[morozov]

@gsherwood here's the interesting fragment from the output of phpcs --report=diff -vvv:

	* fixed 2 violations, starting loop 2 *
---START FILE CONTENT---
 1|<?php
 2|declare(strict_types = 1);
 3|
 4|namespace Doctrine\DBAL;
 5|
 6|class PhpCodeSnifferFailure
 7|{
 8|    public function foo()
 9|    {
10|        return [
11|        <<<HERE
12|HERE
13|,
14|        ];
15|    }
16|}
17|
--- END FILE CONTENT ---

--------------8<------------------

	E: [Line 13] Expected 0 spaces between "HERE" and comma; newline found (Squiz.Arrays.ArrayDeclaration.SpaceBeforeComma)
	PHP_CodeSniffer\Standards\Squiz\Sniffs\Arrays\ArrayDeclarationSniff (line 449) replaced token 46 (T_WHITESPACE) "\n········," => "········,"
	* fixed 8 violations, starting loop 3 *
---START FILE CONTENT---
 1|<?php
 2|
 3|declare(strict_types=1);
 4|
 5|namespace Doctrine\DBAL;
 6|
 7|class PhpCodeSnifferFailure
 8|{
 9|    public function foo()
10|    {
11|        return [
12|        <<<HERE
13|HERE        ,
14|        ];
15|    }
16|}
17|
--- END FILE CONTENT ---

By the beginning of the loop 2, the closing HERE identifier and the comma are on separate lines which is correct, but during the loop, ArrayDeclarationSniff removes the new line character which makes the closing identifier invalid. Therefore (as far as I understand), on the next iteration, the T_CLASS loses its scope closer.

[morozov]

A RuntimeException being thrown would end the run if you were not running with the parallel option as well, so you shouldn't expect a clean shutdown or valid report in this case.

This is true but the output should be the same in both parallel and non-parallel modes. Currently, there's one error in non-parallel mode, and two errors in parallel. This is confusing and requies additional debugging to find the real issue.

[gsherwood]

This is true but the output should be the same in both parallel and non-parallel modes.

I'm not going to silence the error complaining about childOutput being missing. If I did, I'd possibly not be reporting an error when something else goes wrong with the child process, and it may silently fail but look to succeed.

I am going to look into the array sniff and fix it up.

[morozov]

I'm not going to silence the error complaining about childOutput being missing.

I didn't mean the errors should be suppressed. What I meant is that the runner should account for the fact the children may fail and not produce any additional unhandled errors.

For instance, it could show the available part of the report and additionally say that processes X, Y and Z failed, therefore the report may be incomplete. It also could suggest running the same report without parallel execution. With this approach, once a child process fails, a developer could focus on the error from the child and not debug the error in the runner.

[gsherwood]

For instance, it could show the available part of the report and additionally say that processes X, Y and Z failed, therefore the report may be incomplete.

Sorry, understand you now.

[gsherwood]

I've fixed the array sniff issue that was causing the error. I'll leave this open as an improvement to child process handling. Thanks for helping me figure this one out.

[morozov]

@gsherwood, thank you for helping figure out the root cause.

[paslandau]

Hey @gsherwood,

unfortunately, the Squiz.Arrays.ArrayDeclaration (in particular Squiz.Arrays.ArrayDeclaration.NoCommaAfterLast) sniff is generating invalid code (for php <= 7.2) when a HEREDOC string is the last element of an array, because it adds a comma directly after the HEREDOC end token instead of a new line. This will only be valid starting from php 7.3 (see https://wiki.php.net/rfc/flexible_heredoc_nowdoc_syntaxes#closing_marker_new_line )

Rules

<?xml version="1.0"?>
<ruleset name="Error">
    <rule ref="Squiz.Arrays.ArrayDeclaration"/>
</ruleset>

Code

<?php

$a = [
      "foo",
    <<<HEREDOC
   
HEREDOC
];

"Fixed" Code

<?php

$a = [
      "foo",
    <<<HEREDOC
   
HEREDOC, // <-- note the ","
     ];

Debug

root@cbae699fa362:/var/www/current# vendor/bin/phpcbf test4Test.php --standard=qa/phpcs/error.xml -vvv
Processing ruleset /var/www/current/qa/phpcs/error.xml
        Processing rule "Squiz.Arrays.ArrayDeclaration"
                => /var/www/current/vendor/squizlabs/php_codesniffer/src/Standards/Squiz/Sniffs/Arrays/ArrayDeclarationSniff.php
=> Ruleset processing complete; included 1 sniffs and excluded 0
Registered PHP_CodeSniffer\Standards\Squiz\Sniffs\Arrays\ArrayDeclarationSniff
Creating file list... DONE (1 files in queue)
Changing into directory /var/www/current
Processing test4Test.php
        *** START PHP TOKENIZING ***
        Process token [0]: T_OPEN_TAG => <?php\n
        Process token [1]: T_WHITESPACE => \n
        Process token [2]: T_VARIABLE => $a
        Process token [3]: T_WHITESPACE => ·
        Process token  4 : T_EQUAL => =
        Process token [5]: T_WHITESPACE => ·
        Process token  6 : T_OPEN_SQUARE_BRACKET => [
        Process token [7]: T_WHITESPACE => \n······
        Process token [8]: T_CONSTANT_ENCAPSED_STRING => "foo"
        Process token  9 : T_COMMA => ,
        Process token [10]: T_WHITESPACE => \n····
        Process token [11]: T_START_HEREDOC => <<<HEREDOC\n
        Process token [14]: T_WHITESPACE => \n····
        Process token  15 : T_CLOSE_SQUARE_BRACKET => ]
        Process token  16 : T_SEMICOLON => ;
        Process token [17]: T_WHITESPACE => \n
        *** END PHP TOKENIZING ***
        *** START TOKEN MAP ***
        => Found square bracket opener at 6
        => Found square bracket closer at 18 for 6
        *** END TOKEN MAP ***
        *** START SCOPE MAP ***
        Start scope map at 13:T_START_HEREDOC => <<<HEREDOC\n
        => Begin scope map recursion at token 13 with depth 1
        Process token 14 on line 6 [opener:13;]: T_HEREDOC => ···\n
        Process token 15 on line 7 [opener:13;]: T_END_HEREDOC => HEREDOC
        => Found scope closer (15:T_END_HEREDOC) for 13:T_START_HEREDOC
        *** END SCOPE MAP ***
        *** START LEVEL MAP ***
        Process token 0 on line 1 [col:1;len:5;lvl:0;]: T_OPEN_TAG => <?php\n
        Process token 1 on line 2 [col:1;len:0;lvl:0;]: T_WHITESPACE => \n
        Process token 2 on line 3 [col:1;len:2;lvl:0;]: T_VARIABLE => $a
        Process token 3 on line 3 [col:3;len:1;lvl:0;]: T_WHITESPACE => ·
        Process token 4 on line 3 [col:4;len:1;lvl:0;]: T_EQUAL => =
        Process token 5 on line 3 [col:5;len:1;lvl:0;]: T_WHITESPACE => ·
        Process token 6 on line 3 [col:6;len:1;lvl:0;]: T_OPEN_SQUARE_BRACKET => [
        Process token 7 on line 3 [col:7;len:0;lvl:0;]: T_WHITESPACE => \n
        Process token 8 on line 4 [col:1;len:6;lvl:0;]: T_WHITESPACE => ······
        Process token 9 on line 4 [col:7;len:5;lvl:0;]: T_CONSTANT_ENCAPSED_STRING => "foo"
        Process token 10 on line 4 [col:12;len:1;lvl:0;]: T_COMMA => ,
        Process token 11 on line 4 [col:13;len:0;lvl:0;]: T_WHITESPACE => \n
        Process token 12 on line 5 [col:1;len:4;lvl:0;]: T_WHITESPACE => ····
        Process token 13 on line 5 [col:5;len:10;lvl:0;]: T_START_HEREDOC => <<<HEREDOC\n
        => Found scope opener for 13:T_START_HEREDOC
                * level increased *
                * token 13:T_START_HEREDOC added to conditions array *
                Process token 14 on line 6 [col:1;len:3;lvl:1;conds;T_START_HEREDOC;]: T_HEREDOC => ···\n
                Process token 15 on line 7 [col:1;len:7;lvl:1;conds;T_START_HEREDOC;]: T_END_HEREDOC => HEREDOC
                => Found scope closer for 13:T_START_HEREDOC
                * token T_START_HEREDOC removed from conditions array *
                * level decreased *
        Process token 16 on line 7 [col:8;len:0;lvl:0;]: T_WHITESPACE => \n
        Process token 17 on line 8 [col:1;len:4;lvl:0;]: T_WHITESPACE => ····
        Process token 18 on line 8 [col:5;len:1;lvl:0;]: T_CLOSE_SQUARE_BRACKET => ]
        Process token 19 on line 8 [col:6;len:1;lvl:0;]: T_SEMICOLON => ;
        Process token 20 on line 8 [col:7;len:0;lvl:0;]: T_WHITESPACE => \n
        *** END LEVEL MAP ***
        *** START ADDITIONAL PHP PROCESSING ***
        * token 6 on line 3 changed from T_OPEN_SQUARE_BRACKET to T_OPEN_SHORT_ARRAY
        * token 18 on line 8 changed from T_CLOSE_SQUARE_BRACKET to T_CLOSE_SHORT_ARRAY
        *** END ADDITIONAL PHP PROCESSING ***
[PHP => 21 tokens in 8 lines]...
        *** START TOKEN PROCESSING ***
                Process token 0: T_OPEN_TAG => <?php\n
                Process token 1: T_WHITESPACE => \n
                Process token 2: T_VARIABLE => $a
                Process token 3: T_WHITESPACE => ·
                Process token 4: T_EQUAL => =
                Process token 5: T_WHITESPACE => ·
                Process token 6: T_OPEN_SHORT_ARRAY => [
                        Processing PHP_CodeSniffer\Standards\Squiz\Sniffs\Arrays\ArrayDeclarationSniff... DONE in 0.0001 seconds
                Process token 7: T_WHITESPACE => \n
                Process token 8: T_WHITESPACE => ······
                Process token 9: T_CONSTANT_ENCAPSED_STRING => "foo"
                Process token 10: T_COMMA => ,
                Process token 11: T_WHITESPACE => \n
                Process token 12: T_WHITESPACE => ····
                Process token 13: T_START_HEREDOC => <<<HEREDOC\n
                Process token 14: T_HEREDOC => ···\n
                Process token 15: T_END_HEREDOC => HEREDOC
                Process token 16: T_WHITESPACE => \n
                Process token 17: T_WHITESPACE => ····
                Process token 18: T_CLOSE_SHORT_ARRAY => ]
                Process token 19: T_SEMICOLON => ;
                Process token 20: T_WHITESPACE => \n
        *** END TOKEN PROCESSING ***
        *** START SNIFF PROCESSING REPORT ***
        PHP_CodeSniffer\Standards\Squiz\Sniffs\Arrays\ArrayDeclarationSniff: 0.0001 secs
        *** END SNIFF PROCESSING REPORT ***
DONE in 76ms (2 fixable violations)
        => Fixing file: 2/2 violations remaining---START FILE CONTENT---
1|<?php
2|
3|$a = [
4|      "foo",
5|    <<<HEREDOC
6|
7|HEREDOC
8|    ];
9|
--- END FILE CONTENT ---
        PHP_CodeSniffer\Standards\Squiz\Sniffs\Arrays\ArrayDeclarationSniff (line 340) replaced token 17 (T_WHITESPACE) "····]" => "·····]"
        PHP_CodeSniffer\Standards\Squiz\Sniffs\Arrays\ArrayDeclarationSniff (line 599) replaced token 15 (T_END_HEREDOC) "HEREDOC" => "HEREDOC,"
        => Fixing file: 2/2 violations remaining [made 1 pass]...       * fixed 2 violations, starting loop 2 *
---START FILE CONTENT---
1|<?php
2|
3|$a = [
4|      "foo",
5|    <<<HEREDOC
6|
7|HEREDOC,
8|     ];
9|
--- END FILE CONTENT ---
        => Fixing file: 0/2 violations remaining [made 2 passes]... DONE in 2ms
        => File was overwritten

PHPCBF RESULT SUMMARY
----------------------------------------------------------------------
FILE                                                  FIXED  REMAINING
----------------------------------------------------------------------
/var/www/current/test4Test.php                        2      0
----------------------------------------------------------------------
A TOTAL OF 2 ERRORS WERE FIXED IN 1 FILE
----------------------------------------------------------------------

Time: 361ms; Memory: 4MB

[soullivaneuh]

I have the same issue with slevomat/coding-standard.

First, I was thinking an issue with the standard itself: slevomat/coding-standard#644

But according to @kukulich, the class token should always have scope_closer set: slevomat/coding-standard#644 (comment)

Here is the log output:

---START FILE CONTENT---
 1|<?php
 2|
 3|declare(strict_types=1);
 4|
 5|namespace AppBundle\Listener;
 6|
 7|use AppBundle\Entity\User;
 8|use Doctrine\Common\EventSubscriber;
 9|use Doctrine\ORM\Event\OnFlushEventArgs;
10|
11|final class UsernameChangeListener implements EventSubscriber
12|{
13|    /**
14|     * {@inheritdoc}
15|     */
16|    public function getSubscribedEvents()
17|    {
18|        return [
19|            'onFlush',
20|        ];
21|    }
22|
23|    public function onFlush(OnFlushEventArgs $eventArgs): void
24|    {
25|        $em = $eventArgs->getEntityManager();
26|        $uow = $em->getUnitOfWork();
27|
28|        foreach ($uow->getScheduledEntityUpdates() as $entity) {
29|            if (!($entity instanceof User) || !\array_key_exists('username', $uow->getEntityChangeSet($entity))) {
30|                continue;
31|            }
32|
33|            [$oldUsername, $newUsername] = $uow->getEntityChangeSet($entity)['username'];
34|            $queryFormat = <<<'SQL'
35|            UPDATE revisions SET username = '%s' WHERE username = '%s'
36|            SQL;
37|            $em->getConnection()->exec(\sprintf($queryFormat, $newUsername, $oldUsername));
38|        }
39|    }
40|}
41|
--- END FILE CONTENT ---
	PHP_CodeSniffer\Standards\Generic\Sniffs\WhiteSpace\ScopeIndentSniff (line 1405) replaced token 72 (T_WHITESPACE) "····public" => "public"
	PHP_CodeSniffer\Standards\Generic\Sniffs\WhiteSpace\ScopeIndentSniff (line 1405) replaced token 81 (T_WHITESPACE) "····{" => "{"
	PHP_CodeSniffer\Standards\Generic\Sniffs\WhiteSpace\ScopeIndentSniff (line 1405) replaced token 84 (T_WHITESPACE) "········return" => "····return"
	PHP_CodeSniffer\Standards\Generic\Sniffs\WhiteSpace\ScopeIndentSniff (line 1405) replaced token 89 (T_WHITESPACE) "············'onFlush'" => "········'onFlush'"
	PHP_CodeSniffer\Standards\Generic\Sniffs\WhiteSpace\ScopeIndentSniff (line 1405) replaced token 93 (T_WHITESPACE) "········]" => "····]"
	PHP_CodeSniffer\Standards\Generic\Sniffs\WhiteSpace\ScopeIndentSniff (line 1405) replaced token 97 (T_WHITESPACE) "····}" => "}"
	PHP_CodeSniffer\Standards\Generic\Sniffs\WhiteSpace\ScopeIndentSniff (line 1405) replaced token 101 (T_WHITESPACE) "····public" => "public"
	PHP_CodeSniffer\Standards\Generic\Sniffs\WhiteSpace\ScopeIndentSniff (line 1405) replaced token 161 (T_WHITESPACE) "············if" => "if"
	PHP_CodeSniffer\Standards\Generic\Sniffs\WhiteSpace\ScopeIndentSniff (line 1405) replaced token 194 (T_WHITESPACE) "················continue" => "····continue"
	PHP_CodeSniffer\Standards\Generic\Sniffs\WhiteSpace\ScopeIndentSniff (line 1405) replaced token 198 (T_WHITESPACE) "············}" => "}"
DONE in 0.0003 seconds
			Processing PHP_CodeSniffer\Standards\Generic\Sniffs\WhiteSpace\DisallowTabIndentSniff... DONE in 0 seconds
			Processing Symfony\Sniffs\Files\AlphanumericFilenameSniff... DONE in 0 seconds
			Processing PHP_CodeSniffer\Standards\Zend\Sniffs\Files\ClosingTagSniff... DONE in 0 seconds
			Processing SlevomatCodingStandard\Sniffs\Namespaces\FullyQualifiedExceptionsSniff... 
Fatal error: Uncaught PHP_CodeSniffer\Exceptions\RuntimeException: Undefined index: scope_closer in /home/developer/.composer/vendor/slevomat/coding-standard/SlevomatCodingStandard/Helpers/UseStatementHelper.php on line 207 in /home/developer/.composer/vendor/squizlabs/php_codesniffer/src/Runner.php:600
Stack trace:
#0 /home/developer/.composer/vendor/slevomat/coding-standard/SlevomatCodingStandard/Helpers/UseStatementHelper.php(207): PHP_CodeSniffer\Runner->handleErrors(8, 'Undefined index...', '/home/developer...', 207, Array)
#1 /home/developer/.composer/vendor/slevomat/coding-standard/SlevomatCodingStandard/Helpers/UseStatementHelper.php(149): SlevomatCodingStandard\Helpers\UseStatementHelper::getUseStatementPointers(Object(PHP_CodeSniffer\Files\LocalFile), 0)
#2 /home/developer/.composer/vendor/slevomat/coding-standard/SlevomatCodingStandard/Helpers/UseStatementHelper.php(111): SlevomatCodingStandard\Helpers\UseStatementHelper::getFileUseStatements(Object(PHP_CodeSniffer\Files\LocalFile))
#3 /home/developer/.composer in /home/developer/.composer/vendor/squizlabs/php_codesniffer/src/Runner.php on line 600

Hope it helps.

[gsherwood]

@soullivaneuh your file is using PHP 7.3 nowdoc syntax (the closing identifier SQL is indented) but that will not tokenize correctly if you are running PHPCS using a PHP version < 7.3.

When I try running PHPCS over your file with the slevomat coding standard using PHP 7.3, I have no problems. If I try with PHP 7.1, I get the same error you are seeing.

Are you using an older PHP version to check this newer code?

[gsherwood]

@morozov I suspect this might be the same issue for you as well

[morozov]

@gsherwood the issue I reported originally is only reproducible with PHP < 7.3 (e.g. 7.2.16) and only with PHP_CodeSniffer 3.3 (not 3.4). However, as you can see in my example, the closing heredoc identifier (HERE) is not indented.

[gsherwood]

and only with PHP_CodeSniffer 3.3 (not 3.4)

Yep, different issue. So is this original issue now resolved?

[morozov]

Yes, it is.

[wongjn]

This happens for Drupal/Coder too with

<?php
$a = [
  'a' =>
];

Minimal recreation repo here.

[gsherwood]

This happens for Drupal/Coder too with

Thanks for reporting, but this is a completely unrelated issue. I'm going to look into it, but I'm also going to close this issue so it doesn't keep becoming a thread for a range of different issues. If anyone here is having additional problems, please open a new issue in this repo so it can be tracked properly.