PHPCS hangs processing some nested arrow functions inside a function call

[drupol]

Describe the bug
Hello,

I'm using PHP CS in a project and I'm doing experimentation in a local branch.

While testing some stuff, I run from time to time PHP CS with the PSR12 standard.

However, for some reason, it hangs on a file. I streamlined the file to the strict minimum for this issue:

Code sample

<?php

declare(strict_types=1);

namespace Foo;

final class Bar
{
    public function __invoke(): Closure
    {
        return
            static function (callable ...$callbacks): Closure {
                $test = A::of()
                    (B::thunk()(true))
                    (
                        ...C::map()
                        (static fn (callable $callback): callable => static fn ($value, $key, Iterator $iterator): bool => $callback($value, $key, $iterator) === $value)
                        ($callbacks)
                    );

                return $test;
            };
    }
}

To reproduce

./vendor/bin/phpcs --standard=PSR12 --extensions=php --report=full --ignore=/.github/,/.idea/,/build/,/benchmarks/,/node_modules/,/resource/,/spec/,/tests/,/var/,/vendor/ testFile.php

Expected behavior

I expect to have a report.

Versions (please complete the following information):

• OS: Linux
• PHP: 7.4.16
• PHPCS: 3.6 and dev-master
• Standard: PSR12

[jrfnl]

I've just done a git bisect and this appears to be caused by commit: 3decd5e which was made in response to issue #3284

[jrfnl]

The problem is this line which is causing the infinite loop in the tokenizer when an outer and inner arrow function need to share a scope closer:

PHP_CodeSniffer/src/Tokenizers/PHP.php

Line 2364 in 62e6e06

$scopeCloser = ($this->tokens[$scopeCloser]['scope_closer'] - 1);

Adding an additional condition of $this->tokens[$this->tokens[$scopeCloser]['scope_condition']]['code'] !== T_FN to the if doesn't solve this though as it would break the testTernary test.

I'm not sure how to solve this. @gsherwood may have a better idea and at least it's now clear where the problem lies.

[gsherwood]

This is obviously not real code, and I haven't looked into this yet, but the issue can be reproduced on this snippet:

$foo = foo(fn() => fn() => bar() === true);

[gsherwood]

I have confirmed that reverting the fix for #3284 does fix this issue, but only because the inner arrow function is being tokenized incorrectly with that old code.

For this sample:

$foo = foo(fn() => fn() => bar() === true);

The old code see the scope closer of the inner arrow function as the closing parenthesis of the bar() call. The newer code correctly sees the scope closer as true. But because true is not a known scope closing token, we can't share the token and break out of the loop.

[gsherwood]

I have a potential fix for this that I'm playing around with. It basically just uses the scope closer of any nested arrow function it finds, as long as it's not inside a ternary. It's assuming the closer for that arrow function was set correctly. It leaves the match expression code alone though, so there is an exception to that rule.

[gsherwood]

I've pushed a fix for this to master only. I'd appreciate anyone's opinion on this, and additional test cases to either prove or disprove the fix.

I'll finish picking this into 4.0 once I'm more confident that the issue is fixed.

[drupol]

This is obviously not real code, and I haven't looked into this yet, but the issue can be reproduced on this snippet:

Hi,

This is real PHP code, I just stripped out Psalm annotations, use statements and renamed methods.

I'm unable to provide the real code for now on because this is a complete refactoring depending on another library, but if you want to see the previous version of this, it's here (that version doesn't make any issue with PHP CS).

[gsherwood]

This is real PHP code, I just stripped out Psalm annotations, use statements and renamed methods.

Sorry, that's not what I meant. I meant that my one-line code snippet in the comment is not real code, but the smallest possible code I could use to reproduce the issue.

[drupol]

Ooh ok sorry :)

[drupol]

I've pushed a fix for this to master only. I'd appreciate anyone's opinion on this, and additional test cases to either prove or disprove the fix.

I'll finish picking this into 4.0 once I'm more confident that the issue is fixed.

Using the dev version and it seems to work fine. Thanks!

[gsherwood]

@drupol Thanks a lot for testing that out.

I have merged this into the 4.0 branch now, so I'll close this issue as resolved, but please do post again if you find another place this fails before it is released in version 3.6.1.