Problem with custom attribute

[OskarStark]

Bug report

PHP 8.0.2 (cli) (built: Feb  4 2021 18:10:29) ( NTS )
Copyright (c) The PHP Group
Zend Engine v4.0.2, Copyright (c) Zend Technologies
    with Zend OPcache v8.0.2, Copyright (c), by Zend Technologies

PHP CS Fixer 2.18.2 Remote Void by Fabien Potencier and Dariusz Ruminski
Runtime: PHP 8.0.2

php vendor/bin/php-cs-fixer fix src/Controller/Security/Abo/ConfirmAboController.php --verbose

Code:

<?php

declare(strict_types=1);

namespace App\Controller\Security\Abo;

use App\Entity\Abo;
use App\Entity\User;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Security\Http\Attribute\CurrentUser;
use Symfony\Component\Routing\Annotation\Route;

#[Route('/abo/confirm/{aboSecret}', name: 'foo')]
final class ConfirmAboController
{
    public function __invoke(Abo $abo, Request $request, #[CurrentUser] ?User $user = null): Response
    {
return new Response('hi');
    }
}

leads to:

Files that were not fixed due to errors reported during fixing:
   1) /Users/oskar.stark/dev/cockpit/src/Controller/Security/Abo/ConfirmAboController.php
error sending signal urgent I/O condition os: process already finished
exit status 64

Changing to:

-    public function __invoke(Abo $abo, Request $request, #[CurrentUser] ?User $user = null): Response
+    public function __invoke(Abo $abo, Request $request, #[CurrentUser] ?User $user): Response

ends up in a non ending process and my macbook starts to explode 🤔

Changing to:

-    public function __invoke(Abo $abo, Request $request, #[CurrentUser] ?User $user): Response
+    public function __invoke(Abo $abo, Request $request, #[CurrentUser] User $user): Response

Cs-fixer can fix and lint it, but my code is not working anymore 😄

cc @localheinz @derrabus

[localheinz]

As a workaround, perhaps use annotations for now?

[OskarStark]

Unfortunately this is not possible for the CurrentUser attribute 😞

[derrabus]

The workaround would be to remove the attribute and change the type hint to UserInterface. Anyhow, CS Fixer should not explode when using that attribute. 😉

[OskarStark]

I checked this, but unfortunately:

#[CurrentUser] ?UserInterface $user = null

nor

#[CurrentUser] ?UserInterface $user

nor

#[CurrentUser] UserInterface $user = null

are working @derrabus 🤔
Without been nullable all is fine 👍

[derrabus]

No, but this works:

?UserInterface $user = null

[OskarStark]

You mean without the attribute? 🤔

[keradus]

@OskarStark , thanks for raising the concern.
Are you able to raise a PR to fix it?

also, pls share the configuration ruleset that you are using, to help reproducing the issue

[OskarStark]

@OskarStark , thanks for raising the concern.

You are welcome, thanks for your hard work on this tool 👍

Are you able to raise a PR to fix it?

Unfortunately I am not so deep ion this topic and have a lot of work on my desk 😢

also, pls share the configuration ruleset that you are using, to help reproducing the issue

Is there a way I can dump the used config?

[derrabus]

@OskarStark Usually, your project has a .php_cs or .php_cs.dist file that contains the configuration.

[jrmajor]

@keradus @derrabus

I've managed to reproduce this bug with the following code:

<?php

function test(#[Attribute] ?User $user) {}

The problem is in TernaryToElvisOperatorFixer. Nullable type hint is treated as a part of a ternary operator.

https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/9bf2760ce9d2218cc127cb4106e9ddfedca4eb3a/src/Fixer/Operator/TernaryToElvisOperatorFixer.php#L126-L134

$beforeOperator == ['start' => 6, 'end' => 8], which is #[Attribute].

Then inside getAfterOperator() it falls into an infinite loop, because getNextMeaningfulToken() returns the first token after returning the last one:

https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/9bf2760ce9d2218cc127cb4106e9ddfedca4eb3a/src/Fixer/Operator/TernaryToElvisOperatorFixer.php#L210-L219

Unfortunately, I don't know how to fix this, but I hope this will help you.

[kubawerlos]

@OskarStark, @derrabus, @jrmajor I've made a fix ☝🏼 - do not hesitate and drop a review there, please.

[OskarStark]

Please reopen this issue and see my comment in the referenced PR.

The PR which closed this issue is not solving the issue.

Thanks

[kubawerlos]

@OskarStark can you check if your problem is present in v2.18.5?

[OskarStark]

Sure, I did and it is not working:

[kubawerlos]

@OskarStark can you share the config or output when running with -v flag? I cannot reproduce it.

[OskarStark]

Sure, will come back tomorrow 👍

[OskarStark]

Running
symfony php vendor/bin/php-cs-fixer fix --config=.php_cs.dist --diff --diff-format=udiff -v

produces:

PHP CS Fixer 2.18.6 Remote Void by Fabien Potencier and Dariusz Ruminski
Runtime: PHP 8.0.3
Loaded config datana (PHP 7.4) from ".php_cs.dist".
Using cache file "/Users/oskar.stark/dev/datana/cockpit/.build/php-cs-fixer/.php_cs.cache".
SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSEESSSSSSSSESSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS
Legend: ?-unknown, I-invalid file syntax (file ignored), S-skipped (cached or empty file), .-no changes, F-fixed, E-error

Fixed all files in 0.126 seconds, 20.000 MB memory used

Files that were not fixed due to errors reported during linting after fixing:
   1) src/Controller/Registration/RegistrationController.php
   2) src/Controller/Registration/VerifyEmailController.php
   3) src/Controller/Security/Abo/ConfirmAboController.php
exit status 64

This is my config:

<?php

declare(strict_types=1);

use Datana\PhpCsFixer;

$config = PhpCsFixer\Config\Factory::fromRuleSet(new PhpCsFixer\Config\RuleSet\Php74(''), [
    'php_unit_internal_class' => false,
]);

$config->getFinder()
    ->in('build')
    ->in('migrations')
    ->in('src')
    ->in('tests')
    ->in('tools')
;

$config->setCacheFile(__DIR__ . '/.build/php-cs-fixer/.php_cs.cache');

return $config;

this is the ruleset:
https://gist.github.com/OskarStark/6a3c60f384e5a6b57e3958b02a7ae2df

Anything else I can do?
Thanks for your help

[keradus]

can you share minimum code sample that reproduces the issue? or it's the same as in first msg in this thread?

[OskarStark]

It is the same I mentioned

[kubawerlos]

Failing test (and soon hopefully a fix): #5665

[OskarStark]

Thanks