Skip to content

Conversation

@florisluiten
Copy link

Expected output and potential fix for issue with InvokableCommandInputAttributeRector

@TomasVotruba
Copy link
Member

@samsonasik Could you look into this?

@samsonasik
Copy link
Member

I will try

$executeClassMethod->params = array_merge(
array_filter($executeClassMethodParams, fn(Param $param) => is_null($param->default)),
array_filter($executeClassMethodParams, fn(Param $param) => !is_null($param->default)),
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@florisluiten this seems cause errors in tests fixture, see

https://github.com/rectorphp/rector-symfony/actions/runs/20064007047/job/57548394124?pr=899

Please verify other expected output fixtures, you can test locally:

composer install
vendor/bin/phpunit

@TomasVotruba
Copy link
Member

@florisluiten I've updated the rule, so it can keep correct attribute per line formatting. Hopefully it will fix the comma issue.
Can you rebase?

In PHP, optional parameters should be declared after required
parameters, otherwise they will be implicitly treated as required
parameters.
@florisluiten
Copy link
Author

Hi guys, thanks for helping me out here.

I rebased on master and cherry-picked the commits. However, the tests still fail with the trailing comma's. Did I perhaps do something wrong?

I did notice that in one situation, the comma between parameters is missing as well:

9) Rector\Symfony\Tests\Symfony73\Rector\Class_\InvokableCommandInputAttributeRector\InvokableCommandInputAttributeRectorTest::test with data set #13 ('/[...].php.inc
')                                                                                             
Failed on fixture file "with_multiple_arguments_options_fluent.php.inc"
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@                                                                                                                                                                                         
         #[\Symfony\Component\Console\Attribute\Argument(name: 'argument2', description: 'Argument2 description')]
         string $argument2,                                                                                                                                                                   
         #[\Symfony\Component\Console\Attribute\Option(name: 'option1', shortcut: 'o', mode: InputOption::VALUE_NONE, description: 'Option1 description')]
-        bool $option1 = false,
+        bool $option1 = false
         #[\Symfony\Component\Console\Attribute\Option(name: 'option2', shortcut: 'p', mode: InputOption::VALUE_NONE, description: 'Option2 description')]
-        bool $option2 = false     
+        bool $option2 = false,
     ): int                                                                                                                                                                                   
     {     

So I must conclude that my original thought that these comma's were automatically applied is incorrect. I'm now trying to read into how this all works, hoping that better understanding makes debugging easier.

@samsonasik
Copy link
Member

On the middle params, comma is needed. To debug, you probably needs to create temporary variable, the clear up params, then add one by one params on sort.

@florisluiten
Copy link
Author

Thanks for the pointers! I'm a couple of hours later into this thing and I still haven't figured out how to do this... I give up 😞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants